[RFC PATCH] add support for two global pm options that controls acpi hotplug on q35/i440fx

Ani Sinha posted 1 patch 2 years, 8 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210816140155.183703-1-ani@anisinha.ca
src/conf/domain_conf.c       | 10 ++++++++++
src/conf/domain_conf.h       |  2 ++
src/qemu/qemu_capabilities.c |  9 +++++++++
src/qemu/qemu_capabilities.h |  5 +++++
src/qemu/qemu_command.c      | 28 ++++++++++++++++++++++++++++
src/qemu/qemu_validate.c     | 21 +++++++++++++++++++++
6 files changed, 75 insertions(+)
[RFC PATCH] add support for two global pm options that controls acpi hotplug on q35/i440fx
Posted by Ani Sinha 2 years, 8 months ago
'acpi-pci-hotplug-with-bridge-support' and 'acpi-root-pci-hotplug' are two pm
options in qemu that governs acpi hotplug support. This is a WIP patch that
tries to implement support for both these two global qemu options.
'acpi-pci-hotplug-with-bridge-support' applies both for q35 as well as i440fx
platforms. The 'acpi-root-pci-hotplug' only applies for i440fx.

'acpi-pci-hotplug-with-bridge-support' can be turned off by providing the
following xml snippet:

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

Similarly, 'acpi-root-pci-hotplug' can be turned off by providing the following
xml snippet:

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

This change adds no unit tests. It will be added later on. It it sent for
initial comments and suggestions.

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 src/conf/domain_conf.c       | 10 ++++++++++
 src/conf/domain_conf.h       |  2 ++
 src/qemu/qemu_capabilities.c |  9 +++++++++
 src/qemu/qemu_capabilities.h |  5 +++++
 src/qemu/qemu_command.c      | 28 ++++++++++++++++++++++++++++
 src/qemu/qemu_validate.c     | 21 +++++++++++++++++++++
 6 files changed, 75 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 06c1fcf5e5..512973e6f4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19351,6 +19351,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:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ca21082624..94283cbb04 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2643,6 +2643,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/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9558938866..8dad4a6351 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -637,6 +637,11 @@ VIR_ENUM_IMPL(virQEMUCaps,
               "confidential-guest-support",
               "query-display-options",
               "s390-pv-guest",
+              "piix4-acpi-hotplug-bridge",
+              "piix4-acpi-root-hotplug-en",
+
+              /* 410 */
+              "ich9-acpi-hotplug-bridge",
     );
 
 
@@ -1467,6 +1472,9 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsIDEDrive[] = {
 static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsPiix4PM[] = {
     { "disable_s3", QEMU_CAPS_PIIX_DISABLE_S3, NULL },
     { "disable_s4", QEMU_CAPS_PIIX_DISABLE_S4, NULL },
+    { "acpi-pci-hotplug-with-bridge-support", QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE, NULL },
+    { "acpi-root-pci-hotplug", QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG, NULL },
+
 };
 
 static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBRedir[] = {
@@ -1519,6 +1527,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 2b1bb57a49..e988527f39 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -617,6 +617,11 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
     QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT, /* -machine confidential-guest-support */
     QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command present */
     QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */
+    QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE, /* -M pc PIIX4_PM.acpi-pci-hotplug-with-bridge-support */
+    QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG, /* -M pc PIIX4_PM.acpi-root-pci-hotplug */
+
+    /* 410 */
+    QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE, /* -M q35 PIIX4_PM.acpi-pci-hotplug-with-bridge-support */
 
     QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 156af4caee..5a7a23d5b9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6171,6 +6171,34 @@ qemuBuildPMCommandLine(virCommand *cmd,
                                pm_object, def->pm.s4 == VIR_TRISTATE_BOOL_NO);
     }
 
+    if (def->pm.acpi_hp_bridge) {
+        const char *pm_object = "PIIX4_PM";
+        const char *switch_str = "on";
+
+        if (def->pm.acpi_hp_bridge == VIR_TRISTATE_BOOL_NO)
+            switch_str = "off";
+
+        if (qemuDomainIsQ35(def) &&
+            virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE))
+            pm_object = "ICH9-LPC";
+
+        virCommandAddArg(cmd, "-global");
+        virCommandAddArgFormat(cmd, "%s.acpi-pci-hotplug-with-bridge-support=%s",
+                               pm_object, switch_str);
+    }
+
+    if (def->pm.acpi_root_hp) {
+        const char *pm_object = "PIIX4_PM";
+        const char *switch_str = "on";
+
+        if (def->pm.acpi_hp_bridge == VIR_TRISTATE_BOOL_NO)
+            switch_str = "off";
+
+        virCommandAddArg(cmd, "-global");
+        virCommandAddArgFormat(cmd, "%s.acpi-root-pci-hotplug=%s",
+                               pm_object, switch_str);
+    }
+
     return 0;
 }
 
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index a964c8593d..22403e6d01 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -560,6 +560,27 @@ qemuValidateDomainDefPM(const virDomainDef *def,
         }
     }
 
