diff mbox series

Add a new PIIX option to control global PCI hot-plugging

Message ID 1589537195-31392-1-git-send-email-ani.sinha@nutanix.com (mailing list archive)
State New, archived
Headers show
Series Add a new PIIX option to control global PCI hot-plugging | expand

Commit Message

Ani Sinha May 15, 2020, 10:06 a.m. UTC
A new option "acpi-pci-hotplug" is introduced for PIIX which will
globally disable hot-plugging of both hot plugged and
cold plugged PCI devices. This will prevent
hot-plugging and hot un-plugging of devices from within Windows based
guests using system tray.

The patch has been tested on Windows 2016.

Change-Id: I6a79f5c69dab2c06fc3578b098bf085fb8741ce6
Signed-off-by: Ani Sinha <ani.sinha@nutanix.com>
---
 hw/acpi/piix4.c      | 19 ++++++++++++-------
 hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++----------------
 2 files changed, 42 insertions(+), 23 deletions(-)

Comments

Ani Sinha May 15, 2020, 10:10 a.m. UTC | #1
Igor, here’s my attempt in cooking up a patch which disabled global ACPI hot plug.
You said something about SHPC also adding AMLs when enabled but I could not figure out where that is being done.

Please provide suggestions.

> On May 15, 2020, at 3:36 PM, Ani Sinha <ani.sinha@nutanix.com> wrote:
> 
> A new option "acpi-pci-hotplug" is introduced for PIIX which will
> globally disable hot-plugging of both hot plugged and
> cold plugged PCI devices. This will prevent
> hot-plugging and hot un-plugging of devices from within Windows based
> guests using system tray.
> 
> The patch has been tested on Windows 2016.
> 
> Change-Id: I6a79f5c69dab2c06fc3578b098bf085fb8741ce6
> Signed-off-by: Ani Sinha <ani.sinha@nutanix.com>
> ---
> hw/acpi/piix4.c      | 19 ++++++++++++-------
> hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++----------------
> 2 files changed, 42 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 964d6f5..1f86dd9 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -78,6 +78,7 @@ typedef struct PIIX4PMState {
> 
>     AcpiPciHpState acpi_pci_hotplug;
>     bool use_acpi_pci_hotplug;
> +    bool use_acpi_hotplug_bridge;
> 
>     uint8_t disable_s3;
>     uint8_t disable_s4;
> @@ -207,13 +208,13 @@ static const VMStateDescription vmstate_pci_status = {
> static bool vmstate_test_use_acpi_pci_hotplug(void *opaque, int version_id)
> {
>     PIIX4PMState *s = opaque;
> -    return s->use_acpi_pci_hotplug;
> +    return s->use_acpi_hotplug_bridge;
> }
> 
> static bool vmstate_test_no_use_acpi_pci_hotplug(void *opaque, int version_id)
> {
>     PIIX4PMState *s = opaque;
> -    return !s->use_acpi_pci_hotplug;
> +    return !s->use_acpi_hotplug_bridge;
> }
> 
> static bool vmstate_test_use_memhp(void *opaque)
> @@ -505,8 +506,6 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
> 
>     piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
>                                    pci_get_bus(dev), s);
> -    qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), OBJECT(s), &error_abort);
> -

I left this as per your suggestion but this was just an optimization to move this into the init routine correct?

>     piix4_pm_add_propeties(s);
> }
> 
> @@ -528,7 +527,7 @@ I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
>     s->smi_irq = smi_irq;
>     s->smm_enabled = smm_enabled;
>     if (xen_enabled()) {
> -        s->use_acpi_pci_hotplug = false;
> +        s->use_acpi_hotplug_bridge = false;
>     }
> 
>     qdev_init_nofail(dev);
> @@ -593,8 +592,12 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
>     memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe);
> 
>     acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
> -                    s->use_acpi_pci_hotplug);
> +                    s->use_acpi_hotplug_bridge);
> 
> +    if (s->use_acpi_pci_hotplug) {
> +        qbus_set_hotplug_handler(BUS(pci_get_bus(dev)),
> +                                 OBJECT(s), &error_abort);
> +    }

Moved this one from the above. Seems we could have used this check in piix4_pm_realize() as well.

>     s->cpu_hotplug_legacy = true;
>     object_property_add_bool(OBJECT(s), "cpu-hotplug-legacy",
>                              piix4_get_cpu_hotplug_legacy,
> @@ -631,8 +634,10 @@ static Property piix4_pm_properties[] = {
>     DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0),
>     DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_DISABLED, PIIX4PMState, disable_s4, 0),
>     DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
> -    DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
> +    DEFINE_PROP_BOOL("acpi-pci-hotplug", PIIX4PMState,
>                      use_acpi_pci_hotplug, true),
> +    DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
> +                     use_acpi_hotplug_bridge, true),
>     DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>                      acpi_memory_hotplug.is_enabled, true),
>     DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 2e15f68..1737378 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -96,6 +96,7 @@ typedef struct AcpiPmInfo {
>     bool s3_disabled;
>     bool s4_disabled;
>     bool pcihp_bridge_en;
> +    bool acpi_pcihp_en;
>     uint8_t s4_val;
>     AcpiFadtData fadt;
>     uint16_t cpu_hp_io_base;
> @@ -246,6 +247,9 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
>     pm->pcihp_bridge_en =
>         object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
>                                  NULL);
> +    pm->acpi_pcihp_en =
> +        object_property_get_bool(obj, "acpi-pci-hotplug",
> +                                 NULL);
> }
> 
> static void acpi_get_misc_info(AcpiMiscInfo *info)
> @@ -457,7 +461,8 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
> }
> 
> static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
> -                                         bool pcihp_bridge_en)
> +                                         bool pcihp_bridge_en,
> +                                         bool acpi_pcihp_en)
> {
>     Aml *dev, *notify_method = NULL, *method;
>     QObject *bsel;
> @@ -481,18 +486,25 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>         bool bridge_in_acpi;
> 
>         if (!pdev) {
> -            if (bsel) { /* add hotplug slots for non present devices */
> -                dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> -                aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> -                aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
> -                method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> -                aml_append(method,
> -                    aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
> -                );
> -                aml_append(dev, method);
> -                aml_append(parent_scope, dev);
> -
> -                build_append_pcihp_notify_entry(notify_method, slot);
> +            if (bsel) {
> +                /*
> +                 * add hotplug slots for non present devices when
> +                 * acpi hotplug is enabled.
> +                 */
> +                if (acpi_pcihp_en) {
> +                    dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> +                    aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> +                    aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
> +                    method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
> +                    aml_append(method,
> +                               aml_call2("PCEJ", aml_name("BSEL"),
> +                                         aml_name("_SUN"))
> +                        );
> +                    aml_append(dev, method);
> +                    aml_append(parent_scope, dev);
> +
> +                    build_append_pcihp_notify_entry(notify_method, slot);
> +                }
>             }
>             continue;
>         }
> @@ -539,7 +551,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>             method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
>             aml_append(method, aml_return(aml_int(s3d)));
>             aml_append(dev, method);
> -        } else if (hotplug_enabled_dev) {
> +        } else if (hotplug_enabled_dev && acpi_pcihp_en) {
>             /* add _SUN/_EJ0 to make slot hotpluggable  */
>             aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> 
> @@ -559,7 +571,8 @@ static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
>              */
>             PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
> 
> -            build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
> +            build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
> +                                         acpi_pcihp_en);
>         }
>         /* slot descriptor has been composed, add it into parent context */
>         aml_append(parent_scope, dev);
> @@ -2197,7 +2210,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>         if (bus) {
>             Aml *scope = aml_scope("PCI0");
>             /* Scan all PCI buses. Generate tables to support hotplug. */
> -            build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
> +            build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en,
> +                                         pm->acpi_pcihp_en);
> 
>             if (TPM_IS_TIS_ISA(tpm)) {
>                 if (misc->tpm_version == TPM_VERSION_2_0) {
> -- 
> 1.9.4
>
no-reply@patchew.org May 15, 2020, 4:01 p.m. UTC | #2
Patchew URL: https://patchew.org/QEMU/1589537195-31392-1-git-send-email-ani.sinha@nutanix.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

                 from /tmp/qemu-test/src/include/hw/i386/pc.h:6,
                 from /tmp/qemu-test/src/hw/acpi/piix4.c:23:
/tmp/qemu-test/src/hw/acpi/piix4.c: In function 'piix4_acpi_system_hot_add_init':
/tmp/qemu-test/src/hw/acpi/piix4.c:598:50: error: 'dev' undeclared (first use in this function)
         qbus_set_hotplug_handler(BUS(pci_get_bus(dev)),
                                                  ^
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
---
/tmp/qemu-test/src/hw/acpi/piix4.c:598:34: note: in expansion of macro 'BUS'
         qbus_set_hotplug_handler(BUS(pci_get_bus(dev)),
                                  ^
make: *** [hw/acpi/piix4.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=596d9c46226b4cf897ddc2619e5fd42b', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-eddmk7e8/src/docker-src.2020-05-15-11.58.42.12526:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=596d9c46226b4cf897ddc2619e5fd42b
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-eddmk7e8/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m16.404s
user    0m8.435s


The full log is available at
http://patchew.org/logs/1589537195-31392-1-git-send-email-ani.sinha@nutanix.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org May 15, 2020, 5:01 p.m. UTC | #3
Patchew URL: https://patchew.org/QEMU/1589537195-31392-1-git-send-email-ani.sinha@nutanix.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      hw/ipack/tpci200.o
  CC      hw/ipmi/ipmi.o
  CC      hw/ipmi/ipmi_kcs.o
/tmp/qemu-test/src/hw/acpi/piix4.c:598:50: error: use of undeclared identifier 'dev'; did you mean 'div'?
        qbus_set_hotplug_handler(BUS(pci_get_bus(dev)),
                                                 ^~~
                                                 div
---
extern div_t div (int __numer, int __denom)
             ^
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: hw/acpi/piix4.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=0bb5e655a8fc4cc78415e01756d997f3', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-2jscst1a/src/docker-src.2020-05-15-12.58.08.31883:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=0bb5e655a8fc4cc78415e01756d997f3
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-2jscst1a/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    3m11.668s
user    0m8.291s


The full log is available at
http://patchew.org/logs/1589537195-31392-1-git-send-email-ani.sinha@nutanix.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org May 15, 2020, 5:08 p.m. UTC | #4
Patchew URL: https://patchew.org/QEMU/1589537195-31392-1-git-send-email-ani.sinha@nutanix.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

                 from /tmp/qemu-test/src/include/hw/i386/pc.h:6,
                 from /tmp/qemu-test/src/hw/acpi/piix4.c:23:
/tmp/qemu-test/src/hw/acpi/piix4.c: In function 'piix4_acpi_system_hot_add_init':
/tmp/qemu-test/src/hw/acpi/piix4.c:598:50: error: 'dev' undeclared (first use in this function); did you mean 'div'?
         qbus_set_hotplug_handler(BUS(pci_get_bus(dev)),
                                                  ^~~
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
---
/tmp/qemu-test/src/hw/acpi/piix4.c:598:34: note: in expansion of macro 'BUS'
         qbus_set_hotplug_handler(BUS(pci_get_bus(dev)),
                                  ^~~
make: *** [/tmp/qemu-test/src/rules.mak:69: hw/acpi/piix4.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=2a2173c2534246efb3bb6d0af07e746c', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-cq8mpb3q/src/docker-src.2020-05-15-13.06.59.5902:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=2a2173c2534246efb3bb6d0af07e746c
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-cq8mpb3q/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    1m36.551s
user    0m7.829s


The full log is available at
http://patchew.org/logs/1589537195-31392-1-git-send-email-ani.sinha@nutanix.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
diff mbox series

Patch

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 964d6f5..1f86dd9 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -78,6 +78,7 @@  typedef struct PIIX4PMState {
 
     AcpiPciHpState acpi_pci_hotplug;
     bool use_acpi_pci_hotplug;
+    bool use_acpi_hotplug_bridge;
 
     uint8_t disable_s3;
     uint8_t disable_s4;
@@ -207,13 +208,13 @@  static const VMStateDescription vmstate_pci_status = {
 static bool vmstate_test_use_acpi_pci_hotplug(void *opaque, int version_id)
 {
     PIIX4PMState *s = opaque;
-    return s->use_acpi_pci_hotplug;
+    return s->use_acpi_hotplug_bridge;
 }
 
 static bool vmstate_test_no_use_acpi_pci_hotplug(void *opaque, int version_id)
 {
     PIIX4PMState *s = opaque;
-    return !s->use_acpi_pci_hotplug;
+    return !s->use_acpi_hotplug_bridge;
 }
 
 static bool vmstate_test_use_memhp(void *opaque)
@@ -505,8 +506,6 @@  static void piix4_pm_realize(PCIDevice *dev, Error **errp)
 
     piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
                                    pci_get_bus(dev), s);
-    qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), OBJECT(s), &error_abort);
-
     piix4_pm_add_propeties(s);
 }
 
@@ -528,7 +527,7 @@  I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
     s->smi_irq = smi_irq;
     s->smm_enabled = smm_enabled;
     if (xen_enabled()) {
-        s->use_acpi_pci_hotplug = false;
+        s->use_acpi_hotplug_bridge = false;
     }
 
     qdev_init_nofail(dev);
@@ -593,8 +592,12 @@  static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
     memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe);
 
     acpi_pcihp_init(OBJECT(s), &s->acpi_pci_hotplug, bus, parent,
-                    s->use_acpi_pci_hotplug);
+                    s->use_acpi_hotplug_bridge);
 
+    if (s->use_acpi_pci_hotplug) {
+        qbus_set_hotplug_handler(BUS(pci_get_bus(dev)),
+                                 OBJECT(s), &error_abort);
+    }
     s->cpu_hotplug_legacy = true;
     object_property_add_bool(OBJECT(s), "cpu-hotplug-legacy",
                              piix4_get_cpu_hotplug_legacy,
@@ -631,8 +634,10 @@  static Property piix4_pm_properties[] = {
     DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0),
     DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_DISABLED, PIIX4PMState, disable_s4, 0),
     DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
-    DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
+    DEFINE_PROP_BOOL("acpi-pci-hotplug", PIIX4PMState,
                      use_acpi_pci_hotplug, true),
+    DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
+                     use_acpi_hotplug_bridge, true),
     DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
                      acpi_memory_hotplug.is_enabled, true),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2e15f68..1737378 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -96,6 +96,7 @@  typedef struct AcpiPmInfo {
     bool s3_disabled;
     bool s4_disabled;
     bool pcihp_bridge_en;
+    bool acpi_pcihp_en;
     uint8_t s4_val;
     AcpiFadtData fadt;
     uint16_t cpu_hp_io_base;
@@ -246,6 +247,9 @@  static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
     pm->pcihp_bridge_en =
         object_property_get_bool(obj, "acpi-pci-hotplug-with-bridge-support",
                                  NULL);
+    pm->acpi_pcihp_en =
+        object_property_get_bool(obj, "acpi-pci-hotplug",
+                                 NULL);
 }
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
@@ -457,7 +461,8 @@  static void build_append_pcihp_notify_entry(Aml *method, int slot)
 }
 
 static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
-                                         bool pcihp_bridge_en)
+                                         bool pcihp_bridge_en,
+                                         bool acpi_pcihp_en)
 {
     Aml *dev, *notify_method = NULL, *method;
     QObject *bsel;
@@ -481,18 +486,25 @@  static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
         bool bridge_in_acpi;
 
         if (!pdev) {
-            if (bsel) { /* add hotplug slots for non present devices */
-                dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
-                aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
-                aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
-                method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
-                aml_append(method,
-                    aml_call2("PCEJ", aml_name("BSEL"), aml_name("_SUN"))
-                );
-                aml_append(dev, method);
-                aml_append(parent_scope, dev);
-
-                build_append_pcihp_notify_entry(notify_method, slot);
+            if (bsel) {
+                /*
+                 * add hotplug slots for non present devices when
+                 * acpi hotplug is enabled.
+                 */
+                if (acpi_pcihp_en) {
+                    dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
+                    aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
+                    aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 16)));
+                    method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
+                    aml_append(method,
+                               aml_call2("PCEJ", aml_name("BSEL"),
+                                         aml_name("_SUN"))
+                        );
+                    aml_append(dev, method);
+                    aml_append(parent_scope, dev);
+
+                    build_append_pcihp_notify_entry(notify_method, slot);
+                }
             }
             continue;
         }
@@ -539,7 +551,7 @@  static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
             method = aml_method("_S3D", 0, AML_NOTSERIALIZED);
             aml_append(method, aml_return(aml_int(s3d)));
             aml_append(dev, method);
-        } else if (hotplug_enabled_dev) {
+        } else if (hotplug_enabled_dev && acpi_pcihp_en) {
             /* add _SUN/_EJ0 to make slot hotpluggable  */
             aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
 
@@ -559,7 +571,8 @@  static void build_append_pci_bus_devices(Aml *parent_scope, PCIBus *bus,
              */
             PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
 
-            build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en);
+            build_append_pci_bus_devices(dev, sec_bus, pcihp_bridge_en,
+                                         acpi_pcihp_en);
         }
         /* slot descriptor has been composed, add it into parent context */
         aml_append(parent_scope, dev);
@@ -2197,7 +2210,8 @@  build_dsdt(GArray *table_data, BIOSLinker *linker,
         if (bus) {
             Aml *scope = aml_scope("PCI0");
             /* Scan all PCI buses. Generate tables to support hotplug. */
-            build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
+            build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en,
+                                         pm->acpi_pcihp_en);
 
             if (TPM_IS_TIS_ISA(tpm)) {
                 if (misc->tpm_version == TPM_VERSION_2_0) {