[libvirt] [PATCH 0/2] Add support for adjusting the 64-bit PCI hole size
https://bugzilla.redhat.com/show_bug.cgi?id=990418 Ján Tomko (2): Add pcihole64 element to domain features Build QEMU command line for pcihole64 docs/formatdomain.html.in | 8 +++++ docs/schemas/domaincommon.rng | 7 ++++ src/conf/domain_conf.c | 13 ++++++- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_capabilities.c | 14 ++++++++ src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_command.c | 41 ++++++++++++++++++++++ .../qemuxml2argv-pcihole64-none.args | 4 +++ .../qemuxml2argv-pcihole64-none.xml | 24 +++++++++++++ .../qemuxml2argv-pcihole64-q35.args | 9 +++++ .../qemuxml2argv-pcihole64-q35.xml | 34 ++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args | 5 +++ tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml | 24 +++++++++++++ tests/qemuxml2argvtest.c | 10 ++++++ tests/qemuxml2xmltest.c | 4 +++ 15 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml -- 1.8.1.5
<features> <pcihole64 size='0'/> </features> It can be used to adjust (or disable) the size of the 64-bit PCI hole. The size attribute is in gigabytes, since it would get rounded up to nearest GB by QEMU anyway. Disabling it will be needed for guests that crash with the 64-bit PCI hole (like Windows XP), see: https://bugzilla.redhat.com/show_bug.cgi?id=990418 --- docs/formatdomain.html.in | 8 +++++ docs/schemas/domaincommon.rng | 7 +++++ src/conf/domain_conf.c | 13 ++++++++- src/conf/domain_conf.h | 2 ++ .../qemuxml2argv-pcihole64-none.xml | 24 +++++++++++++++ .../qemuxml2argv-pcihole64-q35.xml | 34 ++++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml | 24 +++++++++++++++ tests/qemuxml2xmltest.c | 4 +++ 8 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 83d551a..acfebd4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1248,6 +1248,14 @@ </tr> </table> </dd> + <dt><code>pcihole64</code></dt> + <dd>Adjust the 64-bit PCI hole size. There is a mandatory + attribute <code>size</code> in gigabytes specifying how big + the hole should be. Some guests (like Windows XP or Windows + Server 2003) might crash when QEMU and Seabios are recent + enough to support 64-bit PCI holes, unless this is set to 0. + <span class="since">Since 1.1.2 (QEMU only)</span> + </dd> </dl> <h3><a name="elementsTime">Time keeping</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ac807e6..15b877e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3554,6 +3554,13 @@ <empty/> </element> </optional> + <optional> + <element name="pcihole64"> + <attribute name="size"> + <ref name="unsignedInt"/> + </attribute> + </element> + </optional> </interleave> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7309877..0dd3986 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -142,7 +142,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "hap", "viridian", "privnet", - "hyperv") + "hyperv", + "pcihole64") VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST, "default", @@ -11304,6 +11305,14 @@ virDomainDefParseXML(xmlDocPtr xml, def->apic_eoi = eoi; VIR_FREE(tmp); } + } else if (val == VIR_DOMAIN_FEATURE_PCI_HOLE64) { + if (virXPathUInt("string(./features/pcihole64/@size)", ctxt, + &def->pcihole64) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid 'size' value for " + "64-bit PCI hole")); + goto error; + } } } VIR_FREE(nodes); @@ -16559,6 +16568,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, " eoi='%s'", virDomainFeatureStateTypeToString(def->apic_eoi)); + } else if (i == VIR_DOMAIN_FEATURE_PCI_HOLE64) { + virBufferAsprintf(buf, " size='%u'", def->pcihole64); } virBufferAddLit(buf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3e118d6..03f4f68 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1601,6 +1601,7 @@ enum virDomainFeature { VIR_DOMAIN_FEATURE_VIRIDIAN, VIR_DOMAIN_FEATURE_PRIVNET, VIR_DOMAIN_FEATURE_HYPERV, + VIR_DOMAIN_FEATURE_PCI_HOLE64, VIR_DOMAIN_FEATURE_LAST }; @@ -1973,6 +1974,7 @@ struct _virDomainDef { int features; /* enum virDomainFeatureState */ int apic_eoi; + unsigned int pcihole64; /* These options are of type virDomainFeatureState */ int hyperv_features[VIR_DOMAIN_HYPERV_LAST]; unsigned int hyperv_spinlocks; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml new file mode 100644 index 0000000..f1ffa12 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>foo</name> + <uuid>c84fc647-6198-4ff9-bf81-d65a1f8f5ec0</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-1.2'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <pcihole64 size='0'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pci-root'/> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml new file mode 100644 index 0000000..215ff5c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <pcihole64 size='1'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='sda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'/> + <controller type='pci' index='2' model='pci-bridge'/> + <controller type='sata' index='0'/> + <video> + <model type='qxl' ram='65536' vram='18432' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml new file mode 100644 index 0000000..b2bb0b4 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>foo</name> + <uuid>3c7c30b5-7866-4b05-8a29-efebccba52a0</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='pc-1.2'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <pcihole64 size='1'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pci-root'/> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5c6730d..96b94ac 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -309,6 +309,10 @@ mymain(void) DO_TEST_DIFFERENT("s390-defaultconsole"); + DO_TEST("pcihole64"); + DO_TEST("pcihole64-none"); + DO_TEST("pcihole64-q35"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.8.1.5
QEMU commit 3984890 introduced the "pci-hole64-size" property, to i440FX-pcihost and q35-pcihost with a default setting of 2 GB. Translate <pcihole64 size='x'/> to: -global q35-pcihost.pci-hole64-size=x for q35 machines and -global i440FX-pcihost.pci-hole64-size=x for i440FX-based machines. Error out on other machine types or if the size was specified but the pcihost device lacks 'pci-hole64-size' property. https://bugzilla.redhat.com/show_bug.cgi?id=990418 --- src/qemu/qemu_capabilities.c | 14 ++++++++ src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_command.c | 41 ++++++++++++++++++++++ .../qemuxml2argv-pcihole64-none.args | 4 +++ .../qemuxml2argv-pcihole64-q35.args | 9 +++++ tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args | 5 +++ tests/qemuxml2argvtest.c | 10 ++++++ 7 files changed, 85 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 47cc07a..87c9a96 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -235,6 +235,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vnc-share-policy", /* 150 */ "device-del-event", "dmi-to-pci-bridge", + "i440fx-pci-hole64-size", + "q35-pci-hole64-size", ); struct _virQEMUCaps { @@ -1436,6 +1438,14 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsScsiGeneric[] = { { "bootindex", QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX }, }; +static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsI440FXPciHost[] = { + { "pci-hole64-size", QEMU_CAPS_I440FX_PCI_HOLE64_SIZE }, +}; + +static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQ35PciHost[] = { + { "pci-hole64-size", QEMU_CAPS_Q35_PCI_HOLE64_SIZE }, +}; + struct virQEMUCapsObjectTypeProps { const char *type; struct virQEMUCapsStringFlags *props; @@ -1473,6 +1483,10 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { ARRAY_CARDINALITY(virQEMUCapsObjectPropsUsbHost) }, { "scsi-generic", virQEMUCapsObjectPropsScsiGeneric, ARRAY_CARDINALITY(virQEMUCapsObjectPropsScsiGeneric) }, + { "i440FX-pcihost", virQEMUCapsObjectPropsI440FXPciHost, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsI440FXPciHost) }, + { "q35-pcihost", virQEMUCapsObjectPropsQ35PciHost, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsQ35PciHost) }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 074e55d..69f3395 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -191,6 +191,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */ QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */ + QEMU_CAPS_I440FX_PCI_HOLE64_SIZE = 153, /* i440FX-pcihost.pci-hole64-size */ + QEMU_CAPS_Q35_PCI_HOLE64_SIZE = 154, /* q35-pcihost.pci-hole64-size */ 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 b811e1d..466a787 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2399,6 +2399,17 @@ qemuDomainMachineIsQ35(virDomainDefPtr def) } +static bool +qemuDomainMachineIsI440FX(virDomainDefPtr def) +{ + return (STREQ(def->os.machine, "pc") || + STRPREFIX(def->os.machine, "pc-0.") || + STRPREFIX(def->os.machine, "pc-1.") || + STRPREFIX(def->os.machine, "pc-i440") || + STRPREFIX(def->os.machine, "rhel")); +} + + static int qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, @@ -7799,6 +7810,36 @@ qemuBuildCommandLine(virConnectPtr conn, def->pm.s4 == VIR_DOMAIN_PM_STATE_DISABLED); } + if (def->features & (1 << VIR_DOMAIN_FEATURE_PCI_HOLE64)) { + const char *hoststr = NULL; + bool cap = false; + + if (qemuDomainMachineIsQ35(def)) { + hoststr = "q35-pcihost"; + cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_Q35_PCI_HOLE64_SIZE); + } else if (qemuDomainMachineIsI440FX(def)) { + hoststr = "i440FX-pcihost"; + cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Setting the 64-bit PCI hole size is not " + "supported for machine '%s'"), def->os.machine); + goto error; + } + + if (!cap) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("64-bit PCI hole size setting is not supported " + "with this QEMU binary")); + goto error; + } + + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.pci-hole64-size=%uG", hoststr, + def->pcihole64); + } + + if (!def->os.bootloader) { int boot_nparams = 0; virBuffer boot_buf = VIR_BUFFER_INITIALIZER; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args new file mode 100644 index 0000000..e878d2f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-none.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/libexec/qemu-kvm -S -M pc-1.2 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args new file mode 100644 index 0000000..a00209f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args @@ -0,0 +1,9 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi \ +-global q35-pcihost.pci-hole64-size=1G -boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x2 \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \ +-device ide-drive,unit=0,drive=drive-sata0-0-0,id=sata0-0-0 \ +-vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args new file mode 100644 index 0000000..709b650 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/libexec/qemu-kvm -S -M pc-1.2 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi \ +-global i440FX-pcihost.pci-hole64-size=1G -boot c -usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 679124e..f274ced 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1037,6 +1037,16 @@ mymain(void) DO_TEST_PARSE_ERROR("pci-root-address", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE); + DO_TEST("pcihole64", QEMU_CAPS_DEVICE, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); + DO_TEST_FAILURE("pcihole64-none", QEMU_CAPS_DEVICE); + DO_TEST("pcihole64-q35", + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI, + QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_Q35_PCI_HOLE64_SIZE); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.8.1.5
On Mon, Aug 12, 2013 at 03:08:39PM +0200, Ján Tomko wrote:
QEMU commit 3984890 introduced the "pci-hole64-size" property, to i440FX-pcihost and q35-pcihost with a default setting of 2 GB.
Translate <pcihole64 size='x'/> to: -global q35-pcihost.pci-hole64-size=x for q35 machines and -global i440FX-pcihost.pci-hole64-size=x for i440FX-based machines.
I'm not particularly attracted by the idea of stuffing this info into the <features> XML element. That is in danger of just becoming a generic dumping ground for random data. It was originally intended as just a list of boolean toggles, and the hyper-v support has abused that in a way that's not strictly backwards compatible - eg any app that treats elements as a simple set of booleans will be loosing data. Based on the CLI args there, this PCI hole data is really associated with the PCI root controller device. Can we model this against a <controller> element for the PCI root perhaps. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Ján Tomko