| Message ID | 20210617190739.3673064-4-jusual@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use ACPI PCI hot-plug for Q35 | expand |
On Thu, Jun 17, 2021 at 10:08 PM Julia Suvorova <jusual@redhat.com> wrote: > Add acpi_pcihp to ich9_pm as part of > 'acpi-pci-hotplug-with-bridge-support' option. Set default to false. > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/i386/acpi-build.h | 1 + > include/hw/acpi/ich9.h | 3 ++ > hw/acpi/ich9.c | 67 ++++++++++++++++++++++++++++++++++++++++++ > hw/acpi/pcihp.c | 5 +++- > hw/i386/acpi-build.c | 2 +- > 5 files changed, 76 insertions(+), 2 deletions(-) > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > index 487ec7710f..0dce155c8c 100644 > --- a/hw/i386/acpi-build.h > +++ b/hw/i386/acpi-build.h > @@ -10,5 +10,6 @@ extern const struct AcpiGenericAddress > x86_nvdimm_acpi_dsmio; > #define ACPI_PCIHP_BNMR_BASE 0x10 > > void acpi_setup(void); > +Object *acpi_get_i386_pci_host(void); > > #endif > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h > index 596120d97f..a329ce43ab 100644 > --- a/include/hw/acpi/ich9.h > +++ b/include/hw/acpi/ich9.h > @@ -24,6 +24,7 @@ > #include "hw/acpi/acpi.h" > #include "hw/acpi/cpu_hotplug.h" > #include "hw/acpi/cpu.h" > +#include "hw/acpi/pcihp.h" > #include "hw/acpi/memory_hotplug.h" > #include "hw/acpi/acpi_dev_interface.h" > #include "hw/acpi/tco.h" > @@ -55,6 +56,8 @@ typedef struct ICH9LPCPMRegs { > AcpiCpuHotplug gpe_cpu; > CPUHotplugState cpuhp_state; > > + bool use_acpi_hotplug_bridge; > + AcpiPciHpState acpi_pci_hotplug; > MemHotplugState acpi_memory_hotplug; > > uint8_t disable_s3; > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 4daa79ec8d..bcbd567cb0 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -217,6 +217,26 @@ static const VMStateDescription vmstate_cpuhp_state = > { > } > }; > > +static bool vmstate_test_use_pcihp(void *opaque) > +{ > + ICH9LPCPMRegs *s = opaque; > + > + return s->use_acpi_hotplug_bridge; > +} > + > +static const VMStateDescription vmstate_pcihp_state = { > + .name = "ich9_pm/pcihp", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = vmstate_test_use_pcihp, > + .fields = (VMStateField[]) { > + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, > + ICH9LPCPMRegs, > + NULL, NULL), > + VMSTATE_END_OF_LIST() > + } > +}; > + > const VMStateDescription vmstate_ich9_pm = { > .name = "ich9_pm", > .version_id = 1, > @@ -238,6 +258,7 @@ const VMStateDescription vmstate_ich9_pm = { > &vmstate_memhp_state, > &vmstate_tco_io_state, > &vmstate_cpuhp_state, > + &vmstate_pcihp_state, > NULL > } > }; > @@ -259,6 +280,7 @@ static void pm_reset(void *opaque) > } > pm->smi_en_wmask = ~0; > > + acpi_pcihp_reset(&pm->acpi_pci_hotplug, true); > acpi_update_sci(&pm->acpi_regs, pm->irq); > } > > @@ -297,6 +319,18 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs > *pm, > pm->enable_tco = true; > acpi_pm_tco_init(&pm->tco_regs, &pm->io); > > + if (pm->use_acpi_hotplug_bridge) { > + acpi_pcihp_init(OBJECT(lpc_pci), > + &pm->acpi_pci_hotplug, > + pci_get_bus(lpc_pci), > + pci_address_space_io(lpc_pci), > + true, > + ACPI_PCIHP_ADDR_ICH9); > + > + qbus_set_hotplug_handler(BUS(pci_get_bus(lpc_pci)), > + OBJECT(lpc_pci)); > + } > + > pm->irq = sci_irq; > qemu_register_reset(pm_reset, pm); > pm->powerdown_notifier.notify = pm_powerdown_req; > @@ -368,6 +402,20 @@ static void ich9_pm_set_enable_tco(Object *obj, bool > value, Error **errp) > s->pm.enable_tco = value; > } > > +static bool ich9_pm_get_acpi_pci_hotplug(Object *obj, Error **errp) > +{ > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > + > + return s->pm.use_acpi_hotplug_bridge; > +} > + > +static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value, Error > **errp) > +{ > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > + > + s->pm.use_acpi_hotplug_bridge = value; > +} > + > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > { > static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; > @@ -376,6 +424,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs > *pm) > pm->disable_s3 = 0; > pm->disable_s4 = 0; > pm->s4_val = 2; > + pm->use_acpi_hotplug_bridge = false; > > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE, > &pm->pm_io_base, OBJ_PROP_FLAG_READ); > @@ -399,6 +448,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs > *pm) > object_property_add_bool(obj, ACPI_PM_PROP_TCO_ENABLED, > ich9_pm_get_enable_tco, > ich9_pm_set_enable_tco); > + object_property_add_bool(obj, "acpi-pci-hotplug-with-bridge-support", > + ich9_pm_get_acpi_pci_hotplug, > + ich9_pm_set_acpi_pci_hotplug); > } > > void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState > *dev, > @@ -406,6 +458,11 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler > *hotplug_dev, DeviceState *dev, > { > ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev); > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + acpi_pcihp_device_pre_plug_cb(hotplug_dev, dev, errp); > + return; > + } > + > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) && > !lpc->pm.acpi_memory_hotplug.is_enabled) { > error_setg(errp, > @@ -441,6 +498,9 @@ void ich9_pm_device_plug_cb(HotplugHandler > *hotplug_dev, DeviceState *dev, > } else { > acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, > errp); > } > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + acpi_pcihp_device_plug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, > + dev, errp); > } else { > error_setg(errp, "acpi: device plug request for not supported > device" > " type: %s", object_get_typename(OBJECT(dev))); > @@ -473,6 +533,10 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler > *hotplug_dev, > > acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state, > dev, errp); > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + acpi_pcihp_device_unplug_request_cb(hotplug_dev, > + &lpc->pm.acpi_pci_hotplug, > + dev, errp); > } else { > error_setg(errp, "acpi: device unplug request for not supported > device" > " type: %s", object_get_typename(OBJECT(dev))); > @@ -490,6 +554,9 @@ void ich9_pm_device_unplug_cb(HotplugHandler > *hotplug_dev, DeviceState *dev, > } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) && > !lpc->pm.cpu_hotplug_legacy) { > acpi_cpu_unplug_cb(&lpc->pm.cpuhp_state, dev, errp); > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + acpi_pcihp_device_unplug_cb(hotplug_dev, > &lpc->pm.acpi_pci_hotplug, > + dev, errp); > } else { > error_setg(errp, "acpi: device unplug for not supported device" > " type: %s", object_get_typename(OBJECT(dev))); > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index a55992ed9f..5355618608 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -30,6 +30,8 @@ > #include "hw/pci-host/i440fx.h" > #include "hw/pci/pci.h" > #include "hw/pci/pci_bridge.h" > +#include "hw/pci/pci_host.h" > +#include "hw/i386/acpi-build.h" > #include "hw/acpi/acpi.h" > #include "hw/pci/pci_bus.h" > #include "migration/vmstate.h" > @@ -103,6 +105,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) > static void acpi_set_pci_info(void) > { > static bool bsel_is_set; > + Object *host = acpi_get_i386_pci_host(); > PCIBus *bus; > unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT; > > @@ -111,7 +114,7 @@ static void acpi_set_pci_info(void) > } > bsel_is_set = true; > > - bus = find_i440fx(); /* TODO: Q35 support */ > + bus = PCI_HOST_BRIDGE(host)->bus; > if (bus) { > /* Scan all PCI buses. Set property to enable acpi based hotplug. > */ > pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, > &bsel_alloc); > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 67753638e4..aa973aa746 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -299,7 +299,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) > * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE. > * On i386 arch we only have two pci hosts, so we can look only for them. > */ > -static Object *acpi_get_i386_pci_host(void) > +Object *acpi_get_i386_pci_host(void) > { > PCIHostState *host; > > -- > 2.30.2 > > Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Thanks, Marcel
On Thu, Jun 17, 2021 at 09:07:35PM +0200, Julia Suvorova wrote: > Add acpi_pcihp to ich9_pm as part of > 'acpi-pci-hotplug-with-bridge-support' option. Set default to false. > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > --- > hw/i386/acpi-build.h | 1 + > include/hw/acpi/ich9.h | 3 ++ > hw/acpi/ich9.c | 67 ++++++++++++++++++++++++++++++++++++++++++ > hw/acpi/pcihp.c | 5 +++- > hw/i386/acpi-build.c | 2 +- > 5 files changed, 76 insertions(+), 2 deletions(-) > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > index 487ec7710f..0dce155c8c 100644 > --- a/hw/i386/acpi-build.h > +++ b/hw/i386/acpi-build.h > @@ -10,5 +10,6 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > #define ACPI_PCIHP_BNMR_BASE 0x10 > > void acpi_setup(void); > +Object *acpi_get_i386_pci_host(void); > > #endif > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h > index 596120d97f..a329ce43ab 100644 > --- a/include/hw/acpi/ich9.h > +++ b/include/hw/acpi/ich9.h > @@ -24,6 +24,7 @@ > #include "hw/acpi/acpi.h" > #include "hw/acpi/cpu_hotplug.h" > #include "hw/acpi/cpu.h" > +#include "hw/acpi/pcihp.h" > #include "hw/acpi/memory_hotplug.h" > #include "hw/acpi/acpi_dev_interface.h" > #include "hw/acpi/tco.h" > @@ -55,6 +56,8 @@ typedef struct ICH9LPCPMRegs { > AcpiCpuHotplug gpe_cpu; > CPUHotplugState cpuhp_state; > > + bool use_acpi_hotplug_bridge; > + AcpiPciHpState acpi_pci_hotplug; > MemHotplugState acpi_memory_hotplug; > > uint8_t disable_s3; > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 4daa79ec8d..bcbd567cb0 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -217,6 +217,26 @@ static const VMStateDescription vmstate_cpuhp_state = { > } > }; > > +static bool vmstate_test_use_pcihp(void *opaque) > +{ > + ICH9LPCPMRegs *s = opaque; > + > + return s->use_acpi_hotplug_bridge; > +} > + > +static const VMStateDescription vmstate_pcihp_state = { > + .name = "ich9_pm/pcihp", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = vmstate_test_use_pcihp, > + .fields = (VMStateField[]) { > + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, > + ICH9LPCPMRegs, > + NULL, NULL), > + VMSTATE_END_OF_LIST() > + } > +}; > + > const VMStateDescription vmstate_ich9_pm = { > .name = "ich9_pm", > .version_id = 1, > @@ -238,6 +258,7 @@ const VMStateDescription vmstate_ich9_pm = { > &vmstate_memhp_state, > &vmstate_tco_io_state, > &vmstate_cpuhp_state, > + &vmstate_pcihp_state, > NULL > } > }; > @@ -259,6 +280,7 @@ static void pm_reset(void *opaque) > } > pm->smi_en_wmask = ~0; > > + acpi_pcihp_reset(&pm->acpi_pci_hotplug, true); Doesn't this need to be protected by if (pm->use_acpi_hotplug_bridge) ? Otherwise pm->acpi_pci_hotplug won't be initialized. > acpi_update_sci(&pm->acpi_regs, pm->irq); > } > > @@ -297,6 +319,18 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, > pm->enable_tco = true; > acpi_pm_tco_init(&pm->tco_regs, &pm->io); > > + if (pm->use_acpi_hotplug_bridge) { > + acpi_pcihp_init(OBJECT(lpc_pci), > + &pm->acpi_pci_hotplug, > + pci_get_bus(lpc_pci), > + pci_address_space_io(lpc_pci), > + true, > + ACPI_PCIHP_ADDR_ICH9); > + > + qbus_set_hotplug_handler(BUS(pci_get_bus(lpc_pci)), > + OBJECT(lpc_pci)); > + } > + > pm->irq = sci_irq; > qemu_register_reset(pm_reset, pm); > pm->powerdown_notifier.notify = pm_powerdown_req; > @@ -368,6 +402,20 @@ static void ich9_pm_set_enable_tco(Object *obj, bool value, Error **errp) > s->pm.enable_tco = value; > } > > +static bool ich9_pm_get_acpi_pci_hotplug(Object *obj, Error **errp) > +{ > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > + > + return s->pm.use_acpi_hotplug_bridge; > +} > + > +static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value, Error **errp) > +{ > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > + > + s->pm.use_acpi_hotplug_bridge = value; > +} > + > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > { > static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; > @@ -376,6 +424,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > pm->disable_s3 = 0; > pm->disable_s4 = 0; > pm->s4_val = 2; > + pm->use_acpi_hotplug_bridge = false; > > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE, > &pm->pm_io_base, OBJ_PROP_FLAG_READ); > @@ -399,6 +448,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > object_property_add_bool(obj, ACPI_PM_PROP_TCO_ENABLED, > ich9_pm_get_enable_tco, > ich9_pm_set_enable_tco); > + object_property_add_bool(obj, "acpi-pci-hotplug-with-bridge-support", > + ich9_pm_get_acpi_pci_hotplug, > + ich9_pm_set_acpi_pci_hotplug); > } > > void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > @@ -406,6 +458,11 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > { > ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev); > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + acpi_pcihp_device_pre_plug_cb(hotplug_dev, dev, errp); > + return; > + } > + > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) && > !lpc->pm.acpi_memory_hotplug.is_enabled) { > error_setg(errp, > @@ -441,6 +498,9 @@ void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > } else { > acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, errp); > } > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + acpi_pcihp_device_plug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, > + dev, errp); > } else { > error_setg(errp, "acpi: device plug request for not supported device" > " type: %s", object_get_typename(OBJECT(dev))); > @@ -473,6 +533,10 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev, > > acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state, > dev, errp); > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + acpi_pcihp_device_unplug_request_cb(hotplug_dev, > + &lpc->pm.acpi_pci_hotplug, > + dev, errp); > } else { > error_setg(errp, "acpi: device unplug request for not supported device" > " type: %s", object_get_typename(OBJECT(dev))); > @@ -490,6 +554,9 @@ void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) && > !lpc->pm.cpu_hotplug_legacy) { > acpi_cpu_unplug_cb(&lpc->pm.cpuhp_state, dev, errp); > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + acpi_pcihp_device_unplug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, > + dev, errp); > } else { > error_setg(errp, "acpi: device unplug for not supported device" > " type: %s", object_get_typename(OBJECT(dev))); > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index a55992ed9f..5355618608 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -30,6 +30,8 @@ > #include "hw/pci-host/i440fx.h" > #include "hw/pci/pci.h" > #include "hw/pci/pci_bridge.h" > +#include "hw/pci/pci_host.h" > +#include "hw/i386/acpi-build.h" > #include "hw/acpi/acpi.h" > #include "hw/pci/pci_bus.h" > #include "migration/vmstate.h" > @@ -103,6 +105,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) > static void acpi_set_pci_info(void) > { > static bool bsel_is_set; > + Object *host = acpi_get_i386_pci_host(); > PCIBus *bus; > unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT; > > @@ -111,7 +114,7 @@ static void acpi_set_pci_info(void) > } > bsel_is_set = true; > > - bus = find_i440fx(); /* TODO: Q35 support */ > + bus = PCI_HOST_BRIDGE(host)->bus; Would it make more sense for this change to go in patch #1, along with the similar change to acpi_pcihp_disable_root_bus(). > if (bus) { > /* Scan all PCI buses. Set property to enable acpi based hotplug. */ > pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 67753638e4..aa973aa746 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -299,7 +299,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) > * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE. > * On i386 arch we only have two pci hosts, so we can look only for them. > */ > -static Object *acpi_get_i386_pci_host(void) > +Object *acpi_get_i386_pci_host(void) > { > PCIHostState *host; >
On Thu, Jul 01, 2021 at 02:46:35PM +1000, David Gibson wrote: > On Thu, Jun 17, 2021 at 09:07:35PM +0200, Julia Suvorova wrote: > > Add acpi_pcihp to ich9_pm as part of > > 'acpi-pci-hotplug-with-bridge-support' option. Set default to false. > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > > --- > > hw/i386/acpi-build.h | 1 + > > include/hw/acpi/ich9.h | 3 ++ > > hw/acpi/ich9.c | 67 ++++++++++++++++++++++++++++++++++++++++++ > > hw/acpi/pcihp.c | 5 +++- > > hw/i386/acpi-build.c | 2 +- > > 5 files changed, 76 insertions(+), 2 deletions(-) > > > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > > index 487ec7710f..0dce155c8c 100644 > > --- a/hw/i386/acpi-build.h > > +++ b/hw/i386/acpi-build.h > > @@ -10,5 +10,6 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > #define ACPI_PCIHP_BNMR_BASE 0x10 > > > > void acpi_setup(void); > > +Object *acpi_get_i386_pci_host(void); > > > > #endif > > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h > > index 596120d97f..a329ce43ab 100644 > > --- a/include/hw/acpi/ich9.h > > +++ b/include/hw/acpi/ich9.h > > @@ -24,6 +24,7 @@ > > #include "hw/acpi/acpi.h" > > #include "hw/acpi/cpu_hotplug.h" > > #include "hw/acpi/cpu.h" > > +#include "hw/acpi/pcihp.h" > > #include "hw/acpi/memory_hotplug.h" > > #include "hw/acpi/acpi_dev_interface.h" > > #include "hw/acpi/tco.h" > > @@ -55,6 +56,8 @@ typedef struct ICH9LPCPMRegs { > > AcpiCpuHotplug gpe_cpu; > > CPUHotplugState cpuhp_state; > > > > + bool use_acpi_hotplug_bridge; > > + AcpiPciHpState acpi_pci_hotplug; > > MemHotplugState acpi_memory_hotplug; > > > > uint8_t disable_s3; > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > > index 4daa79ec8d..bcbd567cb0 100644 > > --- a/hw/acpi/ich9.c > > +++ b/hw/acpi/ich9.c > > @@ -217,6 +217,26 @@ static const VMStateDescription vmstate_cpuhp_state = { > > } > > }; > > > > +static bool vmstate_test_use_pcihp(void *opaque) > > +{ > > + ICH9LPCPMRegs *s = opaque; > > + > > + return s->use_acpi_hotplug_bridge; > > +} > > + > > +static const VMStateDescription vmstate_pcihp_state = { > > + .name = "ich9_pm/pcihp", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .needed = vmstate_test_use_pcihp, > > + .fields = (VMStateField[]) { > > + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, > > + ICH9LPCPMRegs, > > + NULL, NULL), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > const VMStateDescription vmstate_ich9_pm = { > > .name = "ich9_pm", > > .version_id = 1, > > @@ -238,6 +258,7 @@ const VMStateDescription vmstate_ich9_pm = { > > &vmstate_memhp_state, > > &vmstate_tco_io_state, > > &vmstate_cpuhp_state, > > + &vmstate_pcihp_state, > > NULL > > } > > }; > > @@ -259,6 +280,7 @@ static void pm_reset(void *opaque) > > } > > pm->smi_en_wmask = ~0; > > > > + acpi_pcihp_reset(&pm->acpi_pci_hotplug, true); > > Doesn't this need to be protected by if (pm->use_acpi_hotplug_bridge) > ? Otherwise pm->acpi_pci_hotplug won't be initialized. Julia can you comment on this pls? Patch on top if appropriate ... > > acpi_update_sci(&pm->acpi_regs, pm->irq); > > } > > > > @@ -297,6 +319,18 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, > > pm->enable_tco = true; > > acpi_pm_tco_init(&pm->tco_regs, &pm->io); > > > > + if (pm->use_acpi_hotplug_bridge) { > > + acpi_pcihp_init(OBJECT(lpc_pci), > > + &pm->acpi_pci_hotplug, > > + pci_get_bus(lpc_pci), > > + pci_address_space_io(lpc_pci), > > + true, > > + ACPI_PCIHP_ADDR_ICH9); > > + > > + qbus_set_hotplug_handler(BUS(pci_get_bus(lpc_pci)), > > + OBJECT(lpc_pci)); > > + } > > + > > pm->irq = sci_irq; > > qemu_register_reset(pm_reset, pm); > > pm->powerdown_notifier.notify = pm_powerdown_req; > > @@ -368,6 +402,20 @@ static void ich9_pm_set_enable_tco(Object *obj, bool value, Error **errp) > > s->pm.enable_tco = value; > > } > > > > +static bool ich9_pm_get_acpi_pci_hotplug(Object *obj, Error **errp) > > +{ > > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > > + > > + return s->pm.use_acpi_hotplug_bridge; > > +} > > + > > +static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value, Error **errp) > > +{ > > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > > + > > + s->pm.use_acpi_hotplug_bridge = value; > > +} > > + > > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > > { > > static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; > > @@ -376,6 +424,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > > pm->disable_s3 = 0; > > pm->disable_s4 = 0; > > pm->s4_val = 2; > > + pm->use_acpi_hotplug_bridge = false; > > > > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE, > > &pm->pm_io_base, OBJ_PROP_FLAG_READ); > > @@ -399,6 +448,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > > object_property_add_bool(obj, ACPI_PM_PROP_TCO_ENABLED, > > ich9_pm_get_enable_tco, > > ich9_pm_set_enable_tco); > > + object_property_add_bool(obj, "acpi-pci-hotplug-with-bridge-support", > > + ich9_pm_get_acpi_pci_hotplug, > > + ich9_pm_set_acpi_pci_hotplug); > > } > > > > void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > @@ -406,6 +458,11 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > { > > ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev); > > > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > + acpi_pcihp_device_pre_plug_cb(hotplug_dev, dev, errp); > > + return; > > + } > > + > > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) && > > !lpc->pm.acpi_memory_hotplug.is_enabled) { > > error_setg(errp, > > @@ -441,6 +498,9 @@ void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > } else { > > acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, errp); > > } > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > + acpi_pcihp_device_plug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, > > + dev, errp); > > } else { > > error_setg(errp, "acpi: device plug request for not supported device" > > " type: %s", object_get_typename(OBJECT(dev))); > > @@ -473,6 +533,10 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev, > > > > acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state, > > dev, errp); > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > + acpi_pcihp_device_unplug_request_cb(hotplug_dev, > > + &lpc->pm.acpi_pci_hotplug, > > + dev, errp); > > } else { > > error_setg(errp, "acpi: device unplug request for not supported device" > > " type: %s", object_get_typename(OBJECT(dev))); > > @@ -490,6 +554,9 @@ void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) && > > !lpc->pm.cpu_hotplug_legacy) { > > acpi_cpu_unplug_cb(&lpc->pm.cpuhp_state, dev, errp); > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > + acpi_pcihp_device_unplug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, > > + dev, errp); > > } else { > > error_setg(errp, "acpi: device unplug for not supported device" > > " type: %s", object_get_typename(OBJECT(dev))); > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > > index a55992ed9f..5355618608 100644 > > --- a/hw/acpi/pcihp.c > > +++ b/hw/acpi/pcihp.c > > @@ -30,6 +30,8 @@ > > #include "hw/pci-host/i440fx.h" > > #include "hw/pci/pci.h" > > #include "hw/pci/pci_bridge.h" > > +#include "hw/pci/pci_host.h" > > +#include "hw/i386/acpi-build.h" > > #include "hw/acpi/acpi.h" > > #include "hw/pci/pci_bus.h" > > #include "migration/vmstate.h" > > @@ -103,6 +105,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) > > static void acpi_set_pci_info(void) > > { > > static bool bsel_is_set; > > + Object *host = acpi_get_i386_pci_host(); > > PCIBus *bus; > > unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT; > > > > @@ -111,7 +114,7 @@ static void acpi_set_pci_info(void) > > } > > bsel_is_set = true; > > > > - bus = find_i440fx(); /* TODO: Q35 support */ > > + bus = PCI_HOST_BRIDGE(host)->bus; > > Would it make more sense for this change to go in patch #1, along with > the similar change to acpi_pcihp_disable_root_bus(). I guess this is not too bad ... > > if (bus) { > > /* Scan all PCI buses. Set property to enable acpi based hotplug. */ > > pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index 67753638e4..aa973aa746 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -299,7 +299,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) > > * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE. > > * On i386 arch we only have two pci hosts, so we can look only for them. > > */ > > -static Object *acpi_get_i386_pci_host(void) > > +Object *acpi_get_i386_pci_host(void) > > { > > PCIHostState *host; > > > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson
On Thu, Jul 1, 2021 at 6:59 AM David Gibson <david@gibson.dropbear.id.au> wrote: > > On Thu, Jun 17, 2021 at 09:07:35PM +0200, Julia Suvorova wrote: > > Add acpi_pcihp to ich9_pm as part of > > 'acpi-pci-hotplug-with-bridge-support' option. Set default to false. > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > > --- > > hw/i386/acpi-build.h | 1 + > > include/hw/acpi/ich9.h | 3 ++ > > hw/acpi/ich9.c | 67 ++++++++++++++++++++++++++++++++++++++++++ > > hw/acpi/pcihp.c | 5 +++- > > hw/i386/acpi-build.c | 2 +- > > 5 files changed, 76 insertions(+), 2 deletions(-) > > > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > > index 487ec7710f..0dce155c8c 100644 > > --- a/hw/i386/acpi-build.h > > +++ b/hw/i386/acpi-build.h > > @@ -10,5 +10,6 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > #define ACPI_PCIHP_BNMR_BASE 0x10 > > > > void acpi_setup(void); > > +Object *acpi_get_i386_pci_host(void); > > > > #endif > > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h > > index 596120d97f..a329ce43ab 100644 > > --- a/include/hw/acpi/ich9.h > > +++ b/include/hw/acpi/ich9.h > > @@ -24,6 +24,7 @@ > > #include "hw/acpi/acpi.h" > > #include "hw/acpi/cpu_hotplug.h" > > #include "hw/acpi/cpu.h" > > +#include "hw/acpi/pcihp.h" > > #include "hw/acpi/memory_hotplug.h" > > #include "hw/acpi/acpi_dev_interface.h" > > #include "hw/acpi/tco.h" > > @@ -55,6 +56,8 @@ typedef struct ICH9LPCPMRegs { > > AcpiCpuHotplug gpe_cpu; > > CPUHotplugState cpuhp_state; > > > > + bool use_acpi_hotplug_bridge; > > + AcpiPciHpState acpi_pci_hotplug; > > MemHotplugState acpi_memory_hotplug; > > > > uint8_t disable_s3; > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > > index 4daa79ec8d..bcbd567cb0 100644 > > --- a/hw/acpi/ich9.c > > +++ b/hw/acpi/ich9.c > > @@ -217,6 +217,26 @@ static const VMStateDescription vmstate_cpuhp_state = { > > } > > }; > > > > +static bool vmstate_test_use_pcihp(void *opaque) > > +{ > > + ICH9LPCPMRegs *s = opaque; > > + > > + return s->use_acpi_hotplug_bridge; > > +} > > + > > +static const VMStateDescription vmstate_pcihp_state = { > > + .name = "ich9_pm/pcihp", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .needed = vmstate_test_use_pcihp, > > + .fields = (VMStateField[]) { > > + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, > > + ICH9LPCPMRegs, > > + NULL, NULL), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > const VMStateDescription vmstate_ich9_pm = { > > .name = "ich9_pm", > > .version_id = 1, > > @@ -238,6 +258,7 @@ const VMStateDescription vmstate_ich9_pm = { > > &vmstate_memhp_state, > > &vmstate_tco_io_state, > > &vmstate_cpuhp_state, > > + &vmstate_pcihp_state, > > NULL > > } > > }; > > @@ -259,6 +280,7 @@ static void pm_reset(void *opaque) > > } > > pm->smi_en_wmask = ~0; > > > > + acpi_pcihp_reset(&pm->acpi_pci_hotplug, true); > > Doesn't this need to be protected by if (pm->use_acpi_hotplug_bridge) > ? Otherwise pm->acpi_pci_hotplug won't be initialized. Yes, you're right. Although it doesn't affect anything now, it should be fixed. I'll send a patch on top. > > acpi_update_sci(&pm->acpi_regs, pm->irq); > > } > > > > @@ -297,6 +319,18 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, > > pm->enable_tco = true; > > acpi_pm_tco_init(&pm->tco_regs, &pm->io); > > > > + if (pm->use_acpi_hotplug_bridge) { > > + acpi_pcihp_init(OBJECT(lpc_pci), > > + &pm->acpi_pci_hotplug, > > + pci_get_bus(lpc_pci), > > + pci_address_space_io(lpc_pci), > > + true, > > + ACPI_PCIHP_ADDR_ICH9); > > + > > + qbus_set_hotplug_handler(BUS(pci_get_bus(lpc_pci)), > > + OBJECT(lpc_pci)); > > + } > > + > > pm->irq = sci_irq; > > qemu_register_reset(pm_reset, pm); > > pm->powerdown_notifier.notify = pm_powerdown_req; > > @@ -368,6 +402,20 @@ static void ich9_pm_set_enable_tco(Object *obj, bool value, Error **errp) > > s->pm.enable_tco = value; > > } > > > > +static bool ich9_pm_get_acpi_pci_hotplug(Object *obj, Error **errp) > > +{ > > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > > + > > + return s->pm.use_acpi_hotplug_bridge; > > +} > > + > > +static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value, Error **errp) > > +{ > > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > > + > > + s->pm.use_acpi_hotplug_bridge = value; > > +} > > + > > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > > { > > static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; > > @@ -376,6 +424,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > > pm->disable_s3 = 0; > > pm->disable_s4 = 0; > > pm->s4_val = 2; > > + pm->use_acpi_hotplug_bridge = false; > > > > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE, > > &pm->pm_io_base, OBJ_PROP_FLAG_READ); > > @@ -399,6 +448,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > > object_property_add_bool(obj, ACPI_PM_PROP_TCO_ENABLED, > > ich9_pm_get_enable_tco, > > ich9_pm_set_enable_tco); > > + object_property_add_bool(obj, "acpi-pci-hotplug-with-bridge-support", > > + ich9_pm_get_acpi_pci_hotplug, > > + ich9_pm_set_acpi_pci_hotplug); > > } > > > > void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > @@ -406,6 +458,11 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > { > > ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev); > > > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > + acpi_pcihp_device_pre_plug_cb(hotplug_dev, dev, errp); > > + return; > > + } > > + > > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) && > > !lpc->pm.acpi_memory_hotplug.is_enabled) { > > error_setg(errp, > > @@ -441,6 +498,9 @@ void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > } else { > > acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, errp); > > } > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > + acpi_pcihp_device_plug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, > > + dev, errp); > > } else { > > error_setg(errp, "acpi: device plug request for not supported device" > > " type: %s", object_get_typename(OBJECT(dev))); > > @@ -473,6 +533,10 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev, > > > > acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state, > > dev, errp); > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > + acpi_pcihp_device_unplug_request_cb(hotplug_dev, > > + &lpc->pm.acpi_pci_hotplug, > > + dev, errp); > > } else { > > error_setg(errp, "acpi: device unplug request for not supported device" > > " type: %s", object_get_typename(OBJECT(dev))); > > @@ -490,6 +554,9 @@ void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) && > > !lpc->pm.cpu_hotplug_legacy) { > > acpi_cpu_unplug_cb(&lpc->pm.cpuhp_state, dev, errp); > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > + acpi_pcihp_device_unplug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, > > + dev, errp); > > } else { > > error_setg(errp, "acpi: device unplug for not supported device" > > " type: %s", object_get_typename(OBJECT(dev))); > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > > index a55992ed9f..5355618608 100644 > > --- a/hw/acpi/pcihp.c > > +++ b/hw/acpi/pcihp.c > > @@ -30,6 +30,8 @@ > > #include "hw/pci-host/i440fx.h" > > #include "hw/pci/pci.h" > > #include "hw/pci/pci_bridge.h" > > +#include "hw/pci/pci_host.h" > > +#include "hw/i386/acpi-build.h" > > #include "hw/acpi/acpi.h" > > #include "hw/pci/pci_bus.h" > > #include "migration/vmstate.h" > > @@ -103,6 +105,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) > > static void acpi_set_pci_info(void) > > { > > static bool bsel_is_set; > > + Object *host = acpi_get_i386_pci_host(); > > PCIBus *bus; > > unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT; > > > > @@ -111,7 +114,7 @@ static void acpi_set_pci_info(void) > > } > > bsel_is_set = true; > > > > - bus = find_i440fx(); /* TODO: Q35 support */ > > + bus = PCI_HOST_BRIDGE(host)->bus; > > Would it make more sense for this change to go in patch #1, along with > the similar change to acpi_pcihp_disable_root_bus(). Not really. This is exactly what enables ACPI hotplug, and should be done with other changes here. The first patch could be merged in this one, though. Best regards, Julia Suvorova. > > if (bus) { > > /* Scan all PCI buses. Set property to enable acpi based hotplug. */ > > pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index 67753638e4..aa973aa746 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -299,7 +299,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) > > * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE. > > * On i386 arch we only have two pci hosts, so we can look only for them. > > */ > > -static Object *acpi_get_i386_pci_host(void) > > +Object *acpi_get_i386_pci_host(void) > > { > > PCIHostState *host; > > > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson
On Fri, Jul 02, 2021 at 04:55:47PM +0200, Julia Suvorova wrote: > > Doesn't this need to be protected by if (pm->use_acpi_hotplug_bridge) > > ? Otherwise pm->acpi_pci_hotplug won't be initialized. > > Yes, you're right. Although it doesn't affect anything now, it should > be fixed. I'll send a patch on top. This is all in my tree pci branch. Pls base your patchset on that. Thanks!
On Fri, Jul 02, 2021 at 04:55:47PM +0200, Julia Suvorova wrote: > On Thu, Jul 1, 2021 at 6:59 AM David Gibson <david@gibson.dropbear.id.au> wrote: > > > > On Thu, Jun 17, 2021 at 09:07:35PM +0200, Julia Suvorova wrote: > > > Add acpi_pcihp to ich9_pm as part of > > > 'acpi-pci-hotplug-with-bridge-support' option. Set default to false. > > > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com> > > > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > > Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > > > --- > > > hw/i386/acpi-build.h | 1 + > > > include/hw/acpi/ich9.h | 3 ++ > > > hw/acpi/ich9.c | 67 ++++++++++++++++++++++++++++++++++++++++++ > > > hw/acpi/pcihp.c | 5 +++- > > > hw/i386/acpi-build.c | 2 +- > > > 5 files changed, 76 insertions(+), 2 deletions(-) > > > > > > diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h > > > index 487ec7710f..0dce155c8c 100644 > > > --- a/hw/i386/acpi-build.h > > > +++ b/hw/i386/acpi-build.h > > > @@ -10,5 +10,6 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; > > > #define ACPI_PCIHP_BNMR_BASE 0x10 > > > > > > void acpi_setup(void); > > > +Object *acpi_get_i386_pci_host(void); > > > > > > #endif > > > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h > > > index 596120d97f..a329ce43ab 100644 > > > --- a/include/hw/acpi/ich9.h > > > +++ b/include/hw/acpi/ich9.h > > > @@ -24,6 +24,7 @@ > > > #include "hw/acpi/acpi.h" > > > #include "hw/acpi/cpu_hotplug.h" > > > #include "hw/acpi/cpu.h" > > > +#include "hw/acpi/pcihp.h" > > > #include "hw/acpi/memory_hotplug.h" > > > #include "hw/acpi/acpi_dev_interface.h" > > > #include "hw/acpi/tco.h" > > > @@ -55,6 +56,8 @@ typedef struct ICH9LPCPMRegs { > > > AcpiCpuHotplug gpe_cpu; > > > CPUHotplugState cpuhp_state; > > > > > > + bool use_acpi_hotplug_bridge; > > > + AcpiPciHpState acpi_pci_hotplug; > > > MemHotplugState acpi_memory_hotplug; > > > > > > uint8_t disable_s3; > > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > > > index 4daa79ec8d..bcbd567cb0 100644 > > > --- a/hw/acpi/ich9.c > > > +++ b/hw/acpi/ich9.c > > > @@ -217,6 +217,26 @@ static const VMStateDescription vmstate_cpuhp_state = { > > > } > > > }; > > > > > > +static bool vmstate_test_use_pcihp(void *opaque) > > > +{ > > > + ICH9LPCPMRegs *s = opaque; > > > + > > > + return s->use_acpi_hotplug_bridge; > > > +} > > > + > > > +static const VMStateDescription vmstate_pcihp_state = { > > > + .name = "ich9_pm/pcihp", > > > + .version_id = 1, > > > + .minimum_version_id = 1, > > > + .needed = vmstate_test_use_pcihp, > > > + .fields = (VMStateField[]) { > > > + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, > > > + ICH9LPCPMRegs, > > > + NULL, NULL), > > > + VMSTATE_END_OF_LIST() > > > + } > > > +}; > > > + > > > const VMStateDescription vmstate_ich9_pm = { > > > .name = "ich9_pm", > > > .version_id = 1, > > > @@ -238,6 +258,7 @@ const VMStateDescription vmstate_ich9_pm = { > > > &vmstate_memhp_state, > > > &vmstate_tco_io_state, > > > &vmstate_cpuhp_state, > > > + &vmstate_pcihp_state, > > > NULL > > > } > > > }; > > > @@ -259,6 +280,7 @@ static void pm_reset(void *opaque) > > > } > > > pm->smi_en_wmask = ~0; > > > > > > + acpi_pcihp_reset(&pm->acpi_pci_hotplug, true); > > > > Doesn't this need to be protected by if (pm->use_acpi_hotplug_bridge) > > ? Otherwise pm->acpi_pci_hotplug won't be initialized. > > Yes, you're right. Although it doesn't affect anything now, it should > be fixed. I'll send a patch on top. > > > > acpi_update_sci(&pm->acpi_regs, pm->irq); > > > } > > > > > > @@ -297,6 +319,18 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, > > > pm->enable_tco = true; > > > acpi_pm_tco_init(&pm->tco_regs, &pm->io); > > > > > > + if (pm->use_acpi_hotplug_bridge) { > > > + acpi_pcihp_init(OBJECT(lpc_pci), > > > + &pm->acpi_pci_hotplug, > > > + pci_get_bus(lpc_pci), > > > + pci_address_space_io(lpc_pci), > > > + true, > > > + ACPI_PCIHP_ADDR_ICH9); > > > + > > > + qbus_set_hotplug_handler(BUS(pci_get_bus(lpc_pci)), > > > + OBJECT(lpc_pci)); > > > + } > > > + > > > pm->irq = sci_irq; > > > qemu_register_reset(pm_reset, pm); > > > pm->powerdown_notifier.notify = pm_powerdown_req; > > > @@ -368,6 +402,20 @@ static void ich9_pm_set_enable_tco(Object *obj, bool value, Error **errp) > > > s->pm.enable_tco = value; > > > } > > > > > > +static bool ich9_pm_get_acpi_pci_hotplug(Object *obj, Error **errp) > > > +{ > > > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > > > + > > > + return s->pm.use_acpi_hotplug_bridge; > > > +} > > > + > > > +static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value, Error **errp) > > > +{ > > > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > > > + > > > + s->pm.use_acpi_hotplug_bridge = value; > > > +} > > > + > > > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > > > { > > > static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; > > > @@ -376,6 +424,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > > > pm->disable_s3 = 0; > > > pm->disable_s4 = 0; > > > pm->s4_val = 2; > > > + pm->use_acpi_hotplug_bridge = false; > > > > > > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE, > > > &pm->pm_io_base, OBJ_PROP_FLAG_READ); > > > @@ -399,6 +448,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) > > > object_property_add_bool(obj, ACPI_PM_PROP_TCO_ENABLED, > > > ich9_pm_get_enable_tco, > > > ich9_pm_set_enable_tco); > > > + object_property_add_bool(obj, "acpi-pci-hotplug-with-bridge-support", > > > + ich9_pm_get_acpi_pci_hotplug, > > > + ich9_pm_set_acpi_pci_hotplug); > > > } > > > > > > void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > > @@ -406,6 +458,11 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > > { > > > ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev); > > > > > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > > + acpi_pcihp_device_pre_plug_cb(hotplug_dev, dev, errp); > > > + return; > > > + } > > > + > > > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) && > > > !lpc->pm.acpi_memory_hotplug.is_enabled) { > > > error_setg(errp, > > > @@ -441,6 +498,9 @@ void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > > } else { > > > acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, errp); > > > } > > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > > + acpi_pcihp_device_plug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, > > > + dev, errp); > > > } else { > > > error_setg(errp, "acpi: device plug request for not supported device" > > > " type: %s", object_get_typename(OBJECT(dev))); > > > @@ -473,6 +533,10 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev, > > > > > > acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state, > > > dev, errp); > > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > > + acpi_pcihp_device_unplug_request_cb(hotplug_dev, > > > + &lpc->pm.acpi_pci_hotplug, > > > + dev, errp); > > > } else { > > > error_setg(errp, "acpi: device unplug request for not supported device" > > > " type: %s", object_get_typename(OBJECT(dev))); > > > @@ -490,6 +554,9 @@ void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, > > > } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) && > > > !lpc->pm.cpu_hotplug_legacy) { > > > acpi_cpu_unplug_cb(&lpc->pm.cpuhp_state, dev, errp); > > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > > + acpi_pcihp_device_unplug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, > > > + dev, errp); > > > } else { > > > error_setg(errp, "acpi: device unplug for not supported device" > > > " type: %s", object_get_typename(OBJECT(dev))); > > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > > > index a55992ed9f..5355618608 100644 > > > --- a/hw/acpi/pcihp.c > > > +++ b/hw/acpi/pcihp.c > > > @@ -30,6 +30,8 @@ > > > #include "hw/pci-host/i440fx.h" > > > #include "hw/pci/pci.h" > > > #include "hw/pci/pci_bridge.h" > > > +#include "hw/pci/pci_host.h" > > > +#include "hw/i386/acpi-build.h" > > > #include "hw/acpi/acpi.h" > > > #include "hw/pci/pci_bus.h" > > > #include "migration/vmstate.h" > > > @@ -103,6 +105,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) > > > static void acpi_set_pci_info(void) > > > { > > > static bool bsel_is_set; > > > + Object *host = acpi_get_i386_pci_host(); > > > PCIBus *bus; > > > unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT; > > > > > > @@ -111,7 +114,7 @@ static void acpi_set_pci_info(void) > > > } > > > bsel_is_set = true; > > > > > > - bus = find_i440fx(); /* TODO: Q35 support */ > > > + bus = PCI_HOST_BRIDGE(host)->bus; > > > > Would it make more sense for this change to go in patch #1, along with > > the similar change to acpi_pcihp_disable_root_bus(). > > Not really. This is exactly what enables ACPI hotplug, and should be > done with other changes here. The first patch could be merged in this > one, though. Ok, understood.
diff --git a/hw/i386/acpi-build.h b/hw/i386/acpi-build.h index 487ec7710f..0dce155c8c 100644 --- a/hw/i386/acpi-build.h +++ b/hw/i386/acpi-build.h @@ -10,5 +10,6 @@ extern const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio; #define ACPI_PCIHP_BNMR_BASE 0x10 void acpi_setup(void); +Object *acpi_get_i386_pci_host(void); #endif diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h index 596120d97f..a329ce43ab 100644 --- a/include/hw/acpi/ich9.h +++ b/include/hw/acpi/ich9.h @@ -24,6 +24,7 @@ #include "hw/acpi/acpi.h" #include "hw/acpi/cpu_hotplug.h" #include "hw/acpi/cpu.h" +#include "hw/acpi/pcihp.h" #include "hw/acpi/memory_hotplug.h" #include "hw/acpi/acpi_dev_interface.h" #include "hw/acpi/tco.h" @@ -55,6 +56,8 @@ typedef struct ICH9LPCPMRegs { AcpiCpuHotplug gpe_cpu; CPUHotplugState cpuhp_state; + bool use_acpi_hotplug_bridge; + AcpiPciHpState acpi_pci_hotplug; MemHotplugState acpi_memory_hotplug; uint8_t disable_s3; diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 4daa79ec8d..bcbd567cb0 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -217,6 +217,26 @@ static const VMStateDescription vmstate_cpuhp_state = { } }; +static bool vmstate_test_use_pcihp(void *opaque) +{ + ICH9LPCPMRegs *s = opaque; + + return s->use_acpi_hotplug_bridge; +} + +static const VMStateDescription vmstate_pcihp_state = { + .name = "ich9_pm/pcihp", + .version_id = 1, + .minimum_version_id = 1, + .needed = vmstate_test_use_pcihp, + .fields = (VMStateField[]) { + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, + ICH9LPCPMRegs, + NULL, NULL), + VMSTATE_END_OF_LIST() + } +}; + const VMStateDescription vmstate_ich9_pm = { .name = "ich9_pm", .version_id = 1, @@ -238,6 +258,7 @@ const VMStateDescription vmstate_ich9_pm = { &vmstate_memhp_state, &vmstate_tco_io_state, &vmstate_cpuhp_state, + &vmstate_pcihp_state, NULL } }; @@ -259,6 +280,7 @@ static void pm_reset(void *opaque) } pm->smi_en_wmask = ~0; + acpi_pcihp_reset(&pm->acpi_pci_hotplug, true); acpi_update_sci(&pm->acpi_regs, pm->irq); } @@ -297,6 +319,18 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, pm->enable_tco = true; acpi_pm_tco_init(&pm->tco_regs, &pm->io); + if (pm->use_acpi_hotplug_bridge) { + acpi_pcihp_init(OBJECT(lpc_pci), + &pm->acpi_pci_hotplug, + pci_get_bus(lpc_pci), + pci_address_space_io(lpc_pci), + true, + ACPI_PCIHP_ADDR_ICH9); + + qbus_set_hotplug_handler(BUS(pci_get_bus(lpc_pci)), + OBJECT(lpc_pci)); + } + pm->irq = sci_irq; qemu_register_reset(pm_reset, pm); pm->powerdown_notifier.notify = pm_powerdown_req; @@ -368,6 +402,20 @@ static void ich9_pm_set_enable_tco(Object *obj, bool value, Error **errp) s->pm.enable_tco = value; } +static bool ich9_pm_get_acpi_pci_hotplug(Object *obj, Error **errp) +{ + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); + + return s->pm.use_acpi_hotplug_bridge; +} + +static void ich9_pm_set_acpi_pci_hotplug(Object *obj, bool value, Error **errp) +{ + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); + + s->pm.use_acpi_hotplug_bridge = value; +} + void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) { static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; @@ -376,6 +424,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) pm->disable_s3 = 0; pm->disable_s4 = 0; pm->s4_val = 2; + pm->use_acpi_hotplug_bridge = false; object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE, &pm->pm_io_base, OBJ_PROP_FLAG_READ); @@ -399,6 +448,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm) object_property_add_bool(obj, ACPI_PM_PROP_TCO_ENABLED, ich9_pm_get_enable_tco, ich9_pm_set_enable_tco); + object_property_add_bool(obj, "acpi-pci-hotplug-with-bridge-support", + ich9_pm_get_acpi_pci_hotplug, + ich9_pm_set_acpi_pci_hotplug); } void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, @@ -406,6 +458,11 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, { ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev); + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { + acpi_pcihp_device_pre_plug_cb(hotplug_dev, dev, errp); + return; + } + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) && !lpc->pm.acpi_memory_hotplug.is_enabled) { error_setg(errp, @@ -441,6 +498,9 @@ void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, } else { acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, errp); } + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { + acpi_pcihp_device_plug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, + dev, errp); } else { error_setg(errp, "acpi: device plug request for not supported device" " type: %s", object_get_typename(OBJECT(dev))); @@ -473,6 +533,10 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev, acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state, dev, errp); + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { + acpi_pcihp_device_unplug_request_cb(hotplug_dev, + &lpc->pm.acpi_pci_hotplug, + dev, errp); } else { error_setg(errp, "acpi: device unplug request for not supported device" " type: %s", object_get_typename(OBJECT(dev))); @@ -490,6 +554,9 @@ void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) && !lpc->pm.cpu_hotplug_legacy) { acpi_cpu_unplug_cb(&lpc->pm.cpuhp_state, dev, errp); + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { + acpi_pcihp_device_unplug_cb(hotplug_dev, &lpc->pm.acpi_pci_hotplug, + dev, errp); } else { error_setg(errp, "acpi: device unplug for not supported device" " type: %s", object_get_typename(OBJECT(dev))); diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c index a55992ed9f..5355618608 100644 --- a/hw/acpi/pcihp.c +++ b/hw/acpi/pcihp.c @@ -30,6 +30,8 @@ #include "hw/pci-host/i440fx.h" #include "hw/pci/pci.h" #include "hw/pci/pci_bridge.h" +#include "hw/pci/pci_host.h" +#include "hw/i386/acpi-build.h" #include "hw/acpi/acpi.h" #include "hw/pci/pci_bus.h" #include "migration/vmstate.h" @@ -103,6 +105,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) static void acpi_set_pci_info(void) { static bool bsel_is_set; + Object *host = acpi_get_i386_pci_host(); PCIBus *bus; unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT; @@ -111,7 +114,7 @@ static void acpi_set_pci_info(void) } bsel_is_set = true; - bus = find_i440fx(); /* TODO: Q35 support */ + bus = PCI_HOST_BRIDGE(host)->bus; if (bus) { /* Scan all PCI buses. Set property to enable acpi based hotplug. */ pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 67753638e4..aa973aa746 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -299,7 +299,7 @@ static void acpi_get_misc_info(AcpiMiscInfo *info) * Because of the PXB hosts we cannot simply query TYPE_PCI_HOST_BRIDGE. * On i386 arch we only have two pci hosts, so we can look only for them. */ -static Object *acpi_get_i386_pci_host(void) +Object *acpi_get_i386_pci_host(void) { PCIHostState *host;