[Qemu-devel] [PATCH v2 00/15] PATCH 00/15] NVDIMM: introduce nvdimm label support

[Qemu-devel] [PATCH v2 00/15] PATCH 00/15] NVDIMM: introduce nvdimm label support

[Xiao Guangrong]

Changelog in v2:
Thanks for Stefan's review, the changes in this version are:
- rename nvdimm device parameter 'reserve-label' to 'label-size' to
  specify the size of label
- add comment to explain why assert() used in nvdimm_assert_rw_label_data()
  is safe
- follow the code style of 'foo() return;' if nothing is returned by fool()
- fix the value of "Non-existing-Memory-Device"
- fix the handling the DSM functions we currently did not support

 
This patchset is against commit 2912f22759 (fixup! virtio: convert to use DMA
api) on pci branch of Michael's git tree and can be found at:
      https://github.com/xiaogr/qemu.git nvdimm-label-v2

This is the last part of vNVDIMM implementation which introduces nvdimm
label support

Currently Linux NVDIMM driver does not support namespace operation on this
kind of PMEM, apply below changes to support dynamical namespace:

@@ -798,7 +823,8 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *a
                        continue;
                }
 
-               if (nfit_mem->bdw && nfit_mem->memdev_pmem)
+               //if (nfit_mem->bdw && nfit_mem->memdev_pmem)
+               if (nfit_mem->memdev_pmem)
                        flags |= NDD_ALIASING;

You can append a NVDIMM device in guest and do:                       
# cd /sys/bus/nd/devices/
# cd namespace0.0/
# echo `uuidgen` > uuid
# echo `expr 1024 \* 1024 \* 128` > size
then reload nd.pmem.ko

You can see /dev/pmem0 appears

Xiao Guangrong (15):
  pc-dimm: get memory region from ->get_memory_region()
  pc-dimm: introduce realize callback
  pc-dimm: keep the state of the whole backend memory
  nvdimm: support nvdimm label
  acpi: add aml_object_type
  acpi: add aml_call5
  nvdimm acpi: set HDLE properly
  nvdimm acpi: save arg3 of _DSM method
  nvdimm acpi: check UUID
  nvdimm acpi: abstract the operations for root & nvdimm devices
  nvdimm acpi: check revision
  nvdimm acpi: support Get Namespace Label Size function
  nvdimm acpi: support Get Namespace Label Data function
  nvdimm acpi: support Set Namespace Label Data function
  docs: add NVDIMM ACPI documentation

 docs/specs/acpi_nvdimm.txt  | 132 +++++++++++++++
 hw/acpi/aml-build.c         |  22 +++
 hw/acpi/nvdimm.c            | 400 ++++++++++++++++++++++++++++++++++++++++----
 hw/mem/nvdimm.c             | 122 ++++++++++++++
 hw/mem/pc-dimm.c            |  21 ++-
 include/hw/acpi/aml-build.h |   3 +
 include/hw/mem/nvdimm.h     |  55 +++++-
 include/hw/mem/pc-dimm.h    |   6 +-
 8 files changed, 723 insertions(+), 38 deletions(-)
 create mode 100644 docs/specs/acpi_nvdimm.txt

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 01/15] pc-dimm: get memory region from ->get_memory_region()

[Xiao Guangrong]

Curretly, the memory region of backed memory is all directly
mapped to guest's address space, however, it will be not true
for nvdimm device if we introduce nvdimm label which only can
be indirectly accessed by ACPI DSM method

Also it improves the comments a bit to reflect this fact

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/mem/pc-dimm.c         | 3 ++-
 include/hw/mem/pc-dimm.h | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 9e7de56..70b9451 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -354,8 +354,9 @@ static void pc_dimm_get_size(Object *obj, Visitor *v, const char *name,
     int64_t value;
     MemoryRegion *mr;
     PCDIMMDevice *dimm = PC_DIMM(obj);
+    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj);
 
-    mr = host_memory_backend_get_memory(dimm->hostmem, errp);
+    mr = ddc->get_memory_region(dimm);
     value = memory_region_size(mr);
 
     visit_type_int(v, name, &value, errp);
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 218dfb0..827f1bc 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -60,7 +60,8 @@ typedef struct PCDIMMDevice {
 
 /**
  * PCDIMMDeviceClass:
- * @get_memory_region: returns #MemoryRegion associated with @dimm
+ * @get_memory_region: returns #MemoryRegion associated with @dimm which
+ * is directly mapped into the physical address space of guest
  */
 typedef struct PCDIMMDeviceClass {
     /* private */
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 01/15] pc-dimm: get memory region from ->get_memory_region()

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Fri, May 20, 2016 at 04:19:58PM +0800, Xiao Guangrong wrote:
> Curretly, the memory region of backed memory is all directly

s/Curretly/Currently/

> mapped to guest's address space, however, it will be not true
> for nvdimm device if we introduce nvdimm label which only can
> be indirectly accessed by ACPI DSM method
> 
> Also it improves the comments a bit to reflect this fact
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/mem/pc-dimm.c         | 3 ++-
>  include/hw/mem/pc-dimm.h | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 02/15] pc-dimm: introduce realize callback

[Xiao Guangrong]

nvdimm needs to  check if the backend memory is large enough to contain
label data and init its memory region when the device is realized, so
introduce realize callback which is called after common dimm has been
realize

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/mem/pc-dimm.c         | 5 +++++
 include/hw/mem/pc-dimm.h | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 70b9451..6de2275 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -400,6 +400,7 @@ static void pc_dimm_init(Object *obj)
 static void pc_dimm_realize(DeviceState *dev, Error **errp)
 {
     PCDIMMDevice *dimm = PC_DIMM(dev);
+    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
 
     if (!dimm->hostmem) {
         error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
@@ -412,6 +413,10 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
                    dimm->node, nb_numa_nodes ? nb_numa_nodes : 1);
         return;
     }
+
+    if (ddc->realize) {
+        ddc->realize(dimm, errp);
+    }
 }
 
 static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 827f1bc..e7b7e5a 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -60,6 +60,8 @@ typedef struct PCDIMMDevice {
 
 /**
  * PCDIMMDeviceClass:
+ * @realize: called after common dimm is realized so that the dimm based
+ * devices get the chance to do specified operations.
  * @get_memory_region: returns #MemoryRegion associated with @dimm which
  * is directly mapped into the physical address space of guest
  */
@@ -68,6 +70,7 @@ typedef struct PCDIMMDeviceClass {
     DeviceClass parent_class;
 
     /* public */
+    void (*realize)(PCDIMMDevice *dimm, Error **errp);
     MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm);
 } PCDIMMDeviceClass;
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 02/15] pc-dimm: introduce realize callback

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

On Fri, May 20, 2016 at 04:19:59PM +0800, Xiao Guangrong wrote:
> nvdimm needs to  check if the backend memory is large enough to contain
> label data and init its memory region when the device is realized, so
> introduce realize callback which is called after common dimm has been
> realize
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/mem/pc-dimm.c         | 5 +++++
>  include/hw/mem/pc-dimm.h | 3 +++
>  2 files changed, 8 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 03/15] pc-dimm: keep the state of the whole backend memory

[Xiao Guangrong]

QEMU keeps the state of memory of dimm device during live migration,
however, it is not enough for nvdimm device as its memory does not
contain its label data, so that we should protect the whole backend
memory instead

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/mem/pc-dimm.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 6de2275..72b33ba 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -105,9 +105,16 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
     }
 
     memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr);
-    vmstate_register_ram(mr, dev);
     numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node);
 
+    /*
+     * save the state only for @mr is not enough as it does not contain
+     * the label data of NVDIMM device, so that we keep the state of
+     * whole hostmem instead.
+     */
+    vmstate_register_ram(host_memory_backend_get_memory(dimm->hostmem, errp),
+                         dev);
+
 out:
     error_propagate(errp, local_err);
 }
@@ -116,10 +123,12 @@ void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
                            MemoryRegion *mr)
 {
     PCDIMMDevice *dimm = PC_DIMM(dev);
+    MemoryRegion *hostmem;
 
+    hostmem = host_memory_backend_get_memory(dimm->hostmem, &error_abort);
     numa_unset_mem_node_id(dimm->addr, memory_region_size(mr), dimm->node);
     memory_region_del_subregion(&hpms->mr, mr);
-    vmstate_unregister_ram(mr, dev);
+    vmstate_unregister_ram(hostmem, dev);
 }
 
 static int pc_existing_dimms_capacity_internal(Object *obj, void *opaque)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 03/15] pc-dimm: keep the state of the whole backend memory

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1608 bytes --]

On Fri, May 20, 2016 at 04:20:00PM +0800, Xiao Guangrong wrote:
> QEMU keeps the state of memory of dimm device during live migration,
> however, it is not enough for nvdimm device as its memory does not
> contain its label data, so that we should protect the whole backend
> memory instead
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/mem/pc-dimm.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 6de2275..72b33ba 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -105,9 +105,16 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
>      }
>  
>      memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr);
> -    vmstate_register_ram(mr, dev);
>      numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node);
>  
> +    /*
> +     * save the state only for @mr is not enough as it does not contain
> +     * the label data of NVDIMM device, so that we keep the state of
> +     * whole hostmem instead.
> +     */
> +    vmstate_register_ram(host_memory_backend_get_memory(dimm->hostmem, errp),
> +                         dev);
> +
>  out:
>      error_propagate(errp, local_err);
>  }

In Patch 1 you introduced a callback to get the guest-visible memory
region.  Instead of mentioning NVDIMM in generic pc-dimm.c code, it
would be cleaner to add another callback to get the vmstate memory
region:

  .get_guest_memory_region() - Patch 1
  .get_vmstate_memory_region() - a new patch in this series

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 03/15] pc-dimm: keep the state of the whole backend memory

[Xiao Guangrong]

On 05/31/2016 02:42 AM, Stefan Hajnoczi wrote:
> On Fri, May 20, 2016 at 04:20:00PM +0800, Xiao Guangrong wrote:
>> QEMU keeps the state of memory of dimm device during live migration,
>> however, it is not enough for nvdimm device as its memory does not
>> contain its label data, so that we should protect the whole backend
>> memory instead
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> ---
>>   hw/mem/pc-dimm.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>> index 6de2275..72b33ba 100644
>> --- a/hw/mem/pc-dimm.c
>> +++ b/hw/mem/pc-dimm.c
>> @@ -105,9 +105,16 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
>>       }
>>
>>       memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr);
>> -    vmstate_register_ram(mr, dev);
>>       numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node);
>>
>> +    /*
>> +     * save the state only for @mr is not enough as it does not contain
>> +     * the label data of NVDIMM device, so that we keep the state of
>> +     * whole hostmem instead.
>> +     */
>> +    vmstate_register_ram(host_memory_backend_get_memory(dimm->hostmem, errp),
>> +                         dev);
>> +
>>   out:
>>       error_propagate(errp, local_err);
>>   }
>
> In Patch 1 you introduced a callback to get the guest-visible memory
> region.  Instead of mentioning NVDIMM in generic pc-dimm.c code, it
> would be cleaner to add another callback to get the vmstate memory
> region:
>
>    .get_guest_memory_region() - Patch 1
>    .get_vmstate_memory_region() - a new patch in this series
>

It is good to me, will do it. Thanks!

[Qemu-devel] [PATCH v2 04/15] nvdimm: support nvdimm label

[Xiao Guangrong]

Introduce a parameter, 'label-size', which is the size of nvdimm label
data area which is reserved at the end of backend memory. It is required
at least 128k

Two callbacks, read_label_data() and write_label_data(), are used to
operate the label area

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/mem/nvdimm.c         | 122 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/mem/nvdimm.h |  55 +++++++++++++++++++++-
 2 files changed, 176 insertions(+), 1 deletion(-)

diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 0a602f2..e45bd76 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -23,20 +23,142 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
 #include "hw/mem/nvdimm.h"
 
+static void nvdimm_get_label_size(Object *obj, Visitor *v, const char *name,
+                                  void *opaque, Error **errp)
+{
+    NVDIMMDevice *nvdimm = NVDIMM(obj);
+    uint64_t value = nvdimm->label_size;
+
+    visit_type_size(v, name, &value, errp);
+}
+
+static void nvdimm_set_label_size(Object *obj, Visitor *v, const char *name,
+                                  void *opaque, Error **errp)
+{
+    NVDIMMDevice *nvdimm = NVDIMM(obj);
+    Error *local_err = NULL;
+    uint64_t value;
+
+    if (memory_region_size(&nvdimm->nvdimm_mr)) {
+        error_setg(&local_err, "cannot change property value");
+        goto out;
+    }
+
+    visit_type_size(v, name, &value, &local_err);
+    if (local_err) {
+        goto out;
+    }
+    if (value < MIN_NAMESPACE_LABEL_SIZE) {
+        error_setg(&local_err, "Property '%s.%s' (0x%" PRIx64 ") is required"
+                   " at least 0x%" PRIx64, object_get_typename(obj),
+                   name, value, MIN_NAMESPACE_LABEL_SIZE);
+        goto out;
+    }
+
+    nvdimm->label_size = value;
+out:
+    error_propagate(errp, local_err);
+}
+
+static void nvdimm_init(Object *obj)
+{
+    object_property_add(obj, "label-size", "int",
+                        nvdimm_get_label_size, nvdimm_set_label_size, NULL,
+                        NULL, NULL);
+}
+
+static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm)
+{
+    NVDIMMDevice *nvdimm = NVDIMM(dimm);
+
+    return &nvdimm->nvdimm_mr;
+}
+
+static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp)
+{
+    MemoryRegion *mr = host_memory_backend_get_memory(dimm->hostmem, errp);
+    NVDIMMDevice *nvdimm = NVDIMM(dimm);
+    uint64_t size = memory_region_size(mr);
+
+    if (size <= nvdimm->label_size) {
+        HostMemoryBackend *hostmem = dimm->hostmem;
+        char *path = object_get_canonical_path_component(OBJECT(hostmem));
+
+        error_setg(errp, "the size of memdev %s (0x%" PRIx64 ") is too"
+                   "small to contain nvdimm label (0x%" PRIx64 ")",
+                   path, memory_region_size(mr), nvdimm->label_size);
+        return;
+    }
+
+    size -= nvdimm->label_size;
+    memory_region_init_alias(&nvdimm->nvdimm_mr, OBJECT(dimm),
+                             "nvdimm-memory", mr, 0, size);
+    nvdimm->nvdimm_mr.align = memory_region_get_alignment(mr);
+
+    nvdimm->label_data = memory_region_get_ram_ptr(mr) + size;
+}
+
+/*
+ * the caller should check the input parameters before calling
+ * label read/write functions.
+ */
+static void nvdimm_validate_rw_label_data(NVDIMMDevice *nvdimm, uint64_t size,
+                                        uint64_t offset)
+{
+    assert((nvdimm->label_size >= size + offset) && (offset + size > offset));
+}
+
+static void nvdimm_read_label_data(NVDIMMDevice *nvdimm, void *buf,
+                                   uint64_t size, uint64_t offset)
+{
+    nvdimm_validate_rw_label_data(nvdimm, size, offset);
+
+    memcpy(buf, nvdimm->label_data + offset, size);
+}
+
+static void nvdimm_write_label_data(NVDIMMDevice *nvdimm, const void *buf,
+                                    uint64_t size, uint64_t offset)
+{
+    MemoryRegion *mr;
+    PCDIMMDevice *dimm = PC_DIMM(nvdimm);
+    uint64_t backend_offset;
+
+    nvdimm_validate_rw_label_data(nvdimm, size, offset);
+
+    memcpy(nvdimm->label_data + offset, buf, size);
+
+    mr = host_memory_backend_get_memory(dimm->hostmem, &error_abort);
+    backend_offset = memory_region_size(mr) - nvdimm->label_size + offset;
+    memory_region_set_dirty(mr, backend_offset, size);
+}
+
 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;
