From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51916) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XPVxP-0001wz-29 for qemu-devel@nongnu.org; Thu, 04 Sep 2014 08:16:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XPVxK-0000Om-Dv for qemu-devel@nongnu.org; Thu, 04 Sep 2014 08:16:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17360) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XPVxK-0000Oe-6q for qemu-devel@nongnu.org; Thu, 04 Sep 2014 08:16:02 -0400 Date: Thu, 4 Sep 2014 14:15:51 +0200 From: Igor Mammedov Message-ID: <20140904141551.740e43fc@nial.usersys.redhat.com> In-Reply-To: <1409126919-22233-2-git-send-email-tangchen@cn.fujitsu.com> References: <1409126919-22233-1-git-send-email-tangchen@cn.fujitsu.com> <1409126919-22233-2-git-send-email-tangchen@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RESEND PATCH v3 1/8] acpi, piix4: Add memory hot unplug support for piix4. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tang Chen Cc: zhugh.fnst@cn.fujitsu.com, mst@redhat.com, hutao@cn.fujitsu.com, qemu-devel@nongnu.org, isimatu.yasuaki@jp.fujitsu.com, pbonzini@redhat.com On Wed, 27 Aug 2014 16:08:32 +0800 Tang Chen wrote: > From: Hu Tao > > 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 > Signed-off-by: Tang Chen > --- > 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) \