From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.158.5]) by mx.groups.io with SMTP id smtpd.web12.7709.1620298674866254975 for ; Thu, 06 May 2021 03:57:55 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@ibm.com header.s=pp1 header.b=kDMndCxz; spf=none, err=permanent DNS error (domain: linux.vnet.ibm.com, ip: 148.163.158.5, mailfrom: dovmurik@linux.vnet.ibm.com) Received: from pps.filterd (m0098419.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 146ApGnh005255; Thu, 6 May 2021 06:57:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=subject : to : cc : references : from : message-id : date : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pp1; bh=kzJeRXoRw3C9QxzjbV0v5nL/B7jHw8mcHNiHTn9PBkM=; b=kDMndCxzaNRw6d6VBmvYmzV6jMbVv20OYqqiIgLDZqXmwnjKw+4ws5Xq909dH1sc769G AJ2KpRJulz0zt37Dt+S8fi5ixnYKY/fjQxjGiK4EvIBF3VuvrDaavAW8qYPoT+tDX3Xz gE7iJF0EHT1yqTX9eVfJyy4CIzQqwLPTgHauaFYsd9IeiW6hD5MQRaHkC0Dtp5/mJpXT fW1H6ENffsORxt7H1/k+jE/ERsFj3Dtuou3zoBmexS9nGgHxK6xZmQp8fYnsMbI6q14j 2ri6nkZGq/z9Rnm0IRCrpcwNgX68xUcAEl28vMIqk6MHOSEtmq/5svh9u0/QIw7WrfGl 6A== Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com with ESMTP id 38cf2rg3f8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 06 May 2021 06:57:52 -0400 Received: from m0098419.ppops.net (m0098419.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 146AuSk4026569; Thu, 6 May 2021 06:57:51 -0400 Received: from ppma03ams.nl.ibm.com (62.31.33a9.ip4.static.sl-reverse.com [169.51.49.98]) by mx0b-001b2d01.pphosted.com with ESMTP id 38cf2rg3ee-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 06 May 2021 06:57:51 -0400 Received: from pps.filterd (ppma03ams.nl.ibm.com [127.0.0.1]) by ppma03ams.nl.ibm.com (8.16.0.43/8.16.0.43) with SMTP id 146AqXFf013440; Thu, 6 May 2021 10:57:49 GMT Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by ppma03ams.nl.ibm.com with ESMTP id 38bedxrt94-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 06 May 2021 10:57:49 +0000 Received: from d06av22.portsmouth.uk.ibm.com (d06av22.portsmouth.uk.ibm.com [9.149.105.58]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 146Avltw24969618 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 6 May 2021 10:57:47 GMT Received: from d06av22.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5DEE14C04A; Thu, 6 May 2021 10:57:47 +0000 (GMT) Received: from d06av22.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7C24D4C046; Thu, 6 May 2021 10:57:46 +0000 (GMT) Received: from [9.148.12.194] (unknown [9.148.12.194]) by d06av22.portsmouth.uk.ibm.com (Postfix) with ESMTP; Thu, 6 May 2021 10:57:46 +0000 (GMT) Subject: Re: [edk2-devel] [PATCH RFC v2 11/28] OvmfPkg: Reserve Secrets page in MEMFD To: Laszlo Ersek , devel@edk2.groups.io, brijesh.singh@amd.com Cc: James Bottomley , Min Xu , Jiewen Yao , Tom Lendacky , Jordan Justen , Ard Biesheuvel , Erdem Aktas , "tobin@ibm.com" References: <20210430115148.22267-1-brijesh.singh@amd.com> <20210430115148.22267-12-brijesh.singh@amd.com> <8b46fe32-beda-0195-8c67-c7ef19194f85@linux.vnet.ibm.com> From: "Dov Murik" Message-ID: Date: Thu, 6 May 2021 13:57:46 +0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 In-Reply-To: X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: Bca8xa8_rRVUNlEkJttnhVLdYNLceYMa X-Proofpoint-GUID: uV3AaY6u5i-2T1mhAkZzBRcZhnqVQToH X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.761 definitions=2021-05-06_06:2021-05-06,2021-05-06 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 spamscore=0 mlxscore=0 bulkscore=0 adultscore=0 suspectscore=0 mlxlogscore=999 lowpriorityscore=0 phishscore=0 impostorscore=0 priorityscore=1501 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104060000 definitions=main-2105060072 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 05/05/2021 22:33, Laszlo Ersek wrote: > On 05/05/21 15:11, Brijesh Singh wrote: >> >> On 5/5/21 1:42 AM, Dov Murik wrote: >>> [+cc: Tobin] >>> >>> Hi Brijesh, >>> >>> On 30/04/2021 14:51, Brijesh Singh wrote: >>>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&data=04%7C01%7Cbrijesh.singh%40amd.com%7C93168c94eb6d44ed08e608d90f910426%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637557937779907471%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=nLpmk3G%2BmXcZrzXxCmO3M9EDPiLRnP1IUmPqRQNbBuU%3D&reserved=0 >>>> >>>> When AMD SEV is enabled in the guest VM, a hypervisor need to insert a >>>> secrets page. >>>> >>>> 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/AmdSev/SecretPei/SecretPei.c | 16 +++++++++++++++- >>>> OvmfPkg/AmdSev/SecretPei/SecretPei.inf | 1 + >>>> OvmfPkg/OvmfPkgX64.dsc | 2 ++ >>>> OvmfPkg/OvmfPkgX64.fdf | 5 +++++ >>>> 4 files changed, 23 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/OvmfPkg/AmdSev/SecretPei/SecretPei.c b/OvmfPkg/AmdSev/SecretPei/SecretPei.c >>>> index ad491515dd..92836c562c 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,23 @@ InitializeSecretPei ( >>>> IN CONST EFI_PEI_SERVICES **PeiServices >>>> ) >>>> { >>>> + UINTN Type; >>>> + >>>> + // >>>> + // The secret page should be mapped encrypted by the guest OS and must not >>>> + // be treated as a system RAM. Mark it as ACPI NVS so that guest OS maps it >>>> + // encrypted. >>>> + // >>>> + if (MemEncryptSevSnpIsEnabled ()) { >>>> + Type = EfiACPIMemoryNVS; >>>> + } else { >>>> + Type = EfiBootServicesData; >>>> + } >>>> + >>> Would it make sense to always use EfiACPIMemoryNVS for the injected secret area, even for regular SEV (non-SNP)? >> >> Ideally yes. Maybe James had some reasons for choosing the >> EfiBootServicesData. If I had to guess, it was mainly because there no >> guest kernel support which consumes the SEV secrets page. > > git-blame fingers commit bff2811c6d99 ("OvmfPkg/AmdSev: assign and > reserve the Sev Secret area", 2020-12-14). > > Commit bff2811c6d99 makes it clear that the area in question lives in MEMFD. > > We're populating the area in the PEI phase. We don't want anything in > DXE to overwrite it. > > Once the bootloader (and/or perhaps the kernel's EFI stub) fetched the > secret from that particular location, there is no need to prevent later > parts of the OS (the actual kernel) from repurposing that area. That's > why EfiBootServicesData was used. > The first use of the secret area was to hold the guest luks disk passphrase; this is used in the grub-inside-OVMF (AmdSev package), and there was no need to keep that page around for the guest kernel. The reason I'm raising this whole point is that we're working now on guest-kernel support for reading secrets from that injected page (for plain SEV). We considered either (a) modifying the secrets page memory type to reserved here, or (b) add code to the kernel EFI stub that would copy this page somewhere else for kernel's later use (which seems more work and not sure what's the advantage). Option (b) seems harder and more fragile, and I'm not sure if there are any advantages (though I'm definitely not an expert in that area). >> Since the >> memory is not marked ACPI NVS, so it can be used as a system RAM after >> the ExitBootServices is called in the kernel. > > Yes. > > I don't think AcpiNVS would be a good fit. Linux saves and restores > AcpiNVS areas upon S3 suspend/resume. Regardless of whether S3 works, or > will work, in SEV* guests, if we don't want the guest kernel to touch > that area *at all*, Reserved is a better type. Thanks for this clarification. > > Please refer to "Table 7-6 Memory Type Usage after ExitBootServices()" > in the UEFI spec (v2.9). > >> >> I am fine with using ACPI NVS for both SEV and SEV-SNP. I was not able >> to build and run AmdSev package in my setup, can you submit a prepatch >> to change the memory type and verify that it works ? > > NB: I've not yet reached this patch in my own review of the series, so > I'm likely missing some context. I do have a thought -- under SEV-SNP, > the secrets page apparently needs different (stronger) protection from > the host as under plain SEV. I don't think that hiding the different > protection requirements behind a single common memory type is helpful. > Not to mention the wasted memory in the plain SEV case -- it's not a lot > of memory, mind you, but the principle matters. > Like I said above, we have plans to have this small amount of memory available also to the guest OS; so maybe that shouldn't be the driving force in the decision here. -Dov > So ATM I would like to keep this patch in the SEV-SNP series, and to > preserve the different memory types between SEV and SEV-SNP. > > Thanks > Laszlo > > > > >> >>> >>> -Dov >>> >>> >>> >>>> BuildMemoryAllocationHob ( >>>> PcdGet32 (PcdSevLaunchSecretBase), >>>> PcdGet32 (PcdSevLaunchSecretSize), >>>> - EfiBootServicesData >>>> + Type >>>> ); >>>> >>>> return EFI_SUCCESS; >>>> diff --git a/OvmfPkg/AmdSev/SecretPei/SecretPei.inf b/OvmfPkg/AmdSev/SecretPei/SecretPei.inf >>>> index 08be156c4b..9265f8adee 100644 >>>> --- a/OvmfPkg/AmdSev/SecretPei/SecretPei.inf >>>> +++ b/OvmfPkg/AmdSev/SecretPei/SecretPei.inf >>>> @@ -26,6 +26,7 @@ >>>> HobLib >>>> PeimEntryPoint >>>> PcdLib >>>> + MemEncryptSevLib >>>> >>>> [FixedPcd] >>>> gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase >>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >>>> index a7d747f6b4..593c0e69f6 100644 >>>> --- a/OvmfPkg/OvmfPkgX64.dsc >>>> +++ b/OvmfPkg/OvmfPkgX64.dsc >>>> @@ -716,6 +716,7 @@ >>>> OvmfPkg/SmmAccess/SmmAccessPei.inf >>>> !endif >>>> UefiCpuPkg/CpuMpPei/CpuMpPei.inf >>>> + OvmfPkg/AmdSev/SecretPei/SecretPei.inf >>>> >>>> !if $(TPM_ENABLE) == TRUE >>>> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >>>> @@ -965,6 +966,7 @@ >>>> 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 d519f85328..b04175f77c 100644 >>>> --- a/OvmfPkg/OvmfPkgX64.fdf >>>> +++ b/OvmfPkg/OvmfPkgX64.fdf >>>> @@ -88,6 +88,9 @@ gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevE >>>> 0x00C000|0x001000 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize >>>> >>>> +0x00D000|0x001000 >>>> +gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize >>>> + >>>> 0x010000|0x010000 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize >>>> >>>> @@ -178,6 +181,7 @@ INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >>>> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf >>>> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf >>>> !endif >>>> +INF OvmfPkg/AmdSev/SecretPei/SecretPei.inf >>>> >>>> ################################################################################ >>>> >>>> @@ -313,6 +317,7 @@ INF OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.inf >>>> INF ShellPkg/Application/Shell/Shell.inf >>>> >>>> INF MdeModulePkg/Logo/LogoDxe.inf >>>> +INF OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf >>>> >>>> # >>>> # Network modules >>>> >> >> >> >> >> >