+
+    nvc->read_label_data = nvdimm_read_label_data;
+    nvc->write_label_data = nvdimm_write_label_data;
 }
 
 static TypeInfo nvdimm_info = {
     .name          = TYPE_NVDIMM,
     .parent        = TYPE_PC_DIMM,
+    .class_size    = sizeof(NVDIMMClass),
     .class_init    = nvdimm_class_init,
+    .instance_size = sizeof(NVDIMMDevice),
+    .instance_init = nvdimm_init,
 };
 
 static void nvdimm_register_types(void)
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 517de9c..cc3301b 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -33,7 +33,60 @@
         }                                                     \
     } while (0)
 
-#define TYPE_NVDIMM             "nvdimm"
+/*
+ * The minimum label data size is required by NVDIMM Namespace
+ * specification, see the chapter 2 Namespaces:
+ *   "NVDIMMs following the NVDIMM Block Mode Specification use an area
+ *    at least 128KB in size, which holds around 1000 labels."
+ */
+#define MIN_NAMESPACE_LABEL_SIZE      (128UL << 10)
+
+#define TYPE_NVDIMM      "nvdimm"
+#define NVDIMM(obj)      OBJECT_CHECK(NVDIMMDevice, (obj), TYPE_NVDIMM)
+#define NVDIMM_CLASS(oc) OBJECT_CLASS_CHECK(NVDIMMClass, (oc), TYPE_NVDIMM)
+#define NVDIMM_GET_CLASS(obj) OBJECT_GET_CLASS(NVDIMMClass, (obj), \
+                                               TYPE_NVDIMM)
+struct NVDIMMDevice {
+    /* private */
+    PCDIMMDevice parent_obj;
+
+    /* public */
+
+    /*
+     * the size of label data in NVDIMM device which is presented to
+     * guest via __DSM "Get Namespace Label Size" function.
+     */
+    uint64_t label_size;
+
+    /*
+     * the address of label data which is read by __DSM "Get Namespace
+     * Label Data" function and written by __DSM "Set Namespace Label
+     * Data" function.
+     */
+    void *label_data;
+
+    /*
+     * it's the PMEM region in NVDIMM device, which is presented to
+     * guest via ACPI NFIT and _FIT method if NVDIMM hotplug is supported.
+     */
+    MemoryRegion nvdimm_mr;
+};
+typedef struct NVDIMMDevice NVDIMMDevice;
+
+struct NVDIMMClass {
+    /* private */
+    PCDIMMDeviceClass parent_class;
+
+    /* public */
+
+    /* read @size bytes from NVDIMM label data at @offset into @buf. */
+    void (*read_label_data)(NVDIMMDevice *nvdimm, void *buf,
+                            uint64_t size, uint64_t offset);
+    /* write @size bytes from @buf to NVDIMM label data at @offset. */
+    void (*write_label_data)(NVDIMMDevice *nvdimm, const void *buf,
+                             uint64_t size, uint64_t offset);
+};
+typedef struct NVDIMMClass NVDIMMClass;
 
 #define NVDIMM_DSM_MEM_FILE     "etc/acpi/nvdimm-mem"
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 04/15] nvdimm: support nvdimm label

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Fri, May 20, 2016 at 04:20:01PM +0800, Xiao Guangrong wrote:
> Introduce a parameter, 'label-size', which is the size of nvdimm label
> data area which is reserved at the end of backend memory. It is required
> at least 128k
> 
> Two callbacks, read_label_data() and write_label_data(), are used to
> operate the label area
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/mem/nvdimm.c         | 122 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/mem/nvdimm.h |  55 +++++++++++++++++++++-
>  2 files changed, 176 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 04/15] nvdimm: support nvdimm label

[Paolo Bonzini]

On 20/05/2016 10:20, Xiao Guangrong wrote:
> +    if (size <= nvdimm->label_size) {
> +        HostMemoryBackend *hostmem = dimm->hostmem;
> +        char *path = object_get_canonical_path_component(OBJECT(hostmem));
> +
> +        error_setg(errp, "the size of memdev %s (0x%" PRIx64 ") is too"
> +                   "small to contain nvdimm label (0x%" PRIx64 ")",
> +                   path, memory_region_size(mr), nvdimm->label_size);

Coverity reports here that path is leaked.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH v2 04/15] nvdimm: support nvdimm label

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

On Tue, Jun 28, 2016 at 09:48:10AM +0200, Paolo Bonzini wrote:
> 
> 
> On 20/05/2016 10:20, Xiao Guangrong wrote:
> > +    if (size <= nvdimm->label_size) {
> > +        HostMemoryBackend *hostmem = dimm->hostmem;
> > +        char *path = object_get_canonical_path_component(OBJECT(hostmem));
> > +
> > +        error_setg(errp, "the size of memdev %s (0x%" PRIx64 ") is too"
> > +                   "small to contain nvdimm label (0x%" PRIx64 ")",
> > +                   path, memory_region_size(mr), nvdimm->label_size);
> 
> Coverity reports here that path is leaked.

I will send a patch.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 05/15] acpi: add aml_object_type

[Xiao Guangrong]

Implement ObjectType which is used by NVDIMM _DSM method in
later patch

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/aml-build.c         | 8 ++++++++
 include/hw/acpi/aml-build.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index ab89ca6..85c3ef7 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1472,6 +1472,14 @@ Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
                                  target);
 }
 
+/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefObjectType */
+Aml *aml_object_type(Aml *object)
+{
+    Aml *var = aml_opcode(0x8E /* ObjectTypeOp */);
+    aml_append(var, object);
+    return var;
+}
+
 void
 build_header(GArray *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 2c994b3..886be49 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -354,6 +354,7 @@ Aml *aml_unicode(const char *str);
 Aml *aml_derefof(Aml *arg);
 Aml *aml_sizeof(Aml *arg);
 Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
+Aml *aml_object_type(Aml *object);
 
 void
 build_header(GArray *linker, GArray *table_data,
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 06/15] acpi: add aml_call5

[Xiao Guangrong]

It will be used by NVDIMM ACPI

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/aml-build.c         | 14 ++++++++++++++
 include/hw/acpi/aml-build.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 85c3ef7..1783216 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -657,6 +657,20 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4)
     return var;
 }
 
