diff mbox

[v2,5/8] pc-dimm: introduce prepare_unplug() callback

Message ID 1470984850-66891-6-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Xiao Guangrong Aug. 12, 2016, 6:54 a.m. UTC 
We should let nvdimm acpi know which nvdimm device is being unplugged
before QEMU interrupts the guest so that nvdimm acpi can update its
FIT properly

prepare_unplug() callback is introduced exactly for this purpose then
the being removed device is skipped when guest reads FIT

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/ich9.c           | 3 +++
 hw/acpi/nvdimm.c         | 9 +++++++++
 hw/acpi/piix4.c          | 3 +++
 hw/mem/nvdimm.c          | 8 ++++++++
 hw/mem/pc-dimm.c         | 5 +++++
 include/hw/mem/nvdimm.h  | 3 +++
 include/hw/mem/pc-dimm.h | 1 +
 7 files changed, 32 insertions(+)

Comments

Igor Mammedov Oct. 3, 2016, 9:45 a.m. UTC | #1 
On Fri, 12 Aug 2016 14:54:07 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:

> We should let nvdimm acpi know which nvdimm device is being unplugged
> before QEMU interrupts the guest so that nvdimm acpi can update its
> FIT properly
> 
> prepare_unplug() callback is introduced exactly for this purpose then
> the being removed device is skipped when guest reads FIT
Hot-unplug for NVDIMMs isn't mentinoned in spec so I'd would drop it for now.

> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/acpi/ich9.c           | 3 +++
>  hw/acpi/nvdimm.c         | 9 +++++++++
>  hw/acpi/piix4.c          | 3 +++
>  hw/mem/nvdimm.c          | 8 ++++++++
>  hw/mem/pc-dimm.c         | 5 +++++
>  include/hw/mem/nvdimm.h  | 3 +++
>  include/hw/mem/pc-dimm.h | 1 +
>  7 files changed, 32 insertions(+)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index e5a3c18..af08471 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -511,6 +511,9 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>  
>      if (lpc->pm.acpi_memory_hotplug.is_enabled &&
>          object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> +        PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dev);
> +
> +        ddc->prepare_unplug(dev);
>          acpi_memory_unplug_request_cb(hotplug_dev,
>                                        &lpc->pm.acpi_memory_hotplug, dev,
>                                        errp);
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index f6f8413..0f569c7 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -355,6 +355,15 @@ static GArray *nvdimm_build_device_structure(GSList *device_list)
>  
>      for (; device_list; device_list = device_list->next) {
>          DeviceState *dev = device_list->data;
> +        NVDIMMDevice *nvdimm = NVDIMM(dev);
> +
> +        /*
> +         * the nvdimm device which is being removed should not be included
> +         * in nfit/fit.
> +         */
> +        if (nvdimm->is_removing) {
> +            continue;
> +        }
>  
>          /* build System Physical Address Range Structure. */
>          nvdimm_build_structure_spa(structures, dev);
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 2adc246..57e19e6 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -400,6 +400,9 @@ static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>  
>      if (s->acpi_memory_hotplug.is_enabled &&
>          object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> +        PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dev);
> +
> +        ddc->prepare_unplug(dev);
>          acpi_memory_unplug_request_cb(hotplug_dev, &s->acpi_memory_hotplug,
>                                        dev, errp);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index 7895805..5bc81fe 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -146,6 +146,13 @@ static MemoryRegion *nvdimm_get_vmstate_memory_region(PCDIMMDevice *dimm)
>      return host_memory_backend_get_memory(dimm->hostmem, &error_abort);
>  }
>  
> +static void nvdimm_prepare_unplug(DeviceState *dev)
> +{
> +    NVDIMMDevice *nvdimm = NVDIMM(dev);
> +
> +    nvdimm->is_removing = true;
> +}
> +
>  static void nvdimm_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -158,6 +165,7 @@ static void nvdimm_class_init(ObjectClass *oc, void *data)
>      ddc->realize = nvdimm_realize;
>      ddc->get_memory_region = nvdimm_get_memory_region;
>      ddc->get_vmstate_memory_region = nvdimm_get_vmstate_memory_region;
> +    ddc->prepare_unplug = nvdimm_prepare_unplug;
>  
>      nvc->read_label_data = nvdimm_read_label_data;
>      nvc->write_label_data = nvdimm_write_label_data;
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 9e8dab0..09364c9 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -436,6 +436,10 @@ static MemoryRegion *pc_dimm_get_vmstate_memory_region(PCDIMMDevice *dimm)
>      return host_memory_backend_get_memory(dimm->hostmem, &error_abort);
>  }
>  
> +static  void pc_dimm_prepare_unplug(DeviceState *dev)
> +{
> +}
> +
>  static void pc_dimm_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -448,6 +452,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data)
>  
>      ddc->get_memory_region = pc_dimm_get_memory_region;
>      ddc->get_vmstate_memory_region = pc_dimm_get_vmstate_memory_region;
> +    ddc->prepare_unplug = pc_dimm_prepare_unplug;
>  }
>  
>  static TypeInfo pc_dimm_info = {
> diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
> index 63a2b20..e626199 100644
> --- a/include/hw/mem/nvdimm.h
> +++ b/include/hw/mem/nvdimm.h
> @@ -71,6 +71,9 @@ struct NVDIMMDevice {
>       * guest via ACPI NFIT and _FIT method if NVDIMM hotplug is supported.
>       */
>      MemoryRegion nvdimm_mr;
> +
> +    /* the device is being unplugged. */
> +    bool is_removing;
>  };
>  typedef struct NVDIMMDevice NVDIMMDevice;
>  
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index 1e483f2..34797ba 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -73,6 +73,7 @@ typedef struct PCDIMMDeviceClass {
>      void (*realize)(PCDIMMDevice *dimm, Error **errp);
>      MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm);
>      MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm);
> +    void (*prepare_unplug)(DeviceState *dev);
>  } PCDIMMDeviceClass;
>  
>  /**

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Xiao Guangrong Oct. 8, 2016, 7:20 a.m. UTC | #2 
On 10/03/2016 05:45 PM, Igor Mammedov wrote:
> On Fri, 12 Aug 2016 14:54:07 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> We should let nvdimm acpi know which nvdimm device is being unplugged
>> before QEMU interrupts the guest so that nvdimm acpi can update its
>> FIT properly
>>
>> prepare_unplug() callback is introduced exactly for this purpose then
>> the being removed device is skipped when guest reads FIT
> Hot-unplug for NVDIMMs isn't mentinoned in spec so I'd would drop it for now.

Okay. Then i will stop hot-unplug and generate a error message if nvdimm device
is being un-plugged:
   "NVDIMM hot-unplug has not been supported yet."

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index e5a3c18..af08471 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -511,6 +511,9 @@  void ich9_pm_device_unplug_request_cb(HotplugHandler *hotplug_dev,
 
     if (lpc->pm.acpi_memory_hotplug.is_enabled &&
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+        PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dev);
+
+        ddc->prepare_unplug(dev);
         acpi_memory_unplug_request_cb(hotplug_dev,
                                       &lpc->pm.acpi_memory_hotplug, dev,
                                       errp);
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index f6f8413..0f569c7 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -355,6 +355,15 @@  static GArray *nvdimm_build_device_structure(GSList *device_list)
 
     for (; device_list; device_list = device_list->next) {
         DeviceState *dev = device_list->data;
+        NVDIMMDevice *nvdimm = NVDIMM(dev);
+
+        /*
+         * the nvdimm device which is being removed should not be included
+         * in nfit/fit.
+         */
+        if (nvdimm->is_removing) {
+            continue;
+        }
 
         /* build System Physical Address Range Structure. */
         nvdimm_build_structure_spa(structures, dev);
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 2adc246..57e19e6 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -400,6 +400,9 @@  static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev,
 
     if (s->acpi_memory_hotplug.is_enabled &&
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+        PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dev);
+
+        ddc->prepare_unplug(dev);
         acpi_memory_unplug_request_cb(hotplug_dev, &s->acpi_memory_hotplug,
                                       dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 7895805..5bc81fe 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -146,6 +146,13 @@  static MemoryRegion *nvdimm_get_vmstate_memory_region(PCDIMMDevice *dimm)
     return host_memory_backend_get_memory(dimm->hostmem, &error_abort);
 }
 
+static void nvdimm_prepare_unplug(DeviceState *dev)
+{
+    NVDIMMDevice *nvdimm = NVDIMM(dev);
+
+    nvdimm->is_removing = true;
+}
+
 static void nvdimm_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -158,6 +165,7 @@  static void nvdimm_class_init(ObjectClass *oc, void *data)
     ddc->realize = nvdimm_realize;
     ddc->get_memory_region = nvdimm_get_memory_region;
     ddc->get_vmstate_memory_region = nvdimm_get_vmstate_memory_region;
+    ddc->prepare_unplug = nvdimm_prepare_unplug;
 
     nvc->read_label_data = nvdimm_read_label_data;
     nvc->write_label_data = nvdimm_write_label_data;
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 9e8dab0..09364c9 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -436,6 +436,10 @@  static MemoryRegion *pc_dimm_get_vmstate_memory_region(PCDIMMDevice *dimm)
     return host_memory_backend_get_memory(dimm->hostmem, &error_abort);
 }
 
+static  void pc_dimm_prepare_unplug(DeviceState *dev)
+{
+}
+
 static void pc_dimm_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -448,6 +452,7 @@  static void pc_dimm_class_init(ObjectClass *oc, void *data)
 
     ddc->get_memory_region = pc_dimm_get_memory_region;
     ddc->get_vmstate_memory_region = pc_dimm_get_vmstate_memory_region;
+    ddc->prepare_unplug = pc_dimm_prepare_unplug;
 }
 
 static TypeInfo pc_dimm_info = {
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 63a2b20..e626199 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -71,6 +71,9 @@  struct NVDIMMDevice {
      * guest via ACPI NFIT and _FIT method if NVDIMM hotplug is supported.
      */
     MemoryRegion nvdimm_mr;
+
+    /* the device is being unplugged. */
+    bool is_removing;
 };
 typedef struct NVDIMMDevice NVDIMMDevice;
 
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 1e483f2..34797ba 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -73,6 +73,7 @@  typedef struct PCDIMMDeviceClass {
     void (*realize)(PCDIMMDevice *dimm, Error **errp);
     MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm);
     MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm);
+    void (*prepare_unplug)(DeviceState *dev);
 } PCDIMMDeviceClass;
 
 /**