[edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Fix possible uninitialized 'InitFlag' field
Ni, Ray
ray.ni at intel.com
Fri Jan 17 08:24:38 UTC 2020
Reviewed-by: Ray Ni <ray.ni at intel.com>
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Wu, Hao
> A
> Sent: Friday, January 17, 2020 2:57 PM
> To: devel at edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu at intel.com>; Dong, Eric <eric.dong at intel.com>; Ni,
> Ray <ray.ni at intel.com>; Laszlo Ersek <lersek at redhat.com>; Kinney, Michael
> D <michael.d.kinney at intel.com>
> Subject: [edk2-devel] [PATCH v1] UefiCpuPkg/MpInitLib: Fix possible
> uninitialized 'InitFlag' field
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2474
>
> Previous commit d786a17232:
> UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
>
> Removed the below assignments for the 'InitFlag' field of CPU_MP_DATA
> structure in function MpInitLibInitialize() when APs are waken up to do
> some initialize sync:
>
> CpuMpData->InitFlag = ApInitReconfig;
> ...
> CpuMpData->InitFlag = ApInitDone;
>
> Under some cases (e.g. when variable OldCpuMpData is not NULL, which
> means
> function CollectProcessorCount() will not be called), this will left the
> 'InitFlag' field being uninitialized with a value of 0, which is a invalid
> value for the type of 'InitFlag' (AP_INIT_STATE).
>
> It may potentially cause the WakeUpAP() function to run some unnecessary
> codes when the APs have been successfully waken up before:
>
> if (CpuMpData->WakeUpByInitSipiSipi ||
> CpuMpData->InitFlag != ApInitDone) {
> ResetVectorRequired = TRUE;
> AllocateResetVector (CpuMpData);
> FillExchangeInfoData (CpuMpData);
> SaveLocalApicTimerSetting (CpuMpData);
> }
>
> This commit will address the above-mentioned issue.
>
> Test done:
> * OS boot on a real platform with multi processors
>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> Signed-off-by: Hao A Wu <hao.a.wu at intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 6ec9b172b8..17e19395f2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1775,11 +1775,12 @@ MpInitLibInitialize (
> // Wakeup APs to do some AP initialize sync (Microcode & MTRR)
> //
> if (CpuMpData->CpuCount > 1) {
> + CpuMpData->InitFlag = ApInitReconfig;
> WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
> while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
> CpuPause ();
> }
> -
> + CpuMpData->InitFlag = ApInitDone;
> for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
> SetApState (&CpuMpData->CpuData[Index], CpuStateIdle);
> }
> --
> 2.12.0.windows.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#53351): https://edk2.groups.io/g/devel/message/53351
Mute This Topic: https://groups.io/mt/69838817/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list