On Thu, Mar 18, 2021 at 05:18:38PM +0100, Michal Privoznik wrote:
> On 3/18/21 1:26 PM, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> > ---
> > 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?
Sounds better, I wanted to change it after copy&paste of the secureBoot
part, but as we can see it did not happen. :)
> > + 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.
Will do. I usually try to create the input XML as minimal as possible so
it can be used as an example of the feature but I don't have a strong
preference.
Thanks,
Pavel
Attachment:
signature.asc
Description: PGP signature
