UefiCpuPkg/MpInitLib: Fix potential issue when IDT table is at above 4G

[edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Fix potential issue when IDT table is at above 4G

Posted by Zhiguang Liu 1 year, 8 months ago

Currently, when waking up AP, IDT table of AP will be set in 16 bit code,
and assume the IDT table base is 32 bit. However, the IDT table is created
by BSP. Issue will happen if the BSP allocates memory above 4G for BSP's
IDT table. Moreover, even the IDT table location is below 4G, the handler
function inside the IDT table is 64 bit, and it won't take effect until
CPU transfers to 64 bit long mode. There is no benefit to set IDT table in
such an early phase.
To avoid such issue, this patch moves the LIDT instruction into 64 bit
code.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 1daaa72b1e..cd95b03da8 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -64,9 +64,6 @@ BITS 16
     mov        si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile)
 o32 lgdt       [cs:si]
 
-    mov        si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)
-o32 lidt       [cs:si]
-
     ;
     ; Switch to protected mode
     ;
@@ -154,6 +151,11 @@ BITS 64
 
 LongModeStart:
     mov        esi, ebx
+
+    ; Set IDT table at the start of 64 bit code
+    lea        edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)]
+    lidt       [edi]
+
     lea        edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)]
     cmp        qword [edi], 1       ; ApInitConfig
     jnz        GetApicId
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92781): https://edk2.groups.io/g/devel/message/92781
Mute This Topic: https://groups.io/mt/93241591/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Fix potential issue when IDT table is at above 4G

Posted by Ni, Ray 1 year, 8 months ago

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Thursday, August 25, 2022 10:55 AM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>
> Subject: [PATCH] UefiCpuPkg/MpInitLib: Fix potential issue when IDT table is
> at above 4G
> 
> Currently, when waking up AP, IDT table of AP will be set in 16 bit code,
> and assume the IDT table base is 32 bit. However, the IDT table is created
> by BSP. Issue will happen if the BSP allocates memory above 4G for BSP's
> IDT table. Moreover, even the IDT table location is below 4G, the handler
> function inside the IDT table is 64 bit, and it won't take effect until
> CPU transfers to 64 bit long mode. There is no benefit to set IDT table in
> such an early phase.
> To avoid such issue, this patch moves the LIDT instruction into 64 bit
> code.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index 1daaa72b1e..cd95b03da8 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -64,9 +64,6 @@ BITS 16
>      mov        si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile)
>  o32 lgdt       [cs:si]
> 
> -    mov        si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)
> -o32 lidt       [cs:si]
> -
>      ;
>      ; Switch to protected mode
>      ;
> @@ -154,6 +151,11 @@ BITS 64
> 
>  LongModeStart:
>      mov        esi, ebx
> +
> +    ; Set IDT table at the start of 64 bit code
> +    lea        edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)]
> +    lidt       [edi]
> +
>      lea        edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)]
>      cmp        qword [edi], 1       ; ApInitConfig
>      jnz        GetApicId
> --
> 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92808): https://edk2.groups.io/g/devel/message/92808
Mute This Topic: https://groups.io/mt/93241591/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-