From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web09.1946.1620243448044271979 for ; Wed, 05 May 2021 12:37:28 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=YJPRF7qO; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620243447; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aKU1VTasp+5qGTR7HI62ZjXWV5JaA6QX5C1Ub9HIExc=; b=YJPRF7qOXafdt5PShUiMcVhHMLwlLi0IPZsqDoKSrGbRCDtwsYh4qgw19LNupPmlOsa2mS we1QO/jXbvE08OSFJiINlOiDDmTtsqRTl71d7f5WOPzUYLCcTe6hrbP2J9N+rlmTaEoLMR Pi3GtRUDvCfbwnxaZRmevBQwOA9AFx8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-113-99P-4uv-M9iOpy-wDmSTpA-1; Wed, 05 May 2021 15:37:23 -0400 X-MC-Unique: 99P-4uv-M9iOpy-wDmSTpA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id D24535020E; Wed, 5 May 2021 19:37:21 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-136.ams2.redhat.com [10.36.113.136]) by smtp.corp.redhat.com (Postfix) with ESMTP id 958C760864; Wed, 5 May 2021 19:37:19 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH RFC v2 27/28] OvmfPkg/AmdSev: Expose the SNP reserved pages through configuration table To: devel@edk2.groups.io, dovmurik@linux.vnet.ibm.com, brijesh.singh@amd.com Cc: James Bottomley , Min Xu , Jiewen Yao , Tom Lendacky , Jordan Justen , Ard Biesheuvel , Erdem Aktas References: <20210430115148.22267-1-brijesh.singh@amd.com> <20210430115148.22267-28-brijesh.singh@amd.com> <54944028-2676-7bf9-25ee-b4d162fead43@linux.vnet.ibm.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 5 May 2021 21:37:18 +0200 MIME-Version: 1.0 In-Reply-To: <54944028-2676-7bf9-25ee-b4d162fead43@linux.vnet.ibm.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 05/05/21 09:10, Dov Murik wrote: > Hi Brijesh, > > On 30/04/2021 14:51, Brijesh Singh wrote: >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275 >> >> Now that both the secrets and cpuid pages are reserved in the HOB, >> extract the location details through fixed PCD and make it available >> to the guest OS through the configuration table. >> >> Cc: James Bottomley >> Cc: Min Xu >> Cc: Jiewen Yao >> Cc: Tom Lendacky >> Cc: Jordan Justen >> Cc: Ard Biesheuvel >> Cc: Laszlo Ersek >> Cc: Erdem Aktas >> Signed-off-by: Brijesh Singh >> --- >> OvmfPkg/AmdSev/SecretDxe/SecretDxe.c | 21 ++++++++++++++++++++ >> OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf | 4 ++++ >> OvmfPkg/Include/Guid/ConfidentialComputingSecret.h | 17 ++++++++++++++++ >> OvmfPkg/OvmfPkg.dec | 1 + >> 4 files changed, 43 insertions(+) >> >> diff --git a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c >> index 308022b5b2..08b6d9bddf 100644 >> --- a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c >> +++ b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c >> @@ -6,6 +6,7 @@ >> **/ >> #include >> #include >> +#include >> #include >> >> STATIC CONFIDENTIAL_COMPUTING_SECRET_LOCATION mSecretDxeTable = { >> @@ -13,6 +14,15 @@ STATIC CONFIDENTIAL_COMPUTING_SECRET_LOCATION mSecretDxeTable = { >> FixedPcdGet32 (PcdSevLaunchSecretSize), >> }; >> >> +STATIC CONFIDENTIAL_COMPUTING_BLOB_LOCATION mSnpBootDxeTable = { >> + 0x414d4445, // AMDE > > (nit: I believe this UINT32 will appear in memory as the string "EDMA".) Please consider the SIGNATURE_32() macro. > > > >> + 1, > > Not sure what's the official stance regarding a version field here. Maybe it's better to generate a new GUID whenever there's a struct change. A version scalar is good for compatible changes (= only appending new fields). Incompatible changes require GUID changes. (I'll review this patch myself from the scratch later; just making some quick comments-on-comments for the time being.) Thanks Laszlo > > > -Dov > > >> + (UINT64)(UINTN) FixedPcdGet32 (PcdSevLaunchSecretBase), >> + FixedPcdGet32 (PcdSevLaunchSecretSize), >> + (UINT64)(UINTN) FixedPcdGet32 (PcdOvmfSnpCpuidBase), >> + FixedPcdGet32 (PcdOvmfSnpCpuidSize), >> +}; >> + >> EFI_STATUS >> EFIAPI >> InitializeSecretDxe( >> @@ -20,6 +30,17 @@ InitializeSecretDxe( >> IN EFI_SYSTEM_TABLE *SystemTable >> ) >> { >> + // >> + // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_BLOB. >> + // It contains the location for both the Secrets and CPUID page. >> + // >> + if (MemEncryptSevSnpIsEnabled ()) { >> + return gBS->InstallConfigurationTable ( >> + &gConfidentialComputingBlobGuid, >> + &mSnpBootDxeTable >> + ); >> + } >> + >> return gBS->InstallConfigurationTable ( >> &gConfidentialComputingSecretGuid, >> &mSecretDxeTable >> diff --git a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf >> index 40bda7ff84..d15194b368 100644 >> --- a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf >> +++ b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf >> @@ -23,13 +23,17 @@ >> MdePkg/MdePkg.dec >> >> [LibraryClasses] >> + MemEncryptSevLib >> UefiBootServicesTableLib >> UefiDriverEntryPoint >> >> [Guids] >> gConfidentialComputingSecretGuid >> + gConfidentialComputingBlobGuid >> >> [FixedPcd] >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize >> gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase >> gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize >> >> diff --git a/OvmfPkg/Include/Guid/ConfidentialComputingSecret.h b/OvmfPkg/Include/Guid/ConfidentialComputingSecret.h >> index 7026fc5b08..0d7f1b8818 100644 >> --- a/OvmfPkg/Include/Guid/ConfidentialComputingSecret.h >> +++ b/OvmfPkg/Include/Guid/ConfidentialComputingSecret.h >> @@ -18,11 +18,28 @@ >> { 0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47 }, \ >> } >> >> +#define CONFIDENTIAL_COMPUTING_BLOB_GUID \ >> + { 0x067b1f5f, \ >> + 0xcf26, \ >> + 0x44c5, \ >> + { 0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42 }, \ >> + } >> + >> typedef struct { >> UINT64 Base; >> UINT64 Size; >> } CONFIDENTIAL_COMPUTING_SECRET_LOCATION; >> >> +typedef struct { >> + UINT32 Header; >> + UINT16 Version; >> + UINT64 SecretsPhysicalAddress; >> + UINT32 SecretsSize; >> + UINT64 CpuidPhysicalAddress; >> + UINT32 CpuidLSize; >> +} CONFIDENTIAL_COMPUTING_BLOB_LOCATION; >> + >> extern EFI_GUID gConfidentialComputingSecretGuid; >> +extern EFI_GUID gConfidentialComputingBlobGuid; >> >> #endif // SEV_LAUNCH_SECRET_H_ >> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec >> index d1bfe49731..f38c5e476a 100644 >> --- a/OvmfPkg/OvmfPkg.dec >> +++ b/OvmfPkg/OvmfPkg.dec >> @@ -126,6 +126,7 @@ >> gQemuKernelLoaderFsMediaGuid = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}} >> gGrubFileGuid = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}} >> gConfidentialComputingSecretGuid = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}} >> + gConfidentialComputingBlobGuid = {0x067b1f5f, 0xcf26, 0x44c5, {0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42}} >> >> [Ppis] >> # PPI whose presence in the PPI database signals that the TPM base address >> > > > > >