From dc79852af88a6ee8b03f8fd7e48e039bae056ed3 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Tue, 20 Sep 2011 13:31:52 -0400 Subject: [PATCH] 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 in the guest XML). rombar is forced on/off by adding: inside a 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. --- docs/formatdomain.html.in | 13 ++++++++ docs/schemas/domaincommon.rng | 11 +++++++ src/conf/domain_conf.c | 32 +++++++++++++++++++ src/conf/domain_conf.h | 10 ++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_capabilities.c | 14 ++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 19 +++++++++++ tests/qemuhelptest.c | 15 ++++++--- .../qemuxml2argv-hostdev-pci-rombar.args | 5 +++ .../qemuxml2argv-hostdev-pci-rombar.xml | 29 +++++++++++++++++ 11 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3087d016e3..9c3c2e8313 100644 --- 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> ... @@ -1407,6 +1408,18 @@ used together with general boot elements in BIOS bootloader section. Since 0.8.8 +
rom
+
The rom element is used to change how a PCI + device's ROM is presented to the guest. The bar + 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"). Since + 0.9.7 +
address
The address element for USB devices has a bus and device attribute to specify the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index be98be03d6..4972fac0b2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2060,6 +2060,17 @@ + + + + + on + off + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a91867915a..c14198271b 100644 --- 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, " \n", rombar); + } + virBufferAddLit(buf, " \n"); return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 86b4c799f7..0bc00426a4 100644 --- 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 8235ea1276..c2a3fab277 100644 --- 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 4325f77101..a653243f34 100644 --- 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 ae3de90ac9..062f239e72 100644 --- 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 9174a5f4ef..a13ba71919 100644 --- 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 0ff8236582..8a06568e53 100644 --- 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 index 0000000000..1a8b14e83e --- /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 index 0000000000..bf17cc49d0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-rombar.xml @@ -0,0 +1,29 @@ + + QEMUGuest2 + c7a5fdbd-edaf-9466-926a-d65c16db1809 + 219100 + 219100 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu + + + + + + +
+ + + + + + -- 2.39.5