On Mon, 16 Mar 2015 16:58:14 +0800 Zhu Guihua <zhugh.f...@cn.fujitsu.com> wrote:
> From: Tang Chen <tangc...@cn.fujitsu.com> > > Memory hot unplug are both asynchronous procedures. > When the unplug operation happens, unplug request cb is called first. > And when guest OS finished handling unplug, unplug cb will be called > to do the real removal of device. Since unplug is rather complicated multi-stage process, it's worth to describe in in acpi_mem_hotplug.txt rather then put it in commit message here. Preferably with process flow diagram. > > This patch adds unplug request cb for memory device, and adds the > is_removing boolean field to MemStatus. This field is used to indicate > whether the memory slot is being removed. This field is set to true in > acpi_memory_unplug_request_cb(). > > Signed-off-by: Tang Chen <tangc...@cn.fujitsu.com> > Signed-off-by: Zhu Guihua <zhugh.f...@cn.fujitsu.com> 'is_removing' field is introduced here without associated documentation about how it's used and then in 6/6 it's usage is extended and some docs are added. Perhaps it would be better to put documentation patch here or as a separate one before this patch so that reviewers would know intended use of new fields that are added here in advance. Also documentation in 6/6 is not complete. It adds 'removing' event description but it doesn't amend following text with unplug part: "ACPI BIOS GPE.3 handler is dedicated for notifying OS about memory hot-add events." > --- > hw/acpi/ich9.c | 10 ++++++++-- > hw/acpi/memory_hotplug.c | 19 +++++++++++++++++++ > hw/acpi/piix4.c | 6 +++++- > include/hw/acpi/memory_hotplug.h | 4 ++++ > 4 files changed, 36 insertions(+), 3 deletions(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 5352e19..b85eed4 100644 > --- a/hw/acpi/ich9.c > +++ b/hw/acpi/ich9.c > @@ -400,8 +400,14 @@ void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, > DeviceState *dev, Error **errp) > void ich9_pm_device_unplug_request_cb(ICH9LPCPMRegs *pm, DeviceState *dev, > Error **errp) > { > - error_setg(errp, "acpi: device unplug request for not supported device" > - " type: %s", object_get_typename(OBJECT(dev))); > + if (pm->acpi_memory_hotplug.is_enabled && > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_unplug_request_cb(&pm->acpi_regs, pm->irq, > + &pm->acpi_memory_hotplug, dev, errp); > + } else { > + error_setg(errp, "acpi: device unplug request for not supported > device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > } > > void ich9_pm_device_unplug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c > index 0efc357..2ef6a94 100644 > --- a/hw/acpi/memory_hotplug.c > +++ b/hw/acpi/memory_hotplug.c > @@ -75,6 +75,7 @@ static uint64_t acpi_memory_hotplug_read(void *opaque, > hwaddr addr, > case 0x14: /* pack and return is_* fields */ > val |= mdev->is_enabled ? 1 : 0; > val |= mdev->is_inserting ? 2 : 0; > + val |= mdev->is_removing ? 4 : 0; > trace_mhp_acpi_read_flags(mem_st->selector, val); > break; > default: > @@ -208,6 +209,24 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, > MemHotplugState *mem_st, > return; > } > > +void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq, > + MemHotplugState *mem_st, > + DeviceState *dev, Error **errp) > +{ > + MemStatus *mdev; > + > + mdev = acpi_memory_slot_status(mem_st, dev, errp); > + if (!mdev) { > + return; > + } > + > + mdev->is_removing = true; > + > + /* Do ACPI magic */ > + ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS; > + acpi_update_sci(ar, irq); > +} > + > 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 d1f1179..f716e91 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -361,7 +361,11 @@ static void > piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev, > { > PIIX4PMState *s = PIIX4_PM(hotplug_dev); > > - if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + if (s->acpi_memory_hotplug.is_enabled && > + object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + acpi_memory_unplug_request_cb(&s->ar, s->irq, > &s->acpi_memory_hotplug, > + dev, errp); > + } else 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 { > diff --git a/include/hw/acpi/memory_hotplug.h > b/include/hw/acpi/memory_hotplug.h > index 7bbf8a0..c437a85 100644 > --- a/include/hw/acpi/memory_hotplug.h > +++ b/include/hw/acpi/memory_hotplug.h > @@ -11,6 +11,7 @@ typedef struct MemStatus { > DeviceState *dimm; > bool is_enabled; > bool is_inserting; > + bool is_removing; > uint32_t ost_event; > uint32_t ost_status; > } MemStatus; > @@ -28,6 +29,9 @@ 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_request_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) \