All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24
@ 2019-05-24 18:44 Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 01/17] qom/object: Display more helpful message when an object type is missing Eduardo Habkost
                   ` (17 more replies)
  0 siblings, 18 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum

The following changes since commit a7b21f6762a2d6ec08106d8a7ccb11829914523f:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-4.1-pull-request' into staging (2019-05-24 12:47:49 +0100)

are available in the Git repository at:

  git://github.com/ehabkost/qemu.git tags/machine-next-pull-request

for you to fetch changes up to 23d1f360f3de1d968d98ba605bd3b718f5309e6f:

  hw/intc/nvic: Use object_initialize_child for correct reference counting (2019-05-24 15:29:02 -0300)

----------------------------------------------------------------
Machine Core queue, 2019-05-24

* Display more helpful message when an object type is missing
  (Philippe Mathieu-Daudé)
* Use object_initialize_child for correct reference counting
  (Philippe Mathieu-Daudé)

----------------------------------------------------------------

Queue for Machine Core patches


Philippe Mathieu-Daudé (17):
  qom/object: Display more helpful message when an object type is
    missing
  hw/ppc/pnv: Use object_initialize_child for correct reference counting
  hw/misc/macio: Use object_initialize_child for correct ref. counting
  hw/virtio: Use object_initialize_child for correct reference counting
  hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string
  hw/arm/bcm2835: Use object_initialize() on PL011State
  hw/arm/bcm2835: Use object_initialize_child for correct ref. counting
  hw/arm/aspeed: Use object_initialize_child for correct ref. counting
  hw/arm: Use object_initialize_child for correct reference counting
  hw/mips: Use object_initialize() on MIPSCPSState
  hw/mips: Use object_initialize_child for correct reference counting
  hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state
  hw/microblaze/zynqmp: Let the SoC manage the IPI devices
  hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
    counting
  hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
    counting
  hw/arm/mps2: Use object_initialize_child for correct reference
    counting
  hw/intc/nvic: Use object_initialize_child for correct reference
    counting

 include/hw/arm/bcm2835_peripherals.h |  3 +-
 hw/arm/aspeed.c                      |  6 +--
 hw/arm/aspeed_soc.c                  | 50 +++++++++--------------
 hw/arm/bcm2835_peripherals.c         | 61 +++++++++++-----------------
 hw/arm/digic.c                       | 17 +++-----
 hw/arm/imx25_pdk.c                   |  5 +--
 hw/arm/kzm.c                         |  5 +--
 hw/arm/mps2-tz.c                     |  8 ++--
 hw/arm/mps2.c                        |  8 ++--
 hw/arm/raspi.c                       |  7 ++--
 hw/arm/sabrelite.c                   |  5 +--
 hw/arm/xlnx-zcu102.c                 |  5 +--
 hw/arm/xlnx-zynqmp.c                 |  8 ++--
 hw/intc/armv7m_nvic.c                |  6 +--
 hw/microblaze/xlnx-zynqmp-pmu.c      | 45 ++++++++++----------
 hw/mips/boston.c                     | 25 ++++++------
 hw/mips/cps.c                        | 20 ++++-----
 hw/mips/mips_malta.c                 | 17 ++++----
 hw/misc/macio/macio.c                |  8 ++--
 hw/ppc/pnv.c                         | 12 ++----
 hw/virtio/virtio.c                   |  5 +--
 qom/object.c                         |  7 +++-
 22 files changed, 146 insertions(+), 187 deletions(-)

-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 01/17] qom/object: Display more helpful message when an object type is missing
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 02/17] hw/ppc/pnv: Use object_initialize_child for correct reference counting Eduardo Habkost
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

When writing a new board, adding device which uses other devices
(container) or simply refactoring, one can discover the hard way
his machine misses some devices. In the case of containers, the
error is not obvious:

  $ qemu-system-microblaze -M xlnx-zynqmp-pmu
  **
  ERROR:/source/qemu/qom/object.c:454:object_initialize_with_type: assertion failed: (type != NULL)
  Aborted (core dumped)

And we have to look at the coredump to figure the error:

  (gdb) bt
  #1  0x00007f84773cf895 in abort () at /lib64/libc.so.6
  #2  0x00007f847961fb53 in  () at /lib64/libglib-2.0.so.0
  #3  0x00007f847967a4de in g_assertion_message_expr () at /lib64/libglib-2.0.so.0
  #4  0x000055c4bcac6c11 in object_initialize_with_type (data=data@entry=0x55c4bdf239e0, size=size@entry=2464, type=<optimized out>) at /source/qemu/qom/object.c:454
  #5  0x000055c4bcac6e6d in object_initialize (data=data@entry=0x55c4bdf239e0, size=size@entry=2464, typename=typename@entry=0x55c4bcc7c643 "xlnx.zynqmp_ipi") at /source/qemu/qom/object.c:474
  #6  0x000055c4bc9ea474 in xlnx_zynqmp_pmu_init (machine=0x55c4bdd46000) at /source/qemu/hw/microblaze/xlnx-zynqmp-pmu.c:176
  #7  0x000055c4bca3b6cb in machine_run_board_init (machine=0x55c4bdd46000) at /source/qemu/hw/core/machine.c:1030
  #8  0x000055c4bc95f6d2 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at /source/qemu/vl.c:4479

Since the caller knows the type name requested, we can simply display it
to ease development.

With this patch applied we get:

  $ qemu-system-microblaze -M xlnx-zynqmp-pmu
  qemu-system-microblaze: missing object type 'xlnx.zynqmp_ipi'
  Aborted (core dumped)

Since the assert(type) check in object_initialize_with_type() is
now impossible, remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190427135642.16464-1-philmd@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qom/object.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 99c4fa707e..3966a3d461 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -28,6 +28,7 @@
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qnum.h"
 #include "qapi/qmp/qstring.h"
+#include "qemu/error-report.h"
 
 #define MAX_INTERFACES 32
 
@@ -448,7 +449,6 @@ static void object_initialize_with_type(void *data, size_t size, TypeImpl *type)
 {
     Object *obj = data;
 
-    g_assert(type != NULL);
     type_initialize(type);
 
     g_assert(type->instance_size >= sizeof(Object));
@@ -468,6 +468,11 @@ void object_initialize(void *data, size_t size, const char *typename)
 {
     TypeImpl *type = type_get_by_name(typename);
 
+    if (!type) {
+        error_report("missing object type '%s'", typename);
+        abort();
+    }
+
     object_initialize_with_type(data, size, type);
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 02/17] hw/ppc/pnv: Use object_initialize_child for correct reference counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 01/17] qom/object: Display more helpful message when an object type is missing Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 03/17] hw/misc/macio: Use object_initialize_child for correct ref. counting Eduardo Habkost
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                           child_type, &error_abort, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
     ...
?-   object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                            child_type, errp, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
     ...
?-   object_unref(OBJECT(child_ptr));
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-2-philmd@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/ppc/pnv.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index dfb4ea5742..31aa20ee25 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -994,14 +994,12 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)
         PnvCore *pnv_core = PNV_CORE(chip->cores + (i * 4) * typesize);
         int core_id = CPU_CORE(pnv_core)->core_id;
 
-        object_initialize(eq, sizeof(*eq), TYPE_PNV_QUAD);
         snprintf(eq_name, sizeof(eq_name), "eq[%d]", core_id);
+        object_initialize_child(OBJECT(chip), eq_name, eq, sizeof(*eq),
+                                TYPE_PNV_QUAD, &error_fatal, NULL);
 
-        object_property_add_child(OBJECT(chip), eq_name, OBJECT(eq),
-                                  &error_fatal);
         object_property_set_int(OBJECT(eq), core_id, "id", &error_fatal);
         object_property_set_bool(OBJECT(eq), true, "realized", &error_fatal);
-        object_unref(OBJECT(eq));
 
         pnv_xscom_add_subregion(chip, PNV9_XSCOM_EQ_BASE(eq->id),
                                 &eq->xscom_regs);
@@ -1165,10 +1163,9 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
             continue;
         }
 
-        object_initialize(pnv_core, typesize, typename);
         snprintf(core_name, sizeof(core_name), "core[%d]", core_hwid);
-        object_property_add_child(OBJECT(chip), core_name, OBJECT(pnv_core),
-                                  &error_fatal);
+        object_initialize_child(OBJECT(chip), core_name, pnv_core, typesize,
+                                typename, &error_fatal, NULL);
         object_property_set_int(OBJECT(pnv_core), smp_threads, "nr-threads",
                                 &error_fatal);
         object_property_set_int(OBJECT(pnv_core), core_hwid,
@@ -1180,7 +1177,6 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
                                        OBJECT(chip), &error_fatal);
         object_property_set_bool(OBJECT(pnv_core), true, "realized",
                                  &error_fatal);
-        object_unref(OBJECT(pnv_core));
 
         /* Each core has an XSCOM MMIO region */
         if (!pnv_chip_is_power9(chip)) {
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 03/17] hw/misc/macio: Use object_initialize_child for correct ref. counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 01/17] qom/object: Display more helpful message when an object type is missing Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 02/17] hw/ppc/pnv: Use object_initialize_child for correct reference counting Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 04/17] hw/virtio: Use object_initialize_child for correct reference counting Eduardo Habkost
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                           child_type, &error_abort, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                            child_type, errp, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-3-philmd@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/misc/macio/macio.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 94da85c8d7..b726c73022 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -346,12 +346,12 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
         object_property_set_bool(OBJECT(&ns->gpio), true, "realized", &err);
 
         /* PMU */
-        object_initialize(&s->pmu, sizeof(s->pmu), TYPE_VIA_PMU);
+        object_initialize_child(OBJECT(s), "pmu", &s->pmu, sizeof(s->pmu),
+                                TYPE_VIA_PMU, &error_abort, NULL);
         object_property_set_link(OBJECT(&s->pmu), OBJECT(sysbus_dev), "gpio",
                                  &error_abort);
         qdev_prop_set_bit(DEVICE(&s->pmu), "has-adb", ns->has_adb);
         qdev_set_parent_bus(DEVICE(&s->pmu), BUS(&s->macio_bus));
-        object_property_add_child(OBJECT(s), "pmu", OBJECT(&s->pmu), NULL);
 
         object_property_set_bool(OBJECT(&s->pmu), true, "realized", &err);
         if (err) {
@@ -365,9 +365,9 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
                                     sysbus_mmio_get_region(sysbus_dev, 0));
     } else {
         /* CUDA */
-        object_initialize(&s->cuda, sizeof(s->cuda), TYPE_CUDA);
+        object_initialize_child(OBJECT(s), "cuda", &s->cuda, sizeof(s->cuda),
+                                TYPE_CUDA, &error_abort, NULL);
         qdev_set_parent_bus(DEVICE(&s->cuda), BUS(&s->macio_bus));
-        object_property_add_child(OBJECT(s), "cuda", OBJECT(&s->cuda), NULL);
         qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency",
                              s->frequency);
 
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 04/17] hw/virtio: Use object_initialize_child for correct reference counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (2 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 03/17] hw/misc/macio: Use object_initialize_child for correct ref. counting Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 05/17] hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string Eduardo Habkost
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script:

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                           child_type, &error_abort, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                            child_type, errp, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-4-philmd@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/virtio/virtio.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 4805727b53..07f4a64b48 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2312,9 +2312,8 @@ void virtio_instance_init_common(Object *proxy_obj, void *data,
 {
     DeviceState *vdev = data;
 
-    object_initialize(vdev, vdev_size, vdev_name);
-    object_property_add_child(proxy_obj, "virtio-backend", OBJECT(vdev), NULL);
-    object_unref(OBJECT(vdev));
+    object_initialize_child(proxy_obj, "virtio-backend", vdev, vdev_size,
+                            vdev_name, &error_abort, NULL);
     qdev_alias_all_properties(vdev, proxy_obj);
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 05/17] hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (3 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 04/17] hw/virtio: Use object_initialize_child for correct reference counting Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 06/17] hw/arm/bcm2835: Use object_initialize() on PL011State Eduardo Habkost
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-5-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/arm/bcm2835_peripherals.h | 1 +
 hw/arm/bcm2835_peripherals.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h
index f5b193f670..959508d57d 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -13,6 +13,7 @@
 
 #include "qemu-common.h"
 #include "hw/sysbus.h"
+#include "hw/char/pl011.h"
 #include "hw/char/bcm2835_aux.h"
 #include "hw/display/bcm2835_fb.h"
 #include "hw/dma/bcm2835_dma.h"
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 6be7660e8c..7ffb51b692 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -46,7 +46,7 @@ static void bcm2835_peripherals_init(Object *obj)
     qdev_set_parent_bus(DEVICE(&s->ic), sysbus_get_default());
 
     /* UART0 */
-    s->uart0 = SYS_BUS_DEVICE(object_new("pl011"));
+    s->uart0 = SYS_BUS_DEVICE(object_new(TYPE_PL011));
     object_property_add_child(obj, "uart0", OBJECT(s->uart0), NULL);
     qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
 
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 06/17] hw/arm/bcm2835: Use object_initialize() on PL011State
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (4 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 05/17] hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 07/17] hw/arm/bcm2835: Use object_initialize_child for correct ref. counting Eduardo Habkost
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

To be coherent with the other peripherals contained in the
BCM2835PeripheralState structure, directly allocate the PL011State
(instead of using the pl011 uart as a pointer to a SysBusDevice).

Initialize the PL011State with object_initialize() instead of
object_new().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-6-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/arm/bcm2835_peripherals.h |  2 +-
 hw/arm/bcm2835_peripherals.c         | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/hw/arm/bcm2835_peripherals.h b/include/hw/arm/bcm2835_peripherals.h
index 959508d57d..e79c21771f 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -38,7 +38,7 @@ typedef struct BCM2835PeripheralState {
     MemoryRegion ram_alias[4];
     qemu_irq irq, fiq;
 
-    SysBusDevice *uart0;
+    PL011State uart0;
     BCM2835AuxState aux;
     BCM2835FBState fb;
     BCM2835DMAState dma;
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 7ffb51b692..2931a82a25 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -46,9 +46,9 @@ static void bcm2835_peripherals_init(Object *obj)
     qdev_set_parent_bus(DEVICE(&s->ic), sysbus_get_default());
 
     /* UART0 */
-    s->uart0 = SYS_BUS_DEVICE(object_new(TYPE_PL011));
-    object_property_add_child(obj, "uart0", OBJECT(s->uart0), NULL);
-    qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
+    object_initialize(&s->uart0, sizeof(s->uart0), TYPE_PL011);
+    object_property_add_child(obj, "uart0", OBJECT(&s->uart0), NULL);
+    qdev_set_parent_bus(DEVICE(&s->uart0), sysbus_get_default());
 
     /* AUX / UART1 */
     object_initialize(&s->aux, sizeof(s->aux), TYPE_BCM2835_AUX);
@@ -166,16 +166,16 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
     sysbus_pass_irq(SYS_BUS_DEVICE(s), SYS_BUS_DEVICE(&s->ic));
 
     /* UART0 */
-    qdev_prop_set_chr(DEVICE(s->uart0), "chardev", serial_hd(0));
-    object_property_set_bool(OBJECT(s->uart0), true, "realized", &err);
+    qdev_prop_set_chr(DEVICE(&s->uart0), "chardev", serial_hd(0));
+    object_property_set_bool(OBJECT(&s->uart0), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
         return;
     }
 
     memory_region_add_subregion(&s->peri_mr, UART0_OFFSET,
-                                sysbus_mmio_get_region(s->uart0, 0));
-    sysbus_connect_irq(s->uart0, 0,
+                sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->uart0), 0));
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart0), 0,
         qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
                                INTERRUPT_UART));
     /* AUX / UART1 */
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 07/17] hw/arm/bcm2835: Use object_initialize_child for correct ref. counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (5 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 06/17] hw/arm/bcm2835: Use object_initialize() on PL011State Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 08/17] hw/arm/aspeed: " Eduardo Habkost
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                           child_type, &error_abort, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                            child_type, errp, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 )

 @use_sysbus_init_child_obj@
 expression parent_obj;
 expression dev;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -                           child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 +                         child_type);
     ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 |
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -                           child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 +                         child_type);
 -   dev = DEVICE(child_ptr);
 -   qdev_set_parent_bus(dev, sysbus_get_default());
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
                             const char *childname, void *child,
                             size_t childsize, const char *childtype)
  {
      object_initialize_child(parent, childname, child, childsize,
                              childtype, &error_abort, NULL);

      qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-7-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/arm/bcm2835_peripherals.c | 53 ++++++++++++++----------------------
 1 file changed, 20 insertions(+), 33 deletions(-)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 2931a82a25..0fb54c7964 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -41,44 +41,36 @@ static void bcm2835_peripherals_init(Object *obj)
                        MBOX_CHAN_COUNT << MBOX_AS_CHAN_SHIFT);
 
     /* Interrupt Controller */
-    object_initialize(&s->ic, sizeof(s->ic), TYPE_BCM2835_IC);
-    object_property_add_child(obj, "ic", OBJECT(&s->ic), NULL);
-    qdev_set_parent_bus(DEVICE(&s->ic), sysbus_get_default());
+    sysbus_init_child_obj(obj, "ic", &s->ic, sizeof(s->ic), TYPE_BCM2835_IC);
 
     /* UART0 */
-    object_initialize(&s->uart0, sizeof(s->uart0), TYPE_PL011);
-    object_property_add_child(obj, "uart0", OBJECT(&s->uart0), NULL);
-    qdev_set_parent_bus(DEVICE(&s->uart0), sysbus_get_default());
+    sysbus_init_child_obj(obj, "uart0", &s->uart0, sizeof(s->uart0),
+                          TYPE_PL011);
 
     /* AUX / UART1 */
-    object_initialize(&s->aux, sizeof(s->aux), TYPE_BCM2835_AUX);
-    object_property_add_child(obj, "aux", OBJECT(&s->aux), NULL);
-    qdev_set_parent_bus(DEVICE(&s->aux), sysbus_get_default());
+    sysbus_init_child_obj(obj, "aux", &s->aux, sizeof(s->aux),
+                          TYPE_BCM2835_AUX);
 
     /* Mailboxes */
-    object_initialize(&s->mboxes, sizeof(s->mboxes), TYPE_BCM2835_MBOX);
-    object_property_add_child(obj, "mbox", OBJECT(&s->mboxes), NULL);
-    qdev_set_parent_bus(DEVICE(&s->mboxes), sysbus_get_default());
+    sysbus_init_child_obj(obj, "mbox", &s->mboxes, sizeof(s->mboxes),
+                          TYPE_BCM2835_MBOX);
 
     object_property_add_const_link(OBJECT(&s->mboxes), "mbox-mr",
                                    OBJECT(&s->mbox_mr), &error_abort);
 
     /* Framebuffer */
-    object_initialize(&s->fb, sizeof(s->fb), TYPE_BCM2835_FB);
-    object_property_add_child(obj, "fb", OBJECT(&s->fb), NULL);
+    sysbus_init_child_obj(obj, "fb", &s->fb, sizeof(s->fb), TYPE_BCM2835_FB);
     object_property_add_alias(obj, "vcram-size", OBJECT(&s->fb), "vcram-size",
                               &error_abort);
-    qdev_set_parent_bus(DEVICE(&s->fb), sysbus_get_default());
 
     object_property_add_const_link(OBJECT(&s->fb), "dma-mr",
                                    OBJECT(&s->gpu_bus_mr), &error_abort);
 
     /* Property channel */
-    object_initialize(&s->property, sizeof(s->property), TYPE_BCM2835_PROPERTY);
-    object_property_add_child(obj, "property", OBJECT(&s->property), NULL);
+    sysbus_init_child_obj(obj, "property", &s->property, sizeof(s->property),
+                          TYPE_BCM2835_PROPERTY);
     object_property_add_alias(obj, "board-rev", OBJECT(&s->property),
                               "board-rev", &error_abort);
-    qdev_set_parent_bus(DEVICE(&s->property), sysbus_get_default());
 
     object_property_add_const_link(OBJECT(&s->property), "fb",
                                    OBJECT(&s->fb), &error_abort);
@@ -86,32 +78,27 @@ static void bcm2835_peripherals_init(Object *obj)
                                    OBJECT(&s->gpu_bus_mr), &error_abort);
 
     /* Random Number Generator */
-    object_initialize(&s->rng, sizeof(s->rng), TYPE_BCM2835_RNG);
-    object_property_add_child(obj, "rng", OBJECT(&s->rng), NULL);
-    qdev_set_parent_bus(DEVICE(&s->rng), sysbus_get_default());
+    sysbus_init_child_obj(obj, "rng", &s->rng, sizeof(s->rng),
+                          TYPE_BCM2835_RNG);
 
     /* Extended Mass Media Controller */
-    object_initialize(&s->sdhci, sizeof(s->sdhci), TYPE_SYSBUS_SDHCI);
-    object_property_add_child(obj, "sdhci", OBJECT(&s->sdhci), NULL);
-    qdev_set_parent_bus(DEVICE(&s->sdhci), sysbus_get_default());
+    sysbus_init_child_obj(obj, "sdhci", &s->sdhci, sizeof(s->sdhci),
+                          TYPE_SYSBUS_SDHCI);
 
     /* SDHOST */
-    object_initialize(&s->sdhost, sizeof(s->sdhost), TYPE_BCM2835_SDHOST);
-    object_property_add_child(obj, "sdhost", OBJECT(&s->sdhost), NULL);
-    qdev_set_parent_bus(DEVICE(&s->sdhost), sysbus_get_default());
+    sysbus_init_child_obj(obj, "sdhost", &s->sdhost, sizeof(s->sdhost),
+                          TYPE_BCM2835_SDHOST);
 
     /* DMA Channels */
-    object_initialize(&s->dma, sizeof(s->dma), TYPE_BCM2835_DMA);
-    object_property_add_child(obj, "dma", OBJECT(&s->dma), NULL);
-    qdev_set_parent_bus(DEVICE(&s->dma), sysbus_get_default());
+    sysbus_init_child_obj(obj, "dma", &s->dma, sizeof(s->dma),
+                          TYPE_BCM2835_DMA);
 
     object_property_add_const_link(OBJECT(&s->dma), "dma-mr",
                                    OBJECT(&s->gpu_bus_mr), &error_abort);
 
     /* GPIO */
-    object_initialize(&s->gpio, sizeof(s->gpio), TYPE_BCM2835_GPIO);
-    object_property_add_child(obj, "gpio", OBJECT(&s->gpio), NULL);
-    qdev_set_parent_bus(DEVICE(&s->gpio), sysbus_get_default());
+    sysbus_init_child_obj(obj, "gpio", &s->gpio, sizeof(s->gpio),
+                          TYPE_BCM2835_GPIO);
 
     object_property_add_const_link(OBJECT(&s->gpio), "sdbus-sdhci",
                                    OBJECT(&s->sdhci.sdbus), &error_abort);
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 08/17] hw/arm/aspeed: Use object_initialize_child for correct ref. counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (6 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 07/17] hw/arm/bcm2835: Use object_initialize_child for correct ref. counting Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 09/17] hw/arm: Use object_initialize_child for correct reference counting Eduardo Habkost
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                           child_type, &error_abort, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                            child_type, errp, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 )

 @use_sysbus_init_child_obj@
 expression parent_obj;
 expression dev;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -                           child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 +                         child_type);
     ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 |
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -                           child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 +                         child_type);
 -   dev = DEVICE(child_ptr);
 -   qdev_set_parent_bus(dev, sysbus_get_default());
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
                             const char *childname, void *child,
                             size_t childsize, const char *childtype)
  {
      object_initialize_child(parent, childname, child, childsize,
                              childtype, &error_abort, NULL);

      qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Message-Id: <20190507163416.24647-8-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/arm/aspeed.c     |  6 +++---
 hw/arm/aspeed_soc.c | 50 ++++++++++++++++++---------------------------
 2 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 415cff7a01..33070a6df8 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -160,9 +160,9 @@ static void aspeed_board_init(MachineState *machine,
     ram_addr_t max_ram_size;
 
     bmc = g_new0(AspeedBoardState, 1);
-    object_initialize(&bmc->soc, (sizeof(bmc->soc)), cfg->soc_name);
-    object_property_add_child(OBJECT(machine), "soc", OBJECT(&bmc->soc),
-                              &error_abort);
+    object_initialize_child(OBJECT(machine), "soc", &bmc->soc,
+                            (sizeof(bmc->soc)), cfg->soc_name, &error_abort,
+                            NULL);
 
     sc = ASPEED_SOC_GET_CLASS(&bmc->soc);
 
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index a27233d487..faff42b84a 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -106,12 +106,11 @@ static void aspeed_soc_init(Object *obj)
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     int i;
 
-    object_initialize(&s->cpu, sizeof(s->cpu), sc->info->cpu_type);
-    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+    object_initialize_child(obj, "cpu", OBJECT(&s->cpu), sizeof(s->cpu),
+                            sc->info->cpu_type, &error_abort, NULL);
 
-    object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
-    object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
-    qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
+    sysbus_init_child_obj(obj, "scu", OBJECT(&s->scu), sizeof(s->scu),
+                          TYPE_ASPEED_SCU);
     qdev_prop_set_uint32(DEVICE(&s->scu), "silicon-rev",
                          sc->info->silicon_rev);
     object_property_add_alias(obj, "hw-strap1", OBJECT(&s->scu),
@@ -121,36 +120,29 @@ static void aspeed_soc_init(Object *obj)
     object_property_add_alias(obj, "hw-prot-key", OBJECT(&s->scu),
                               "hw-prot-key", &error_abort);
 
-    object_initialize(&s->vic, sizeof(s->vic), TYPE_ASPEED_VIC);
-    object_property_add_child(obj, "vic", OBJECT(&s->vic), NULL);
-    qdev_set_parent_bus(DEVICE(&s->vic), sysbus_get_default());
+    sysbus_init_child_obj(obj, "vic", OBJECT(&s->vic), sizeof(s->vic),
+                          TYPE_ASPEED_VIC);
 
-    object_initialize(&s->timerctrl, sizeof(s->timerctrl), TYPE_ASPEED_TIMER);
-    object_property_add_child(obj, "timerctrl", OBJECT(&s->timerctrl), NULL);
+    sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
+                          sizeof(s->timerctrl), TYPE_ASPEED_TIMER);
     object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
                                    OBJECT(&s->scu), &error_abort);
-    qdev_set_parent_bus(DEVICE(&s->timerctrl), sysbus_get_default());
 
-    object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
-    object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
-    qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
+    sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
+                          TYPE_ASPEED_I2C);
 
-    object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename);
-    object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
-    qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default());
+    sysbus_init_child_obj(obj, "fmc", OBJECT(&s->fmc), sizeof(s->fmc),
+                          sc->info->fmc_typename);
     object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs",
                               &error_abort);
 
     for (i = 0; i < sc->info->spis_num; i++) {
-        object_initialize(&s->spi[i], sizeof(s->spi[i]),
-                          sc->info->spi_typename[i]);
-        object_property_add_child(obj, "spi[*]", OBJECT(&s->spi[i]), NULL);
-        qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
+        sysbus_init_child_obj(obj, "spi[*]", OBJECT(&s->spi[i]),
+                              sizeof(s->spi[i]), sc->info->spi_typename[i]);
     }
 
-    object_initialize(&s->sdmc, sizeof(s->sdmc), TYPE_ASPEED_SDMC);
-    object_property_add_child(obj, "sdmc", OBJECT(&s->sdmc), NULL);
-    qdev_set_parent_bus(DEVICE(&s->sdmc), sysbus_get_default());
+    sysbus_init_child_obj(obj, "sdmc", OBJECT(&s->sdmc), sizeof(s->sdmc),
+                          TYPE_ASPEED_SDMC);
     qdev_prop_set_uint32(DEVICE(&s->sdmc), "silicon-rev",
                          sc->info->silicon_rev);
     object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc),
@@ -159,16 +151,14 @@ static void aspeed_soc_init(Object *obj)
                               "max-ram-size", &error_abort);
 
     for (i = 0; i < sc->info->wdts_num; i++) {
-        object_initialize(&s->wdt[i], sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
-        object_property_add_child(obj, "wdt[*]", OBJECT(&s->wdt[i]), NULL);
-        qdev_set_parent_bus(DEVICE(&s->wdt[i]), sysbus_get_default());
+        sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
+                              sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
         qdev_prop_set_uint32(DEVICE(&s->wdt[i]), "silicon-rev",
                                     sc->info->silicon_rev);
     }
 
-    object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100);
-    object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL);
-    qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default());
+    sysbus_init_child_obj(obj, "ftgmac100", OBJECT(&s->ftgmac100),
+                          sizeof(s->ftgmac100), TYPE_FTGMAC100);
 }
 
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 09/17] hw/arm: Use object_initialize_child for correct reference counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (7 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 08/17] hw/arm/aspeed: " Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 10/17] hw/mips: Use object_initialize() on MIPSCPSState Eduardo Habkost
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                           child_type, &error_abort, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                            child_type, errp, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
     ...
 ?-  object_unref(OBJECT(child_ptr));
 )

 @use_sysbus_init_child_obj@
 expression parent_obj;
 expression dev;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -                           child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 +                         child_type);
     ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 |
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -                           child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 +                         child_type);
 -   dev = DEVICE(child_ptr);
 -   qdev_set_parent_bus(dev, sysbus_get_default());
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
                             const char *childname, void *child,
                             size_t childsize, const char *childtype)
  {
      object_initialize_child(parent, childname, child, childsize,
                              childtype, &error_abort, NULL);

      qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-9-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/arm/digic.c       | 17 ++++++-----------
 hw/arm/imx25_pdk.c   |  5 ++---
 hw/arm/kzm.c         |  5 ++---
 hw/arm/raspi.c       |  7 +++----
 hw/arm/sabrelite.c   |  5 ++---
 hw/arm/xlnx-zcu102.c |  5 ++---
 hw/arm/xlnx-zynqmp.c |  8 ++++----
 7 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index 726abb9b48..6ef26c6bac 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -32,27 +32,22 @@
 static void digic_init(Object *obj)
 {
     DigicState *s = DIGIC(obj);
-    DeviceState *dev;
     int i;
 
-    object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
-    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
+                            "arm946-" TYPE_ARM_CPU, &error_abort, NULL);
 
     for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
 #define DIGIC_TIMER_NAME_MLEN    11
         char name[DIGIC_TIMER_NAME_MLEN];
 
-        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
-        dev = DEVICE(&s->timer[i]);
-        qdev_set_parent_bus(dev, sysbus_get_default());
         snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
-        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
+        sysbus_init_child_obj(obj, name, &s->timer[i], sizeof(s->timer[i]),
+                              TYPE_DIGIC_TIMER);
     }
 
-    object_initialize(&s->uart, sizeof(s->uart), TYPE_DIGIC_UART);
-    dev = DEVICE(&s->uart);
-    qdev_set_parent_bus(dev, sysbus_get_default());
-    object_property_add_child(obj, "uart", OBJECT(&s->uart), NULL);
+    sysbus_init_child_obj(obj, "uart", &s->uart, sizeof(s->uart),
+                          TYPE_DIGIC_UART);
 }
 
 static void digic_realize(DeviceState *dev, Error **errp)
diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
index 9f3ee14739..eef1b184b0 100644
--- a/hw/arm/imx25_pdk.c
+++ b/hw/arm/imx25_pdk.c
@@ -72,9 +72,8 @@ static void imx25_pdk_init(MachineState *machine)
     unsigned int alias_offset;
     int i;
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_FSL_IMX25);
-    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
-                              &error_abort);
+    object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
+                            TYPE_FSL_IMX25, &error_abort, NULL);
 
     object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
 
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 139934c4ec..44cba8782b 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -71,9 +71,8 @@ static void kzm_init(MachineState *machine)
     unsigned int alias_offset;
     unsigned int i;
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_FSL_IMX31);
-    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
-                              &error_abort);
+    object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
+                            TYPE_FSL_IMX31, &error_abort, NULL);
 
     object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
 
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 2b5fe10e2f..8c249fcabb 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -182,10 +182,9 @@ static void raspi_init(MachineState *machine, int version)
         exit(1);
     }
 
-    object_initialize(&s->soc, sizeof(s->soc),
-                      version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
-    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
-                              &error_abort);
+    object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
+                            version == 3 ? TYPE_BCM2837 : TYPE_BCM2836,
+                            &error_abort, NULL);
 
     /* Allocate and map RAM */
     memory_region_allocate_system_memory(&s->ram, OBJECT(machine), "ram",
diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
index ee140e5d9e..f1b00de229 100644
--- a/hw/arm/sabrelite.c
+++ b/hw/arm/sabrelite.c
@@ -55,9 +55,8 @@ static void sabrelite_init(MachineState *machine)
         exit(1);
     }
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_FSL_IMX6);
-    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
-                              &error_abort);
+    object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
+                            TYPE_FSL_IMX6, &error_abort, NULL);
 
     object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
     if (err != NULL) {
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index b6bc6a93b8..c802f26fbd 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -91,9 +91,8 @@ static void xlnx_zcu102_init(MachineState *machine)
     memory_region_allocate_system_memory(&s->ddr_ram, NULL, "ddr-ram",
                                          ram_size);
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
-    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
-                              &error_abort);
+    object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
+                            TYPE_XLNX_ZYNQMP, &error_abort, NULL);
 
     object_property_set_link(OBJECT(&s->soc), OBJECT(&s->ddr_ram),
                          "ddr-ram", &error_abort);
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 4f8bc41d9d..6e99190302 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -191,10 +191,10 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
     for (i = 0; i < num_rpus; i++) {
         char *name;
 
-        object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
-                          "cortex-r5f-" TYPE_ARM_CPU);
-        object_property_add_child(OBJECT(&s->rpu_cluster), "rpu-cpu[*]",
-                                  OBJECT(&s->rpu_cpu[i]), &error_abort);
+        object_initialize_child(OBJECT(&s->rpu_cluster), "rpu-cpu[*]",
+                                &s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
+                                "cortex-r5f-" TYPE_ARM_CPU, &error_abort,
+                                NULL);
 
         name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
         if (strcmp(name, boot_cpu)) {
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 10/17] hw/mips: Use object_initialize() on MIPSCPSState
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (8 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 09/17] hw/arm: Use object_initialize_child for correct reference counting Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 11/17] hw/mips: Use object_initialize_child for correct reference counting Eduardo Habkost
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Initialize the MIPSCPSState with object_initialize() instead of
object_new(). This will allow us to add it as children of the
machine container.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-10-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/mips/boston.c     | 25 ++++++++++++-------------
 hw/mips/mips_malta.c | 17 ++++++++---------
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index a8b29f62f5..cb3ea85fdc 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -49,7 +49,7 @@ typedef struct {
     SysBusDevice parent_obj;
 
     MachineState *mach;
-    MIPSCPSState *cps;
+    MIPSCPSState cps;
     SerialState *uart;
 
     CharBackend lcd_display;
@@ -188,7 +188,7 @@ static uint64_t boston_platreg_read(void *opaque, hwaddr addr,
     case PLAT_DDR3_STATUS:
         return PLAT_DDR3_STATUS_LOCKED | PLAT_DDR3_STATUS_CALIBRATED;
     case PLAT_MMCM_DIV:
-        gic_freq = mips_gictimer_get_freq(s->cps->gic.gic_timer) / 1000000;
+        gic_freq = mips_gictimer_get_freq(s->cps.gic.gic_timer) / 1000000;
         val = gic_freq << PLAT_MMCM_DIV_INPUT_SHIFT;
         val |= 1 << PLAT_MMCM_DIV_MUL_SHIFT;
         val |= 1 << PLAT_MMCM_DIV_CLK0DIV_SHIFT;
@@ -455,20 +455,19 @@ static void boston_mach_init(MachineState *machine)
 
     is_64b = cpu_supports_isa(machine->cpu_type, ISA_MIPS64);
 
-    s->cps = MIPS_CPS(object_new(TYPE_MIPS_CPS));
-    qdev_set_parent_bus(DEVICE(s->cps), sysbus_get_default());
-
-    object_property_set_str(OBJECT(s->cps), machine->cpu_type, "cpu-type",
+    object_initialize(&s->cps, sizeof(s->cps), TYPE_MIPS_CPS);
+    qdev_set_parent_bus(DEVICE(&s->cps), sysbus_get_default());
+    object_property_set_str(OBJECT(&s->cps), machine->cpu_type, "cpu-type",
                             &err);
-    object_property_set_int(OBJECT(s->cps), smp_cpus, "num-vp", &err);
-    object_property_set_bool(OBJECT(s->cps), true, "realized", &err);
+    object_property_set_int(OBJECT(&s->cps), smp_cpus, "num-vp", &err);
+    object_property_set_bool(OBJECT(&s->cps), true, "realized", &err);
 
     if (err != NULL) {
         error_report("%s", error_get_pretty(err));
         exit(1);
     }
 
-    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->cps), 0, 0, 1);
+    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
 
     flash =  g_new(MemoryRegion, 1);
     memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB, &err);
@@ -487,17 +486,17 @@ static void boston_mach_init(MachineState *machine)
     xilinx_pcie_init(sys_mem, 0,
                      0x10000000, 32 * MiB,
                      0x40000000, 1 * GiB,
-                     get_cps_irq(s->cps, 2), false);
+                     get_cps_irq(&s->cps, 2), false);
 
     xilinx_pcie_init(sys_mem, 1,
                      0x12000000, 32 * MiB,
                      0x20000000, 512 * MiB,
-                     get_cps_irq(s->cps, 1), false);
+                     get_cps_irq(&s->cps, 1), false);
 
     pcie2 = xilinx_pcie_init(sys_mem, 2,
                              0x14000000, 32 * MiB,
                              0x16000000, 1 * MiB,
-                             get_cps_irq(s->cps, 0), true);
+                             get_cps_irq(&s->cps, 0), true);
 
     platreg = g_new(MemoryRegion, 1);
     memory_region_init_io(platreg, NULL, &boston_platreg_ops, s,
@@ -505,7 +504,7 @@ static void boston_mach_init(MachineState *machine)
     memory_region_add_subregion_overlap(sys_mem, 0x17ffd000, platreg, 0);
 
     s->uart = serial_mm_init(sys_mem, 0x17ffe000, 2,
-                             get_cps_irq(s->cps, 3), 10000000,
+                             get_cps_irq(&s->cps, 3), 10000000,
                              serial_hd(0), DEVICE_NATIVE_ENDIAN);
 
     lcd = g_new(MemoryRegion, 1);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 439665ab45..04f2117d71 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -94,7 +94,7 @@ typedef struct {
 typedef struct {
     SysBusDevice parent_obj;
 
-    MIPSCPSState *cps;
+    MIPSCPSState cps;
     qemu_irq *i8259;
 } MaltaState;
 
@@ -1151,20 +1151,19 @@ static void create_cps(MaltaState *s, const char *cpu_type,
 {
     Error *err = NULL;
 
-    s->cps = MIPS_CPS(object_new(TYPE_MIPS_CPS));
-    qdev_set_parent_bus(DEVICE(s->cps), sysbus_get_default());
-
-    object_property_set_str(OBJECT(s->cps), cpu_type, "cpu-type", &err);
-    object_property_set_int(OBJECT(s->cps), smp_cpus, "num-vp", &err);
-    object_property_set_bool(OBJECT(s->cps), true, "realized", &err);
+    object_initialize(&s->cps, sizeof(s->cps), TYPE_MIPS_CPS);
+    qdev_set_parent_bus(DEVICE(&s->cps), sysbus_get_default());
+    object_property_set_str(OBJECT(&s->cps), cpu_type, "cpu-type", &err);
+    object_property_set_int(OBJECT(&s->cps), smp_cpus, "num-vp", &err);
+    object_property_set_bool(OBJECT(&s->cps), true, "realized", &err);
     if (err != NULL) {
         error_report("%s", error_get_pretty(err));
         exit(1);
     }
 
-    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->cps), 0, 0, 1);
+    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
 
-    *i8259_irq = get_cps_irq(s->cps, 3);
+    *i8259_irq = get_cps_irq(&s->cps, 3);
     *cbus_irq = NULL;
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 11/17] hw/mips: Use object_initialize_child for correct reference counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (9 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 10/17] hw/mips: Use object_initialize() on MIPSCPSState Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 12/17] hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state Eduardo Habkost
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script:

 @use_sysbus_init_child_obj_missing_parent@
 expression child_ptr;
 expression child_type;
 expression child_size;
 @@
 -   object_initialize(child_ptr, child_size, child_type);
     ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
     ...
 ?-  object_unref(OBJECT(child_ptr));
 +   sysbus_init_child_obj(OBJECT(PARENT_OBJ), "CHILD_NAME", child_ptr,
 +                         child_size, child_type);

We let the Malta/Boston machines adopt the CPS child, and similarly
the CPS adopts the ITU/CPC/GIC/GCR children.

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
                             const char *childname, void *child,
                             size_t childsize, const char *childtype)
  {
      object_initialize_child(parent, childname, child, childsize,
                              childtype, &error_abort, NULL);

      qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-11-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/mips/boston.c     |  4 ++--
 hw/mips/cps.c        | 20 ++++++++------------
 hw/mips/mips_malta.c |  4 ++--
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index cb3ea85fdc..1ffccc8da9 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -455,8 +455,8 @@ static void boston_mach_init(MachineState *machine)
 
     is_64b = cpu_supports_isa(machine->cpu_type, ISA_MIPS64);
 
-    object_initialize(&s->cps, sizeof(s->cps), TYPE_MIPS_CPS);
-    qdev_set_parent_bus(DEVICE(&s->cps), sysbus_get_default());
+    sysbus_init_child_obj(OBJECT(machine), "cps", OBJECT(&s->cps),
+                          sizeof(s->cps), TYPE_MIPS_CPS);
     object_property_set_str(OBJECT(&s->cps), machine->cpu_type, "cpu-type",
                             &err);
     object_property_set_int(OBJECT(&s->cps), smp_cpus, "num-vp", &err);
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index fc97f59af4..649b35a76c 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -94,9 +94,8 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 
     /* Inter-Thread Communication Unit */
     if (itu_present) {
-        object_initialize(&s->itu, sizeof(s->itu), TYPE_MIPS_ITU);
-        qdev_set_parent_bus(DEVICE(&s->itu), sysbus_get_default());
-
+        sysbus_init_child_obj(OBJECT(dev), "itu", &s->itu, sizeof(s->itu),
+                              TYPE_MIPS_ITU);
         object_property_set_int(OBJECT(&s->itu), 16, "num-fifo", &err);
         object_property_set_int(OBJECT(&s->itu), 16, "num-semaphores", &err);
         object_property_set_bool(OBJECT(&s->itu), saar_present, "saar-present",
@@ -115,9 +114,8 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
     }
 
     /* Cluster Power Controller */
-    object_initialize(&s->cpc, sizeof(s->cpc), TYPE_MIPS_CPC);
-    qdev_set_parent_bus(DEVICE(&s->cpc), sysbus_get_default());
-
+    sysbus_init_child_obj(OBJECT(dev), "cpc", &s->cpc, sizeof(s->cpc),
+                          TYPE_MIPS_CPC);
     object_property_set_int(OBJECT(&s->cpc), s->num_vp, "num-vp", &err);
     object_property_set_int(OBJECT(&s->cpc), 1, "vp-start-running", &err);
     object_property_set_bool(OBJECT(&s->cpc), true, "realized", &err);
@@ -130,9 +128,8 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
                             sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->cpc), 0));
 
     /* Global Interrupt Controller */
-    object_initialize(&s->gic, sizeof(s->gic), TYPE_MIPS_GIC);
-    qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
-
+    sysbus_init_child_obj(OBJECT(dev), "gic", &s->gic, sizeof(s->gic),
+                          TYPE_MIPS_GIC);
     object_property_set_int(OBJECT(&s->gic), s->num_vp, "num-vp", &err);
     object_property_set_int(OBJECT(&s->gic), 128, "num-irq", &err);
     object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
@@ -147,9 +144,8 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
     /* Global Configuration Registers */
     gcr_base = env->CP0_CMGCRBase << 4;
 
-    object_initialize(&s->gcr, sizeof(s->gcr), TYPE_MIPS_GCR);
-    qdev_set_parent_bus(DEVICE(&s->gcr), sysbus_get_default());
-
+    sysbus_init_child_obj(OBJECT(dev), "gcr", &s->gcr, sizeof(s->gcr),
+                          TYPE_MIPS_GCR);
     object_property_set_int(OBJECT(&s->gcr), s->num_vp, "num-vp", &err);
     object_property_set_int(OBJECT(&s->gcr), 0x800, "gcr-rev", &err);
     object_property_set_int(OBJECT(&s->gcr), gcr_base, "gcr-base", &err);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 04f2117d71..aff8464f2a 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1151,8 +1151,8 @@ static void create_cps(MaltaState *s, const char *cpu_type,
 {
     Error *err = NULL;
 
-    object_initialize(&s->cps, sizeof(s->cps), TYPE_MIPS_CPS);
-    qdev_set_parent_bus(DEVICE(&s->cps), sysbus_get_default());
+    sysbus_init_child_obj(OBJECT(s), "cps", OBJECT(&s->cps), sizeof(s->cps),
+                          TYPE_MIPS_CPS);
     object_property_set_str(OBJECT(&s->cps), cpu_type, "cpu-type", &err);
     object_property_set_int(OBJECT(&s->cps), smp_cpus, "num-vp", &err);
     object_property_set_bool(OBJECT(&s->cps), true, "realized", &err);
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 12/17] hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (10 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 11/17] hw/mips: Use object_initialize_child for correct reference counting Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 13/17] hw/microblaze/zynqmp: Let the SoC manage the IPI devices Eduardo Habkost
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The Inter Processor Interrupt is a block part of the SoC, not the
"machine" (talking about machine is borderline with the PMU, since
it is embedded into the ZynqMP SoC, but currentl QEMU doesn't
support multi-arch cores).

Move the IPI state to the SoC state, this will simplify the review
of the next patch.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-12-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/microblaze/xlnx-zynqmp-pmu.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index 57dc1ccd42..eba9945c19 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -55,6 +55,7 @@ typedef struct XlnxZynqMPPMUSoCState {
     /*< public >*/
     MicroBlazeCPU cpu;
     XlnxPMUIOIntc intc;
+    XlnxZynqMPIPI ipi[XLNX_ZYNQMP_PMU_NUM_IPIS];
 }  XlnxZynqMPPMUSoCState;
 
 
@@ -144,7 +145,6 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *pmu_rom = g_new(MemoryRegion, 1);
     MemoryRegion *pmu_ram = g_new(MemoryRegion, 1);
-    XlnxZynqMPIPI *ipi[XLNX_ZYNQMP_PMU_NUM_IPIS];
     qemu_irq irq[32];
     int i;
 
@@ -172,16 +172,16 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
 
     /* Create and connect the IPI device */
     for (i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-        ipi[i] = g_new0(XlnxZynqMPIPI, 1);
-        object_initialize(ipi[i], sizeof(XlnxZynqMPIPI), TYPE_XLNX_ZYNQMP_IPI);
-        qdev_set_parent_bus(DEVICE(ipi[i]), sysbus_get_default());
+        object_initialize(&pmu->ipi[i], sizeof(XlnxZynqMPIPI),
+                          TYPE_XLNX_ZYNQMP_IPI);
+        qdev_set_parent_bus(DEVICE(&pmu->ipi[i]), sysbus_get_default());
     }
 
     for (i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-        object_property_set_bool(OBJECT(ipi[i]), true, "realized",
+        object_property_set_bool(OBJECT(&pmu->ipi[i]), true, "realized",
                                  &error_abort);
-        sysbus_mmio_map(SYS_BUS_DEVICE(ipi[i]), 0, ipi_addr[i]);
-        sysbus_connect_irq(SYS_BUS_DEVICE(ipi[i]), 0, irq[ipi_irq[i]]);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&pmu->ipi[i]), 0, ipi_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&pmu->ipi[i]), 0, irq[ipi_irq[i]]);
     }
 
     /* Load the kernel */
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 13/17] hw/microblaze/zynqmp: Let the SoC manage the IPI devices
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (11 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 12/17] hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 14/17] hw/microblaze/zynqmp: Use object_initialize_child for correct ref. counting Eduardo Habkost
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The Inter Processor Interrupt is a block part of the SoC, not the
"machine" (See Zynq UltraScale+ Device TRM UG1085, "Platform
Management Unit", Power Domains and Islands).

Move the IPI management from the machine to the SoC.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-13-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/microblaze/xlnx-zynqmp-pmu.c | 36 +++++++++++++++------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index eba9945c19..20e973edf5 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -68,6 +68,13 @@ static void xlnx_zynqmp_pmu_soc_init(Object *obj)
 
     sysbus_init_child_obj(obj, "intc", &s->intc, sizeof(s->intc),
                           TYPE_XLNX_PMU_IO_INTC);
+
+    /* Create the IPI device */
+    for (int i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
+        object_initialize(&s->ipi[i], sizeof(XlnxZynqMPIPI),
+                          TYPE_XLNX_ZYNQMP_IPI);
+        qdev_set_parent_bus(DEVICE(&s->ipi[i]), sysbus_get_default());
+    }
 }
 
 static void xlnx_zynqmp_pmu_soc_realize(DeviceState *dev, Error **errp)
@@ -113,6 +120,15 @@ static void xlnx_zynqmp_pmu_soc_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->intc), 0, XLNX_ZYNQMP_PMU_INTC_ADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->intc), 0,
                        qdev_get_gpio_in(DEVICE(&s->cpu), MB_CPU_IRQ));
+
+    /* Connect the IPI device */
+    for (int i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
+        object_property_set_bool(OBJECT(&s->ipi[i]), true, "realized",
+                                 &error_abort);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ipi[i]), 0, ipi_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ipi[i]), 0,
+                           qdev_get_gpio_in(DEVICE(&s->intc), ipi_irq[i]));
+    }
 }
 
 static void xlnx_zynqmp_pmu_soc_class_init(ObjectClass *oc, void *data)
@@ -145,8 +161,6 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *pmu_rom = g_new(MemoryRegion, 1);
     MemoryRegion *pmu_ram = g_new(MemoryRegion, 1);
-    qemu_irq irq[32];
-    int i;
 
     /* Create the ROM */
     memory_region_init_rom(pmu_rom, NULL, "xlnx-zynqmp-pmu.rom",
@@ -166,24 +180,6 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
                               &error_abort);
     object_property_set_bool(OBJECT(pmu), true, "realized", &error_fatal);
 
-    for (i = 0; i < 32; i++) {
-        irq[i] = qdev_get_gpio_in(DEVICE(&pmu->intc), i);
-    }
-
-    /* Create and connect the IPI device */
-    for (i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-        object_initialize(&pmu->ipi[i], sizeof(XlnxZynqMPIPI),
-                          TYPE_XLNX_ZYNQMP_IPI);
-        qdev_set_parent_bus(DEVICE(&pmu->ipi[i]), sysbus_get_default());
-    }
-
-    for (i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-        object_property_set_bool(OBJECT(&pmu->ipi[i]), true, "realized",
-                                 &error_abort);
-        sysbus_mmio_map(SYS_BUS_DEVICE(&pmu->ipi[i]), 0, ipi_addr[i]);
-        sysbus_connect_irq(SYS_BUS_DEVICE(&pmu->ipi[i]), 0, irq[ipi_irq[i]]);
-    }
-
     /* Load the kernel */
     microblaze_load_kernel(&pmu->cpu, XLNX_ZYNQMP_PMU_RAM_ADDR,
                            machine->ram_size,
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 14/17] hw/microblaze/zynqmp: Use object_initialize_child for correct ref. counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (12 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 13/17] hw/microblaze/zynqmp: Let the SoC manage the IPI devices Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 15/17] " Eduardo Habkost
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(then manually modified to use numbered IPI name)

 @use_sysbus_init_child_obj_missing_parent@
 expression child_ptr;
 expression child_type;
 expression child_size;
 @@
 -   object_initialize(child_ptr, child_size, child_type);
     ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
     ...
 ?-  object_unref(OBJECT(child_ptr));
 +   sysbus_init_child_obj(OBJECT(PARENT_OBJ), "CHILD_NAME", child_ptr,
 +                         child_size, child_type);

We let the SoC adopt the IPI children.

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
                             const char *childname, void *child,
                             size_t childsize, const char *childtype)
  {
      object_initialize_child(parent, childname, child, childsize,
                              childtype, &error_abort, NULL);

      qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-14-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/microblaze/xlnx-zynqmp-pmu.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index 20e973edf5..0948b1fd5f 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -71,9 +71,10 @@ static void xlnx_zynqmp_pmu_soc_init(Object *obj)
 
     /* Create the IPI device */
     for (int i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-        object_initialize(&s->ipi[i], sizeof(XlnxZynqMPIPI),
-                          TYPE_XLNX_ZYNQMP_IPI);
-        qdev_set_parent_bus(DEVICE(&s->ipi[i]), sysbus_get_default());
+        char *name = g_strdup_printf("ipi%d", i);
+        sysbus_init_child_obj(obj, name, &s->ipi[i],
+                              sizeof(XlnxZynqMPIPI), TYPE_XLNX_ZYNQMP_IPI);
+        g_free(name);
     }
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 15/17] hw/microblaze/zynqmp: Use object_initialize_child for correct ref. counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (13 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 14/17] hw/microblaze/zynqmp: Use object_initialize_child for correct ref. counting Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 16/17] hw/arm/mps2: Use object_initialize_child for correct reference counting Eduardo Habkost
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                           child_type, &error_abort, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
     ...
?-   object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +                            child_type, errp, NULL);
     ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
     ...
?-   object_unref(OBJECT(child_ptr));
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-15-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/microblaze/xlnx-zynqmp-pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index 0948b1fd5f..df6c0048aa 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -176,9 +176,9 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
                                 pmu_ram);
 
     /* Create the PMU device */
-    object_initialize(pmu, sizeof(XlnxZynqMPPMUSoCState), TYPE_XLNX_ZYNQMP_PMU_SOC);
-    object_property_add_child(OBJECT(machine), "pmu", OBJECT(pmu),
-                              &error_abort);
+    object_initialize_child(OBJECT(machine), "pmu", pmu,
+                            sizeof(XlnxZynqMPPMUSoCState),
+                            TYPE_XLNX_ZYNQMP_PMU_SOC, &error_abort, NULL);
     object_property_set_bool(OBJECT(pmu), true, "realized", &error_fatal);
 
     /* Load the kernel */
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 16/17] hw/arm/mps2: Use object_initialize_child for correct reference counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (14 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 15/17] " Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-24 18:44 ` [Qemu-devel] [PULL 17/17] hw/intc/nvic: " Eduardo Habkost
  2019-05-28 10:51 ` [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Peter Maydell
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script:

 @use_sysbus_init_child_obj_missing_parent@
 expression child_ptr;
 expression child_type;
 expression child_size;
 @@
 -   object_initialize(child_ptr, child_size, child_type);
     ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
     ...
 ?-  object_unref(OBJECT(child_ptr));
 +   sysbus_init_child_obj(OBJECT(PARENT_OBJ), "CHILD_NAME", child_ptr,
 +                         child_size, child_type);

We let the MPS2 boards adopt the cpu core, the FPGA and the SCC children.

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
                             const char *childname, void *child,
                             size_t childsize, const char *childtype)
  {
      object_initialize_child(parent, childname, child, childsize,
                              childtype, &error_abort, NULL);

      qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-16-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/arm/mps2-tz.c | 8 ++++----
 hw/arm/mps2.c    | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index c167a5fa59..d85dc2c4bd 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -214,9 +214,9 @@ static MemoryRegion *make_scc(MPS2TZMachineState *mms, void *opaque,
     DeviceState *sccdev;
     MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_GET_CLASS(mms);
 
-    object_initialize(scc, sizeof(mms->scc), TYPE_MPS2_SCC);
+    sysbus_init_child_obj(OBJECT(mms), "scc", scc,
+                          sizeof(mms->scc), TYPE_MPS2_SCC);
     sccdev = DEVICE(scc);
-    qdev_set_parent_bus(sccdev, sysbus_get_default());
     qdev_prop_set_uint32(sccdev, "scc-cfg4", 0x2);
     qdev_prop_set_uint32(sccdev, "scc-aid", 0x00200008);
     qdev_prop_set_uint32(sccdev, "scc-id", mmc->scc_id);
@@ -229,8 +229,8 @@ static MemoryRegion *make_fpgaio(MPS2TZMachineState *mms, void *opaque,
 {
     MPS2FPGAIO *fpgaio = opaque;
 
-    object_initialize(fpgaio, sizeof(mms->fpgaio), TYPE_MPS2_FPGAIO);
-    qdev_set_parent_bus(DEVICE(fpgaio), sysbus_get_default());
+    sysbus_init_child_obj(OBJECT(mms), "fpgaio", fpgaio,
+                          sizeof(mms->fpgaio), TYPE_MPS2_FPGAIO);
     object_property_set_bool(OBJECT(fpgaio), true, "realized", &error_fatal);
     return sysbus_mmio_get_region(SYS_BUS_DEVICE(fpgaio), 0);
 }
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index b74f1378c9..10efff36b2 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -174,9 +174,9 @@ static void mps2_common_init(MachineState *machine)
         g_assert_not_reached();
     }
 
-    object_initialize(&mms->armv7m, sizeof(mms->armv7m), TYPE_ARMV7M);
+    sysbus_init_child_obj(OBJECT(mms), "armv7m", &mms->armv7m,
+                          sizeof(mms->armv7m), TYPE_ARMV7M);
     armv7m = DEVICE(&mms->armv7m);
-    qdev_set_parent_bus(armv7m, sysbus_get_default());
     switch (mmc->fpga_type) {
     case FPGA_AN385:
         qdev_prop_set_uint32(armv7m, "num-irq", 32);
@@ -308,9 +308,9 @@ static void mps2_common_init(MachineState *machine)
                        qdev_get_gpio_in(armv7m, 10));
     sysbus_mmio_map(SYS_BUS_DEVICE(&mms->dualtimer), 0, 0x40002000);
 
-    object_initialize(&mms->scc, sizeof(mms->scc), TYPE_MPS2_SCC);
+    sysbus_init_child_obj(OBJECT(mms), "scc", &mms->scc,
+                          sizeof(mms->scc), TYPE_MPS2_SCC);
     sccdev = DEVICE(&mms->scc);
-    qdev_set_parent_bus(sccdev, sysbus_get_default());
     qdev_prop_set_uint32(sccdev, "scc-cfg4", 0x2);
     qdev_prop_set_uint32(sccdev, "scc-aid", 0x00200008);
     qdev_prop_set_uint32(sccdev, "scc-id", mmc->scc_id);
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 17/17] hw/intc/nvic: Use object_initialize_child for correct reference counting
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (15 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 16/17] hw/arm/mps2: Use object_initialize_child for correct reference counting Eduardo Habkost
@ 2019-05-24 18:44 ` Eduardo Habkost
  2019-05-28 10:51 ` [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Peter Maydell
  17 siblings, 0 replies; 19+ messages in thread
From: Eduardo Habkost @ 2019-05-24 18:44 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, Marcel Apfelbaum; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script:

 @use_sysbus_init_child_obj_missing_parent@
 expression child_ptr;
 expression child_type;
 expression child_size;
 @@
 -   object_initialize(child_ptr, child_size, child_type);
     ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
     ...
 ?-  object_unref(OBJECT(child_ptr));
 +   sysbus_init_child_obj(OBJECT(PARENT_OBJ), "CHILD_NAME", child_ptr,
 +                         child_size, child_type);

We let NVIC adopt the SysTick timer.

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the &error_abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
                             const char *childname, void *child,
                             size_t childsize, const char *childtype)
  {
      object_initialize_child(parent, childname, child, childsize,
                              childtype, &error_abort, NULL);

      qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190507163416.24647-17-philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/intc/armv7m_nvic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 815e720cfa..dc2c206d9a 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -2595,9 +2595,9 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
          * as we didn't know then if the CPU had the security extensions;
          * so we have to do it here.
          */
-        object_initialize(&s->systick[M_REG_S], sizeof(s->systick[M_REG_S]),
-                          TYPE_SYSTICK);
-        qdev_set_parent_bus(DEVICE(&s->systick[M_REG_S]), sysbus_get_default());
+        sysbus_init_child_obj(OBJECT(dev), "systick-reg-s",
+                              &s->systick[M_REG_S],
+                              sizeof(s->systick[M_REG_S]), TYPE_SYSTICK);
 
         object_property_set_bool(OBJECT(&s->systick[M_REG_S]), true,
                                  "realized", &err);
-- 
2.18.0.rc1.1.g3f1ff2140



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24
  2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
                   ` (16 preceding siblings ...)
  2019-05-24 18:44 ` [Qemu-devel] [PULL 17/17] hw/intc/nvic: " Eduardo Habkost
@ 2019-05-28 10:51 ` Peter Maydell
  17 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2019-05-28 10:51 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: QEMU Developers

