On 5/13/22 08:22, Michael Roth wrote:
A full-featured SEV-SNP guest will not rely on the AP jump table, and
will instead use the AP Creation interface defined by the GHCB. However,
a guest is still allowed to use the AP jump table if desired.

However, unlike with SEV-ES guests, SEV-SNP guests should not
store/retrieve the jump table address via GHCB requests to the
hypervisor, they should instead store/retrieve it via the SEV-SNP
secrets page. Implement the store side of this for OVMF.

Suggested-by: Tom Lendacky <thomas.lenda...@amd.com>
Signed-off-by: Michael Roth <michael.r...@amd.com>
---
  MdePkg/Include/AmdSevSnpSecretsPage.h         | 51 +++++++++++++++++++
  MdePkg/MdePkg.dec                             |  4 ++
  OvmfPkg/AmdSev/AmdSevX64.dsc                  |  3 ++
  OvmfPkg/CloudHv/CloudHvX64.dsc                |  3 ++
  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |  3 ++
  OvmfPkg/Microvm/MicrovmX64.dsc                |  3 ++
  OvmfPkg/OvmfPkgIa32.dsc                       |  3 ++
  OvmfPkg/OvmfPkgIa32X64.dsc                    |  3 ++
  OvmfPkg/OvmfPkgX64.dsc                        |  3 ++
  OvmfPkg/PlatformPei/AmdSev.c                  |  5 ++
  OvmfPkg/PlatformPei/PlatformPei.inf           |  1 +
  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  9 ++++
  13 files changed, 92 insertions(+)
  create mode 100644 MdePkg/Include/AmdSevSnpSecretsPage.h

diff --git a/MdePkg/Include/AmdSevSnpSecretsPage.h 
b/MdePkg/Include/AmdSevSnpSecretsPage.h
new file mode 100644
index 0000000000..55c7475ff0
--- /dev/null
+++ b/MdePkg/Include/AmdSevSnpSecretsPage.h

Just wondering if this should be in the MdePkg/Include/Register/Amd directory?

