Re: [PATCH v6 2/4] conf: introduce support for acpi-bridge-hotplug feature

2021-10-07 Thread Ani Sinha



On Fri, 8 Oct 2021, Laine Stump wrote:

> On 10/5/21 1:51 AM, Ani Sinha wrote:
> > This change introduces a new libvirt sub-element  under  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:
> >
> > 
> >
> >  
> >
> > 
> >
> > The above option is only available for qemu driver and that too for x86
> > guests
> > only. It is a global option.
> >
> > 'acpi-bridge-hotplug' option enables or disables ACPI hotplug support for
> > cold-plugged pci bridges. Examples of bridges include PCI-PCI bridge
> > (pci-bridge controller) or PCIe-PCI bridges for pc machines and
> > pcie-root-port controller for q35 machines. Being global option, no other
> > bridge specific option 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 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 machine type, 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 
> > ---
> >   docs/formatdomain.rst | 11 +++
> >   docs/schemas/domaincommon.rng | 15 
> >   src/conf/domain_conf.c| 89 ++-
> >   src/conf/domain_conf.h|  9 ++
> >   src/qemu/qemu_validate.c  | 46 ++
> >   .../aarch64-acpi-hotplug-bridge-disable.xml   | 33 +++
> >   .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 33 +++
> >   .../pc-i440fx-acpi-hotplug-bridge-enable.xml  | 33 +++
> >   .../q35-acpi-hotplug-bridge-disable.xml   | 47 ++
> >   .../q35-acpi-hotplug-bridge-enable.xml| 47 ++
> >   .../pc-i440fx-acpi-hotplug-bridge-disable.xml |  1 +
> >   .../pc-i440fx-acpi-hotplug-bridge-enable.xml  |  1 +
> >   .../q35-acpi-hotplug-bridge-disable.xml   |  1 +
> >   .../q35-acpi-hotplug-bridge-enable.xml|  1 +
> >   tests/qemuxml2xmltest.c   | 14 +++
> >   15 files changed, 380 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 12
> > tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
> >   create mode 12
> > tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
> >   create mode 12
> > tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
> >   create mode 12
> > tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.xml
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index a02802a954..8d916eefa6 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -1847,6 +1847,9 @@ Hypervisors may allow certain CPU / machine features
> > to be toggled on/off.
> >  
> >  
> >
> > + 
> > +   
> > + 
> >
> >
> >
> > @@ -1942,6 +1945,14 @@ 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-hotp

Re: [PATCH v6 2/4] conf: introduce support for acpi-bridge-hotplug feature

2021-10-07 Thread Laine Stump

On 10/5/21 1:51 AM, Ani Sinha wrote:

This change introduces a new libvirt sub-element  under  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:


   
 
   


The above option is only available for qemu driver and that too for x86 guests
only. It is a global option.

'acpi-bridge-hotplug' option enables or disables ACPI hotplug support for
cold-plugged pci bridges. Examples of bridges include PCI-PCI bridge
(pci-bridge controller) or PCIe-PCI bridges for pc machines and
pcie-root-port controller for q35 machines. Being global option, no other
bridge specific option 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 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 machine type, 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 
---
  docs/formatdomain.rst | 11 +++
  docs/schemas/domaincommon.rng | 15 
  src/conf/domain_conf.c| 89 ++-
  src/conf/domain_conf.h|  9 ++
  src/qemu/qemu_validate.c  | 46 ++
  .../aarch64-acpi-hotplug-bridge-disable.xml   | 33 +++
  .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 33 +++
  .../pc-i440fx-acpi-hotplug-bridge-enable.xml  | 33 +++
  .../q35-acpi-hotplug-bridge-disable.xml   | 47 ++
  .../q35-acpi-hotplug-bridge-enable.xml| 47 ++
  .../pc-i440fx-acpi-hotplug-bridge-disable.xml |  1 +
  .../pc-i440fx-acpi-hotplug-bridge-enable.xml  |  1 +
  .../q35-acpi-hotplug-bridge-disable.xml   |  1 +
  .../q35-acpi-hotplug-bridge-enable.xml|  1 +
  tests/qemuxml2xmltest.c   | 14 +++
  15 files changed, 380 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 12 
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
  create mode 12 
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
  create mode 12 
tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
  create mode 12 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.xml

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index a02802a954..8d916eefa6 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1847,6 +1847,9 @@ Hypervisors may allow certain CPU / machine features to 
be toggled on/off.
 
 
   
+ 
+   
+ 
   
   
   
@@ -1942,6 +1945,14 @@ 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) on, off :since:`7.8.0 (QEMU 6.1.0)`


Weren't the qemu properties there in a much earlier version of QEMU? 
(Oh, and also it needs to say 7.9.0 instead of 7.8.0)


Somewhere we need to put the extra info that on 440fx machinetypes ACPI 
is on by default, and disabling it means that only SHPC hotplug

Re: [PATCH v6 2/4] conf: introduce support for acpi-bridge-hotplug feature

2021-10-07 Thread Laine Stump

On 10/7/21 11:12 PM, Laine Stump wrote:

@@ -27932,6 +27995,30 @@ virDomainDefFormatFeatures(virBuffer *buf,
 
virDomainIBSTypeToString(def->features[i]));

   break;
