Re: [libvirt PATCH 9/9] qemu: implement support for firmware auto-selection feature filtering

On 3/18/21 1:26 PM, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > src/qemu/qemu_firmware.c | 40 +++++++++++++++ > ...re-efi-no-enrolled-keys.x86_64-latest.args | 49 ++++++++++++++++++ > .../os-firmware-efi-no-enrolled-keys.xml | 25 ++++++++++ > tests/qemuxml2argvtest.c | 1 + > ...are-efi-no-enrolled-keys.x86_64-latest.xml | 50 +++++++++++++++++++ > tests/qemuxml2xmltest.c | 1 + > 6 files changed, 166 insertions(+) > create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.x86_64-latest.args > create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml > create mode 100644 tests/qemuxml2xmloutdata/os-firmware-efi-no-enrolled-keys.x86_64-latest.xml > > diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c > index d3198e2d45..f6f371f51f 100644 > --- a/src/qemu/qemu_firmware.c > +++ b/src/qemu/qemu_firmware.c > @@ -930,6 +930,10 @@ qemuFirmwareMatchDomain(const virDomainDef *def, > bool supportsS4 = false; > bool requiresSMM = false; > bool supportsSEV = false; > + bool supportsSecureBoot = false; > + bool hasEnrolledKeys = false; > + int reqSecureBoot; > + int reqEnrolledKeys; > want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.firmware); > @@ -979,7 +983,13 @@ qemuFirmwareMatchDomain(const virDomainDef *def, > break; > case QEMU_FIRMWARE_FEATURE_SECURE_BOOT: > + supportsSecureBoot = true; > + break; > + > case QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS: > + hasEnrolledKeys = true; > + break; > + > case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: > case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC: > case QEMU_FIRMWARE_FEATURE_NONE: > @@ -1000,6 +1010,36 @@ qemuFirmwareMatchDomain(const virDomainDef *def, > return false; > } > + if (def->os.firmwareFeatures) { > + reqSecureBoot = def->os.firmwareFeatures[VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_SECURE_BOOT]; > + if (reqSecureBoot != VIR_TRISTATE_BOOL_ABSENT) { > + if (reqSecureBoot == VIR_TRISTATE_BOOL_YES && !supportsSecureBoot) { > + VIR_DEBUG("User requested Secure Boot, firmware '%s' doesn't support it", > + path); > + return false; > + } > + > + if (reqSecureBoot == VIR_TRISTATE_BOOL_NO && supportsSecureBoot) { > + VIR_DEBUG("User refused Secure Boot, firmware '%s' supports it", path); > + return false; > + } > + } > + > + reqEnrolledKeys = def->os.firmwareFeatures[VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_ENROLLED_KEYS]; > + if (reqEnrolledKeys != VIR_TRISTATE_BOOL_ABSENT) { > + if (reqEnrolledKeys == VIR_TRISTATE_BOOL_YES && !hasEnrolledKeys) { > + VIR_DEBUG("User requested Enrolled keys, firmware '%s' doesn't support it", "doesn't have them" perhaps? > + path); > + return false; > + } > + > + if (reqEnrolledKeys == VIR_TRISTATE_BOOL_NO && hasEnrolledKeys) { > + VIR_DEBUG("User refused Enrolled keys, firmware '%s' supports it", path); "has them" perhaps?

> + return false; > + } > + } > + } > + > if (def->os.loader && > def->os.loader->secure == VIR_TRISTATE_BOOL_YES && > !requiresSMM) { > diff --git a/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.x86_64-latest.args b/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.x86_64-latest.args > new file mode 100644 > index 0000000000..561a905e78 > --- /dev/null > +++ b/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.x86_64-latest.args > @@ -0,0 +1,49 @@ > +LC_ALL=C \ > +PATH=/bin \ > +HOME=/tmp/lib/domain--1-fedora \ > +USER=test \ > +LOGNAME=test \ > +XDG_DATA_HOME=/tmp/lib/domain--1-fedora/.local/share \ > +XDG_CACHE_HOME=/tmp/lib/domain--1-fedora/.cache \ > +XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \ > +/usr/bin/qemu-system-x86_64 \ > +-name guest=fedora,debug-threads=on \ > +-S \ > +-object secret,id=masterKey0,format=raw,\ > +file=/tmp/lib/domain--1-fedora/master-key.aes \ > +-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd",\ > +"node-name":"libvirt-pflash0-storage","auto-read-only":true,\ > +"discard":"unmap"}' \ > +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,\ > +"driver":"raw","file":"libvirt-pflash0-storage"}' \ > +-blockdev '{"driver":"file",\ > +"filename":"/var/lib/libvirt/qemu/nvram/fedora_VARS.fd",\ > +"node-name":"libvirt-pflash1-storage","auto-read-only":true,\ > +"discard":"unmap"}' \ > +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,\ > +"driver":"raw","file":"libvirt-pflash1-storage"}' \ > +-machine pc-q35-4.0,accel=kvm,usb=off,dump-guest-core=off,\ > +pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,\ > +memory-backend=pc.ram \ > +-cpu qemu64 \ > +-m 8 \ > +-object memory-backend-ram,id=pc.ram,size=8388608 \ > +-overcommit mem-lock=off \ > +-smp 1,sockets=1,cores=1,threads=1 \ > +-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \ > +-display none \ > +-no-user-config \ > +-nodefaults \ > +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ > +-mon chardev=charmonitor,id=monitor,mode=control \ > +-rtc base=utc \ > +-no-shutdown \ > +-boot strict=on \ > +-device pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\ > +addr=0x1 \ > +-device pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \ > +-device qemu-xhci,id=usb,bus=pci.1,addr=0x0 \ > +-audiodev id=audio1,driver=none \ > +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ > +resourcecontrol=deny \ > +-msg timestamp=on > diff --git a/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml b/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml > new file mode 100644 > index 0000000000..6c0b323fd4 > --- /dev/null > +++ b/tests/qemuxml2argvdata/os-firmware-efi-no-enrolled-keys.xml > @@ -0,0 +1,25 @@ > +<domain type='kvm'> > + <name>fedora</name> > + <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid> > + <memory unit='KiB'>8192</memory> > + <currentMemory unit='KiB'>8192</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os firmware='efi'> > + <firmware type='efi'> > + <feature enabled='no' name='enrolled-keys'/> > + </firmware> > + <type arch='x86_64' machine='pc-q35-4.0'>hvm</type> > + </os> > + <features> > + <acpi/> > + <apic/> > + <pae/> > + </features> > + <devices> > + <emulator>/usr/bin/qemu-system-x86_64</emulator> > + <controller type='pci' index='0' model='pcie-root'/> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <memballoon model='none'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index 2b32b7f303..44c2a316b0 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -3549,6 +3549,7 @@ mymain(void) > DO_TEST_CAPS_LATEST("os-firmware-bios"); > DO_TEST_CAPS_LATEST("os-firmware-efi"); > DO_TEST_CAPS_LATEST("os-firmware-efi-secboot"); > + DO_TEST_CAPS_LATEST("os-firmware-efi-no-enrolled-keys"); > DO_TEST_CAPS_LATEST_PARSE_ERROR("os-firmware-invalid-type"); > DO_TEST_CAPS_ARCH_LATEST("aarch64-os-firmware-efi", "aarch64"); > diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi-no-enrolled-keys.x86_64-latest.xml b/tests/qemuxml2xmloutdata/os-firmware-efi-no-enrolled-keys.x86_64-latest.xml Alternatively, let this be link to the XML above, since the difference between them is not in the area of interest of this feature.