On 10/29/15 21:35, Michael Kinney wrote: > The PiSmmCpuDxeSmm module makes some assumptions about GDT selectors > that are based on the GDT layout from the DxeIplPeim. This updates the > CpuDxe module to use the same GDT layout as the DxeIplPeim. > > Using a consistent GDT also improves debug experience. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Michael Kinney <[email protected]> > --- > UefiCpuPkg/CpuDxe/CpuGdt.c | 83 > ++++++++++++++++++++++++++-------------------- > UefiCpuPkg/CpuDxe/CpuGdt.h | 10 +++--- > 2 files changed, 53 insertions(+), 40 deletions(-)
I'll try to test this and report back, but until then: - Can you please add a note to the commit message about this bug breaking S3 resume? For example: [The PiSmmCpuDxeSmm module makes some assumptions about GDT selectors that are based on the GDT layout from the DxeIplPeim.] For example, the protected mode entry code and (where appropriate) the long mode entry code in the UefiCpuPkg/PiSmmCpuDxeSmm/*/MpFuncs.* assembly files, which are used during S3 resume, open-code segment selector values that depend on DxeIplPeim's GDT layout. [This patch updates the CpuDxe module to use the same GDT layout as the DxeIplPeim.] This enables modules that are dispatched after CpuDxe to find, and potentially save and restore, a GDT layout that matches that of DxeIplPeim. [Using a consistent GDT also...] - Can you please add the following tags (bragging rights are important! ;)) Reported-by: Laszlo Ersek <[email protected]> Analyzed-by: Laszlo Ersek <[email protected]> Link: http://article.gmane.org/gmane.comp.bios.edk2.devel/3568 Thank you, Laszlo > > diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.c b/UefiCpuPkg/CpuDxe/CpuGdt.c > index 35a87a6..9ef2fdf 100644 > --- a/UefiCpuPkg/CpuDxe/CpuGdt.c > +++ b/UefiCpuPkg/CpuDxe/CpuGdt.c > @@ -1,10 +1,10 @@ > /** @file > C based implemention of IA32 interrupt handling only > requiring a minimal assembly interrupt entry point. > > - Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > http://opensource.org/licenses/bsd-license.php > > @@ -33,82 +33,93 @@ STATIC GDT_ENTRIES GdtTemplate = { > }, > // > // LINEAR_SEL > // > { > - 0x0FFFF, // limit 0xFFFFF > - 0x0, // base 0 > - 0x0, > - 0x092, // present, ring 0, data, expand-up, writable > + 0x0FFFF, // limit 15:0 > + 0x0, // base 15:0 > + 0x0, // base 23:16 > + 0x092, // present, ring 0, data, read/write > 0x0CF, // page-granular, 32-bit > 0x0, > }, > // > // LINEAR_CODE_SEL > // > { > - 0x0FFFF, // limit 0xFFFFF > - 0x0, // base 0 > - 0x0, > - 0x09A, // present, ring 0, data, expand-up, writable > + 0x0FFFF, // limit 15:0 > + 0x0, // base 15:0 > + 0x0, // base 23:16 > + 0x09F, // present, ring 0, code, execute/read, conforming, > accessed > 0x0CF, // page-granular, 32-bit > 0x0, > }, > // > // SYS_DATA_SEL > // > { > - 0x0FFFF, // limit 0xFFFFF > - 0x0, // base 0 > - 0x0, > - 0x092, // present, ring 0, data, expand-up, writable > + 0x0FFFF, // limit 15:0 > + 0x0, // base 15:0 > + 0x0, // base 23:16 > + 0x093, // present, ring 0, data, read/write, accessed > 0x0CF, // page-granular, 32-bit > 0x0, > }, > // > // SYS_CODE_SEL > // > { > - 0x0FFFF, // limit 0xFFFFF > - 0x0, // base 0 > - 0x0, > - 0x09A, // present, ring 0, data, expand-up, writable > + 0x0FFFF, // limit 15:0 > + 0x0, // base 15:0 > + 0x0, // base 23:16 > + 0x09A, // present, ring 0, code, execute/read > 0x0CF, // page-granular, 32-bit > 0x0, > }, > // > - // LINEAR_CODE64_SEL > + // SPARE4_SEL > // > { > - 0x0FFFF, // limit 0xFFFFF > - 0x0, // base 0 > - 0x0, > - 0x09B, // present, ring 0, code, expand-up, writable > - 0x0AF, // LimitHigh (CS.L=1, CS.D=0) > - 0x0, // base (high) > + 0x0, // limit 15:0 > + 0x0, // base 15:0 > + 0x0, // base 23:16 > + 0x0, // type > + 0x0, // limit 19:16, flags > + 0x0, // base 31:24 > }, > // > - // SPARE4_SEL > + // LINEAR_DATA64_SEL > // > { > - 0x0, // limit 0 > - 0x0, // base 0 > - 0x0, > - 0x0, // present, ring 0, data, expand-up, writable > - 0x0, // page-granular, 32-bit > + 0x0FFFF, // limit 15:0 > + 0x0, // base 15:0 > + 0x0, // base 23:16 > + 0x092, // present, ring 0, data, read/write > + 0x0CF, // page-granular, 32-bit > 0x0, > }, > // > + // LINEAR_CODE64_SEL > + // > + { > + 0x0FFFF, // limit 15:0 > + 0x0, // base 15:0 > + 0x0, // base 23:16 > + 0x09A, // present, ring 0, code, execute/read > + 0x0AF, // page-granular, 64-bit code > + 0x0, // base (high) > + }, > + // > // SPARE5_SEL > // > { > - 0x0, // limit 0 > - 0x0, // base 0 > - 0x0, > - 0x0, // present, ring 0, data, expand-up, writable > - 0x0, // page-granular, 32-bit > - 0x0, > + 0x0, // limit 15:0 > + 0x0, // base 15:0 > + 0x0, // base 23:16 > + 0x0, // type > + 0x0, // limit 19:16, flags > + 0x0, // base 31:24 > }, > }; > > /** > Initialize Global Descriptor Table. > diff --git a/UefiCpuPkg/CpuDxe/CpuGdt.h b/UefiCpuPkg/CpuDxe/CpuGdt.h > index 7ecec5d..2a00751 100644 > --- a/UefiCpuPkg/CpuDxe/CpuGdt.h > +++ b/UefiCpuPkg/CpuDxe/CpuGdt.h > @@ -1,10 +1,10 @@ > /** @file > C based implemention of IA32 interrupt handling only > requiring a minimal assembly interrupt entry point. > > - Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > http://opensource.org/licenses/bsd-license.php > > @@ -40,32 +40,34 @@ struct _GDT_ENTRIES { > GDT_ENTRY Null; > GDT_ENTRY Linear; > GDT_ENTRY LinearCode; > GDT_ENTRY SysData; > GDT_ENTRY SysCode; > - GDT_ENTRY LinearCode64; > GDT_ENTRY Spare4; > + GDT_ENTRY LinearData64; > + GDT_ENTRY LinearCode64; > GDT_ENTRY Spare5; > } GDT_ENTRIES; > > #pragma pack () > > #define NULL_SEL OFFSET_OF (GDT_ENTRIES, Null) > #define LINEAR_SEL OFFSET_OF (GDT_ENTRIES, Linear) > #define LINEAR_CODE_SEL OFFSET_OF (GDT_ENTRIES, LinearCode) > #define SYS_DATA_SEL OFFSET_OF (GDT_ENTRIES, SysData) > #define SYS_CODE_SEL OFFSET_OF (GDT_ENTRIES, SysCode) > -#define LINEAR_CODE64_SEL OFFSET_OF (GDT_ENTRIES, LinearCode64) > #define SPARE4_SEL OFFSET_OF (GDT_ENTRIES, Spare4) > +#define LINEAR_DATA64_SEL OFFSET_OF (GDT_ENTRIES, LinearData64) > +#define LINEAR_CODE64_SEL OFFSET_OF (GDT_ENTRIES, LinearCode64) > #define SPARE5_SEL OFFSET_OF (GDT_ENTRIES, Spare5) > > #if defined (MDE_CPU_IA32) > #define CPU_CODE_SEL LINEAR_CODE_SEL > #define CPU_DATA_SEL LINEAR_SEL > #elif defined (MDE_CPU_X64) > #define CPU_CODE_SEL LINEAR_CODE64_SEL > -#define CPU_DATA_SEL LINEAR_SEL > +#define CPU_DATA_SEL LINEAR_DATA64_SEL > #else > #error CPU type not supported for CPU GDT initialization! > #endif > > #endif // _CPU_GDT_H_ > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