@@ -0,0 +1,51 @@
+/** @file
+Definitions for AMD SEV-SNP Secrets Page
+
+Copyright (c) 2022 AMD Inc. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef AMD_SEV_SNP_SECRETS_PAGE_H_
+#define AMD_SEV_SNP_SECRETS_PAGE_H_
+
+//
+// OS-defined area of secrets page
+//
+// As defined by "SEV-ES Guest-Hypervisor Communication Block Standardization",
+// revision 1.50, section 2.7, "SEV-SNP Secrets Page".

This should be using at least revision 2.00 (if not 2.01 which is in the process of being published). 2.01 uses some of the 40-byte reserved area to hold the high 32-bits of the message sequence numbers (since the SNP API changed after the GHCB spec was published to convert the sequence numbers from 32-bit to 64-bit). The changes are backwards compatible, so not a big deal as to whether to implement since OVMF doesn't make any guest request API calls.

Thanks,
Tom

+//
+typedef PACKED struct _SNP_SECRETS_OS_AREA {
+  UINT32    MsgSeqNum0;
+  UINT32    MsgSeqNum1;
+  UINT32    MsgSeqNum2;
+  UINT32    MsgSeqNum3;
+  UINT64    ApJumpTablePa;
+  UINT8     Reserved[40];
+  UINT8     GuestUsage[32];
+} SNP_SECRETS_OS_AREA;
+
+#define VMPCK_KEY_LEN  32
+
+//
+// SEV-SNP Secrets page
+//
+// As defined by "SEV-SNP Firmware ABI", revision 1.51, section 8.17.2.5,
+// "PAGE_TYPE_SECRETS".
+//
+typedef PACKED struct _SNP_SECRETS_PAGE {
+  UINT32                 Version;
+  UINT32                 ImiEn    : 1,
+                         Reserved : 31;
+  UINT32                 Fms;
+  UINT32                 Reserved2;
+  UINT8                  Gosvw[16];
+  UINT8                  Vmpck0[VMPCK_KEY_LEN];
+  UINT8                  Vmpck1[VMPCK_KEY_LEN];
+  UINT8                  Vmpck2[VMPCK_KEY_LEN];
+  UINT8                  Vmpck3[VMPCK_KEY_LEN];
+  SNP_SECRETS_OS_AREA    OsArea;
+  UINT8                  Reserved3[3840];
+} SNP_SECRETS_PAGE;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index f1ebf9e251..a365bfcfe8 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2417,5 +2417,9 @@
    # @Prompt Memory encryption attribute
    
gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0|UINT64|0x0000002e
+ ## This dynamic PCD indicates the location of the SEV-SNP secrets page.
+  # @Prompt SEV-SNP secrets page address
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress|0|UINT64|0x0000002f
+
  [UserExtensions.TianoCore."ExtraFiles"]
    MdePkgExtra.uni
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index f0700035c1..02306945fd 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -575,6 +575,9 @@
    # Set ConfidentialComputing defaults
    gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+ # Set SEV-SNP Secrets page address default
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress|0
+
  !include OvmfPkg/OvmfTpmPcds.dsc.inc
gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index d1c85f60c7..7143698253 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -630,6 +630,9 @@
    # Set ConfidentialComputing defaults
    gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+ # Set SEV-SNP Secrets page address default
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress|0
+
  [PcdsDynamicHii]
  !include OvmfPkg/OvmfTpmPcdsHii.dsc.inc
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 80c331ea23..b19718c572 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -512,6 +512,9 @@
gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000 + # Set SEV-SNP Secrets page address default
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress|0
+
  
################################################################################
  #
  # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 20c3c9c4d8..42673c29ee 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -613,6 +613,9 @@
    # Set ConfidentialComputing defaults
    gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+ # Set SEV-SNP Secrets page address default
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress|0
+
  
################################################################################
  #
  # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 533bbdb435..8ffef069a3 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -649,6 +649,9 @@
    # Set ConfidentialComputing defaults
    gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+ # Set SEV-SNP Secrets page address default
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress|0
+
  !if $(CSM_ENABLE) == FALSE
    gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
  !endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index cb68e612bd..0b4d5001b2 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -657,6 +657,9 @@
    # Set ConfidentialComputing defaults
    gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+ # Set SEV-SNP Secrets page address default
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress|0
+
  !if $(CSM_ENABLE) == FALSE
    gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
  !endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 71526bba31..3a3223be6b 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -680,6 +680,9 @@
    # Set ConfidentialComputing defaults
    gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr|0
+ # Set SEV-SNP Secrets page address default
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress|0
+
  !if $(CSM_ENABLE) == FALSE
    gEfiMdePkgTokenSpaceGuid.PcdFSBClock|100000000
  !endif
diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 385562b44c..70352ca43b 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -408,6 +408,11 @@ AmdSevInitialize (
    //
    if (MemEncryptSevSnpIsEnabled ()) {
      PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, 
CCAttrAmdSevSnp);
+    ASSERT_RETURN_ERROR (PcdStatus);
+    PcdStatus = PcdSet64S (
+                  PcdSevSnpSecretsAddress,
+                  (UINT64)(UINTN)PcdGet32 (PcdOvmfSnpSecretsBase)
+                  );
    } else if (MemEncryptSevEsIsEnabled ()) {
      PcdStatus = PcdSet64S (PcdConfidentialComputingGuestAttr, CCAttrAmdSevEs);
    } else {
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 00372fa0eb..c688e4ee24 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -114,6 +114,7 @@
    gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr
    gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures
    gEfiMdeModulePkgTokenSpaceGuid.PcdTdxSharedBitMask
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress
[FixedPcd]
    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index e1cd0b3500..d8cfddcd82 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -80,3 +80,4 @@
    gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                      ## 
CONSUMES
    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                           ## 
CONSUMES
    gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr           ## 
CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress                     ## 
CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 60d14a5a0e..6014dce136 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -15,6 +15,7 @@
  #include <Library/VmgExitLib.h>
  #include <Register/Amd/Fam17Msr.h>
  #include <Register/Amd/Ghcb.h>
+#include <AmdSevSnpSecretsPage.h>
#include <Protocol/Timer.h> @@ -216,6 +217,14 @@ GetSevEsAPMemory ( DEBUG ((DEBUG_INFO, "Dxe: SevEsAPMemory = %lx\n", (UINTN)StartAddress)); + if (ConfidentialComputingGuestHas (CCAttrAmdSevSnp)) {
+    SNP_SECRETS_PAGE  *Secrets = (SNP_SECRETS_PAGE *)(INTN)PcdGet64 
(PcdSevSnpSecretsAddress);
+
+    Secrets->OsArea.ApJumpTablePa = (UINT64)(UINTN)StartAddress;
+
+    return (UINTN)StartAddress;
+  }
+
    //
    // Save the SevEsAPMemory as the AP jump table.
    //


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89723): https://edk2.groups.io/g/devel/message/89723
Mute This Topic: https://groups.io/mt/91081159/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to