From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52185) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1tZU-0005Ar-7Y for qemu-devel@nongnu.org; Wed, 02 Nov 2016 07:19:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1tZR-00076r-HZ for qemu-devel@nongnu.org; Wed, 02 Nov 2016 07:19:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33588) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c1tZR-00075P-AP for qemu-devel@nongnu.org; Wed, 02 Nov 2016 07:19:05 -0400 Date: Wed, 2 Nov 2016 12:19:00 +0100 From: Igor Mammedov Message-ID: <20161102121900.675a4e07@nial.brq.redhat.com> In-Reply-To: <1477850917-1214-40-git-send-email-mst@redhat.com> References: <1477850917-1214-1-git-send-email-mst@redhat.com> <1477850917-1214-40-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, Peter Maydell , Xiao Guangrong , Paolo Bonzini , Richard Henderson , Eduardo Habkost 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. > + 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. > mdev->is_removing = true; > acpi_send_event(DEVICE(hotplug_dev), ACPI_MEMORY_HOTPLUG_STATUS); > } > @@ -276,6 +289,8 @@ void acpi_memory_unplug_cb(MemHotplugState *mem_st, > return; > } > > + /* nvdimm device hot unplug is not supported yet. */ > + assert(!object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)); ditto > mdev->is_enabled = false; > mdev->dimm = NULL; > } > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index 03a5386..7aaa07a 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2069,6 +2069,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > method = aml_method("_E03", 0, AML_NOTSERIALIZED); > aml_append(method, aml_call0(MEMORY_HOTPLUG_HANDLER_PATH)); > aml_append(scope, method); > + > + if (pcms->acpi_nvdimm_state.is_enabled) { > + method = aml_method("_E04", 0, AML_NOTSERIALIZED); > + aml_append(method, aml_notify(aml_name("\\_SB.NVDR"), > + aml_int(0x80))); > + aml_append(scope, method); > + } > } > aml_append(dsdt, scope); > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index b395717..c011552 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1744,6 +1744,12 @@ static void pc_dimm_unplug_request(HotplugHandler *hotplug_dev, > goto out; > } > > + if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) { > + error_setg(&local_err, > + "nvdimm device hot unplug is not supported yet."); > + goto out; > + } > + > hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev); > hhc->unplug_request(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err); > > @@ -1761,6 +1767,12 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev, > HotplugHandlerClass *hhc; > Error *local_err = NULL; > > + if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) { > + error_setg(&local_err, > + "nvdimm device hot unplug is not supported yet."); > + goto out; > + } > + > hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev); > hhc->unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err); > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > index 7895805..db896b0 100644 > --- a/hw/mem/nvdimm.c > +++ b/hw/mem/nvdimm.c > @@ -148,13 +148,9 @@ static MemoryRegion *nvdimm_get_vmstate_memory_region(PCDIMMDevice *dimm) > > static void nvdimm_class_init(ObjectClass *oc, void *data) > { > - DeviceClass *dc = DEVICE_CLASS(oc); > PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc); > NVDIMMClass *nvc = NVDIMM_CLASS(oc); > > - /* nvdimm hotplug has not been supported yet. */ > - dc->hotpluggable = false; > - > ddc->realize = nvdimm_realize; > ddc->get_memory_region = nvdimm_get_memory_region; > ddc->get_vmstate_memory_region = nvdimm_get_vmstate_memory_region; > diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt > index 3df3620..cb26dd2 100644 > --- a/docs/specs/acpi_mem_hotplug.txt > +++ b/docs/specs/acpi_mem_hotplug.txt > @@ -4,6 +4,9 @@ QEMU<->ACPI BIOS memory hotplug interface > ACPI BIOS GPE.3 handler is dedicated for notifying OS about memory hot-add > and hot-remove events. > > +ACPI BIOS GPE.4 handler is dedicated for notifying OS about nvdimm device > +hot-add and hot-remove events. > + > Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access): > --------------------------------------------------------------- > 0xa00: