[Skiboot] [PATCH 2/2] astbmc: Try IPMI HIOMAP for P8 (again)

Andrew Jeffery andrew at aj.id.au
Thu Feb 21 15:50:23 AEDT 2019


The HIOMAP protocol was developed after the release of P8 in preparation
for P9. As a consequence P9 always uses it, but it has rarely been
enabled for P8. P8DTU has recently added IPMI HIOMAP support to its BMC
firmware, so enable its use in skiboot with P8 machines. Doing so
requires some rework to ensure fallback works correctly as in the past
the fallback was to mbox, which will only work for P9.

Tested on Garrison, Palmetto without HIOMAP, Palmetto with HIOMAP, and
Witherspoon.

Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
---
 hw/ast-bmc/ast-io.c       |  7 ++++-
 include/ast.h             |  3 ++-
 platforms/astbmc/common.c | 36 +++++++++++++++++++++-----
 platforms/astbmc/pnor.c   | 54 ++++++++++++++++++++++-----------------
 4 files changed, 67 insertions(+), 33 deletions(-)

diff --git a/hw/ast-bmc/ast-io.c b/hw/ast-bmc/ast-io.c
index a9da06b24057..472b15172471 100644
--- a/hw/ast-bmc/ast-io.c
+++ b/hw/ast-bmc/ast-io.c
@@ -381,7 +381,12 @@ bool ast_io_init(void)
 	return ast_io_is_rw();
 }
 
-bool ast_lpc_fw_needs_hiomap(void)
+bool ast_lpc_fw_ipmi_hiomap(void)
+{
+	return platform.bmc->sw->ipmi_oem_hiomap_cmd != 0;
+}
+
+bool ast_lpc_fw_mbox_hiomap(void)
 {
 	struct dt_node *n;
 
diff --git a/include/ast.h b/include/ast.h
index c6684fc88265..5c46cf261e1c 100644
--- a/include/ast.h
+++ b/include/ast.h
@@ -86,7 +86,8 @@ bool ast_sio_init(void);
 bool ast_io_init(void);
 bool ast_io_is_rw(void);
 bool ast_lpc_fw_maps_flash(void);
-bool ast_lpc_fw_needs_hiomap(void);
+bool ast_lpc_fw_ipmi_hiomap(void);
+bool ast_lpc_fw_mbox_hiomap(void);
 bool ast_scratch_reg_is_mbox(void);
 
 /* UART configuration */
diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
index 210b3ec29b52..bc0e58f88e59 100644
--- a/platforms/astbmc/common.c
+++ b/platforms/astbmc/common.c
@@ -208,8 +208,15 @@ static void astbmc_fixup_dt_mbox(struct dt_node *lpc)
 	struct dt_node *mbox;
 	char namebuf[32];
 
-	/* All P9 machines use mbox. P8 machines can indicate they support
-	 * it using the scratch register */
+	if (!lpc)
+		return;
+
+	/*
+	 * P9 machines always use hiomap, either by ipmi or mbox. P8 machines
+	 * can indicate they support mbox using the scratch register, or ipmi
+	 * by configuring the hiomap ipmi command. If neither are configured
+	 * for P8 then skiboot will drive the flash controller directly.
+	 */
 	if (proc_gen != proc_gen_p9 && !ast_scratch_reg_is_mbox())
 		return;
 
@@ -310,7 +317,7 @@ static void astbmc_fixup_bmc_sensors(void)
 	}
 }
 
-static void astbmc_fixup_dt(void)
+static struct dt_node *dt_find_primary_lpc(void)
 {
 	struct dt_node *n, *primary_lpc = NULL;
 
@@ -328,6 +335,15 @@ static void astbmc_fixup_dt(void)
 			break;
 	}
 
