public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Marvin H?user <Marvin.Haeuser@outlook.com>
To: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>,
	"leo.duran@amd.com" <leo.duran@amd.com>
Subject: Re: [PATCH v4 0/5] Enhanced SMM support for AMD-based x86 systems.
Date: Thu, 5 Oct 2017 01:18:48 +0000	[thread overview]
Message-ID: <AM4PR06MB1491E7084F4B9B8C9EAD63FE80700@AM4PR06MB1491.eurprd06.prod.outlook.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A9CFB71@shsmsx102.ccr.corp.intel.com>

Hey Jiewen,
Hey Leo,

May I suggest replacing "StandardSignatureIsAuthenticAMD()" and the PCDs introduced by the series with a Fixed "PcdCpuVendor" enum or alike?
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


  reply	other threads:[~2017-10-05  1:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-04 19:02 [PATCH v4 0/5] Enhanced SMM support for AMD-based x86 systems Leo Duran
2017-10-04 19:02 ` [PATCH v4 1/5] UefiCpuPkg/UefiCpuPkg.dec: Create FixedPCDs for SMM support Leo Duran
2017-10-04 19:02 ` [PATCH v4 2/5] UefiCpuPkg/PiSmmCpuDxeSmm: Consume FixedPCDs to enhance " Leo Duran
2017-10-04 19:02 ` [PATCH v4 3/5] UefiCpuPkg/PiSmmCpuDxeSmm: Use " Leo Duran
2017-10-04 19:02 ` [PATCH v4 4/5] UefiCpuPkg/SmmCpuFeaturesLib: Consume FixedPCD " Leo Duran
2017-10-04 19:02 ` [PATCH v4 5/5] UefiCpuPkg/SmmCpuFeaturesLib: Use FixedPCD on non-STM library Leo Duran
2017-10-05  0:49 ` [PATCH v4 0/5] Enhanced SMM support for AMD-based x86 systems Yao, Jiewen
2017-10-05  1:18   ` Marvin H?user [this message]
2017-10-05  3:19     ` Yao, Jiewen
2017-10-05 14:57     ` Duran, Leo
2017-10-05 19:09       ` Marvin H?user
2017-10-06 15:27   ` Duran, Leo

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=AM4PR06MB1491E7084F4B9B8C9EAD63FE80700@AM4PR06MB1491.eurprd06.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