Re: [PATCH RFC v3 05/22] OvmfPkg: reserve Secrets page in MEMFD

["Brijesh Singh" <brijesh.singh@amd.com>]

On 6/7/21 7:48 AM, Laszlo Ersek wrote:
> On 06/07/21 14:26, Laszlo Ersek wrote:
>> On 05/27/21 01:11, 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%7Cc7a508dbd4af461b413208d929b2a231%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637586669489236720%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=OjlLNpUW8U%2FykMMU7JwjddEZd9Zi%2BsHNK%2FqoQCwW3vo%3D&reserved=0
>>>
>>> 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 <jejb@linux.ibm.com>
>>> Cc: Min Xu <min.m.xu@intel.com>
>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Erdem Aktas <erdemaktas@google.com>
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> ---
>>>  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:
>
> - https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.mail-archive.com%2F9804ecb5-8afd-c56e-4982-d1a6ebad3de8%40redhat.com&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc7a508dbd4af461b413208d929b2a231%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637586669489236720%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=K8FRcks19dQ4BM4DBOh%2F7uO4hNvIsM0eqdNvwUQzDUU%3D&amp;reserved=0
> - https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F74797&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc7a508dbd4af461b413208d929b2a231%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637586669489236720%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=8rfpRAEvBdWex0BQctCbbGnHb691gcKSIEvVA3ZKDkg%3D&amp;reserved=0
> - https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2021-May%2Fmsg00112.html&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc7a508dbd4af461b413208d929b2a231%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637586669489246713%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NAL8jAfiq1EApkDBOBjgL7b3NIsmjginZSDxB1NDCk8%3D&amp;reserved=0
>
> 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.
>
> - https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmid.mail-archive.com%2Faed7d3490fe6edee74440ed8e4cd5364fb2ba4af.camel%40linux.ibm.com&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc7a508dbd4af461b413208d929b2a231%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637586669489246713%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=2UV6KcGYb9CoKzgIU%2FscCX2l%2F5pKaSkFYshP%2BPSWHSM%3D&amp;reserved=0
> - https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F74801&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc7a508dbd4af461b413208d929b2a231%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637586669489246713%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=jnHpYxYkijt2LtcH772m88%2BLNH3Zjfn3Zqc3uuttL1M%3D&amp;reserved=0
> - https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2021-May%2Fmsg00116.html&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cc7a508dbd4af461b413208d929b2a231%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637586669489246713%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=XzePjtTDS8blsXhBNOg52uo81uFhoYpgcNMvU4RupSI%3D&amp;reserved=0
>
> )
>
> 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.


As highlighted in the previous email, in the case of SEV, the secrets
page contains the private data provided by the guest owner to the guest.
Whereas, in SEV-SNP, the secrets page includes the key and other
metadata used by the guest (OVMF or kernel) to construct a message for
the PSP. The secrets page contains some information (e.g key and
sequence number) that must persist across kexec boots. If we mark the
SEV-SNP secrets page as "Boot Data," I believe it gets free'd on
ExitBootService(). In the kexec'ed kernel, we need to retrieve the
secret page to get the key and message counters to construct the next
PSP quest request command.


I have not looked into detail on how EFI configuration table and other
data is preserved during the kexec boot, but I thought making the
secrets reserved should ensure that memory is not free'd on
ExitBootServices() and we can reach it after the kexec. I can
investigate a bit more.

Dov/James,
 
Does kexec works with the SEV secrets page?

-Brijesh