Thanks Mike for fixing this.
On 21/12/2022 18:06, Michael Roth wrote:
> 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 <dovmu...@linux.ibm.com>
> Suggested-by: Dov Murik <dovmu...@linux.ibm.com>
> Signed-off-by: Michael Roth <michael.r...@amd.com>
Reviewed-by: Dov Murik <dovmu...@linux.ibm.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
>
> );
>
> }
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97711): https://edk2.groups.io/g/devel/message/97711
Mute This Topic: https://groups.io/mt/95815540/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
