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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux