* [PATCH v2 0/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB @ 2021-03-17 11:06 Ni, Ray 2021-03-17 11:06 ` [PATCH v2 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard Ni, Ray 2021-03-17 11:06 ` [PATCH v2 2/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB Ni, Ray 0 siblings, 2 replies; 6+ messages in thread From: Ni, Ray @ 2021-03-17 11:06 UTC (permalink / raw) To: devel Ray Ni (2): UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB UefiCpuPkg/CpuDxe/CpuGdt.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) -- 2.27.0.windows.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard 2021-03-17 11:06 [PATCH v2 0/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB Ni, Ray @ 2021-03-17 11:06 ` Ni, Ray 2021-03-17 16:31 ` Laszlo Ersek 2021-03-18 0:37 ` Dong, Eric 2021-03-17 11:06 ` [PATCH v2 2/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB Ni, Ray 1 sibling, 2 replies; 6+ messages in thread From: Ni, Ray @ 2021-03-17 11:06 UTC (permalink / raw) To: devel; +Cc: Eric Dong, Laszlo Ersek, Rahul Kumar The change doesn't impact any functionality. Signed-off-by: Ray Ni <ray.ni@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> --- UefiCpuPkg/CpuDxe/CpuGdt.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c index a1ab543f2d..8847bc4819 100644 --- a/UefiCpuPkg/CpuDxe/CpuGdt.c +++ b/UefiCpuPkg/CpuDxe/CpuGdt.c @@ -2,7 +2,7 @@ C based implementation of IA32 interrupt handling only requiring a minimal assembly interrupt entry point. - Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR> + Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -13,7 +13,7 @@ // // Global descriptor table (GDT) Template // -STATIC GDT_ENTRIES GdtTemplate = { +STATIC GDT_ENTRIES mGdtTemplate = { // // NULL_SEL // @@ -124,27 +124,27 @@ InitGlobalDescriptorTable ( VOID ) { - GDT_ENTRIES *gdt; - IA32_DESCRIPTOR gdtPtr; + GDT_ENTRIES *Gdt; + IA32_DESCRIPTOR Gdtr; // // Allocate Runtime Data for the GDT // - gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8); - ASSERT (gdt != NULL); - gdt = ALIGN_POINTER (gdt, 8); + Gdt = AllocateRuntimePool (sizeof (mGdtTemplate) + 8); + ASSERT (Gdt != NULL); + Gdt = ALIGN_POINTER (Gdt, 8); // // Initialize all GDT entries // - CopyMem (gdt, &GdtTemplate, sizeof (GdtTemplate)); + CopyMem (Gdt, &mGdtTemplate, sizeof (mGdtTemplate)); // // Write GDT register // - gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt; - gdtPtr.Limit = (UINT16) (sizeof (GdtTemplate) - 1); - AsmWriteGdtr (&gdtPtr); + Gdtr.Base = (UINT32) (UINTN) Gdt; + Gdtr.Limit = (UINT16) (sizeof (mGdtTemplate) - 1); + AsmWriteGdtr (&Gdtr); // // Update selector (segment) registers base on new GDT @@ -152,4 +152,3 @@ InitGlobalDescriptorTable ( SetCodeSelector ((UINT16)CPU_CODE_SEL); SetDataSelectors ((UINT16)CPU_DATA_SEL); } - -- 2.27.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard 2021-03-17 11:06 ` [PATCH v2 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard Ni, Ray @ 2021-03-17 16:31 ` Laszlo Ersek 2021-03-18 0:37 ` Dong, Eric 1 sibling, 0 replies; 6+ messages in thread From: Laszlo Ersek @ 2021-03-17 16:31 UTC (permalink / raw) To: Ray Ni, devel; +Cc: Eric Dong, Rahul Kumar On 03/17/21 12:06, Ray Ni wrote: > The change doesn't impact any functionality. > > Signed-off-by: Ray Ni <ray.ni@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > --- > UefiCpuPkg/CpuDxe/CpuGdt.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c > index a1ab543f2d..8847bc4819 100644 > --- a/UefiCpuPkg/CpuDxe/CpuGdt.c > +++ b/UefiCpuPkg/CpuDxe/CpuGdt.c > @@ -2,7 +2,7 @@ > C based implementation of IA32 interrupt handling only > requiring a minimal assembly interrupt entry point. > > - Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -13,7 +13,7 @@ > // > // Global descriptor table (GDT) Template > // > -STATIC GDT_ENTRIES GdtTemplate = { > +STATIC GDT_ENTRIES mGdtTemplate = { > // > // NULL_SEL > // > @@ -124,27 +124,27 @@ InitGlobalDescriptorTable ( > VOID > ) > { > - GDT_ENTRIES *gdt; > - IA32_DESCRIPTOR gdtPtr; > + GDT_ENTRIES *Gdt; > + IA32_DESCRIPTOR Gdtr; > > // > // Allocate Runtime Data for the GDT > // > - gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8); > - ASSERT (gdt != NULL); > - gdt = ALIGN_POINTER (gdt, 8); > + Gdt = AllocateRuntimePool (sizeof (mGdtTemplate) + 8); > + ASSERT (Gdt != NULL); > + Gdt = ALIGN_POINTER (Gdt, 8); > > // > // Initialize all GDT entries > // > - CopyMem (gdt, &GdtTemplate, sizeof (GdtTemplate)); > + CopyMem (Gdt, &mGdtTemplate, sizeof (mGdtTemplate)); > > // > // Write GDT register > // > - gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt; > - gdtPtr.Limit = (UINT16) (sizeof (GdtTemplate) - 1); > - AsmWriteGdtr (&gdtPtr); > + Gdtr.Base = (UINT32) (UINTN) Gdt; > + Gdtr.Limit = (UINT16) (sizeof (mGdtTemplate) - 1); > + AsmWriteGdtr (&Gdtr); > > // > // Update selector (segment) registers base on new GDT > @@ -152,4 +152,3 @@ InitGlobalDescriptorTable ( > SetCodeSelector ((UINT16)CPU_CODE_SEL); > SetDataSelectors ((UINT16)CPU_DATA_SEL); > } > - > This patch does *slightly* more than what it says on the tin (empty line removed from the end, also a useless (VOID*) cast is dropped), but I'm fine with those. Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard 2021-03-17 11:06 ` [PATCH v2 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard Ni, Ray 2021-03-17 16:31 ` Laszlo Ersek @ 2021-03-18 0:37 ` Dong, Eric 1 sibling, 0 replies; 6+ messages in thread From: Dong, Eric @ 2021-03-18 0:37 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io; +Cc: Laszlo Ersek, Kumar, Rahul1 Reviewed-by: Eric Dong <eric.dong@intel.com> -----Original Message----- From: Ni, Ray <ray.ni@intel.com> Sent: Wednesday, March 17, 2021 7:07 PM To: devel@edk2.groups.io Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com> Subject: [PATCH v2 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard The change doesn't impact any functionality. Signed-off-by: Ray Ni <ray.ni@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> --- UefiCpuPkg/CpuDxe/CpuGdt.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c index a1ab543f2d..8847bc4819 100644 --- a/UefiCpuPkg/CpuDxe/CpuGdt.c +++ b/UefiCpuPkg/CpuDxe/CpuGdt.c @@ -2,7 +2,7 @@ C based implementation of IA32 interrupt handling only requiring a minimal assembly interrupt entry point. - Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>+ Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/@@ -13,7 +13,7 @@ // // Global descriptor table (GDT) Template //-STATIC GDT_ENTRIES GdtTemplate = {+STATIC GDT_ENTRIES mGdtTemplate = { // // NULL_SEL //@@ -124,27 +124,27 @@ InitGlobalDescriptorTable ( VOID ) {- GDT_ENTRIES *gdt;- IA32_DESCRIPTOR gdtPtr;+ GDT_ENTRIES *Gdt;+ IA32_DESCRIPTOR Gdtr; // // Allocate Runtime Data for the GDT //- gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8);- ASSERT (gdt != NULL);- gdt = ALIGN_POINTER (gdt, 8);+ Gdt = AllocateRuntimePool (sizeof (mGdtTemplate) + 8);+ ASSERT (Gdt != NULL);+ Gdt = ALIGN_POINTER (Gdt, 8); // // Initialize all GDT entries //- CopyMem (gdt, &GdtTemplate, sizeof (GdtTemplate));+ CopyMem (Gdt, &mGdtTemplate, sizeof (mGdtTemplate)); // // Write GDT register //- gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt;- gdtPtr.Limit = (UINT16) (sizeof (GdtTemplate) - 1);- AsmWriteGdtr (&gdtPtr);+ Gdtr.Base = (UINT32) (UINTN) Gdt;+ Gdtr.Limit = (UINT16) (sizeof (mGdtTemplate) - 1);+ AsmWriteGdtr (&Gdtr); // // Update selector (segment) registers base on new GDT@@ -152,4 +152,3 @@ InitGlobalDescriptorTable ( SetCodeSelector ((UINT16)CPU_CODE_SEL); SetDataSelectors ((UINT16)CPU_DATA_SEL); }--- 2.27.0.windows.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB 2021-03-17 11:06 [PATCH v2 0/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB Ni, Ray 2021-03-17 11:06 ` [PATCH v2 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard Ni, Ray @ 2021-03-17 11:06 ` Ni, Ray 2021-03-17 16:31 ` Laszlo Ersek 1 sibling, 1 reply; 6+ messages in thread From: Ni, Ray @ 2021-03-17 11:06 UTC (permalink / raw) To: devel; +Cc: Eric Dong, Laszlo Ersek, Rahul Kumar REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3233 GDT needs to be allocated below 4GB in 64bit environment because AP needs it for entering to protected mode. CPU running in big real mode cannot access above 4GB GDT. But CpuDxe driver contains below code: gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8); ..... gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt; The AllocateRuntimePool() may allocate memory above 4GB. Thus, we cannot use AllocateRuntimePool (), instead, we should use AllocatePages() to make sure GDT is below 4GB space. Signed-off-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> --- UefiCpuPkg/CpuDxe/CpuGdt.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c index 8847bc4819..692402c55d 100644 --- a/UefiCpuPkg/CpuDxe/CpuGdt.c +++ b/UefiCpuPkg/CpuDxe/CpuGdt.c @@ -124,15 +124,26 @@ InitGlobalDescriptorTable ( VOID ) { + EFI_STATUS Status; GDT_ENTRIES *Gdt; IA32_DESCRIPTOR Gdtr; + EFI_PHYSICAL_ADDRESS Memory; // - // Allocate Runtime Data for the GDT - // - Gdt = AllocateRuntimePool (sizeof (mGdtTemplate) + 8); - ASSERT (Gdt != NULL); - Gdt = ALIGN_POINTER (Gdt, 8); + // Allocate Runtime Data below 4GB for the GDT + // AP uses the same GDT when it's waken up from real mode so + // the GDT needs to be below 4GB. + // + Memory = SIZE_4GB - 1; + Status = gBS->AllocatePages ( + AllocateMaxAddress, + EfiRuntimeServicesData, + EFI_SIZE_TO_PAGES (sizeof (mGdtTemplate)), + &Memory + ); + ASSERT_EFI_ERROR (Status); + ASSERT ((Memory != 0) && (Memory < SIZE_4GB)); + Gdt = (GDT_ENTRIES *) (UINTN) Memory; // // Initialize all GDT entries -- 2.27.0.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB 2021-03-17 11:06 ` [PATCH v2 2/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB Ni, Ray @ 2021-03-17 16:31 ` Laszlo Ersek 0 siblings, 0 replies; 6+ messages in thread From: Laszlo Ersek @ 2021-03-17 16:31 UTC (permalink / raw) To: Ray Ni, devel; +Cc: Eric Dong, Rahul Kumar On 03/17/21 12:06, Ray Ni wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3233 > > GDT needs to be allocated below 4GB in 64bit environment > because AP needs it for entering to protected mode. > CPU running in big real mode cannot access above 4GB GDT. > > But CpuDxe driver contains below code: > gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8); > ..... > gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt; > > The AllocateRuntimePool() may allocate memory above 4GB. > Thus, we cannot use AllocateRuntimePool (), instead, > we should use AllocatePages() to make sure GDT is below 4GB space. > > Signed-off-by: Ray Ni <ray.ni@intel.com> > Reviewed-by: Eric Dong <eric.dong@intel.com> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > --- > UefiCpuPkg/CpuDxe/CpuGdt.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c > index 8847bc4819..692402c55d 100644 > --- a/UefiCpuPkg/CpuDxe/CpuGdt.c > +++ b/UefiCpuPkg/CpuDxe/CpuGdt.c > @@ -124,15 +124,26 @@ InitGlobalDescriptorTable ( > VOID > ) > { > + EFI_STATUS Status; > GDT_ENTRIES *Gdt; > IA32_DESCRIPTOR Gdtr; > + EFI_PHYSICAL_ADDRESS Memory; > > // > - // Allocate Runtime Data for the GDT > - // > - Gdt = AllocateRuntimePool (sizeof (mGdtTemplate) + 8); > - ASSERT (Gdt != NULL); > - Gdt = ALIGN_POINTER (Gdt, 8); > + // Allocate Runtime Data below 4GB for the GDT > + // AP uses the same GDT when it's waken up from real mode so > + // the GDT needs to be below 4GB. > + // > + Memory = SIZE_4GB - 1; > + Status = gBS->AllocatePages ( > + AllocateMaxAddress, > + EfiRuntimeServicesData, > + EFI_SIZE_TO_PAGES (sizeof (mGdtTemplate)), > + &Memory > + ); > + ASSERT_EFI_ERROR (Status); > + ASSERT ((Memory != 0) && (Memory < SIZE_4GB)); > + Gdt = (GDT_ENTRIES *) (UINTN) Memory; > > // > // Initialize all GDT entries > Reviewed-by: Laszlo Ersek <lersek@redhat.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-18 0:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-17 11:06 [PATCH v2 0/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB Ni, Ray 2021-03-17 11:06 ` [PATCH v2 1/2] UefiCpuPkg/CpuDxe: Rename variables to follow EDKII coding standard Ni, Ray 2021-03-17 16:31 ` Laszlo Ersek 2021-03-18 0:37 ` Dong, Eric 2021-03-17 11:06 ` [PATCH v2 2/2] UefiCpuPkg/CpuDxe: Guarantee GDT is below 4GB Ni, Ray 2021-03-17 16:31 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox