[+cc Tom, Brijesh, Ashish - see SEV-related changes in this series]
On 13/01/2022 18:55, Daniel P. Berrangé wrote: > The AMD SEV build of EDK2 only emits a single file, intended to be > > mapped readonly. There is explicitly no separate writable VARS > > store for persisting non-volatile firmware variables. > > > > This can be used with QEMU's traditional pflash configuration > > mechanism by only populating pflash0, leaving pflash1 unconfigured. > > Conceptually, however, it is odd to be using pflash at all when we > > have no intention of supporting any writable variables. The -bios > > option should be sufficient for any firmware that is exclusively > > readonly code. > > > > > > A second issue is that the firmware descriptor schema does not allow > > for describing a firmware that uses pflash, without any associated > > non-volatile storage. > > > > In docs/interop/firmware.json > > > > 'struct' : 'FirmwareMappingFlash', > > 'data' : { 'executable' : 'FirmwareFlashFile', > > 'nvram-template' : 'FirmwareFlashFile' } } > > > > Notice that nvram-template is mandatory, and when consuming these > > files libvirt will thus complain if the nvram-template field is > > missing. > > > > We could in theory make nvram-template optional in the schema and > > then update libvirt to take account of it, but this feels dubious > > when we have a perfectly good way of describing a firmware without > > NVRAM, using 'FirmwareMappingMemory' which is intended to be used > > with QEMU's -bios option. > > > > > > A third issue is in libvirt, where again the code handling the > > configuration of pflash supports two scenarios > > > > - A single pflash image, which is writable > > - A pair of pflash images, one writable one readonly > > > > There is no support for a single read-only pflash image in libvirt > > today. > > > > > > This all points towards the fact that we should be using -bios > > to load the AMD SEV firmware build of EDK. > > > > The only thing preventing us doing that is that QEMU does not > > initialize the SEV firmware when using -bios. That is fairly > > easily solved, as done in this patch series. > > > > For testing I've launched QEMU in in these scenarios > > > > - SEV guest using -bios and boot from HD > > - SEV guest using pflash and boot from HD > > - SEV-ES guest using -bios and direct kernel boot > > - SEV-ES guest using pflash and direct kernel boot > > > > In all these cases I was able to validate the reported SEV > > guest measurement. > > I'm having trouble testing this series (applied on top of master commit 69353c332c): it hangs with -bios but works OK with pflash: Here's with -bios: $ sudo /home/dmurik/git/qemu/build/qemu-system-x86_64 -enable-kvm \ -cpu host -machine q35 -smp 4 -m 2G \ -machine confidential-guest-support=sev0 \ -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x0 \ -bios /home/dmurik/git/edk2/Build/AmdSev/DEBUG_GCC5/FV/OVMF.fd \ -nographic \ -global isa-debugcon.iobase=0x402 -debugcon file:ovmf-1.log \ -monitor pty -trace 'enable=kvm_sev_*' char device redirected to /dev/pts/14 (label compat_monitor0) kvm_sev_init kvm_sev_launch_start policy 0x0 session (nil) pdh (nil) kvm_sev_change_state uninit -> launch-update kvm_sev_launch_update_data addr 0x7f42e9bff010 len 0x400000 kvm_sev_change_state launch-update -> launch-secret kvm_sev_launch_measurement data PF6n7+Vujx5sW8PC6iMRtHXfpXdJ4osbcfYvoknu7gg4ypMqs727NTzG86Ft8Llu kvm_sev_launch_finish kvm_sev_change_state launch-secret -> running Here it hangs. The ovmf-1.log file is empty. Notice that kvm_sev_launch_update_data is called, so the new -bios behaviour is triggered correctly. But the guest doesn't start running. Here is the guest's state: (qemu) info registers EAX=0000606b EBX=00001268 ECX=0000440c EDX=008328d2 ESI=000091e2 EDI=0000e9e3 EBP=0000a451 ESP=00009af0 EIP=00003612 EFL=00000082 [--S----] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0000 00000000 0000ffff 00009300 CS =a76e 000a76e0 0000ffff 00009b00 SS =0000 00000000 0000ffff 00009300 DS =0000 00000000 0000ffff 00009300 FS =0000 00000000 0000ffff 00009300 GS =0000 00000000 0000ffff 00009300 LDT=0000 00000000 0000ffff 00008200 TR =0000 00000000 0000ffff 00008b00 GDT= 00000000 0000ffff IDT= 00000000 0000ffff CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000 DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000 DR6=00000000ffff0ff0 DR7=0000000000000400 EFER=0000000000000000 FCW=037f FSW=0000 [ST=0] FTW=00 MXCSR=00001f80 ... (qemu) info sev handle: 1 state: running build: 10 api version: 0.23 debug: on key-sharing: on If I try the same with pflash (instead of -bios), I get: # sudo /home/dmurik/git/qemu/build/qemu-system-x86_64 -enable-kvm \ -cpu host -machine q35 -smp 4 -m 2G \ -machine confidential-guest-support=sev0 \ -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x0 \ -drive if=pflash,format=raw,unit=0,file=/home/dmurik/git/edk2/Build/AmdSev/DEBUG_GCC5/FV/OVMF.fd,readonly=on \ -nographic \ -global isa-debugcon.iobase=0x402 -debugcon file:ovmf-1.log \ -monitor pty -trace 'enable=kvm_sev_*' char device redirected to /dev/pts/14 (label compat_monitor0) kvm_sev_init kvm_sev_launch_start policy 0x0 session (nil) pdh (nil) kvm_sev_change_state uninit -> launch-update kvm_sev_launch_update_data addr 0x7f0343000000 len 0x400000 kvm_sev_change_state launch-update -> launch-secret kvm_sev_launch_measurement data esqzlr4xX2eEY92xsKEKL7FyKRDW7VYiyIb+aXS4S/ctON2s1xxwFjAKU7ImfULJ kvm_sev_launch_finish kvm_sev_change_state launch-secret -> running BdsDxe: failed to load Boot0003 "Grub Bootloader" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(B5AE312C-BC8A-43B1-9C62-EBB826DD5D07): Not Found The "failed to load Grub" is what I expected. So this works OK. The ovmf-1.log file shows normal sequence of AmdSev boot. I tried the two options with the standard OvmfX64 package as well - same behaviour. Do I need to build the OVMF file differently to use with -bios ? -Dov > > Daniel P. Berrangé (2): > > hw/i386: refactor logic for setting up SEV firmware > > hw/i386: support loading OVMF using -bios too > > > > hw/i386/pc_sysfw.c | 24 +++--------------------- > > hw/i386/pc_sysfw_ovmf-stubs.c | 10 ++++++++++ > > hw/i386/pc_sysfw_ovmf.c | 27 +++++++++++++++++++++++++++ > > hw/i386/x86.c | 5 +++++ > > include/hw/i386/pc.h | 1 + > > 5 files changed, 46 insertions(+), 21 deletions(-) > > >