[PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options

Ani Sinha posted 5 patches 4 years, 5 months ago
[PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Ani Sinha 4 years, 5 months ago
This change introduces libvirt xml support for the following two pm options:

<pm>
  <acpi-hotplug-bridge enabled='no'/>
  <acpi-root-hotplug enabled='yes'/>
</pm>

The above two options are only available for qemu driver and that too for x86
guests only. Both of them are global options.

'acpi-hotplug-bridge' option enables or disables ACPI hotplug support for cold
plugged bridges. Examples of cold plugged bridges include PCI-PCI bridge
(pci-bridge controller) for pc machines and pcie-root-port controller for q35
machines. Being global options, no other bridge specific options for pci-bridge
controller or pcie-root-port controllers are required. For pc machine type in
x86, this option is available in qemu for a long time, from version 2.1.
Please see the changes in qemu.git:

9e047b982452c6 ("piix4: add acpi pci hotplug support")
133a2da488062e ("pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled")

For q35 machine type, this was introduced in qemu 6.1 with the following
changes in qemu.git:

(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). It is possible that some users
might still want to use native hotplug on PCIe. Therefore, this config option
enables users to choose either ACPI based hotplug or native hotplug for cold
plugged bridges (for example for pcie root port controller in q35 machines).

'acpi-root-hotplug' option enables or disables ACPI based hotplug for PCI root
bus (pci-root controller). This option is only available for pc machine type.
This additional option enables users to disable hotplug for all devices in the
system without adding an additional PCI-PCI bridge, putting the devices behind
the bridge and using the existing "acpi-hotplug-bridge" option to disable
hotplug on that bridge. This feature was introduced from qemu version 5.2 with
the following change in qemu.git:

3d7e78aa7777f ("Introduce a new flag for i440fx to disable PCI hotplug on the root bus")

The above qemu commit describes some compelling reasons why users might to
disable hotplug on PCI root buses.

This change also adds related unit tests to exercise the new conf options.

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 docs/formatdomain.rst                         | 36 ++++++++++++---
 docs/schemas/domaincommon.rng                 | 17 +++++++
 src/conf/domain_conf.c                        | 21 ++++++++-
 src/conf/domain_conf.h                        |  2 +
 .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 17 +++++++
 .../pc-i440fx-acpi-root-hotplug-disable.xml   | 17 +++++++
 .../q35-acpi-hotplug-bridge-disable.xml       | 17 +++++++
 .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 31 +++++++++++++
 .../pc-i440fx-acpi-root-hotplug-disable.xml   | 31 +++++++++++++
 .../q35-acpi-hotplug-bridge-disable.xml       | 45 +++++++++++++++++++
 tests/qemuxml2xmltest.c                       |  9 ++++
 11 files changed, 236 insertions(+), 7 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
 create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml
 create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml
 create mode 100644 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
 create mode 100644 tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml
 create mode 100644 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index ad3b4ea92c..c8b04c625c 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1793,16 +1793,40 @@ advertisements to the guest OS. (NB: Only qemu driver support)
    <pm>
      <suspend-to-disk enabled='no'/>
      <suspend-to-mem enabled='yes'/>
+     <acpi-hotplug-bridge enabled='no'/>
+     <acpi-root-hotplug enabled='yes'/>
    </pm>
    ...
 
 ``pm``
-   These elements enable ('yes') or disable ('no') BIOS support for S3
-   (suspend-to-mem) and S4 (suspend-to-disk) ACPI sleep states. If nothing is
-   specified, then the hypervisor will be left with its default value.
-   Note: This setting cannot prevent the guest OS from performing a suspend as
-   the guest OS itself can choose to circumvent the unavailability of the sleep
-   states (e.g. S4 by turning off completely).
+   These elements enable ('yes') or disable ('no') certain BIOS advertisements.
+   If nothing is specified, then the hypervisor will be left with its default value.
+   The following BIOS options are available:
+
+``suspend-to-mem``
+   support for S3 (suspend-to-mem) ACPI sleep states.
+``suspend-to-disk``
+   support for S4 (suspend-to-disk) ACPI sleep states.
+
+   Note that for the above two options, the setting cannot prevent the guest OS
+   from performing a suspend as the guest OS itself can choose to circumvent the
+   unavailability of the sleep states (e.g. S4 by turning off completely).
+
+``acpi-hotplug-bridge``
+   :since:`Since 7.8.0` This option enables or disables BIOS ACPI based hotplug support
+   for cold plugged bridges. It is available only for x86 guests, both for q35 and pc
+   machine types. For pc machines, the support is available from `QEMU 2.12`. For q35
+   machines, the support is available from `QEMU 6.1`. Examples of cold plugged bridges
+   include PCI-PCI bridges for pc machine types (pci-bridge controller). For q35 machines,
+   it includes PCIE root ports (pcie-root-port controller). This is a global option that
+   affects all bridges. No other bridge specific option is required to be specified.
+
+``acpi-root-hotplug``
+   :since:`Since 7.8.0 (QEMU 5.2)` This option enables or disables BIOS ACPI based
+   hotplug support for PCI root bus (pci-root controller). This is available only
+   for x86 guests, only for pc machine type. For q35 machines, PCIE root complex,
+   the pcie-root controller, does not support hotplug capability. This is also a global
+   option.
 
 :anchor:`<a id="elementsFeatures"/>`
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 11fa24f398..a51efb933d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4333,6 +4333,16 @@
             <ref name="suspendChoices"/>
           </element>
         </optional>
+        <optional>
+          <element name="acpi-hotplug-bridge">
+            <ref name="acpiHotplugChoices"/>
+          </element>
+        </optional>
+        <optional>
+          <element name="acpi-root-hotplug">
+            <ref name="acpiHotplugChoices"/>
+          </element>
+        </optional>
       </interleave>
       <empty/>
     </element>
@@ -4344,6 +4354,13 @@
       </attribute>
     </optional>
   </define>
+  <define name="acpiHotplugChoices">
+    <optional>
+      <attribute name="enabled">
+        <ref name="virYesNo"/>
+      </attribute>
+    </optional>
+  </define>
   <!--
       Specific setup for a qemu emulated character device.  Note: this
       definition doesn't fully specify the constraints on this node.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cb9e7218ff..e48661534f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19339,6 +19339,16 @@ virDomainDefLifecycleParse(virDomainDef *def,
                                  &def->pm.s4) < 0)
         goto error;
 
+    if (virDomainPMStateParseXML(ctxt,
+                                 "string(./pm/acpi-hotplug-bridge/@enabled)",
+                                 &def->pm.acpi_hp_bridge) < 0)
+        goto error;
+
+    if (virDomainPMStateParseXML(ctxt,
+                                 "string(./pm/acpi-root-hotplug/@enabled)",
+                                 &def->pm.acpi_root_hp) < 0)
+        goto error;
+
     return 0;
 
  error:
