[PATCH 0/2] OvmgPkg: Add SEV launch secret and hashes table areas

[PATCH 0/2] OvmgPkg: Add SEV launch secret and hashes table areas

[Dov Murik]

The SEV launch secret area and the QEMU hashes table area were specified
in the OvmfPkg/AmdSev/AmdSevX64 MEMFD but not in OvmfPkg/OvmfPkgX64 and
in OvmgPkg/Microvm/MicrovmX64.

This series adds theses MEMFD entries to both targets.  It allows QEMU
to discover the secrets area when performing SEV/SEV-ES secret
injection, and to properly fill the hashes table (though currently these
targets do not perform hashes verification when loading
kernel/initrd/cmdline from QEMU via fw_cfg).

After applying the patches, the MEMFD section of the three targets' fdf
files is identical:

    $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/OvmfPkgX64.fdf | sha1sum
    6ff89173952413fbdb7ffbbf42f8bc389c928500  -
    $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/Microvm/MicrovmX64.fdf | sha1sum
    6ff89173952413fbdb7ffbbf42f8bc389c928500  -
    $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/AmdSev/AmdSevX64.fdf | sha1sum
    6ff89173952413fbdb7ffbbf42f8bc389c928500  -

Code is in:
https://github.com/confidential-containers-demo/edk2/tree/add-sev-secret-and-hashes

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>

Dov Murik (2):
  OvmfPkg/OvmfPkgX64: Add SEV launch secret and hashes table areas to
    MEMFD
  OvmfPkg/Microvm: Add SEV launch secret and hashes table areas to MEMFD

 OvmfPkg/Microvm/MicrovmX64.fdf | 8 +++++++-
 OvmfPkg/OvmfPkgX64.fdf         | 8 +++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/2] OvmfPkg/OvmfPkgX64: Add SEV launch secret and hashes table areas to MEMFD

[Dov Murik]

The SEV launch secret area and the QEMU hashes table area were specified
in the OvmfPkg/AmdSev/AmdSevX64 MEMFD but not in OvmfPkg/OvmfPkgX64.

Add them in OvmfPkgX64.fdf.

After this change the two MEMFD descriptions are identical:

    $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/OvmfPkgX64.fdf | sha1sum
    6ff89173952413fbdb7ffbbf42f8bc389c928500  -
    $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/AmdSev/AmdSevX64.fdf | sha1sum
    6ff89173952413fbdb7ffbbf42f8bc389c928500  -

Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Reported-by: Brijesh Singh <brijesh.singh@amd.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
---
 OvmfPkg/OvmfPkgX64.fdf | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index b6cc3cabdd69..ee323082b465 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -85,7 +85,13 @@ [FD.MEMFD]
 0x00B000|0x001000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize
 
-0x00C000|0x001000
+0x00C000|0x000C00
+gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize
+
+0x00CC00|0x000400
+gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize
+
+0x00D000|0x001000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
 
 0x010000|0x010000
-- 
2.25.1

Re: [PATCH 1/2] OvmfPkg/OvmfPkgX64: Add SEV launch secret and hashes table areas to MEMFD

[Gerd Hoffmann]

On Tue, Nov 02, 2021 at 07:34:21AM +0000, Dov Murik wrote:
> The SEV launch secret area and the QEMU hashes table area were specified
> in the OvmfPkg/AmdSev/AmdSevX64 MEMFD but not in OvmfPkg/OvmfPkgX64.
> 
> Add them in OvmfPkgX64.fdf.
> 
> After this change the two MEMFD descriptions are identical:
> 
>     $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/OvmfPkgX64.fdf | sha1sum
>     6ff89173952413fbdb7ffbbf42f8bc389c928500  -
>     $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/AmdSev/AmdSevX64.fdf | sha1sum
>     6ff89173952413fbdb7ffbbf42f8bc389c928500  -

I'm wondering whenever you actually tried to boot a sev guest
in microvm?

I suspect it'll need more changes to actually work.

take care,
  Gerd

Re: [PATCH 1/2] OvmfPkg/OvmfPkgX64: Add SEV launch secret and hashes table areas to MEMFD

[Dov Murik]

Hi Gerd,

(I assume your comments are for patch 2/2)

On 02/11/2021 12:03, Gerd Hoffmann wrote:
> On Tue, Nov 02, 2021 at 07:34:21AM +0000, Dov Murik wrote:
>> The SEV launch secret area and the QEMU hashes table area were specified
>> in the OvmfPkg/AmdSev/AmdSevX64 MEMFD but not in OvmfPkg/OvmfPkgX64.
>>
>> Add them in OvmfPkgX64.fdf.
>>
>> After this change the two MEMFD descriptions are identical:
>>
>>     $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/OvmfPkgX64.fdf | sha1sum
>>     6ff89173952413fbdb7ffbbf42f8bc389c928500  -
>>     $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/AmdSev/AmdSevX64.fdf | sha1sum
>>     6ff89173952413fbdb7ffbbf42f8bc389c928500  -
> 
> I'm wondering whenever you actually tried to boot a sev guest
> in microvm?
> 

