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

["Yao, Jiewen" <jiewen.yao@intel.com>]

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