Re: [Qemu-devel] [PATCH v2 5/8] pc-dimm: introduce prepare_unplug() callback

[Igor Mammedov]

On Fri, 12 Aug 2016 14:54:07 +0800
Xiao Guangrong <address@hidden> 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 <address@hidden>
> ---
>  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;
>  
>  /**