| Message ID | 20180615140448.32234-11-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pc-dimm: next bunch of cleanups | expand |
On Fri, Jun 15, 2018 at 04:04:45PM +0200, David Hildenbrand wrote: > This way we can easily check if the region has already been inititalized > without having to rely on the size of an uninitialized region being 0. > > Signed-off-by: David Hildenbrand <david@redhat.com> I'm not terribly convinced that this is a worthwhile change, but in the sense that the patch appears to be technically correct: Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/mem/nvdimm.c | 9 +++++---- > include/hw/mem/nvdimm.h | 2 +- > 2 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > index 7260c9c6b1..db7d8c3050 100644 > --- a/hw/mem/nvdimm.c > +++ b/hw/mem/nvdimm.c > @@ -43,7 +43,7 @@ static void nvdimm_set_label_size(Object *obj, Visitor *v, const char *name, > Error *local_err = NULL; > uint64_t value; > > - if (memory_region_size(&nvdimm->nvdimm_mr)) { > + if (nvdimm->nvdimm_mr) { > error_setg(&local_err, "cannot change property value"); > goto out; > } > @@ -75,7 +75,7 @@ static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error **errp) > { > NVDIMMDevice *nvdimm = NVDIMM(dimm); > > - return &nvdimm->nvdimm_mr; > + return nvdimm->nvdimm_mr; > } > > static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) > @@ -102,9 +102,10 @@ static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) > return; > } > > - memory_region_init_alias(&nvdimm->nvdimm_mr, OBJECT(dimm), > + nvdimm->nvdimm_mr = g_new(MemoryRegion, 1); > + memory_region_init_alias(nvdimm->nvdimm_mr, OBJECT(dimm), > "nvdimm-memory", mr, 0, pmem_size); > - nvdimm->nvdimm_mr.align = align; > + nvdimm->nvdimm_mr->align = align; > } > > /* > diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h > index 9340631cfc..c5c9b3c7f8 100644 > --- a/include/hw/mem/nvdimm.h > +++ b/include/hw/mem/nvdimm.h > @@ -74,7 +74,7 @@ struct NVDIMMDevice { > * 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; > + MemoryRegion *nvdimm_mr; > > /* > * The 'on' value results in the unarmed flag set in ACPI NFIT,
On 18.06.2018 02:49, David Gibson wrote: > On Fri, Jun 15, 2018 at 04:04:45PM +0200, David Hildenbrand wrote: >> This way we can easily check if the region has already been inititalized >> without having to rely on the size of an uninitialized region being 0. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> > > I'm not terribly convinced that this is a worthwhile change, but in > the sense that the patch appears to be technically correct: > Igor requested this, and as I had no strong feelings about it, I included it. At least we are not able to hand out "uninitialized regions" this way anymore. Thanks!
On Fri, 15 Jun 2018 16:04:45 +0200 David Hildenbrand <david@redhat.com> wrote: > This way we can easily check if the region has already been inititalized > without having to rely on the size of an uninitialized region being 0. > > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/mem/nvdimm.c | 9 +++++---- > include/hw/mem/nvdimm.h | 2 +- > 2 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c > index 7260c9c6b1..db7d8c3050 100644 > --- a/hw/mem/nvdimm.c > +++ b/hw/mem/nvdimm.c > @@ -43,7 +43,7 @@ static void nvdimm_set_label_size(Object *obj, Visitor *v, const char *name, > Error *local_err = NULL; > uint64_t value; > > - if (memory_region_size(&nvdimm->nvdimm_mr)) { > + if (nvdimm->nvdimm_mr) { > error_setg(&local_err, "cannot change property value"); > goto out; > } > @@ -75,7 +75,7 @@ static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error **errp) > { > NVDIMMDevice *nvdimm = NVDIMM(dimm); > > - return &nvdimm->nvdimm_mr; > + return nvdimm->nvdimm_mr; > } > > static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) > @@ -102,9 +102,10 @@ static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) > return; > } > > - memory_region_init_alias(&nvdimm->nvdimm_mr, OBJECT(dimm), > + nvdimm->nvdimm_mr = g_new(MemoryRegion, 1); > + memory_region_init_alias(nvdimm->nvdimm_mr, OBJECT(dimm), > "nvdimm-memory", mr, 0, pmem_size); > - nvdimm->nvdimm_mr.align = align; > + nvdimm->nvdimm_mr->align = align; > } > > /* > diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h > index 9340631cfc..c5c9b3c7f8 100644 > --- a/include/hw/mem/nvdimm.h > +++ b/include/hw/mem/nvdimm.h > @@ -74,7 +74,7 @@ struct NVDIMMDevice { > * 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; > + MemoryRegion *nvdimm_mr; > > /* > * The 'on' value results in the unarmed flag set in ACPI NFIT,
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c index 7260c9c6b1..db7d8c3050 100644 --- a/hw/mem/nvdimm.c +++ b/hw/mem/nvdimm.c @@ -43,7 +43,7 @@ static void nvdimm_set_label_size(Object *obj, Visitor *v, const char *name, Error *local_err = NULL; uint64_t value; - if (memory_region_size(&nvdimm->nvdimm_mr)) { + if (nvdimm->nvdimm_mr) { error_setg(&local_err, "cannot change property value"); goto out; } @@ -75,7 +75,7 @@ static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error **errp) { NVDIMMDevice *nvdimm = NVDIMM(dimm); - return &nvdimm->nvdimm_mr; + return nvdimm->nvdimm_mr; } static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) @@ -102,9 +102,10 @@ static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp) return; } - memory_region_init_alias(&nvdimm->nvdimm_mr, OBJECT(dimm), + nvdimm->nvdimm_mr = g_new(MemoryRegion, 1); + memory_region_init_alias(nvdimm->nvdimm_mr, OBJECT(dimm), "nvdimm-memory", mr, 0, pmem_size); - nvdimm->nvdimm_mr.align = align; + nvdimm->nvdimm_mr->align = align; } /* diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h index 9340631cfc..c5c9b3c7f8 100644 --- a/include/hw/mem/nvdimm.h +++ b/include/hw/mem/nvdimm.h @@ -74,7 +74,7 @@ struct NVDIMMDevice { * 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; + MemoryRegion *nvdimm_mr; /* * The 'on' value results in the unarmed flag set in ACPI NFIT,
This way we can easily check if the region has already been inititalized without having to rely on the size of an uninitialized region being 0. Signed-off-by: David Hildenbrand <david@redhat.com> --- hw/mem/nvdimm.c | 9 +++++---- include/hw/mem/nvdimm.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-)