[PATCH v2 0/9] Enhanced SMM support to AMD-based x86 systems.

[PATCH v2 0/9] Enhanced SMM support to AMD-based x86 systems.

[Leo Duran]

UefiCpuPkg:
This patch-set introduces a couple of FixedPCDs to replace
Intel-specific macros, for SMM support on AMD-based x86 systems.
    
1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

OvmfPkg and QuarkSocPkg:
The PcdCpuSmmSmramSaveStateMapOffset PCD is declared just to resolve
the macro replaced by the shared Library/SmmCpuFeaturesLib.h file.

Changes since v1:
Revision to Cc list for UefiCpuPkg.

Leo Duran (9):
  UefiCpuPkg: UefiCpuPkg.dec
  UefiCpuPkg: PiSmmCpuDxeSmm driver.
  UefiCpuPkg: SmmCpuFeaturesLib library.
  OvmfPkg: SmmCpuFeaturesLib library.
  QuarkSocPkg: SmmCpuFeaturesLib library.
  UefiCpuPkg: Register/SmramSaveStateMap.h
  UefiCpuPkg: PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
  UefiCpuPkg: PiSmmCpuDxeSmm driver.
  UefiCpuPkg: SmmCpuFeaturesLib library.

 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf          | 5 +++++
 .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf              | 4 ++++
 UefiCpuPkg/Include/Register/SmramSaveStateMap.h                  | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S             | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm           | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm          | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf       | 5 +++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf    | 6 ++++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.S              | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.asm            | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm           | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S                        | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm                      | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                     | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h                       | 2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf                     | 4 ++++
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S                         | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm                       | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                      | 4 +++-
 UefiCpuPkg/UefiCpuPkg.dec                                        | 9 +++++++++
 20 files changed, 73 insertions(+), 14 deletions(-)

-- 
2.7.4

[PATCH v2 1/9] UefiCpuPkg: UefiCpuPkg.dec

[Leo Duran]

Introduce a couple of FixedPCDs to replace Intel-specific macros.
The new PCDs will allow SMM support on AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 UefiCpuPkg/UefiCpuPkg.dec | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 3bd8740..c92c56e 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -2,6 +2,7 @@
 # This Package provides UEFI compatible CPU modules and libraries.
 #
 # Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Incorporated. 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.
@@ -204,6 +205,14 @@
   # @Prompt If CPU features will be initialized during S3 resume.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitOnS3Resume|FALSE|BOOLEAN|0x0000001D
 
+  ## Specifies the Offset of SMRAM Save State Map from SMBASE.
+  # @Prompt SMRAM Save State Map Offset.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset|0xFC00|UINT16|0x32132113
+
+  ## Specifies the PROCESSOR SMM DESCRIPTOR Offset in SMRAM.
+  # @Prompt SMRAM PSD Offset.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmPSDOffset|0xFB00|UINT16|0x32132114
+
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## Specifies max supported number of Logical Processors.
   # @Prompt Configure max supported number of Logical Processors
-- 
2.7.4

[PATCH v2 2/9] UefiCpuPkg: PiSmmCpuDxeSmm driver.

[Leo Duran]

Introduce a couple of FixedPCDs to replace Intel-specific macros.
The new PCDs will allow SMM support on AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 099792e..bf237f7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -147,6 +147,10 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer             ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock         ## CONSUMES
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset       ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmPSDOffset                     ## CONSUMES
+
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileSize                   ## SOMETIMES_CONSUMES
-- 
2.7.4

[PATCH v2 3/9] UefiCpuPkg: SmmCpuFeaturesLib library.

[Leo Duran]

Introduce a couple of FixedPCDs to replace Intel-specific macros.
The new PCDs will allow SMM support on AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf    | 5 +++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 77908b0..1be2671 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -2,6 +2,8 @@
 #  The CPU specific programming for PiSmmCpuDxeSmm module.
 #
 #  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017, AMD Incorporated. 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
@@ -36,5 +38,8 @@
   MemoryAllocationLib
   DebugLib
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset       ## CONSUMES
+
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index db8dcdc..b1937de 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -3,6 +3,8 @@
 #  is included.
 #
 #  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017, AMD Incorporated. 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