+/* helper to call method with 5 arguments */
+Aml *aml_call5(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
+               Aml *arg5)
+{
+    Aml *var = aml_alloc();
+    build_append_namestring(var->buf, "%s", method);
+    aml_append(var, arg1);
+    aml_append(var, arg2);
+    aml_append(var, arg3);
+    aml_append(var, arg4);
+    aml_append(var, arg5);
+    return var;
+}
+
 /*
  * ACPI 5.0: 6.4.3.8.1 GPIO Connection Descriptor
  * Type 1, Large Item Name 0xC
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 886be49..73d16a3 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -269,6 +269,8 @@ Aml *aml_call1(const char *method, Aml *arg1);
 Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
 Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
 Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4);
+Aml *aml_call5(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
+               Aml *arg5);
 Aml *aml_gpio_int(AmlConsumerAndProducer con_and_pro,
                   AmlLevelAndEdge edge_level,
                   AmlActiveHighAndLow active_level, AmlShared shared,
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 07/15] nvdimm acpi: set HDLE properly

[Xiao Guangrong]

Now we pass HDLE to Qemu properly, use 0 for root device and use the
handle for nvdimm devices

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 9531340..586c02a 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -488,7 +488,7 @@ static void nvdimm_build_common_dsm(Aml *dev)
     Aml *method, *ifctx, *function, *dsm_mem, *unpatched, *result_size;
     uint8_t byte_list[1];
 
-    method = aml_method(NVDIMM_COMMON_DSM, 4, AML_SERIALIZED);
+    method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
     function = aml_arg(2);
     dsm_mem = aml_name(NVDIMM_ACPI_MEM_ADDR);
 
@@ -514,11 +514,10 @@ static void nvdimm_build_common_dsm(Aml *dev)
 
     /*
      * The HDLE indicates the DSM function is issued from which device,
-     * it is not used at this time as no function is supported yet.
-     * Currently we make it always be 0 for all the devices and will set
-     * the appropriate value once real function is implemented.
+     * it reserves 0 for root device and is the handle for NVDIMM devices.
+     * See the comments in nvdimm_slot_to_handle().
      */
-    aml_append(method, aml_store(aml_int(0x0), aml_name("HDLE")));
+    aml_append(method, aml_store(aml_arg(4), aml_name("HDLE")));
     aml_append(method, aml_store(aml_arg(1), aml_name("REVS")));
     aml_append(method, aml_store(aml_arg(2), aml_name("FUNC")));
 
@@ -540,13 +539,14 @@ static void nvdimm_build_common_dsm(Aml *dev)
     aml_append(dev, method);
 }
 
-static void nvdimm_build_device_dsm(Aml *dev)
+static void nvdimm_build_device_dsm(Aml *dev, uint32_t handle)
 {
     Aml *method;
 
     method = aml_method("_DSM", 4, AML_NOTSERIALIZED);
-    aml_append(method, aml_return(aml_call4(NVDIMM_COMMON_DSM, aml_arg(0),
-                                  aml_arg(1), aml_arg(2), aml_arg(3))));
+    aml_append(method, aml_return(aml_call5(NVDIMM_COMMON_DSM, aml_arg(0),
+                                  aml_arg(1), aml_arg(2), aml_arg(3),
+                                  aml_int(handle))));
     aml_append(dev, method);
 }
 
@@ -571,7 +571,7 @@ static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev)
          */
         aml_append(nvdimm_dev, aml_name_decl("_ADR", aml_int(handle)));
 
-        nvdimm_build_device_dsm(nvdimm_dev);
+        nvdimm_build_device_dsm(nvdimm_dev, handle);
         aml_append(root_dev, nvdimm_dev);
     }
 }
@@ -664,7 +664,9 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
     aml_append(dev, field);
 
     nvdimm_build_common_dsm(dev);
-    nvdimm_build_device_dsm(dev);
+
+    /* 0 is reserved for root device. */
+    nvdimm_build_device_dsm(dev, 0);
 
     nvdimm_build_nvdimm_devices(device_list, dev);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 08/15] nvdimm acpi: save arg3 of _DSM method

[Xiao Guangrong]

Check if the input Arg3 is valid then store it into ARG3 if it is
needed

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 586c02a..38aba3d 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -486,6 +486,7 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
 static void nvdimm_build_common_dsm(Aml *dev)
 {
     Aml *method, *ifctx, *function, *dsm_mem, *unpatched, *result_size;
+    Aml *pckg, *pckg_index, *pckg_buf;
     uint8_t byte_list[1];
 
     method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
@@ -522,6 +523,25 @@ static void nvdimm_build_common_dsm(Aml *dev)
     aml_append(method, aml_store(aml_arg(2), aml_name("FUNC")));
 
     /*
+     * The fourth parameter (Arg3) of _DSM is a package which contains
+     * a buffer, the layout of the buffer is specified by UUID (Arg0),
+     * Revision ID (Arg1) and Function Index (Arg2) which are documented
+     * in the DSM Spec.
+     */
+    pckg = aml_arg(3);
+    ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
+                   aml_int(4 /* Package */)) /* It is a Package? */,
+                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */,
+                   NULL));
+
+    pckg_index = aml_local(2);
+    pckg_buf = aml_local(3);
+    aml_append(ifctx, aml_store(aml_index(pckg, aml_int(0)), pckg_index));
+    aml_append(ifctx, aml_store(aml_derefof(pckg_index), pckg_buf));
+    aml_append(ifctx, aml_store(pckg_buf, aml_name("ARG3")));
+    aml_append(method, ifctx);
+
+    /*
      * tell QEMU about the real address of DSM memory, then QEMU
      * gets the control and fills the result in DSM memory.
      */
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 09/15] nvdimm acpi: check UUID

[Xiao Guangrong]

Check arg0 which indicates UUID to see if it is valid

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 38aba3d..4177227 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -485,19 +485,39 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
 
 static void nvdimm_build_common_dsm(Aml *dev)
 {
-    Aml *method, *ifctx, *function, *dsm_mem, *unpatched, *result_size;
+    Aml *method, *ifctx, *function, *handle, *uuid, *dsm_mem, *result_size;
+    Aml *elsectx, *unsupport, *unpatched, *expected_uuid, *uuid_invalid;
     Aml *pckg, *pckg_index, *pckg_buf;
     uint8_t byte_list[1];
 
     method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
+    uuid = aml_arg(0);
     function = aml_arg(2);
+    handle = aml_arg(4);
     dsm_mem = aml_name(NVDIMM_ACPI_MEM_ADDR);
 
     /*
      * do not support any method if DSM memory address has not been
      * patched.
      */
-    unpatched = aml_if(aml_equal(dsm_mem, aml_int(0x0)));
+    unpatched = aml_equal(dsm_mem, aml_int(0x0));
+
+    expected_uuid = aml_local(0);
+
+    ifctx = aml_if(aml_equal(handle, aml_int(0x0)));
+    aml_append(ifctx, aml_store(
+               aml_touuid("2F10E7A4-9E91-11E4-89D3-123B93F75CBA")
+               /* UUID for NVDIMM Root Device */, expected_uuid));
+    aml_append(method, ifctx);
+    elsectx = aml_else();
+    aml_append(elsectx, aml_store(
+               aml_touuid("4309AC30-0D11-11E4-9191-0800200C9A66")
+               /* UUID for NVDIMM Devices */, expected_uuid));
+    aml_append(method, elsectx);
+
+    uuid_invalid = aml_lnot(aml_equal(uuid, expected_uuid));
+
+    unsupport = aml_if(aml_or(unpatched, uuid_invalid, NULL));
 
     /*
      * function 0 is called to inquire what functions are supported by
@@ -506,19 +526,19 @@ static void nvdimm_build_common_dsm(Aml *dev)
     ifctx = aml_if(aml_equal(function, aml_int(0)));
     byte_list[0] = 0 /* No function Supported */;
     aml_append(ifctx, aml_return(aml_buffer(1, byte_list)));
-    aml_append(unpatched, ifctx);
+    aml_append(unsupport, ifctx);
 
     /* No function is supported yet. */
     byte_list[0] = 1 /* Not Supported */;
-    aml_append(unpatched, aml_return(aml_buffer(1, byte_list)));
-    aml_append(method, unpatched);
+    aml_append(unsupport, aml_return(aml_buffer(1, byte_list)));
+    aml_append(method, unsupport);
 
     /*
      * The HDLE indicates the DSM function is issued from which device,
      * it reserves 0 for root device and is the handle for NVDIMM devices.
      * See the comments in nvdimm_slot_to_handle().
      */
-    aml_append(method, aml_store(aml_arg(4), aml_name("HDLE")));
+    aml_append(method, aml_store(handle, aml_name("HDLE")));
     aml_append(method, aml_store(aml_arg(1), aml_name("REVS")));
     aml_append(method, aml_store(aml_arg(2), aml_name("FUNC")));
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 10/15] nvdimm acpi: abstract the operations for root & nvdimm devices

