From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54251) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YS7cq-0005ue-Dz for qemu-devel@nongnu.org; Sun, 01 Mar 2015 12:25:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YS7cl-00069L-Fe for qemu-devel@nongnu.org; Sun, 01 Mar 2015 12:25:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51654) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YS7cl-00069H-7x for qemu-devel@nongnu.org; Sun, 01 Mar 2015 12:25:51 -0500 Date: Sun, 1 Mar 2015 18:25:40 +0100 From: "Michael S. Tsirkin" Message-ID: <20150301172540.GE8233@redhat.com> References: <18b8af3c6f5552a337ec922d372a69774a205ff5.1424912878.git.zhugh.fnst@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <18b8af3c6f5552a337ec922d372a69774a205ff5.1424912878.git.zhugh.fnst@cn.fujitsu.com> Subject: Re: [Qemu-devel] [PATCH v3 04/10] acpi, mem-hotplug: Add unplug request cb for memory device. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhu Guihua Cc: hutao@cn.fujitsu.com, qemu-devel@nongnu.org, tangchen@cn.fujitsu.com, isimatu.yasuaki@jp.fujitsu.com, pbonzini@redhat.com, guz.fnst@cn.fujitsu.com, imammedo@redhat.com On Thu, Feb 26, 2015 at 09:16:46AM +0800, Zhu Guihua wrote: > From: Tang Chen > > Memory hot unplug are both asynchronize procedures. asynchronous > When the unplug operation happens, unplug request cb is called first. > And when ghest guest > OS finished handling unplug, unplug cb will be called > to do the real removal of device. > > This patch adds unplug request cb for memory device. It does not such thing apparently. It just implements an unused function - I guess it will be used as a callback by some other patch. > Add a new bool > member named is_removing to MemStatus indicating that the memory slot > is being removed. Set it to true in acpi_memory_unplug_request_cb(), > and send SCI to guest. You describe what code does in great detail but not why it does it. There's a new field that is ever only set to true and never read. I guess this will make sense eventually with follow-up patches but why not include it all there? > > Signed-off-by: Tang Chen > Signed-off-by: Zhu Guihua > --- > hw/acpi/memory_hotplug.c | 17 +++++++++++++++++ > include/hw/acpi/memory_hotplug.h | 4 ++++ > 2 files changed, 21 insertions(+) > > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c > index 5b13baa..02231d2 100644 > --- a/hw/acpi/memory_hotplug.c > +++ b/hw/acpi/memory_hotplug.c > @@ -206,6 +206,23 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st, > acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS); > } > > +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 */ > + acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS); > +} > + > static const VMStateDescription vmstate_memhp_sts = { > .name = "memory hotplug device state", > .version_id = 1, > 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) \ > -- > 1.9.3