@@ -78,6 +80,10 @@
   gEfiAcpi20TableGuid                      ## SOMETIMES_CONSUMES ## SystemTable
   gEfiAcpi10TableGuid                      ## SOMETIMES_CONSUMES ## SystemTable
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset       ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmPSDOffset                     ## CONSUMES
+
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMsegSize                         ## SOMETIMES_CONSUMES
-- 
2.7.4

[PATCH v2 4/9] OvmfPkg: SmmCpuFeaturesLib library.

[Leo Duran]

Introduce a FixedPCD to replace an Intel-specific macro.

PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.

The new PCD allows for SMM support on AMD-based x86 systems,
and it is declared here just to resolve the macro that will be
replaced by the shared Library/SmmCpuFeaturesLib.h file.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 31edf3a..f6c2c4c 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -2,6 +2,8 @@
 #  The CPU specific programming for PiSmmCpuDxeSmm module.
 #
 #  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017, AMD Incorporated. 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
@@ -36,3 +38,6 @@
   PcdLib
   DebugLib
   SmmServicesTableLib
+
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset       ## CONSUMES
-- 
2.7.4

[PATCH v2 5/9] QuarkSocPkg: SmmCpuFeaturesLib library.

[Leo Duran]

Introduce a FixedPCD to replace an Intel-specific macro.

PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.

The new PCD allows for SMM support on AMD-based x86 systems,
and it is declared here just to resolve the macro that will be
replaced by the shared Library/SmmCpuFeaturesLib.h file.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Kelly Steele <kelly.steele@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 .../QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 267ac84..8b1f202 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -2,6 +2,8 @@
 #  The CPU specific programming for PiSmmCpuDxeSmm module.
 #
 #  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017, AMD Incorporated. 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
@@ -34,3 +36,5 @@
   DebugLib
   QNCAccessLib
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset       ## CONSUMES
-- 
2.7.4

[PATCH v2 6/9] UefiCpuPkg: Register/SmramSaveStateMap.h

[Leo Duran]

Add a FixedPCD to replace an Intel-specific hard-coded macro.
The new PCD allows SMM support on AMD-based x86 systems.

PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 UefiCpuPkg/Include/Register/SmramSaveStateMap.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Include/Register/SmramSaveStateMap.h b/UefiCpuPkg/Include/Register/SmramSaveStateMap.h
index a7c7562..2167f33 100644
--- a/UefiCpuPkg/Include/Register/SmramSaveStateMap.h
+++ b/UefiCpuPkg/Include/Register/SmramSaveStateMap.h
@@ -8,6 +8,8 @@ Intel(R) 64 and IA-32 Architectures Software Developer's Manual
   Volume 3C, Section 34.7 Managing Synchronous and Asynchronous SMIs
 
 Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017, AMD Incorporated. 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
@@ -34,7 +36,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 ///
 /// Offset of SMRAM Save State Map from SMBASE
 ///
-#define SMRAM_SAVE_STATE_MAP_OFFSET  0xfc00
+#define SMRAM_SAVE_STATE_MAP_OFFSET  (FixedPcdGet16 (PcdCpuSmmSmramSaveStateMapOffset))
 
 #pragma pack (1)
 
-- 
2.7.4

[PATCH v2 7/9] UefiCpuPkg: PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h

[Leo Duran]

Add a FixedPCD to replace an Intel-specific hard-coded macro.
The new PCD allows SMM support on AMD-based x86 systems.

PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 1cf85c1..06b9767 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -360,7 +360,7 @@ typedef struct {
   UINT32       MsrIndex;
 } MP_MSR_LOCK;
 
-#define SMM_PSD_OFFSET              0xfb00
+#define SMM_PSD_OFFSET		(FixedPcdGet16 (PcdCpuSmmPSDOffset))
 
 ///
 /// All global semaphores' pointer
-- 
2.7.4

[PATCH v2 8/9] UefiCpuPkg: PiSmmCpuDxeSmm driver.

[Leo Duran]

Add a FixedPCD to replace an Intel-specific hard-coded macro.
The new PCD allows SMM support on AMD-based x86 systems.

PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S    | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm  | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S     | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm   | 4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm  | 4 +++-
 6 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
index 3243a91..25af6e7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
@@ -1,6 +1,8 @@
 #------------------------------------------------------------------------------
 #
 # Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Incorporated. 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
@@ -35,7 +37,7 @@ ASM_GLOBAL  ASM_PFX(gSmiHandlerIdtr)
 #
 # Constants relating to PROCESSOR_SMM_DESCRIPTOR
 #
-.equ            DSC_OFFSET, 0xfb00
+.equ            DSC_OFFSET, (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 .equ            DSC_GDTPTR, 0x30
 .equ            DSC_GDTSIZ, 0x38
 .equ            DSC_CS, 14
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
index 8296f36..f526778 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
@@ -1,5 +1,7 @@
 ;------------------------------------------------------------------------------ ;
 ; Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Incorporated. 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
@@ -29,7 +31,7 @@ MSR_EFER_XD   EQU     0800h
 ;
 ; Constants relating to PROCESSOR_SMM_DESCRIPTOR
 ;
-DSC_OFFSET    EQU     0fb00h
+DSC_OFFSET    EQU     (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 DSC_GDTPTR    EQU     30h
 DSC_GDTSIZ    EQU     38h
 DSC_CS        EQU     14
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
index 4d2383f..9092dcc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
@@ -1,5 +1,7 @@
 ;------------------------------------------------------------------------------ ;
 ; Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Incorporated. 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
@@ -25,7 +27,7 @@
 ;
 ; Constants relating to PROCESSOR_SMM_DESCRIPTOR
 ;
-%define DSC_OFFSET 0xfb00
+%define DSC_OFFSET (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 %define DSC_GDTPTR 0x30
 %define DSC_GDTSIZ 0x38
 %define DSC_CS 14
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S
index 600d862..ebea477 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S
@@ -1,6 +1,8 @@
 #------------------------------------------------------------------------------
 #
 # Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Incorporated. 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
@@ -34,7 +36,7 @@ ASM_GLOBAL  ASM_PFX(gSmiHandlerIdtr)
 #
 # Constants relating to PROCESSOR_SMM_DESCRIPTOR
 #
-.equ            DSC_OFFSET, 0xfb00
+.equ            DSC_OFFSET, (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 .equ            DSC_GDTPTR, 0x30
 .equ            DSC_GDTSIZ, 0x38
 .equ            DSC_CS, 14
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm
index c74f82a..ff46838 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm
@@ -1,5 +1,7 @@
 ;------------------------------------------------------------------------------ ;
 ; Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Incorporated. 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
@@ -39,7 +41,7 @@ MSR_EFER_XD   EQU     0800h
 ;
 ; Constants relating to PROCESSOR_SMM_DESCRIPTOR
 ;
-DSC_OFFSET    EQU     0fb00h
+DSC_OFFSET    EQU     (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 DSC_GDTPTR    EQU     30h
 DSC_GDTSIZ    EQU     38h
 DSC_CS        EQU     14
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
index c3c094f..f55da9b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
@@ -1,5 +1,7 @@
 ;------------------------------------------------------------------------------ ;
 ; Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Incorporated. 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
@@ -29,7 +31,7 @@
 ;
 ; Constants relating to PROCESSOR_SMM_DESCRIPTOR
 ;
-%define DSC_OFFSET 0xfb00
+%define DSC_OFFSET (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 %define DSC_GDTPTR 0x30
 %define DSC_GDTSIZ 0x38
 %define DSC_CS 14
-- 
2.7.4

[PATCH v2 9/9] UefiCpuPkg: SmmCpuFeaturesLib library.

[Leo Duran]

Add a FixedPCD to replace an Intel-specific hard-coded macro.
The new PCD allows SMM support on AMD-based x86 systems.

PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S    | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm  | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.S     | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.asm   | 4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm  | 4 +++-
 6 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S
index 4c0f8c8..d3c291e 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S
@@ -1,6 +1,8 @@
 #------------------------------------------------------------------------------
 #
 # Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Incorporated. 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
@@ -36,7 +38,7 @@ ASM_GLOBAL  ASM_PFX(gStmSmiHandlerIdtr)
 #
 # Constants relating to TXT_PROCESSOR_SMM_DESCRIPTOR
 #
-.equ            DSC_OFFSET, 0xfb00
+.equ            DSC_OFFSET, (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 .equ            DSC_GDTPTR, 0x48
 .equ            DSC_GDTSIZ, 0x50
 .equ            DSC_CS, 0x14
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm
index 91dc1eb..096f353 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm
@@ -1,5 +1,7 @@
 ;------------------------------------------------------------------------------ ;
 ; Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Incorporated. 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
@@ -29,7 +31,7 @@ MSR_EFER_XD   EQU     0800h
 ;
 ; Constants relating to TXT_PROCESSOR_SMM_DESCRIPTOR
 ;
-DSC_OFFSET    EQU     0fb00h
+DSC_OFFSET    EQU     (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 DSC_GDTPTR    EQU     48h
 DSC_GDTSIZ    EQU     50h
 DSC_CS        EQU     14h
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm
index 00c0f067..696af46 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm
@@ -1,5 +1,7 @@
 ;------------------------------------------------------------------------------ ;
 ; Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Incorporated. 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
@@ -25,7 +27,7 @@
 ;
 ; Constants relating to TXT_PROCESSOR_SMM_DESCRIPTOR
 ;
-%define DSC_OFFSET 0xfb00
+%define DSC_OFFSET (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 %define DSC_GDTPTR 0x48
 %define DSC_GDTSIZ 0x50
 %define DSC_CS 0x14
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.S b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.S
index 1f9f91c..de0f71b 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.S
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.S
@@ -1,6 +1,8 @@
 #------------------------------------------------------------------------------
 #
 # Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Incorporated. 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
@@ -35,7 +37,7 @@ ASM_GLOBAL  ASM_PFX(gStmSmiHandlerIdtr)
 #
 # Constants relating to TXT_PROCESSOR_SMM_DESCRIPTOR
 #
-.equ            DSC_OFFSET, 0xfb00
+.equ            DSC_OFFSET, (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 .equ            DSC_GDTPTR, 0x48
 .equ            DSC_GDTSIZ, 0x50
 .equ            DSC_CS, 0x14
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.asm b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.asm
index ad51e07..3e4e080 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.asm
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.asm
@@ -1,5 +1,7 @@
 ;------------------------------------------------------------------------------ ;
 ; Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Incorporated. 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
@@ -40,7 +42,7 @@ MSR_EFER_XD   EQU     0800h
 ;
 ; Constants relating to TXT_PROCESSOR_SMM_DESCRIPTOR
 ;
-DSC_OFFSET    EQU     0fb00h
+DSC_OFFSET    EQU     (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 DSC_GDTPTR    EQU     48h
 DSC_GDTSIZ    EQU     50h
 DSC_CS        EQU     14h
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm
index bcac643..8425499 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm
@@ -1,5 +1,7 @@
 ;------------------------------------------------------------------------------ ;
 ; Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2017, AMD Incorporated. 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
@@ -29,7 +31,7 @@
 ;
 ; Constants relating to TXT_PROCESSOR_SMM_DESCRIPTOR
 ;
-%define DSC_OFFSET 0xfb00
+%define DSC_OFFSET (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 %define DSC_GDTPTR 0x48
 %define DSC_GDTSIZ 0x50
 %define DSC_CS 0x14
-- 
2.7.4

Re: [PATCH v2 0/9] Enhanced SMM support to AMD-based x86 systems.

[Yao, Jiewen]

Thanks Leo.
I agree we should support AMD-based X86, since the most code is sharable.

[1] Comment for PcdCpuSmmSmramSaveStateMapOffset.

I am a little concerned about the patch 6/9, because it is an incompatible change.
This change will cause all existing intel platform code change, to add PCD in INF, such as OvmfPkg (patch 4/9) and QuarkPkg (patch 5/9).

I am thinking an compatible way, to prevent existing intel platform code change.

1) We can define below according to Intel SDM and AMD SDM.

#define INTEL_SMRAM_SAVE_STATE_MAP_OFFSET  0xfc00
#define AMD_SMRAM_SAVE_STATE_MAP_OFFSET  0xfe00
// This is to provide compatibility.
#define SMRAM_SAVE_STATE_MAP_OFFSET  INTEL_SMRAM_SAVE_STATE_MAP_OFFSET

2) Because the system has capability to *detect* the CPU, there is no need to let user to *configure*.
I do not suggest we use PCD. IMHO, it is more a system attribute, instead of a user configurable data. For example, a user cannot configure it to 0xFE00 for Intel CPU, or 0xFC00 for AMD CPU.

3) I think we can have a CPUID check in the entrypoint, and patch the OFFSET at anywhere.


[2] Comment for PcdCpuSmmPSDOffset.
I do not mind, since it is driver internal configuration and it won't break the compatibility.

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo
> Duran
> Sent: Wednesday, October 4, 2017 6:38 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH v2 0/9] Enhanced SMM support to AMD-based x86
> systems.
> 
> UefiCpuPkg:
> This patch-set introduces a couple of FixedPCDs to replace
> Intel-specific macros, for SMM support on AMD-based x86 systems.
> 
> 1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
> 2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.
> 
> OvmfPkg and QuarkSocPkg:
> The PcdCpuSmmSmramSaveStateMapOffset PCD is declared just to resolve
> the macro replaced by the shared Library/SmmCpuFeaturesLib.h file.
> 
> Changes since v1:
> Revision to Cc list for UefiCpuPkg.
> 
> Leo Duran (9):
>   UefiCpuPkg: UefiCpuPkg.dec
>   UefiCpuPkg: PiSmmCpuDxeSmm driver.
>   UefiCpuPkg: SmmCpuFeaturesLib library.
>   OvmfPkg: SmmCpuFeaturesLib library.
>   QuarkSocPkg: SmmCpuFeaturesLib library.
>   UefiCpuPkg: Register/SmramSaveStateMap.h
>   UefiCpuPkg: PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
>   UefiCpuPkg: PiSmmCpuDxeSmm driver.
>   UefiCpuPkg: SmmCpuFeaturesLib library.
> 
>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf          | 5
> +++++
>  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf              | 4
> ++++
>  UefiCpuPkg/Include/Register/SmramSaveStateMap.h                  | 4
> +++-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S             | 4
> +++-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm           | 4
> +++-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm          | 4
> +++-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf       | 5
> +++++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf    | 6
> ++++++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.S              | 4
> +++-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.asm            | 4
> +++-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm           | 4
> +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S                        | 4
> +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm                      |
> 4 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                     |
> 4 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> | 2 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> | 4 ++++
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S                         |
> 4 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm                       |
> 4 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                      |
> 4 +++-
>  UefiCpuPkg/UefiCpuPkg.dec                                        | 9
> +++++++++
>  20 files changed, 73 insertions(+), 14 deletions(-)
> 
> --
> 2.7.4
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 0/9] Enhanced SMM support to AMD-based x86 systems.

[Kinney, Michael D]

Leo,

The general design issue here is that a PCD and PCD macro were
added to a UefiCpuPkg include file Include/Register/SmramSaveStateMap.h.
This means any library or module that includes this package include
file will break the build until the PCD is added to the INF file for
that library or module.

The INF file for a module is supposed to express the PCDs that the
library or module sources access.  We do not expect #include
statements from package include files to require adding additional
PCDs.

Updating the code for these values to be detected or configurable
is a good idea.

One option is to update the C code that currently uses the #define
names to use the PCD access function directly, instead of adding
the PcdGetxxx() to the Include/Register/SmramSaveStateMap.h file.

Jiewen's idea to remove the PCD and detect the offset from CPUID
also looks like a reasonable approach.

Thanks,

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> Behalf Of Yao, Jiewen
> Sent: Tuesday, October 3, 2017 5:50 PM
> To: Leo Duran <leo.duran@amd.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v2 0/9] Enhanced SMM support to AMD-
> based x86 systems.
> 
> Thanks Leo.
> I agree we should support AMD-based X86, since the most code is
> sharable.
> 
> [1] Comment for PcdCpuSmmSmramSaveStateMapOffset.
> 
> I am a little concerned about the patch 6/9, because it is an
> incompatible change.
> This change will cause all existing intel platform code change,
> to add PCD in INF, such as OvmfPkg (patch 4/9) and QuarkPkg
> (patch 5/9).
> 
> I am thinking an compatible way, to prevent existing intel
> platform code change.
> 
> 1) We can define below according to Intel SDM and AMD SDM.
> 
> #define INTEL_SMRAM_SAVE_STATE_MAP_OFFSET  0xfc00
> #define AMD_SMRAM_SAVE_STATE_MAP_OFFSET  0xfe00
> // This is to provide compatibility.
> #define SMRAM_SAVE_STATE_MAP_OFFSET
> INTEL_SMRAM_SAVE_STATE_MAP_OFFSET
> 
> 2) Because the system has capability to *detect* the CPU, there
> is no need to let user to *configure*.
> I do not suggest we use PCD. IMHO, it is more a system
> attribute, instead of a user configurable data. For example, a
> user cannot configure it to 0xFE00 for Intel CPU, or 0xFC00 for
> AMD CPU.
> 
> 3) I think we can have a CPUID check in the entrypoint, and
> patch the OFFSET at anywhere.
> 
> 
> [2] Comment for PcdCpuSmmPSDOffset.
> I do not mind, since it is driver internal configuration and it
> won't break the compatibility.
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> Behalf Of Leo
> > Duran
> > Sent: Wednesday, October 4, 2017 6:38 AM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH v2 0/9] Enhanced SMM support to AMD-
> based x86
> > systems.
> >
> > UefiCpuPkg:
> > This patch-set introduces a couple of FixedPCDs to replace
> > Intel-specific macros, for SMM support on AMD-based x86
> systems.
> >
> > 1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map
> Offset.
> > 2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in
> SMRAM.
> >
> > OvmfPkg and QuarkSocPkg:
> > The PcdCpuSmmSmramSaveStateMapOffset PCD is declared just to
> resolve
> > the macro replaced by the shared Library/SmmCpuFeaturesLib.h
> file.
> >
> > Changes since v1:
> > Revision to Cc list for UefiCpuPkg.
> >
> > Leo Duran (9):
> >   UefiCpuPkg: UefiCpuPkg.dec
> >   UefiCpuPkg: PiSmmCpuDxeSmm driver.
> >   UefiCpuPkg: SmmCpuFeaturesLib library.
> >   OvmfPkg: SmmCpuFeaturesLib library.
> >   QuarkSocPkg: SmmCpuFeaturesLib library.
> >   UefiCpuPkg: Register/SmramSaveStateMap.h
> >   UefiCpuPkg: PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> >   UefiCpuPkg: PiSmmCpuDxeSmm driver.
> >   UefiCpuPkg: SmmCpuFeaturesLib library.
> >
> >  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> | 5
> > +++++
> >  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> | 4
> > ++++
> >  UefiCpuPkg/Include/Register/SmramSaveStateMap.h
> | 4
> > +++-
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S
> | 4
> > +++-
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm
> | 4
> > +++-
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm
> | 4
> > +++-
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> | 5
> > +++++
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> | 6
> > ++++++
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.S
> | 4
> > +++-
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.asm
> | 4
> > +++-
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm
> | 4
> > +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
> | 4
> > +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
> |
> > 4 +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> |
> > 4 +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> > | 2 +-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> > | 4 ++++
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S
> |
> > 4 +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm
> |
> > 4 +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> |
> > 4 +++-
> >  UefiCpuPkg/UefiCpuPkg.dec
> | 9
> > +++++++++
> >  20 files changed, 73 insertions(+), 14 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

Re: [PATCH v2 0/9] Enhanced SMM support to AMD-based x86 systems.

[Laszlo Ersek]

On 10/04/17 03:22, Kinney, Michael D wrote:
> Leo,
> 
> The general design issue here is that a PCD and PCD macro were
> added to a UefiCpuPkg include file Include/Register/SmramSaveStateMap.h.
> This means any library or module that includes this package include
> file will break the build until the PCD is added to the INF file for
> that library or module.
> 
> The INF file for a module is supposed to express the PCDs that the
> library or module sources access.  We do not expect #include
> statements from package include files to require adding additional
> PCDs.
> 
> Updating the code for these values to be detected or configurable
> is a good idea.
> 
> One option is to update the C code that currently uses the #define
> names to use the PCD access function directly, instead of adding
> the PcdGetxxx() to the Include/Register/SmramSaveStateMap.h file.
> 
> Jiewen's idea to remove the PCD and detect the offset from CPUID
> also looks like a reasonable approach.

In addition to the above, I'd like to request a bit more "telling"
subject lines for the patches. Using the OVMF patch as example,

  OvmfPkg: SmmCpuFeaturesLib library.

is not really helpful; I'd consider the following an improvement:

  OvmfPkg/SmmCpuFeaturesLib: consume SMRAM Save State Map Offset PCD

66 characters -- we generally limit subjects to 74.

(Of course I realize the patch might entirely be replaced in the next
version, based on Jiewen's and Mike's feedback -- that's OK with me, I
just wanted to give an example.)

Thanks!
Laszlo