[Xiao Guangrong]

It separates the operations between root device and nvdimm devices
in order to introducing label functions support for nvdimm device

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 74 ++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 56 insertions(+), 18 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 4177227..897d0a6 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -404,6 +404,55 @@ struct NvdimmDsmFuncNoPayloadOut {
 } QEMU_PACKED;
 typedef struct NvdimmDsmFuncNoPayloadOut NvdimmDsmFuncNoPayloadOut;
 
+static void
+nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
+{
+    NvdimmDsmFunc0Out func0 = {
+        .len = cpu_to_le32(sizeof(func0)),
+        .supported_func = cpu_to_le32(supported_func),
+    };
+    cpu_physical_memory_write(dsm_mem_addr, &func0, sizeof(func0));
+}
+
+static void
+nvdimm_dsm_no_payload(uint32_t func_ret_status, hwaddr dsm_mem_addr)
+{
+    NvdimmDsmFuncNoPayloadOut out = {
+        .len = cpu_to_le32(sizeof(out)),
+        .func_ret_status = cpu_to_le32(func_ret_status),
+    };
+    cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
+}
+
+static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
+{
+    /*
+     * function 0 is called to inquire which functions are supported by
+     * OSPM
+     */
+    if (!in->function) {
+        nvdimm_dsm_function0(0 /* No function supported other than
+                                  function 0 */, dsm_mem_addr);
+        return;
+    }
+
+    /* No function except function 0 is supported yet. */
+    nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
+}
+
+static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
+{
+    /* See the comments in nvdimm_dsm_root(). */
+    if (!in->function) {
+        nvdimm_dsm_function0(0 /* No function supported other than
+                                  function 0 */, dsm_mem_addr);
+        return;
+    }
+
+    /* No function except function 0 is supported yet. */
+    nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
+}
+
 static uint64_t
 nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
 {
@@ -434,26 +483,15 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
                  in->handle, in->function);
 
-    /*
-     * function 0 is called to inquire which functions are supported by
-     * OSPM
-     */
-    if (in->function == 0) {
-        NvdimmDsmFunc0Out func0 = {
-            .len = cpu_to_le32(sizeof(func0)),
-             /* No function supported other than function 0 */
-            .supported_func = cpu_to_le32(0),
-        };
-        cpu_physical_memory_write(dsm_mem_addr, &func0, sizeof func0);
-    } else {
-        /* No function except function 0 is supported yet. */
-        NvdimmDsmFuncNoPayloadOut out = {
-            .len = cpu_to_le32(sizeof(out)),
-            .func_ret_status = cpu_to_le32(1)  /* Not Supported */,
-        };
-        cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
+     /* Handle 0 is reserved for NVDIMM Root Device. */
+    if (!in->handle) {
+        nvdimm_dsm_root(in, dsm_mem_addr);
+        goto exit;
     }
 
+    nvdimm_dsm_device(in, dsm_mem_addr);
+
+exit:
     g_free(in);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 11/15] nvdimm acpi: check revision

[Xiao Guangrong]

Cuurently only revision 1 is supported

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 897d0a6..7af428f 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -483,6 +483,13 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
                  in->handle, in->function);
 
+    if (in->revision != 0x1 /* Currently we only support DSM Spec Rev1. */) {
+        nvdimm_debug("Revision %#x is not supported, expect %#x.\n",
+                     in->revision, 0x1);
+        nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
+        goto exit;
+    }
+
      /* Handle 0 is reserved for NVDIMM Root Device. */
     if (!in->handle) {
         nvdimm_dsm_root(in, dsm_mem_addr);
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 12/15] nvdimm acpi: support Get Namespace Label Size function

[Xiao Guangrong]

Function 4 is used to get Namespace label size

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 127 insertions(+), 3 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 7af428f..94424a4 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -216,6 +216,26 @@ static uint32_t nvdimm_slot_to_dcr_index(int slot)
     return nvdimm_slot_to_spa_index(slot) + 1;
 }
 
+static NVDIMMDevice *nvdimm_get_device_by_handle(uint32_t handle)
+{
+    NVDIMMDevice *nvdimm = NULL;
+    GSList *list, *device_list = nvdimm_get_plugged_device_list();
+
+    for (list = device_list; list; list = list->next) {
+        NVDIMMDevice *nvd = list->data;
+        int slot = object_property_get_int(OBJECT(nvd), PC_DIMM_SLOT_PROP,
+                                           NULL);
+
+        if (nvdimm_slot_to_handle(slot) == handle) {
+            nvdimm = nvd;
+            break;
+        }
+    }
+
+    g_slist_free(device_list);
+    return nvdimm;
+}
+
 /* ACPI 6.0: 5.2.25.1 System Physical Address Range Structure */
 static void
 nvdimm_build_structure_spa(GArray *structures, DeviceState *dev)
@@ -404,6 +424,35 @@ struct NvdimmDsmFuncNoPayloadOut {
 } QEMU_PACKED;
 typedef struct NvdimmDsmFuncNoPayloadOut NvdimmDsmFuncNoPayloadOut;
 
+struct NvdimmFuncGetLabelSizeOut {
+    /* the size of buffer filled by QEMU. */
+    uint32_t len;
+    uint32_t func_ret_status; /* return status code. */
+    uint32_t label_size; /* the size of label data area. */
+    /*
+     * Maximum size of the namespace label data length supported by
+     * the platform in Get/Set Namespace Label Data functions.
+     */
+    uint32_t max_xfer;
+} QEMU_PACKED;
+typedef struct NvdimmFuncGetLabelSizeOut NvdimmFuncGetLabelSizeOut;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelSizeOut) > TARGET_PAGE_SIZE);
+
+struct NvdimmFuncGetLabelDataOut {
+    /* the size of buffer filled by QEMU. */
+    uint32_t len;
+    uint32_t func_ret_status; /* return status code. */
+    uint8_t out_buf[0]; /* the data got via Get Namesapce Label function. */
+} QEMU_PACKED;
+typedef struct NvdimmFuncGetLabelDataOut NvdimmFuncGetLabelDataOut;
+
+struct NvdimmFuncSetLabelDataIn {
+    uint32_t offset; /* the offset in the namespace label data area. */
+    uint32_t length; /* the size of data is to be written via the function. */
+    uint8_t in_buf[0]; /* the data written to label data area. */
+} QEMU_PACKED;
+typedef struct NvdimmFuncSetLabelDataIn NvdimmFuncSetLabelDataIn;
+
 static void
 nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
 {
@@ -440,16 +489,91 @@ static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
     nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
 }
 
