[Qemu-devel] [PATCH v2 0/3] hw/arm/exynos: QOM-ify the SoC

[Qemu-devel] [PATCH v2 0/3] hw/arm/exynos: QOM-ify the SoC

[Krzysztof Kozlowski]

Hi,

Changes since v1:
=================
1. s/RAM/DRAM/ in commit msg of first patch (as suggested by Philippe).
2. Add Philippe's reviewed-by.


Convert the Exynos4210 SoC driver into QOM model.

No external dependencies, rebased on v2.9.0-363-g0de9191deb14.

Best regards,
Krzysztof


Krzysztof Kozlowski (3):
  hw/arm/exynos: Move DRAM initialization next boards
  hw/arm/exynos: Declare local variables in some order
  hw/arm/exynos: QOM-ify the SoC

 hw/arm/exynos4210.c         | 40 +++++++++++++++-------------------
 hw/arm/exynos4_boards.c     | 53 +++++++++++++++++++++++++++++++++++++++------
 include/hw/arm/exynos4210.h | 11 +++++-----
 3 files changed, 69 insertions(+), 35 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH v2 1/3] hw/arm/exynos: Move DRAM initialization next boards

[Krzysztof Kozlowski]

Before QOM-ifying the Exynos4 SoC model, move the DRAM initialization
from exynos4210.c to exynos4_boards.c because DRAM is board specific,
not SoC.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/exynos4210.c         | 20 +-----------------
 hw/arm/exynos4_boards.c     | 50 ++++++++++++++++++++++++++++++++++++++-------
 include/hw/arm/exynos4210.h |  5 +----
 3 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 960f27e45a36..0da877f8db0a 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -160,13 +160,11 @@ static uint64_t exynos4210_calc_affinity(int cpu)
     return mp_affinity;
 }
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
-        unsigned long ram_size)
+Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
     int i, n;
     Exynos4210State *s = g_new(Exynos4210State, 1);
     qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
-    unsigned long mem_size;
     DeviceState *dev;
     SysBusDevice *busdev;
     ObjectClass *cpu_oc;
@@ -299,22 +297,6 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
     memory_region_add_subregion(system_mem, EXYNOS4210_IRAM_BASE_ADDR,
                                 &s->iram_mem);
 
