qemu: add ability to set PCI device "rombar" on or off

This patch was made in response to:

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

In short, qemu's default for the rombar setting (which makes the
firmware ROM of a PCI device visible/not on the guest) was previously
0 (not visible), but they recently changed the default to 1
(visible). Unfortunately, there are some PCI devices that fail in the
guest when rombar is 1, so the setting must be exposed in libvirt to
prevent a regression in behavior (it will still require explicitly
setting <rom bar='off'/> in the guest XML).

rombar is forced on/off by adding:

  <rom bar='on|off'/>

inside a <hostdev> element that defines a PCI device. It is currently
ignored for all other types of devices.

At the moment there is no clean method to determine whether or not the
rombar option is supported by QEMU - this patch uses the advice of a
QEMU developer to assume support for qemu-0.12+. There is currently a
patch in the works to put this information in the output of "qemu-kvm
-device pci-assign,?", but of course if we switch to keying off that,
we would lose support for setting rombar on all the versions of qemu
between 0.12 and whatever version gets that patch.

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3087d016e3a9c7ad000f04869a4cdc4002c0ce30..9c3c2e83138b3efc509d58e39943e88b88449b03 100644 (file)
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1376,6 +1376,7 @@
         <address bus='0x06' slot='0x02' function='0x0'/>
       </source>
       <boot order='1'/>
+      <rom bar='off'/>
     </hostdev>
   </devices>
   ...</pre>
@@ -1407,6 +1408,18 @@
       used together with general boot elements in
       <a href="#elementsOSBIOS">BIOS bootloader</a> section.
       <span class="since">Since 0.8.8</span></dd>
+      <dt><code>rom</code></dt>
+      <dd>The <code>rom</code> element is used to change how a PCI
+        device's ROM is presented to the guest. The <code>bar</code>
+        attribute can be set to "on" or "off", and determines whether
+        or not the device's ROM will be visible in the guest's memory
+        map. (In PCI documentation, the "rombar" setting controls the
+        presence of the Base Address Register for the ROM). If no rom
+        bar is specified, the qemu default will be used (older
+        versions of qemu used a default of "off", while newer qemus
+        have a default of "on"). <span class="since">Since
+        0.9.7</span>
+      </dd>
       <dt><code>address</code></dt>
       <dd>The <code>address</code> element for USB devices has a
       <code>bus</code> and <code>device</code> attribute to specify the

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index be98be03d67d78f2038be9d3d410a762daadba5e..4972fac0b24c911224a65b0f5a4586ea534bf91f 100644 (file)
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2060,6 +2060,17 @@
       <optional>
         <ref name="address"/>
       </optional>
+      <optional>
+        <element name="rom">
+          <attribute name="bar">
+            <choice>
+              <value>on</value>
+              <value>off</value>
+            </choice>
+          </attribute>
+          <empty/>
+        </element>
+      </optional>
     </element>
   </define>
   <define name="usbproduct">

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a91867915af0be0bc5c569bf9b568ae40736bfdc..c14198271bd20e2697e60bd845829354268f9290 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -443,6 +443,12 @@ VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST,
               "usb",
               "pci")
 
+VIR_ENUM_IMPL(virDomainPciRombarMode,
+              VIR_DOMAIN_PCI_ROMBAR_LAST,
+              "default",
+              "on",
+              "off")
+
 VIR_ENUM_IMPL(virDomainHub, VIR_DOMAIN_HUB_TYPE_LAST,
               "usb")
 