+/*
+ * the max transfer size is the max size transferred by both a
+ * 'Get Namespace Label Data' function and a 'Set Namespace Label Data'
+ * function.
+ */
+static uint32_t nvdimm_get_max_xfer_label_size(void)
+{
+    uint32_t max_get_size, max_set_size, dsm_memory_size = TARGET_PAGE_SIZE;
+
+    /*
+     * the max data ACPI can read one time which is transferred by
+     * the response of 'Get Namespace Label Data' function.
+     */
+    max_get_size = dsm_memory_size - sizeof(NvdimmFuncGetLabelDataOut);
+
+    /*
+     * the max data ACPI can write one time which is transferred by
+     * 'Set Namespace Label Data' function.
+     */
+    max_set_size = dsm_memory_size - sizeof(NvdimmDsmIn) -
+                   sizeof(NvdimmFuncSetLabelDataIn);
+
+    return MIN(max_get_size, max_set_size);
+}
+
+/*
+ * DSM Spec Rev1 4.4 Get Namespace Label Size (Function Index 4).
+ *
+ * It gets the size of Namespace Label data area and the max data size
+ * that Get/Set Namespace Label Data functions can transfer.
+ */
+static void nvdimm_dsm_label_size(NVDIMMDevice *nvdimm, hwaddr dsm_mem_addr)
+{
+    NvdimmFuncGetLabelSizeOut label_size_out = {
+        .len = cpu_to_le32(sizeof(label_size_out)),
+    };
+    uint32_t label_size, mxfer;
+
+    label_size = nvdimm->label_size;
+    mxfer = nvdimm_get_max_xfer_label_size();
+
+    nvdimm_debug("label_size %#x, max_xfer %#x.\n", label_size, mxfer);
+
+    label_size_out.func_ret_status = cpu_to_le32(0 /* Success */);
+    label_size_out.label_size = cpu_to_le32(label_size);
+    label_size_out.max_xfer = cpu_to_le32(mxfer);
+
+    cpu_physical_memory_write(dsm_mem_addr, &label_size_out,
+                              sizeof(label_size_out));
+}
+
 static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
 {
+    NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(in->handle);
+
     /* See the comments in nvdimm_dsm_root(). */
     if (!in->function) {
-        nvdimm_dsm_function0(0 /* No function supported other than
-                                  function 0 */, dsm_mem_addr);
+        uint32_t supported_func = 0;
+
+        if (nvdimm && nvdimm->label_size) {
+            supported_func |= 0x1 /* Bit 0 indicates whether there is
+                                     support for any functions other
+                                     than function 0. */ |
+                              1 << 4 /* Get Namespace Label Size */;
+        }
+        nvdimm_dsm_function0(supported_func, dsm_mem_addr);
         return;
     }
 
-    /* No function except function 0 is supported yet. */
+    if (!nvdimm) {
+        nvdimm_dsm_no_payload(2 /* Non-Existing Memory Device */,
+                              dsm_mem_addr);
+        return;
+    }
+
+    /* Encode DSM function according to DSM Spec Rev1. */
+    switch (in->function) {
+    case 4 /* Get Namespace Label Size */:
+        if (nvdimm->label_size) {
+            nvdimm_dsm_label_size(nvdimm, dsm_mem_addr);
+            return;
+        }
+        break;
+    }
+
     nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 12/15] nvdimm acpi: support Get Namespace Label Size function

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 383 bytes --]

On Fri, May 20, 2016 at 04:20:09PM +0800, Xiao Guangrong wrote:
> Function 4 is used to get Namespace label size
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/acpi/nvdimm.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 127 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 13/15] nvdimm acpi: support Get Namespace Label Data function

[Xiao Guangrong]

Function 5 is used to get Namespace Label Data

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 94424a4..0c0b4bf 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -438,6 +438,14 @@ struct NvdimmFuncGetLabelSizeOut {
 typedef struct NvdimmFuncGetLabelSizeOut NvdimmFuncGetLabelSizeOut;
 QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelSizeOut) > TARGET_PAGE_SIZE);
 
+struct NvdimmFuncGetLabelDataIn {
+    uint32_t offset; /* the offset in the namespace label data area. */
+    uint32_t length; /* the size of data is to be read via the function. */
+} QEMU_PACKED;
+typedef struct NvdimmFuncGetLabelDataIn NvdimmFuncGetLabelDataIn;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelDataIn) + sizeof(NvdimmDsmIn) >
+                  TARGET_PAGE_SIZE);
+
 struct NvdimmFuncGetLabelDataOut {
     /* the size of buffer filled by QEMU. */
     uint32_t len;
@@ -445,6 +453,7 @@ struct NvdimmFuncGetLabelDataOut {
     uint8_t out_buf[0]; /* the data got via Get Namesapce Label function. */
 } QEMU_PACKED;
 typedef struct NvdimmFuncGetLabelDataOut NvdimmFuncGetLabelDataOut;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelDataOut) > TARGET_PAGE_SIZE);
 
 struct NvdimmFuncSetLabelDataIn {
     uint32_t offset; /* the offset in the namespace label data area. */
@@ -540,6 +549,71 @@ static void nvdimm_dsm_label_size(NVDIMMDevice *nvdimm, hwaddr dsm_mem_addr)
                               sizeof(label_size_out));
 }
 
+static uint32_t nvdimm_rw_label_data_check(NVDIMMDevice *nvdimm,
+                                           uint32_t offset, uint32_t length)
+{
+    uint32_t ret = 3 /* Invalid Input Parameters */;
+
+    if (offset + length < offset) {
+        nvdimm_debug("offset %#x + length %#x is overflow.\n", offset,
+                     length);
+        return ret;
+    }
+
+    if (nvdimm->label_size < offset + length) {
+        nvdimm_debug("position %#x is beyond label data (len = %#lx).\n",
+                     offset + length, nvdimm->label_size);
+        return ret;
+    }
+
+    if (length > nvdimm_get_max_xfer_label_size()) {
+        nvdimm_debug("length (%#x) is larger than max_xfer (%#x).\n",
+                     length, nvdimm_get_max_xfer_label_size());
+        return ret;
+    }
+
+    return 0 /* Success */;
+}
+
+/*
+ * DSM Spec Rev1 4.5 Get Namespace Label Data (Function Index 5).
+ */
+static void nvdimm_dsm_get_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
+                                      hwaddr dsm_mem_addr)
+{
+    NVDIMMClass *nvc = NVDIMM_GET_CLASS(nvdimm);
+    NvdimmFuncGetLabelDataIn *get_label_data;
+    NvdimmFuncGetLabelDataOut *get_label_data_out;
+    uint32_t status;
+    int size;
+
+    get_label_data = (NvdimmFuncGetLabelDataIn *)in->arg3;
+    le32_to_cpus(&get_label_data->offset);
+    le32_to_cpus(&get_label_data->length);
+
+    nvdimm_debug("Read Label Data: offset %#x length %#x.\n",
+                 get_label_data->offset, get_label_data->length);
+
+    status = nvdimm_rw_label_data_check(nvdimm, get_label_data->offset,
+                                        get_label_data->length);
+    if (status != 0 /* Success */) {
+        nvdimm_dsm_no_payload(status, dsm_mem_addr);
+        return;
+    }
+
+    size = sizeof(*get_label_data_out) + get_label_data->length;
+    assert(size <= TARGET_PAGE_SIZE);
+    get_label_data_out = g_malloc(size);
+
+    get_label_data_out->len = cpu_to_le32(size);
+    get_label_data_out->func_ret_status = cpu_to_le32(0 /* Success */);
+    nvc->read_label_data(nvdimm, get_label_data_out->out_buf,
+                         get_label_data->length, get_label_data->offset);
+
+    cpu_physical_memory_write(dsm_mem_addr, get_label_data_out, size);
+    g_free(get_label_data_out);
+}
+
 static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
 {
     NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(in->handle);
@@ -552,7 +626,8 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
             supported_func |= 0x1 /* Bit 0 indicates whether there is
                                      support for any functions other
                                      than function 0. */ |
-                              1 << 4 /* Get Namespace Label Size */;
+                              1 << 4 /* Get Namespace Label Size */ |
+                              1 << 5 /* Get Namespace Label Data */;
         }
         nvdimm_dsm_function0(supported_func, dsm_mem_addr);
         return;
@@ -572,6 +647,12 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
             return;
         }
         break;
+    case 5 /* Get Namespace Label Data */:
+        if (nvdimm->label_size) {
+            nvdimm_dsm_get_label_data(nvdimm, in, dsm_mem_addr);
+            return;
+        }
+        break;
     }
 
     nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 13/15] nvdimm acpi: support Get Namespace Label Data function

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

On Fri, May 20, 2016 at 04:20:10PM +0800, Xiao Guangrong wrote:
> Function 5 is used to get Namespace Label Data
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/acpi/nvdimm.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 82 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 14/15] nvdimm acpi: support Set Namespace Label Data function

[Xiao Guangrong]

Function 6 is used to set Namespace Label Data

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 0c0b4bf..0e7a81f 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -461,6 +461,8 @@ struct NvdimmFuncSetLabelDataIn {
     uint8_t in_buf[0]; /* the data written to label data area. */
 } QEMU_PACKED;
 typedef struct NvdimmFuncSetLabelDataIn NvdimmFuncSetLabelDataIn;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncSetLabelDataIn) + sizeof(NvdimmDsmIn) >
+                  TARGET_PAGE_SIZE);
 
 static void
 nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
@@ -614,6 +616,39 @@ static void nvdimm_dsm_get_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
     g_free(get_label_data_out);
 }
 
+/*
+ * DSM Spec Rev1 4.6 Set Namespace Label Data (Function Index 6).
+ */
+static void nvdimm_dsm_set_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
+                                      hwaddr dsm_mem_addr)
+{
+    NVDIMMClass *nvc = NVDIMM_GET_CLASS(nvdimm);
+    NvdimmFuncSetLabelDataIn *set_label_data;
+    uint32_t status;
+
+    set_label_data = (NvdimmFuncSetLabelDataIn *)in->arg3;
+
+    le32_to_cpus(&set_label_data->offset);
+    le32_to_cpus(&set_label_data->length);
+
+    nvdimm_debug("Write Label Data: offset %#x length %#x.\n",
+                 set_label_data->offset, set_label_data->length);
+
+    status = nvdimm_rw_label_data_check(nvdimm, set_label_data->offset,
+                                        set_label_data->length);
+    if (status != 0 /* Success */) {
+        nvdimm_dsm_no_payload(status, dsm_mem_addr);
+        return;
+    }
+
+    assert(sizeof(*in) + sizeof(*set_label_data) + set_label_data->length <=
+           TARGET_PAGE_SIZE);
+
+    nvc->write_label_data(nvdimm, set_label_data->in_buf,
+                          set_label_data->length, set_label_data->offset);
+    nvdimm_dsm_no_payload(0 /* Success */, dsm_mem_addr);
+}
+
 static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
 {
     NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(in->handle);
@@ -627,7 +662,8 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
                                      support for any functions other
                                      than function 0. */ |
                               1 << 4 /* Get Namespace Label Size */ |
-                              1 << 5 /* Get Namespace Label Data */;
+                              1 << 5 /* Get Namespace Label Data */ |
+                              1 << 6 /* Set Namespace Label Data */;
         }
         nvdimm_dsm_function0(supported_func, dsm_mem_addr);
         return;
@@ -653,6 +689,12 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
             return;
         }
         break;
+    case 0x6 /* Set Namespace Label Data */:
+        if (nvdimm->label_size) {
+            nvdimm_dsm_set_label_data(nvdimm, in, dsm_mem_addr);
+            return;
+        }
+        break;
     }
 
     nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 14/15] nvdimm acpi: support Set Namespace Label Data function

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

On Fri, May 20, 2016 at 04:20:11PM +0800, Xiao Guangrong wrote:
> Function 6 is used to set Namespace Label Data
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/acpi/nvdimm.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 43 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v2 15/15] docs: add NVDIMM ACPI documentation

[Xiao Guangrong]

It describes the basic concepts of NVDIMM ACPI and the interfaces
between QEMU and the ACPI BIOS

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 docs/specs/acpi_nvdimm.txt | 132 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 docs/specs/acpi_nvdimm.txt

diff --git a/docs/specs/acpi_nvdimm.txt b/docs/specs/acpi_nvdimm.txt
new file mode 100644
index 0000000..0fdd251
--- /dev/null
+++ b/docs/specs/acpi_nvdimm.txt
@@ -0,0 +1,132 @@
+QEMU<->ACPI BIOS NVDIMM interface
+---------------------------------
+
+QEMU supports NVDIMM via ACPI. This document describes the basic concepts of
+NVDIMM ACPI and the interface between QEMU and the ACPI BIOS.
+
+NVDIMM ACPI Background
+----------------------
+NVDIMM is introduced in ACPI 6.0 which defines an NVDIMM root device under
+_SB scope with a _HID of “ACPI0012”. For each NVDIMM present or intended
+to be supported by platform, platform firmware also exposes an ACPI
+Namespace Device under the root device.
+
+The NVDIMM child devices under the NVDIMM root device are defined with _ADR
+corresponding to the NFIT device handle. The NVDIMM root device and the
+NVDIMM devices can have device specific methods (_DSM) to provide additional
+functions specific to a particular NVDIMM implementation.
+
+This is an example from ACPI 6.0, a platform contains one NVDIMM:
+
+Scope (\_SB){
+   Device (NVDR) // Root device
+   {
+      Name (_HID, “ACPI0012”)
+      Method (_STA) {...}
+      Method (_FIT) {...}
+      Method (_DSM, ...) {...}
+      Device (NVD)
+      {
+         Name(_ADR, h) //where h is NFIT Device Handle for this NVDIMM
+         Method (_DSM, ...) {...}
+      }
+   }
+}
+
+Method supported on both NVDIMM root device and NVDIMM device
+_DSM (Device Specific Method)
+   It is a control method that enables devices to provide device specific
+   control functions that are consumed by the device driver.
+   The NVDIMM DSM specification can be found at:
+        http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
+
+   Arguments:
+   Arg0 – A Buffer containing a UUID (16 Bytes)
+   Arg1 – An Integer containing the Revision ID (4 Bytes)
+   Arg2 – An Integer containing the Function Index (4 Bytes)
+   Arg3 – A package containing parameters for the function specified by the
+          UUID, Revision ID, and Function Index
+
+   Return Value:
+   If Function Index = 0, a Buffer containing a function index bitfield.
+   Otherwise, the return value and type depends on the UUID, revision ID
+   and function index which are described in the DSM specification.
+
+Methods on NVDIMM ROOT Device
+_FIT(Firmware Interface Table)
+   It evaluates to a buffer returning data in the format of a series of NFIT
+   Type Structure.
+
+   Arguments: None
+
+   Return Value:
+   A Buffer containing a list of NFIT Type structure entries.
+
+   The detailed definition of the structure can be found at ACPI 6.0: 5.2.25
+   NVDIMM Firmware Interface Table (NFIT).
+
+QEMU NVDIMM Implemention
+========================
+QEMU uses 4 bytes IO Port starting from 0x0a18 and a RAM-based memory page
+for NVDIMM ACPI.
+
+Memory:
+   QEMU uses BIOS Linker/loader feature to ask BIOS to allocate a memory
+   page and dynamically patch its into a int32 object named "MEMA" in ACPI.
+
+   This page is RAM-based and it is used to transfer data between _DSM
+   method and QEMU. If ACPI has control, this pages is owned by ACPI which
+   writes _DSM input data to it, otherwise, it is owned by QEMU which
+   emulates _DSM access and writes the output data to it.
+
+   ACPI writes _DSM Input Data (based on the offset in the page):
+   [0x0 - 0x3]: 4 bytes, NVDIMM Device Handle, 0 is reserved for NVDIMM
+                Root device.
+   [0x4 - 0x7]: 4 bytes, Revision ID, that is the Arg1 of _DSM method.
+   [0x8 - 0xB]: 4 bytes. Function Index, that is the Arg2 of _DSM method.
+   [0xC - 0xFFF]: 4084 bytes, the Arg3 of _DSM method.
+
+   QEMU Writes Output Data (based on the offset in the page):
+   [0x0 - 0x3]: 4 bytes, the length of result
+   [0x4 - 0xFFF]: 4092 bytes, the DSM result filled by QEMU
+
+IO Port 0x0a18 - 0xa1b:
+   ACPI writes the address of the memory page allocated by BIOS to this
+   port then QEMU gets the control and fills the result in the memory page.
+
+   write Access:
+   [0x0a18 - 0xa1b]: 4 bytes, the address of the memory page allocated
+                     by BIOS.
+
+_DSM process diagram:
+---------------------
+"MEMA" indicates the address of memory page allocated by BIOS.
+
+ +----------------------+      +-----------------------+
+ |    1. OSPM           |      |    2. OSPM            |
+ | save _DSM input data |      |  write "MEMA" to      | Exit to QEMU
+ | to the page          +----->|  IO port 0x0a18       +------------+
+ | indicated by "MEMA"  |      |                       |            |
+ +----------------------+      +-----------------------+            |
+                                                                    |
+                                                                    v
+ +-------------   ----+       +-----------+      +------------------+--------+
+ |      5 QEMU        |       | 4 QEMU    |      |        3. QEMU            |
+ | write _DSM result  |       |  emulate  |      | get _DSM input data from  |
+ | to the page        +<------+ _DSM      +<-----+ the page indicated by the |
+ |                    |       |           |      | value from the IO port    |
+ +--------+-----------+       +-----------+      +---------------------------+
+          |
+          | Enter Guest
+          |
+          v
+ +--------------------------+      +--------------+
+ |     6 OSPM               |      |   7 OSPM     |
+ | result size is returned  |      |  _DSM return |
+ | by reading  DSM          +----->+              |
+ | result from the page     |      |              |
+ +--------------------------+      +--------------+
+
+ _FIT implementation
+ -------------------
+ TODO (will fill it when nvdimm hotplug is introduced)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 00/15] PATCH 00/15] NVDIMM: introduce nvdimm label support

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2995 bytes --]

On Fri, May 20, 2016 at 04:19:57PM +0800, Xiao Guangrong wrote:
> Changelog in v2:
> Thanks for Stefan's review, the changes in this version are:
> - rename nvdimm device parameter 'reserve-label' to 'label-size' to
>   specify the size of label
> - add comment to explain why assert() used in nvdimm_assert_rw_label_data()
>   is safe
> - follow the code style of 'foo() return;' if nothing is returned by fool()
> - fix the value of "Non-existing-Memory-Device"
> - fix the handling the DSM functions we currently did not support
> 
>  
> This patchset is against commit 2912f22759 (fixup! virtio: convert to use DMA
> api) on pci branch of Michael's git tree and can be found at:
>       https://github.com/xiaogr/qemu.git nvdimm-label-v2
> 
> This is the last part of vNVDIMM implementation which introduces nvdimm
> label support
> 
> Currently Linux NVDIMM driver does not support namespace operation on this
> kind of PMEM, apply below changes to support dynamical namespace:
> 
> @@ -798,7 +823,8 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *a
>                         continue;
>                 }
>  
> -               if (nfit_mem->bdw && nfit_mem->memdev_pmem)
> +               //if (nfit_mem->bdw && nfit_mem->memdev_pmem)
> +               if (nfit_mem->memdev_pmem)
>                         flags |= NDD_ALIASING;
> 
> You can append a NVDIMM device in guest and do:                       
> # cd /sys/bus/nd/devices/
> # cd namespace0.0/
> # echo `uuidgen` > uuid
> # echo `expr 1024 \* 1024 \* 128` > size
> then reload nd.pmem.ko
> 
> You can see /dev/pmem0 appears
> 
> Xiao Guangrong (15):
>   pc-dimm: get memory region from ->get_memory_region()
>   pc-dimm: introduce realize callback
>   pc-dimm: keep the state of the whole backend memory
>   nvdimm: support nvdimm label
>   acpi: add aml_object_type
>   acpi: add aml_call5
>   nvdimm acpi: set HDLE properly
>   nvdimm acpi: save arg3 of _DSM method
>   nvdimm acpi: check UUID
>   nvdimm acpi: abstract the operations for root & nvdimm devices
>   nvdimm acpi: check revision
>   nvdimm acpi: support Get Namespace Label Size function
>   nvdimm acpi: support Get Namespace Label Data function
>   nvdimm acpi: support Set Namespace Label Data function
>   docs: add NVDIMM ACPI documentation
> 
>  docs/specs/acpi_nvdimm.txt  | 132 +++++++++++++++
>  hw/acpi/aml-build.c         |  22 +++
>  hw/acpi/nvdimm.c            | 400 ++++++++++++++++++++++++++++++++++++++++----
>  hw/mem/nvdimm.c             | 122 ++++++++++++++
>  hw/mem/pc-dimm.c            |  21 ++-
>  include/hw/acpi/aml-build.h |   3 +
>  include/hw/mem/nvdimm.h     |  55 +++++-
>  include/hw/mem/pc-dimm.h    |   6 +-
>  8 files changed, 723 insertions(+), 38 deletions(-)
>  create mode 100644 docs/specs/acpi_nvdimm.txt

I have reviewed the non-ACPI parts of this series.  Aside from minor
comments:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 00/15] PATCH 00/15] NVDIMM: introduce nvdimm label support

[Xiao Guangrong]

On 05/31/2016 02:52 AM, Stefan Hajnoczi wrote:

> I have reviewed the non-ACPI parts of this series.  Aside from minor
> comments:
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>


Really appreciate all your review! Thanks your, Stefan!