Blob Blame History Raw
From 6f02748897062d40b411177ef752644505189a72 Mon Sep 17 00:00:00 2001
Message-Id: <6f02748897062d40b411177ef752644505189a72@dist-git>
From: Pavel Hrdina <phrdina@redhat.com>
Date: Fri, 21 May 2021 14:16:11 +0200
Subject: [PATCH] conf: introduce support for firmware auto-selection feature
 filtering

When the firmware auto-selection was introduced it always picked first
usable firmware based on the JSON descriptions on the host. It is
possible to add/remove/change the JSON files but it will always be for
the whole host.

This patch introduces support for configuring the auto-selection per VM
by adding users an option to limit what features they would like to have
available in the firmware.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit cff524af6c5e1ddc11149394ed7f985242ebea0f)

Conflicts:
    docs/formatdomain.rst
        - we still have formatdomain.html.in in downstream
    src/conf/domain_conf.c
        - missing following upstream commits:
            0280fc72708b9d0f162a808bcc8d78137a68d58d
            104dadcff6023da676df3905d1ed8688aea15e86
            2d5f7a49ae0780143566932ab38215433982c89f

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1929357

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Message-Id: <631e05bc5363abb3e48d8b652a806324801cce16.1621599207.git.phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
 docs/formatdomain.html.in                     | 58 +++++++++++++
 docs/schemas/domaincommon.rng                 | 23 +++++
 src/conf/domain_conf.c                        | 83 ++++++++++++++++++-
 src/conf/domain_conf.h                        | 10 +++
 ...os-firmware-invalid-type.x86_64-latest.err |  1 +
 .../os-firmware-invalid-type.xml              | 28 +++++++
 tests/qemuxml2argvtest.c                      |  1 +
 ...aarch64-os-firmware-efi.aarch64-latest.xml |  1 +
 .../os-firmware-bios.x86_64-latest.xml        |  1 +
 .../os-firmware-efi-secboot.x86_64-latest.xml |  1 +
 .../os-firmware-efi.x86_64-latest.xml         |  1 +
 tests/vmx2xmldata/vmx2xml-firmware-efi.xml    |  1 +
 12 files changed, 206 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err
 create mode 100644 tests/qemuxml2argvdata/os-firmware-invalid-type.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a40bed347b..11f31618af 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -180,6 +180,64 @@
         <code>ESX</code> and <code>VMWare</code> hypervisor drivers, however,
         the <code>i686</code> arch will always be chosen even on an
         <code>x86_64</code> host. <span class="since">Since 0.0.1</span></dd>
+      <dt><a id="elementFirmware"><code>firmware</code></a></dt>
+      <dd>
+        <p><span class="since">Since 7.2.0 QEMU/KVM only</span></p>
+        <p>
+          When used together with <code>firmware</code> attribute of
+          <code>os</code> element the <code>type</code> attribute must
+          have the same value.
+        </p>
+        <p>
+          List of mandatory attributes:
+          <ul>
+            <li>
+              <code>type</code> (accepted values are <code>bios</code>
+              and <code>efi</code>) same as the <code>firmware</code>
+              attribute of <code>os</code> element.
+            </li>
+          </ul>
+        </p>
+        <p>
+          When using firmware auto-selection there are different features
+          enabled in the firmwares. The list of features can be used to
+          limit what firmware should be automatically selected for the VM.
+          The list of features can be specified using zero or more
+          <code>feature</code> elements. Libvirt will take into consideration
+          only the listed features and ignore the rest when selecting the firmware.
+
+          <dl>
+            <dt><code>feature</code></dt>
+            <dd>
+              The list of mandatory attributes:
+
+              <ul>
+                <li>
+                  <code>enabled</code> (accepted values are <code>yes</code>
+                  and <code>no</code>) is used to tell libvirt if the feature
+                  must be enabled or not in the automatically selected firmware
+                </li>
+                <li>
+                  <code>name</code> the name of the feature, the list of the features:
+                  <ul>
+                    <li>
+                      <code>enrolled-keys</code> whether the selected nvram template
+                      has default certificate enrolled. Firmware with Secure Boot
+                      feature but without enrolled keys will successfully boot
+                      non-signed binaries as well. Valid only for firmwares with
+                      Secure Boot feature.
+                    </li>
+                    <li>
+                      <code>secure-boot</code> whether the firmware implements
+                      UEFI Secure boot feature.
+                    </li>
+                  </ul>
+                </li>
+              </ul>
+            </dd>
+          </dl>
+        </p>
+      </dd>
       <dt><a id="elementLoader"><code>loader</code></a></dt>
       <dd>The optional <code>loader</code> tag refers to a firmware blob,
         which is specified by absolute path,
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6671ef3dfa..b7f6a6b494 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -268,6 +268,29 @@
           </attribute>
         </optional>
         <ref name="ostypehvm"/>
