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

2016-06-28 Thread Stefan Hajnoczi
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


signature.asc
Description: PGP signature


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

2016-06-28 Thread 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

2016-05-30 Thread Stefan Hajnoczi
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 
> ---
>  hw/mem/nvdimm.c | 122 
> 
>  include/hw/mem/nvdimm.h |  55 +-
>  2 files changed, 176 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


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

2016-05-20 Thread 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 
---
 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