diff mbox

[memory,v3,6/9] memory: MemoryRegion: QOMify

Message ID b7edec48a845a3f8e1f37da87910d7534260d360.1401681566.git.peter.crosthwaite@xilinx.com
State New
Headers show

Commit Message

Peter Crosthwaite June 2, 2014, 4:17 a.m. UTC 
QOMify memory regions as an Object. The former init() and destroy()
routines become instance_init() and instance_finalize() resp.

memory_region_init() is re-implemented to be:
object_initialize() + set fields

memory_region_destroy() is re-implemented to call finalize().

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v2:
use object_unref() to Destroy MRs properly (Paolo Review)
Drop stale FIXME

 include/exec/memory.h |  6 ++++++
 memory.c              | 53 +++++++++++++++++++++++++++++++++------------------
 2 files changed, 40 insertions(+), 19 deletions(-)

Comments

Peter Maydell June 2, 2014, 12:18 p.m. UTC | #1 
On 2 June 2014 05:17, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> QOMify memory regions as an Object. The former init() and destroy()
> routines become instance_init() and instance_finalize() resp.
>
> memory_region_init() is re-implemented to be:
> object_initialize() + set fields

> +static void memory_region_initfn(Object *obj)
> +{
> +    MemoryRegion *mr = MEMORY_REGION(obj);
> +
> +    mr->ops = &unassigned_mem_ops;
>      mr->enabled = true;
> -    mr->terminates = false;
> -    mr->ram = false;
>      mr->romd_mode = true;
> -    mr->readonly = false;
> -    mr->rom_device = false;
>      mr->destructor = memory_region_destructor_none;
> -    mr->priority = 0;
> -    mr->may_overlap = false;
> -    mr->alias = NULL;
>      QTAILQ_INIT(&mr->subregions);
>      memset(&mr->subregions_link, 0, sizeof mr->subregions_link);

We rely on QOM objects being zero-initialized for other
fields, so why leave the explicit memset for this one?

thanks
-- PMM
Peter Crosthwaite June 2, 2014, 10:39 p.m. UTC | #2 
On Mon, Jun 2, 2014 at 10:18 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 2 June 2014 05:17, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> QOMify memory regions as an Object. The former init() and destroy()
>> routines become instance_init() and instance_finalize() resp.
>>
>> memory_region_init() is re-implemented to be:
>> object_initialize() + set fields
>
>> +static void memory_region_initfn(Object *obj)
>> +{
>> +    MemoryRegion *mr = MEMORY_REGION(obj);
>> +
>> +    mr->ops = &unassigned_mem_ops;
>>      mr->enabled = true;
>> -    mr->terminates = false;
>> -    mr->ram = false;
>>      mr->romd_mode = true;
>> -    mr->readonly = false;
>> -    mr->rom_device = false;
>>      mr->destructor = memory_region_destructor_none;
>> -    mr->priority = 0;
>> -    mr->may_overlap = false;
>> -    mr->alias = NULL;
>>      QTAILQ_INIT(&mr->subregions);
>>      memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
>
> We rely on QOM objects being zero-initialized for other
> fields, so why leave the explicit memset for this one?
>

It slipped through the cracks in my scan for 0s. Will Fix.

Regards,
Peter

> thanks
> -- PMM
>
diff mbox

Patch

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1d55ad9..371c066 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -31,10 +31,15 @@ 
 #include "qemu/queue.h"
 #include "qemu/int128.h"
 #include "qemu/notify.h"
+#include "qom/object.h"
 
 #define MAX_PHYS_ADDR_SPACE_BITS 62
 #define MAX_PHYS_ADDR            (((hwaddr)1 << MAX_PHYS_ADDR_SPACE_BITS) - 1)
 
+#define TYPE_MEMORY_REGION "qemu:memory-region"
+#define MEMORY_REGION(obj) \
+        OBJECT_CHECK(MemoryRegion, (obj), TYPE_MEMORY_REGION)
+
 typedef struct MemoryRegionOps MemoryRegionOps;
 typedef struct MemoryRegionMmio MemoryRegionMmio;
 
@@ -130,6 +135,7 @@  typedef struct CoalescedMemoryRange CoalescedMemoryRange;
 typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
 
 struct MemoryRegion {
+    Object parent_obj;
     /* All fields are private - violators will be prosecuted */
     const MemoryRegionOps *ops;
     const MemoryRegionIOMMUOps *iommu_ops;
diff --git a/memory.c b/memory.c
index aa25667..90c0557 100644
--- a/memory.c
+++ b/memory.c
@@ -850,35 +850,27 @@  void memory_region_init(MemoryRegion *mr,
                         const char *name,
                         uint64_t size)
 {
-    mr->ops = &unassigned_mem_ops;
-    mr->opaque = NULL;
+    object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
+
     mr->owner = owner;
-    mr->iommu_ops = NULL;
-    mr->parent = NULL;
     mr->size = int128_make64(size);
     if (size == UINT64_MAX) {
         mr->size = int128_2_64();
     }
-    mr->addr = 0;
-    mr->subpage = false;
+    mr->name = g_strdup(name);
+}
+
+static void memory_region_initfn(Object *obj)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
+    mr->ops = &unassigned_mem_ops;
     mr->enabled = true;
-    mr->terminates = false;
-    mr->ram = false;
     mr->romd_mode = true;
-    mr->readonly = false;
-    mr->rom_device = false;
     mr->destructor = memory_region_destructor_none;
-    mr->priority = 0;
-    mr->may_overlap = false;
-    mr->alias = NULL;
     QTAILQ_INIT(&mr->subregions);
     memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
     QTAILQ_INIT(&mr->coalesced);
-    mr->name = g_strdup(name);
-    mr->dirty_log_mask = 0;
-    mr->ioeventfd_nb = 0;
-    mr->ioeventfds = NULL;
-    mr->flush_coalesced_mmio = false;
 }
 
 static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
@@ -1099,8 +1091,10 @@  void memory_region_init_reservation(MemoryRegion *mr,
     memory_region_init_io(mr, owner, &unassigned_mem_ops, mr, name, size);
 }
 
-void memory_region_destroy(MemoryRegion *mr)
+static void memory_region_finalize(Object *obj)
 {
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
     assert(QTAILQ_EMPTY(&mr->subregions));
     assert(memory_region_transaction_depth == 0);
     mr->destructor(mr);
@@ -1109,6 +1103,12 @@  void memory_region_destroy(MemoryRegion *mr)
     g_free(mr->ioeventfds);
 }
 
+void memory_region_destroy(MemoryRegion *mr)
+{
+    object_unref(OBJECT(mr));
+}
+
+
 Object *memory_region_owner(MemoryRegion *mr)
 {
     return mr->owner;
@@ -1886,3 +1886,18 @@  void mtree_info(fprintf_function mon_printf, void *f)
         g_free(ml);
     }
 }
+
+static const TypeInfo memory_region_info = {
+    .parent             = TYPE_OBJECT,
+    .name               = TYPE_MEMORY_REGION,
+    .instance_size      = sizeof(MemoryRegion),
+    .instance_init      = memory_region_initfn,
+    .instance_finalize  = memory_region_finalize,
+};
+
+static void memory_register_types(void)
+{
+    type_register_static(&memory_region_info);
+}
+
+type_init(memory_register_types)