[libvirt] [PATCH RESEND v2 0/4] re-introduce <acpi-hotplug-bridge>
Changelog: v2 - rebased the patch series to latest master. I am re-introducing the patchset for <acpi-hotplug-bridge> which got reverted here few months back: https://www.spinics.net/linux/fedora/libvir/msg224089.html The reason for the reversal was that there seemed to be some instability/issues around the use of the qemu commandline which this patchset tries to support. In particular, some guest operating systems did not like the way QEMU was trying to disable native hotplug on pcie root ports. Subsequently, in QEMU 6.2, we have changed our mechanism using which we disable native hotplug. As I understand, we do not have any reported issues so far in 6.2 around this area. QEMU will enter a soft feature freeze in the first week of march in prep for 7.0 release. Libvirt is also entering a new release cycle phaze. Hence, I am introducing this patchset early enough in the release cycles so that if we do see any issues on the qemu side during the rc0, rc1 cycles and if reversal of this patchset is again required, it can be done in time before the next libvirt release end of March. All the patches in this series had been previously reviewed. Some subsequent fixes were made after my initial patches were pushed. I have squashed all those fixes and consolidated them into four patches. I have also updated the documentation to reflect the new changes from the QEMU side and rebased my changes fixing the tests in the process. What changed in QEMU post version 6.1 ? ========================================= We have made basically two major changes in QEMU. First is this change: (1) commit 211afe5c69b597acf85fdd577eb497f5be1ffbd8 Author: Julia Suvorova <jusual@redhat.com> Date: Fri Nov 12 06:08:56 2021 -0500 hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC There are two ways to enable ACPI PCI Hot-plug: * Disable the Hot-plug Capable bit on PCIe slots. This was the first approach which led to regression [1-2], as I/O space for a port is allocated only when it is hot-pluggable, which is determined by HPC bit. * Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC method. This removes the (future) ability of hot-plugging switches with PCIe Native hotplug since ACPI PCI Hot-plug only works with cold-plugged bridges. If the user wants to explicitely use this feature, they can disable ACPI PCI Hot-plug with: --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug instead of PCIe Native. [1] https://gitlab.com/qemu-project/qemu/-/issues/641 [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409 Signed-off-by: Julia Suvorova <jusual@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <20211112110857.3116853-5-imammedo@redhat.com> Reviewed-by: Ani Sinha <ani@anisinha.ca> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> The patch description says it all. Instead of masking out the HPC bit in pcie slots, we keep them turned on. Instead, we do not advertize native hotplug capability for PCIE using _OSC control method. See section 6.2.11 in ACPI spec 6.2. At the same time, we turn on ACPI hotplug for these slots so now the guest OS can select ACPI hotplug instead. The second change is introduction of a property with which we keep the existing behavior for pc-q35-6.1 machines. This means HPC bit is masked and ACPI hotplug is enabled by default for pcie root ports. The QEMU commit is: (2) commit c318bef76206c2ecb6016e8e68c4ac6ff9a4c8cb Author: Julia Suvorova <jusual@redhat.com> Date: Fri Nov 12 06:08:54 2021 -0500 hw/acpi/ich9: Add compat prop to keep HPC bit set for 6.1 machine type To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be turned on, while the switch to ACPI Hot-plug will be done in the DSDT table. Introducing 'x-keep-native-hpc' property disables the HPC bit only in 6.1 and as a result keeps the forced 'reserve-io' on pcie-root-ports in 6.1 too. [1] https://gitlab.com/qemu-project/qemu/-/issues/641 [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409 Signed-off-by: Julia Suvorova <jusual@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <20211112110857.3116853-3-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Lastly, as a related side note, because from QEMU 6.2 onwards, we do not mask out HPC bit in PCIE, the work done by this patch is no longer needed: (3) commit e2a6290aab578b2170c1f5909fa556385dc0d820 Author: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Date: Mon Aug 2 12:00:57 2021 +0300 hw/pcie-root-port: Fix hotplug for PCI devices requiring IO Q35 has now ACPI hotplug enabled by default for PCI(e) devices. As opposed to native PCIe hotplug, guests like Fedora 34 will not assign IO range to pcie-root-ports not supporting native hotplug, resulting into a regression. Reproduce by: qemu-bin -M q35 -device pcie-root-port,id=p1 -monitor stdio device_add e1000,bus=p1 In the Guest OS the respective pcie-root-port will have the IO range disabled. Fix it by setting the "reserve-io" hint capability of the pcie-root-ports so the firmware will allocate the IO range instead. Acked-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> Message-Id: <20210802090057.1709775-1-marcel@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> This is what commit (2) alludes to. In pc-q35-6.1 machines we do need patch (3) since we mask out HPC bit from pcie ports. I know this is convoluted mess. In fairness I am trying all I can in my spare time to help from the QEMU side. I am determined to see this patchset through into libvirt. Thanks Ani Sinha (4): qemu: capablities: detect acpi-pci-hotplug-with-bridge-support conf: introduce support for acpi-bridge-hotplug feature qemu: command: add support for acpi-bridge-hotplug feature NEWS: document new acpi pci hotplug config option NEWS.rst | 8 ++ docs/formatdomain.rst | 32 +++++++ docs/schemas/domaincommon.rng | 15 ++++ src/conf/domain_conf.c | 89 ++++++++++++++++++- src/conf/domain_conf.h | 9 ++ src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 19 ++++ src/qemu/qemu_validate.c | 42 +++++++++ .../caps_6.1.0.x86_64.xml | 1 + .../caps_6.2.0.x86_64.xml | 1 + .../caps_7.0.0.x86_64.xml | 1 + ...-hotplug-bridge-disable.aarch64-latest.err | 1 + .../aarch64-acpi-hotplug-bridge-disable.xml | 13 +++ ...-hotplug-bridge-disable.x86_64-latest.args | 35 ++++++++ .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 36 ++++++++ .../pc-i440fx-acpi-hotplug-bridge-enable.xml | 36 ++++++++ ...pi-hotplug-bridge-disable.x86_64-6.0.0.err | 1 + ...-hotplug-bridge-disable.x86_64-latest.args | 38 ++++++++ .../q35-acpi-hotplug-bridge-disable.xml | 53 +++++++++++ .../q35-acpi-hotplug-bridge-enable.xml | 53 +++++++++++ tests/qemuxml2argvtest.c | 7 ++ ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 4 + 27 files changed, 504 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml -- 2.25.1
qemu added support for i440fx specific global boolean flag PIIX4_PM.acpi-pci-hotplug-with-bridge-support around version 2.1. This flag is enabled by default. When disabled, it turns off acpi pci hotplug for cold plugged pci bridges in i440fx machine types. Very recently, in qemu version 6.1, the same global option was also added for q35 machine types as well. ICH9-LPC.acpi-pci-hotplug-with-bridge-support This option turns on or off acpi based hotplug for cold plugged pcie bridges like pcie root ports. This flag is also enabled by default. Please refer to the following qemu changes: c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug") 17858a16950860 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35") This patch adds the corresponding qemu capabilities in libvirt. For i440fx, the capability is detected as QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE. For q35, the capability is detected as QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE. Please note that the test specific qemu capabilities .replies files has already been updated as a part of regular refreshing them when a new qemu version is released. Hence, no updates to those files are required. Signed-off-by: Ani Sinha <ani@anisinha.ca> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml | 1 + 5 files changed, 10 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 529e9ceaf5..08d5d733ce 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -665,6 +665,9 @@ VIR_ENUM_IMPL(virQEMUCaps, "virtio-mem-pci.prealloc", /* QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI_PREALLOC */ "calc-dirty-rate", /* QEMU_CAPS_CALC_DIRTY_RATE */ "dirtyrate-param.mode", /* QEMU_CAPS_DIRTYRATE_MODE */ + + /* 425 */ + "ich9.acpi-hotplug-bridge", /* QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE */ ); @@ -1551,6 +1554,7 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioGpu[] = { static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsICH9[] = { { "disable_s3", QEMU_CAPS_ICH9_DISABLE_S3, NULL }, { "disable_s4", QEMU_CAPS_ICH9_DISABLE_S4, NULL }, + { "acpi-pci-hotplug-with-bridge-support", QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE, NULL }, }; static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBNECXHCI[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f6188b42de..51dc668913 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -641,6 +641,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_CALC_DIRTY_RATE, /* accepts calc-dirty-rate */ QEMU_CAPS_DIRTYRATE_MODE , /* calc-dirty-rate accepts mode parameter */ + /* 425 */ + QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE, /* -M q35 ICH9-LPC.acpi-pci-hotplug-with-bridge-support */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index ba1aecc37e..51e1e07d2f 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -239,6 +239,7 @@ <flag name='rbd-encryption'/> <flag name='sev-inject-launch-secret'/> <flag name='calc-dirty-rate'/> + <flag name='ich9.acpi-hotplug-bridge'/> <version>6001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml index d77907af55..7b665c82e8 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml @@ -241,6 +241,7 @@ <flag name='sev-inject-launch-secret'/> <flag name='calc-dirty-rate'/> <flag name='dirtyrate-param.mode'/> + <flag name='ich9.acpi-hotplug-bridge'/> <version>6002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml index ae800abcc4..692e2f14da 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml @@ -243,6 +243,7 @@ <flag name='virtio-mem-pci.prealloc'/> <flag name='calc-dirty-rate'/> <flag name='dirtyrate-param.mode'/> + <flag name='ich9.acpi-hotplug-bridge'/> <version>6002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> -- 2.25.1
This change introduces a new libvirt sub-element <pci> under <features> that can be used to configure all pci related features. Currently the only sub-sub element supported by this sub-element is 'acpi-bridge-hotplug' as shown below: <features> <pci> <acpi-bridge-hotplug state='on|off'/> </pci> </features> The above option is only available for the QEMU driver, for x86 guests only. It is a global option, affecting all PCI bridge controllers on the guest. The 'acpi-bridge-hotplug' option enables or disables ACPI hotplug support for cold-plugged pci bridges. Examples of bridges include the PCI-PCI bridge (pci-bridge controller) for pc (i440fx) machinetypes, or PCIe-PCI bridges and pcie-root-port controllers for q35 machinetypes. For pc machinetypes in x86, this option has been available in QEMU since version 2.1. Please see the following changes in qemu repo: 9e047b982452c6 ("piix4: add acpi pci hotplug support") 133a2da488062e ("pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled") For q35 machinetypes, this was introduced in QEMU 6.1 with the following changes in qemu repo: (a) c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug") (b) 17858a16950860 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35") The reasons for enabling ACPI based hotplug for PCIe (q35) based machines (as opposed to native hotplug) are outlined in (b). There are use cases where users would still want to use native hotplug. Therefore, this config option enables users to choose either ACPI based hotplug or native hotplug for bridges (for example for pcie root port controller in q35 machines). Qemu capability validation checks have also been added along with related unit tests to exercise the new conf option. Signed-off-by: Ani Sinha <ani@anisinha.ca> --- docs/formatdomain.rst | 32 +++++++ docs/schemas/domaincommon.rng | 15 ++++ src/conf/domain_conf.c | 89 ++++++++++++++++++- src/conf/domain_conf.h | 9 ++ src/qemu/qemu_validate.c | 42 +++++++++ .../aarch64-acpi-hotplug-bridge-disable.xml | 13 +++ .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 36 ++++++++ .../pc-i440fx-acpi-hotplug-bridge-enable.xml | 36 ++++++++ .../q35-acpi-hotplug-bridge-disable.xml | 53 +++++++++++ .../q35-acpi-hotplug-bridge-enable.xml | 53 +++++++++++ ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 4 + 15 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 9202cd3107..864dfd8bd8 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1879,6 +1879,9 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <e820_host state='on'/> <passthrough state='on' mode='share_pt'/> </xen> + <pci> + <acpi-bridge-hotplug state="on"/> + </pci> <pvspinlock state='on'/> <gic version='2'/> <ioapic driver='qemu'/> @@ -1997,6 +2000,35 @@ are: passthrough Enable IOMMU mappings allowing PCI passthrough on, off; mode - optional string sync_pt or share_pt :since:`6.3.0` =========== ============================================== =================================================== ============== +``pci`` + Various PCI bus related features of the hypervisor. + + ==================== ========================================================================================================= ======= ============== + Feature Description Value Since + ==================== ========================================================================================================= ======= ============== + acpi-bridge-hotplug Enable ACPI based hotplug on the cold-plugged PCI bridges (pc) and pcie-root-ports (q35) (also see notes) on, off :since:`8.2.0` + ==================== ========================================================================================================= ======= ============== + + Note: + pc machine types (i440fx) have ACPI hotplug enabled by + default on cold plugged bridges (bridges that were present in the + VM's domain definition before it was started). `acpi-bridge-hotplug` + xml option can be used to disable hotplug for these bridges. + Disabling ACPI hotplug leaves only SHPC hotplug enabled; many OSes don't + support SHPC hotplug, so this may have the effect of completely + disabling hotplug. + + On q35 machinetypes earlier than pc-q35-6.1, ACPI hotplug for cold + plugged bridges is disabled by default, and native PCIe hotplug is + enabled instead. Starting with the pc-q35-6.1 machinetype, ACPI + hotplug is enabled on cold plugged bridges by default while native + PCIe hotplug is disabled or not advertized to the OS. From QEMU binary + version 6.1 and above, it is possible to revert this default behavior + using `acpi-bridge-hotplug` xml feature as described above. Enabling + ACPI hotplug will disable or suppress advertizing native PCIe hotplug. + Disabling ACPI hotplug will re-enable PCIe native hotplug or start + advertizing its availability to the OS once more. + ``pmu`` Depending on the ``state`` attribute (values ``on``, ``off``, default ``on``) enable or disable the performance monitoring unit for the guest. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 964b0c9e2f..5f00b3ab60 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6198,6 +6198,9 @@ <optional> <ref name="ioapic"/> </optional> + <optional> + <ref name="pci"/> + </optional> <optional> <ref name="hpt"/> </optional> @@ -6432,6 +6435,18 @@ </element> </define> + <define name="pci"> + <element name="pci"> + <interleave> + <optional> + <element name="acpi-bridge-hotplug"> + <ref name="featurestate"/> + </element> + </optional> + </interleave> + </element> + </define> + <define name="ioapic"> <element name="ioapic"> <attribute name="driver"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 34fec887a3..c9938f3cf1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -181,6 +181,7 @@ VIR_ENUM_IMPL(virDomainFeature, "sbbc", "ibs", "tcg", + "pci", ); VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, @@ -223,6 +224,11 @@ VIR_ENUM_IMPL(virDomainXen, "passthrough", ); +VIR_ENUM_IMPL(virDomainPCI, + VIR_DOMAIN_PCI_LAST, + "acpi-bridge-hotplug", +); + VIR_ENUM_IMPL(virDomainXenPassthroughMode, VIR_DOMAIN_XEN_PASSTHROUGH_MODE_LAST, "default", @@ -17419,6 +17425,36 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, return 0; } +static int +virDomainFeaturesPCIDefParse(virDomainDef *def, + xmlNodePtr node) +{ + def->features[VIR_DOMAIN_FEATURE_PCI] = VIR_TRISTATE_SWITCH_ON; + + node = xmlFirstElementChild(node); + while (node) { + int feature; + virTristateSwitch value; + + feature = virDomainPCITypeFromString((const char *)node->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported PCI feature: %s"), + node->name); + return -1; + } + + if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED, + &value) < 0) + return -1; + + def->pci_features[feature] = value; + + node = xmlNextElementSibling(node); + } + + return 0; +} static int virDomainFeaturesXENDefParse(virDomainDef *def, @@ -17747,6 +17783,10 @@ virDomainFeaturesDefParse(virDomainDef *def, return -1; break; + case VIR_DOMAIN_FEATURE_PCI: + if (virDomainFeaturesPCIDefParse(def, nodes[i]) < 0) + return -1; + case VIR_DOMAIN_FEATURE_LAST: break; } @@ -17755,7 +17795,6 @@ virDomainFeaturesDefParse(virDomainDef *def, return 0; } - static int virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDef *def) { @@ -21419,6 +21458,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_FEATURE_HTM: case VIR_DOMAIN_FEATURE_NESTED_HV: case VIR_DOMAIN_FEATURE_CCF_ASSIST: + case VIR_DOMAIN_FEATURE_PCI: if (src->features[i] != dst->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of feature '%s' differs: " @@ -21686,6 +21726,29 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, } } + /* pci */ + if (src->features[VIR_DOMAIN_FEATURE_PCI] == VIR_TRISTATE_SWITCH_ON) { + for (i = 0; i < VIR_DOMAIN_PCI_LAST; i++) { + switch ((virDomainPCI) i) { + case VIR_DOMAIN_PCI_ACPI_BRIDGE_HOTPLUG: + if (src->pci_features[i] != dst->pci_features[i]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of PCI feature '%s' differs: " + "source: '%s', destination: '%s'"), + virDomainPCITypeToString(i), + virTristateSwitchTypeToString(src->pci_features[i]), + virTristateSwitchTypeToString(dst->pci_features[i])); + return false; + } + + break; + + case VIR_DOMAIN_PCI_LAST: + break; + } + } + } + /* smm */ if (src->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) { if (src->tseg_specified != dst->tseg_specified) { @@ -27912,6 +27975,30 @@ virDomainDefFormatFeatures(virBuffer *buf, virDomainFeatureTCGFormat(&childBuf, def); break; + case VIR_DOMAIN_FEATURE_PCI: + if (def->features[i] != VIR_TRISTATE_SWITCH_ON) + break; + + virBufferAddLit(&childBuf, "<pci>\n"); + virBufferAdjustIndent(&childBuf, 2); + for (j = 0; j < VIR_DOMAIN_PCI_LAST; j++) { + switch ((virDomainPCI) j) { + case VIR_DOMAIN_PCI_ACPI_BRIDGE_HOTPLUG: + if (def->pci_features[j] != VIR_TRISTATE_SWITCH_ABSENT) + virBufferAsprintf(&childBuf, "<%s state='%s'/>\n", + virDomainPCITypeToString(j), + virTristateSwitchTypeToString( + def->pci_features[j])); + break; + + case VIR_DOMAIN_PCI_LAST: + break; + } + } + virBufferAdjustIndent(&childBuf, -2); + virBufferAddLit(&childBuf, "</pci>\n"); + break; + case VIR_DOMAIN_FEATURE_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9fcf842ee7..0cc97de7a8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2072,6 +2072,7 @@ typedef enum { VIR_DOMAIN_FEATURE_SBBC, VIR_DOMAIN_FEATURE_IBS, VIR_DOMAIN_FEATURE_TCG, + VIR_DOMAIN_FEATURE_PCI, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -2097,6 +2098,12 @@ typedef enum { VIR_DOMAIN_HYPERV_LAST } virDomainHyperv; +typedef enum { + VIR_DOMAIN_PCI_ACPI_BRIDGE_HOTPLUG = 0, + + VIR_DOMAIN_PCI_LAST +} virDomainPCI; + typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0, VIR_DOMAIN_KVM_DEDICATED, @@ -2848,6 +2855,7 @@ struct _virDomainDef { int caps_features[VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST]; int hyperv_features[VIR_DOMAIN_HYPERV_LAST]; virDomainFeatureKVM *kvm_features; + int pci_features[VIR_DOMAIN_PCI_LAST]; int msrs_features[VIR_DOMAIN_MSRS_LAST]; int xen_features[VIR_DOMAIN_XEN_LAST]; virDomainXenPassthroughMode xen_passthrough_mode; @@ -3979,6 +3987,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceStreamingMode); VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode); VIR_ENUM_DECL(virDomainGraphicsVNCSharePolicy); VIR_ENUM_DECL(virDomainHyperv); +VIR_ENUM_DECL(virDomainPCI); VIR_ENUM_DECL(virDomainKVM); VIR_ENUM_DECL(virDomainXen); VIR_ENUM_DECL(virDomainXenPassthroughMode); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index f27e480696..e7c992f9e9 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -173,6 +173,43 @@ qemuValidateDomainDefPSeriesFeature(const virDomainDef *def, return 0; } +static int +qemuValidateDomainDefPCIFeature(const virDomainDef *def, + virQEMUCaps *qemuCaps, + int feature) +{ + size_t i; + + if (def->features[feature] == VIR_TRISTATE_SWITCH_ABSENT) + return 0; + + for (i = 0; i < VIR_DOMAIN_PCI_LAST; i++) { + if (def->pci_features[i] == VIR_TRISTATE_SWITCH_ABSENT) + continue; + + switch ((virDomainPCI) i) { + case VIR_DOMAIN_PCI_ACPI_BRIDGE_HOTPLUG: + if (!ARCH_IS_X86(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("acpi-bridge-hotplug is not available for architecture '%s'"), + virArchToString(def->os.arch)); + return -1; + } + + if (qemuDomainIsQ35(def) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("acpi-bridge-hotplug is not available with this QEMU binary")); + return -1; + } + break; + + case VIR_DOMAIN_PCI_LAST: + break; + } + } + return 0; +} static int qemuValidateDomainDefFeatures(const virDomainDef *def, @@ -311,6 +348,11 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, } break; + case VIR_DOMAIN_FEATURE_PCI: + if (qemuValidateDomainDefPCIFeature(def, qemuCaps, i) < 0) + return -1; + break; + case VIR_DOMAIN_FEATURE_SMM: case VIR_DOMAIN_FEATURE_KVM: case VIR_DOMAIN_FEATURE_XEN: diff --git a/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml b/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml new file mode 100644 index 0000000000..47107e93f3 --- /dev/null +++ b/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml @@ -0,0 +1,13 @@ +<domain type='qemu'> + <name>test</name> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <features> + <pci> + <acpi-bridge-hotplug state='off'/> + </pci> + </features> +</domain> diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml new file mode 100644 index 0000000000..1b63ff9539 --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>i440fx</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + <boot dev='network'/> + </os> + <features> + <pci> + <acpi-bridge-hotplug state='off'/> + </pci> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml new file mode 100644 index 0000000000..f7b2cbb9ce --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>i440fx</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + <boot dev='network'/> + </os> + <features> + <pci> + <acpi-bridge-hotplug state='on'/> + </pci> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml new file mode 100644 index 0000000000..87e61763fe --- /dev/null +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml @@ -0,0 +1,53 @@ +<domain type='qemu'> + <name>q35</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.5'>hvm</type> + <boot dev='network'/> + </os> + <features> + <pci> + <acpi-bridge-hotplug state='off'/> + </pci> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='usb' index='0' model='qemu-xhci'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml new file mode 100644 index 0000000000..9a8043849c --- /dev/null +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml @@ -0,0 +1,53 @@ +<domain type='qemu'> + <name>q35</name> + <uuid>56f5055c-1b8d-490c-844a-ad646a1caaaa</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-2.5'>hvm</type> + <boot dev='network'/> + </os> + <features> + <pci> + <acpi-bridge-hotplug state='on'/> + </pci> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='usb' index='0' model='qemu-xhci'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml new file mode 120000 index 0000000000..8154897401 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml new file mode 120000 index 0000000000..6b9e5492f8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml b/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml new file mode 120000 index 0000000000..77719b1325 --- /dev/null +++ b/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml \ No newline at end of file diff --git a/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml b/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml new file mode 120000 index 0000000000..3cd8ee569e --- /dev/null +++ b/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 052950b86f..a8dc0747e2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -436,6 +436,10 @@ mymain(void) DO_TEST("misc-disable-s3", QEMU_CAPS_PIIX_DISABLE_S3); DO_TEST_CAPS_LATEST("pc-i440fx-acpi-root-hotplug-disable"); DO_TEST_CAPS_LATEST("pc-i440fx-acpi-root-hotplug-enable"); + DO_TEST_CAPS_LATEST("pc-i440fx-acpi-hotplug-bridge-disable"); + DO_TEST_CAPS_LATEST("pc-i440fx-acpi-hotplug-bridge-enable"); + DO_TEST_CAPS_LATEST("q35-acpi-hotplug-bridge-disable"); + DO_TEST_CAPS_LATEST("q35-acpi-hotplug-bridge-enable"); DO_TEST("misc-disable-suspends", QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4); -- 2.25.1
This change adds backend qemu command line support for new libvirt global feature 'acpi-bridge-hotplug'. This option can be used as following: <feature> <pci> <acpi-bridge-hotplug state='off|on'/> </pci> </feature> The '<pci>' sub-element under '<feature>' is also newly introduced. 'acpi-bridge-hotplug' turns on the following command line option to qemu for x86 guests: (pc): -global PIIX4_PM.acpi-pci-hotplug-with-bridge-support=<off|on> (q35): -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=<off|on> This change also adds the required qemuxml2argv unit tests in order to test correct qemu arguments. Unit tests have also been added to test qemu capability validation checks as well as checks for using this option with the right architecture. Signed-off-by: Ani Sinha <ani@anisinha.ca> --- src/qemu/qemu_command.c | 19 ++++++++++ ...-hotplug-bridge-disable.aarch64-latest.err | 1 + ...-hotplug-bridge-disable.x86_64-latest.args | 35 +++++++++++++++++ ...pi-hotplug-bridge-disable.x86_64-6.0.0.err | 1 + ...-hotplug-bridge-disable.x86_64-latest.args | 38 +++++++++++++++++++ tests/qemuxml2argvtest.c | 7 ++++ 6 files changed, 101 insertions(+) create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c836799888..206a9794cb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6280,6 +6280,7 @@ qemuBuildPMCommandLine(virCommand *cmd, qemuDomainObjPrivate *priv) { virQEMUCaps *qemuCaps = priv->qemuCaps; + int acpihp_br = def->pci_features[VIR_DOMAIN_PCI_ACPI_BRIDGE_HOTPLUG]; if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SET_ACTION)) { /* with new qemu we always want '-no-shutdown' on startup and we set @@ -6325,6 +6326,24 @@ qemuBuildPMCommandLine(virCommand *cmd, pm_object, def->pm.s4 == VIR_TRISTATE_BOOL_NO); } + if (acpihp_br != VIR_TRISTATE_SWITCH_ABSENT) { + const char *pm_object = NULL; + + if (!qemuDomainIsQ35(def)) + pm_object = "PIIX4_PM"; + + if (qemuDomainIsQ35(def) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE)) + pm_object = "ICH9-LPC"; + + if (pm_object != NULL) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.acpi-pci-hotplug-with-bridge-support=%s", + pm_object, + virTristateSwitchTypeToString(acpihp_br)); + } + } + return 0; } diff --git a/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err b/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err new file mode 100644 index 0000000000..9f0a88b826 --- /dev/null +++ b/tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err @@ -0,0 +1 @@ +unsupported configuration: acpi-bridge-hotplug is not available for architecture 'aarch64' diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args new file mode 100644 index 0000000000..90527dfefd --- /dev/null +++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-i440fx \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-i440fx/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-i440fx/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-i440fx/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=i440fx,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-i440fx/master-key.aes"}' \ +-machine pc-i440fx-2.5,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 56f5055c-1b8d-490c-844a-ad646a1caaaa \ +-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 \ +-no-acpi \ +-global PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err new file mode 100644 index 0000000000..8c09a3cd76 --- /dev/null +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err @@ -0,0 +1 @@ +unsupported configuration: acpi-bridge-hotplug is not available with this QEMU binary diff --git a/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.args b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.args new file mode 100644 index 0000000000..6e3aeb1f30 --- /dev/null +++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.args @@ -0,0 +1,38 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-q35 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-q35/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-q35/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-q35/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=q35,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-q35/master-key.aes"}' \ +-machine pc-q35-2.5,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 56f5055c-1b8d-490c-844a-ad646a1caaaa \ +-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 \ +-no-acpi \ +-global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off \ +-boot strict=on \ +-device '{"driver":"i82801b11-bridge","id":"pci.1","bus":"pcie.0","addr":"0x1e"}' \ +-device '{"driver":"pci-bridge","chassis_nr":2,"id":"pci.2","bus":"pci.1","addr":"0x0"}' \ +-device '{"driver":"ioh3420","port":8,"chassis":3,"id":"pci.3","bus":"pcie.0","addr":"0x1"}' \ +-device '{"driver":"qemu-xhci","id":"usb","bus":"pci.3","addr":"0x0"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.2","addr":"0x1"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ce475df466..bc4f9cbcf1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2592,6 +2592,13 @@ mymain(void) DO_TEST_CAPS_LATEST("pc-i440fx-acpi-root-hotplug-enable"); DO_TEST_CAPS_VER_PARSE_ERROR("pc-i440fx-acpi-root-hotplug-disable", "5.1.0"); DO_TEST_CAPS_VER_PARSE_ERROR("pc-i440fx-acpi-root-hotplug-enable", "5.1.0"); + DO_TEST_CAPS_LATEST("q35-acpi-hotplug-bridge-disable"); + DO_TEST_CAPS_LATEST("pc-i440fx-acpi-hotplug-bridge-disable"); + DO_TEST_CAPS_VER_PARSE_ERROR("q35-acpi-hotplug-bridge-disable", "6.0.0"); + /* verify that we fail when acpi-bridge-hotplug option is specified for + * archs other than x86 + */ + DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("aarch64-acpi-hotplug-bridge-disable", "aarch64"); DO_TEST("q35-usb2", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, -- 2.25.1
Added the following new libvirt conf option to the release note to indicate their availability with the next release: <feature> <pci> <acpi-bridge-hotplug state='off|on'/> </pci> </feature> Signed-off-by: Ani Sinha <ani@anisinha.ca> --- NEWS.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index a5b6106bc2..e474b32e69 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,14 @@ v8.2.0 (unreleased) * **New features** + * qemu: Add a new global feature for controlling ACPI PCI hotplug on bridges + + A new ``<feature>`` config option ``<acpi-bridge-hotplug>`` under ``<pci>`` + sub-element have been added to allow users enable/disable ACPI based PCI + hotplug for cold plugged bridges (that is, bridges that were present in the + domain definition when the guest was first started).This setting is only + applicable for x86 guests using qemu driver. + * **Improvements** * **Bug fixes** -- 2.25.1
On 3/8/22 1:39 AM, Ani Sinha wrote:
Changelog: v2 - rebased the patch series to latest master.
I am re-introducing the patchset for <acpi-hotplug-bridge> which got reverted here few months back:
https://www.spinics.net/linux/fedora/libvir/msg224089.html
The reason for the reversal was that there seemed to be some instability/issues around the use of the qemu commandline which this patchset tries to support. In particular, some guest operating systems did not like the way QEMU was trying to disable native hotplug on pcie root ports.
My memory isn't completely clear, but I think there was also the issue that the option claims to enable ACPI hotplug when set to on, but instead what it actually does (in the Q35 case at least) is to enable native PCI hotplug when set to off (without actually disabling ACPI hotplug) and disable native PCI hotplug when set to on, or something like that. This ends up leaving it up to the guest OS to decide which type of hotplug to use, meaning its decision could override what's in the libvirt config, thus confusing everyone. Again, I probably have the details mixed up, but it was something like this. I asked mst about this this morning, and he suggested something that you've already done - Cc'ing the series to qemu-devel and the relevant maintainers so we can have a discussion with all involved parties about their opinions on whether we really should expose this existing option in libvirt, or if we should instead have two new options that are more orthogonal about enabling/disabling the two types of hotplug, so that libvirt config can more accurately represent what is being presented to the guest rather than a "best guess" of what we think the guest is going to do with what is presented. (Michael did also say that, with the current flurry of bug reports for the QEMU rc's, this discusion may not happen until closer to release when the bug reports die down. I know this doesn't mesh with your desire to "push now to allow for testing" (which in general would be a good thing if we were certain that we wanted the option like this and were just expecting some minor bugs that could be fixed), but my opinion is that 1) it's possible for anyone interested to test the functionality using <qemu:commandline>, and 2) we should avoid turning libvirt git into a revolving door of experiments. The only practical difference between using <qemu:commandline> and having a dedicated option is that the use of <qemu:commandline> causes the domain to be tainted, and the XML is a bit more complicated. But since the people we're talking about here will already have built their own libvirt binaries, the tainted status of any guests is irrelevant and the extra complexity of using <qemu:commandline> is probably trivial to them :-).
Subsequently, in QEMU 6.2, we have changed our mechanism using which we disable native hotplug. As I understand, we do not have any reported issues so far in 6.2 around this area. QEMU will enter a soft feature freeze in the first week of march in prep for 7.0 release. Libvirt is also entering a new release cycle phaze. Hence, I am introducing this patchset early enough in the release cycles so that if we do see any issues on the qemu side during the rc0, rc1 cycles and if reversal of this patchset is again required, it can be done in time before the next libvirt release end of March.
All the patches in this series had been previously reviewed. Some subsequent fixes were made after my initial patches were pushed. I have squashed all those fixes and consolidated them into four patches. I have also updated the documentation to reflect the new changes from the QEMU side and rebased my changes fixing the tests in the process.
What changed in QEMU post version 6.1 ? =========================================
We have made basically two major changes in QEMU. First is this change:
(1) commit 211afe5c69b597acf85fdd577eb497f5be1ffbd8 Author: Julia Suvorova <jusual@redhat.com> Date: Fri Nov 12 06:08:56 2021 -0500
hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC
There are two ways to enable ACPI PCI Hot-plug:
* Disable the Hot-plug Capable bit on PCIe slots.
This was the first approach which led to regression [1-2], as I/O space for a port is allocated only when it is hot-pluggable, which is determined by HPC bit.
* Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC method.
This removes the (future) ability of hot-plugging switches with PCIe Native hotplug since ACPI PCI Hot-plug only works with cold-plugged bridges. If the user wants to explicitely use this feature, they can disable ACPI PCI Hot-plug with: --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug instead of PCIe Native.
[1] https://gitlab.com/qemu-project/qemu/-/issues/641 [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
Signed-off-by: Julia Suvorova <jusual@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <20211112110857.3116853-5-imammedo@redhat.com> Reviewed-by: Ani Sinha <ani@anisinha.ca> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The patch description says it all. Instead of masking out the HPC bit in pcie slots, we keep them turned on. Instead, we do not advertize native hotplug capability for PCIE using _OSC control method. See section 6.2.11 in ACPI spec 6.2. At the same time, we turn on ACPI hotplug for these slots so now the guest OS can select ACPI hotplug instead.
The second change is introduction of a property with which we keep the existing behavior for pc-q35-6.1 machines. This means HPC bit is masked and ACPI hotplug is enabled by default for pcie root ports. The QEMU commit is:
(2) commit c318bef76206c2ecb6016e8e68c4ac6ff9a4c8cb Author: Julia Suvorova <jusual@redhat.com> Date: Fri Nov 12 06:08:54 2021 -0500
hw/acpi/ich9: Add compat prop to keep HPC bit set for 6.1 machine type
To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be turned on, while the switch to ACPI Hot-plug will be done in the DSDT table.
Introducing 'x-keep-native-hpc' property disables the HPC bit only in 6.1 and as a result keeps the forced 'reserve-io' on pcie-root-ports in 6.1 too.
[1] https://gitlab.com/qemu-project/qemu/-/issues/641 [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
Signed-off-by: Julia Suvorova <jusual@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <20211112110857.3116853-3-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Lastly, as a related side note, because from QEMU 6.2 onwards, we do not mask out HPC bit in PCIE, the work done by this patch is no longer needed:
(3) commit e2a6290aab578b2170c1f5909fa556385dc0d820 Author: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Date: Mon Aug 2 12:00:57 2021 +0300
hw/pcie-root-port: Fix hotplug for PCI devices requiring IO
Q35 has now ACPI hotplug enabled by default for PCI(e) devices. As opposed to native PCIe hotplug, guests like Fedora 34 will not assign IO range to pcie-root-ports not supporting native hotplug, resulting into a regression.
Reproduce by: qemu-bin -M q35 -device pcie-root-port,id=p1 -monitor stdio device_add e1000,bus=p1 In the Guest OS the respective pcie-root-port will have the IO range disabled.
Fix it by setting the "reserve-io" hint capability of the pcie-root-ports so the firmware will allocate the IO range instead.
Acked-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> Message-Id: <20210802090057.1709775-1-marcel@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This is what commit (2) alludes to. In pc-q35-6.1 machines we do need patch (3) since we mask out HPC bit from pcie ports.
I know this is convoluted mess. In fairness I am trying all I can in my spare time to help from the QEMU side. I am determined to see this patchset through into libvirt.
Thanks
Ani Sinha (4): qemu: capablities: detect acpi-pci-hotplug-with-bridge-support conf: introduce support for acpi-bridge-hotplug feature qemu: command: add support for acpi-bridge-hotplug feature NEWS: document new acpi pci hotplug config option
NEWS.rst | 8 ++ docs/formatdomain.rst | 32 +++++++ docs/schemas/domaincommon.rng | 15 ++++ src/conf/domain_conf.c | 89 ++++++++++++++++++- src/conf/domain_conf.h | 9 ++ src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 19 ++++ src/qemu/qemu_validate.c | 42 +++++++++ .../caps_6.1.0.x86_64.xml | 1 + .../caps_6.2.0.x86_64.xml | 1 + .../caps_7.0.0.x86_64.xml | 1 + ...-hotplug-bridge-disable.aarch64-latest.err | 1 + .../aarch64-acpi-hotplug-bridge-disable.xml | 13 +++ ...-hotplug-bridge-disable.x86_64-latest.args | 35 ++++++++ .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 36 ++++++++ .../pc-i440fx-acpi-hotplug-bridge-enable.xml | 36 ++++++++ ...pi-hotplug-bridge-disable.x86_64-6.0.0.err | 1 + ...-hotplug-bridge-disable.x86_64-latest.args | 38 ++++++++ .../q35-acpi-hotplug-bridge-disable.xml | 53 +++++++++++ .../q35-acpi-hotplug-bridge-enable.xml | 53 +++++++++++ tests/qemuxml2argvtest.c | 7 ++ ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 4 + 27 files changed, 504 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml
Aha! the domain of qemu-devel@nongnu.org was incorrect in the original send (it was "nognu.org"), so none of this thread was making it to that list. I've corrected it in this message, but interested parties from qemu-devel will need to look on the libvir-list archives for the actual patch mails: https://listman.redhat.com/archives/libvir-list/2022-March/229089.html Anyone else who responds to any of the mail on this thread should fix the qemu-devel address accordingly. On 3/8/22 10:33 AM, Laine Stump wrote:
On 3/8/22 1:39 AM, Ani Sinha wrote:
Changelog: v2 - rebased the patch series to latest master.
I am re-introducing the patchset for <acpi-hotplug-bridge> which got reverted here few months back:
https://www.spinics.net/linux/fedora/libvir/msg224089.html
The reason for the reversal was that there seemed to be some instability/issues around the use of the qemu commandline which this patchset tries to support. In particular, some guest operating systems did not like the way QEMU was trying to disable native hotplug on pcie root ports.
My memory isn't completely clear, but I think there was also the issue that the option claims to enable ACPI hotplug when set to on, but instead what it actually does (in the Q35 case at least) is to enable native PCI hotplug when set to off (without actually disabling ACPI hotplug) and disable native PCI hotplug when set to on, or something like that. This ends up leaving it up to the guest OS to decide which type of hotplug to use, meaning its decision could override what's in the libvirt config, thus confusing everyone. Again, I probably have the details mixed up, but it was something like this.
I asked mst about this this morning, and he suggested something that you've already done - Cc'ing the series to qemu-devel and the relevant maintainers so we can have a discussion with all involved parties about their opinions on whether we really should expose this existing option in libvirt, or if we should instead have two new options that are more orthogonal about enabling/disabling the two types of hotplug, so that libvirt config can more accurately represent what is being presented to the guest rather than a "best guess" of what we think the guest is going to do with what is presented.
(Michael did also say that, with the current flurry of bug reports for the QEMU rc's, this discusion may not happen until closer to release when the bug reports die down. I know this doesn't mesh with your desire to "push now to allow for testing" (which in general would be a good thing if we were certain that we wanted the option like this and were just expecting some minor bugs that could be fixed), but my opinion is that 1) it's possible for anyone interested to test the functionality using <qemu:commandline>, and 2) we should avoid turning libvirt git into a revolving door of experiments. The only practical difference between using <qemu:commandline> and having a dedicated option is that the use of <qemu:commandline> causes the domain to be tainted, and the XML is a bit more complicated. But since the people we're talking about here will already have built their own libvirt binaries, the tainted status of any guests is irrelevant and the extra complexity of using <qemu:commandline> is probably trivial to them :-).
Subsequently, in QEMU 6.2, we have changed our mechanism using which we disable native hotplug. As I understand, we do not have any reported issues so far in 6.2 around this area. QEMU will enter a soft feature freeze in the first week of march in prep for 7.0 release. Libvirt is also entering a new release cycle phaze. Hence, I am introducing this patchset early enough in the release cycles so that if we do see any issues on the qemu side during the rc0, rc1 cycles and if reversal of this patchset is again required, it can be done in time before the next libvirt release end of March.
All the patches in this series had been previously reviewed. Some subsequent fixes were made after my initial patches were pushed. I have squashed all those fixes and consolidated them into four patches. I have also updated the documentation to reflect the new changes from the QEMU side and rebased my changes fixing the tests in the process.
What changed in QEMU post version 6.1 ? =========================================
We have made basically two major changes in QEMU. First is this change:
(1) commit 211afe5c69b597acf85fdd577eb497f5be1ffbd8 Author: Julia Suvorova <jusual@redhat.com> Date: Fri Nov 12 06:08:56 2021 -0500
hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC There are two ways to enable ACPI PCI Hot-plug: * Disable the Hot-plug Capable bit on PCIe slots. This was the first approach which led to regression [1-2], as I/O space for a port is allocated only when it is hot-pluggable, which is determined by HPC bit. * Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC method. This removes the (future) ability of hot-plugging switches with PCIe Native hotplug since ACPI PCI Hot-plug only works with cold-plugged bridges. If the user wants to explicitely use this feature, they can disable ACPI PCI Hot-plug with: --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug instead of PCIe Native. [1] https://gitlab.com/qemu-project/qemu/-/issues/641 [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409 Signed-off-by: Julia Suvorova <jusual@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <20211112110857.3116853-5-imammedo@redhat.com> Reviewed-by: Ani Sinha <ani@anisinha.ca> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The patch description says it all. Instead of masking out the HPC bit in pcie slots, we keep them turned on. Instead, we do not advertize native hotplug capability for PCIE using _OSC control method. See section 6.2.11 in ACPI spec 6.2. At the same time, we turn on ACPI hotplug for these slots so now the guest OS can select ACPI hotplug instead.
The second change is introduction of a property with which we keep the existing behavior for pc-q35-6.1 machines. This means HPC bit is masked and ACPI hotplug is enabled by default for pcie root ports. The QEMU commit is:
(2) commit c318bef76206c2ecb6016e8e68c4ac6ff9a4c8cb Author: Julia Suvorova <jusual@redhat.com> Date: Fri Nov 12 06:08:54 2021 -0500
hw/acpi/ich9: Add compat prop to keep HPC bit set for 6.1 machine type To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be turned on, while the switch to ACPI Hot-plug will be done in the DSDT table. Introducing 'x-keep-native-hpc' property disables the HPC bit only in 6.1 and as a result keeps the forced 'reserve-io' on pcie-root-ports in 6.1 too. [1] https://gitlab.com/qemu-project/qemu/-/issues/641 [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409 Signed-off-by: Julia Suvorova <jusual@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <20211112110857.3116853-3-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Lastly, as a related side note, because from QEMU 6.2 onwards, we do not mask out HPC bit in PCIE, the work done by this patch is no longer needed:
(3) commit e2a6290aab578b2170c1f5909fa556385dc0d820 Author: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Date: Mon Aug 2 12:00:57 2021 +0300
hw/pcie-root-port: Fix hotplug for PCI devices requiring IO Q35 has now ACPI hotplug enabled by default for PCI(e) devices. As opposed to native PCIe hotplug, guests like Fedora 34 will not assign IO range to pcie-root-ports not supporting native hotplug, resulting into a regression. Reproduce by: qemu-bin -M q35 -device pcie-root-port,id=p1 -monitor stdio device_add e1000,bus=p1 In the Guest OS the respective pcie-root-port will have the IO range disabled. Fix it by setting the "reserve-io" hint capability of the pcie-root-ports so the firmware will allocate the IO range instead. Acked-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> Message-Id: <20210802090057.1709775-1-marcel@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This is what commit (2) alludes to. In pc-q35-6.1 machines we do need patch (3) since we mask out HPC bit from pcie ports.
I know this is convoluted mess. In fairness I am trying all I can in my spare time to help from the QEMU side. I am determined to see this patchset through into libvirt.
Thanks
Ani Sinha (4): qemu: capablities: detect acpi-pci-hotplug-with-bridge-support conf: introduce support for acpi-bridge-hotplug feature qemu: command: add support for acpi-bridge-hotplug feature NEWS: document new acpi pci hotplug config option
NEWS.rst | 8 ++ docs/formatdomain.rst | 32 +++++++ docs/schemas/domaincommon.rng | 15 ++++ src/conf/domain_conf.c | 89 ++++++++++++++++++- src/conf/domain_conf.h | 9 ++ src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 19 ++++ src/qemu/qemu_validate.c | 42 +++++++++ .../caps_6.1.0.x86_64.xml | 1 + .../caps_6.2.0.x86_64.xml | 1 + .../caps_7.0.0.x86_64.xml | 1 + ...-hotplug-bridge-disable.aarch64-latest.err | 1 + .../aarch64-acpi-hotplug-bridge-disable.xml | 13 +++ ...-hotplug-bridge-disable.x86_64-latest.args | 35 ++++++++ .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 36 ++++++++ .../pc-i440fx-acpi-hotplug-bridge-enable.xml | 36 ++++++++ ...pi-hotplug-bridge-disable.x86_64-6.0.0.err | 1 + ...-hotplug-bridge-disable.x86_64-latest.args | 38 ++++++++ .../q35-acpi-hotplug-bridge-disable.xml | 53 +++++++++++ .../q35-acpi-hotplug-bridge-enable.xml | 53 +++++++++++ tests/qemuxml2argvtest.c | 7 ++ ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 4 + 27 files changed, 504 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err
create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml
create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml
create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml
create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml
On Tue, Mar 8, 2022 at 21:21 Laine Stump <laine@redhat.com> wrote:
Aha! the domain of qemu-devel@nongnu.org was incorrect in the original send (it was "nognu.org"), so none of this thread was making it to that list. I've corrected it in this message, but interested parties from qemu-devel will need to look on the libvir-list archives for the actual patch mails:
https://listman.redhat.com/archives/libvir-list/2022-March/229089.html
Anyone else who responds to any of the mail on this thread should fix the qemu-devel address accordingly.
This patch set has been a true test of my diligence and perseverance 🙂
On 3/8/22 10:33 AM, Laine Stump wrote:
On 3/8/22 1:39 AM, Ani Sinha wrote:
Changelog: v2 - rebased the patch series to latest master.
I am re-introducing the patchset for <acpi-hotplug-bridge> which got reverted here few months back:
https://www.spinics.net/linux/fedora/libvir/msg224089.html
The reason for the reversal was that there seemed to be some instability/issues around the use of the qemu commandline which this patchset tries to support. In particular, some guest operating systems did not like the way QEMU was trying to disable native hotplug on pcie root ports.
My memory isn't completely clear, but I think there was also the issue that the option claims to enable ACPI hotplug when set to on, but instead what it actually does (in the Q35 case at least) is to enable native PCI hotplug when set to off (without actually disabling ACPI hotplug) and disable native PCI hotplug when set to on, or something like that. This ends up leaving it up to the guest OS to decide which type of hotplug to use, meaning its decision could override what's in the libvirt config, thus confusing everyone. Again, I probably have the details mixed up, but it was something like this.
I asked mst about this this morning, and he suggested something that you've already done - Cc'ing the series to qemu-devel and the relevant maintainers so we can have a discussion with all involved parties about their opinions on whether we really should expose this existing option in libvirt, or if we should instead have two new options that are more orthogonal about enabling/disabling the two types of hotplug, so that libvirt config can more accurately represent what is being presented to the guest rather than a "best guess" of what we think the guest is going to do with what is presented.
(Michael did also say that, with the current flurry of bug reports for the QEMU rc's, this discusion may not happen until closer to release when the bug reports die down. I know this doesn't mesh with your desire to "push now to allow for testing" (which in general would be a good thing if we were certain that we wanted the option like this and were just expecting some minor bugs that could be fixed), but my opinion is that 1) it's possible for anyone interested to test the functionality using <qemu:commandline>, and 2) we should avoid turning libvirt git into a revolving door of experiments. The only practical difference between using <qemu:commandline> and having a dedicated option is that the use of <qemu:commandline> causes the domain to be tainted, and the XML is a bit more complicated. But since the people we're talking about here will already have built their own libvirt binaries, the tainted status of any guests is irrelevant and the extra complexity of using <qemu:commandline> is probably trivial to them :-).
Subsequently, in QEMU 6.2, we have changed our mechanism using which we disable native hotplug. As I understand, we do not have any reported issues so far in 6.2 around this area. QEMU will enter a soft feature freeze in the first week of march in prep for 7.0 release. Libvirt is also entering a new release cycle phaze. Hence, I am introducing this patchset early enough in the release cycles so that if we do see any issues on the qemu side during the rc0, rc1 cycles and if reversal of this patchset is again required, it can be done in time before the next libvirt release end of March.
All the patches in this series had been previously reviewed. Some subsequent fixes were made after my initial patches were pushed. I have squashed all those fixes and consolidated them into four patches. I have also updated the documentation to reflect the new changes from the QEMU side and rebased my changes fixing the tests in the process.
What changed in QEMU post version 6.1 ? =========================================
We have made basically two major changes in QEMU. First is this change:
(1) commit 211afe5c69b597acf85fdd577eb497f5be1ffbd8 Author: Julia Suvorova <jusual@redhat.com> Date: Fri Nov 12 06:08:56 2021 -0500
hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC There are two ways to enable ACPI PCI Hot-plug: * Disable the Hot-plug Capable bit on PCIe slots. This was the first approach which led to regression [1-2], as I/O space for a port is allocated only when it is hot-pluggable, which is determined by HPC bit. * Leave the HPC bit on and disable PCIe Native Hot-plug in _OSC method. This removes the (future) ability of hot-plugging switches with PCIe Native hotplug since ACPI PCI Hot-plug only works with cold-plugged bridges. If the user wants to explicitely use this feature, they can disable ACPI PCI Hot-plug with: --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off Change the bit in _OSC method so that the OS selects ACPI PCI Hot-plug instead of PCIe Native. [1] https://gitlab.com/qemu-project/qemu/-/issues/641 [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409 Signed-off-by: Julia Suvorova <jusual@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <20211112110857.3116853-5-imammedo@redhat.com> Reviewed-by: Ani Sinha <ani@anisinha.ca> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The patch description says it all. Instead of masking out the HPC bit in pcie slots, we keep them turned on. Instead, we do not advertize native hotplug capability for PCIE using _OSC control method. See section 6.2.11 in ACPI spec 6.2. At the same time, we turn on ACPI hotplug for these slots so now the guest OS can select ACPI hotplug instead.
The second change is introduction of a property with which we keep the existing behavior for pc-q35-6.1 machines. This means HPC bit is masked and ACPI hotplug is enabled by default for pcie root ports. The QEMU commit is:
(2) commit c318bef76206c2ecb6016e8e68c4ac6ff9a4c8cb Author: Julia Suvorova <jusual@redhat.com> Date: Fri Nov 12 06:08:54 2021 -0500
hw/acpi/ich9: Add compat prop to keep HPC bit set for 6.1 machine type To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be turned on, while the switch to ACPI Hot-plug will be done in the DSDT table. Introducing 'x-keep-native-hpc' property disables the HPC bit only in 6.1 and as a result keeps the forced 'reserve-io' on pcie-root-ports in 6.1 too. [1] https://gitlab.com/qemu-project/qemu/-/issues/641 [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409 Signed-off-by: Julia Suvorova <jusual@redhat.com> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <20211112110857.3116853-3-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Lastly, as a related side note, because from QEMU 6.2 onwards, we do not mask out HPC bit in PCIE, the work done by this patch is no longer needed:
(3) commit e2a6290aab578b2170c1f5909fa556385dc0d820 Author: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Date: Mon Aug 2 12:00:57 2021 +0300
hw/pcie-root-port: Fix hotplug for PCI devices requiring IO Q35 has now ACPI hotplug enabled by default for PCI(e) devices. As opposed to native PCIe hotplug, guests like Fedora 34 will not assign IO range to pcie-root-ports not supporting native hotplug, resulting into a regression. Reproduce by: qemu-bin -M q35 -device pcie-root-port,id=p1 -monitor stdio device_add e1000,bus=p1 In the Guest OS the respective pcie-root-port will have the IO range disabled. Fix it by setting the "reserve-io" hint capability of the pcie-root-ports so the firmware will allocate the IO range instead. Acked-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> Message-Id: <20210802090057.1709775-1-marcel@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This is what commit (2) alludes to. In pc-q35-6.1 machines we do need patch (3) since we mask out HPC bit from pcie ports.
I know this is convoluted mess. In fairness I am trying all I can in my spare time to help from the QEMU side. I am determined to see this patchset through into libvirt.
Thanks
Ani Sinha (4): qemu: capablities: detect acpi-pci-hotplug-with-bridge-support conf: introduce support for acpi-bridge-hotplug feature qemu: command: add support for acpi-bridge-hotplug feature NEWS: document new acpi pci hotplug config option
NEWS.rst | 8 ++ docs/formatdomain.rst | 32 +++++++ docs/schemas/domaincommon.rng | 15 ++++ src/conf/domain_conf.c | 89 ++++++++++++++++++- src/conf/domain_conf.h | 9 ++ src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 19 ++++ src/qemu/qemu_validate.c | 42 +++++++++ .../caps_6.1.0.x86_64.xml | 1 + .../caps_6.2.0.x86_64.xml | 1 + .../caps_7.0.0.x86_64.xml | 1 + ...-hotplug-bridge-disable.aarch64-latest.err | 1 + .../aarch64-acpi-hotplug-bridge-disable.xml | 13 +++ ...-hotplug-bridge-disable.x86_64-latest.args | 35 ++++++++ .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 36 ++++++++ .../pc-i440fx-acpi-hotplug-bridge-enable.xml | 36 ++++++++ ...pi-hotplug-bridge-disable.x86_64-6.0.0.err | 1 + ...-hotplug-bridge-disable.x86_64-latest.args | 38 ++++++++ .../q35-acpi-hotplug-bridge-disable.xml | 53 +++++++++++ .../q35-acpi-hotplug-bridge-enable.xml | 53 +++++++++++ tests/qemuxml2argvtest.c | 7 ++ ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 4 + 27 files changed, 504 insertions(+), 1 deletion(-) create mode 100644
tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err
create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml create mode 100644
tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err create mode 100644
tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml create mode 120000
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml
create mode 120000
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml
create mode 120000
tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.x86_64-latest.xml
create mode 120000
tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.x86_64-latest.xml
On Tue, Mar 08, 2022 at 09:57:04PM +0530, Ani Sinha wrote:
On Tue, Mar 8, 2022 at 21:21 Laine Stump <laine@redhat.com> wrote:
Aha! the domain of qemu-devel@nongnu.org was incorrect in the original send (it was "nognu.org"), so none of this thread was making it to that list. I've corrected it in this message, but interested parties from qemu-devel will need to look on the libvir-list archives for the actual patch mails:
https://listman.redhat.com/archives/libvir-list/2022-March/229089.html
Anyone else who responds to any of the mail on this thread should fix the qemu-devel address accordingly.
This patch set has been a true test of my diligence and perseverance 🙂
Right. But could you maybe repost to the proper qemu-devel? I'd like to be able to pull the whole email not just the summary listman deems safe to show me.
On 3/8/22 10:33 AM, Laine Stump wrote: > On 3/8/22 1:39 AM, Ani Sinha wrote: >> Changelog: >> v2 - rebased the patch series to latest master. >> >> I am re-introducing the patchset for <acpi-hotplug-bridge> which got >> reverted here few months back: >> >> https://www.spinics.net/linux/fedora/libvir/msg224089.html >> >> The reason for the reversal was that there seemed to be some >> instability/issues around the use of the qemu commandline which this >> patchset tries to support. In particular, some guest operating systems >> did not like the way QEMU was trying to disable native hotplug on pcie >> root ports. > > My memory isn't completely clear, but I think there was also the issue > that the option claims to enable ACPI hotplug when set to on, but > instead what it actually does (in the Q35 case at least) is to enable > native PCI hotplug when set to off (without actually disabling ACPI > hotplug) and disable native PCI hotplug when set to on, or something > like that. This ends up leaving it up to the guest OS to decide which > type of hotplug to use, meaning its decision could override what's in > the libvirt config, thus confusing everyone. Again, I probably have the > details mixed up, but it was something like this. > > I asked mst about this this morning, and he suggested something that > you've already done - Cc'ing the series to qemu-devel and the relevant > maintainers so we can have a discussion with all involved parties about > their opinions on whether we really should expose this existing option > in libvirt, or if we should instead have two new options that are more > orthogonal about enabling/disabling the two types of hotplug, so that > libvirt config can more accurately represent what is being presented to > the guest rather than a "best guess" of what we think the guest is going > to do with what is presented. > > (Michael did also say that, with the current flurry of bug reports for > the QEMU rc's, this discusion may not happen until closer to release > when the bug reports die down. I know this doesn't mesh with your desire > to "push now to allow for testing" (which in general would be a good > thing if we were certain that we wanted the option like this and were > just expecting some minor bugs that could be fixed), but my opinion is > that 1) it's possible for anyone interested to test the functionality > using <qemu:commandline>, and 2) we should avoid turning libvirt git > into a revolving door of experiments. The only practical difference > between using <qemu:commandline> and having a dedicated option is that > the use of <qemu:commandline> causes the domain to be tainted, and the > XML is a bit more complicated. But since the people we're talking about > here will already have built their own libvirt binaries, the tainted > status of any guests is irrelevant and the extra complexity of using > <qemu:commandline> is probably trivial to them :-). > >> Subsequently, in QEMU 6.2, we have changed our mechanism >> using which we disable native hotplug. As I understand, we do not have >> any reported issues so far in 6.2 around this area. QEMU will enter a >> soft feature freeze in the first week of march in prep for 7.0 release. >> Libvirt is also entering a new release cycle phaze. Hence, I am >> introducing this patchset early enough in the release cycles so that if >> we do see any issues on the qemu side during the rc0, rc1 cycles and if >> reversal of this patchset is again required, it can be done in time >> before the next libvirt release end of March. >> >> All the patches in this series had been previously reviewed. Some >> subsequent fixes were made after my initial patches were pushed. I have >> squashed all those fixes and consolidated them into four patches. I have >> also updated the documentation to reflect the new changes from the QEMU >> side and rebased my changes fixing the tests in the process. >> >> What changed in QEMU post version 6.1 ? >> ========================================= >> >> We have made basically two major changes in QEMU. First is this change: >> >> (1) commit 211afe5c69b597acf85fdd577eb497f5be1ffbd8 >> Author: Julia Suvorova <jusual@redhat.com> >> Date: Fri Nov 12 06:08:56 2021 -0500 >> >> hw/i386/acpi-build: Deny control on PCIe Native Hot-plug in _OSC >> There are two ways to enable ACPI PCI Hot-plug: >> * Disable the Hot-plug Capable bit on PCIe slots. >> This was the first approach which led to regression [1-2], as >> I/O space for a port is allocated only when it is hot-pluggable, >> which is determined by HPC bit. >> * Leave the HPC bit on and disable PCIe Native Hot-plug >> in _OSC >> method. >> This removes the (future) ability of hot-plugging switches with PCIe >> Native hotplug since ACPI PCI Hot-plug only works with cold-plugged >> bridges. If the user wants to explicitely use this feature, they can >> disable ACPI PCI Hot-plug with: >> --global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off >> Change the bit in _OSC method so that the OS selects ACPI PCI >> Hot-plug >> instead of PCIe Native. >> [1] https://gitlab.com/qemu-project/qemu/-/issues/641 >> [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409 >> Signed-off-by: Julia Suvorova <jusual@redhat.com> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >> Message-Id: <20211112110857.3116853-5-imammedo@redhat.com> >> Reviewed-by: Ani Sinha <ani@anisinha.ca> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> >> >> The patch description says it all. Instead of masking out the HPC bit in >> pcie slots, we keep them turned on. Instead, we do not advertize native >> hotplug capability for PCIE using _OSC control method. See section >> 6.2.11 in ACPI spec 6.2. At the same time, we turn on ACPI hotplug for >> these slots so now the guest OS can select ACPI hotplug instead. >> >> The second change is introduction of a property with which we keep the >> existing behavior for pc-q35-6.1 machines. This means HPC bit is masked >> and ACPI hotplug is enabled by default for pcie root ports. >> The QEMU commit is: >> >> (2) commit c318bef76206c2ecb6016e8e68c4ac6ff9a4c8cb >> Author: Julia Suvorova <jusual@redhat.com> >> Date: Fri Nov 12 06:08:54 2021 -0500 >> >> hw/acpi/ich9: Add compat prop to keep HPC bit set for 6.1 machine >> type >> To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be >> turned on, while the switch to ACPI Hot-plug will be done in the >> DSDT table. >> Introducing 'x-keep-native-hpc' property disables the HPC bit only >> in 6.1 and as a result keeps the forced 'reserve-io' on >> pcie-root-ports in 6.1 too. >> [1] https://gitlab.com/qemu-project/qemu/-/issues/641 >> [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409 >> Signed-off-by: Julia Suvorova <jusual@redhat.com> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >> Message-Id: <20211112110857.3116853-3-imammedo@redhat.com> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> >> Lastly, as a related side note, because from QEMU 6.2 onwards, we do not >> mask out HPC bit in PCIE, the work done by this patch is no longer >> needed: >> >> (3) commit e2a6290aab578b2170c1f5909fa556385dc0d820 >> Author: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> >> Date: Mon Aug 2 12:00:57 2021 +0300 >> >> hw/pcie-root-port: Fix hotplug for PCI devices requiring IO >> Q35 has now ACPI hotplug enabled by default for PCI(e) devices. >> As opposed to native PCIe hotplug, guests like Fedora 34 >> will not assign IO range to pcie-root-ports not supporting >> native hotplug, resulting into a regression. >> Reproduce by: >> qemu-bin -M q35 -device pcie-root-port,id=p1 -monitor stdio >> device_add e1000,bus=p1 >> In the Guest OS the respective pcie-root-port will have the IO range >> disabled. >> Fix it by setting the "reserve-io" hint capability of the >> pcie-root-ports so the firmware will allocate the IO range instead. >> Acked-by: Igor Mammedov <imammedo@redhat.com> >> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> >> Message-Id: <20210802090057.1709775-1-marcel@redhat.com> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> >> This is what commit (2) alludes to. In pc-q35-6.1 machines we do need >> patch (3) since we mask out HPC bit from pcie ports. >> >> >> I know this is convoluted mess. In fairness I am trying all I can in my >> spare time to help from the QEMU side. I am determined to see this >> patchset through into libvirt. >> >> Thanks >> >> >> Ani Sinha (4): >> qemu: capablities: detect acpi-pci-hotplug-with-bridge-support >> conf: introduce support for acpi-bridge-hotplug feature >> qemu: command: add support for acpi-bridge-hotplug feature >> NEWS: document new acpi pci hotplug config option >> >> NEWS.rst | 8 ++ >> docs/formatdomain.rst | 32 +++++++ >> docs/schemas/domaincommon.rng | 15 ++++ >> src/conf/domain_conf.c | 89 ++++++++++++++++++- >> src/conf/domain_conf.h | 9 ++ >> src/qemu/qemu_capabilities.c | 4 + >> src/qemu/qemu_capabilities.h | 3 + >> src/qemu/qemu_command.c | 19 ++++ >> src/qemu/qemu_validate.c | 42 +++++++++ >> .../caps_6.1.0.x86_64.xml | 1 + >> .../caps_6.2.0.x86_64.xml | 1 + >> .../caps_7.0.0.x86_64.xml | 1 + >> ...-hotplug-bridge-disable.aarch64-latest.err | 1 + >> .../aarch64-acpi-hotplug-bridge-disable.xml | 13 +++ >> ...-hotplug-bridge-disable.x86_64-latest.args | 35 ++++++++ >> .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 36 ++++++++ >> .../pc-i440fx-acpi-hotplug-bridge-enable.xml | 36 ++++++++ >> ...pi-hotplug-bridge-disable.x86_64-6.0.0.err | 1 + >> ...-hotplug-bridge-disable.x86_64-latest.args | 38 ++++++++ >> .../q35-acpi-hotplug-bridge-disable.xml | 53 +++++++++++ >> .../q35-acpi-hotplug-bridge-enable.xml | 53 +++++++++++ >> tests/qemuxml2argvtest.c | 7 ++ >> ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + >> ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + >> ...i-hotplug-bridge-disable.x86_64-latest.xml | 1 + >> ...pi-hotplug-bridge-enable.x86_64-latest.xml | 1 + >> tests/qemuxml2xmltest.c | 4 + >> 27 files changed, 504 insertions(+), 1 deletion(-) >> create mode 100644 >> tests/qemuxml2argvdata/ aarch64-acpi-hotplug-bridge-disable.aarch64-latest.err >> >> create mode 100644 >> tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml >> create mode 100644 >> tests/qemuxml2argvdata/ pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.args >> >> create mode 100644 >> tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml >> create mode 100644 >> tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml >> create mode 100644 >> tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.x86_64-6.0.0.err >> create mode 100644 >> tests/qemuxml2argvdata/ q35-acpi-hotplug-bridge-disable.x86_64-latest.args >> create mode 100644 >> tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml >> create mode 100644 >> tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml >> create mode 120000 >> tests/qemuxml2xmloutdata/ pc-i440fx-acpi-hotplug-bridge-disable.x86_64-latest.xml >> >> create mode 120000 >> tests/qemuxml2xmloutdata/ pc-i440fx-acpi-hotplug-bridge-enable.x86_64-latest.xml >> >> create mode 120000 >> tests/qemuxml2xmloutdata/ q35-acpi-hotplug-bridge-disable.x86_64-latest.xml >> >> create mode 120000 >> tests/qemuxml2xmloutdata/ q35-acpi-hotplug-bridge-enable.x86_64-latest.xml >> >
On Tue, 8 Mar 2022, Laine Stump wrote:
Aha! the domain of qemu-devel@nongnu.org was incorrect in the original send (it was "nognu.org"), so none of this thread was making it to that list.
Not to give any excuses but this happened because on Qemu side I never have to type this manually. My git config is set up so that the cc in send-email is filled up automatically using scripts/get_maintainer.pl. On libvirt side also the domain and mailing list is easy to remember. Its only when I have to manually type stuff that shit happens :-)
On Tue, Mar 08, 2022 at 10:15:11PM +0530, Ani Sinha wrote:
On Tue, 8 Mar 2022, Laine Stump wrote:
Aha! the domain of qemu-devel@nongnu.org was incorrect in the original send (it was "nognu.org"), so none of this thread was making it to that list.
Not to give any excuses but this happened because on Qemu side I never have to type this manually. My git config is set up so that the cc in send-email is filled up automatically using scripts/get_maintainer.pl. On libvirt side also the domain and mailing list is easy to remember. Its only when I have to manually type stuff that shit happens :-)
Donnu about alpine, but with mutt you can easily set up and alias and then it expands for you. -- MST
On Tue, Mar 8, 2022 at 10:17 PM Michael S. Tsirkin <mst@redhat.com> wrote:
On Tue, Mar 08, 2022 at 10:15:11PM +0530, Ani Sinha wrote:
On Tue, 8 Mar 2022, Laine Stump wrote:
Aha! the domain of qemu-devel@nongnu.org was incorrect in the original send (it was "nognu.org"), so none of this thread was making it to that list.
Not to give any excuses but this happened because on Qemu side I never have to type this manually. My git config is set up so that the cc in send-email is filled up automatically using scripts/get_maintainer.pl. On libvirt side also the domain and mailing list is easy to remember. Its only when I have to manually type stuff that shit happens :-)
Donnu about alpine, but with mutt you can easily set up and alias and then it expands for you.
I use alpine to only reply/review patches. I use git send-email to actually send the patch. There I am not sure the best way to avoid manually typing in the mailing list address.
On Tue, Mar 08, 2022 at 10:23:20PM +0530, Ani Sinha wrote:
On Tue, Mar 8, 2022 at 10:17 PM Michael S. Tsirkin <mst@redhat.com> wrote:
On Tue, Mar 08, 2022 at 10:15:11PM +0530, Ani Sinha wrote:
On Tue, 8 Mar 2022, Laine Stump wrote:
Aha! the domain of qemu-devel@nongnu.org was incorrect in the original send (it was "nognu.org"), so none of this thread was making it to that list.
Not to give any excuses but this happened because on Qemu side I never have to type this manually. My git config is set up so that the cc in send-email is filled up automatically using scripts/get_maintainer.pl. On libvirt side also the domain and mailing list is easy to remember. Its only when I have to manually type stuff that shit happens :-)
Donnu about alpine, but with mutt you can easily set up and alias and then it expands for you.
I use alpine to only reply/review patches. I use git send-email to actually send the patch. There I am not sure the best way to avoid manually typing in the mailing list address.
send-email supports aliases too. -- MST
On Tue, 8 Mar 2022, Michael S. Tsirkin wrote:
On Tue, Mar 08, 2022 at 10:23:20PM +0530, Ani Sinha wrote:
On Tue, Mar 8, 2022 at 10:17 PM Michael S. Tsirkin <mst@redhat.com> wrote:
On Tue, Mar 08, 2022 at 10:15:11PM +0530, Ani Sinha wrote:
On Tue, 8 Mar 2022, Laine Stump wrote:
Aha! the domain of qemu-devel@nongnu.org was incorrect in the original send (it was "nognu.org"), so none of this thread was making it to that list.
Not to give any excuses but this happened because on Qemu side I never have to type this manually. My git config is set up so that the cc in send-email is filled up automatically using scripts/get_maintainer.pl. On libvirt side also the domain and mailing list is easy to remember. Its only when I have to manually type stuff that shit happens :-)
Donnu about alpine, but with mutt you can easily set up and alias and then it expands for you.
I use alpine to only reply/review patches. I use git send-email to actually send the patch. There I am not sure the best way to avoid manually typing in the mailing list address.
send-email supports aliases too.
Ah cool. I just set this up with some help from https://felipec.wordpress.com/2009/10/25/git-send-email-tricks/ . Now I can simply say $ git send-email --to=qemu-list <patch> without worrying about typo :-) Thanks for the pointer.
On Wed, Mar 09, 2022 at 12:47:26PM +0530, Ani Sinha wrote:
On Tue, 8 Mar 2022, Michael S. Tsirkin wrote:
On Tue, Mar 08, 2022 at 10:23:20PM +0530, Ani Sinha wrote:
On Tue, Mar 8, 2022 at 10:17 PM Michael S. Tsirkin <mst@redhat.com> wrote:
On Tue, Mar 08, 2022 at 10:15:11PM +0530, Ani Sinha wrote:
On Tue, 8 Mar 2022, Laine Stump wrote:
Aha! the domain of qemu-devel@nongnu.org was incorrect in the original send (it was "nognu.org"), so none of this thread was making it to that list.
Not to give any excuses but this happened because on Qemu side I never have to type this manually. My git config is set up so that the cc in send-email is filled up automatically using scripts/get_maintainer.pl. On libvirt side also the domain and mailing list is easy to remember. Its only when I have to manually type stuff that shit happens :-)
Donnu about alpine, but with mutt you can easily set up and alias and then it expands for you.
I use alpine to only reply/review patches. I use git send-email to actually send the patch. There I am not sure the best way to avoid manually typing in the mailing list address.
send-email supports aliases too.
Ah cool. I just set this up with some help from https://felipec.wordpress.com/2009/10/25/git-send-email-tricks/ . Now I can simply say
$ git send-email --to=qemu-list <patch>
without worrying about typo :-) Thanks for the pointer.
So, in context of sending patches to mailing lists, especially with libvirt and QEMU, you could utilize 'git publish'. Both QEMU and libvirt have a .gitpublish config in the repo which sets some git-send email options, especially the QEMU's one is nice listing a bunch of common profile templates as well. I'd say that if you're contributing to a project which doesn't have a .gitpublish config yet, then rather than not using 'git publish', the project should adopt the config instead :). Regards, Erik
participants (4)
-
Ani Sinha -
Erik Skultety -
Laine Stump -
Michael S. Tsirkin