Re: Is the PEI Core MP Safe? UefiCpuPkg seems to think so calling GetFirstGuidHob on the APs?

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Andrew,
Good catch! It does break the rule that AP shouldn't call PEI services.
But considering the specific case, it should be safe.
Because:
1. HOB only stores a pointer to the buffer that contains all the MP information.
2. BSP modifies the HOB by only appending data to the end. It may modifies some HOB content. But MpInitLib
implementation itself doesn't modify the pointer value stored in HOB.

In PEI environment where global variable is read-only, it's hard to not rely on HOB.
I guess that's the reason of today's tricky implementation.

Thanks/Ray

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Andrew
> Fish
> Sent: Thursday, June 21, 2018 1:23 AM
> To: edk2-devel <edk2-devel@lists.01.org>
> Subject: [edk2] Is the PEI Core MP Safe? UefiCpuPkg seems to think so calling
> GetFirstGuidHob on the APs?
> 
> Is there some MP safe contract with the PEI Core implementation I don't
> know about?
> 
> Looks like the APs are calling PeiServicesGetHobList() to figure out "who they
> are"? How does this work? Or am I missing something?
> 
> 
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpIni
> tLib/MpLib.c#L1945
> 
> /**
>   This return the handle number for the calling processor.  This service may be
>   called from the BSP and APs.
>   @param[out] ProcessorNumber  Pointer to the handle number of AP.
>                                The range is from 0 to the total number of
>                                logical processors minus 1. The total number of
>                                logical processors can be retrieved by
>                                MpInitLibGetNumberOfProcessors().
>   @retval EFI_SUCCESS             The current processor handle number was
> returned
>                                   in ProcessorNumber.
>   @retval EFI_INVALID_PARAMETER   ProcessorNumber is NULL.
>   @retval EFI_NOT_READY           MP Initialize Library is not initialized.
> **/
> EFI_STATUS
> EFIAPI
> MpInitLibWhoAmI (
>   OUT UINTN                    *ProcessorNumber
>   )
> {
>   CPU_MP_DATA           *CpuMpData;
> 
>   if (ProcessorNumber == NULL) {
>     return EFI_INVALID_PARAMETER;
>   }
> 
>   CpuMpData = GetCpuMpData ();
> 
>   return GetProcessorNumber (CpuMpData, ProcessorNumber); }
> 
> 
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpIni
> tLib/PeiMpLib.c#L34
> 
> /**
>   Get pointer to CPU MP Data structure.
>   @return  The pointer to CPU MP Data structure.
> **/
> CPU_MP_DATA *
> GetCpuMpData (
>   VOID
>   )
> {
>   CPU_MP_DATA      *CpuMpData;
> 
>   CpuMpData = GetCpuMpDataFromGuidedHob ();
>   ASSERT (CpuMpData != NULL);
>   return CpuMpData;
> }
> 
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpIni
> tLib/MpLib.c#L2302
> 
> 
> /**
>   Get pointer to CPU MP Data structure from GUIDed HOB.
>   @return  The pointer to CPU MP Data structure.
> **/
> CPU_MP_DATA *
> GetCpuMpDataFromGuidedHob (
>   VOID
>   )
> {
>   EFI_HOB_GUID_TYPE       *GuidHob;
>   VOID                    *DataInHob;
>   CPU_MP_DATA             *CpuMpData;
> 
>   CpuMpData = NULL;
>   GuidHob = GetFirstGuidHob (&mCpuInitMpLibHobGuid);
>   if (GuidHob != NULL) {
>     DataInHob = GET_GUID_HOB_DATA (GuidHob);
>     CpuMpData = (CPU_MP_DATA *) (*(UINTN *) DataInHob);
>   }
>   return CpuMpData;
> }
> 
> Thanks,
> 
> Andrew Fish
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel