On Mon, Jun 02, 2014 at 03:25:22PM +0200, Igor Mammedov wrote: > Add memory hotplug initialization/handling to ICH9 LPC device > and enable it by default for post 2.0 machine types > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
I applied this, resolving a conflict. Minor comments below, would like to see them addressed in follow-up patches. > --- > hw/acpi/ich9.c | 38 ++++++++++++++++++++++++++++++++++++++ > hw/isa/lpc_ich9.c | 20 ++++++++++++++++++++ > include/hw/acpi/ich9.h | 4 ++++ > include/hw/i386/pc.h | 7 ++++++- > 4 files changed, 68 insertions(+), 1 deletions(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 0afac42..7b10c27 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -34,6 +34,7 @@ > #include "exec/address-spaces.h" > > #include "hw/i386/ich9.h" > +#include "hw/mem/pc-dimm.h" > > //#define DEBUG > > @@ -224,6 +225,11 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, > &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE); > pm->cpu_added_notifier.notify = ich9_cpu_added_req; > qemu_register_cpu_added_notifier(&pm->cpu_added_notifier); > + > + if (pm->acpi_memory_hotplug.is_enabled) { > + acpi_memory_hotplug_init(pci_address_space_io(lpc_pci), > OBJECT(lpc_pci), > + &pm->acpi_memory_hotplug); > + } > } > > static void ich9_pm_get_gpe0_blk(Object *obj, Visitor *v, > @@ -236,9 +242,25 @@ static void ich9_pm_get_gpe0_blk(Object *obj, Visitor *v, > visit_type_uint32(v, &value, name, errp); > } > > +static bool ich9_pm_get_memory_hotplug_support(Object *obj, Error **errp) > +{ > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > + > + return s->pm.acpi_memory_hotplug.is_enabled; > +} > + > +static void ich9_pm_set_memory_hotplug_support(Object *obj, bool value, > + Error **errp) > +{ > + ICH9LPCState *s = ICH9_LPC_DEVICE(obj); > + > + s->pm.acpi_memory_hotplug.is_enabled = value; > +} > + > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm, Error **errp) > { > static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN; > + pm->acpi_memory_hotplug.is_enabled = true; > > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE, > &pm->pm_io_base, errp); > @@ -247,4 +269,20 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs > *pm, Error **errp) > NULL, NULL, pm, NULL); > object_property_add_uint32_ptr(obj, ACPI_PM_PROP_GPE0_BLK_LEN, > &gpe0_len, errp); > + object_property_add_bool(obj, "memory-hotplug-support", > + ich9_pm_get_memory_hotplug_support, > + ich9_pm_set_memory_hotplug_support, > + NULL); > +} > + > +void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error > **errp) > +{ > + if (pm->acpi_memory_hotplug.is_enabled && > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_plug_cb(&pm->acpi_regs, pm->irq, > &pm->acpi_memory_hotplug, > + dev, errp); > + } else { > + error_setg(errp, "acpi: device plug request for not supported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > } > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c > index 46de3b6..2adf29a 100644 > --- a/hw/isa/lpc_ich9.c > +++ b/hw/isa/lpc_ich9.c > @@ -599,6 +599,19 @@ static int ich9_lpc_init(PCIDevice *d) > return 0; > } > > +static void ich9_device_plug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev); > + > + ich9_pm_device_plug_cb(&lpc->pm, dev, errp); > +} > + > +static void ich9_device_unplug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ Can this set error so user knows this does not work? > +} > + > static bool ich9_rst_cnt_needed(void *opaque) > { > ICH9LPCState *lpc = opaque; > @@ -643,6 +656,7 @@ static void ich9_lpc_class_init(ObjectClass *klass, void > *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > + HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass); > > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > dc->reset = ich9_lpc_reset; > @@ -659,6 +673,8 @@ static void ich9_lpc_class_init(ObjectClass *klass, void > *data) > * pc_q35_init() > */ > dc->cannot_instantiate_with_device_add_yet = true; > + hc->plug = ich9_device_plug_cb; > + hc->unplug = ich9_device_unplug_cb; > } > > static const TypeInfo ich9_lpc_info = { > @@ -667,6 +683,10 @@ static const TypeInfo ich9_lpc_info = { > .instance_size = sizeof(struct ICH9LPCState), > .instance_init = ich9_lpc_initfn, > .class_init = ich9_lpc_class_init, > + .interfaces = (InterfaceInfo[]) { > + { TYPE_HOTPLUG_HANDLER }, > + { } > + } > }; > > static void ich9_lpc_register(void) > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h > index 104f419..1977f1b 100644 > --- a/include/hw/acpi/ich9.h > +++ b/include/hw/acpi/ich9.h > @@ -23,6 +23,7 @@ > > #include "hw/acpi/acpi.h" > #include "hw/acpi/cpu_hotplug.h" > +#include "hw/acpi/memory_hotplug.h" > > typedef struct ICH9LPCPMRegs { > /* > @@ -46,6 +47,8 @@ typedef struct ICH9LPCPMRegs { > > AcpiCpuHotplug gpe_cpu; > Notifier cpu_added_notifier; > + > + MemHotplugState acpi_memory_hotplug; > } ICH9LPCPMRegs; > > void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, > @@ -55,4 +58,5 @@ extern const VMStateDescription vmstate_ich9_pm; > > void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm, Error **errp); > > +void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error > **errp); > #endif /* HW_ACPI_ICH9_H */ > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 34c3a63..cecd4c2 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -286,7 +286,12 @@ int e820_get_num_entries(void); > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > #define PC_Q35_COMPAT_2_0 \ > - PC_COMPAT_2_0 > + PC_COMPAT_2_0, \ > + {\ > + .driver = "ICH9 LPC",\ > + .property = "memory-hotplug-support",\ > + .value = "off",\ > + } > Spaces in names are evil. Let's rename to ICH9_LPC or something. > #define PC_Q35_COMPAT_1_7 \ > PC_COMPAT_1_7, \ > -- > 1.7.1