From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "Ni, Ray" <ray.ni@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"abdattar@amd.com" <abdattar@amd.com>
Cc: Paul Grimes <paul.grimes@amd.com>,
Garrett Kirkendall <garrett.kirkendall@amd.com>,
Abner Chang <abner.chang@amd.com>,
"Gao, Liming" <gaoliming@byosoft.com.cn>,
"Liu, Zhiguang" <zhiguang.liu@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v8 1/9] MdePkg: Adds AMD SMRAM save state map
Date: Tue, 11 Apr 2023 18:49:55 +0000 [thread overview]
Message-ID: <CO1PR11MB4929EF1501C5BCB6BF8E62F1D29A9@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <MN6PR11MB824451348393FAF795D13B318C9A9@MN6PR11MB8244.namprd11.prod.outlook.com>
I agree. This is proper use of the term "Smram".
Mike
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, April 11, 2023 12:38 AM
> To: devel@edk2.groups.io; abdattar@amd.com; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Paul Grimes <paul.grimes@amd.com>; Garrett Kirkendall <garrett.kirkendall@amd.com>; Abner Chang <abner.chang@amd.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@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 <abdattar@amd.com>; Paul Grimes
> > <paul.grimes@amd.com>; Garrett Kirkendall <garrett.kirkendall@amd.com>;
> > Abner Chang <abner.chang@amd.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> > Liu, Zhiguang <zhiguang.liu@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.grimes@amd.com>
> > Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> >
> > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > Reviewed-by: Abner Chang <abner.chang@amd.com>
> > Reviewed-by: Michael D Kinney <michael.d.kinney@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
> >
> >
> >
> >
> >
next prev parent reply other threads:[~2023-04-11 18:50 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-10 11:09 [PATCH v8 0/9] Adds AmdSmmCpuFeaturesLib and SmmSmramSaveStateLib Abdul Lateef Attar
2023-04-10 11:09 ` [PATCH v8 1/9] MdePkg: Adds AMD SMRAM save state map Abdul Lateef Attar
2023-04-11 7:38 ` [edk2-devel] " Ni, Ray
2023-04-11 18:49 ` Michael D Kinney [this message]
2023-04-12 6:44 ` Attar, AbdulLateef (Abdul Lateef)
2023-04-10 11:09 ` [PATCH v8 2/9] UefiCpuPkg: Adds SmmSmramSaveStateLib library class Abdul Lateef Attar
2023-04-11 7:50 ` [edk2-devel] " Ni, Ray
2023-04-17 8:46 ` Attar, AbdulLateef (Abdul Lateef)
2023-04-10 11:09 ` [PATCH v8 3/9] UefiCpuPkg: Implements " Abdul Lateef Attar
2023-04-11 8:30 ` [edk2-devel] " Ni, Ray
2023-04-10 11:09 ` [PATCH v8 4/9] UefiCpuPkg/SmmCpuFeaturesLib: Restructure arch-dependent code Abdul Lateef Attar
2023-04-10 11:09 ` [PATCH v8 5/9] UefiCpuPkg: Initial implementation of AMD's SmmCpuFeaturesLib Abdul Lateef Attar
2023-04-11 8:32 ` [edk2-devel] " Ni, Ray
2023-04-10 11:09 ` [PATCH v8 6/9] UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family Abdul Lateef Attar
2023-04-11 8:35 ` [edk2-devel] " Ni, Ray
2023-04-10 11:09 ` [PATCH v8 7/9] UefiCpuPkg: Implements SmmSmramSaveStateLib for Intel Abdul Lateef Attar
2023-04-11 8:54 ` [edk2-devel] " Ni, Ray
2023-04-10 11:09 ` [PATCH v8 8/9] UefiCpuPkg: Uses SmmSmramSaveStateLib library Abdul Lateef Attar
2023-04-10 11:48 ` [edk2-devel] " Chang, Abner
2023-04-11 9:00 ` Ni, Ray
2023-04-10 11:09 ` [PATCH v8 9/9] OvmfPkg: " Abdul Lateef Attar
2023-04-11 9:42 ` Gerd Hoffmann
2023-04-11 10:09 ` [edk2-devel] " Ni, Ray
2023-04-11 10:48 ` Gerd Hoffmann
2023-04-11 11:17 ` Attar, AbdulLateef (Abdul Lateef)
2023-04-11 12:49 ` Ni, Ray
2023-04-18 5:22 ` Attar, AbdulLateef (Abdul Lateef)
2023-04-18 5:38 ` Ni, Ray
2023-04-18 6:42 ` Gerd Hoffmann
2023-04-11 11:12 ` Attar, AbdulLateef (Abdul Lateef)
2023-04-10 16:29 ` [PATCH v8 0/9] Adds AmdSmmCpuFeaturesLib and SmmSmramSaveStateLib Michael D Kinney
2023-04-11 1:07 ` Chang, Abner
2023-04-11 4:16 ` Attar, AbdulLateef (Abdul Lateef)
2023-04-11 18:48 ` Michael D Kinney
2023-04-12 2:21 ` Attar, AbdulLateef (Abdul Lateef)
2023-04-11 18:45 ` [edk2-devel] " Michael D Kinney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO1PR11MB4929EF1501C5BCB6BF8E62F1D29A9@CO1PR11MB4929.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox