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

Alexey Kardashevskiy aik at ozlabs.ru
Thu Feb 7 16:21:47 AEDT 2019



On 07/02/2019 11:13, Joel Stanley wrote:
> 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>.

Whatever :) This fixes garrison2 for me.

Tested-by: 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
>>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
> 

-- 
Alexey


More information about the Skiboot mailing list