[edk2-devel] [PATCH 1/4] OvmfPkg/AmdSevDxe: Allocate SEV-SNP CC blob as EfiACPIReclaimMemory
Yao, Jiewen
jiewen.yao at intel.com
Sat Jan 7 02:01:32 UTC 2023
Hi Dov/Ard
Please allow me to clarify:
EfiACPIReclaimMemory in UEFI spec means: OS may use the memory, after it copies the ACPI table to its own location. It is also called "AddressRangeACPI" in ACPI spec.
[2] https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces.html, search AddressRangeACPI.
[3] https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html, search EfiACPIReclaimMemory.
However, in the description, you mentioned "The SEV-SNP Confidential Computing blob contains metadata that should remain accessible for the life of the guest."
That requirement conflicts with the definition of ACPIReclaim memory.
I would like to suggest either of below, to meet the need "that should remain accessible for the life of the guest."
a) EfiACPIMemoryNVS in UEFI, also known as AddressRangeNVS in ACPI (or)
b) EfiReservedMemoryType in UEFI, also knowns as AddressRangeReserved in ACPI.
Please double confirm that.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Dov Murik <dovmurik at linux.ibm.com>
> Sent: Saturday, January 7, 2023 4:26 AM
> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io;
> Michael.Roth at amd.com; Ard Biesheuvel <ardb at kernel.org>
> Cc: Tom Lendacky <thomas.lendacky at amd.com>; Ni, Ray
> <ray.ni at intel.com>; Dov Murik <dovmurik at linux.ibm.com>
> Subject: Re: [edk2-devel] [PATCH 1/4] OvmfPkg/AmdSevDxe: Allocate SEV-
> SNP CC blob as EfiACPIReclaimMemory
>
> Hi Jiewen,
>
> On 06/01/2023 11:18, Yao, Jiewen wrote:
> > Are you sure you want to use EfiACPIReclaimMemory ?
> >
> > Usually EfiACPIReclaimMemory is only for ACPI table, which can be
> reclaimed and used by OS, after copy ACPI table.
> >
> > If you want to claim the memory owned by firmware (not owned by OS),
> you need use ACPINvs or reserved.
> >
>
> EfiACPIReclaimMemory type was suggested by Ard [1] for a similar fix
> another SEV-related memory area that should remain in-place throughout
> the guest OS lifetime (not reused by OS).
>
> Ard -- can you please explain that choice?
>
>
> [1] https://edk2.groups.io/g/devel/message/97154
>
>
>
> -Dov
>
>
> >
> > Although I don't fully understand SEV, this seems suspicious.
> >
> > Please double confirm if this is really you want.
> >
> > Thank you
> > Yao, Jiewen
> >
> >
> >> -----Original Message-----
> >> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Roth,
> >> Michael via groups.io
> >> Sent: Thursday, December 22, 2022 12:07 AM
> >> To: devel at edk2.groups.io
> >> Cc: Tom Lendacky <thomas.lendacky at amd.com>; Ni, Ray
> >> <ray.ni at intel.com>; Dov Murik <dovmurik at linux.ibm.com>
> >> Subject: [edk2-devel] [PATCH 1/4] OvmfPkg/AmdSevDxe: Allocate SEV-SNP
> CC
> >> blob as EfiACPIReclaimMemory
> >>
> >> The SEV-SNP Confidential Computing blob contains metadata that should
> >> remain accessible for the life of the guest. Allocate it as
> >> EfiACPIReclaimMemory to ensure the memory isn't overwritten by the
> guest
> >> operating system later.
> >>
> >> Reported-by: Dov Murik <dovmurik at linux.ibm.com>
> >> Suggested-by: Dov Murik <dovmurik at linux.ibm.com>
> >> Signed-off-by: Michael Roth <michael.roth at amd.com>
> >> ---
> >> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 62
> +++++++++++++++++++++++++++---
> >> -----
> >> 1 file changed, 48 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> >> b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> >> index 662d3c4ccb..8dfda961d7 100644
> >> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> >> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> >> @@ -21,15 +21,36 @@
> >> #include <Guid/ConfidentialComputingSevSnpBlob.h>
> >>
> >> #include <Library/PcdLib.h>
> >>
> >>
> >>
> >> -STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION
> >> mSnpBootDxeTable = {
> >>
> >> - SIGNATURE_32 ('A', 'M', 'D', 'E'),
> >>
> >> - 1,
> >>
> >> - 0,
> >>
> >> - (UINT64)(UINTN)FixedPcdGet32 (PcdOvmfSnpSecretsBase),
> >>
> >> - FixedPcdGet32 (PcdOvmfSnpSecretsSize),
> >>
> >> - (UINT64)(UINTN)FixedPcdGet32 (PcdOvmfCpuidBase),
> >>
> >> - FixedPcdGet32 (PcdOvmfCpuidSize),
> >>
> >> -};
> >>
> >> +STATIC
> >>
> >> +EFI_STATUS
> >>
> >> +AllocateConfidentialComputingBlob (
> >>
> >> + OUT CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION **CcBlobPtr
> >>
> >> + )
> >>
> >> +{
> >>
> >> + EFI_STATUS Status;
> >>
> >> + CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION *CcBlob;
> >>
> >> +
> >>
> >> + Status = gBS->AllocatePool (
> >>
> >> + EfiACPIReclaimMemory,
> >>
> >> + sizeof (CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION),
> >>
> >> + (VOID **)&CcBlob
> >>
> >> + );
> >>
> >> + if (EFI_ERROR (Status)) {
> >>
> >> + return Status;
> >>
> >> + }
> >>
> >> +
> >>
> >> + CcBlob->Header = SIGNATURE_32 ('A', 'M', 'D', 'E');
> >>
> >> + CcBlob->Version = 1;
> >>
> >> + CcBlob->Reserved1 = 0;
> >>
> >> + CcBlob->SecretsPhysicalAddress = (UINT64)(UINTN)FixedPcdGet32
> >> (PcdOvmfSnpSecretsBase);
> >>
> >> + CcBlob->SecretsSize = FixedPcdGet32 (PcdOvmfSnpSecretsSize);
> >>
> >> + CcBlob->CpuidPhysicalAddress = (UINT64)(UINTN)FixedPcdGet32
> >> (PcdOvmfCpuidBase);
> >>
> >> + CcBlob->CpuidLSize = FixedPcdGet32 (PcdOvmfCpuidSize);
> >>
> >> +
> >>
> >> + *CcBlobPtr = CcBlob;
> >>
> >> +
> >>
> >> + return EFI_SUCCESS;
> >>
> >> +}
> >>
> >>
> >>
> >> EFI_STATUS
> >>
> >> EFIAPI
> >>
> >> @@ -38,10 +59,11 @@ AmdSevDxeEntryPoint (
> >> IN EFI_SYSTEM_TABLE *SystemTable
> >>
> >> )
> >>
> >> {
> >>
> >> - EFI_STATUS Status;
> >>
> >> - EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap;
> >>
> >> - UINTN NumEntries;
> >>
> >> - UINTN Index;
> >>
> >> + EFI_STATUS Status;
> >>
> >> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap;
> >>
> >> + UINTN NumEntries;
> >>
> >> + UINTN Index;
> >>
> >> + CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION *SnpBootDxeTable;
> >>
> >>
> >>
> >> //
> >>
> >> // Do nothing when SEV is not enabled
> >>
> >> @@ -147,6 +169,18 @@ AmdSevDxeEntryPoint (
> >> }
> >>
> >> }
> >>
> >>
> >>
> >> + Status = AllocateConfidentialComputingBlob (&SnpBootDxeTable);
> >>
> >> + if (EFI_ERROR (Status)) {
> >>
> >> + DEBUG ((
> >>
> >> + DEBUG_ERROR,
> >>
> >> + "%a: AllocateConfidentialComputingBlob(): %r\n",
> >>
> >> + __FUNCTION__,
> >>
> >> + Status
> >>
> >> + ));
> >>
> >> + ASSERT (FALSE);
> >>
> >> + CpuDeadLoop ();
> >>
> >> + }
> >>
> >> +
> >>
> >> //
> >>
> >> // If its SEV-SNP active guest then install the
> >> CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
> >>
> >> // It contains the location for both the Secrets and CPUID page.
> >>
> >> @@ -154,7 +188,7 @@ AmdSevDxeEntryPoint (
> >> if (MemEncryptSevSnpIsEnabled ()) {
> >>
> >> return gBS->InstallConfigurationTable (
> >>
> >> &gConfidentialComputingSevSnpBlobGuid,
> >>
> >> - &mSnpBootDxeTable
> >>
> >> + SnpBootDxeTable
> >>
> >> );
> >>
> >> }
> >>
> >>
> >>
> >> --
> >> 2.25.1
> >>
> >>
> >>
> >>
> >>
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98146): https://edk2.groups.io/g/devel/message/98146
Mute This Topic: https://groups.io/mt/95815540/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