| Message ID | 20190207001141.31275-1-joel@jms.id.au |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Revert "astbmc: Try IPMI HIOMAP for P8" | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
On Thu, 7 Feb 2019 at 10:41, Joel Stanley <joel@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@mendozajonas.com> > Reported-by: Sam Mendoza-Jonas <sam@mendozajonas.com> The second one was supposed to read Alexey Kardashevskiy <aik@ozlabs.ru>. > Signed-off-by: Joel Stanley <joel@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 >
On Thu, 2019-02-07 at 10:41 +1030, 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@mendozajonas.com> > Reported-by: Sam Mendoza-Jonas <sam@mendozajonas.com> > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- Acked-by: Sam Mendoza-Jonas <sam@mendozajonas.com> Acked-by: Sam Mendoza-Jonas <sam@mendozajonas.com> > 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;
On 07/02/2019 11:13, Joel Stanley wrote: > On Thu, 7 Feb 2019 at 10:41, Joel Stanley <joel@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@mendozajonas.com> >> Reported-by: Sam Mendoza-Jonas <sam@mendozajonas.com> > > The second one was supposed to read Alexey Kardashevskiy <aik@ozlabs.ru>. Whatever :) This fixes garrison2 for me. Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru> > >> Signed-off-by: Joel Stanley <joel@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 >> > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot >
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@mendozajonas.com> > Reported-by: Sam Mendoza-Jonas <sam@mendozajonas.com> > Signed-off-by: Joel Stanley <joel@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@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 > >
Joel Stanley <joel@jms.id.au> writes: > 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@mendozajonas.com> > Reported-by: Sam Mendoza-Jonas <sam@mendozajonas.com> > Signed-off-by: Joel Stanley <joel@jms.id.au> Cheers, merged to master as of 23470f10d0b1e120dc2d2f1606444fb6fc07b56 Sorry for being poor on the testing on my end.
On Mon, 11 Feb 2019, at 17:22, Stewart Smith wrote: > Joel Stanley <joel@jms.id.au> writes: > > 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@mendozajonas.com> > > Reported-by: Sam Mendoza-Jonas <sam@mendozajonas.com> > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > Cheers, merged to master as of 23470f10d0b1e120dc2d2f1606444fb6fc07b56 > > Sorry for being poor on the testing on my end. Ugh, me to. Andrew
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;
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@mendozajonas.com> Reported-by: Sam Mendoza-Jonas <sam@mendozajonas.com> Signed-off-by: Joel Stanley <joel@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(-)