@@ -28151,7 +28161,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
                                       virDomainLockFailureTypeToString) < 0)
         return -1;
 
-    if (def->pm.s3 || def->pm.s4) {
+    if (def->pm.s3 || def->pm.s4 || def->pm.acpi_hp_bridge ||
+        def->pm.acpi_root_hp) {
         virBufferAddLit(buf, "<pm>\n");
         virBufferAdjustIndent(buf, 2);
         if (def->pm.s3) {
@@ -28162,6 +28173,14 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
             virBufferAsprintf(buf, "<suspend-to-disk enabled='%s'/>\n",
                               virTristateBoolTypeToString(def->pm.s4));
         }
+        if (def->pm.acpi_hp_bridge) {
+            virBufferAsprintf(buf, "<acpi-hotplug-bridge enabled='%s'/>\n",
+                              virTristateBoolTypeToString(def->pm.acpi_hp_bridge));
+        }
+        if (def->pm.acpi_root_hp) {
+            virBufferAsprintf(buf, "<acpi-root-hotplug enabled='%s'/>\n",
+                              virTristateBoolTypeToString(def->pm.acpi_root_hp));
+        }
         virBufferAdjustIndent(buf, -2);
         virBufferAddLit(buf, "</pm>\n");
     }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c7e6df7981..863bdf54f8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2644,6 +2644,8 @@ struct _virDomainPowerManagement {
     /* These options are of type enum virTristateBool */
     int s3;
     int s4;
+    int acpi_hp_bridge;
+    int acpi_root_hp;
 };
 
 struct _virDomainPerfDef {
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..b96caa9b2d
--- /dev/null
+++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
@@ -0,0 +1,17 @@
+<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>
+  <pm>
+    <acpi-hotplug-bridge enabled='no'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml
new file mode 100644
index 0000000000..ef563200bf
--- /dev/null
+++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.xml
@@ -0,0 +1,17 @@
+<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>
+  <pm>
+    <acpi-root-hotplug enabled='no'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+  </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..bb9e8d4ee2
--- /dev/null
+++ b/tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml
@@ -0,0 +1,17 @@
+<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>
+  <pm>
+    <acpi-hotplug-bridge enabled='no'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
new file mode 100644
index 0000000000..2534dc7e35
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
@@ -0,0 +1,31 @@
+<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>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <pm>
+    <acpi-hotplug-bridge enabled='no'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0'>
+      <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/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml
new file mode 100644
index 0000000000..0ee404ee46
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/pc-i440fx-acpi-root-hotplug-disable.xml
@@ -0,0 +1,31 @@
+<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>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <pm>
+    <acpi-root-hotplug enabled='no'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0'>
+      <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/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml b/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
new file mode 100644
index 0000000000..e8c6f82145
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
@@ -0,0 +1,45 @@
+<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>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <pm>
+    <acpi-hotplug-bridge enabled='no'/>
+  </pm>
+  <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>
+    <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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 6d3526f91f..cbf2306e05 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -425,6 +425,15 @@ mymain(void)
     DO_TEST_NOCAPS("input-usbtablet");
     DO_TEST_NOCAPS("misc-acpi");
     DO_TEST("misc-disable-s3", QEMU_CAPS_PIIX_DISABLE_S3);
+    DO_TEST("pc-i440fx-acpi-hotplug-bridge-disable",
+            QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE);
+    DO_TEST("q35-acpi-hotplug-bridge-disable",
+            QEMU_CAPS_DEVICE_PCI_BRIDGE,
+            QEMU_CAPS_DEVICE_IOH3420,
+            QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+            QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE);
+    DO_TEST("pc-i440fx-acpi-root-hotplug-disable",
+            QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG);
     DO_TEST("misc-disable-suspends",
             QEMU_CAPS_PIIX_DISABLE_S3,
             QEMU_CAPS_PIIX_DISABLE_S4);
-- 
2.25.1

Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Daniel P. Berrangé 4 years, 4 months ago
On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
> This change introduces libvirt xml support for the following two pm options:
> 
> <pm>
>   <acpi-hotplug-bridge enabled='no'/>
>   <acpi-root-hotplug enabled='yes'/>
> </pm>


> +``acpi-hotplug-bridge``
> +   :since:`Since 7.8.0` This option enables or disables BIOS ACPI based hotplug support
> +   for cold plugged bridges. It is available only for x86 guests, both for q35 and pc
> +   machine types. For pc machines, the support is available from `QEMU 2.12`. For q35
> +   machines, the support is available from `QEMU 6.1`. Examples of cold plugged bridges
> +   include PCI-PCI bridges for pc machine types (pci-bridge controller). For q35 machines,
> +   it includes PCIE root ports (pcie-root-port controller). This is a global option that
> +   affects all bridges. No other bridge specific option is required to be specified.

Can you confirm my understanding of the situation..

 - i440fx / PCI topology - hotplug always uses ACPI

 - q35 / PCIe topology - hotplug historically used native PCIe hotplug,
                         but in 6.1 switched to ACPI

Given, the name "acpi-hotplug-bridge",  am I right that this option
has *no* effect, if the q35 machine is using native PCIe hotplug
approach ?  IOW, is it a no-op until 6.1 based machine types for q35 ?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Ani Sinha 4 years, 4 months ago

On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:

> On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
> > This change introduces libvirt xml support for the following two pm options:
> >
> > <pm>
> >   <acpi-hotplug-bridge enabled='no'/>
> >   <acpi-root-hotplug enabled='yes'/>
> > </pm>
>
>
> > +``acpi-hotplug-bridge``
> > +   :since:`Since 7.8.0` This option enables or disables BIOS ACPI based hotplug support
> > +   for cold plugged bridges. It is available only for x86 guests, both for q35 and pc
> > +   machine types. For pc machines, the support is available from `QEMU 2.12`. For q35
> > +   machines, the support is available from `QEMU 6.1`. Examples of cold plugged bridges
> > +   include PCI-PCI bridges for pc machine types (pci-bridge controller). For q35 machines,
> > +   it includes PCIE root ports (pcie-root-port controller). This is a global option that
> > +   affects all bridges. No other bridge specific option is required to be specified.
>
> Can you confirm my understanding of the situation..
>
>  - i440fx / PCI topology - hotplug always uses ACPI
>

ACPI is the primary means of enabling hotplug. shpc might also have a role
here but I think it is disabled. Igor (cc'd) might throw some lights on
how shpc comes to play.

>  - q35 / PCIe topology - hotplug historically used native PCIe hotplug,
>                          but in 6.1 switched to ACPI
>

Correct.

> Given, the name "acpi-hotplug-bridge",  am I right that this option
> has *no* effect, if the q35 machine is using native PCIe hotplug
> approach ?

Its complicated.
With "acpi-hotplug-bridge" ON, native hotplug is disabled in qemu.
With "acpi-hotplug-bridge" OFF, native hotplug is enabled in qemu.

Libvirt does not allow it, but by directly using qemu commandline it is
possible to enable ACPI hotplug for pcie-root-ports by turning
"acpi-hotplug-bridge" ON _and_ enable native hotplug at the same time. In
that case, as Julia has mentioned in some other thread, the guest OS
always choses native over ACPI based hotplug.

The <target hotplug="on/off"> switch in libvirt for pcie-root-ports
currently does not care whether native or acpi hotplug is used. It simply
turns on the hotplug for that particular port. Whether ACPI or native is
used is controlled by this global flag that Julia has introduced in 6.1.

> IOW, is it a no-op until 6.1 based machine types for q35 ?
>

Correct. For q35 this command line did not exist until 6.1.
Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Daniel P. Berrangé 4 years, 4 months ago
On Tue, Sep 28, 2021 at 02:35:47PM +0530, Ani Sinha wrote:
> 
> 
> On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
> 
> > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
> > > This change introduces libvirt xml support for the following two pm options:
> > >
> > > <pm>
> > >   <acpi-hotplug-bridge enabled='no'/>
> > >   <acpi-root-hotplug enabled='yes'/>
> > > </pm>
> >
> >
> > > +``acpi-hotplug-bridge``
> > > +   :since:`Since 7.8.0` This option enables or disables BIOS ACPI based hotplug support
> > > +   for cold plugged bridges. It is available only for x86 guests, both for q35 and pc
> > > +   machine types. For pc machines, the support is available from `QEMU 2.12`. For q35
> > > +   machines, the support is available from `QEMU 6.1`. Examples of cold plugged bridges
> > > +   include PCI-PCI bridges for pc machine types (pci-bridge controller). For q35 machines,
> > > +   it includes PCIE root ports (pcie-root-port controller). This is a global option that
> > > +   affects all bridges. No other bridge specific option is required to be specified.
> >
> > Can you confirm my understanding of the situation..
> >
> >  - i440fx / PCI topology - hotplug always uses ACPI
> >
> 
> ACPI is the primary means of enabling hotplug. shpc might also have a role
> here but I think it is disabled. Igor (cc'd) might throw some lights on
> how shpc comes to play.

Yes, I think it will be important to understand if 'shpc' becomes relevant
when ACPI hotplug is disabled for PCI

> 
> >  - q35 / PCIe topology - hotplug historically used native PCIe hotplug,
> >                          but in 6.1 switched to ACPI
> >
> 
> Correct.
> 
> > Given, the name "acpi-hotplug-bridge",  am I right that this option
> > has *no* effect, if the q35 machine is using native PCIe hotplug
> > approach ?
> 
> Its complicated.
> With "acpi-hotplug-bridge" ON, native hotplug is disabled in qemu.
> With "acpi-hotplug-bridge" OFF, native hotplug is enabled in qemu.

Oh, I mis-read and didn't realize this was controlling the QEMU
"acpi-pci-hotplug-with-bridge-support" configuration.

With this in mind I think the naming is somewhat misleading. Setting it
to off would give users the impression that hotplug is disabled, which
is not the case for Q35 at least. It is just switching to a different
hotplug implementation.

At least from Q35 pov, I think it would be better to call it

    hotplug-mode="acpi|pcie"

so it is clear that no matter what value it is set to, hotplug
is still available.

If we also consider PIIX, then depending on the answer wrt shpc
above, we might want one of

    hotplug-mode="acpi|pcie|none"
    hotplug-mode="acpi|pcie|shpc"


> Libvirt does not allow it, but by directly using qemu commandline it is
> possible to enable ACPI hotplug for pcie-root-ports by turning
> "acpi-hotplug-bridge" ON _and_ enable native hotplug at the same time. In
> that case, as Julia has mentioned in some other thread, the guest OS
> always choses native over ACPI based hotplug.

I see that is refering to pie-root-port gaining  native_hotplug=bool in 6.1

You say the guest OS always chooses native over ACPI - is that reqiired
in the spec, or is that just how some common guest OS have choosen to
impl it today ?


> The <target hotplug="on/off"> switch in libvirt for pcie-root-ports
> currently does not care whether native or acpi hotplug is used. It simply
> turns on the hotplug for that particular port. Whether ACPI or native is
> used is controlled by this global flag that Julia has introduced in 6.1.

Right so we have

  * PIIX4

      - acpi-root-pci-hotplug=bool

        Whether hotplug is enabled for the root bridge or not

          <target hotplug="on/off"> for pci-root controller


      - acpi-pci-hotplug-with-bridge-support=bool

        Toggles support for ACPI based hotplug across all bridges.
	If disabled will there will be no hotplug at all for PIIX4 ?
	Or does 'shpc' come into play in that scenario ?


   PIIX combinations

       (1) acpi-root-pci-hotplug=yes
           acpi-pci-hotplug-with-bridge-support=yes

         - All bridges have hotplug

       (2) acpi-root-pci-hotplug=yes
           acpi-pci-hotplug-with-bridge-support=no

         - No bridges have hotplug

       (3) acpi-root-pci-hotplug=no
           acpi-pci-hotplug-with-bridge-support=yes

         - All bridges except root have hotplug

       (4) acpi-root-pci-hotplug=no
           acpi-pci-hotplug-with-bridge-support=no

         - No bridges have hotplug. Essentially identical to (2)


  * Q35


      - acpi-pci-hotplug-with-bridge-support=bool

        Toggles support for ACPI based hotplug. If disabled native
	PCIe hotplug is activated instead


  * pcie-root-port

      - hotplug=bool

        Toggles whether hotplug is enabled on the port. Affects
	either native and ACPI based hotplug, depending on what
	acpi-pci-hotplug-with-bridge-support=bool is set to ?

          <target hotplug="on/off"> for pcie-root-port controller

      - native_hotplug=bool

        Can be used to also enable native hotplug, even when ACPI
	based hotplug is globally enabled. IOW only really relevant
	when acpi-pci-hotplug-with-bridge-support=true


   Q35 combinations

       (1) acpi-pci-hotplug-with-bridge-support=yes
           pcie-root-port.hotplug=yes
           pcie-root-port.native_hotplug=yes

           ports have both ACPI + native hotplug, guest prefers native


       (2) acpi-pci-hotplug-with-bridge-support=yes
           pcie-root-port.hotplug=no
           pcie-root-port.native_hotplug=yes

           ports have no hotplug, despite native_hotplug=yes IIUC


       (3) acpi-pci-hotplug-with-bridge-support=yes
           pcie-root-port.hotplug=yes
           pcie-root-port.native_hotplug=no

           ports have ACPI hotplug only


       (4) acpi-pci-hotplug-with-bridge-support=yes
           pcie-root-port.hotplug=no
           pcie-root-port.native_hotplug=no

           ports have no hotplug


       (5) acpi-pci-hotplug-with-bridge-support=no
           pcie-root-port.hotplug=yes
           pcie-root-port.native_hotplug=yes

           ports have native hotplug


       (6) acpi-pci-hotplug-with-bridge-support=no
           pcie-root-port.hotplug=no
           pcie-root-port.native_hotplug=yes

           ports have no hotplug, despite native_hotplug=yes IIUC           


       (7) acpi-pci-hotplug-with-bridge-support=no
           pcie-root-port.hotplug=yes
           pcie-root-port.native_hotplug=no

           ports have no hotplug, despite hotplug=yes IIUC


       (8) acpi-pci-hotplug-with-bridge-support=no
           pcie-root-port.hotplug=no
           pcie-root-port.native_hotplug=no

           ports have no hotplug



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Ani Sinha 4 years, 4 months ago

On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:

> On Tue, Sep 28, 2021 at 02:35:47PM +0530, Ani Sinha wrote:
> >
> >
> > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
> >
> > > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
> > > > This change introduces libvirt xml support for the following two pm options:
> > > >
> > > > <pm>
> > > >   <acpi-hotplug-bridge enabled='no'/>
> > > >   <acpi-root-hotplug enabled='yes'/>
> > > > </pm>
> > >
> > >
> > > > +``acpi-hotplug-bridge``
> > > > +   :since:`Since 7.8.0` This option enables or disables BIOS ACPI based hotplug support
> > > > +   for cold plugged bridges. It is available only for x86 guests, both for q35 and pc
> > > > +   machine types. For pc machines, the support is available from `QEMU 2.12`. For q35
> > > > +   machines, the support is available from `QEMU 6.1`. Examples of cold plugged bridges
> > > > +   include PCI-PCI bridges for pc machine types (pci-bridge controller). For q35 machines,
> > > > +   it includes PCIE root ports (pcie-root-port controller). This is a global option that
> > > > +   affects all bridges. No other bridge specific option is required to be specified.
> > >
> > > Can you confirm my understanding of the situation..
> > >
> > >  - i440fx / PCI topology - hotplug always uses ACPI
> > >
> >
> > ACPI is the primary means of enabling hotplug. shpc might also have a role
> > here but I think it is disabled. Igor (cc'd) might throw some lights on
> > how shpc comes to play.
>
> Yes, I think it will be important to understand if 'shpc' becomes relevant
> when ACPI hotplug is disabled for PCI
>
> >
> > >  - q35 / PCIe topology - hotplug historically used native PCIe hotplug,
> > >                          but in 6.1 switched to ACPI
> > >
> >
> > Correct.
> >
> > > Given, the name "acpi-hotplug-bridge",  am I right that this option
> > > has *no* effect, if the q35 machine is using native PCIe hotplug
> > > approach ?
> >
> > Its complicated.
> > With "acpi-hotplug-bridge" ON, native hotplug is disabled in qemu.
> > With "acpi-hotplug-bridge" OFF, native hotplug is enabled in qemu.
>
> Oh, I mis-read and didn't realize this was controlling the QEMU
> "acpi-pci-hotplug-with-bridge-support" configuration.
>
> With this in mind I think the naming is somewhat misleading. Setting it
> to off would give users the impression that hotplug is disabled, which
> is not the case for Q35 at least. It is just switching to a different
> hotplug implementation.
>
> At least from Q35 pov, I think it would be better to call it
>
>     hotplug-mode="acpi|pcie"
>
> so it is clear that no matter what value it is set to, hotplug
> is still available.
>
> If we also consider PIIX, then depending on the answer wrt shpc
> above, we might want one of
>
>     hotplug-mode="acpi|pcie|none"
>     hotplug-mode="acpi|pcie|shpc"
>

If libvirt does not deal with shpc today I think we should not bother with
shpc at all. We should simply have a boolean mode appropriately named that
choses between acpi hotplug vs native.

>
> > Libvirt does not allow it, but by directly using qemu commandline it is
> > possible to enable ACPI hotplug for pcie-root-ports by turning
> > "acpi-hotplug-bridge" ON _and_ enable native hotplug at the same time. In
> > that case, as Julia has mentioned in some other thread, the guest OS
> > always choses native over ACPI based hotplug.
>
> I see that is refering to pie-root-port gaining  native_hotplug=bool in 6.1
>
> You say the guest OS always chooses native over ACPI - is that reqiired
> in the spec, or is that just how some common guest OS have choosen to
> impl it today ?
>

I am not sure but I *think* this is guest OS dependent too? Julia to
rescue here :-)

>
> > The <target hotplug="on/off"> switch in libvirt for pcie-root-ports
> > currently does not care whether native or acpi hotplug is used. It simply
> > turns on the hotplug for that particular port. Whether ACPI or native is
> > used is controlled by this global flag that Julia has introduced in 6.1.
>
> Right so we have
>
>   * PIIX4
>
>       - acpi-root-pci-hotplug=bool
>
>         Whether hotplug is enabled for the root bridge or not
>
>           <target hotplug="on/off"> for pci-root controller
>
>
>       - acpi-pci-hotplug-with-bridge-support=bool
>
>         Toggles support for ACPI based hotplug across all bridges.
> 	If disabled will there will be no hotplug at all for PIIX4 ?
> 	Or does 'shpc' come into play in that scenario ?
>
>
>    PIIX combinations
>
>        (1) acpi-root-pci-hotplug=yes
>            acpi-pci-hotplug-with-bridge-support=yes
>
>          - All bridges have hotplug
>
>        (2) acpi-root-pci-hotplug=yes
>            acpi-pci-hotplug-with-bridge-support=no
>
>          - No bridges have hotplug
>
>        (3) acpi-root-pci-hotplug=no
>            acpi-pci-hotplug-with-bridge-support=yes
>
>          - All bridges except root have hotplug
>
>        (4) acpi-root-pci-hotplug=no
>            acpi-pci-hotplug-with-bridge-support=no
>
>          - No bridges have hotplug. Essentially identical to (2)

no (4) is not identical to (2). In (4) no hotplug is enabled. In (2) pci
root bus still has hotplug enabled.


>
>   * Q35
>
>
>       - acpi-pci-hotplug-with-bridge-support=bool
>
>         Toggles support for ACPI based hotplug. If disabled native
> 	PCIe hotplug is activated instead
>
>
>   * pcie-root-port
>
>       - hotplug=bool
>
>         Toggles whether hotplug is enabled on the port. Affects
> 	either native and ACPI based hotplug, depending on what
> 	acpi-pci-hotplug-with-bridge-support=bool is set to ?
>
>           <target hotplug="on/off"> for pcie-root-port controller
>
>       - native_hotplug=bool
>
>         Can be used to also enable native hotplug, even when ACPI
> 	based hotplug is globally enabled. IOW only really relevant
> 	when acpi-pci-hotplug-with-bridge-support=true
>
>
>    Q35 combinations
>
>        (1) acpi-pci-hotplug-with-bridge-support=yes
>            pcie-root-port.hotplug=yes
>            pcie-root-port.native_hotplug=yes
>
>            ports have both ACPI + native hotplug, guest prefers native
>
>
>        (2) acpi-pci-hotplug-with-bridge-support=yes
>            pcie-root-port.hotplug=no
>            pcie-root-port.native_hotplug=yes
>
>            ports have no hotplug, despite native_hotplug=yes IIUC
>
>
>        (3) acpi-pci-hotplug-with-bridge-support=yes
>            pcie-root-port.hotplug=yes
>            pcie-root-port.native_hotplug=no
>
>            ports have ACPI hotplug only
>
>
>        (4) acpi-pci-hotplug-with-bridge-support=yes
>            pcie-root-port.hotplug=no
>            pcie-root-port.native_hotplug=no
>
>            ports have no hotplug
>
>
>        (5) acpi-pci-hotplug-with-bridge-support=no
>            pcie-root-port.hotplug=yes
>            pcie-root-port.native_hotplug=yes
>
>            ports have native hotplug
>
>
>        (6) acpi-pci-hotplug-with-bridge-support=no
>            pcie-root-port.hotplug=no
>            pcie-root-port.native_hotplug=yes
>
>            ports have no hotplug, despite native_hotplug=yes IIUC
>
>
>        (7) acpi-pci-hotplug-with-bridge-support=no
>            pcie-root-port.hotplug=yes
>            pcie-root-port.native_hotplug=no
>
>            ports have no hotplug, despite hotplug=yes IIUC
>
>
>        (8) acpi-pci-hotplug-with-bridge-support=no
>            pcie-root-port.hotplug=no
>            pcie-root-port.native_hotplug=no
>
>            ports have no hotplug
>
>
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
>
Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Ani Sinha 4 years, 4 months ago
On Tue, Sep 28, 2021 at 3:28 PM Ani Sinha <ani@anisinha.ca> wrote:
>
>
>
> On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
>
> > On Tue, Sep 28, 2021 at 02:35:47PM +0530, Ani Sinha wrote:
> > >
> > >
> > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote:
> > >
> > > > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
> > > > > This change introduces libvirt xml support for the following two pm options:
> > > > >
> > > > > <pm>
> > > > >   <acpi-hotplug-bridge enabled='no'/>
> > > > >   <acpi-root-hotplug enabled='yes'/>
> > > > > </pm>
> > > >
> > > >
> > > > > +``acpi-hotplug-bridge``
> > > > > +   :since:`Since 7.8.0` This option enables or disables BIOS ACPI based hotplug support
> > > > > +   for cold plugged bridges. It is available only for x86 guests, both for q35 and pc
> > > > > +   machine types. For pc machines, the support is available from `QEMU 2.12`. For q35
> > > > > +   machines, the support is available from `QEMU 6.1`. Examples of cold plugged bridges
> > > > > +   include PCI-PCI bridges for pc machine types (pci-bridge controller). For q35 machines,
> > > > > +   it includes PCIE root ports (pcie-root-port controller). This is a global option that
> > > > > +   affects all bridges. No other bridge specific option is required to be specified.
> > > >
> > > > Can you confirm my understanding of the situation..
> > > >
> > > >  - i440fx / PCI topology - hotplug always uses ACPI
> > > >
> > >
> > > ACPI is the primary means of enabling hotplug. shpc might also have a role
> > > here but I think it is disabled. Igor (cc'd) might throw some lights on
> > > how shpc comes to play.
> >
> > Yes, I think it will be important to understand if 'shpc' becomes relevant
> > when ACPI hotplug is disabled for PCI

https://patchwork.kernel.org/project/qemu-devel/patch/1478099802-14188-1-git-send-email-marcel@redhat.com
has some discussions around shpc.

Also
"PCI devices can be hot-plugged into PCI Express to PCI and PCI-PCI Bridges.
The PCI hot-plug into PCI-PCI bridge is ACPI based, whereas hot-plug into
PCI Express to PCI bridges is SHPC-based. They both can work side by side with
the PCI Express native hot-plug."

https://android.googlesource.com/platform/external/qemu/+/emu-master-dev/docs/pcie.txt

My own experiments on i440fx has shown that Windows does not seem to
care about shpc.
Disabling acpi hotplug on bridges disables hotplug.

> >
> > >
> > > >  - q35 / PCIe topology - hotplug historically used native PCIe hotplug,
> > > >                          but in 6.1 switched to ACPI
> > > >
> > >
> > > Correct.
> > >
> > > > Given, the name "acpi-hotplug-bridge",  am I right that this option
> > > > has *no* effect, if the q35 machine is using native PCIe hotplug
> > > > approach ?
> > >
> > > Its complicated.
> > > With "acpi-hotplug-bridge" ON, native hotplug is disabled in qemu.
> > > With "acpi-hotplug-bridge" OFF, native hotplug is enabled in qemu.
> >
> > Oh, I mis-read and didn't realize this was controlling the QEMU
> > "acpi-pci-hotplug-with-bridge-support" configuration.
> >
> > With this in mind I think the naming is somewhat misleading. Setting it
> > to off would give users the impression that hotplug is disabled, which
> > is not the case for Q35 at least. It is just switching to a different
> > hotplug implementation.
> >
> > At least from Q35 pov, I think it would be better to call it
> >
> >     hotplug-mode="acpi|pcie"
> >
> > so it is clear that no matter what value it is set to, hotplug
> > is still available.
> >
> > If we also consider PIIX, then depending on the answer wrt shpc
> > above, we might want one of
> >
> >     hotplug-mode="acpi|pcie|none"
> >     hotplug-mode="acpi|pcie|shpc"
> >
>
> If libvirt does not deal with shpc today I think we should not bother with
> shpc at all. We should simply have a boolean mode appropriately named that
> choses between acpi hotplug vs native.
>
> >
> > > Libvirt does not allow it, but by directly using qemu commandline it is
> > > possible to enable ACPI hotplug for pcie-root-ports by turning
> > > "acpi-hotplug-bridge" ON _and_ enable native hotplug at the same time. In
> > > that case, as Julia has mentioned in some other thread, the guest OS
> > > always choses native over ACPI based hotplug.
> >
> > I see that is refering to pie-root-port gaining  native_hotplug=bool in 6.1
> >
> > You say the guest OS always chooses native over ACPI - is that reqiired
> > in the spec, or is that just how some common guest OS have choosen to
> > impl it today ?
> >
>
> I am not sure but I *think* this is guest OS dependent too? Julia to
> rescue here :-)
>
> >
> > > The <target hotplug="on/off"> switch in libvirt for pcie-root-ports
> > > currently does not care whether native or acpi hotplug is used. It simply
> > > turns on the hotplug for that particular port. Whether ACPI or native is
> > > used is controlled by this global flag that Julia has introduced in 6.1.
> >
> > Right so we have
> >
> >   * PIIX4
> >
> >       - acpi-root-pci-hotplug=bool
> >
> >         Whether hotplug is enabled for the root bridge or not
> >
> >           <target hotplug="on/off"> for pci-root controller
> >
> >
> >       - acpi-pci-hotplug-with-bridge-support=bool
> >
> >         Toggles support for ACPI based hotplug across all bridges.
> >       If disabled will there will be no hotplug at all for PIIX4 ?
> >       Or does 'shpc' come into play in that scenario ?
> >
> >
> >    PIIX combinations
> >
> >        (1) acpi-root-pci-hotplug=yes
> >            acpi-pci-hotplug-with-bridge-support=yes
> >
> >          - All bridges have hotplug
> >
> >        (2) acpi-root-pci-hotplug=yes
> >            acpi-pci-hotplug-with-bridge-support=no
> >
> >          - No bridges have hotplug
> >
> >        (3) acpi-root-pci-hotplug=no
> >            acpi-pci-hotplug-with-bridge-support=yes
> >
> >          - All bridges except root have hotplug
> >
> >        (4) acpi-root-pci-hotplug=no
> >            acpi-pci-hotplug-with-bridge-support=no
> >
> >          - No bridges have hotplug. Essentially identical to (2)
>
> no (4) is not identical to (2). In (4) no hotplug is enabled. In (2) pci
> root bus still has hotplug enabled.
>
>
> >
> >   * Q35
> >
> >
> >       - acpi-pci-hotplug-with-bridge-support=bool
> >
> >         Toggles support for ACPI based hotplug. If disabled native
> >       PCIe hotplug is activated instead
> >
> >
> >   * pcie-root-port
> >
> >       - hotplug=bool
> >
> >         Toggles whether hotplug is enabled on the port. Affects
> >       either native and ACPI based hotplug, depending on what
> >       acpi-pci-hotplug-with-bridge-support=bool is set to ?
> >
> >           <target hotplug="on/off"> for pcie-root-port controller
> >
> >       - native_hotplug=bool
> >
> >         Can be used to also enable native hotplug, even when ACPI
> >       based hotplug is globally enabled. IOW only really relevant
> >       when acpi-pci-hotplug-with-bridge-support=true
> >
> >
> >    Q35 combinations
> >
> >        (1) acpi-pci-hotplug-with-bridge-support=yes
> >            pcie-root-port.hotplug=yes
> >            pcie-root-port.native_hotplug=yes
> >
> >            ports have both ACPI + native hotplug, guest prefers native
> >
> >
> >        (2) acpi-pci-hotplug-with-bridge-support=yes
> >            pcie-root-port.hotplug=no
> >            pcie-root-port.native_hotplug=yes
> >
> >            ports have no hotplug, despite native_hotplug=yes IIUC
> >
> >
> >        (3) acpi-pci-hotplug-with-bridge-support=yes
> >            pcie-root-port.hotplug=yes
> >            pcie-root-port.native_hotplug=no
> >
> >            ports have ACPI hotplug only
> >
> >
> >        (4) acpi-pci-hotplug-with-bridge-support=yes
> >            pcie-root-port.hotplug=no
> >            pcie-root-port.native_hotplug=no
> >
> >            ports have no hotplug
> >
> >
> >        (5) acpi-pci-hotplug-with-bridge-support=no
> >            pcie-root-port.hotplug=yes
> >            pcie-root-port.native_hotplug=yes
> >
> >            ports have native hotplug
> >
> >
> >        (6) acpi-pci-hotplug-with-bridge-support=no
> >            pcie-root-port.hotplug=no
> >            pcie-root-port.native_hotplug=yes
> >
> >            ports have no hotplug, despite native_hotplug=yes IIUC
> >
> >
> >        (7) acpi-pci-hotplug-with-bridge-support=no
> >            pcie-root-port.hotplug=yes
> >            pcie-root-port.native_hotplug=no
> >
> >            ports have no hotplug, despite hotplug=yes IIUC
> >
> >
> >        (8) acpi-pci-hotplug-with-bridge-support=no
> >            pcie-root-port.hotplug=no
> >            pcie-root-port.native_hotplug=no
> >
> >            ports have no hotplug
> >
> >
> >
> > Regards,
> > Daniel
> > --
> > |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> > |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> > |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
> >
> >


Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Laine Stump 4 years, 4 months ago
On 9/28/21 4:44 AM, Daniel P. Berrangé wrote:
> On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
>> This change introduces libvirt xml support for the following two pm options:
>>
>> <pm>
>>    <acpi-hotplug-bridge enabled='no'/>
>>    <acpi-root-hotplug enabled='yes'/>
>> </pm>
> 
> 
>> +``acpi-hotplug-bridge``
>> +   :since:`Since 7.8.0` This option enables or disables BIOS ACPI based hotplug support
>> +   for cold plugged bridges. It is available only for x86 guests, both for q35 and pc
>> +   machine types. For pc machines, the support is available from `QEMU 2.12`. For q35
>> +   machines, the support is available from `QEMU 6.1`. Examples of cold plugged bridges
>> +   include PCI-PCI bridges for pc machine types (pci-bridge controller). For q35 machines,
>> +   it includes PCIE root ports (pcie-root-port controller). This is a global option that
>> +   affects all bridges. No other bridge specific option is required to be specified.
> 
> Can you confirm my understanding of the situation..
> 
>   - i440fx / PCI topology - hotplug always uses ACPI
> 
>   - q35 / PCIe topology - hotplug historically used native PCIe hotplug,
>                           but in 6.1 switched to ACPI
> 
> Given, the name "acpi-hotplug-bridge",  am I right that this option
> has *no* effect, if the q35 machine is using native PCIe hotplug
> approach ?  IOW, is it a no-op until 6.1 based machine types for q35 ?

I *think* that in machinetypes where the default is native-pcie hotplug, 
setting acpi-hotplug-bridge=on will simultaneously enable ACPI hotplug 
and disable native-pcie hotplug for all pcie-root-ports and 
pcie-downstream-ports. Similarly on 6.1-based machinetypes, setting 
acpi-hotplug-bridge=off will disable ACPI hotplug and enable native-pcie 
hotplug.

On 440fx, where the default has always been ACPI (and where SHPC hotplug 
has been disabled), acpi-hotplug-bridge=on will be a NOP, and 
acpi-hotplug-bridge=off will completely disable hotplug on any 
pci-bridge (but *not* on pci-root).

As for the acpi-hotplug-root option, that is only valid for 440fx, is 
"on" by default, and when acpi-hotplug-root=off it will completely 
disable hotplug to any slot on pci-root.

(for completeness - when a pcie-root-port or pcie-downstream-port has 
<target hotplug='off'/>, that will disable whatever hotplug mode would 
have been enabled for the controller - no hotplug at all will be 
possible on that controller. QEMU also has a "native_hotplug" option 
(not supported in libvirt) for pcie-root-ports and pcie-downstream-ports 
which can be used to enable native-pcie hotplug on a specific controller 
when it had been disabled (in favor of ACPI) with the global 
acpi-hotplug-bridge ).

Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Ani Sinha 4 years, 4 months ago
On Tue, Sep 28, 2021 at 10:17 PM Laine Stump <laine@redhat.com> wrote:

> On 9/28/21 4:44 AM, Daniel P. Berrangé wrote:
> > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
> >> This change introduces libvirt xml support for the following two pm
> options:
> >>
> >> <pm>
> >>    <acpi-hotplug-bridge enabled='no'/>
> >>    <acpi-root-hotplug enabled='yes'/>
> >> </pm>
> >
> >
> >> +``acpi-hotplug-bridge``
> >> +   :since:`Since 7.8.0` This option enables or disables BIOS ACPI
> based hotplug support
> >> +   for cold plugged bridges. It is available only for x86 guests, both
> for q35 and pc
> >> +   machine types. For pc machines, the support is available from `QEMU
> 2.12`. For q35
> >> +   machines, the support is available from `QEMU 6.1`. Examples of
> cold plugged bridges
> >> +   include PCI-PCI bridges for pc machine types (pci-bridge
> controller). For q35 machines,
> >> +   it includes PCIE root ports (pcie-root-port controller). This is a
> global option that
> >> +   affects all bridges. No other bridge specific option is required to
> be specified.
> >
> > Can you confirm my understanding of the situation..
> >
> >   - i440fx / PCI topology - hotplug always uses ACPI
> >
> >   - q35 / PCIe topology - hotplug historically used native PCIe hotplug,
> >                           but in 6.1 switched to ACPI
> >
> > Given, the name "acpi-hotplug-bridge",  am I right that this option
> > has *no* effect, if the q35 machine is using native PCIe hotplug
> > approach ?  IOW, is it a no-op until 6.1 based machine types for q35 ?
>
> I *think* that in machinetypes where the default is native-pcie hotplug,
> setting acpi-hotplug-bridge=on


6.1 not only introduced this option/command line in Qemu but also flipped
the switch to make ACPI hotplug default for pcie root ports. So there are
 no officially released  version of Qemu where this command line exists and
the default is native pcie hotplug.


will simultaneously enable ACPI hotplug
> and disable native-pcie hotplug for all pcie-root-ports and
> pcie-downstream-ports.


This is for 6.1 based machines as well.

Similarly on 6.1-based machinetypes, setting
> acpi-hotplug-bridge=off will disable ACPI hotplug and enable native-pcie
> hotplug.
>
> On 440fx, where the default has always been ACPI (and where SHPC hotplug
> has been disabled), acpi-hotplug-bridge=on will be a NOP, and
> acpi-hotplug-bridge=off will completely disable hotplug on any
> pci-bridge (but *not* on pci-root).
>
> As for the acpi-hotplug-root option, that is only valid for 440fx, is
> "on" by default, and when acpi-hotplug-root=off it will completely
> disable hotplug to any slot on pci-root.
>
> (for completeness - when a pcie-root-port or pcie-downstream-port has
> <target hotplug='off'/>, that will disable whatever hotplug mode would
> have been enabled for the controller - no hotplug at all will be
> possible on that controller. QEMU also has a "native_hotplug" option
> (not supported in libvirt) for pcie-root-ports and pcie-downstream-ports
> which can be used to enable native-pcie hotplug on a specific controller
> when it had been disabled (in favor of ACPI) with the global
> acpi-hotplug-bridge ).
>
>
Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Laine Stump 4 years, 4 months ago
On 9/28/21 12:54 PM, Ani Sinha wrote:
> 
> 
> On Tue, Sep 28, 2021 at 10:17 PM Laine Stump <laine@redhat.com 
> <mailto:laine@redhat.com>> wrote:
> 
>     On 9/28/21 4:44 AM, Daniel P. Berrangé wrote:
>      > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
>      >> This change introduces libvirt xml support for the following two
>     pm options:
>      >>
>      >> <pm>
>      >>    <acpi-hotplug-bridge enabled='no'/>
>      >>    <acpi-root-hotplug enabled='yes'/>
>      >> </pm>
>      >
>      >
>      >> +``acpi-hotplug-bridge``
>      >> +   :since:`Since 7.8.0` This option enables or disables BIOS
>     ACPI based hotplug support
>      >> +   for cold plugged bridges. It is available only for x86
>     guests, both for q35 and pc
>      >> +   machine types. For pc machines, the support is available
>     from `QEMU 2.12`. For q35
>      >> +   machines, the support is available from `QEMU 6.1`. Examples
>     of cold plugged bridges
>      >> +   include PCI-PCI bridges for pc machine types (pci-bridge
>     controller). For q35 machines,
>      >> +   it includes PCIE root ports (pcie-root-port controller).
>     This is a global option that
>      >> +   affects all bridges. No other bridge specific option is
>     required to be specified.
>      >
>      > Can you confirm my understanding of the situation..
>      >
>      >   - i440fx / PCI topology - hotplug always uses ACPI
>      >
>      >   - q35 / PCIe topology - hotplug historically used native PCIe
>     hotplug,
>      >                           but in 6.1 switched to ACPI
>      >
>      > Given, the name "acpi-hotplug-bridge",  am I right that this option
>      > has *no* effect, if the q35 machine is using native PCIe hotplug
>      > approach ?  IOW, is it a no-op until 6.1 based machine types for
>     q35 ?
> 
>     I *think* that in machinetypes where the default is native-pcie
>     hotplug,
>     setting acpi-hotplug-bridge=on 
> 
> 
> 6.1 not only introduced this option/command line in Qemu but also 
> flipped the switch to make ACPI hotplug default for pcie root ports. So 
> there are  no officially released  version of Qemu where this command 
> line exists and the default is native pcie hotplug.

You're assuming that everyone will use the canonical "q35" machinetype 
rather than a specific versioned machinetype (e.g. "pc-q35-6.0"). For 
pre-6.1 machinetypes, the default will still be native-pcie hotplug, 
even when running qemu-6.1+.

> 
> 
>     will simultaneously enable ACPI hotplug
>     and disable native-pcie hotplug for all pcie-root-ports and
>     pcie-downstream-ports.
> 
> 
> This is for 6.1 based machines as well.
> 
>     Similarly on 6.1-based machinetypes, setting
>     acpi-hotplug-bridge=off will disable ACPI hotplug and enable
>     native-pcie
>     hotplug.
> 
>     On 440fx, where the default has always been ACPI (and where SHPC
>     hotplug
>     has been disabled), acpi-hotplug-bridge=on will be a NOP, and
>     acpi-hotplug-bridge=off will completely disable hotplug on any
>     pci-bridge (but *not* on pci-root).
> 
>     As for the acpi-hotplug-root option, that is only valid for 440fx, is
>     "on" by default, and when acpi-hotplug-root=off it will completely
>     disable hotplug to any slot on pci-root.
> 
>     (for completeness - when a pcie-root-port or pcie-downstream-port has
>     <target hotplug='off'/>, that will disable whatever hotplug mode would
>     have been enabled for the controller - no hotplug at all will be
>     possible on that controller. QEMU also has a "native_hotplug" option
>     (not supported in libvirt) for pcie-root-ports and
>     pcie-downstream-ports
>     which can be used to enable native-pcie hotplug on a specific
>     controller
>     when it had been disabled (in favor of ACPI) with the global
>     acpi-hotplug-bridge ).
> 

Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options
Posted by Ani Sinha 4 years, 4 months ago

On Tue, 28 Sep 2021, Laine Stump wrote:

> On 9/28/21 12:54 PM, Ani Sinha wrote:
> >
> >
> > On Tue, Sep 28, 2021 at 10:17 PM Laine Stump <laine@redhat.com
> > <mailto:laine@redhat.com>> wrote:
> >
> >     On 9/28/21 4:44 AM, Daniel P. Berrangé wrote:
> >      > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote:
> >      >> This change introduces libvirt xml support for the following two
> >     pm options:
> >      >>
> >      >> <pm>
> >      >>    <acpi-hotplug-bridge enabled='no'/>
> >      >>    <acpi-root-hotplug enabled='yes'/>
> >      >> </pm>
> >      >
> >      >
> >      >> +``acpi-hotplug-bridge``
> >      >> +   :since:`Since 7.8.0` This option enables or disables BIOS
> >     ACPI based hotplug support
> >      >> +   for cold plugged bridges. It is available only for x86
> >     guests, both for q35 and pc
> >      >> +   machine types. For pc machines, the support is available
> >     from `QEMU 2.12`. For q35
> >      >> +   machines, the support is available from `QEMU 6.1`. Examples
> >     of cold plugged bridges
> >      >> +   include PCI-PCI bridges for pc machine types (pci-bridge
> >     controller). For q35 machines,
> >      >> +   it includes PCIE root ports (pcie-root-port controller).
> >     This is a global option that
> >      >> +   affects all bridges. No other bridge specific option is
> >     required to be specified.
> >      >
> >      > Can you confirm my understanding of the situation..
> >      >
> >      >   - i440fx / PCI topology - hotplug always uses ACPI
> >      >
> >      >   - q35 / PCIe topology - hotplug historically used native PCIe
> >     hotplug,
> >      >                           but in 6.1 switched to ACPI
> >      >
> >      > Given, the name "acpi-hotplug-bridge",  am I right that this option
> >      > has *no* effect, if the q35 machine is using native PCIe hotplug
> >      > approach ?  IOW, is it a no-op until 6.1 based machine types for
> >     q35 ?
> >
> >     I *think* that in machinetypes where the default is native-pcie
> >     hotplug,
> >     setting acpi-hotplug-bridge=on
> >
> > 6.1 not only introduced this option/command line in Qemu but also flipped
> > the switch to make ACPI hotplug default for pcie root ports. So there are
> >  no officially released  version of Qemu where this command line exists and
> > the default is native pcie hotplug.
>
> You're assuming that everyone will use the canonical "q35" machinetype rather
> than a specific versioned machinetype (e.g. "pc-q35-6.0"). For pre-6.1
> machinetypes, the default will still be native-pcie hotplug, even when running
> qemu-6.1+.

Yes you are right. If one uses the 6.1 binary but specifies a compat pre
6.1 machine type then yes, native hotplug would be the default and
acpi-hotplug-bridge will be off. If someone forces it on (not sure why
they would though because they could simply use the 6.1 machine), the acpi
hotplug would be enabled and native hotplug would get disabled on q35.


> >
> >
> >     will simultaneously enable ACPI hotplug
> >     and disable native-pcie hotplug for all pcie-root-ports and
> >     pcie-downstream-ports.
> >
> >
> > This is for 6.1 based machines as well.
> >
> >     Similarly on 6.1-based machinetypes, setting
> >     acpi-hotplug-bridge=off will disable ACPI hotplug and enable
> >     native-pcie
> >     hotplug.
> >
> >     On 440fx, where the default has always been ACPI (and where SHPC
> >     hotplug
> >     has been disabled), acpi-hotplug-bridge=on will be a NOP, and
> >     acpi-hotplug-bridge=off will completely disable hotplug on any
> >     pci-bridge (but *not* on pci-root).
> >
> >     As for the acpi-hotplug-root option, that is only valid for 440fx, is
> >     "on" by default, and when acpi-hotplug-root=off it will completely
> >     disable hotplug to any slot on pci-root.
> >
> >     (for completeness - when a pcie-root-port or pcie-downstream-port has
> >     <target hotplug='off'/>, that will disable whatever hotplug mode would
> >     have been enabled for the controller - no hotplug at all will be
> >     possible on that controller. QEMU also has a "native_hotplug" option
> >     (not supported in libvirt) for pcie-root-ports and
> >     pcie-downstream-ports
> >     which can be used to enable native-pcie hotplug on a specific
> >     controller
> >     when it had been disabled (in favor of ACPI) with the global
> >     acpi-hotplug-bridge ).
> >
>
>