Reviewed-by: Ray Ni <ray...@intel.com>
Thanks,
Ray
> -----Original Message-----
> From: Xie, Yuanhao <yuanhao....@intel.com>
> Sent: Monday, November 20, 2023 12:55 PM
> To: devel@edk2.groups.io
> Cc: Xie, Yuanhao <yuanhao....@intel.com>; Laszlo Ersek
> <ler...@redhat.com>; Dong, Eric <eric.d...@intel.com>; Ni, Ray
> <ray...@intel.com>; Kumar, Rahul R <rahul.r.ku...@intel.com>; Gerd
> Hoffmann <kra...@redhat.com>
> Subject: [Patch V6 1/2] UefiCpuPkg/MpInitLib: Enable execute disable bit.
>
> From: Yuanhao Xie <yuanhao....@intel.com>
>
> This patch synchronizes the No-Execute bit in the IA32_EFER
> register for the APs before the RestoreVolatileRegisters operation.
>
> The commit 964a4f0, titled "Eliminate the second INIT-SIPI-SIPI
> sequence," replaces the second INIT-SIPI-SIPI sequence with the BSP
> calling the SwitchApContext function to initiate a specialized start-up
> signal, waking up APs in the DXE instead of using INIT-SIPI-SIPI.
>
> Due to this change, the logic for "Enable execute disable bit" in
> MpFuncs.nasm is no longer executed. However, to ensure the proper setup
> of the page table, it is necessary to synchronize the IA32_EFER.NXE for
> APs before executing RestoreVolatileRegisters .
>
> Based on SDM:
> If IA32_EFER.NXE is set to 1, it signifies execute-disable, meaning
> instruction fetches are not allowed from the 4-KByte page controlled by
> this entry. Conversely, if it is set to 0, it is reserved.
>
> Signed-off-by: Yuanhao Xie <yuanhao....@intel.com>
> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
> Cc: Laszlo Ersek ler...@redhat.com
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> ---
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 14 +++++++++++---
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 1 +
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 9a6ec5db5c..f29e66a14f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -910,9 +910,16 @@ DxeApEntryPoint (
> CPU_MP_DATA *CpuMpData
> )
> {
> - UINTN ProcessorNumber;
> + UINTN ProcessorNumber;
> + MSR_IA32_EFER_REGISTER EferMsr;
>
> GetProcessorNumber (CpuMpData, &ProcessorNumber);
> + if (CpuMpData->EnableExecuteDisableForSwitchContext) {
> + EferMsr.Uint64 = AsmReadMsr64 (MSR_IA32_EFER);
> + EferMsr.Bits.NXE = 1;
> + AsmWriteMsr64 (MSR_IA32_EFER, EferMsr.Uint64);
> + }
> +
> RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters,
> FALSE);
> InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
> PlaceAPInMwaitLoopOrRunLoop (
> @@ -2188,8 +2195,9 @@ MpInitLibInitialize (
> if (MpHandOff->WaitLoopExecutionMode == sizeof (VOID *)) {
> ASSERT (CpuMpData->ApLoopMode != ApInHltLoop);
>
> - CpuMpData->FinishedCount = 0;
> - CpuMpData->InitFlag = ApInitDone;
> + CpuMpData->FinishedCount = 0;
> + CpuMpData->InitFlag =
> ApInitDone;
> + CpuMpData->EnableExecuteDisableForSwitchContext =
> IsBspExecuteDisableEnabled ();
> SaveCpuMpData (CpuMpData);
> //
> // In scenarios where both the PEI and DXE phases run in the same
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 763db4963d..af296f6ac0 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -270,6 +270,7 @@ struct _CPU_MP_DATA {
> UINT64 TotalTime;
> EFI_EVENT WaitEvent;
> UINTN **FailedCpuList;
> + BOOLEAN
> EnableExecuteDisableForSwitchContext;
>
> AP_INIT_STATE InitFlag;
> BOOLEAN SwitchBspFlag;
> --
> 2.39.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111495): https://edk2.groups.io/g/devel/message/111495
Mute This Topic: https://groups.io/mt/102702384/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
