Hi Ray,
Please check V4 patch series.
Thanks for the feedbacks.
Yuanhao
>
> The SaveCpuMpData() function was updated to construct the MP_HAND_OFF
> Hob. Additionally, the function introduced the MP_HAND_OFF_SIGNAL,
> which solely served the purpose of awakening the APs and transitioning
> their context from PEI to DXE. The WaitLoopExecutionMode field
> indicated whether the bit mode of PEI matched that of DXE. Both of
> them were filled only if the ApLoopMode was not ApInHltLoop. In the
> case of ApLoopMode, it remained necessary
1. "ApLoopMode" -> "ApInHltMode"?
2. can you update commit message to highlight that this patch still sends
INIT-SIPI-SIPI even AP is wait in run-loop or mwait loop?
A: I updated commit message in patch V4.
> +
> +/**
> + Get pointer to CPU MP Data structure from GUIDed HOB.
> +
> + @param[in] CpuMpData The pointer to CPU MP Data structure.
> +**/
> +VOID
> +AmdSevUpdateCpuMpData (
> + IN CPU_MP_DATA *CpuMpData
> + )
> +{
> + CPU_MP_DATA *OldCpuMpData;
> +
> + OldCpuMpData = GetCpuMpDataFromGuidedHob ();
> +
> + OldCpuMpData->NewCpuMpData = CpuMpData; }
3. Can you please move this function implementation into MpInitLib/AmdSev.c?
A: This function is added in AmdSev.c
> +//
> +// To trigger the start-up signal, BSP writes the specified //
> +StartupSignalValue to the StartupSignalAddress of each processor.
> +// This address is monitored by the APs, and as soon as they receive
> +// the value that matches the MP_HAND_OFF_SIGNAL, they will wake up
> +// and switch the context from PEI to DXE phase.
> +//
> +#define MP_HAND_OFF_SIGNAL SIGNATURE_32 ('M', 'P', 'H', 'O')
4. It's ok to define the signal signature here.
> +#define MP_HANDOFF_GUID \
> + { \
> + 0x11e2bd88, 0xed38, 0x4abd, {0xa3, 0x99, 0x21, 0xf2, 0x5f, 0xd0,
> +0x7a, 0x60 }
> \
> + }
5. can you please move this GUID definition to MpInitLib/MpHandOff.h?
A: MP_HANDOFF_GUID and 2 structure definitions in below are put in
MpInitLib/MpHandOff in patch V4.
> +//
> +// The information required to transfer from the PEI phase to the //
> +DXE phase is contained within the MP_HAND_OFF and
> PROCESSOR_HAND_OFF.
> +// If the SizeOfPointer (WaitLoopExecutionMode) of both phases are
> +equal, // and the APs is not in halt mode, // then the APs can be
> +awakened by triggering the start-up // signal, rather than using
> +INIT-SIPI-SIPI.
> +// To trigger the start-up signal, BSP writes the specified //
> +StartupSignalValue to the StartupSignalAddress of each processor.
> +// This address is monitored by the APs.
> +//
> +typedef struct {
> + UINT32 ApicId;
> + UINT32 Health;
> + UINT64 StartupSignalAddress;
> + UINT64 StartupProcedureAddress;
> +} PROCESSOR_HAND_OFF;
> +
> +typedef struct {
> + //
> + // The ProcessorIndex indicates the range of processors. If it is
> +set to 0, it
> signifies
> + // processors from 0 to CpuCount - 1. Multiple instances in the HOB
> + list
> describe
> + // processors from ProcessorIndex to ProcessorIndex + CpuCount - 1.
> + //
> + UINT32 ProcessorIndex;
> + UINT32 CpuCount;
> + UINT32 WaitLoopExecutionMode;
> + UINT32 StartupSignalValue;
> + PROCESSOR_HAND_OFF Info[];
> +} MP_HAND_OFF;
6. can you please move the above 2 structure definitions to
MpInitLib/MpHandOff.h?
> +
> //
> // Data structure for microcode patch information // @@ -347,6
> +391,7 @@ typedef
> );
>
> extern EFI_GUID mCpuInitMpLibHobGuid;
> +extern EFI_GUID mMpHandOffGuid;
>
> /**
> Assembly code to place AP into safe loop mode.
> @@ -452,6 +497,17 @@ SaveCpuMpData (
> IN CPU_MP_DATA *CpuMpData
> );
>
> +/**
> + This function Get BspNumber.
> +
> + @param[in] MpHandOff Pointer to MpHandOff
> + @return BspNumber
> +**/
> +UINT32
> +GetBspNumber (
> + IN CONST MP_HAND_OFF *MpHandOff
> + );
7. It seems above "GetBspNumber" prototype declaration can be removed. Can you
confirm?
A: I agree. GetBspNumber is removed in patch V4.
> +
> /**
> Get available system memory below 1MB by specified size.
>
> @@ -652,6 +708,16 @@ EnableDisableApWorker (
> IN UINT32 *HealthFlag OPTIONAL
> );
>
> +/**
> + Get pointer to MP_HAND_OFF GUIDed HOB.
> +
> + @return The pointer to MP_HAND_OFF structure.
> +**/
> +MP_HAND_OFF *
> +GetMpHandOffHob (
> + VOID
> + );
> +
8. It seems above "GetMpHandOffHob" prototype declaration can be removed if you
implement it in front of
MpInitLibInitialize() in MpLib.c. can you confirm?
A: I agree. GetMpHandOffHob is removed in patch V4.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106466): https://edk2.groups.io/g/devel/message/106466
Mute This Topic: https://groups.io/mt/99782482/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