+        <optional>
+          <element name="firmware">
+            <attribute name="type">
+              <choice>
+                <value>bios</value>
+                <value>efi</value>
+              </choice>
+            </attribute>
+            <zeroOrMore>
+              <element name="feature">
+                <attribute name="enabled">
+                  <ref name="virYesNo"/>
+                </attribute>
+                <attribute name="name">
+                  <choice>
+                    <value>enrolled-keys</value>
+                    <value>secure-boot</value>
+                  </choice>
+                </attribute>
+              </element>
+            </zeroOrMore>
+          </element>
+        </optional>
         <optional>
           <element name="loader">
             <optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 93a78f8277..28c8d0ecbd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1240,6 +1240,12 @@ VIR_ENUM_IMPL(virDomainOsDefFirmware,
               "efi",
 );
 
+VIR_ENUM_IMPL(virDomainOsDefFirmwareFeature,
+              VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST,
+              "enrolled-keys",
+              "secure-boot",
+);
+
 /* Internal mapping: subset of block job types that can be present in
  * <mirror> XML (remaining types are not two-phase). */
 VIR_ENUM_DECL(virDomainBlockJob);
@@ -19382,22 +19388,67 @@ virDomainDefParseBootFirmwareOptions(virDomainDefPtr def,
                                      xmlXPathContextPtr ctxt)
 {
     g_autofree char *firmware = virXPathString("string(./os/@firmware)", ctxt);
+    g_autofree char *type = virXPathString("string(./os/firmware/@type)", ctxt);
+    g_autofree xmlNodePtr *nodes = NULL;
+    g_autofree int *features = NULL;
     int fw = 0;
+    int n = 0;
+    size_t i;
 
-    if (!firmware)
+    if (!firmware && !type)
         return 0;
 
-    fw = virDomainOsDefFirmwareTypeFromString(firmware);
+    if (firmware && type && STRNEQ(firmware, type)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("firmware attribute and firmware type has to be the same"));
+        return -1;
+    }
+
+    if (!type)
+        type = g_steal_pointer(&firmware);
+
+    fw = virDomainOsDefFirmwareTypeFromString(type);
 
     if (fw <= 0) {
         virReportError(VIR_ERR_XML_ERROR,
                        _("unknown firmware value %s"),
-                       firmware);
+                       type);
         return -1;
     }
 
     def->os.firmware = fw;
 
+    if ((n = virXPathNodeSet("./os/firmware/feature", ctxt, &nodes)) < 0)
+        return -1;
+
+    if (n > 0)
+        features = g_new0(int, VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST);
+
+    for (i = 0; i < n; i++) {
+        g_autofree char *name = virXMLPropString(nodes[i], "name");
+        g_autofree char *enabled = virXMLPropString(nodes[i], "enabled");
+        int feature = virDomainOsDefFirmwareFeatureTypeFromString(name);
+        int val = virTristateBoolTypeFromString(enabled);
+
+        if (feature < 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("invalid firmware feature name '%s'"),
+                           name);
+            return -1;
+        }
+
+        if (val < 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("invalid firmware feature enabled value '%s'"),
+                           enabled);
+            return -1;
+        }
+
+        features[feature] = val;
+    }
+
+    def->os.firmwareFeatures = g_steal_pointer(&features);
+
     return 0;
 }
 
@@ -28987,6 +29038,32 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def,
         virBufferAsprintf(buf, ">%s</type>\n",
                           virDomainOSTypeToString(def->os.type));
 
