I agree. This is proper use of the term "Smram". Mike
> -----Original Message----- > From: Ni, Ray <ray...@intel.com> > Sent: Tuesday, April 11, 2023 12:38 AM > To: devel@edk2.groups.io; abdat...@amd.com; Kinney, Michael D > <michael.d.kin...@intel.com> > Cc: Paul Grimes <paul.gri...@amd.com>; Garrett Kirkendall > <garrett.kirkend...@amd.com>; Abner Chang <abner.ch...@amd.com>; > Gao, Liming <gaolim...@byosoft.com.cn>; Liu, Zhiguang <zhiguang....@intel.com> > Subject: RE: [edk2-devel] [PATCH v8 1/9] MdePkg: Adds AMD SMRAM save state map > > The patch looks good in general. > @Kinney, Michael D, I think it's ok to use "SmramSaveStateMap.h" instead of > "MmramSaveStateMap.h" because it matches to > the CPU spec. > Do you agree? > > "MM" is more like a general term that applies to all archs, X86, ARM, etc. > > Abdul, just curious why do you add prefix "_" for some of the fields? I am ok > if it matches to the AMD CPU spec. > > Thanks, > Ray > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul > > Lateef Attar via groups.io > > Sent: Monday, April 10, 2023 7:10 PM > > To: devel@edk2.groups.io > > Cc: Abdul Lateef Attar <abdat...@amd.com>; Paul Grimes > > <paul.gri...@amd.com>; Garrett Kirkendall <garrett.kirkend...@amd.com>; > > Abner Chang <abner.ch...@amd.com>; Kinney, Michael D > > <michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>; > > Liu, Zhiguang <zhiguang....@intel.com> > > Subject: [edk2-devel] [PATCH v8 1/9] MdePkg: Adds AMD SMRAM save state > > map > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4182 > > > > Adds an SMM SMRAM save-state map for AMD processors. > > SMRAM save state maps for the AMD processor family are now supported. > > > > Save state map structure is added based on > > AMD64 Architecture Programmer's Manual, Volume 2, Section 10.2. > > > > The AMD legacy save state map for 32-bit architecture is defined. > > The AMD64 save state map for 64-bit architecture is defined. > > > > Also added Amd/SmramSaveStateMap.h to IgnoreFiles of EccCheck, > > because structures defined in this file are derived from > > Intel/SmramSaveStateMap.h. > > > > Cc: Paul Grimes <paul.gri...@amd.com> > > Cc: Garrett Kirkendall <garrett.kirkend...@amd.com> > > Cc: Abner Chang <abner.ch...@amd.com> > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > Cc: Liming Gao <gaolim...@byosoft.com.cn> > > Cc: Zhiguang Liu <zhiguang....@intel.com> > > > > Signed-off-by: Abdul Lateef Attar <abdat...@amd.com> > > Reviewed-by: Abner Chang <abner.ch...@amd.com> > > Reviewed-by: Michael D Kinney <michael.d.kin...@intel.com> > > --- > > .../Include/Register/Amd/SmramSaveStateMap.h | 194 > > ++++++++++++++++++ > > MdePkg/MdePkg.ci.yaml | 4 +- > > 2 files changed, 197 insertions(+), 1 deletion(-) > > create mode 100644 > > MdePkg/Include/Register/Amd/SmramSaveStateMap.h > > > > diff --git a/MdePkg/Include/Register/Amd/SmramSaveStateMap.h > > b/MdePkg/Include/Register/Amd/SmramSaveStateMap.h > > new file mode 100644 > > index 000000000000..0607d2a19145 > > --- /dev/null > > +++ b/MdePkg/Include/Register/Amd/SmramSaveStateMap.h > > @@ -0,0 +1,194 @@ > > +/** @file > > + AMD SMRAM Save State Map Definitions. > > + > > + SMRAM Save State Map definitions based on contents of the > > + AMD64 Architecture Programmer Manual: > > + Volume 2, System Programming, Section 10.2 SMM Resources > > + > > + Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR> > > + Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved .<BR> > > + SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef AMD_SMRAM_SAVE_STATE_MAP_H_ > > +#define AMD_SMRAM_SAVE_STATE_MAP_H_ > > + > > +/// > > +/// Default SMBASE address > > +/// > > +#define SMM_DEFAULT_SMBASE 0x30000 > > + > > +/// > > +/// Offset of SMM handler from SMBASE > > +/// > > +#define SMM_HANDLER_OFFSET 0x8000 > > + > > +// SMM-Revision Identifier for AMD64 Architecture. > > +#define AMD_SMM_MIN_REV_ID_X64 0x30064 > > + > > +#pragma pack (1) > > + > > +/// > > +/// 32-bit SMRAM Save State Map > > +/// > > +typedef struct { > > + // Padded an extra 0x200 bytes to match Intel/EDK2 > > + UINT8 Reserved[0x200]; // fc00h > > + // AMD Save State area starts @ 0xfe00 > > + UINT8 Reserved1[0xf8]; // fe00h > > + UINT32 SMBASE; // fef8h > > + UINT32 SMMRevId; // fefch > > + UINT16 IORestart; // ff00h > > + UINT16 AutoHALTRestart; // ff02h > > + UINT8 Reserved2[0x84]; // ff04h > > + UINT32 GDTBase; // ff88h > > + UINT64 Reserved3; // ff8ch > > + UINT32 IDTBase; // ff94h > > + UINT8 Reserved4[0x10]; // ff98h > > + UINT32 _ES; // ffa8h > > + UINT32 _CS; // ffach > > + UINT32 _SS; // ffb0h > > + UINT32 _DS; // ffb4h > > + UINT32 _FS; // ffb8h > > + UINT32 _GS; // ffbch > > + UINT32 LDTBase; // ffc0h > > + UINT32 _TR; // ffc4h > > + UINT32 _DR7; // ffc8h > > + UINT32 _DR6; // ffcch > > + UINT32 _EAX; // ffd0h > > + UINT32 _ECX; // ffd4h > > + UINT32 _EDX; // ffd8h > > + UINT32 _EBX; // ffdch > > + UINT32 _ESP; // ffe0h > > + UINT32 _EBP; // ffe4h > > + UINT32 _ESI; // ffe8h > > + UINT32 _EDI; // ffech > > + UINT32 _EIP; // fff0h > > + UINT32 _EFLAGS; // fff4h > > + UINT32 _CR3; // fff8h > > + UINT32 _CR0; // fffch > > +} AMD_SMRAM_SAVE_STATE_MAP32; > > + > > +/// > > +/// 64-bit SMRAM Save State Map > > +/// > > +typedef struct { > > + // Padded an extra 0x200 bytes to match Intel/EDK2 > > + UINT8 Reserved[0x200]; // fc00h > > + // AMD Save State area starts @ 0xfe00 > > + UINT16 _ES; // fe00h > > + UINT16 _ESAttributes; // fe02h > > + UINT32 _ESLimit; // fe04h > > + UINT64 _ESBase; // fe08h > > + > > + UINT16 _CS; // fe10h > > + UINT16 _CSAttributes; // fe12h > > + UINT32 _CSLimit; // fe14h > > + UINT64 _CSBase; // fe18h > > + > > + UINT16 _SS; // fe20h > > + UINT16 _SSAttributes; // fe22h > > + UINT32 _SSLimit; // fe24h > > + UINT64 _SSBase; // fe28h > > + > > + UINT16 _DS; // fe30h > > + UINT16 _DSAttributes; // fe32h > > + UINT32 _DSLimit; // fe34h > > + UINT64 _DSBase; // fe38h > > + > > + UINT16 _FS; // fe40h > > + UINT16 _FSAttributes; // fe42h > > + UINT32 _FSLimit; // fe44h > > + UINT64 _FSBase; // fe48h > > + > > + UINT16 _GS; // fe50h > > + UINT16 _GSAttributes; // fe52h > > + UINT32 _GSLimit; // fe54h > > + UINT64 _GSBase; // fe58h > > + > > + UINT32 _GDTRReserved1; // fe60h > > + UINT16 _GDTRLimit; // fe64h > > + UINT16 _GDTRReserved2; // fe66h > > + // UINT64 _GDTRBase; // fe68h > > + UINT32 _GDTRBaseLoDword; > > + UINT32 _GDTRBaseHiDword; > > + > > + UINT16 _LDTR; // fe70h > > + UINT16 _LDTRAttributes; // fe72h > > + UINT32 _LDTRLimit; // fe74h > > + // UINT64 _LDTRBase; // fe78h > > + UINT32 _LDTRBaseLoDword; > > + UINT32 _LDTRBaseHiDword; > > + > > + UINT32 _IDTRReserved1; // fe80h > > + UINT16 _IDTRLimit; // fe84h > > + UINT16 _IDTRReserved2; // fe86h > > + // UINT64 _IDTRBase; // fe88h > > + UINT32 _IDTRBaseLoDword; > > + UINT32 _IDTRBaseHiDword; > > + > > + UINT16 _TR; // fe90h > > + UINT16 _TRAttributes; // fe92h > > + UINT32 _TRLimit; // fe94h > > + UINT64 _TRBase; // fe98h > > + > > + UINT64 IO_RIP; // fea0h > > + UINT64 IO_RCX; // fea8h > > + UINT64 IO_RSI; // feb0h > > + UINT64 IO_RDI; // feb8h > > + UINT32 IO_DWord; // fec0h > > + UINT8 Reserved1[0x04]; // fec4h > > + UINT8 IORestart; // fec8h > > + UINT8 AutoHALTRestart; // fec9h > > + UINT8 Reserved2[0x06]; // fecah > > + UINT64 EFER; // fed0h > > + UINT64 SVM_Guest; // fed8h > > + UINT64 SVM_GuestVMCB; // fee0h > > + UINT64 SVM_GuestVIntr; // fee8h > > + UINT8 Reserved3[0x0c]; // fef0h > > + UINT32 SMMRevId; // fefch > > + UINT32 SMBASE; // ff00h > > + UINT8 Reserved4[0x14]; // ff04h > > + UINT64 SSP; // ff18h > > + UINT64 SVM_GuestPAT; // ff20h > > + UINT64 SVM_HostEFER; // ff28h > > + UINT64 SVM_HostCR4; // ff30h > > + UINT64 SVM_HostCR3; // ff38h > > + UINT64 SVM_HostCR0; // ff40h > > + UINT64 _CR4; // ff48h > > + UINT64 _CR3; // ff50h > > + UINT64 _CR0; // ff58h > > + UINT64 _DR7; // ff60h > > + UINT64 _DR6; // ff68h > > + UINT64 _RFLAGS; // ff70h > > + UINT64 _RIP; // ff78h > > + UINT64 _R15; // ff80h > > + UINT64 _R14; // ff88h > > + UINT64 _R13; // ff90h > > + UINT64 _R12; // ff98h > > + UINT64 _R11; // ffa0h > > + UINT64 _R10; // ffa8h > > + UINT64 _R9; // ffb0h > > + UINT64 _R8; // ffb8h > > + UINT64 _RDI; // ffc0h > > + UINT64 _RSI; // ffc8h > > + UINT64 _RBP; // ffd0h > > + UINT64 _RSP; // ffd8h > > + UINT64 _RBX; // ffe0h > > + UINT64 _RDX; // ffe8h > > + UINT64 _RCX; // fff0h > > + UINT64 _RAX; // fff8h > > +} AMD_SMRAM_SAVE_STATE_MAP64; > > + > > +/// > > +/// Union of 32-bit and 64-bit SMRAM Save State Maps > > +/// > > +typedef union { > > + AMD_SMRAM_SAVE_STATE_MAP32 x86; > > + AMD_SMRAM_SAVE_STATE_MAP64 x64; > > +} AMD_SMRAM_SAVE_STATE_MAP; > > + > > +#pragma pack () > > + > > +#endif > > diff --git a/MdePkg/MdePkg.ci.yaml b/MdePkg/MdePkg.ci.yaml > > index 6ba85ebe6186..27772ab3be0a 100644 > > --- a/MdePkg/MdePkg.ci.yaml > > +++ b/MdePkg/MdePkg.ci.yaml > > @@ -5,6 +5,7 @@ > > # Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> > > # Copyright (c) 2021, Arm Limited. All rights reserved.<BR> > > # Copyright (c) 2023, Loongson Technology Corporation Limited. All rights > > reserved.<BR> > > +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR> > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > ## > > { > > @@ -67,7 +68,8 @@ > > "Include/Library/PcdLib.h", > > "Include/Library/SafeIntLib.h", > > "Include/Protocol/DebugSupport.h", > > - "Test/UnitTest/Library/BaseSafeIntLib/TestBaseSafeIntLib.c" > > + "Test/UnitTest/Library/BaseSafeIntLib/TestBaseSafeIntLib.c", > > + "Include/Register/Amd/SmramSaveStateMap.h" > > ] > > }, > > ## options defined ci/Plugin/CompilerPlugin > > -- > > 2.25.1 > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102855): https://edk2.groups.io/g/devel/message/102855 Mute This Topic: https://groups.io/mt/98172945/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-