On Fri, 24 May 2019 at 19:44, Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> The following changes since commit a7b21f6762a2d6ec08106d8a7ccb11829914523f:
>
>   Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-4.1-pull-request' into staging (2019-05-24 12:47:49 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
>
> for you to fetch changes up to 23d1f360f3de1d968d98ba605bd3b718f5309e6f:
>
>   hw/intc/nvic: Use object_initialize_child for correct reference counting (2019-05-24 15:29:02 -0300)
>
> ----------------------------------------------------------------
> Machine Core queue, 2019-05-24
>
> * Display more helpful message when an object type is missing
>   (Philippe Mathieu-Daudé)
> * Use object_initialize_child for correct reference counting
>   (Philippe Mathieu-Daudé)
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-05-28 10:53 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-24 18:44 [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 01/17] qom/object: Display more helpful message when an object type is missing Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 02/17] hw/ppc/pnv: Use object_initialize_child for correct reference counting Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 03/17] hw/misc/macio: Use object_initialize_child for correct ref. counting Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 04/17] hw/virtio: Use object_initialize_child for correct reference counting Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 05/17] hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 06/17] hw/arm/bcm2835: Use object_initialize() on PL011State Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 07/17] hw/arm/bcm2835: Use object_initialize_child for correct ref. counting Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 08/17] hw/arm/aspeed: " Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 09/17] hw/arm: Use object_initialize_child for correct reference counting Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 10/17] hw/mips: Use object_initialize() on MIPSCPSState Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 11/17] hw/mips: Use object_initialize_child for correct reference counting Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 12/17] hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 13/17] hw/microblaze/zynqmp: Let the SoC manage the IPI devices Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 14/17] hw/microblaze/zynqmp: Use object_initialize_child for correct ref. counting Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 15/17] " Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 16/17] hw/arm/mps2: Use object_initialize_child for correct reference counting Eduardo Habkost
2019-05-24 18:44 ` [Qemu-devel] [PULL 17/17] hw/intc/nvic: " Eduardo Habkost
2019-05-28 10:51 ` [Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24 Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.