From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60848) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ya06q-00051Q-Ch for qemu-devel@nongnu.org; Mon, 23 Mar 2015 07:01:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ya06m-0005iG-8U for qemu-devel@nongnu.org; Mon, 23 Mar 2015 07:01:28 -0400 Received: from [59.151.112.132] (port=61673 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ya06k-0005fj-4E for qemu-devel@nongnu.org; Mon, 23 Mar 2015 07:01:24 -0400 Message-ID: <550FF210.7090400@cn.fujitsu.com> Date: Mon, 23 Mar 2015 18:59:28 +0800 From: Zhu Guihua MIME-Version: 1.0 References: <3509802a09b0e89aa9c169ba7a25e624e826ba5a.1426494342.git.zhugh.fnst@cn.fujitsu.com> <20150316155933.1186d483@nial.brq.redhat.com> In-Reply-To: <20150316155933.1186d483@nial.brq.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RESEND PATCH v4 6/6] acpi: Add hardware implementation for memory hot unplug List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: mst@redhat.com, qemu-devel@nongnu.org, tangchen@cn.fujitsu.com, izumi.taku@jp.fujitsu.com, guz.fnst@cn.fujitsu.com, pbonzini@redhat.com On 03/16/2015 10:59 PM, Igor Mammedov wrote: [...] > > diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl > index 1e9ec39..ef847e2 100644 > --- a/hw/i386/acpi-dsdt-mem-hotplug.dsl > +++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl > @@ -29,6 +29,7 @@ > External(MEMORY_SLOT_PROXIMITY, FieldUnitObj) // read only > External(MEMORY_SLOT_ENABLED, FieldUnitObj) // 1 if enabled, read only > External(MEMORY_SLOT_INSERT_EVENT, FieldUnitObj) // (read) 1 if has a insert event. (write) 1 to clear event > + External(MEMORY_SLOT_REMOVE_EVENT, FieldUnitObj) // (read) 1 if has a remove event. (write) 1 to clear event > External(MEMORY_SLOT_SLECTOR, FieldUnitObj) // DIMM selector, write only > External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST event code, write only > External(MEMORY_SLOT_OST_STATUS, FieldUnitObj) // _OST status code, write only > @@ -55,6 +56,8 @@ > If (LEqual(MEMORY_SLOT_INSERT_EVENT, One)) { // Memory device needs check > MEMORY_SLOT_NOTIFY_METHOD(Local0, 1) > Store(1, MEMORY_SLOT_INSERT_EVENT) > + } Elseif (LEqual(MEMORY_SLOT_REMOVE_EVENT, One)) { // Ejection request > + MEMORY_SLOT_NOTIFY_METHOD(Local0, 3) > clear removing field here. You mean clear remove event here? > >> } >> // TODO: handle memory eject request >> Add(Local0, One, Local0) // goto next DIMM >> @@ -156,5 +159,12 @@ >> Store(Arg2, MEMORY_SLOT_OST_STATUS) >> Release(MEMORY_SLOT_LOCK) >> } >> + >> + Method(MEMORY_SLOT_EJECT_METHOD, 2) { >> + Acquire(MEMORY_SLOT_LOCK, 0xFFFF) >> + Store(ToInteger(Arg0), MEMORY_SLOT_SLECTOR) // select DIMM >> + Store(One, MEMORY_SLOT_REMOVE_EVENT) > redo it using enabled field. Otherwise it could cause guest/QEMU crash: > > - if 1st memory was asked to be removed > - then OSPM processes it but has not called _EJ0 yet leaving is_removed set > - then QEMU adds/removes another DIMM triggering slots scan > which would issue second Notify(remove) since is_removed is still set > - as result it will cause failure in OSPM or in QEMU if OSPM issues double EJ0() > If OSPM processed the ejection request but not called _EJ0, the device will still be present in qemu, we must handle this. So I think OSPM issues double EJ0 maybe reasonable in this situation. What's your opinion? Thanks, Zhu >> + Release(MEMORY_SLOT_LOCK) >> + } >> } // Device() >> } // Scope() >> diff --git a/include/hw/acpi/pc-hotplug.h b/include/hw/acpi/pc-hotplug.h >> index efa6ed7..680810b 100644 >> --- a/include/hw/acpi/pc-hotplug.h >> +++ b/include/hw/acpi/pc-hotplug.h >> @@ -43,6 +43,7 @@ >> #define MEMORY_SLOT_PROXIMITY MPX >> #define MEMORY_SLOT_ENABLED MES >> #define MEMORY_SLOT_INSERT_EVENT MINS >> +#define MEMORY_SLOT_REMOVE_EVENT MRMV >> #define MEMORY_SLOT_SLECTOR MSEL >> #define MEMORY_SLOT_OST_EVENT MOEV >> #define MEMORY_SLOT_OST_STATUS MOSC >> @@ -51,6 +52,7 @@ >> #define MEMORY_SLOT_CRS_METHOD MCRS >> #define MEMORY_SLOT_OST_METHOD MOST >> #define MEMORY_SLOT_PROXIMITY_METHOD MPXM >> +#define MEMORY_SLOT_EJECT_METHOD MEJ0 >> #define MEMORY_SLOT_NOTIFY_METHOD MTFY >> #define MEMORY_SLOT_SCAN_METHOD MSCN >> >> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h >> index 4e673f9..5b7acf1 100644 >> --- a/include/hw/qdev-core.h >> +++ b/include/hw/qdev-core.h >> @@ -266,6 +266,7 @@ int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT; >> void qdev_init_nofail(DeviceState *dev); >> void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, >> int required_for_version); >> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev); >> void qdev_unplug(DeviceState *dev, Error **errp); >> void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, >> DeviceState *dev, Error **errp); >> diff --git a/trace-events b/trace-events >> index 30eba92..e552355 100644 >> --- a/trace-events >> +++ b/trace-events >> @@ -1572,6 +1572,7 @@ mhp_acpi_write_slot(uint32_t slot) "set active slot: 0x%"PRIx32 >> mhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "slot[0x%"PRIx32"] OST EVENT: 0x%"PRIx32 >> mhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "slot[0x%"PRIx32"] OST STATUS: 0x%"PRIx32 >> mhp_acpi_clear_insert_evt(uint32_t slot) "slot[0x%"PRIx32"] clear insert event" >> +mhp_acpi_clear_remove_evt(uint32_t slot) "slot[0x%"PRIx32"] clear remove event" >> >> # hw/i386/pc.c >> mhp_pc_dimm_assigned_slot(int slot) "0x%d" > . >