No I haven't tried.  Do you want Microvm to be able to boot SEV guests,
or do you intentionally want to keep functionality out so it stays small?

> I suspect it'll need more changes to actually work.
> 

I saw MicrovmX64.fdf already has some SEV-related entries (like
PcdOvmfSecGhcbBackupBase), so I just added these so that its MEMFD will
be identical to AmdSevX64 and OvmfPkgX64.

-Dov

Re: [PATCH 1/2] OvmfPkg/OvmfPkgX64: Add SEV launch secret and hashes table areas to MEMFD

[Gerd Hoffmann]

  Hi,

> > I'm wondering whenever you actually tried to boot a sev guest
> > in microvm?
> 
> No I haven't tried.  Do you want Microvm to be able to boot SEV guests,
> or do you intentionally want to keep functionality out so it stays small?

Need to look at it on a case by case base.  It is clearly not a
priority, but if it makes sense we can discuss adding it.

microvm has no support for SMM mode, and that is unlikely to change,
so anything requiring SMM mode is not going to work, thats why I dropped
SMM + secure boot + TPM bits for the initial patch series.

Having support for tpm makes sense even without secure boot, so we might
bring that back, but it'll also require some (small) changes on the host
side so qemu allows creating a tpm, generates acpi tables for the tpm etc.

Does SEV need and/or use SMM mode?  Looking through AmdSevX64.dsc
doesn't give a clear answer, on one hand there is a
LibraryClasses.common.SMM_CORE section, but on the other hand it uses
the non-SMM variable driver stack.

take care,
  Gerd

Re: [PATCH 1/2] OvmfPkg/OvmfPkgX64: Add SEV launch secret and hashes table areas to MEMFD

[Dov Murik]

On 02/11/2021 15:29, Gerd Hoffmann wrote:
>   Hi,
> 
>>> I'm wondering whenever you actually tried to boot a sev guest
>>> in microvm?
>>
>> No I haven't tried.  Do you want Microvm to be able to boot SEV guests,
>> or do you intentionally want to keep functionality out so it stays small?
> 
> Need to look at it on a case by case base.  It is clearly not a
> priority, but if it makes sense we can discuss adding it.
> 
> microvm has no support for SMM mode, and that is unlikely to change,
> so anything requiring SMM mode is not going to work, thats why I dropped
> SMM + secure boot + TPM bits for the initial patch series.
> 
> Having support for tpm makes sense even without secure boot, so we might
> bring that back, but it'll also require some (small) changes on the host
> side so qemu allows creating a tpm, generates acpi tables for the tpm etc.
> 
> Does SEV need and/or use SMM mode?  Looking through AmdSevX64.dsc
> doesn't give a clear answer, on one hand there is a
> LibraryClasses.common.SMM_CORE section, but on the other hand it uses
> the non-SMM variable driver stack.

I think SEV doesn't work with SMM.  James - can you please give a more
definitive answer here?

-Dov

Re: [PATCH 1/2] OvmfPkg/OvmfPkgX64: Add SEV launch secret and hashes table areas to MEMFD

[Lendacky, Thomas]

On 11/2/21 8:53 AM, Dov Murik wrote:
> 
> 
> On 02/11/2021 15:29, Gerd Hoffmann wrote:
>>    Hi,
>>
>>>> I'm wondering whenever you actually tried to boot a sev guest
>>>> in microvm?
>>>
>>> No I haven't tried.  Do you want Microvm to be able to boot SEV guests,
>>> or do you intentionally want to keep functionality out so it stays small?
>>
>> Need to look at it on a case by case base.  It is clearly not a
>> priority, but if it makes sense we can discuss adding it.
>>
>> microvm has no support for SMM mode, and that is unlikely to change,
>> so anything requiring SMM mode is not going to work, thats why I dropped
>> SMM + secure boot + TPM bits for the initial patch series.
>>
>> Having support for tpm makes sense even without secure boot, so we might
>> bring that back, but it'll also require some (small) changes on the host
>> side so qemu allows creating a tpm, generates acpi tables for the tpm etc.
>>
>> Does SEV need and/or use SMM mode?  Looking through AmdSevX64.dsc
>> doesn't give a clear answer, on one hand there is a
>> LibraryClasses.common.SMM_CORE section, but on the other hand it uses
>> the non-SMM variable driver stack.
> 
> I think SEV doesn't work with SMM.  James - can you please give a more
> definitive answer here?

SEV works with SMM, but SEV-ES (and likely SEV-SNP) doesn't work with SMM 
because of the fact that the hypervisor wants to change the guest register 
state to enter SMM, which isn't allowed and results in a VMRUN failure.

It might be possible to get SMM to work by having separate VMSAs for the 
SMM state, but it is not anything that really has been investigated too 
deeply.

Thanks,
Tom

> 
> -Dov
> 

Re: [PATCH 1/2] OvmfPkg/OvmfPkgX64: Add SEV launch secret and hashes table areas to MEMFD

[Gerd Hoffmann]

  Hi,

