Re: [PATCH v2] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page


Michael Roth <michael.roth@...>
 

On Tue, May 17, 2022 at 02:17:28PM +0000, Ni, Ray wrote:
can you please split the patches so one patch for one package?
Hi Ray,

Sorry I missed your reply somehow. I'll send a v3 that splits the
series in 4 patches:

1/4 MdePkg: introduce SnpSecretPageDef.h
2/4 MdePkg: introduce gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress PCD
3/4 OvmfPkg: initialize PcdSevSnpSecretsAddress PCD
4/4 UefiCpuPkg: use PcdSevSnpSecretsAddress to access secrets page and
set AP jump table address

but if you were thinking something else just let me know.

Thanks!

-Mike


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Roth via groups.io
Sent: Monday, May 16, 2022 8:02 PM
To: devel@edk2.groups.io
Cc: Tom Lendacky <thomas.lendacky@...>
Subject: [edk2-devel] [PATCH v2] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page

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@...>
Signed-off-by: Michael Roth <michael.roth@...>
---
v2:
- Update Secrets OS area to match latest GHCB 2.01 spec
- Move Secrets header file into ./Register/AMD subdirectory
- 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(+)
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

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




Join devel@edk2.groups.io to automatically receive all group messages.