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.35311.1623070145771149430 for ; Mon, 07 Jun 2021 05:49:06 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=FeUrdg/C; 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=1623070145; 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=p+W2gxFwBYRAPC3o4B23QX2IC1avjRz7Ed+r/lHoads=; b=FeUrdg/CQIlqR2l499I8+52hZzj9UntGtvH6utvLHBgFsI38Ianpaqwh9O2nX/llQRjXge 9VuTN+OWuAa9L1450OGVni9/yRV3H9zE+e3bmfu3qLvUmN/FpzMu91SEm2tEwFuP+aeSJh Uj2IDQnbfEFIcNcuNVGps8VvrZ1elJc= 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-415-sJNR89hFPTG8lywzGVhGwA-1; Mon, 07 Jun 2021 08:49:01 -0400 X-MC-Unique: sJNR89hFPTG8lywzGVhGwA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9EAC3809839; Mon, 7 Jun 2021 12:48:59 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-75.ams2.redhat.com [10.36.114.75]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0110019C66; Mon, 7 Jun 2021 12:48:56 +0000 (UTC) Subject: Re: [PATCH RFC v3 05/22] OvmfPkg: reserve Secrets page in MEMFD From: "Laszlo Ersek" To: Brijesh Singh , James Bottomley , Min Xu , Jiewen Yao , Tom Lendacky , Jordan Justen , Erdem Aktas , Eric Dong , Ray Ni , Rahul Kumar , devel@edk2.groups.io Cc: Ard Biesheuvel References: <20210526231118.12946-1-brijesh.singh@amd.com> <20210526231118.12946-6-brijesh.singh@amd.com> <6c1d0c68-0537-9b58-ada4-ec9deb1a7c9d@redhat.com> Message-ID: <55475e6f-d2fa-b33f-57a1-f82a1ea3fc2f@redhat.com> Date: Mon, 7 Jun 2021 14:48:55 +0200 MIME-Version: 1.0 In-Reply-To: <6c1d0c68-0537-9b58-ada4-ec9deb1a7c9d@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 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 06/07/21 14:26, Laszlo Ersek wrote: > On 05/27/21 01:11, Brijesh Singh wrote: >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275 >> >> When AMD SEV is enabled in the guest VM, a hypervisor need to insert a >> secrets page. > > For pure SEV? > >> >> When SEV-SNP is enabled, the secrets page contains the VM platform >> communication keys. The guest BIOS and OS can use this key to communicate >> with the SEV firmware to get attesation report. See the SEV-SNP firmware >> spec for more details for the content of the secrets page. >> >> When SEV and SEV-ES is enabled, the secrets page contains the information >> provided by the guest owner after the attestation. See the SEV >> LAUNCH_SECRET command for more details. >> >> 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/OvmfPkgX64.dsc | 2 ++ >> OvmfPkg/OvmfPkgX64.fdf | 5 +++++ >> OvmfPkg/AmdSev/SecretPei/SecretPei.inf | 1 + >> OvmfPkg/AmdSev/SecretPei/SecretPei.c | 15 ++++++++++++++- >> 4 files changed, 22 insertions(+), 1 deletion(-) > > How is all of the above related to the "OvmfPkg/OvmfPkgX64.dsc" > platform, where remote attestation is not a goal? > > What you describe makes sense to me, but only for the remote-attested > "OvmfPkg/AmdSev/AmdSevX64.dsc" platform. (Which already includes > SecretPei and SecretDxe, and sets the necessary PCDs.) > > Then, even if we limit this patch only to the "OvmfPkg/AmdSev/SecretPei" > module, the commit message does not explain sufficiently why the secrets > page must be reserved for good. The "SEV-SNP firmware spec" reference is > vague at best; I'm permanently lost between the dozen PDF files I have > downloaded locally from the AMD website. Please include a specific > document number, revision number, and chapter/section identifier. > > Honestly I'm getting a *rushed* vibe on this whole series. Why is that? > > Assume that I'm dumb. You won't be far from the truth. Then hold my hand > through all this? Here's the v2 discussion: - http://mid.mail-archive.com/9804ecb5-8afd-c56e-4982-d1a6ebad3de8@redhat.com - https://edk2.groups.io/g/devel/message/74797 - https://listman.redhat.com/archives/edk2-devel-archive/2021-May/msg00112.html That discussion refers to a different use case, raised by Dov. That use case might justify reserving the area even for plain SEV. It's out of scope for now, AIUI. ( And even for that separate use case, James showed down-thread that *not* reserving the page forever in the firmware is more flexible. - http://mid.mail-archive.com/aed7d3490fe6edee74440ed8e4cd5364fb2ba4af.camel@linux.ibm.com - https://edk2.groups.io/g/devel/message/74801 - https://listman.redhat.com/archives/edk2-devel-archive/2021-May/msg00116.html ) AFAICT, the only effect of the v2 sub-thread on the patch has been that we now use the Reserved memory type rather than AcpiNVS (when SEV-SNP is in use). I have two comments on that: - It's good that we're not mixing in the other use case raised by Dov (i.e., enabling the guest-kernel to read secrets from the injected page even under plain SEV). - It's still unclear to me why the reservation needs to be permanent under SEV-SNP. Thanks Laszlo >> >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >> index 999738dc39cd..ea08e1fabc65 100644 >> --- a/OvmfPkg/OvmfPkgX64.dsc >> +++ b/OvmfPkg/OvmfPkgX64.dsc >> @@ -716,6 +716,7 @@ [Components] >> OvmfPkg/SmmAccess/SmmAccessPei.inf >> !endif >> UefiCpuPkg/CpuMpPei/CpuMpPei.inf >> + OvmfPkg/AmdSev/SecretPei/SecretPei.inf >> >> !if $(TPM_ENABLE) == TRUE >> OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> @@ -966,6 +967,7 @@ [Components] >> OvmfPkg/PlatformDxe/Platform.inf >> OvmfPkg/AmdSevDxe/AmdSevDxe.inf >> OvmfPkg/IoMmuDxe/IoMmuDxe.inf >> + OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf >> >> !if $(SMM_REQUIRE) == TRUE >> OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf >> index d6be798fcadd..9126b8eb5014 100644 >> --- a/OvmfPkg/OvmfPkgX64.fdf >> +++ b/OvmfPkg/OvmfPkgX64.fdf >> @@ -88,6 +88,9 @@ [FD.MEMFD] >> 0x00C000|0x001000 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize >> >> +0x00D000|0x001000 >> +gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize >> + >> 0x010000|0x010000 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize >> >> @@ -179,6 +182,7 @@ [FV.PEIFV] >> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf >> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf >> !endif >> +INF OvmfPkg/AmdSev/SecretPei/SecretPei.inf >> >> ################################################################################ >> >> @@ -314,6 +318,7 @@ [FV.DXEFV] >> INF ShellPkg/Application/Shell/Shell.inf >> >> INF MdeModulePkg/Logo/LogoDxe.inf >> +INF OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf >> >> # >> # Network modules >> diff --git a/OvmfPkg/AmdSev/SecretPei/SecretPei.inf b/OvmfPkg/AmdSev/SecretPei/SecretPei.inf >> index 08be156c4bc0..9265f8adee12 100644 >> --- a/OvmfPkg/AmdSev/SecretPei/SecretPei.inf >> +++ b/OvmfPkg/AmdSev/SecretPei/SecretPei.inf >> @@ -26,6 +26,7 @@ [LibraryClasses] >> HobLib >> PeimEntryPoint >> PcdLib >> + MemEncryptSevLib >> >> [FixedPcd] >> gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase >> diff --git a/OvmfPkg/AmdSev/SecretPei/SecretPei.c b/OvmfPkg/AmdSev/SecretPei/SecretPei.c >> index ad491515dd5d..51eb094555aa 100644 >> --- a/OvmfPkg/AmdSev/SecretPei/SecretPei.c >> +++ b/OvmfPkg/AmdSev/SecretPei/SecretPei.c >> @@ -7,6 +7,7 @@ >> #include >> #include >> #include >> +#include >> >> EFI_STATUS >> EFIAPI >> @@ -15,10 +16,22 @@ InitializeSecretPei ( >> IN CONST EFI_PEI_SERVICES **PeiServices >> ) >> { >> + UINTN Type; >> + >> + // >> + // The location of the secret page should be marked reserved so that guest OS >> + // does not treated as a system RAM. >> + // >> + if (MemEncryptSevSnpIsEnabled ()) { >> + Type = EfiReservedMemoryType; >> + } else { >> + Type = EfiBootServicesData; >> + } >> + >> BuildMemoryAllocationHob ( >> PcdGet32 (PcdSevLaunchSecretBase), >> PcdGet32 (PcdSevLaunchSecretSize), >> - EfiBootServicesData >> + Type >> ); >> >> return EFI_SUCCESS; >> >