+    if (def->pm.acpi_hp_bridge) {
+        bool q35ICH9_pcihpbr = q35Dom &&
+                                virQEMUCapsGet(qemuCaps,
+                                              QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE);
+
+        if (!q35ICH9_pcihpbr && !virQEMUCapsGet(qemuCaps,
+                                                QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           "%s", _("setting ACPI hotplug bridge not supported"));
+            return -1;
+        }
+    }
+
+    if (def->pm.acpi_root_hp) {
+        if (!q35Dom && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           "%s", _("setting ACPI root pci hotplug not supported"));
+            return -1;
+        }
+    }
+
     return 0;
 }
 
-- 
2.25.1

Re: [RFC PATCH] add support for two global pm options that controls acpi hotplug on q35/i440fx
Posted by Ani Sinha 2 years, 8 months ago
Hi:

Please disregard this patch. This patch has issues and schema was not
updated.
I have sent an updated patch set with subject:
pm/i386: add support for two options that controls acpi hotplug on
q35/i440fx

Not sure if I was able to send the patch series correctly (I wanted to add
a cover letter but somehow it went out without one). If required I can
send it again. The new patch series has the issues fixed, unit tests added
and they all pass fine.

Requesting for a review and feedback.

Thanks
ani


On Mon, 16 Aug 2021, Ani Sinha wrote:

