On 2/27/24 12:41, Gerd Hoffmann wrote:
> Move the WaitLoopExecutionMode and StartupSignalValue fields to a
> separate HOB with the new struct.
>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> ---
> UefiCpuPkg/Library/MpInitLib/MpHandOff.h | 13 ++++++--
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 ++++++++++++++++++++----
> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 34 +++++++++++++--------
> 4 files changed, 66 insertions(+), 22 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> index 77854d6a81f8..ae93b7e3d7c9 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpHandOff.h
> @@ -15,7 +15,13 @@
> 0x11e2bd88, 0xed38, 0x4abd, {0xa3, 0x99, 0x21, 0xf2, 0x5f, 0xd0, 0x7a,
> 0x60 } \
> }
>
> +#define MP_HANDOFF_CONFIG_GUID \
> + { \
> + 0xdabbd793, 0x7b46, 0x4144, {0x8a, 0xd4, 0x10, 0x1c, 0x7c, 0x08, 0xeb,
> 0xfa } \
> + }
> +
> extern EFI_GUID mMpHandOffGuid;
> +extern EFI_GUID mMpHandOffConfigGuid;
>
> //
> // The information required to transfer from the PEI phase to the
> @@ -43,8 +49,11 @@ typedef struct {
> //
> UINT32 ProcessorIndex;
> UINT32 CpuCount;
> - UINT32 WaitLoopExecutionMode;
> - UINT32 StartupSignalValue;
> PROCESSOR_HAND_OFF Info[];
> } MP_HAND_OFF;
> +
> +typedef struct {
> + UINT32 WaitLoopExecutionMode;
> + UINT32 StartupSignalValue;
> +} MP_HAND_OFF_CONFIG;
> #endif
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 3a7b9896cff4..d26035559f22 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -482,7 +482,8 @@ GetWakeupBuffer (
> **/
> VOID
> SwitchApContext (
> - IN CONST MP_HAND_OFF *FirstMpHandOff
> + IN CONST MP_HAND_OFF_CONFIG *MpHandOffConfig,
> + IN CONST MP_HAND_OFF *FirstMpHandOff
> );
>
> /**
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 8c186211fb38..a22bcfc0bc30 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -15,6 +15,7 @@
>
> EFI_GUID mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
> EFI_GUID mMpHandOffGuid = MP_HANDOFF_GUID;
> +EFI_GUID mMpHandOffConfigGuid = MP_HANDOFF_CONFIG_GUID;
>
> /**
> Save the volatile registers required to be restored following INIT IPI.
> @@ -1935,11 +1936,13 @@ GetBspNumber (
> This procedure allows the AP to switch to another section of
> memory and continue its loop there.
>
> - @param[in] FirstMpHandOff Pointer to first MP hand-off HOB body.
> + @param[in] MpHandOffConfig Pointer to MP hand-off config HOB body.
> + @param[in] FirstMpHandOff Pointer to first MP hand-off HOB body.
> **/
> VOID
> SwitchApContext (
> - IN CONST MP_HAND_OFF *FirstMpHandOff
> + IN CONST MP_HAND_OFF_CONFIG *MpHandOffConfig,
> + IN CONST MP_HAND_OFF *FirstMpHandOff
> )
> {
> UINTN Index;
> @@ -1955,7 +1958,7 @@ SwitchApContext (
> for (Index = 0; Index < MpHandOff->CpuCount; Index++) {
> if (MpHandOff->ProcessorIndex + Index != BspNumber) {
> *(UINTN *)(UINTN)MpHandOff->Info[Index].StartupProcedureAddress =
> (UINTN)SwitchContextPerAp;
> - *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress =
> MpHandOff->StartupSignalValue;
> + *(UINT32 *)(UINTN)MpHandOff->Info[Index].StartupSignalAddress =
> MpHandOffConfig->StartupSignalValue;
> }
> }
> }
> @@ -1975,6 +1978,26 @@ SwitchApContext (
> }
> }
>
> +/**
> + Get pointer to MP_HAND_OFF_CONFIG GUIDed HOB body.
> +
> + @return The pointer to MP_HAND_OFF_CONFIG structure.
> +**/
> +MP_HAND_OFF_CONFIG *
> +GetMpHandOffConfigHob (
> + VOID
> + )
> +{
> + EFI_HOB_GUID_TYPE *GuidHob;
> +
> + GuidHob = GetFirstGuidHob (&mMpHandOffConfigGuid);
> + if (GuidHob == NULL) {
> + return NULL;
> + }
> +
> + return (MP_HAND_OFF_CONFIG *)GET_GUID_HOB_DATA (GuidHob);
> +}
> +
> /**
> Get pointer to next MP_HAND_OFF GUIDed HOB body.
>
> @@ -2022,6 +2045,7 @@ MpInitLibInitialize (
> VOID
> )
> {
> + MP_HAND_OFF_CONFIG *MpHandOffConfig;
> MP_HAND_OFF *FirstMpHandOff;
> MP_HAND_OFF *MpHandOff;
> CPU_INFO_IN_HOB *CpuInfoInHob;
> @@ -2239,13 +2263,15 @@ MpInitLibInitialize (
> }
> }
>
> + MpHandOffConfig = GetMpHandOffConfigHob ();
> + ASSERT (MpHandOffConfig != NULL);
So, in connection to the other subthread
Re: [edk2-devel] CodeQL Analysis in edk2
msgid: <80abb140-9a9c-43b8-ba0b-d8ea631d9...@linux.microsoft.com>
https://edk2.groups.io/g/devel/message/116054
I suggest replacing this with:
if (MpHandOffConfig == NULL) {
DEBUG ((
DEBUG_ERROR,
"%a: at least one MpHandOff HOB, but no MpHandOffConfig HOB\n",
__func__
));
ASSERT (MpHandOffConfig != NULL);
CpuDeadLoop ();
}
(We should use a generic "panic" here; but for now, CpuDeadLoop() should
do. There are two prior examples for that in MpInitLib already.)
With that update:
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Thanks
Laszlo
> DEBUG ((
> DEBUG_INFO,
> "FirstMpHandOff->WaitLoopExecutionMode: %04d, sizeof (VOID *): %04d\n",
> - FirstMpHandOff->WaitLoopExecutionMode,
> + MpHandOffConfig->WaitLoopExecutionMode,
> sizeof (VOID *)
> ));
> - if (FirstMpHandOff->WaitLoopExecutionMode == sizeof (VOID *)) {
> + if (MpHandOffConfig->WaitLoopExecutionMode == sizeof (VOID *)) {
> ASSERT (CpuMpData->ApLoopMode != ApInHltLoop);
>
> CpuMpData->FinishedCount = 0;
> @@ -2261,7 +2287,7 @@ MpInitLibInitialize (
> // enables the APs to switch to a different memory section and
> continue their
> // looping process there.
> //
> - SwitchApContext (FirstMpHandOff);
> + SwitchApContext (MpHandOffConfig, FirstMpHandOff);
> //
> // Wait for all APs finished initialization
> //
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index ec1aa666923d..4d3acb491f36 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -126,14 +126,15 @@ SaveCpuMpData (
> IN CPU_MP_DATA *CpuMpData
> )
> {
> - UINT32 MaxCpusPerHob;
> - UINT32 CpusInHob;
> - UINT64 Data64;
> - UINT32 Index;
> - UINT32 HobBase;
> - CPU_INFO_IN_HOB *CpuInfoInHob;
> - MP_HAND_OFF *MpHandOff;
> - UINTN MpHandOffSize;
> + UINT32 MaxCpusPerHob;
> + UINT32 CpusInHob;
> + UINT64 Data64;
> + UINT32 Index;
> + UINT32 HobBase;
> + CPU_INFO_IN_HOB *CpuInfoInHob;
> + MP_HAND_OFF *MpHandOff;
> + MP_HAND_OFF_CONFIG MpHandOffConfig;
> + UINTN MpHandOffSize;
>
> MaxCpusPerHob = (0xFFF8 - sizeof (EFI_HOB_GUID_TYPE) - sizeof
> (MP_HAND_OFF)) / sizeof (PROCESSOR_HAND_OFF);
>
> @@ -155,11 +156,6 @@ SaveCpuMpData (
>
> MpHandOff->ProcessorIndex = HobBase;
> MpHandOff->CpuCount = CpusInHob;
> -
> - if (CpuMpData->ApLoopMode != ApInHltLoop) {
> - MpHandOff->StartupSignalValue = MP_HAND_OFF_SIGNAL;
> - MpHandOff->WaitLoopExecutionMode = sizeof (VOID *);
> - }
> }
>
> MpHandOff->Info[Index-HobBase].ApicId = CpuInfoInHob[Index].ApicId;
> @@ -170,6 +166,18 @@ SaveCpuMpData (
> }
> }
>
> + ZeroMem (&MpHandOffConfig, sizeof (MpHandOffConfig));
> + if (CpuMpData->ApLoopMode != ApInHltLoop) {
> + MpHandOffConfig.StartupSignalValue = MP_HAND_OFF_SIGNAL;
> + MpHandOffConfig.WaitLoopExecutionMode = sizeof (VOID *);
> + }
> +
> + BuildGuidDataHob (
> + &mMpHandOffConfigGuid,
> + (VOID *)&MpHandOffConfig,
> + sizeof (MpHandOffConfig)
> + );
> +
> //
> // Build location of CPU MP DATA buffer in HOB
> //
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116077): https://edk2.groups.io/g/devel/message/116077
Mute This Topic: https://groups.io/mt/104600810/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
