[Skiboot] [PATCH] Revert "astbmc: Try IPMI HIOMAP for P8"

Andrew Jeffery andrew at aj.id.au
Thu Feb 7 19:42:23 AEDT 2019



On Thu, 7 Feb 2019, at 10:41, Joel Stanley wrote:
> This reverts commit bd9839684d482417e8c60449592f4308e9a91dac as it broke
> booting on P8 systems, including Garrison (AMI BMC), Firestone (AMI BMC)
> and QEMU (BMC simulator).
> 
> Issue https://github.com/open-power/skiboot/issues/217 tracks the
> failure. The P8 IPMI HIOMAP feature can be re-enabled once this issue is
> resolved.
> 
> Reported-by: Sam Mendoza-Jonas <sam at mendozajonas.com>
> Reported-by: Sam Mendoza-Jonas <sam at mendozajonas.com>
> Signed-off-by: Joel Stanley <joel at jms.id.au>

Whoops! I'll fix this when I'm back next week. I promise I tested it, but maybe not with the right configurations. Sorry for the headaches.

Acked-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, 33 insertions(+), 67 deletions(-)
> 
> diff --git a/hw/ast-bmc/ast-io.c b/hw/ast-bmc/ast-io.c
> index 11cc084c1b62..38ca86c46003 100644
> --- a/hw/ast-bmc/ast-io.c
> +++ b/hw/ast-bmc/ast-io.c
> @@ -360,12 +360,7 @@ bool ast_io_init(void)
>  	return ast_io_is_rw();
>  }
>  
> -bool ast_lpc_fw_ipmi_hiomap(void)
> -{
> -	return platform.bmc->sw->ipmi_oem_hiomap_cmd != 0;
> -}
> -
> -bool ast_lpc_fw_mbox_hiomap(void)
> +bool ast_lpc_fw_needs_hiomap(void)
>  {
>  	struct dt_node *n;
>  
> diff --git a/include/ast.h b/include/ast.h
> index 5c46cf261e1c..c6684fc88265 100644
> --- a/include/ast.h
> +++ b/include/ast.h
> @@ -86,8 +86,7 @@ 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_ipmi_hiomap(void);
> -bool ast_lpc_fw_mbox_hiomap(void);
> +bool ast_lpc_fw_needs_hiomap(void);
>  bool ast_scratch_reg_is_mbox(void);
>  
>  /* UART configuration */
> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
> index bc0e58f88e59..210b3ec29b52 100644
> --- a/platforms/astbmc/common.c
> +++ b/platforms/astbmc/common.c
> @@ -208,15 +208,8 @@ static void astbmc_fixup_dt_mbox(struct dt_node *lpc)
>  	struct dt_node *mbox;
>  	char namebuf[32];
>  
> -	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.
> -	 */
> +	/* All P9 machines use mbox. P8 machines can indicate they support
> +	 * it using the scratch register */
>  	if (proc_gen != proc_gen_p9 && !ast_scratch_reg_is_mbox())
>  		return;
>  
> @@ -317,7 +310,7 @@ static void astbmc_fixup_bmc_sensors(void)
>  	}
>  }
>  
> -static struct dt_node *dt_find_primary_lpc(void)
> +static void astbmc_fixup_dt(void)
>  {
>  	struct dt_node *n, *primary_lpc = NULL;
>  
> @@ -335,15 +328,6 @@ static struct dt_node *dt_find_primary_lpc(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;
>  
> @@ -353,6 +337,9 @@ 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();
> @@ -425,16 +412,7 @@ void astbmc_early_init(void)
>  		} else
>  			prerror("PLAT: AST IO initialisation failed!\n");
>  
> -		/*
> -		 * 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);
> -		}
> +		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 1c364351e065..d2694768e330 100644
> --- a/platforms/astbmc/pnor.c
> +++ b/platforms/astbmc/pnor.c
> @@ -34,54 +34,48 @@ 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 = 0;
> +	int rc;
>  
> -	if (ast_lpc_fw_ipmi_hiomap()) {
> +	if (ast_lpc_fw_needs_hiomap()) {
>  		style = ipmi_hiomap;
>  		rc = ipmi_hiomap_init(&bl);
> -	}
> -
> -	if (!ast_lpc_fw_ipmi_hiomap() || rc) {
> -		if (!ast_sio_is_enabled())
> -			return -ENODEV;
> -
> -		style = ast_flash_get_fallback_style();
> -		if (style == mbox_hiomap)
> +		if (rc) {
> +			style = mbox_hiomap;
>  			rc = mbox_flash_init(&bl);
> -		else if (style == raw_flash)
> +		}
> +	} 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;
>  			rc = ast_sf_open(AST_SF_TYPE_PNOR, &pnor_ctrl);
> -		else if (style == raw_mem)
> +		} else {
> +			printf("PLAT: Memboot detected\n");
> +			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 init PNOR driver\n");
> +		prerror("PLAT: Failed to open 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.20.1
> 
>


More information about the Skiboot mailing list