> 'acpi-pci-hotplug-with-bridge-support' and 'acpi-root-pci-hotplug' are two pm
> options in qemu that governs acpi hotplug support. This is a WIP patch that
> tries to implement support for both these two global qemu options.
> 'acpi-pci-hotplug-with-bridge-support' applies both for q35 as well as i440fx
> platforms. The 'acpi-root-pci-hotplug' only applies for i440fx.
>
> 'acpi-pci-hotplug-with-bridge-support' can be turned off by providing the
> following xml snippet:
>
> <pm>
>    <acpi-hotplug-bridge enabled='no'>
> </pm>
>
> Similarly, 'acpi-root-pci-hotplug' can be turned off by providing the following
> xml snippet:
>
> <pm>
>    <acpi-root-hotplug enabled='no'>
> </pm>
>
> This change adds no unit tests. It will be added later on. It it sent for
> initial comments and suggestions.
>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
>  src/conf/domain_conf.c       | 10 ++++++++++
>  src/conf/domain_conf.h       |  2 ++
>  src/qemu/qemu_capabilities.c |  9 +++++++++
>  src/qemu/qemu_capabilities.h |  5 +++++
>  src/qemu/qemu_command.c      | 28 ++++++++++++++++++++++++++++
>  src/qemu/qemu_validate.c     | 21 +++++++++++++++++++++
>  6 files changed, 75 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 06c1fcf5e5..512973e6f4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19351,6 +19351,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:
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ca21082624..94283cbb04 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2643,6 +2643,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/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 9558938866..8dad4a6351 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -637,6 +637,11 @@ VIR_ENUM_IMPL(virQEMUCaps,
>                "confidential-guest-support",
>                "query-display-options",
>                "s390-pv-guest",
> +              "piix4-acpi-hotplug-bridge",
> +              "piix4-acpi-root-hotplug-en",
> +
> +              /* 410 */
> +              "ich9-acpi-hotplug-bridge",
>      );
>
>
> @@ -1467,6 +1472,9 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsIDEDrive[] = {
>  static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsPiix4PM[] = {
>      { "disable_s3", QEMU_CAPS_PIIX_DISABLE_S3, NULL },
>      { "disable_s4", QEMU_CAPS_PIIX_DISABLE_S4, NULL },
> +    { "acpi-pci-hotplug-with-bridge-support", QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE, NULL },
> +    { "acpi-root-pci-hotplug", QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG, NULL },
> +
>  };
>
>  static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsUSBRedir[] = {
> @@ -1519,6 +1527,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 2b1bb57a49..e988527f39 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -617,6 +617,11 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
>      QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT, /* -machine confidential-guest-support */
>      QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command present */
>      QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */
> +    QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE, /* -M pc PIIX4_PM.acpi-pci-hotplug-with-bridge-support */
> +    QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG, /* -M pc PIIX4_PM.acpi-root-pci-hotplug */
> +
> +    /* 410 */
> +    QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE, /* -M q35 PIIX4_PM.acpi-pci-hotplug-with-bridge-support */
>
>      QEMU_CAPS_LAST /* this must always be the last item */
>  } virQEMUCapsFlags;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 156af4caee..5a7a23d5b9 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6171,6 +6171,34 @@ qemuBuildPMCommandLine(virCommand *cmd,
>                                 pm_object, def->pm.s4 == VIR_TRISTATE_BOOL_NO);
>      }
>
> +    if (def->pm.acpi_hp_bridge) {
> +        const char *pm_object = "PIIX4_PM";
> +        const char *switch_str = "on";
> +
> +        if (def->pm.acpi_hp_bridge == VIR_TRISTATE_BOOL_NO)
> +            switch_str = "off";
> +
> +        if (qemuDomainIsQ35(def) &&
> +            virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE))
> +            pm_object = "ICH9-LPC";
> +
> +        virCommandAddArg(cmd, "-global");
> +        virCommandAddArgFormat(cmd, "%s.acpi-pci-hotplug-with-bridge-support=%s",
> +                               pm_object, switch_str);
> +    }
> +
> +    if (def->pm.acpi_root_hp) {
> +        const char *pm_object = "PIIX4_PM";
> +        const char *switch_str = "on";
> +
> +        if (def->pm.acpi_hp_bridge == VIR_TRISTATE_BOOL_NO)
> +            switch_str = "off";
> +
> +        virCommandAddArg(cmd, "-global");
> +        virCommandAddArgFormat(cmd, "%s.acpi-root-pci-hotplug=%s",
> +                               pm_object, switch_str);
> +    }
> +
>      return 0;
>  }
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index a964c8593d..22403e6d01 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -560,6 +560,27 @@ qemuValidateDomainDefPM(const virDomainDef *def,
>          }
>      }
>
> +    if (def->pm.acpi_hp_bridge) {
> +        bool q35ICH9_pcihpbr = q35Dom &&
> +                                virQEMUCapsGet(qemuCaps,
> +                                              QEMU_CAPS_ICH9_ACPI_HOTPLUG_BRIDGE);
> +
> +        if (!q35ICH9_pcihpbr && !virQEMUCapsGet(qemuCaps,
> +                                                QEMU_CAPS_PIIX_ACPI_HOTPLUG_BRIDGE)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           "%s", _("setting ACPI hotplug bridge not supported"));
> +            return -1;
> +        }
> +    }
> +
> +    if (def->pm.acpi_root_hp) {
> +        if (!q35Dom && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_ACPI_ROOT_PCI_HOTPLUG)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           "%s", _("setting ACPI root pci hotplug not supported"));
> +            return -1;
> +        }
> +    }
> +
>      return 0;
>  }
>
> --
> 2.25.1
>
>