@@ -5486,6 +5492,20 @@ virDomainHostdevDefParseXML(const xmlNodePtr node,
                 if (virDomainDeviceBootParseXML(cur, &def->bootIndex,
                                                 bootMap))
                     goto error;
+            } else if (xmlStrEqual(cur->name, BAD_CAST "rom")) {
+                char *rombar = virXMLPropString(cur, "bar");
+                if (!rombar) {
+                    virDomainReportError(VIR_ERR_XML_ERROR,
+                                         "%s", _("missing rom bar attribute"));
+                    goto error;
+                }
+                if ((def->rombar = virDomainPciRombarModeTypeFromString(rombar)) <= 0) {
+                    virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                         _("unknown rom bar value '%s'"), rombar);
+                    VIR_FREE(rombar);
+                    goto error;
+                }
+                VIR_FREE(rombar);
             } else {
                 virDomainReportError(VIR_ERR_INTERNAL_ERROR,
                                      _("unknown node %s"), cur->name);
@@ -10388,6 +10408,18 @@ virDomainHostdevDefFormat(virBufferPtr buf,
     if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
         return -1;
 
+    if (def->rombar) {
+        const char *rombar
+            = virDomainPciRombarModeTypeToString(def->rombar);
+        if (!rombar) {
+            virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+                                 _("unexpected rom bar value %d"),
+                                 def->rombar);
+            return -1;
+        }
+        virBufferAsprintf(buf, "      <rom bar='%s'/>\n", rombar);
+    }
+
     virBufferAddLit(buf, "    </hostdev>\n");
 
     return 0;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 86b4c799f7fb22a8097c1c98bcb1cd2e2d9ab5eb..0bc00426a42cf6515b5e612e63d5724df41e71d1 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -937,6 +937,14 @@ enum virDomainHostdevSubsysType {
     VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST
 };
 
+enum virDomainPciRombarMode {
+    VIR_DOMAIN_PCI_ROMBAR_DEFAULT = 0,
+    VIR_DOMAIN_PCI_ROMBAR_ON,
+    VIR_DOMAIN_PCI_ROMBAR_OFF,
+
+    VIR_DOMAIN_PCI_ROMBAR_LAST
+};
+
 typedef struct _virDomainHostdevDef virDomainHostdevDef;
 typedef virDomainHostdevDef *virDomainHostdevDefPtr;
 struct _virDomainHostdevDef {
@@ -965,6 +973,7 @@ struct _virDomainHostdevDef {
     } source;
     int bootIndex;
     virDomainDeviceInfo info; /* Guest address */
+    int rombar;               /* enum virDomainPciRombarMode */
 };
 
 enum virDomainRedirdevBus {
@@ -1856,6 +1865,7 @@ VIR_ENUM_DECL(virDomainWatchdogAction)
 VIR_ENUM_DECL(virDomainVideo)
 VIR_ENUM_DECL(virDomainHostdevMode)
 VIR_ENUM_DECL(virDomainHostdevSubsys)
+VIR_ENUM_DECL(virDomainPciRombarMode)
 VIR_ENUM_DECL(virDomainHub)
 VIR_ENUM_DECL(virDomainRedirdevBus)
 VIR_ENUM_DECL(virDomainInput)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8235ea1276f91a34e8004d9880e94a7309a11015..c2a3fab2774ba8745800ca6eb9dc6bb89ae0a9a4 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -385,6 +385,8 @@ virDomainObjUnlock;
 virDomainObjUnref;
 virDomainPausedReasonTypeFromString;
 virDomainPausedReasonTypeToString;
+virDomainPciRombarModeTypeFromString;
+virDomainPciRombarModeTypeToString;
 virDomainRedirdevBusTypeFromString;
 virDomainRedirdevBusTypeToString;
 virDomainRemoveInactive;

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4325f7710103398a984aeef1551bc3859f1ee3e1..a653243f348abff6d72b5639a77f4eb810fc4885 100644 (file)
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -139,6 +139,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
               "no-shutdown",
 
               "cache-unsafe", /* 75 */
+              "rombar",
     );
 
 struct qemu_feature_flags {
@@ -1063,6 +1064,19 @@ qemuCapsComputeCmdFlags(const char *help,
 
     if (version >= 13000)
         qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION);
+
+    /* Although very new versions of qemu advertise the presence of
+     * the rombar option in the output of "qemu -device pci-assign,?",
+     * this advertisement was added to the code long after the option
+     * itself. According to qemu developers, though, rombar is
+     * available in all qemu binaries from release 0.12 onward.
+     * Setting the capability this way makes it available in more
+     * cases where it might be needed, and shouldn't cause any false
+     * positives (in the case that it did, qemu would produce an error
+     * log and refuse to start, so it would be immediately obvious).
+     */
+    if (version >= 12000)
+        qemuCapsSet(flags, QEMU_CAPS_PCI_ROMBAR);
 }
 
 /* We parse the output of 'qemu -help' to get the QEMU

diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index ae3de90ac947fecc6b2a946d41adafd536abb798..062f239e72b5c43d3a15313997ffb7b6bc37c3f7 100644 (file)
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -113,6 +113,7 @@ enum qemuCapsFlags {
     QEMU_CAPS_NO_SHUTDOWN       = 74, /* usable -no-shutdown */
 
     QEMU_CAPS_DRIVE_CACHE_UNSAFE = 75, /* Is cache=unsafe supported? */
+    QEMU_CAPS_PCI_ROMBAR         = 76, /* -device rombar=0|1 */
 
     QEMU_CAPS_LAST,                   /* this must always be the last item */
 };

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9174a5f4efd79aa082877a6501ea23cd77e29696..a13ba7191970d7a6e5adbc47d303f332499fdc8f 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2369,6 +2369,25 @@ qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, const char *configfd,
     if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
         goto error;
 
