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

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

Hi Leo
I just have another thought, when I review code again.

Do we *have to* make AMD SMM PSD offset to 0xfc00?
SMM PSD is just a *software* concept. Not hardware requirement.
What is broken, if we design AMD SMM PSD offset to be 0xfb00, (same as existing code)?

Thank you
Yao Jiewen

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen
Sent: Friday, October 13, 2017 9:53 AM
To: Leo Duran <leo.duran@amd.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.

HI Leo
Thank you very much. This patch looks good to me in general.

Some minor comment:

1) For AMD smm save state.
I saw Paolo gave the comment on how to detect AMD save state. I do not have strong opinion on that. I think you and Paolo can make decision.

I recommend we move AMD_SMRAM_SAVE_STATE_MAP_OFFSET to UefiCpuPkg\Include\Register\Amd\SmramSaveStateMap.h, because it is standard.
+//
+// Definitions for AMD systems are based on contents of the
+// AMD64 Architecture Programmer's Manual
+// Volume 2: System Programming, Section 10 System-Management Mode
+//
+#define AMD_SMRAM_SAVE_STATE_MAP_OFFSET  0xfe00

We can leave AMD_SMM_PSD_OFFSET in UefiCpuPkg/PiSmmCpuDxeSmm, if it is implementation specific.
+#define       AMD_SMM_PSD_OFFSET         0xfc00



2) For Intel save state, I assume you already did some test to make sure there is no regression.
If so, would you please add some description on what test you have done?
For example,
  If both IA32 and X64 are validated?
  If all three .S, .asm, .nasm are validated?
  If OS boot and S3 resume are validated?

If you did any other test, please also add.

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo
> Duran
> Sent: Thursday, October 12, 2017 3:45 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86
> systems.
>
> This patch-set replaces Intel-specific macros with global variables
> to provide support for AMD-based x86 systems.
>
> The replaced macros are:
> 1) SRAM_SAVE_STATE_MAP_OFFSET
> 2) TXT_SMM_PSD_OFFSET
> 3) SMM_PSD_OFFSET
>
> Changes since v4:
> Make runtime CPUID checks and use global variables instead of PCD's.
>
> 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 (2):
>   UefiCpuPkg/SmmCpuFeaturesLib: Use global variables to replace macros
>   UefiCpuPkg/PiSmmCpuDxeSmm: Use global variables to replace macros
>
>  .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.S      | 28 ++++++---
>  .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.asm    | 29 ++++++---
>  .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm   | 43 +++++++++----
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCommon.h   | 48
> +++++++++++++++
>  .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  | 59
> +++++++++++++++---
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf        |  3 +
>  .../SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf     |  3 +
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c      | 39 ++++++++++--
>  .../Library/SmmCpuFeaturesLib/X64/SmiEntry.S       | 28 ++++++---
>  .../Library/SmmCpuFeaturesLib/X64/SmiEntry.asm     | 30 ++++++---
>  .../Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm    | 47 ++++++++++----
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c         | 22 ++++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S          | 28 ++++++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm        | 21 +++++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm       | 43
> +++++++++----
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c         | 72
> ++++++++++++++++++++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h         | 17 ++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 18 +++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c         | 20 +++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c          | 22 ++++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S           | 34 ++++++----
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm         | 22 +++++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm        | 45
> ++++++++++----
>  23 files changed, 547 insertions(+), 174 deletions(-)
>  create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCommon.h
>
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel