From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53021) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1y59-0008FM-Oa for qemu-devel@nongnu.org; Wed, 02 Nov 2016 12:08:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1y54-0007OU-PG for qemu-devel@nongnu.org; Wed, 02 Nov 2016 12:08:07 -0400 Received: from mga01.intel.com ([192.55.52.88]:41891) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1y54-0007Nl-IR for qemu-devel@nongnu.org; Wed, 02 Nov 2016 12:08:02 -0400 References: <1477850917-1214-1-git-send-email-mst@redhat.com> <1477850917-1214-40-git-send-email-mst@redhat.com> <20161102121900.675a4e07@nial.brq.redhat.com> From: Xiao Guangrong Message-ID: Date: Thu, 3 Nov 2016 00:00:45 +0800 MIME-Version: 1.0 In-Reply-To: <20161102121900.675a4e07@nial.brq.redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 39/47] pc: memhp: enable nvdimm device hotplug List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov , "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, Peter Maydell , Paolo Bonzini , Richard Henderson , Eduardo Habkost On 11/02/2016 07:19 PM, Igor Mammedov wrote: > On Sun, 30 Oct 2016 23:25:10 +0200 > "Michael S. Tsirkin" wrote: > >> From: Xiao Guangrong >> >> _GPE.E04 is dedicated for nvdimm device hotplug >> >> Signed-off-by: Xiao Guangrong >> Reviewed-by: Michael S. Tsirkin >> Signed-off-by: Michael S. Tsirkin >> --- >> include/hw/acpi/acpi_dev_interface.h | 1 + >> hw/acpi/memory_hotplug.c | 31 +++++++++++++++++++++++-------- >> hw/i386/acpi-build.c | 7 +++++++ >> hw/i386/pc.c | 12 ++++++++++++ >> hw/mem/nvdimm.c | 4 ---- >> docs/specs/acpi_mem_hotplug.txt | 3 +++ >> 6 files changed, 46 insertions(+), 12 deletions(-) >> >> diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h >> index da4ef7f..901a4ae 100644 >> --- a/include/hw/acpi/acpi_dev_interface.h >> +++ b/include/hw/acpi/acpi_dev_interface.h >> @@ -10,6 +10,7 @@ typedef enum { >> ACPI_PCI_HOTPLUG_STATUS = 2, >> ACPI_CPU_HOTPLUG_STATUS = 4, >> ACPI_MEMORY_HOTPLUG_STATUS = 8, >> + ACPI_NVDIMM_HOTPLUG_STATUS = 16, >> } AcpiEventStatusBits; >> >> #define TYPE_ACPI_DEVICE_IF "acpi-device-interface" >> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c >> index ec4e64b..70f6451 100644 >> --- a/hw/acpi/memory_hotplug.c >> +++ b/hw/acpi/memory_hotplug.c >> @@ -2,6 +2,7 @@ >> #include "hw/acpi/memory_hotplug.h" >> #include "hw/acpi/pc-hotplug.h" >> #include "hw/mem/pc-dimm.h" >> +#include "hw/mem/nvdimm.h" >> #include "hw/boards.h" >> #include "hw/qdev-core.h" >> #include "trace.h" >> @@ -232,11 +233,8 @@ void acpi_memory_plug_cb(HotplugHandler *hotplug_dev, MemHotplugState *mem_st, >> DeviceState *dev, Error **errp) >> { >> MemStatus *mdev; >> - DeviceClass *dc = DEVICE_GET_CLASS(dev); >> - >> - if (!dc->hotpluggable) { >> - return; >> - } > shouldn't be removed. Well, all memory devices support hotplug now, i thought it is reasonable to drop it, actually it was introduced by nvdimm... > >> + AcpiEventStatusBits event; >> + bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); >> >> mdev = acpi_memory_slot_status(mem_st, dev, errp); >> if (!mdev) { >> @@ -244,10 +242,23 @@ void acpi_memory_plug_cb(HotplugHandler *hotplug_dev, MemHotplugState *mem_st, >> } >> >> mdev->dimm = dev; >> - mdev->is_enabled = true; >> + >> + /* >> + * do not set is_enabled and is_inserting if the slot is plugged with >> + * a nvdimm device to stop OSPM inquires memory region from the slot. >> + */ >> + if (is_nvdimm) { >> + event = ACPI_NVDIMM_HOTPLUG_STATUS; >> + } else { >> + mdev->is_enabled = true; >> + event = ACPI_MEMORY_HOTPLUG_STATUS; >> + } >> + >> if (dev->hotplugged) { >> - mdev->is_inserting = true; >> - acpi_send_event(DEVICE(hotplug_dev), ACPI_MEMORY_HOTPLUG_STATUS); >> + if (!is_nvdimm) { >> + mdev->is_inserting = true; >> + } >> + acpi_send_event(DEVICE(hotplug_dev), event); >> } >> } >> >> @@ -262,6 +273,8 @@ void acpi_memory_unplug_request_cb(HotplugHandler *hotplug_dev, >> return; >> } >> >> + /* nvdimm device hot unplug is not supported yet. */ >> + assert(!object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)); > that would crash guest instead of failing gracefully as it's supposed to. This is really a bug if it is triggered as nvdimm hot unplug is stopped in the upper caller.