+    if (dev->rombar) {
+        if (!qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_ROMBAR)) {
+            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                            "%s", _("rombar not supported in this QEMU binary"));
+            goto error;
+        }
+
+        switch (dev->rombar) {
+        case VIR_DOMAIN_PCI_ROMBAR_OFF:
+            virBufferAddLit(&buf, ",rombar=0");
+            break;
+        case VIR_DOMAIN_PCI_ROMBAR_ON:
+            virBufferAddLit(&buf, ",rombar=1");
+            break;
+        default:
+            break;
+        }
+    }
+
     if (virBufferError(&buf)) {
         virReportOOMError();
         goto error;

diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 0ff8236582de6b476ac711a08423f9d83d13be76..8a06568e53745934f8f569cd882120e688595088 100644 (file)
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -314,7 +314,8 @@ mymain(void)
             QEMU_CAPS_VGA_NONE,
             QEMU_CAPS_MIGRATE_QEMU_FD,
             QEMU_CAPS_DRIVE_AIO,
-            QEMU_CAPS_NO_SHUTDOWN);
+            QEMU_CAPS_NO_SHUTDOWN,
+            QEMU_CAPS_PCI_ROMBAR);
     DO_TEST("qemu-kvm-0.12.1.2-rhel60", 12001, 1, 0,
             QEMU_CAPS_VNC_COLON,
             QEMU_CAPS_NO_REBOOT,
@@ -359,7 +360,8 @@ mymain(void)
             QEMU_CAPS_PIIX3_USB_UHCI,
             QEMU_CAPS_PIIX4_USB_UHCI,
             QEMU_CAPS_USB_HUB,
-            QEMU_CAPS_NO_SHUTDOWN);
+            QEMU_CAPS_NO_SHUTDOWN,
+            QEMU_CAPS_PCI_ROMBAR);
     DO_TEST("qemu-kvm-0.12.3", 12003, 1, 0,
             QEMU_CAPS_VNC_COLON,
             QEMU_CAPS_NO_REBOOT,
@@ -397,7 +399,8 @@ mymain(void)
             QEMU_CAPS_VGA_NONE,
             QEMU_CAPS_MIGRATE_QEMU_FD,
             QEMU_CAPS_DRIVE_AIO,
-            QEMU_CAPS_NO_SHUTDOWN);
+            QEMU_CAPS_NO_SHUTDOWN,
+            QEMU_CAPS_PCI_ROMBAR);
     DO_TEST("qemu-kvm-0.13.0", 13000, 1, 0,
             QEMU_CAPS_VNC_COLON,
             QEMU_CAPS_NO_REBOOT,
@@ -451,7 +454,8 @@ mymain(void)
             QEMU_CAPS_VT82C686B_USB_UHCI,
             QEMU_CAPS_PCI_OHCI,
             QEMU_CAPS_USB_HUB,
-            QEMU_CAPS_NO_SHUTDOWN);
+            QEMU_CAPS_NO_SHUTDOWN,
+            QEMU_CAPS_PCI_ROMBAR);
     DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0,
             QEMU_CAPS_VNC_COLON,
             QEMU_CAPS_NO_REBOOT,
@@ -501,7 +505,8 @@ mymain(void)
             QEMU_CAPS_PIIX3_USB_UHCI,
             QEMU_CAPS_PIIX4_USB_UHCI,
             QEMU_CAPS_USB_HUB,
-            QEMU_CAPS_NO_SHUTDOWN);
+            QEMU_CAPS_NO_SHUTDOWN,
+            QEMU_CAPS_PCI_ROMBAR);
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.args
new file mode 100644 (file)
index 0000000..1a8b14e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.args
@@ -0,0 +1,5 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
+pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \
+/dev/HostVG/QEMUGuest2 -usb -device pci-assign,host=06:12.5,id=hostdev0,\
+bus=pci.0,addr=0x3,rombar=0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.xml
new file mode 100644 (file)
index 0000000..bf17cc4
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.xml
@@ -0,0 +1,29 @@
+<domain type='qemu'>
+  <name>QEMUGuest2</name>
+  <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
+  <memory>219100</memory>
+  <currentMemory>219100</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest2'/>
+      <target dev='hda' bus='ide'/>
+    </disk>
+    <hostdev mode='subsystem' type='pci' managed='yes'>
+      <source>
+        <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/>
+      </source>
+      <rom bar='off'/>
+    </hostdev>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>