+	return primary_lpc;
+}
+
+static void astbmc_fixup_dt(void)
+{
+	struct dt_node *primary_lpc;
+
+	primary_lpc = dt_find_primary_lpc();
+
 	if (!primary_lpc)
 		return;
 
@@ -337,9 +353,6 @@ static void astbmc_fixup_dt(void)
 	/* BT is not in HB either */
 	astbmc_fixup_dt_bt(primary_lpc);
 
-	/* MBOX is not in HB */
-	astbmc_fixup_dt_mbox(primary_lpc);
-
 	/* The pel logging code needs a system-id property to work so
 	   make sure we have one. */
 	astbmc_fixup_dt_system_id();
@@ -412,7 +425,16 @@ void astbmc_early_init(void)
 		} else
 			prerror("PLAT: AST IO initialisation failed!\n");
 
-		ast_setup_sio_mbox(MBOX_IO_BASE, MBOX_LPC_IRQ);
+		/*
+		 * P9 prefers IPMI for HIOMAP but will use MBOX if IPMI is not
+		 * supported. P8 either uses IPMI HIOMAP or direct IO, and
+		 * never MBOX. Thus only populate the MBOX node on P9 to allow
+		 * fallback.
+		 */
+		if (proc_gen == proc_gen_p9) {
+			astbmc_fixup_dt_mbox(dt_find_primary_lpc());
+			ast_setup_sio_mbox(MBOX_IO_BASE, MBOX_LPC_IRQ);
+		}
 	} else {
 		/*
 		 * This may or may not be an error depending on if we set up
diff --git a/platforms/astbmc/pnor.c b/platforms/astbmc/pnor.c
index d2694768e330..1c364351e065 100644
--- a/platforms/astbmc/pnor.c
+++ b/platforms/astbmc/pnor.c
@@ -34,48 +34,54 @@ enum ast_flash_style {
     mbox_hiomap,
 };
 
+static enum ast_flash_style ast_flash_get_fallback_style(void)
+{
+    if (ast_lpc_fw_mbox_hiomap())
+	return mbox_hiomap;
+
+    if (ast_lpc_fw_maps_flash())
+	return raw_flash;
+
+    return raw_mem;
+}
+
 int pnor_init(void)
 {
 	struct spi_flash_ctrl *pnor_ctrl = NULL;
 	struct blocklevel_device *bl = NULL;
 	enum ast_flash_style style;
-	int rc;
+	int rc = 0;
 
-	if (ast_lpc_fw_needs_hiomap()) {
+	if (ast_lpc_fw_ipmi_hiomap()) {
 		style = ipmi_hiomap;
 		rc = ipmi_hiomap_init(&bl);
-		if (rc) {
-			style = mbox_hiomap;
+	}
+
+	if (!ast_lpc_fw_ipmi_hiomap() || rc) {
+		if (!ast_sio_is_enabled())
+			return -ENODEV;
+
+		style = ast_flash_get_fallback_style();
+		if (style == mbox_hiomap)
 			rc = mbox_flash_init(&bl);
-		}
-	} else {
-		/* Open controller and flash. If the LPC->AHB doesn't point to
-		 * the PNOR flash base we assume we're booting from BMC system
-		 * memory (or some other place setup by the BMC to support LPC
-		 * FW reads & writes).
-		 */
-
-		if (ast_lpc_fw_maps_flash()) {
-			style = raw_flash;
+		else if (style == raw_flash)
 			rc = ast_sf_open(AST_SF_TYPE_PNOR, &pnor_ctrl);
-		} else {
-			printf("PLAT: Memboot detected\n");
-			style = raw_mem;
+		else if (style == raw_mem)
 			rc = ast_sf_open(AST_SF_TYPE_MEM, &pnor_ctrl);
+		else {
+			prerror("Unhandled flash mode: %d\n", style);
+			return -ENODEV;
 		}
-		if (rc) {
-			prerror("PLAT: Failed to open PNOR flash controller\n");
-			goto fail;
-		}
-
-		rc = flash_init(pnor_ctrl, &bl, NULL);
 	}
 
 	if (rc) {
-		prerror("PLAT: Failed to open init PNOR driver\n");
+		prerror("PLAT: Failed to init PNOR driver\n");
 		goto fail;
 	}
 
+	if (style == raw_flash || style == raw_mem)
+	    rc = flash_init(pnor_ctrl, &bl, NULL);
+
 	rc = flash_register(bl);
 	if (!rc)
 		return 0;
-- 
2.19.1



More information about the Skiboot mailing list