[Skiboot] [PATCH] Revert "astbmc: Try IPMI HIOMAP for P8"
Joel Stanley
joel at jms.id.au
Thu Feb 7 11:13:39 AEDT 2019
On Thu, 7 Feb 2019 at 10:41, Joel Stanley <joel at jms.id.au> 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>
The second one was supposed to read Alexey Kardashevskiy <aik at ozlabs.ru>.
> Signed-off-by: Joel Stanley <joel at jms.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