+    case VIR_DOMAIN_FEATURE_PCI:
+    if (def->features[i] != VIR_TRISTATE_SWITCH_ON)
+    break;
+
+    virBufferAddLit(&childBuf, "\n");
+    virBufferAdjustIndent(&childBuf, 2);
+    for (j = 0; j < VIR_DOMAIN_PCI_LAST; j++) {
+    switch ((virDomainPCI) j) {
+    case VIR_DOMAIN_PCI_ACPI_BRIDGE_HP:
+    if (def->pci_features[j])


Oops, I missed this the first time I went through - this should compare 
to VIR_TRISTATE_SWITCH_ABSENT rather than just checking for non-0. It 
ends up being the same thing, but you never know - some day someone may 
decide to change the values of the VIR_TRISTATE_* enums just for fun...



+    virBufferAsprintf(&childBuf, "<%s state='%s'/>\n",
+  virDomainPCITypeToString(j),
+  virTristateSwitchTypeToString(
+  def->pci_features[j]));
+    break;




Re: [PATCH v6 2/4] conf: introduce support for acpi-bridge-hotplug feature

2021-10-07 Thread Laine Stump

On 10/5/21 1:51 AM, Ani Sinha wrote:

This change introduces a new libvirt sub-element  under  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:


   
 
   




I guess this is as good a place as any other. Any other opinions?




The above option is only available for qemu driver and that too for x86 guests
only. It is a global option.

'acpi-bridge-hotplug' option enables or disables ACPI hotplug support for
cold-plugged pci bridges. Examples of bridges include PCI-PCI bridge
(pci-bridge controller) or PCIe-PCI bridges for pc machines and
pcie-root-port controller for q35 machines. Being global option, no other
bridge specific option 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 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 machine type, 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 
---
  docs/formatdomain.rst | 11 +++
  docs/schemas/domaincommon.rng | 15 
  src/conf/domain_conf.c| 89 ++-
  src/conf/domain_conf.h|  9 ++
  src/qemu/qemu_validate.c  | 46 ++
  .../aarch64-acpi-hotplug-bridge-disable.xml   | 33 +++
  .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 33 +++
  .../pc-i440fx-acpi-hotplug-bridge-enable.xml  | 33 +++
  .../q35-acpi-hotplug-bridge-disable.xml   | 47 ++
  .../q35-acpi-hotplug-bridge-enable.xml| 47 ++
  .../pc-i440fx-acpi-hotplug-bridge-disable.xml |  1 +
  .../pc-i440fx-acpi-hotplug-bridge-enable.xml  |  1 +
  .../q35-acpi-hotplug-bridge-disable.xml   |  1 +
  .../q35-acpi-hotplug-bridge-enable.xml|  1 +
  tests/qemuxml2xmltest.c   | 14 +++
  15 files changed, 380 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 12 
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
  create mode 12 
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
  create mode 12 
tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
  create mode 12 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.xml

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index a02802a954..8d916eefa6 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1847,6 +1847,9 @@ Hypervisors may allow certain CPU / machine features to 
be toggled on/off.
 
 
   
+ 
+   
+ 
   
   
   
@@ -1942,6 +1945,14 @@ 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) on, off :since:`7.8.0 (QEMU 6.1.0)`
+    

 === 
+
  ``pmu``
 Depending on the ``sta

[PATCH v6 2/4] conf: introduce support for acpi-bridge-hotplug feature

2021-10-04 Thread Ani Sinha
This change introduces a new libvirt sub-element  under  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:


  

  


The above option is only available for qemu driver and that too for x86 guests
only. It is a global option.

'acpi-bridge-hotplug' option enables or disables ACPI hotplug support for
cold-plugged pci bridges. Examples of bridges include PCI-PCI bridge
(pci-bridge controller) or PCIe-PCI bridges for pc machines and
pcie-root-port controller for q35 machines. Being global option, no other
bridge specific option 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 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 machine type, 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 
---
 docs/formatdomain.rst | 11 +++
 docs/schemas/domaincommon.rng | 15 
 src/conf/domain_conf.c| 89 ++-
 src/conf/domain_conf.h|  9 ++
 src/qemu/qemu_validate.c  | 46 ++
 .../aarch64-acpi-hotplug-bridge-disable.xml   | 33 +++
 .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 33 +++
 .../pc-i440fx-acpi-hotplug-bridge-enable.xml  | 33 +++
 .../q35-acpi-hotplug-bridge-disable.xml   | 47 ++
 .../q35-acpi-hotplug-bridge-enable.xml| 47 ++
 .../pc-i440fx-acpi-hotplug-bridge-disable.xml |  1 +
 .../pc-i440fx-acpi-hotplug-bridge-enable.xml  |  1 +
 .../q35-acpi-hotplug-bridge-disable.xml   |  1 +
 .../q35-acpi-hotplug-bridge-enable.xml|  1 +
 tests/qemuxml2xmltest.c   | 14 +++
 15 files changed, 380 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 12 
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
 create mode 12 
tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
 create mode 12 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
 create mode 12 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.xml

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index a02802a954..8d916eefa6 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1847,6 +1847,9 @@ Hypervisors may allow certain CPU / machine features to 
be toggled on/off.


  
+ 
+   
+ 
  
  
  
@@ -1942,6 +1945,14 @@ 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) on, off :since:`7.8.0 (QEMU 6.1.0)`
+    

 === 
+
 ``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/