From: "Lendacky, Thomas" <thomas.lendacky@amd.com>
To: Michael Roth <michael.roth@amd.com>, devel@edk2.groups.io
Subject: Re: [PATCH] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page
Date: Fri, 13 May 2022 08:56:06 -0500 [thread overview]
Message-ID: <62b8bc5f-3d0a-9ad6-1049-ea260cf01b19@amd.com> (raw)
In-Reply-To: <20220513132253.397679-1-michael.roth@amd.com>
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.lendacky@amd.com>
> Signed-off-by: Michael Roth <michael.roth@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?
> @@ -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.
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.
> //
next prev parent reply other threads:[~2022-05-13 13:56 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-13 13:22 [PATCH] UefiCpuPkg: Store SEV-SNP AP jump table in the secrets page Michael Roth
2022-05-13 13:56 ` Lendacky, Thomas [this message]
2022-05-13 14:20 ` Michael Roth
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=62b8bc5f-3d0a-9ad6-1049-ea260cf01b19@amd.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox