On Wed, 27 Aug 2014 16:08:32 +0800
Tang Chen <tangc...@cn.fujitsu.com> wrote:
> From: Hu Tao <hu...@cn.fujitsu.com>
>
> Implement acpi_memory_unplug_cb(), sending an sci to guest to trigger
> memory hot-remove, and call it in piix4_device_unplug_cb().
>
> Signed-off-by: Hu Tao <hu...@cn.fujitsu.com>
> Signed-off-by: Tang Chen <tangc...@cn.fujitsu.com>
> ---
> hw/acpi/memory_hotplug.c | 25 +++++++++++++++++++++++++
> hw/acpi/piix4.c | 3 +++
> include/hw/acpi/memory_hotplug.h | 2 ++
> 3 files changed, 30 insertions(+)
>
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index ed39241..c310b44 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -195,6 +195,31 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq,
> MemHotplugState *mem_st,
> return;
> }
>
> +void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState
> *mem_st,
> + DeviceState *dev, Error **errp)
> +{
> + Error *local_err = NULL;
> + int slot = object_property_get_int(OBJECT(dev), "slot", &local_err);
might be worth to use PC_DIMM_SLOT_PROP here, and while at it
do the same to acpi_memory_plug_cb() as well in separate patch.
> +
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + if (slot >= mem_st->dev_count) {
> + char *dev_path = object_get_canonical_path(OBJECT(dev));
> + error_setg(errp, "acpi_memory_plug_cb: "
> + "device [%s] returned invalid memory slot[%d]",
> + dev_path, slot);
> + g_free(dev_path);
> + return;
> + }
move 3 above blocks into separate function to reduce code duplication
with acpi_memory_plug_cb(), perhaps something like following:
MemStatus *
acpi_memory_get_slot_status_descriptor(MemHotplugState *mem_st,
DeviceState *dev, Error **errp);
>+
place here also missing removal signaling for selected mdev,
perhaps it's due to wrong patch ordering
> + /* do ACPI magic */
> + ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
> + acpi_update_sci(ar, irq);
worth to make this block a separate function and reuse in
acpi_memory_plug_cb()
> +}
> +
> static const VMStateDescription vmstate_memhp_sts = {
> .name = "memory hotplug device state",
> .version_id = 1,
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index b72b34e..c1d5187 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -362,6 +362,9 @@ static void piix4_device_unplug_cb(HotplugHandler
> *hotplug_dev,
> if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> acpi_pcihp_device_unplug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug,
> dev,
> errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> + acpi_memory_unplug_cb(&s->ar, s->irq, &s->acpi_memory_hotplug, dev,
> + errp);
> } else {
> error_setg(errp, "acpi: device unplug request for not supported
> device"
> " type: %s", object_get_typename(OBJECT(dev)));
> diff --git a/include/hw/acpi/memory_hotplug.h
> b/include/hw/acpi/memory_hotplug.h
> index 7bbf8a0..fc6b868 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -28,6 +28,8 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object
> *owner,
>
> void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
> DeviceState *dev, Error **errp);
> +void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState
> *mem_st,
> + DeviceState *dev, Error **errp);
>
> extern const VMStateDescription vmstate_memory_hotplug;
> #define VMSTATE_MEMORY_HOTPLUG(memhp, state) \
