Re: [PATCH] UefiCpuPkg/MpInitLib: AP uses memory preceding IDT to store CpuMpData

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

On 6/26/2018 1:01 AM, Laszlo Ersek wrote:
> On 06/25/18 18:01, Laszlo Ersek wrote:
>> Hello Ray,
>>
>> On 06/25/18 04:54, Ruiyu Ni wrote:
>>> Today's MpInitLib PEI implementation directly calls
>>> PeiServices->GetHobList() from AP which may cause racing issue.
>>>
>>> This patch fixes this issue by storing the CpuMpData to memory
>>> preceding IDT. Pointer to PeiServices pointer is stored there,
>>> so after AP procedure returns, the PeiServices pointer should be
>>> restored.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
>>> Cc: Jeff Fan <vanjeff_919@hotmail.com>
>>> Cc: Eric Dong <eric.dong@intel.com>
>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>> Cc: Fish Andrew <afish@apple.com>
>>> ---
>>>   UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 33 ++++++++++++++++++-
>>>   UefiCpuPkg/Library/MpInitLib/MpLib.c    |  8 +++++
>>>   UefiCpuPkg/Library/MpInitLib/MpLib.h    | 27 +++++++++++++++-
>>>   UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 56 +++++++++++++++++++++++++++++++--
>>>   4 files changed, 119 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>>> index e7ed21c6cd..26fead2c66 100644
>>> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>>> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>>
>> [snip]
>>
>>> +/**
>>> +  Pop the CpuMpData.
>>> +
>>> +  @param[in] Pointer  The pointer value which was stored in where the CPU MP Data is pushed.
>>> +  @param[in] Context  The context of push/pop operation.
>>> +**/
>>> +VOID
>>> +PopCpuMpData (
>>> +  IN VOID           *Pointer,
>>> +  IN VOID           *Context
>>> +  )
>>> +{
>>
>> (1) When I applied this patch for regression-testing, "git am"
>> complained that the above line added two trailing space characters. Can
>> you please remove them?
>>
>> (2) Other than that, I tested this patch with OVMF -- the patch breaks
>> OVMF. The last part of the log is:
>>
>>> Loading PEIM [CpuMpPei]
>>> Loading PEIM at 0x000BFE81000 EntryPoint=0x000BFE86DC6 CpuMpPei.efi
>>> AP Loop Mode is 1
>>> WakeupBufferStart = 9F000, WakeupBufferSize = 1000
>>> TimedWaitForApFinish: reached FinishedApLimit=7 in 121 microseconds
>>> APIC MODE is 1
>>> MpInitLib: Find 8 processors in system.
>>> Does not find any stored CPU BIST information from PPI!
>>>    APICID - 0x00000000, BIST - 0x00000000
>>>    APICID - 0x00000001, BIST - 0x00000000
>>>    APICID - 0x00000002, BIST - 0x00000000
>>>    APICID - 0x00000003, BIST - 0x00000000
>>>    APICID - 0x00000004, BIST - 0x00000000
>>>    APICID - 0x00000005, BIST - 0x00000000
>>>    APICID - 0x00000006, BIST - 0x00000000
>>>    APICID - 0x00000007, BIST - 0x00000000
>>> Install PPI: [EfiSecPlatformInformation2Ppi]
>>> Install PPI: [EfiPeiMpServicesPpi]
>>> Notify: PPI Guid: [EfiPeiMpServicesPpi], Peim notify entry point: 8524F8
>>> PlatformPei: OnMpServicesAvailable
> 
> Sorry, I failed to mention: after the last line is printed, the boot
> process is stuck, and all CPUs appear spinning.

I tried to directly boot OVMF in my Win10. The OnMpServicesAvailable() 
is not called because the PPI notification is not installed.
So I tried to modify InstallFeatureControlCallback() in
PlatformPei/FeatureControl.c to unconditionally call 
"PeiServicesNotifyPpi (&mMpServicesNotify);".
The new image can boot to Shell and I can find "OnMpServicesAvailable" 
debug message in the log.

> 
> Thanks
> Laszlo
> 
>> The last line is printed by OvmfPkg/PlatformPei, in
>> "OvmfPkg/PlatformPei/FeatureControl.c". OVMF uses
>> EFI_PEI_MP_SERVICES_PPI to write the Feature Control MSR on all CPUs.
>>
>> https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt#enable-nested-virtualization
>> https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt#confirm-simple-multiprocessing-during-boot
>>
>> Thanks,
>> Laszlo
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 


-- 
Thanks,
Ray