On Fri, May 13, 2022 at 08:56:06AM -0500, Tom Lendacky wrote:
> 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?
That might be the better spot. I was thinking that was reserved for
definitions related to specific registers, but I somehow missed that it's
also where the GHCB struct is defined. So probably makes sense to put
the secrets page definition there as well.
>
> > @@ -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.
Okay, will go ahead and update these while I'm at it and send a v2.
Thanks!
-Mike
>
> 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 (#89728): https://edk2.groups.io/g/devel/message/89728
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]
-=-=-=-=-=-=-=-=-=-=-=-
