public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP
@ 2022-05-20 15:27 Michael Roth
  2022-05-20 15:27 ` [PATCH v3 1/4] MdePkg: Add header for SEV-SNP secrets page struct Michael Roth
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Michael Roth @ 2022-05-20 15:27 UTC (permalink / raw)
  To: devel; +Cc: Tom Lendacky, Ni, Ray

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.

This series implements the store side of this for OVMF by introducing a
PCD that can be used to pass the SEV-SNP secrets page address to
UefiCpuPkg, where the jump table address is allocated. It also
introduces a struct that defines the SEV-SNP secrets page format
according to the GHCB v2.01 and SEV-SNP FW ABI specifications.

v3:
 - Break up single patch into a set of patches containing the specific
   changes for each package. (Ray)

v2:
 - Update Secrets OS area to match latest GHCB 2.01 spec (Tom)
 - Move Secrets header file into ./Register/AMD subdirectory (Tom)
 - Fix CI EccCheck due to assignment in variable declaration

 MdePkg/Include/Register/Amd/SnpSecretsPage.h  | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 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       | 10 ++++++++++
 13 files changed, 98 insertions(+)



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/4] MdePkg: Add header for SEV-SNP secrets page struct
  2022-05-20 15:27 [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP Michael Roth
@ 2022-05-20 15:27 ` Michael Roth
  2022-05-20 15:27 ` [PATCH v3 2/4] MdePkg: Add PcdSevSnpSecretsAddress to export SEV-SNP secrets page Michael Roth
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Michael Roth @ 2022-05-20 15:27 UTC (permalink / raw)
  To: devel; +Cc: Tom Lendacky, Ni, Ray

This will be needed so that the AP Jump Table address can be stored for
use by the operating system later, and possibly for other things in the
future.

Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 MdePkg/Include/Register/Amd/SnpSecretsPage.h | 56 ++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 MdePkg/Include/Register/Amd/SnpSecretsPage.h

diff --git a/MdePkg/Include/Register/Amd/SnpSecretsPage.h b/MdePkg/Include/Register/Amd/SnpSecretsPage.h
new file mode 100644
index 0000000000..3188459150
--- /dev/null
+++ b/MdePkg/Include/Register/Amd/SnpSecretsPage.h
@@ -0,0 +1,56 @@
+/** @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 SNP_SECRETS_PAGE_H_
+#define SNP_SECRETS_PAGE_H_
+
+//
+// OS-defined area of secrets page
+//
+// As defined by "SEV-ES Guest-Hypervisor Communication Block Standardization",
+// revision 2.01, section 2.7, "SEV-SNP Secrets Page".
+//
+typedef PACKED struct _SNP_SECRETS_OS_AREA {
+  UINT32    Vmpl0MsgSeqNumLo;
+  UINT32    Vmpl1MsgSeqNumLo;
+  UINT32    Vmpl2MsgSeqNumLo;
+  UINT32    Vmpl3MsgSeqNumLo;
+  UINT64    ApJumpTablePa;
+  UINT32    Vmpl0MsgSeqNumHi;
+  UINT32    Vmpl1MsgSeqNumHi;
+  UINT32    Vmpl2MsgSeqNumHi;
+  UINT32    Vmpl3MsgSeqNumHi;
+  UINT8     Reserved2[22];
+  UINT16    Version;
+  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
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/4] MdePkg: Add PcdSevSnpSecretsAddress to export SEV-SNP secrets page
  2022-05-20 15:27 [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP Michael Roth
  2022-05-20 15:27 ` [PATCH v3 1/4] MdePkg: Add header for SEV-SNP secrets page struct Michael Roth
@ 2022-05-20 15:27 ` Michael Roth
  2022-05-20 15:27 ` [PATCH v3 3/4] OvmfPkg: Initialize the PcdSevSnpSecretsAddress PCD during PEI phase Michael Roth
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Michael Roth @ 2022-05-20 15:27 UTC (permalink / raw)
  To: devel; +Cc: Tom Lendacky, Ni, Ray

OvmfPkg will initially use this to hand off the secrets page address to
UefiCpuPkg, which will need this PCD to access the SEV-SNP secrets page
address. Define this as an MdePkg PCD so it can be accessed by other
packages alongside the secrets page struct defined in MdePkg/Include.

Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 MdePkg/MdePkg.dec | 4 ++++
 1 file changed, 4 insertions(+)

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 3/4] OvmfPkg: Initialize the PcdSevSnpSecretsAddress PCD during PEI phase
  2022-05-20 15:27 [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP Michael Roth
  2022-05-20 15:27 ` [PATCH v3 1/4] MdePkg: Add header for SEV-SNP secrets page struct Michael Roth
  2022-05-20 15:27 ` [PATCH v3 2/4] MdePkg: Add PcdSevSnpSecretsAddress to export SEV-SNP secrets page Michael Roth
@ 2022-05-20 15:27 ` Michael Roth
  2022-05-20 15:27 ` [PATCH v3 4/4] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page Michael Roth
  2022-05-23 13:58 ` [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP Lendacky, Thomas
  4 siblings, 0 replies; 7+ messages in thread
From: Michael Roth @ 2022-05-20 15:27 UTC (permalink / raw)
  To: devel; +Cc: Tom Lendacky, Ni, Ray

This needs to be set so that UefiCpuPkg can locate the SEV-SNP secrets
page later to set the AP Jump Table address.

Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 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 +
 9 files changed, 27 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index bead9722ea..c0a3548f22 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -576,6 +576,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 92664f319b..ba4c14dd02 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -631,6 +631,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 00bc1255bc..c069bd9d1e 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -513,6 +513,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 f8fc977cb2..774e5e2ca9 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -614,6 +614,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 c16a840fff..a531fcd070 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -650,6 +650,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 d3a80cb568..cd579246f8 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -658,6 +658,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 7b3d48aac4..a026706279 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -683,6 +683,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
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 4/4] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page
  2022-05-20 15:27 [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP Michael Roth
                   ` (2 preceding siblings ...)
  2022-05-20 15:27 ` [PATCH v3 3/4] OvmfPkg: Initialize the PcdSevSnpSecretsAddress PCD during PEI phase Michael Roth
@ 2022-05-20 15:27 ` Michael Roth
  2022-05-23 13:57   ` Lendacky, Thomas
  2022-05-23 13:58 ` [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP Lendacky, Thomas
  4 siblings, 1 reply; 7+ messages in thread
From: Michael Roth @ 2022-05-20 15:27 UTC (permalink / raw)
  To: devel; +Cc: Tom Lendacky, Ni, Ray

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.lendacky@amd.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       | 10 ++++++++++
 2 files changed, 11 insertions(+)

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..4d6f7643db 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 <Register/Amd/SnpSecretsPage.h>
 
 #include <Protocol/Timer.h>
 
@@ -216,6 +217,15 @@ GetSevEsAPMemory (
 
   DEBUG ((DEBUG_INFO, "Dxe: SevEsAPMemory = %lx\n", (UINTN)StartAddress));
 
+  if (ConfidentialComputingGuestHas (CCAttrAmdSevSnp)) {
+    SNP_SECRETS_PAGE  *Secrets;
+
+    Secrets                       = (SNP_SECRETS_PAGE *)(INTN)PcdGet64 (PcdSevSnpSecretsAddress);
+    Secrets->OsArea.ApJumpTablePa = (UINT64)(UINTN)StartAddress;
+
+    return (UINTN)StartAddress;
+  }
+
   //
   // Save the SevEsAPMemory as the AP jump table.
   //
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 4/4] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page
  2022-05-20 15:27 ` [PATCH v3 4/4] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page Michael Roth
@ 2022-05-23 13:57   ` Lendacky, Thomas
  0 siblings, 0 replies; 7+ messages in thread
From: Lendacky, Thomas @ 2022-05-23 13:57 UTC (permalink / raw)
  To: Michael Roth, devel; +Cc: Ni, Ray

On 5/20/22 10:27, 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.lendacky@amd.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>   UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
>   UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       | 10 ++++++++++
>   2 files changed, 11 insertions(+)
> 
> 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..4d6f7643db 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 <Register/Amd/SnpSecretsPage.h>
>   
>   #include <Protocol/Timer.h>
>   
> @@ -216,6 +217,15 @@ GetSevEsAPMemory (
>   
>     DEBUG ((DEBUG_INFO, "Dxe: SevEsAPMemory = %lx\n", (UINTN)StartAddress));
>   
> +  if (ConfidentialComputingGuestHas (CCAttrAmdSevSnp)) {
> +    SNP_SECRETS_PAGE  *Secrets;
> +
> +    Secrets                       = (SNP_SECRETS_PAGE *)(INTN)PcdGet64 (PcdSevSnpSecretsAddress);
> +    Secrets->OsArea.ApJumpTablePa = (UINT64)(UINTN)StartAddress;
> +
> +    return (UINTN)StartAddress;
> +  }
> +

Just a nit, but I probably would have still put this under the comment 
below, because you are still saving the SevEsAPMemory as the AP jump table.

It might look cleaner with the non-SNP path as the else and have the 
single return.

Probably not worth another version, but up to you.

Thanks,
Tom

>     //
>     // Save the SevEsAPMemory as the AP jump table.
>     //

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP
  2022-05-20 15:27 [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP Michael Roth
                   ` (3 preceding siblings ...)
  2022-05-20 15:27 ` [PATCH v3 4/4] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page Michael Roth
@ 2022-05-23 13:58 ` Lendacky, Thomas
  4 siblings, 0 replies; 7+ messages in thread
From: Lendacky, Thomas @ 2022-05-23 13:58 UTC (permalink / raw)
  To: Michael Roth, devel; +Cc: Ni, Ray

On 5/20/22 10:27, 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.
> 
> This series implements the store side of this for OVMF by introducing a
> PCD that can be used to pass the SEV-SNP secrets page address to
> UefiCpuPkg, where the jump table address is allocated. It also
> introduces a struct that defines the SEV-SNP secrets page format
> according to the GHCB v2.01 and SEV-SNP FW ABI specifications.
> 
> v3:
>   - Break up single patch into a set of patches containing the specific
>     changes for each package. (Ray)
> 
> v2:
>   - Update Secrets OS area to match latest GHCB 2.01 spec (Tom)
>   - Move Secrets header file into ./Register/AMD subdirectory (Tom)
>   - Fix CI EccCheck due to assignment in variable declaration

Aside from the minor comment on patch 4, for the series:

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> 
>   MdePkg/Include/Register/Amd/SnpSecretsPage.h  | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   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       | 10 ++++++++++
>   13 files changed, 98 insertions(+)
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-05-23 13:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-20 15:27 [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP Michael Roth
2022-05-20 15:27 ` [PATCH v3 1/4] MdePkg: Add header for SEV-SNP secrets page struct Michael Roth
2022-05-20 15:27 ` [PATCH v3 2/4] MdePkg: Add PcdSevSnpSecretsAddress to export SEV-SNP secrets page Michael Roth
2022-05-20 15:27 ` [PATCH v3 3/4] OvmfPkg: Initialize the PcdSevSnpSecretsAddress PCD during PEI phase Michael Roth
2022-05-20 15:27 ` [PATCH v3 4/4] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page Michael Roth
2022-05-23 13:57   ` Lendacky, Thomas
2022-05-23 13:58 ` [PATCH v3 0/4] Fix AP Jump Table Handling for SEV-SNP Lendacky, Thomas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox