Tom Lendacky and Brijesh Singh reported two issues with launching SEV
guests with the -kernel QEMU option when an old [1] or wrongly configured [2]
OVMF images are used.

To fix these issues, these series "hides" the whole kernel hashes
additions behind a kernel-hashes=on option (with default value of
"off").  This allows existing scenarios to work without change, and
explicitly forces kernel hashes additions for guests that require that.

Patch 1 introduces a new boolean option "kernel-hashes" on the sev-guest
object, and patch 2 causes QEMU to add kernel hashes only if its
explicitly set to "on".  This will mitigate both experienced issues
because the default of the new setting is off, and therefore is backward
compatible with older OVMF images (which don't have a designated hashes
table area) or with guests that don't wish to measure the kernel/initrd.

Patch 3 fixes the wording on the error message displayed when no hashes
table is found in the guest firmware.

Patch 4 detects incorrect address and length of the guest firmware
hashes table area and fails the boot.

Patch 5 is a refactoring of parts of the same function
sev_add_kernel_loader_hashes() to calculate all padding sizes at
compile-time.  Patch 6 also changes the same function and replaces the
call to qemu_map_ram_ptr() with address_space_map() to allow for error
detection.  Patches 5-6 are not required to fix the issues above, but
are suggested as an improvement (no functional change intended).

To enable addition of kernel/initrd/cmdline hashes into the SEV guest at
launch time, specify:

    qemu-system-x86_64 ... -object sev-guest,...,kernel-hashes=on


[1] https://lore.kernel.org/qemu-devel/3b9d10d9-5d9c-da52-f18c-cd93c1931706@amd.com/
[2] https://lore.kernel.org/qemu-devel/001dd81a-282d-c307-a657-e228480d4af3@amd.com/



Changes in v2:
 - Instead of trying to figure out whether to add hashes or not,
   explicity declare an option (kernel-hashes=on) for that.  When that
   option is turned on, fail if the hashes cannot be added.
 - Rephrase error message when no hashes table GUID is found.
 - Replace qemu_map_ram_ptr with address_space_map

v1: https://lore.kernel.org/qemu-devel/20211101102136.1706421-1-dovmurik@linux.ibm.com/


Dov Murik (6):
  qapi/qom,target/i386: sev-guest: Introduce kernel-hashes=on|off option
  target/i386/sev: Add kernel hashes only if sev-guest.kernel-hashes=on
  target/i386/sev: Rephrase error message when no hashes table in guest
    firmware
  target/i386/sev: Fail when invalid hashes table area detected
  target/i386/sev: Perform padding calculations at compile-time
  target/i386/sev: Replace qemu_map_ram_ptr with address_space_map

 qapi/qom.json     |  7 ++++-
 target/i386/sev.c | 77 +++++++++++++++++++++++++++++++++++++++--------
 qemu-options.hx   |  6 +++-
 3 files changed, 75 insertions(+), 15 deletions(-)


base-commit: af531756d25541a1b3b3d9a14e72e7fedd941a2e
-- 
2.25.1

On 11/8/21 7:48 AM, Dov Murik wrote:
> Tom Lendacky and Brijesh Singh reported two issues with launching SEV
> guests with the -kernel QEMU option when an old [1] or wrongly configured [2]
> OVMF images are used.
> 
> To fix these issues, these series "hides" the whole kernel hashes
> additions behind a kernel-hashes=on option (with default value of
> "off").  This allows existing scenarios to work without change, and
> explicitly forces kernel hashes additions for guests that require that.
> 
> Patch 1 introduces a new boolean option "kernel-hashes" on the sev-guest
> object, and patch 2 causes QEMU to add kernel hashes only if its
> explicitly set to "on".  This will mitigate both experienced issues
> because the default of the new setting is off, and therefore is backward
> compatible with older OVMF images (which don't have a designated hashes
> table area) or with guests that don't wish to measure the kernel/initrd.
> 
> Patch 3 fixes the wording on the error message displayed when no hashes
> table is found in the guest firmware.
> 
> Patch 4 detects incorrect address and length of the guest firmware
> hashes table area and fails the boot.
> 
> Patch 5 is a refactoring of parts of the same function
> sev_add_kernel_loader_hashes() to calculate all padding sizes at
> compile-time.  Patch 6 also changes the same function and replaces the
> call to qemu_map_ram_ptr() with address_space_map() to allow for error
> detection.  Patches 5-6 are not required to fix the issues above, but
> are suggested as an improvement (no functional change intended).
> 
> To enable addition of kernel/initrd/cmdline hashes into the SEV guest at
> launch time, specify:
> 
>      qemu-system-x86_64 ... -object sev-guest,...,kernel-hashes=on
> 
> 
> [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fqemu-devel%2F3b9d10d9-5d9c-da52-f18c-cd93c1931706%40amd.com%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7C908b739400a747e1b22308d9a2be7e07%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637719761315906327%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=cMoOlNU2faGwRk6dXVmOE1SuNrg3VvySAC1Ds8fcaFQ%3D&reserved=0
> [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fqemu-devel%2F001dd81a-282d-c307-a657-e228480d4af3%40amd.com%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7C908b739400a747e1b22308d9a2be7e07%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637719761315916323%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=7IZ1%2B%2Fh%2B88xWDlHd%2FMKPN0fJfI6dmSX%2F1TbK8aL8bAs%3D&reserved=0
> 
> 
> 
> Changes in v2:
>   - Instead of trying to figure out whether to add hashes or not,
>     explicity declare an option (kernel-hashes=on) for that.  When that
>     option is turned on, fail if the hashes cannot be added.
>   - Rephrase error message when no hashes table GUID is found.
>   - Replace qemu_map_ram_ptr with address_space_map
> 
> v1: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fqemu-devel%2F20211101102136.1706421-1-dovmurik%40linux.ibm.com%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7C908b739400a747e1b22308d9a2be7e07%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637719761315916323%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=SrE9kYP0Qdhx0WqIXbnwHgeX%2BjBVT9BsK6I0OLU3naI%3D&reserved=0
> 
> 
> Dov Murik (6):
>    qapi/qom,target/i386: sev-guest: Introduce kernel-hashes=on|off option
>    target/i386/sev: Add kernel hashes only if sev-guest.kernel-hashes=on
>    target/i386/sev: Rephrase error message when no hashes table in guest
>      firmware
>    target/i386/sev: Fail when invalid hashes table area detected
>    target/i386/sev: Perform padding calculations at compile-time
>    target/i386/sev: Replace qemu_map_ram_ptr with address_space_map
> 
>   qapi/qom.json     |  7 ++++-
>   target/i386/sev.c | 77 +++++++++++++++++++++++++++++++++++++++--------
>   qemu-options.hx   |  6 +++-
>   3 files changed, 75 insertions(+), 15 deletions(-)
> 
> 

Thanks for the fixing it Dov.

Acked-by: Brijesh Singh <brijesh.singh@amd.com>

thanks

On Mon, Nov 08, 2021 at 01:48:34PM +0000, Dov Murik wrote:
> Tom Lendacky and Brijesh Singh reported two issues with launching SEV
> guests with the -kernel QEMU option when an old [1] or wrongly configured [2]
> OVMF images are used.
> 
> To fix these issues, these series "hides" the whole kernel hashes
> additions behind a kernel-hashes=on option (with default value of
> "off").  This allows existing scenarios to work without change, and
> explicitly forces kernel hashes additions for guests that require that.

We need to to get this into 6.2 to adress the regression vs  previous
QEMU releases.

There's just a couple of small review points. If you can repost
with the easy fixes, then we can get this queued.

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 :|

On 11/11/2021 11:39, Daniel P. Berrangé wrote:
> On Mon, Nov 08, 2021 at 01:48:34PM +0000, Dov Murik wrote:
>> Tom Lendacky and Brijesh Singh reported two issues with launching SEV
>> guests with the -kernel QEMU option when an old [1] or wrongly configured [2]
>> OVMF images are used.
>>
>> To fix these issues, these series "hides" the whole kernel hashes
>> additions behind a kernel-hashes=on option (with default value of
>> "off").  This allows existing scenarios to work without change, and
>> explicitly forces kernel hashes additions for guests that require that.
> 
> We need to to get this into 6.2 to adress the regression vs  previous
> QEMU releases.
> 
> There's just a couple of small review points. If you can repost
> with the easy fixes, then we can get this queued.
> 

Sent v3 now.

Patch 3/6 (error message rephrase) still misses Reviewed-by.

Thanks,
-Dov