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

Leo Duran posted 2 patches 6 years, 6 months ago
Failed in applying to current master (apply log)
.../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
[edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Leo Duran 6 years, 6 months ago
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
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Yao, Jiewen 6 years, 6 months ago
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
> 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
> 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: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Yao, Jiewen 6 years, 6 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Duran, Leo 6 years, 6 months ago

> -----Original Message-----
> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> Sent: Thursday, October 12, 2017 8:53 PM
> To: Duran, Leo <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.
Hi Yao,

Sure thing, I will do that.

Leo

> +//
> +// 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.
> 

Hi Yao,

I have not done runtime validation testing.
Instead, I tested the assembly code snippets in a vacuum (in their .asm, .nasm, and .S forms),
to ensure the source produced the expected compiled code, and the expected execution.

So any runtime 'Tested-by' would be greatly appreciated
Thanks,
Leo.

> 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
> > 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
> > 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: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Yao, Jiewen 6 years, 6 months ago
for runtime test, I recommend using ovmf. You don't need real hardware. It can run both 32bit or 64bit. It can run in both Linux and windows.

You need use -D SMM_REQUIRE option to build ovmf. 
If you have any problem, Laszlo is the good contact.

thank you!
Yao, Jiewen


> 在 2017年10月15日,上午12:04,Duran, Leo <leo.duran@amd.com> 写道:
> 
> 
> 
>> -----Original Message-----
>> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
>> Sent: Thursday, October 12, 2017 8:53 PM
>> To: Duran, Leo <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.
> Hi Yao,
> 
> Sure thing, I will do that.
> 
> Leo
> 
>> +//
>> +// 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.
>> 
> 
> Hi Yao,
> 
> I have not done runtime validation testing.
> Instead, I tested the assembly code snippets in a vacuum (in their .asm, .nasm, and .S forms),
> to ensure the source produced the expected compiled code, and the expected execution.
> 
> So any runtime 'Tested-by' would be greatly appreciated
> Thanks,
> Leo.
> 
>> 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
>>> 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
>>> 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: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Laszlo Ersek 6 years, 6 months ago
On 10/15/17 02:58, Yao, Jiewen wrote:
> for runtime test, I recommend using ovmf. You don't need real hardware. It can run both 32bit or 64bit. It can run in both Linux and windows.
> 
> You need use -D SMM_REQUIRE option to build ovmf. 
> If you have any problem, Laszlo is the good contact.

I don't have much context about this series, but looking at the blurb, I
see that version 3 removed OvmfPkg patches:

> 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.

Judged on the diffstat of patch #1 -- only
"UefiCpuPkg/Library/SmmCpuFeaturesLib" files are modified -- I would say
that changes in patch #1 are invisible to OVMF. The reason is that OVMF
uses a separate SmmCpuFeaturesLib instance, namely

  OvmfPkg/Library/SmmCpuFeaturesLib

This means two things:

- changes from patch #1 cannot be tested with OVMF, simply because
  "UefiCpuPkg/Library/SmmCpuFeaturesLib" is never built for OVMF;

- changes from patch #2 may or may not break SMM in OVMF, dependent on
  whether patch #2 is tied closely to patch #1.

In order to see why OvmfPkg has a separate SmmCpuFeaturesLib instance,
please review the commit log:

  git log --reverse -- OvmfPkg/Library/SmmCpuFeaturesLib

At this point I cannot determine if this patch set should ignore OvmfPkg
completely, or else patch #1 should be duplicated for
"OvmfPkg/Library/SmmCpuFeaturesLib" as well. (I guess I don't understand
the goal of the patch set -- I've read the blurb, but the problem has
not been stated well enough for me to understand. Or maybe it was stated
long ago, and I've forgotten it :) )


Leo, I have a separate request: the diffstats in the blurb and on patch
#1 are practically unreadable. This is because edk2 uses long names for
files and directories, and because the nesting can get deep -- and "git"
by default truncates the diffstat to quite narrow lines.

In order to avoid this, I recommend formatting edk2 patch sets as follows:

  git format-patch --stat=1000 --stat-graph-width=20 ...

This makes the pathname column just as wide as necessary, while keeping
the actual "stats" column reasonably narrow.

Thanks,
Laszlo

>> 在 2017年10月15日,上午12:04,Duran, Leo <leo.duran@amd.com> 写道:
>>
>>
>>
>>> -----Original Message-----
>>> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
>>> Sent: Thursday, October 12, 2017 8:53 PM
>>> To: Duran, Leo <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.
>> Hi Yao,
>>
>> Sure thing, I will do that.
>>
>> Leo
>>
>>> +//
>>> +// 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.
>>>
>>
>> Hi Yao,
>>
>> I have not done runtime validation testing.
>> Instead, I tested the assembly code snippets in a vacuum (in their .asm, .nasm, and .S forms),
>> to ensure the source produced the expected compiled code, and the expected execution.
>>
>> So any runtime 'Tested-by' would be greatly appreciated
>> Thanks,
>> Leo.
>>
>>> 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
>>>> 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
>>>> 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
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Paolo Bonzini 6 years, 6 months ago
On 16/10/2017 19:06, Laszlo Ersek wrote:
>   git log --reverse -- OvmfPkg/Library/SmmCpuFeaturesLib
> 
> At this point I cannot determine if this patch set should ignore OvmfPkg
> completely, or else patch #1 should be duplicated for
> "OvmfPkg/Library/SmmCpuFeaturesLib" as well. (I guess I don't understand
> the goal of the patch set -- I've read the blurb, but the problem has
> not been stated well enough for me to understand. Or maybe it was stated
> long ago, and I've forgotten it :) )

I _think_ they should be duplicated, but I also don't understand the
goal and that is just from reasoning on why OVMF has a separate
SmmCpuFeaturesLib implementation.

Paolo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Duran, Leo 6 years, 6 months ago

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, October 16, 2017 12:06 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Duran, Leo
> <leo.duran@amd.com>
> Cc: edk2-devel@lists.01.org; Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based
> x86 systems.
> 
> On 10/15/17 02:58, Yao, Jiewen wrote:
> > for runtime test, I recommend using ovmf. You don't need real hardware.
> It can run both 32bit or 64bit. It can run in both Linux and windows.
> >
> > You need use -D SMM_REQUIRE option to build ovmf.
> > If you have any problem, Laszlo is the good contact.
> 
> I don't have much context about this series, but looking at the blurb, I see
> that version 3 removed OvmfPkg patches:
> 
> > 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.
> 
> Judged on the diffstat of patch #1 -- only
> "UefiCpuPkg/Library/SmmCpuFeaturesLib" files are modified -- I would say
> that changes in patch #1 are invisible to OVMF. The reason is that OVMF uses
> a separate SmmCpuFeaturesLib instance, namely
> 
>   OvmfPkg/Library/SmmCpuFeaturesLib
> 
> This means two things:
> 
> - changes from patch #1 cannot be tested with OVMF, simply because
>   "UefiCpuPkg/Library/SmmCpuFeaturesLib" is never built for OVMF;
> 
> - changes from patch #2 may or may not break SMM in OVMF, dependent on
>   whether patch #2 is tied closely to patch #1.
> 
> In order to see why OvmfPkg has a separate SmmCpuFeaturesLib instance,
> please review the commit log:
> 
>   git log --reverse -- OvmfPkg/Library/SmmCpuFeaturesLib
> 
> At this point I cannot determine if this patch set should ignore OvmfPkg
> completely, or else patch #1 should be duplicated for
> "OvmfPkg/Library/SmmCpuFeaturesLib" as well. (I guess I don't understand
> the goal of the patch set -- I've read the blurb, but the problem has not been
> stated well enough for me to understand. Or maybe it was stated long ago,
> and I've forgotten it :) )
> 

Lazlo,
I purposely left out changes to OVMF and Quark, consistent with previous feedback.
Leo

> 
> Leo, I have a separate request: the diffstats in the blurb and on patch
> #1 are practically unreadable. This is because edk2 uses long names for files
> and directories, and because the nesting can get deep -- and "git"
> by default truncates the diffstat to quite narrow lines.
> 
> In order to avoid this, I recommend formatting edk2 patch sets as follows:
> 
>   git format-patch --stat=1000 --stat-graph-width=20 ...
> 
Lazlo,
Sounds good. Will do.
Leo.

> This makes the pathname column just as wide as necessary, while keeping
> the actual "stats" column reasonably narrow.
> 
> Thanks,
> Laszlo
> 
> >> 在 2017年10月15日,上午12:04,Duran, Leo <leo.duran@amd.com>
> 写道:
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> >>> Sent: Thursday, October 12, 2017 8:53 PM
> >>> To: Duran, Leo <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.
> >> Hi Yao,
> >>
> >> Sure thing, I will do that.
> >>
> >> Leo
> >>
> >>> +//
> >>> +// 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.
> >>>
> >>
> >> Hi Yao,
> >>
> >> I have not done runtime validation testing.
> >> Instead, I tested the assembly code snippets in a vacuum (in their
> >> .asm, .nasm, and .S forms), to ensure the source produced the expected
> compiled code, and the expected execution.
> >>
> >> So any runtime 'Tested-by' would be greatly appreciated Thanks, Leo.
> >>
> >>> 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
> >>>> 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
> >>>> 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
> >

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Laszlo Ersek 6 years, 6 months ago
On 10/16/17 19:31, Duran, Leo wrote:
> 
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Monday, October 16, 2017 12:06 PM
>> To: Yao, Jiewen <jiewen.yao@intel.com>; Duran, Leo
>> <leo.duran@amd.com>
>> Cc: edk2-devel@lists.01.org; Paolo Bonzini <pbonzini@redhat.com>
>> Subject: Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based
>> x86 systems.
>>
>> On 10/15/17 02:58, Yao, Jiewen wrote:
>>> for runtime test, I recommend using ovmf. You don't need real hardware.
>> It can run both 32bit or 64bit. It can run in both Linux and windows.
>>>
>>> You need use -D SMM_REQUIRE option to build ovmf.
>>> If you have any problem, Laszlo is the good contact.
>>
>> I don't have much context about this series, but looking at the blurb, I see
>> that version 3 removed OvmfPkg patches:
>>
>>> 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.
>>
>> Judged on the diffstat of patch #1 -- only
>> "UefiCpuPkg/Library/SmmCpuFeaturesLib" files are modified -- I would say
>> that changes in patch #1 are invisible to OVMF. The reason is that OVMF uses
>> a separate SmmCpuFeaturesLib instance, namely
>>
>>   OvmfPkg/Library/SmmCpuFeaturesLib
>>
>> This means two things:
>>
>> - changes from patch #1 cannot be tested with OVMF, simply because
>>   "UefiCpuPkg/Library/SmmCpuFeaturesLib" is never built for OVMF;
>>
>> - changes from patch #2 may or may not break SMM in OVMF, dependent on
>>   whether patch #2 is tied closely to patch #1.
>>
>> In order to see why OvmfPkg has a separate SmmCpuFeaturesLib instance,
>> please review the commit log:
>>
>>   git log --reverse -- OvmfPkg/Library/SmmCpuFeaturesLib
>>
>> At this point I cannot determine if this patch set should ignore OvmfPkg
>> completely, or else patch #1 should be duplicated for
>> "OvmfPkg/Library/SmmCpuFeaturesLib" as well. (I guess I don't understand
>> the goal of the patch set -- I've read the blurb, but the problem has not been
>> stated well enough for me to understand. Or maybe it was stated long ago,
>> and I've forgotten it :) )
>>
> 
> Lazlo,
> I purposely left out changes to OVMF and Quark, consistent with previous feedback.

I've found my previous comments:

http://mid.mail-archive.com/2d3efa5a-ad72-bb35-1e6a-b9b78379337c@redhat.com

There I only suggested a different (more telling) subject for the
OvmfPkg patch, and wrote,

> (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.)

I didn't try to validate Jiewen's / Mike's feedback; I just stated *if*,
according to them, patching OvmfPkg was not necessary, I'd be OK with that.

Since we're talking anyway, can you (and/or Jiewen & Mike) please state
the problem being solved here, and explain why patching the
SmmCpuFeaturesLib instance in OvmfPkg is, or is not, necessary to update?

Hmmm... Is it the case that

  UefiCpuPkg/Library/SmmCpuFeaturesLib

runs correctly on Intel *hosts* only at the moment (so it needs fixing,
for AMD *hosts*), while

  OvmfPkg/Library/SmmCpuFeaturesLib

deals with AMD-looking *guests* anyway, so it needs no fixing, for AMD
compatibility?

If this is correct, then I agree patch #1 does not need to be duplicated
for OvmfPkg.

*However*, in turn, patch #2 (for PiSmmCpuDxeSmm) might be necessary to
update for QEMU. PiSmmCpuDxeSmm runs on both bare metal and on QEMU.
And, as Paolo says, a pure CPUID / manufacturer check (for determining
the state save layout) is wrong on QEMU, even if the same would work on
bare metal:

@@ -547,6 +595,20 @@ PiCpuSmmEntry (
     );

   //
+  // Override SMRAM offsets for AMD
+  //
+  if (StandardSignatureIsAuthenticAMD ()) {
+    gSmmSmramStateMapOffset = AMD_SMRAM_SAVE_STATE_MAP_OFFSET;
+    gSmmPsdOffset = AMD_SMM_PSD_OFFSET;
+  }
+

If patch v5 2/2 is merely a refactoring (i.e., it causes PiSmmCpuDxeSmm
to behave exactly the same as before, just with an improved
implementation), then I agree a CPUID-based check is not necessarily a
bug (regression). Instead, it might be called a missed opportunity (or,
more nicely put, a "basis") for bringing PiSmmCpuDxeSmm closer to QEMU.

My apologies if I'm confused.

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Duran, Leo 6 years, 6 months ago

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, October 16, 2017 1:44 PM
> To: Duran, Leo <leo.duran@amd.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Cc: edk2-devel@lists.01.org; Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based
> x86 systems.
> 
> On 10/16/17 19:31, Duran, Leo wrote:
> >
> >
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Monday, October 16, 2017 12:06 PM
> >> To: Yao, Jiewen <jiewen.yao@intel.com>; Duran, Leo
> >> <leo.duran@amd.com>
> >> Cc: edk2-devel@lists.01.org; Paolo Bonzini <pbonzini@redhat.com>
> >> Subject: Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-
> based
> >> x86 systems.
> >>
> >> On 10/15/17 02:58, Yao, Jiewen wrote:
> >>> for runtime test, I recommend using ovmf. You don't need real
> hardware.
> >> It can run both 32bit or 64bit. It can run in both Linux and windows.
> >>>
> >>> You need use -D SMM_REQUIRE option to build ovmf.
> >>> If you have any problem, Laszlo is the good contact.
> >>
> >> I don't have much context about this series, but looking at the
> >> blurb, I see that version 3 removed OvmfPkg patches:
> >>
> >>> 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.
> >>
> >> Judged on the diffstat of patch #1 -- only
> >> "UefiCpuPkg/Library/SmmCpuFeaturesLib" files are modified -- I would
> >> say that changes in patch #1 are invisible to OVMF. The reason is
> >> that OVMF uses a separate SmmCpuFeaturesLib instance, namely
> >>
> >>   OvmfPkg/Library/SmmCpuFeaturesLib
> >>
> >> This means two things:
> >>
> >> - changes from patch #1 cannot be tested with OVMF, simply because
> >>   "UefiCpuPkg/Library/SmmCpuFeaturesLib" is never built for OVMF;
> >>
> >> - changes from patch #2 may or may not break SMM in OVMF, dependent
> on
> >>   whether patch #2 is tied closely to patch #1.
> >>
> >> In order to see why OvmfPkg has a separate SmmCpuFeaturesLib
> >> instance, please review the commit log:
> >>
> >>   git log --reverse -- OvmfPkg/Library/SmmCpuFeaturesLib
> >>
> >> At this point I cannot determine if this patch set should ignore
> >> OvmfPkg completely, or else patch #1 should be duplicated for
> >> "OvmfPkg/Library/SmmCpuFeaturesLib" as well. (I guess I don't
> >> understand the goal of the patch set -- I've read the blurb, but the
> >> problem has not been stated well enough for me to understand. Or
> >> maybe it was stated long ago, and I've forgotten it :) )
> >>
> >
> > Lazlo,
> > I purposely left out changes to OVMF and Quark, consistent with previous
> feedback.
> 
> I've found my previous comments:
> 
> http://mid.mail-archive.com/2d3efa5a-ad72-bb35-1e6a-
> b9b78379337c@redhat.com
> 
> There I only suggested a different (more telling) subject for the OvmfPkg
> patch, and wrote,
> 
> > (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.)
> 
> I didn't try to validate Jiewen's / Mike's feedback; I just stated *if*, according
> to them, patching OvmfPkg was not necessary, I'd be OK with that.
> 
> Since we're talking anyway, can you (and/or Jiewen & Mike) please state the
> problem being solved here, and explain why patching the
> SmmCpuFeaturesLib instance in OvmfPkg is, or is not, necessary to update?
> 
> Hmmm... Is it the case that
> 
>   UefiCpuPkg/Library/SmmCpuFeaturesLib
> 
> runs correctly on Intel *hosts* only at the moment (so it needs fixing, for
> AMD *hosts*), while
> 
>   OvmfPkg/Library/SmmCpuFeaturesLib
> 
> deals with AMD-looking *guests* anyway, so it needs no fixing, for AMD
> compatibility?
> 
> If this is correct, then I agree patch #1 does not need to be duplicated for
> OvmfPkg.

Lazlo,
Yes, that is my understanding.

> 
> *However*, in turn, patch #2 (for PiSmmCpuDxeSmm) might be necessary to
> update for QEMU. PiSmmCpuDxeSmm runs on both bare metal and on
> QEMU.
> And, as Paolo says, a pure CPUID / manufacturer check (for determining the
> state save layout) is wrong on QEMU, even if the same would work on bare
> metal:
> 
> @@ -547,6 +595,20 @@ PiCpuSmmEntry (
>      );
> 
>    //
> +  // Override SMRAM offsets for AMD
> +  //
> +  if (StandardSignatureIsAuthenticAMD ()) {
> +    gSmmSmramStateMapOffset =
> AMD_SMRAM_SAVE_STATE_MAP_OFFSET;
> +    gSmmPsdOffset = AMD_SMM_PSD_OFFSET;  }
> +
> 
> If patch v5 2/2 is merely a refactoring (i.e., it causes PiSmmCpuDxeSmm to
> behave exactly the same as before, just with an improved implementation),
> then I agree a CPUID-based check is not necessarily a bug (regression).
> Instead, it might be called a missed opportunity (or, more nicely put, a
> "basis") for bringing PiSmmCpuDxeSmm closer to QEMU.
Lazlo,

1) Per feedback from Yao:
I'm currently investigating if we *really*really* have to change the PSD offset.

2) Per feedback from Paolo:
And if the PSD offset change is indeed required, I will investigate an alternate detection mechanism.

Thanks.
Leo

> 
> My apologies if I'm confused.
> 
> Thanks
> Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Duran, Leo 6 years, 6 months ago
> -----Original Message-----
> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> Sent: Thursday, October 12, 2017 9:37 PM
> To: Duran, Leo <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
> 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)?
>
HI Yao,
I will look further into this to see if it's possible to re-use the same offset without too much pain.
Thanks,
Leo.

> Thank you
> Yao Jiewen

> -----Original Message-----
> From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
> Sent: Thursday, October 12, 2017 8:53 PM
> To: Duran, Leo <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
> > 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
> > 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: [edk2] [PATCH v5 0/2] Enhanced SMM support for AMD-based x86 systems.
Posted by Paolo Bonzini 6 years, 6 months ago
On 13/10/2017 03:52, Yao, Jiewen wrote:
> 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

If I understand correctly, I think AMD can keep on using 0xfc00 and just 
adjust the offsets up by 0x200 bytes.  For example, QEMU_SMRAM_SAVE_STATE_MAP
in OvmfPkg/Include/Register/QemuSmramStateSaveMap.h is essentially the AMD
format and it starts with:

  UINT8   Reserved0[0x200]; // 7c00h
  UINT8   Reserved1[0xf8];  // 7e00h

See also the comment in SMRAM_SAVE_STATE_MAP32:

  UINT8   Reserved[0x200];  // 7c00h
                            // Padded an extra 0x200 bytes so 32-bit and 64-bit
                            // SMRAM Save State Maps are the same size
  UINT8   Reserved1[0xf8];  // 7e00h

Thanks,

Paolo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel