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.lenda...@amd.com> > > 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.lenda...@amd.com> > > Signed-off-by: Michael Roth <michael.r...@amd.com> > > --- > > 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 > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#89905): https://edk2.groups.io/g/devel/message/89905 Mute This Topic: https://groups.io/mt/91139607/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-