1. Patchew
  2. QEMU
  3. [PATCH 0/3] SEV: fixes for -kernel launch with incompatible OVMF
  4. View patch

[PATCH 1/3] sev/i386: Allow launching with -kernel if no OVMF hashes table found

Dov Murik posted 3 patches 4 years, 3 months ago
Maintainers: Marcelo Tosatti <mtosatti@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH 1/3] sev/i386: Allow launching with -kernel if no OVMF hashes table found
Posted by Dov Murik 4 years, 3 months ago 
Commit cff03145ed3c ("sev/i386: Introduce sev_add_kernel_loader_hashes
for measured linux boot", 2021-09-30) introduced measured direct boot
with -kernel, using an OVMF-designated hashes table which QEMU fills.

However, if OVMF doesn't designate such an area, QEMU would completely
abort the VM launch.  This breaks launching with -kernel using older
OVMF images which don't publish the SEV_HASH_TABLE_RV_GUID.

Instead, just warn the user that -kernel was supplied by OVMF doesn't
specify the GUID for the hashes table.  The following warning will be
displayed during VM launch:

    qemu-system-x86_64: warning: SEV: kernel specified but OVMF has no hash table guid

Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 target/i386/sev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index eede07f11d..682b8ccf6c 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1204,7 +1204,7 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
     int aligned_len;
 
     if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
-        error_setg(errp, "SEV: kernel specified but OVMF has no hash table guid");
+        warn_report("SEV: kernel specified but OVMF has no hash table guid");
         return false;
     }
     area = (SevHashTableDescriptor *)data;
-- 
2.25.1
Re: [PATCH 1/3] sev/i386: Allow launching with -kernel if no OVMF hashes table found
Posted by Tom Lendacky 4 years, 3 months ago 
On 11/1/21 5:21 AM, Dov Murik wrote:
> Commit cff03145ed3c ("sev/i386: Introduce sev_add_kernel_loader_hashes
> for measured linux boot", 2021-09-30) introduced measured direct boot
> with -kernel, using an OVMF-designated hashes table which QEMU fills.
> 
> However, if OVMF doesn't designate such an area, QEMU would completely
> abort the VM launch.  This breaks launching with -kernel using older
> OVMF images which don't publish the SEV_HASH_TABLE_RV_GUID.
> 
> Instead, just warn the user that -kernel was supplied by OVMF doesn't
> specify the GUID for the hashes table.  The following warning will be
> displayed during VM launch:
> 
>      qemu-system-x86_64: warning: SEV: kernel specified but OVMF has no hash table guid
> 
> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
> Reported-by: Tom Lendacky <thomas.lendacky@amd.com>

Just a few minor comments/questions below, otherwise:

Acked-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   target/i386/sev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index eede07f11d..682b8ccf6c 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -1204,7 +1204,7 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
>       int aligned_len;
>   
>       if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
> -        error_setg(errp, "SEV: kernel specified but OVMF has no hash table guid");
> +        warn_report("SEV: kernel specified but OVMF has no hash table guid");

Not sure if warn or info would be better, either way works for me, though, 
since this allows the guest to boot now.

Unrelated to this change, but do we explicitly want to say OVMF? Can't 
another BIOS/UEFI implementation have this support?

and should guid be GUID?

Thanks,
Tom

>           return false;
>       }
>       area = (SevHashTableDescriptor *)data;
>
Re: [PATCH 1/3] sev/i386: Allow launching with -kernel if no OVMF hashes table found
Posted by Dov Murik 4 years, 3 months ago 

On 01/11/2021 16:25, Tom Lendacky wrote:
> On 11/1/21 5:21 AM, Dov Murik wrote:
>> Commit cff03145ed3c ("sev/i386: Introduce sev_add_kernel_loader_hashes
>> for measured linux boot", 2021-09-30) introduced measured direct boot
>> with -kernel, using an OVMF-designated hashes table which QEMU fills.
>>
>> However, if OVMF doesn't designate such an area, QEMU would completely
>> abort the VM launch.  This breaks launching with -kernel using older
>> OVMF images which don't publish the SEV_HASH_TABLE_RV_GUID.
>>
>> Instead, just warn the user that -kernel was supplied by OVMF doesn't
>> specify the GUID for the hashes table.  The following warning will be
>> displayed during VM launch:
>>
>>      qemu-system-x86_64: warning: SEV: kernel specified but OVMF has
>> no hash table guid
>>
>> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
>> Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
> 
> Just a few minor comments/questions below, otherwise:
> 
> Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
> 
>> ---
>>   target/i386/sev.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index eede07f11d..682b8ccf6c 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -1204,7 +1204,7 @@ bool
>> sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
>>       int aligned_len;
>>         if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data,
>> NULL)) {
>> -        error_setg(errp, "SEV: kernel specified but OVMF has no hash
>> table guid");
>> +        warn_report("SEV: kernel specified but OVMF has no hash table
>> guid");
> 
> Not sure if warn or info would be better, either way works for me,
> though, since this allows the guest to boot now.

OK.

> 
> Unrelated to this change, but do we explicitly want to say OVMF? Can't
> another BIOS/UEFI implementation have this support?
> 

I don't mind saying "guest firmware" or "VM firmware" or "guest flash"
instead.  Note that the function name has "ovmf" in it...


> and should guid be GUID?

I agree; if we change "OVMF" I'll fix that as well.

-Dov


> 
> Thanks,
> Tom
> 
>>           return false;
>>       }
>>       area = (SevHashTableDescriptor *)data;
>>
Re: [PATCH 1/3] sev/i386: Allow launching with -kernel if no OVMF hashes table found
Posted by Daniel P. Berrangé 4 years, 3 months ago 
On Mon, Nov 01, 2021 at 10:21:34AM +0000, Dov Murik wrote:
> Commit cff03145ed3c ("sev/i386: Introduce sev_add_kernel_loader_hashes
> for measured linux boot", 2021-09-30) introduced measured direct boot
> with -kernel, using an OVMF-designated hashes table which QEMU fills.
> 
> However, if OVMF doesn't designate such an area, QEMU would completely
> abort the VM launch.  This breaks launching with -kernel using older
> OVMF images which don't publish the SEV_HASH_TABLE_RV_GUID.
> 
> Instead, just warn the user that -kernel was supplied by OVMF doesn't
> specify the GUID for the hashes table.  The following warning will be
> displayed during VM launch:
> 
>     qemu-system-x86_64: warning: SEV: kernel specified but OVMF has no hash table guid
> 
> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
> Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  target/i386/sev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index eede07f11d..682b8ccf6c 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -1204,7 +1204,7 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
>      int aligned_len;
>  
>      if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
> -        error_setg(errp, "SEV: kernel specified but OVMF has no hash table guid");
> +        warn_report("SEV: kernel specified but OVMF has no hash table guid");
>          return false;

I'm pretty wary of doing this kind of thing.

If someone is using QEMU and they required to have the hashes populated
for their use case, they now don't get a fatal error if something goes
wrong with the process. This is bad as it hides a serious mistake.

If someone is using QEMU and they don't require to have the hashes
populated and they knowingly use a firmware that doesn't support
this, they'll now get a irrelevant warning every time they boot
QEMU. This is bad because IME users will file bugs complaining
about this bogus warning.


If we genuinely need to support both uses cases, then we should have
an explicit command line flag to request the desired behaviour.

This could be a -machine option to indicate that the hashes must
be populated.

 - unset: try to populate hashes, *silently* ignore missing table
          in ovmf
 - set == on: try to populate hashes, report error on missing
             table in ovmf
  -set == off: never try to populate hashes, nor look for the
               table in ovmf

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.