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 <dovmu...@linux.ibm.com>
>> Reported-by: Tom Lendacky <thomas.lenda...@amd.com>
>
> Just a few minor comments/questions below, otherwise:
>
> Acked-by: Tom Lendacky <thomas.lenda...@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;
>>