> > > Does SEV need and/or use SMM mode?  Looking through AmdSevX64.dsc
> > > doesn't give a clear answer, on one hand there is a
> > > LibraryClasses.common.SMM_CORE section, but on the other hand it uses
> > > the non-SMM variable driver stack.
> > 
> > I think SEV doesn't work with SMM.  James - can you please give a more
> > definitive answer here?
> 
> SEV works with SMM, but SEV-ES (and likely SEV-SNP) doesn't work with SMM
> because of the fact that the hypervisor wants to change the guest register
> state to enter SMM, which isn't allowed and results in a VMRUN failure.

Ok.  So the same reason why TDX doesn't support SMM either.

> It might be possible to get SMM to work by having separate VMSAs for the SMM
> state, but it is not anything that really has been investigated too deeply.

Should we just drop the SMM leftovers in AmdSevX64.{dsc,fdf} then?

take care,
  Gerd

Re: [PATCH 1/2] OvmfPkg/OvmfPkgX64: Add SEV launch secret and hashes table areas to MEMFD

[Dov Murik]

On 03/11/2021 8:07, Gerd Hoffmann wrote:
>   Hi,
> 
>>>> Does SEV need and/or use SMM mode?  Looking through AmdSevX64.dsc
>>>> doesn't give a clear answer, on one hand there is a
>>>> LibraryClasses.common.SMM_CORE section, but on the other hand it uses
>>>> the non-SMM variable driver stack.
>>>
>>> I think SEV doesn't work with SMM.  James - can you please give a more
>>> definitive answer here?
>>
>> SEV works with SMM, but SEV-ES (and likely SEV-SNP) doesn't work with SMM
>> because of the fact that the hypervisor wants to change the guest register
>> state to enter SMM, which isn't allowed and results in a VMRUN failure.
> 
> Ok.  So the same reason why TDX doesn't support SMM either.
> 
>> It might be possible to get SMM to work by having separate VMSAs for the SMM
>> state, but it is not anything that really has been investigated too deeply.
> 
> Should we just drop the SMM leftovers in AmdSevX64.{dsc,fdf} then?
> 

Yes please. I can test such changes with the AmdSevX86 build.

-Dov

[PATCH 2/2] OvmfPkg/Microvm: Add SEV launch secret and hashes table areas to MEMFD

[Dov Murik]

The SEV launch secret area and the QEMU hashes table area were specified
in the OvmfPkg/AmdSev/AmdSevX64 MEMFD but not in
OvmfPkg/Microvm/MicrovmX64.

Add them in MicrovmX64.fdf.

After this change the two MEMFD descriptions are identical:

    $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/Microvm/MicrovmX64.fdf | sha1sum
    6ff89173952413fbdb7ffbbf42f8bc389c928500  -
    $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/AmdSev/AmdSevX64.fdf | sha1sum
    6ff89173952413fbdb7ffbbf42f8bc389c928500  -

Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
---
 OvmfPkg/Microvm/MicrovmX64.fdf | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Microvm/MicrovmX64.fdf b/OvmfPkg/Microvm/MicrovmX64.fdf
index 6314014f3de7..5545a7ffc4ec 100644
--- a/OvmfPkg/Microvm/MicrovmX64.fdf
+++ b/OvmfPkg/Microvm/MicrovmX64.fdf
@@ -85,7 +85,13 @@ [FD.MEMFD]
 0x00B000|0x001000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize
 
-0x00C000|0x001000
+0x00C000|0x000C00
+gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize
+
+0x00CC00|0x000400
+gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize
+
+0x00D000|0x001000
 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
 
 0x010000|0x010000
-- 
2.25.1

Re: [PATCH 0/2] OvmgPkg: Add SEV launch secret and hashes table areas

[Gerd Hoffmann]

On Tue, Nov 02, 2021 at 07:34:20AM +0000, Dov Murik wrote:
> The SEV launch secret area and the QEMU hashes table area were specified
> in the OvmfPkg/AmdSev/AmdSevX64 MEMFD but not in OvmfPkg/OvmfPkgX64 and
> in OvmgPkg/Microvm/MicrovmX64.
> 
> This series adds theses MEMFD entries to both targets.  It allows QEMU
> to discover the secrets area when performing SEV/SEV-ES secret
> injection, and to properly fill the hashes table (though currently these
> targets do not perform hashes verification when loading
> kernel/initrd/cmdline from QEMU via fw_cfg).
> 
> After applying the patches, the MEMFD section of the three targets' fdf
> files is identical:
> 
>     $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/OvmfPkgX64.fdf | sha1sum
>     6ff89173952413fbdb7ffbbf42f8bc389c928500  -
>     $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/Microvm/MicrovmX64.fdf | sha1sum
>     6ff89173952413fbdb7ffbbf42f8bc389c928500  -
>     $ sed -n -e '/FD.MEMFD/,/FV.SECFV/p' OvmfPkg/AmdSev/AmdSevX64.fdf | sha1sum
>     6ff89173952413fbdb7ffbbf42f8bc389c928500  -
> 
> Code is in:
> https://github.com/confidential-containers-demo/edk2/tree/add-sev-secret-and-hashes

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd