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

Michael Roth via groups.io michael.roth=amd.com at groups.io
Thu May 19 18:19:52 UTC 2022


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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Michael Roth via groups.io
> > Sent: Monday, May 16, 2022 8:02 PM
> > To: devel at edk2.groups.io
> > Cc: Tom Lendacky <thomas.lendacky at 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.lendacky at amd.com>
> > Signed-off-by: Michael Roth <michael.roth at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-




More information about the edk2-devel-archive mailing list