[Skiboot] [PATCH v3 4/4] API to verify the STOP API and image compatibility

Oliver O'Halloran oohall at gmail.com
Mon Feb 10 12:38:43 AEDT 2020


On Fri, Feb 7, 2020 at 3:21 PM Pratik Rajesh Sampat
<psampat at linux.ibm.com> wrote:
>
> From: Prem Shanker Jha <premjha2 at in.ibm.com>
>
> Commit defines a new API primarily intended for OPAL to determine
> cpu register save API's compatibility with HOMER layout and
> self save restore. It can help OPAL determine if version of
> API integrated with OPAL is different from hostboot.
>
> This is used in the context of identifying support for self save in the
> firmware. In the case the version of the firmware is older then we
> simply cut support for the feature in the device tree rather than
> catching the failure when the call is made.
>
> Change-Id: Ic0de45a336cfb8b6b6096a10ac1cd3ffbaa44fc0
> Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/77612
> Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot at us.ibm.com>
> Tested-by: Jenkins Server <pfd-jenkins+hostboot at us.ibm.com>
> Tested-by: Hostboot CI <hostboot-ci+hostboot at us.ibm.com>
> Reviewed-by: RANGANATHPRASAD G. BRAHMASAMUDRA <prasadbgr at in.ibm.com>
> Reviewed-by: Gregory S Still <stillgs at us.ibm.com>
> Reviewed-by: Jennifer A Stofer <stofer at us.ibm.com>
> Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/77614
> Tested-by: Jenkins OP Build CI <op-jenkins+hostboot at us.ibm.com>
> Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot at us.ibm.com>
> Reviewed-by: Daniel M Crowell <dcrowell at us.ibm.com>
> Signed-off-by: Pratik Rajesh Sampat <psampat at linux.ibm.com>
>
> Co-authored-by: Prem Shanker Jha <premjha2 at in.ibm.com>
> Co-authored-by: Pratik Rajesh Sampat <psampat at linux.ibm.com>

There is a bisect hazard between patch 3 (where self-save is added)
and patch 4 (where we check if it's supported). IMO, best way to
resolve this would be:

1. Take the slw.c changes out of this patch and move them into the current 3/4.
2. Re-order 3/4 and 4/4 so we add support for checking for support
before we add code to use it.


> ---
>  hw/slw.c                                 | 22 ++++++++-
>  include/p9_stop_api.H                    | 26 ++++++++++
>  libpore/p9_cpu_reg_restore_instruction.H |  7 ++-
>  libpore/p9_hcd_memmap_base.H             |  7 +++
>  libpore/p9_stop_api.C                    | 60 +++++++++++++++++++++++-
>  libpore/p9_stop_api.H                    | 26 +++++++++-
>  6 files changed, 143 insertions(+), 5 deletions(-)
>
> diff --git a/hw/slw.c b/hw/slw.c
> index 087a8619..95ba8dcd 100644
> --- a/hw/slw.c
> +++ b/hw/slw.c
> @@ -756,9 +756,12 @@ static void slw_late_init_p9(struct proc_chip *chip)
>  /* Add device tree properties to determine self-save | restore */
>  void add_cpu_self_save_properties()
>  {
> -       int i;
> +       int i, rc;
> +       bool self_save_supported = true;
>         struct dt_node *self_restore, *self_save, *power_mgt;
>         bitmap_t *self_restore_map, *self_save_map;
> +       uint64_t compVector = -1;
> +       struct proc_chip *chip;
>
>         const uint64_t self_restore_regs[] = {
>                 P8_SPR_HRMOR,
> @@ -791,6 +794,21 @@ void add_cpu_self_save_properties()
>         self_save_map = zalloc(BITMAP_BYTES(SPR_BITMAP_LENGTH));
>         self_restore_map = zalloc(BITMAP_BYTES(SPR_BITMAP_LENGTH));
>
> +       chip = next_chip(NULL);
> +       assert(chip);
> +       rc = proc_stop_api_discover_capability((void *) chip->homer_base,
> +                                              &compVector);
> +       if (rc == STOP_SAVE_ARG_INVALID_IMG) {
> +               prlog(PR_DEBUG, "HOMER BASE INVALID\n");
> +               goto bail;
> +       } else if (rc == STOP_SAVE_API_IMG_INCOMPATIBLE) {
> +               prlog(PR_DEBUG, "STOP API running incompatible versions\n");
> +               if ((compVector & SELF_RESTORE_VER_MISMATCH) == 0) {
> +                       prlog(PR_DEBUG, "Self-save API unsupported\n");
> +                       self_save_supported = false;
> +               }
> +       }
> +
>         for (i = 0; i < ARRAY_SIZE(self_save_regs); i++)
>                 bitmap_set_bit(*self_save_map, self_save_regs[i]);
>
> @@ -818,7 +836,7 @@ void add_cpu_self_save_properties()
>                 prerror("OCC: Failed to create self save node");
>                 goto bail;
>         }
> -       if (proc_gen == proc_gen_p9) {
> +       if (proc_gen == proc_gen_p9 && self_save_supported) {
>                 dt_add_property_string(self_save, "status", "enabled");
>
>                 dt_add_property(self_save, "sprn-bitmask", *self_save_map,
> diff --git a/include/p9_stop_api.H b/include/p9_stop_api.H
> index c304f70f..09ce3dc1 100644
> --- a/include/p9_stop_api.H
> +++ b/include/p9_stop_api.H
> @@ -110,6 +110,7 @@ typedef enum
>      STOP_SAVE_FAIL                       = 14,  // for internal failure within firmware.
>      STOP_SAVE_SPR_ENTRY_MISSING          =  15,
>      STOP_SAVE_SPR_BIT_POS_RESERVE        =  16,
> +    STOP_SAVE_API_IMG_INCOMPATIBLE       =  18,
>  } StopReturnCode_t;
>
>  /**
> @@ -164,6 +165,14 @@ typedef enum
>
>  } ScomSection_t;
>
> +/**
> + * @brief   versions pertaining relvant to STOP API.
> + */
> +typedef enum
> +{
> +    STOP_API_VER            =   0x00,
> +    STOP_API_VER_CONTROL    =   0x02,
> +} VersionList_t;
>
>
>  /**
> @@ -195,6 +204,14 @@ typedef enum
>      BIT_POS_USPRG1      =   30,
>  } SprBitPositionList_t;
>
> +typedef enum
> +{
> +    SMF_SUPPORT_MISSING_IN_HOMER         =   0x01,
> +    SELF_SUPPORT_MISSING_FOR_LE_HYP      =   0x02,
> +    IPL_RUNTIME_CPU_SAVE_VER_MISMATCH    =   0x04,
> +    SELF_RESTORE_VER_MISMATCH            =   0x08,
> +} VersionIncompList_t;
> +
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
> @@ -247,6 +264,15 @@ StopReturnCode_t p9_stop_save_scom( void* const   i_pImage,
>  StopReturnCode_t
>  p9_stop_save_cpureg_control( void* i_pImage, const uint64_t i_pir,
>                               const uint32_t  i_saveRegVector );
> +
> +/**
> + * @brief       verifies if API is compatible of current HOMER image.
> + * @param[in]   i_pImage        points to the start of HOMER image of P9 chip.
> + * @param[out]  o_inCompVector  list of incompatibilities found.
> + * @return      STOP_SAVE_SUCCESS if if API succeeds, error code otherwise.
> + */
> +StopReturnCode_t proc_stop_api_discover_capability( void* const i_pImage, uint64_t* o_inCompVector );
> +
>  #ifdef __cplusplus
>  } // extern "C"
>  };  // namespace stopImageSection ends
> diff --git a/libpore/p9_cpu_reg_restore_instruction.H b/libpore/p9_cpu_reg_restore_instruction.H
> index d69a4212..5f168855 100644
> --- a/libpore/p9_cpu_reg_restore_instruction.H
> +++ b/libpore/p9_cpu_reg_restore_instruction.H
> @@ -5,7 +5,7 @@
>  /*                                                                        */
>  /* OpenPOWER HostBoot Project                                             */
>  /*                                                                        */
> -/* Contributors Listed Below - COPYRIGHT 2015,2018                        */
> +/* Contributors Listed Below - COPYRIGHT 2015,2020                        */
>  /* [+] International Business Machines Corp.                              */
>  /*                                                                        */
>  /*                                                                        */
> @@ -69,6 +69,11 @@ enum
>      OPCODE_18           =   18,
>      SELF_SAVE_FUNC_ADD  =   0x2300,
>      SELF_SAVE_OFFSET    =   0x180,
> +    SKIP_SPR_REST_INST  =   0x4800001c, //b . +0x01c
> +    MFLR_R30            =   0x7fc802a6,
> +    SKIP_SPR_SELF_SAVE  =   0x3bff0020, //addi r31 r31, 0x20
> +    MTLR_INST           =   0x7fc803a6,  //mtlr r30
> +    BRANCH_BE_INST      =   0x48000020,
>  };
>
>  #define MR_R0_TO_R10            0x7c0a0378UL //mr r10 r0
> diff --git a/libpore/p9_hcd_memmap_base.H b/libpore/p9_hcd_memmap_base.H
> index 000fafef..ddb56728 100644
> --- a/libpore/p9_hcd_memmap_base.H
> +++ b/libpore/p9_hcd_memmap_base.H
> @@ -444,6 +444,13 @@ HCD_CONST(CME_QUAD_PSTATE_SIZE,                 HALF_KB)
>
>  HCD_CONST(CME_REGION_SIZE,                      (64 * ONE_KB))
>
> +
> +// HOMER compatibility
> +
> +HCD_CONST(STOP_API_CPU_SAVE_VER,                0x02)
> +HCD_CONST(SELF_SAVE_RESTORE_VER,                0x02)
> +HCD_CONST(SMF_SUPPORT_SIGNATURE_OFFSET,         0x1300)
> +HCD_CONST(SMF_SELF_SIGNATURE,                   (0x5f534d46))
>  // Debug
>
>  HCD_CONST(CPMR_TRACE_REGION_OFFSET,             (512 * ONE_KB))
> diff --git a/libpore/p9_stop_api.C b/libpore/p9_stop_api.C
> index 2d9bb549..74341cff 100644
> --- a/libpore/p9_stop_api.C
> +++ b/libpore/p9_stop_api.C
> @@ -5,7 +5,7 @@
>  /*                                                                        */
>  /* OpenPOWER HostBoot Project                                             */
>  /*                                                                        */
> -/* Contributors Listed Below - COPYRIGHT 2015,2018                        */
> +/* Contributors Listed Below - COPYRIGHT 2015,2020                        */
>  /* [+] International Business Machines Corp.                              */
>  /*                                                                        */
>  /*                                                                        */
> @@ -1828,6 +1828,64 @@ StopReturnCode_t proc_stop_init_self_save(  void* const i_pImage, const uint32_t
>      return l_rc;
>  }
>
> +StopReturnCode_t proc_stop_api_discover_capability( void* const i_pImage, uint64_t * o_inCompVector )
> +{
> +    StopReturnCode_t l_rc       =   STOP_SAVE_SUCCESS;
> +    uint64_t l_incompVector     =   0;
> +    uint32_t l_tempWord         =   0;
> +    *o_inCompVector             =   0;
> +
> +    do
> +    {
> +        if( !i_pImage )
> +        {
> +            l_rc    =   STOP_SAVE_ARG_INVALID_IMG;
> +            break;
> +        }
> +
> +        l_tempWord      =
> +                *(uint32_t*)((uint8_t*)i_pImage + CPMR_HOMER_OFFSET + SMF_SUPPORT_SIGNATURE_OFFSET);
> +
> +        if( l_tempWord != SWIZZLE_4_BYTE(SMF_SELF_SIGNATURE) )
> +        {
> +            l_incompVector  |=  SMF_SUPPORT_MISSING_IN_HOMER;
> +        }
> +
> +        l_tempWord      =   *(uint32_t *)((uint8_t *)i_pImage + CPMR_HOMER_OFFSET + CPMR_HEADER_SIZE );
> +
> +        if( l_tempWord != SWIZZLE_4_BYTE(BRANCH_BE_INST) )
> +        {
> +            l_incompVector  |=  SELF_SUPPORT_MISSING_FOR_LE_HYP;
> +        }
> +
> +        l_tempWord      =   *(uint8_t *)((uint8_t *)i_pImage + CPMR_HOMER_OFFSET + CPMR_SELF_RESTORE_VER_BYTE );
> +
> +        prlog(PR_EMERG, "SLW: self save ltemp: 0x%x\n", l_tempWord);
That should be PR_DEBUG at most.

> +        if( l_tempWord < SELF_SAVE_RESTORE_VER )
> +        {
> +            l_incompVector  |=  SELF_RESTORE_VER_MISMATCH;
> +        }
> +
> +        l_tempWord      =   *(uint8_t *)((uint8_t *)i_pImage + CPMR_HOMER_OFFSET + CPMR_STOP_API_VER_BYTE );
> +
> +        if( l_tempWord < STOP_API_CPU_SAVE_VER )
> +        {
> +            l_incompVector  |=  IPL_RUNTIME_CPU_SAVE_VER_MISMATCH;
> +        }
> +
> +        prlog(PR_EMERG, "SLW: Comp: 0x%llx\n", l_incompVector);
Same here.

> +        *o_inCompVector     =   l_incompVector;
> +
> +        if( l_incompVector )
> +        {
> +            l_rc    =  STOP_SAVE_API_IMG_INCOMPATIBLE;
> +        }
> +
> +    }while(0);
> +
> +    return l_rc;
> +}
> +
>  #ifdef __cplusplus
>  } //namespace stopImageSection ends
>
> diff --git a/libpore/p9_stop_api.H b/libpore/p9_stop_api.H
> index 3f6420ff..983a3845 100644
> --- a/libpore/p9_stop_api.H
> +++ b/libpore/p9_stop_api.H
> @@ -5,7 +5,7 @@
>  /*                                                                        */
>  /* OpenPOWER HostBoot Project                                             */
>  /*                                                                        */
> -/* Contributors Listed Below - COPYRIGHT 2015,2018                        */
> +/* Contributors Listed Below - COPYRIGHT 2015,2020                        */
>  /* [+] International Business Machines Corp.                              */
>  /*                                                                        */
>  /*                                                                        */
> @@ -114,6 +114,7 @@ typedef enum
>      STOP_SAVE_FAIL                       =  14,  // for internal failure within firmware.
>      STOP_SAVE_SPR_ENTRY_MISSING          =  15,
>      STOP_SAVE_SPR_BIT_POS_RESERVE        =  16,
> +    STOP_SAVE_API_IMG_INCOMPATIBLE       =  18,
>  } StopReturnCode_t;
>
>  /**
> @@ -198,6 +199,21 @@ typedef enum
>      BIT_POS_USPRG1      =   30,
>  } SprBitPositionList_t;
>
> +/**
> + * @brief   List of major incompatibilities between API version.
> + * @note    STOP APIs assumes a specific HOMER layout, certain
> + * level of CME-SGPE hcode and certain version of self-save restore
> + * binary. A mismatch can break STOP function.
> + */
> +
> +typedef enum
> +{
> +    SMF_SUPPORT_MISSING_IN_HOMER         =   0x01,
> +    SELF_SUPPORT_MISSING_FOR_LE_HYP      =   0x02,
> +    IPL_RUNTIME_CPU_SAVE_VER_MISMATCH    =   0x04,
> +    SELF_RESTORE_VER_MISMATCH            =   0x08,
> +} VersionIncompList_t;
> +
>
>  #ifdef __cplusplus
>  extern "C" {
> @@ -341,6 +357,14 @@ StopReturnCode_t proc_stop_save_cpureg(  void* const i_pImage,
>   */
>  StopReturnCode_t proc_stop_init_self_save(  void* const i_pImage, const uint32_t i_corePos );
>
> +/**
> + * @brief       verifies if API is compatible of current HOMER image.
> + * @param[in]   i_pImage        points to the start of HOMER image of P9 chip.
> + * @param[out]  o_inCompVector  list of incompatibilities found.
> + * @return      STOP_SAVE_SUCCESS if if API succeeds, error code otherwise.
> + */
> +StopReturnCode_t proc_stop_api_discover_capability( void* const i_pImage, uint64_t* o_inCompVector );
> +
>  #ifdef __cplusplus
>  } // extern "C"
>  };  // namespace stopImageSection ends
> --
> 2.24.1
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot


More information about the Skiboot mailing list