+    if (def->os.firmware) {
+        virBufferAsprintf(buf, "<firmware type='%s'",
+                          virDomainOsDefFirmwareTypeToString(def->os.firmware));
+
+        if (def->os.firmwareFeatures) {
+            virBufferAddLit(buf, ">\n");
+
+            virBufferAdjustIndent(buf, 2);
+
+            for (i = 0; i < VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST; i++) {
+                if (def->os.firmwareFeatures[i] == VIR_TRISTATE_BOOL_ABSENT)
+                    continue;
+
+                virBufferAsprintf(buf, "<feature enabled='%s' name='%s'/>\n",
+                                  virTristateBoolTypeToString(def->os.firmwareFeatures[i]),
+                                  virDomainOsDefFirmwareFeatureTypeToString(i));
+            }
+
+            virBufferAdjustIndent(buf, -2);
+
+            virBufferAddLit(buf, "</firmware>\n");
+        } else {
+            virBufferAddLit(buf, "/>\n");
+        }
+    }
+
     virBufferEscapeString(buf, "<init>%s</init>\n",
                           def->os.init);
     for (i = 0; def->os.initargv && def->os.initargv[i]; i++)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3aed1fb22a..1ad77ecac6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1967,9 +1967,19 @@ G_STATIC_ASSERT((int)VIR_DOMAIN_OS_DEF_FIRMWARE_LAST == (int)VIR_DOMAIN_LOADER_T
 
 VIR_ENUM_DECL(virDomainOsDefFirmware);
 
+typedef enum {
+    VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_ENROLLED_KEYS,
+    VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_SECURE_BOOT,
+
+    VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_LAST
+} virDomainOsDefFirmwareFeature;
+
+VIR_ENUM_DECL(virDomainOsDefFirmwareFeature);
+
 struct _virDomainOSDef {
     int type;
     virDomainOsDefFirmware firmware;
+    int *firmwareFeatures;
     virArch arch;
     char *machine;
     size_t nBootDevs;
diff --git a/tests/qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err b/tests/qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err
new file mode 100644
index 0000000000..c8174b1c8b
--- /dev/null
+++ b/tests/qemuxml2argvdata/os-firmware-invalid-type.x86_64-latest.err
@@ -0,0 +1 @@
+unsupported configuration: firmware attribute and firmware type has to be the same
diff --git a/tests/qemuxml2argvdata/os-firmware-invalid-type.xml b/tests/qemuxml2argvdata/os-firmware-invalid-type.xml
new file mode 100644
index 0000000000..41360df0f7
--- /dev/null
+++ b/tests/qemuxml2argvdata/os-firmware-invalid-type.xml
@@ -0,0 +1,28 @@
+<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'>
+    <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <firmware type='bios'/>
+    <loader secure='no'/>
+    <nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a22e3ba157..bc04bea692 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3094,6 +3094,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_PARSE_ERROR("os-firmware-invalid-type");
     DO_TEST_CAPS_ARCH_LATEST("aarch64-os-firmware-efi", "aarch64");
 
     DO_TEST_CAPS_LATEST("vhost-user-vga");
diff --git a/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml b/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml
index 1e51d55305..3cac8fc5c6 100644
--- a/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml
+++ b/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.aarch64-latest.xml
@@ -6,6 +6,7 @@
   <vcpu placement='static'>1</vcpu>
   <os firmware='efi'>
     <type arch='aarch64' machine='virt-4.0'>hvm</type>
+    <firmware type='efi'/>
     <kernel>/aarch64.kernel</kernel>
     <initrd>/aarch64.initrd</initrd>
     <cmdline>earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait</cmdline>
diff --git a/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml b/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml
index 60d3498765..ef24f2fece 100644
--- a/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/os-firmware-bios.x86_64-latest.xml
@@ -6,6 +6,7 @@
   <vcpu placement='static'>1</vcpu>
   <os firmware='bios'>
     <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <firmware type='bios'/>
     <loader secure='no'/>
     <nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
     <boot dev='hd'/>
diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.x86_64-latest.xml b/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.x86_64-latest.xml
index 938da73711..3757191e8e 100644
--- a/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.x86_64-latest.xml
@@ -6,6 +6,7 @@
   <vcpu placement='static'>1</vcpu>
   <os firmware='efi'>
     <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <firmware type='efi'/>
     <loader secure='yes'/>
     <nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
     <boot dev='hd'/>
diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml b/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml
index 97ce8a75c7..f2e6b7f36d 100644
--- a/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/os-firmware-efi.x86_64-latest.xml
@@ -6,6 +6,7 @@
   <vcpu placement='static'>1</vcpu>
   <os firmware='efi'>
     <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <firmware type='efi'/>
     <loader secure='no'/>
     <nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
     <boot dev='hd'/>
diff --git a/tests/vmx2xmldata/vmx2xml-firmware-efi.xml b/tests/vmx2xmldata/vmx2xml-firmware-efi.xml
index e21158cebf..375c47d281 100644
--- a/tests/vmx2xmldata/vmx2xml-firmware-efi.xml
+++ b/tests/vmx2xmldata/vmx2xml-firmware-efi.xml
@@ -5,6 +5,7 @@
   <vcpu placement='static'>1</vcpu>
   <os firmware='efi'>
     <type arch='i686'>hvm</type>
+    <firmware type='efi'/>
   </os>
   <clock offset='utc'/>
   <on_poweroff>destroy</on_poweroff>
-- 
2.31.1