-    /* DRAM */
-    mem_size = ram_size;
-    if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
-        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
-                mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_fatal);
-        vmstate_register_ram_global(&s->dram1_mem);
-        memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
-                &s->dram1_mem);
-        mem_size = EXYNOS4210_DRAM_MAX_SIZE;
-    }
-    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
-                           &error_fatal);
-    vmstate_register_ram_global(&s->dram0_mem);
-    memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
-            &s->dram0_mem);
-
    /* PMU.
     * The only reason of existence at the moment is that secondary CPU boot
     * loader uses PMU INFORM5 register as a holding pen.
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 4853c318023c..6240b26839cd 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -22,6 +22,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu-common.h"
 #include "cpu.h"
@@ -56,6 +57,12 @@ typedef enum Exynos4BoardType {
     EXYNOS4_NUM_OF_BOARDS
 } Exynos4BoardType;
 
+typedef struct Exynos4BoardState {
+    Exynos4210State *soc;
+    MemoryRegion dram0_mem;
+    MemoryRegion dram1_mem;
+} Exynos4BoardState;
+
 static int exynos4_board_id[EXYNOS4_NUM_OF_BOARDS] = {
     [EXYNOS4_BOARD_NURI]     = 0xD33,
     [EXYNOS4_BOARD_SMDKC210] = 0xB16,
@@ -96,9 +103,34 @@ static void lan9215_init(uint32_t base, qemu_irq irq)
     }
 }
 
-static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
-                                                   Exynos4BoardType board_type)
+static void exynos4_boards_init_ram(Exynos4BoardState *s,
+                                    MemoryRegion *system_mem,
+                                    unsigned long ram_size)
+{
+    unsigned long mem_size = ram_size;
+
+    if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
+        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
+                               mem_size - EXYNOS4210_DRAM_MAX_SIZE,
+                               &error_fatal);
+        vmstate_register_ram_global(&s->dram1_mem);
+        memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
+                                    &s->dram1_mem);
+        mem_size = EXYNOS4210_DRAM_MAX_SIZE;
+    }
+
+    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
+                           &error_fatal);
+    vmstate_register_ram_global(&s->dram0_mem);
+    memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
+                                &s->dram0_mem);
+}
+
+static Exynos4BoardState *
+exynos4_boards_init_common(MachineState *machine,
+                           Exynos4BoardType board_type)
 {
+    Exynos4BoardState *s = g_new(Exynos4BoardState, 1);
     MachineClass *mc = MACHINE_GET_CLASS(machine);
 
     if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
@@ -127,8 +159,12 @@ static Exynos4210State *exynos4_boards_init_common(MachineState *machine,
             machine->kernel_cmdline,
             machine->initrd_filename);
 
-    return exynos4210_init(get_system_memory(),
-            exynos4_board_ram_size[board_type]);
+    exynos4_boards_init_ram(s, get_system_memory(),
+                            exynos4_board_ram_size[board_type]);
+
+    s->soc = exynos4210_init(get_system_memory());
+
+    return s;
 }
 
 static void nuri_init(MachineState *machine)
@@ -140,11 +176,11 @@ static void nuri_init(MachineState *machine)
 
 static void smdkc210_init(MachineState *machine)
 {
-    Exynos4210State *s = exynos4_boards_init_common(machine,
-                                                    EXYNOS4_BOARD_SMDKC210);
+    Exynos4BoardState *s = exynos4_boards_init_common(machine,
+                                                      EXYNOS4_BOARD_SMDKC210);
 
     lan9215_init(SMDK_LAN9118_BASE_ADDR,
-            qemu_irq_invert(s->irq_table[exynos4210_get_irq(37, 1)]));
+            qemu_irq_invert(s->soc->irq_table[exynos4210_get_irq(37, 1)]));
     arm_load_kernel(ARM_CPU(first_cpu), &exynos4_board_binfo);
 }
 
diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index d9e08014d6ff..098a69ec73d3 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -93,8 +93,6 @@ typedef struct Exynos4210State {
     MemoryRegion iram_mem;
     MemoryRegion irom_mem;
     MemoryRegion irom_alias_mem;
-    MemoryRegion dram0_mem;
-    MemoryRegion dram1_mem;
     MemoryRegion boot_secondary;
     MemoryRegion bootreg_mem;
     I2CBus *i2c_if[EXYNOS4210_I2C_NUMBER];
@@ -103,8 +101,7 @@ typedef struct Exynos4210State {
 void exynos4210_write_secondary(ARMCPU *cpu,
         const struct arm_boot_info *info);
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
-        unsigned long ram_size);
+Exynos4210State *exynos4210_init(MemoryRegion *system_mem);
 
 /* Initialize exynos4210 IRQ subsystem stub */
 qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);
-- 
2.9.3

[Qemu-devel] [PATCH v2 2/3] hw/arm/exynos: Declare local variables in some order

[Krzysztof Kozlowski]

Bring some more readability by declaring local function variables: first
initialized ones and then the rest (with reversed-christmas-tree order).

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/exynos4210.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 0da877f8db0a..27a7bf28a5a9 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -162,12 +162,12 @@ static uint64_t exynos4210_calc_affinity(int cpu)
 
 Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
-    int i, n;
     Exynos4210State *s = g_new(Exynos4210State, 1);
     qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
-    DeviceState *dev;
     SysBusDevice *busdev;
     ObjectClass *cpu_oc;
+    DeviceState *dev;
+    int i, n;
 
     cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, "cortex-a9");
     assert(cpu_oc);
-- 
2.9.3

[Qemu-devel] [PATCH v2 3/3] hw/arm/exynos: QOM-ify the SoC

[Krzysztof Kozlowski]

Convert the Exynos4210 SoC code into a QOM model which is a preferred
approach instead of directly initializing SoC-related devices from the
board file.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/exynos4210.c         | 18 +++++++++++++++---
 hw/arm/exynos4_boards.c     |  9 ++++++---
 include/hw/arm/exynos4210.h |  8 ++++++--
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 27a7bf28a5a9..034fc8be9d76 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -160,9 +160,10 @@ static uint64_t exynos4210_calc_affinity(int cpu)
     return mp_affinity;
 }
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
+static void exynos4210_init(Object *obj)
 {
-    Exynos4210State *s = g_new(Exynos4210State, 1);
+    MemoryRegion *system_mem = get_system_memory();
+    Exynos4210State *s = EXYNOS4210(obj);
     qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
     SysBusDevice *busdev;
     ObjectClass *cpu_oc;
@@ -402,6 +403,17 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 
     sysbus_create_simple(TYPE_EXYNOS4210_EHCI, EXYNOS4210_EHCI_BASE_ADDR,
             s->irq_table[exynos4210_get_irq(28, 3)]);
+}
+
+static const TypeInfo exynos4210_type_info = {
+    .name = TYPE_EXYNOS4210,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(Exynos4210State),
+    .instance_init = exynos4210_init,
+};
 
-    return s;
+static void exynos4210_register_types(void)
+{
+    type_register_static(&exynos4210_type_info);
 }
+type_init(exynos4210_register_types)
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 6240b26839cd..5e7c6b562ae2 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -58,7 +58,7 @@ typedef enum Exynos4BoardType {
 } Exynos4BoardType;
 
 typedef struct Exynos4BoardState {
-    Exynos4210State *soc;
+    Exynos4210State soc;
     MemoryRegion dram0_mem;
     MemoryRegion dram1_mem;
 } Exynos4BoardState;
@@ -162,7 +162,10 @@ exynos4_boards_init_common(MachineState *machine,
     exynos4_boards_init_ram(s, get_system_memory(),
                             exynos4_board_ram_size[board_type]);
 
-    s->soc = exynos4210_init(get_system_memory());
+    object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210);
+    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
+                              &error_abort);
+    object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
 
     return s;
 }
@@ -180,7 +183,7 @@ static void smdkc210_init(MachineState *machine)
                                                       EXYNOS4_BOARD_SMDKC210);
 
     lan9215_init(SMDK_LAN9118_BASE_ADDR,
-            qemu_irq_invert(s->soc->irq_table[exynos4210_get_irq(37, 1)]));
+            qemu_irq_invert(s->soc.irq_table[exynos4210_get_irq(37, 1)]));
     arm_load_kernel(ARM_CPU(first_cpu), &exynos4_board_binfo);
 }
 
diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 098a69ec73d3..116eae62756b 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -29,6 +29,10 @@
 #include "exec/memory.h"
 #include "target/arm/cpu-qom.h"
 
+#define TYPE_EXYNOS4210 "exynos4210"
+#define EXYNOS4210(obj) \
+    OBJECT_CHECK(Exynos4210State, (obj), TYPE_EXYNOS4210)
+
 #define EXYNOS4210_NCPUS                    2
 
 #define EXYNOS4210_DRAM0_BASE_ADDR          0x40000000
@@ -85,6 +89,8 @@ typedef struct Exynos4210Irq {
 } Exynos4210Irq;
 
 typedef struct Exynos4210State {
+    DeviceState parent_obj;
+
     ARMCPU *cpu[EXYNOS4210_NCPUS];
     Exynos4210Irq irqs;
     qemu_irq *irq_table;
@@ -101,8 +107,6 @@ typedef struct Exynos4210State {
 void exynos4210_write_secondary(ARMCPU *cpu,
         const struct arm_boot_info *info);
 
-Exynos4210State *exynos4210_init(MemoryRegion *system_mem);
-
 /* Initialize exynos4210 IRQ subsystem stub */
 qemu_irq *exynos4210_init_irq(Exynos4210Irq *env);
 
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 0/3] hw/arm/exynos: QOM-ify the SoC

[Peter Maydell]

On 7 May 2017 at 19:19, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> Hi,
>
> Changes since v1:
> =================
> 1. s/RAM/DRAM/ in commit msg of first patch (as suggested by Philippe).
> 2. Add Philippe's reviewed-by.
>
>
> Convert the Exynos4210 SoC driver into QOM model.
>
> No external dependencies, rebased on v2.9.0-363-g0de9191deb14.
>
> Best regards,
> Krzysztof
>
>
> Krzysztof Kozlowski (3):
>   hw/arm/exynos: Move DRAM initialization next boards
>   hw/arm/exynos: Declare local variables in some order
>   hw/arm/exynos: QOM-ify the SoC



Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2 0/3] hw/arm/exynos: QOM-ify the SoC

[Peter Maydell]

On 30 May 2017 at 13:07, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 May 2017 at 19:19, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>> Hi,
>>
>> Changes since v1:
>> =================
>> 1. s/RAM/DRAM/ in commit msg of first patch (as suggested by Philippe).
>> 2. Add Philippe's reviewed-by.
>>
>>
>> Convert the Exynos4210 SoC driver into QOM model.
>>
>> No external dependencies, rebased on v2.9.0-363-g0de9191deb14.
>>
>> Best regards,
>> Krzysztof
>>
>>
>> Krzysztof Kozlowski (3):
>>   hw/arm/exynos: Move DRAM initialization next boards
>>   hw/arm/exynos: Declare local variables in some order
>>   hw/arm/exynos: QOM-ify the SoC
>
>
>
> Applied to target-arm.next, thanks

...unfortunately I've just found that patch 3 breaks "make check":
tests/device-introspect-test subtest /arm/device/introspect/concrete
segfaults due to memory corruption. Running it under valgrind:

(cd build/x86 && QTEST_QEMU_BINARY="valgrind --vgdb-error=1
arm-softmmu/qemu-system-arm" QTEST_QEMU_IMG=qemu-img
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k
--verbose -m=quick tests/device-introspect-test -p
/arm/device/introspect/concrete)
TEST: tests/device-introspect-test... (pid=8506)
  /arm/device/introspect/concrete:
==8508== Memcheck, a memory error detector
==8508== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==8508== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
[etc etc]
==8508== Thread 2:
==8508== Invalid read of size 8
==8508==    at 0x394333: memory_region_unref (memory.c:1540)
==8508==    by 0x3908D9: flatview_destroy (memory.c:288)
==8508==    by 0x390951: flatview_unref (memory.c:302)
==8508==    by 0x8F9F9A: call_rcu_thread (rcu.c:272)
==8508==    by 0x1DF6E6B9: start_thread (pthread_create.c:333)
==8508==    by 0x1E28A82C: clone (clone.S:109)
==8508==  Address 0x2d400668 is 10,952 bytes inside a block of size
12,512 free'd
==8508==    at 0x4C2EDEB: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8508==    by 0x7D7EDE: object_finalize (object.c:472)
==8508==    by 0x7D8DD5: object_unref (object.c:903)
==8508==    by 0x53A71B: qmp_device_list_properties (qmp.c:580)
==8508==    by 0x52B42E: qmp_marshal_device_list_properties (qmp-marshal.c:1355)
==8508==    by 0x8CE2B5: do_qmp_dispatch (qmp-dispatch.c:104)
==8508==    by 0x8CE3ED: qmp_dispatch (qmp-dispatch.c:131)
==8508==    by 0x3847E9: handle_qmp_command (monitor.c:3824)
==8508==    by 0x8D5812: json_message_process_token (json-streamer.c:105)
==8508==    by 0x90100C: json_lexer_feed_char (json-lexer.c:319)
==8508==    by 0x901154: json_lexer_feed (json-lexer.c:369)
==8508==    by 0x8D58B9: json_message_parser_feed (json-streamer.c:124)
==8508==  Block was alloc'd at
==8508==    at 0x4C2DB8F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8508==    by 0x1B7A5718: g_malloc (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
==8508==    by 0x7D7F34: object_new_with_type (object.c:483)
==8508==    by 0x7D7F90: object_new (object.c:494)
==8508==    by 0x53A5C6: qmp_device_list_properties (qmp.c:545)
==8508==    by 0x52B42E: qmp_marshal_device_list_properties (qmp-marshal.c:1355)
==8508==    by 0x8CE2B5: do_qmp_dispatch (qmp-dispatch.c:104)
==8508==    by 0x8CE3ED: qmp_dispatch (qmp-dispatch.c:131)
==8508==    by 0x3847E9: handle_qmp_command (monitor.c:3824)
==8508==    by 0x8D5812: json_message_process_token (json-streamer.c:105)
==8508==    by 0x90100C: json_lexer_feed_char (json-lexer.c:319)
==8508==    by 0x901154: json_lexer_feed (json-lexer.c:369)
==8508==

This is because the TYPE_EXYNOS4310 device now creates and maps
the exynos4220.chipid MemoryRegion into the system memory space
in its instance_init method, but it doesn't have any code for
unmapping it when the device is deleted. This test does a
"create but don't realize; then delete it" for each device, so
what happens is that the memory for the device is allocated on
create, freed on object deletion, and then referenced because
the MR is still in the system memory's flatview.

For devices like this exynos one which are never going to really
need to be deleted, the only requirement is that we can at least
do a create-delete so we can introspect them for information
about properties. This means that they shouldn't do things
like adding MRs to the system memory space until the realize
method. I think that should fix this problem.

I'm dropping this patchset from my queue.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/3] hw/arm/exynos: QOM-ify the SoC

[Krzysztof Kozlowski]

On Tue, May 30, 2017 at 3:00 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 30 May 2017 at 13:07, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 7 May 2017 at 19:19, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>> Hi,
>>>
>>> Changes since v1:
>>> =================
>>> 1. s/RAM/DRAM/ in commit msg of first patch (as suggested by Philippe).
>>> 2. Add Philippe's reviewed-by.
>>>
>>>
>>> Convert the Exynos4210 SoC driver into QOM model.
>>>
>>> No external dependencies, rebased on v2.9.0-363-g0de9191deb14.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>>
>>> Krzysztof Kozlowski (3):
>>>   hw/arm/exynos: Move DRAM initialization next boards
>>>   hw/arm/exynos: Declare local variables in some order
>>>   hw/arm/exynos: QOM-ify the SoC
>>
>>
>>
>> Applied to target-arm.next, thanks
>
> ...unfortunately I've just found that patch 3 breaks "make check":
> tests/device-introspect-test subtest /arm/device/introspect/concrete
> segfaults due to memory corruption. Running it under valgrind:
>
> (cd build/x86 && QTEST_QEMU_BINARY="valgrind --vgdb-error=1
> arm-softmmu/qemu-system-arm" QTEST_QEMU_IMG=qemu-img
> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k
> --verbose -m=quick tests/device-introspect-test -p
> /arm/device/introspect/concrete)
> TEST: tests/device-introspect-test... (pid=8506)
>   /arm/device/introspect/concrete:
> ==8508== Memcheck, a memory error detector
> ==8508== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
> ==8508== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
> [etc etc]
> ==8508== Thread 2:
> ==8508== Invalid read of size 8
> ==8508==    at 0x394333: memory_region_unref (memory.c:1540)
> ==8508==    by 0x3908D9: flatview_destroy (memory.c:288)
> ==8508==    by 0x390951: flatview_unref (memory.c:302)
> ==8508==    by 0x8F9F9A: call_rcu_thread (rcu.c:272)
> ==8508==    by 0x1DF6E6B9: start_thread (pthread_create.c:333)
> ==8508==    by 0x1E28A82C: clone (clone.S:109)
> ==8508==  Address 0x2d400668 is 10,952 bytes inside a block of size
> 12,512 free'd
> ==8508==    at 0x4C2EDEB: free (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==8508==    by 0x7D7EDE: object_finalize (object.c:472)
> ==8508==    by 0x7D8DD5: object_unref (object.c:903)
> ==8508==    by 0x53A71B: qmp_device_list_properties (qmp.c:580)
> ==8508==    by 0x52B42E: qmp_marshal_device_list_properties (qmp-marshal.c:1355)
> ==8508==    by 0x8CE2B5: do_qmp_dispatch (qmp-dispatch.c:104)
> ==8508==    by 0x8CE3ED: qmp_dispatch (qmp-dispatch.c:131)
> ==8508==    by 0x3847E9: handle_qmp_command (monitor.c:3824)
> ==8508==    by 0x8D5812: json_message_process_token (json-streamer.c:105)
> ==8508==    by 0x90100C: json_lexer_feed_char (json-lexer.c:319)
> ==8508==    by 0x901154: json_lexer_feed (json-lexer.c:369)
> ==8508==    by 0x8D58B9: json_message_parser_feed (json-streamer.c:124)
> ==8508==  Block was alloc'd at
> ==8508==    at 0x4C2DB8F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==8508==    by 0x1B7A5718: g_malloc (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
> ==8508==    by 0x7D7F34: object_new_with_type (object.c:483)
> ==8508==    by 0x7D7F90: object_new (object.c:494)
> ==8508==    by 0x53A5C6: qmp_device_list_properties (qmp.c:545)
> ==8508==    by 0x52B42E: qmp_marshal_device_list_properties (qmp-marshal.c:1355)
> ==8508==    by 0x8CE2B5: do_qmp_dispatch (qmp-dispatch.c:104)
> ==8508==    by 0x8CE3ED: qmp_dispatch (qmp-dispatch.c:131)
> ==8508==    by 0x3847E9: handle_qmp_command (monitor.c:3824)
> ==8508==    by 0x8D5812: json_message_process_token (json-streamer.c:105)
> ==8508==    by 0x90100C: json_lexer_feed_char (json-lexer.c:319)
> ==8508==    by 0x901154: json_lexer_feed (json-lexer.c:369)
> ==8508==
>
> This is because the TYPE_EXYNOS4310 device now creates and maps
> the exynos4220.chipid MemoryRegion into the system memory space
> in its instance_init method, but it doesn't have any code for
> unmapping it when the device is deleted. This test does a
> "create but don't realize; then delete it" for each device, so
> what happens is that the memory for the device is allocated on
> create, freed on object deletion, and then referenced because
> the MR is still in the system memory's flatview.
>
> For devices like this exynos one which are never going to really
> need to be deleted, the only requirement is that we can at least
> do a create-delete so we can introspect them for information
> about properties. This means that they shouldn't do things
> like adding MRs to the system memory space until the realize
> method. I think that should fix this problem.
>
> I'm dropping this patchset from my queue.

Thanks for hints, I'll fix it up and resend.

Best regards,
Krzysztof