Re: [PATCH v4 0/5] Enhanced SMM support for AMD-based x86 systems.

["Duran, Leo" <leo.duran@amd.com>]

> -----Original Message-----
> From: Marvin H?user [mailto:Marvin.Haeuser@outlook.com]
> Sent: Wednesday, October 04, 2017 8:19 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Duran, Leo
> <leo.duran@amd.com>
> Subject: RE: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based
> x86 systems.
> 
> Hey Jiewen,
> Hey Leo,
> 
> May I suggest replacing "StandardSignatureIsAuthenticAMD()" and the PCDs
> introduced by the series with a Fixed "PcdCpuVendor" enum or alike?

Hi Marvin,
I'm not sure I follow your suggestion.
It seems like the platform .DSC would then need to set PcdCpuVendor to an appropriate value at build-time... No?
Leo.

> The contra of "StandardSignatureIsAuthenticAMD()" is that it's a runtime
> action. From my point of view, this has no notable advantage as boards use
> either an Intel or an AMD chipset and hence the EDK2 Firmware Package has
> compilation-time knowledge of the target platform.
> On the other hand, the PCDs introduced by this patch cause the contra that
> the platform DSC must set the correct vendor-specific (intel, AMD) value.
> If the code checked for the CPU Vendor PCD and used the correct values
> based on that, the values for the other vendors would get optimized away
> (no unnecessary runtime actions) and the platform package owner does not
> need to worry about setting PCDs to AMD-specific values except for the CPU
> Vendor PCD.
> Backwards-compatibility could be ensured by defaulting to some reserved
> value for the PCD and handling detection via
> StandardSignatureIsAuthenticAMD() if the platform DSC does not change it.
> 
> Thanks,
> Marvin.
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Yao, Jiewen
> > Sent: Thursday, October 5, 2017 2:49 AM
> > To: Leo Duran <leo.duran@amd.com>; edk2-devel@lists.01.org
> > Subject: Re: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based
> > x86 systems.
> >
> > Hi Leo
> > I do not suggest we introduce PcdCpuSmmSmramSaveStateMapOffset.
> >
> > This is unnecessary, because it is CPU attribute but not some end user
> > configurable data.
> >
> > I think we can use CPUID to distinguish AMD from INTEL. Is that
> > technically possible?
> >
> > I found we already have code at
> >
> >
> C:\home\EdkIIGit\edk2\UefiCpuPkg\Library\BaseXApicX2ApicLib\BaseXApic
> > X2ApicLib.c(1206):    if (StandardSignatureIsAuthenticAMD()) {
> >
> >
> C:\home\EdkIIGit\edk2\UefiCpuPkg\Library\BaseXApicLib\BaseXApicLib.c(11
> > 11):    if (StandardSignatureIsAuthenticAMD()) {
> >
> >
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > > Of Leo Duran
> > > Sent: Thursday, October 5, 2017 3:02 AM
> > > To: edk2-devel@lists.01.org
> > > Subject: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based
> > > x86 systems.
> > >
> > > This patch-set introduces a couple of FixedPCDs to replace
> > > Intel-specific macros, and better support AMD-based x86 systems.
> > >
> > > 1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map
> > Offset.
> > > 2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.
> > >
> > > Changes since v3:
> > > Correction on cover letter.
> > >
> > > Changes since v2:
> > > The intent of this revision is to maintain compatibility with
> > > existing packages. To that end, changes to OvmgfPkg and QuarkSocPkg
> > > are
> > reverted.
> > > Moreover, pertinent macros are replaced in the C code, rather than
> > > on header files that are shared globally.
> > >
> > > Changes since v1:
> > > Revision to Cc list for UefiCpuPkg.
> > >
> > > Leo Duran (5):
> > >   UefiCpuPkg/UefiCpuPkg.dec: Create FixedPCDs for SMM support
> > >   UefiCpuPkg/PiSmmCpuDxeSmm: Consume FixedPCDs to enhance SMM
> > support
> > >   UefiCpuPkg/PiSmmCpuDxeSmm: Use FixedPCDs to enhance SMM
> > support
> > >   UefiCpuPkg/SmmCpuFeaturesLib: Consume FixedPCD to enhance SMM
> > > support
> > >   UefiCpuPkg/SmmCpuFeaturesLib: Use FixedPCD on non-STM library
> > >
> > >  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c      |  4
> > > +++-
> > >  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf    |  5
> > > +++++
> > >  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |
> 5
> > > +++++
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c                    |  4
> > > +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S                     |  4
> > > +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm                   |  4
> > > +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                  |  4
> > > +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c                    |
> > > 10 +++++-----
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h                    |
> > > 2 --
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf                  |
> > > 4 ++++
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> > > |  4 +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c                    |  4
> > > +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c                     |
> > > 4 +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S                      |  4
> > > +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm                    |  4
> > > +++-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                   |  4
> > > +++-
> > >  UefiCpuPkg/UefiCpuPkg.dec                                     |  9
> > > +++++++++
> > >  17 files changed, 61 insertions(+), 18 deletions(-)
> > >
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel