[Qemu-devel] [PATCH 0/6] hw/arm: Use ARM_CPU_TYPE_NAME() and object_initialize_child()

[Qemu-devel] [PATCH 0/6] hw/arm: Use ARM_CPU_TYPE_NAME() and object_initialize_child()

[Philippe Mathieu-Daudé]

First we use ARM_CPU_TYPE_NAME() when we should.

Then is follow up of
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg01492.html

  This series looks at Eduardo suggestions from [1]
  and Thomas commit aff39be0ed97 to replace various
  object_initialize + qdev_set_parent_bus calls by
  sysbus_init_child_obj().

Finally, some devices are declared orphean while they have a parent,
let them be together again.

[1] https://patchwork.ozlabs.org/patch/943333/#1953608

Philippe Mathieu-Daudé (6):
  hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate
  hw/arm: Use object_initialize_child for correct reference counting
  hw/arm: Use sysbus_init_child_obj for correct reference counting
  hw/arm/fsl-imx: Add the cpu as child of the SoC object
  hw/dma/xilinx_axi: Use object_initialize_child for correct ref.
    counting
  hw/net/xilinx_axi: Use object_initialize_child for correct ref.
    counting

 hw/arm/allwinner-a10.c  |  3 ++-
 hw/arm/cubieboard.c     |  3 ++-
 hw/arm/digic.c          |  3 ++-
 hw/arm/exynos4_boards.c |  4 ++--
 hw/arm/fsl-imx25.c      |  4 +++-
 hw/arm/fsl-imx31.c      |  4 +++-
 hw/arm/fsl-imx6.c       |  3 ++-
 hw/arm/fsl-imx6ul.c     |  3 ++-
 hw/arm/mcimx7d-sabre.c  |  9 ++++-----
 hw/arm/mps2-tz.c        | 15 +++++++--------
 hw/arm/musca.c          |  9 +++++----
 hw/arm/xlnx-zynqmp.c    |  8 ++++----
 hw/dma/xilinx_axidma.c  | 16 ++++++++--------
 hw/net/xilinx_axienet.c | 17 ++++++++---------
 14 files changed, 54 insertions(+), 47 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 1/6] hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate

[Philippe Mathieu-Daudé]

Commit ba1ba5cca introduce the ARM_CPU_TYPE_NAME() macro.
Unify the code base by use it in all places.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/allwinner-a10.c | 3 ++-
 hw/arm/cubieboard.c    | 3 ++-
 hw/arm/digic.c         | 3 ++-
 hw/arm/fsl-imx6.c      | 3 ++-
 hw/arm/fsl-imx6ul.c    | 3 ++-
 hw/arm/xlnx-zynqmp.c   | 8 ++++----
 6 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 35e906ca54..49d4d76686 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -28,7 +28,8 @@ static void aw_a10_init(Object *obj)
     AwA10State *s = AW_A10(obj);
 
     object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
-                            "cortex-a8-" TYPE_ARM_CPU, &error_abort, NULL);
+                            ARM_CPU_TYPE_NAME("cortex-a8"),
+                            &error_abort, NULL);
 
     sysbus_init_child_obj(obj, "intc", &s->intc, sizeof(s->intc),
                           TYPE_AW_A10_PIC);
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index f7c8a5985a..a4d3d7a6a0 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -80,7 +80,8 @@ static void cubieboard_init(MachineState *machine)
 
 static void cubieboard_machine_init(MachineClass *mc)
 {
-    mc->desc = "cubietech cubieboard";
+    mc->desc = "cubietech cubieboard (Cortex-A9)";
+    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
     mc->init = cubieboard_init;
     mc->block_default_type = IF_IDE;
     mc->units_per_default_bus = 1;
diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index 9015b60c23..05db06be5e 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -36,7 +36,8 @@ static void digic_init(Object *obj)
     int i;
 
     object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
-                            "arm946-" TYPE_ARM_CPU, &error_abort, NULL);
+                            ARM_CPU_TYPE_NAME("arm946"),
+                            &error_abort, NULL);
 
     for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
 #define DIGIC_TIMER_NAME_MLEN    11
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index 7129517378..9b259c54b3 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -40,7 +40,8 @@ static void fsl_imx6_init(Object *obj)
     for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) {
         snprintf(name, NAME_SIZE, "cpu%d", i);
         object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]),
-                                "cortex-a9-" TYPE_ARM_CPU, &error_abort, NULL);
+                                ARM_CPU_TYPE_NAME("cortex-a9"),
+                                &error_abort, NULL);
     }
 
     sysbus_init_child_obj(obj, "a9mpcore", &s->a9mpcore, sizeof(s->a9mpcore),
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index 05505bac56..1cbd8674bf 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -35,7 +35,8 @@ static void fsl_imx6ul_init(Object *obj)
     for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
         snprintf(name, NAME_SIZE, "cpu%d", i);
         object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]),
-                                "cortex-a7-" TYPE_ARM_CPU, &error_abort, NULL);
+                                ARM_CPU_TYPE_NAME("cortex-a7"),
+                                &error_abort, NULL);
     }
 
     /*
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index a1ca9b5adf..2acd032df6 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -193,8 +193,8 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
 
         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);
+                                ARM_CPU_TYPE_NAME("cortex-r5f"),
+                                &error_abort, NULL);
 
         name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
         if (strcmp(name, boot_cpu)) {
@@ -233,8 +233,8 @@ static void xlnx_zynqmp_init(Object *obj)
     for (i = 0; i < num_apus; i++) {
         object_initialize_child(OBJECT(&s->apu_cluster), "apu-cpu[*]",
                                 &s->apu_cpu[i], sizeof(s->apu_cpu[i]),
-                                "cortex-a53-" TYPE_ARM_CPU, &error_abort,
-                                NULL);
+                                ARM_CPU_TYPE_NAME("cortex-a53"),
+                                &error_abort, NULL);
     }
 
     sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic),
-- 
2.20.1

Re: [Qemu-devel] [PATCH 1/6] hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:37 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Commit ba1ba5cca introduce the ARM_CPU_TYPE_NAME() macro.
> Unify the code base by use it in all places.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/arm/allwinner-a10.c | 3 ++-
>  hw/arm/cubieboard.c    | 3 ++-
>  hw/arm/digic.c         | 3 ++-
>  hw/arm/fsl-imx6.c      | 3 ++-
>  hw/arm/fsl-imx6ul.c    | 3 ++-
>  hw/arm/xlnx-zynqmp.c   | 8 ++++----
>  6 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
> index 35e906ca54..49d4d76686 100644
> --- a/hw/arm/allwinner-a10.c
> +++ b/hw/arm/allwinner-a10.c
> @@ -28,7 +28,8 @@ static void aw_a10_init(Object *obj)
>      AwA10State *s = AW_A10(obj);
>
>      object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
> -                            "cortex-a8-" TYPE_ARM_CPU, &error_abort, NULL);
> +                            ARM_CPU_TYPE_NAME("cortex-a8"),
> +                            &error_abort, NULL);
>
>      sysbus_init_child_obj(obj, "intc", &s->intc, sizeof(s->intc),
>                            TYPE_AW_A10_PIC);
> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
> index f7c8a5985a..a4d3d7a6a0 100644
> --- a/hw/arm/cubieboard.c
> +++ b/hw/arm/cubieboard.c
> @@ -80,7 +80,8 @@ static void cubieboard_init(MachineState *machine)
>
>  static void cubieboard_machine_init(MachineClass *mc)
>  {
> -    mc->desc = "cubietech cubieboard";
> +    mc->desc = "cubietech cubieboard (Cortex-A9)";
> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
>      mc->init = cubieboard_init;
>      mc->block_default_type = IF_IDE;
>      mc->units_per_default_bus = 1;
> diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> index 9015b60c23..05db06be5e 100644
> --- a/hw/arm/digic.c
> +++ b/hw/arm/digic.c
> @@ -36,7 +36,8 @@ static void digic_init(Object *obj)
>      int i;
>
>      object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
> -                            "arm946-" TYPE_ARM_CPU, &error_abort, NULL);
> +                            ARM_CPU_TYPE_NAME("arm946"),
> +                            &error_abort, NULL);
>
>      for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
>  #define DIGIC_TIMER_NAME_MLEN    11
> diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
> index 7129517378..9b259c54b3 100644
> --- a/hw/arm/fsl-imx6.c
> +++ b/hw/arm/fsl-imx6.c
> @@ -40,7 +40,8 @@ static void fsl_imx6_init(Object *obj)
>      for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) {
>          snprintf(name, NAME_SIZE, "cpu%d", i);
>          object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]),
> -                                "cortex-a9-" TYPE_ARM_CPU, &error_abort, NULL);
> +                                ARM_CPU_TYPE_NAME("cortex-a9"),
> +                                &error_abort, NULL);
>      }
>
>      sysbus_init_child_obj(obj, "a9mpcore", &s->a9mpcore, sizeof(s->a9mpcore),
> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index 05505bac56..1cbd8674bf 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -35,7 +35,8 @@ static void fsl_imx6ul_init(Object *obj)
>      for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
>          snprintf(name, NAME_SIZE, "cpu%d", i);
>          object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]),
> -                                "cortex-a7-" TYPE_ARM_CPU, &error_abort, NULL);
> +                                ARM_CPU_TYPE_NAME("cortex-a7"),
> +                                &error_abort, NULL);
>      }
>
>      /*
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index a1ca9b5adf..2acd032df6 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -193,8 +193,8 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
>
>          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);
> +                                ARM_CPU_TYPE_NAME("cortex-r5f"),
> +                                &error_abort, NULL);
>
>          name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
>          if (strcmp(name, boot_cpu)) {
> @@ -233,8 +233,8 @@ static void xlnx_zynqmp_init(Object *obj)
>      for (i = 0; i < num_apus; i++) {
>          object_initialize_child(OBJECT(&s->apu_cluster), "apu-cpu[*]",
>                                  &s->apu_cpu[i], sizeof(s->apu_cpu[i]),
> -                                "cortex-a53-" TYPE_ARM_CPU, &error_abort,
> -                                NULL);
> +                                ARM_CPU_TYPE_NAME("cortex-a53"),
> +                                &error_abort, NULL);
>      }
>
>      sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic),
> --
> 2.20.1
>
>

Re: [Qemu-devel] [PATCH 1/6] hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate

[Peter Maydell]

On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Commit ba1ba5cca introduce the ARM_CPU_TYPE_NAME() macro.
> Unify the code base by use it in all places.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---


> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index 05505bac56..1cbd8674bf 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -35,7 +35,8 @@ static void fsl_imx6ul_init(Object *obj)
>      for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
>          snprintf(name, NAME_SIZE, "cpu%d", i);
>          object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]),
> -                                "cortex-a7-" TYPE_ARM_CPU, &error_abort, NULL);
> +                                ARM_CPU_TYPE_NAME("cortex-a7"),
> +                                &error_abort, NULL);
>      }
>
>      /*

You'll find this part needs a minor fix to apply to master
now we've got rid of this loop-over-CPUs in fsl-imx6ul.c.

thanks
-- PMM

[Qemu-devel] [PATCH 2/6] hw/arm: Use object_initialize_child for correct reference counting

[Philippe Mathieu-Daudé]

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/mcimx7d-sabre.c |  9 ++++-----
 hw/arm/mps2-tz.c       | 15 +++++++--------
 hw/arm/musca.c         |  9 +++++----
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
index d6b190d85d..8e211aa8df 100644
--- a/hw/arm/mcimx7d-sabre.c
+++ b/hw/arm/mcimx7d-sabre.c
@@ -29,7 +29,6 @@ static void mcimx7d_sabre_init(MachineState *machine)
 {
     static struct arm_boot_info boot_info;
     MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1);
-    Object *soc;
     int i;
 
     if (machine->ram_size > FSL_IMX7_MMDC_SIZE) {
@@ -48,10 +47,10 @@ static void mcimx7d_sabre_init(MachineState *machine)
         .nb_cpus = smp_cpus,
     };
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_FSL_IMX7);
-    soc = OBJECT(&s->soc);
-    object_property_add_child(OBJECT(machine), "soc", soc, &error_fatal);
-    object_property_set_bool(soc, true, "realized", &error_fatal);
+    object_initialize_child(OBJECT(machine), "soc",
+                            &s->soc, sizeof(s->soc),
+                            TYPE_FSL_IMX7, &error_fatal, NULL);
+    object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
 
     memory_region_allocate_system_memory(&s->ram, NULL, "mcimx7d-sabre.ram",
                                          machine->ram_size);
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index d85dc2c4bd..6b24aaacde 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -427,10 +427,10 @@ static void mps2tz_common_init(MachineState *machine)
     /* The sec_resp_cfg output from the IoTKit must be split into multiple
      * lines, one for each of the PPCs we create here, plus one per MSC.
      */
-    object_initialize(&mms->sec_resp_splitter, sizeof(mms->sec_resp_splitter),
-                      TYPE_SPLIT_IRQ);
-    object_property_add_child(OBJECT(machine), "sec-resp-splitter",
-                              OBJECT(&mms->sec_resp_splitter), &error_abort);
+    object_initialize_child(OBJECT(machine), "sec-resp-splitter",
+                            &mms->sec_resp_splitter,
+                            sizeof(mms->sec_resp_splitter),
+                            TYPE_SPLIT_IRQ, &error_abort, NULL);
     object_property_set_int(OBJECT(&mms->sec_resp_splitter),
                             ARRAY_SIZE(mms->ppc) + ARRAY_SIZE(mms->msc),
                             "num-lines", &error_fatal);
@@ -465,10 +465,9 @@ static void mps2tz_common_init(MachineState *machine)
      * Tx, Rx and "combined" IRQs are sent to the NVIC separately.
      * Create the OR gate for this.
      */
-    object_initialize(&mms->uart_irq_orgate, sizeof(mms->uart_irq_orgate),
-                      TYPE_OR_IRQ);
-    object_property_add_child(OBJECT(mms), "uart-irq-orgate",
-                              OBJECT(&mms->uart_irq_orgate), &error_abort);
+    object_initialize_child(OBJECT(mms), "uart-irq-orgate",
+                            &mms->uart_irq_orgate, sizeof(mms->uart_irq_orgate),
+                            TYPE_OR_IRQ, &error_abort, NULL);
     object_property_set_int(OBJECT(&mms->uart_irq_orgate), 10, "num-lines",
                             &error_fatal);
     object_property_set_bool(OBJECT(&mms->uart_irq_orgate), true,
diff --git a/hw/arm/musca.c b/hw/arm/musca.c
index ddd8842732..68db4b5b38 100644
--- a/hw/arm/musca.c
+++ b/hw/arm/musca.c
@@ -424,10 +424,11 @@ static void musca_init(MachineState *machine)
      * The sec_resp_cfg output from the SSE-200 must be split into multiple
      * lines, one for each of the PPCs we create here.
      */
-    object_initialize(&mms->sec_resp_splitter, sizeof(mms->sec_resp_splitter),
-                      TYPE_SPLIT_IRQ);
-    object_property_add_child(OBJECT(machine), "sec-resp-splitter",
-                              OBJECT(&mms->sec_resp_splitter), &error_fatal);
+    object_initialize_child(OBJECT(machine), "sec-resp-splitter",
+                            &mms->sec_resp_splitter,
+                            sizeof(mms->sec_resp_splitter),
+                            TYPE_SPLIT_IRQ, &error_fatal, NULL);
+
     object_property_set_int(OBJECT(&mms->sec_resp_splitter),
                             ARRAY_SIZE(mms->ppc), "num-lines", &error_fatal);
     object_property_set_bool(OBJECT(&mms->sec_resp_splitter), true,
-- 
2.20.1

Re: [Qemu-devel] [PATCH 2/6] hw/arm: Use object_initialize_child for correct reference counting

[Peter Maydell]

On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> 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.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH 3/6] hw/arm: Use sysbus_init_child_obj for correct reference counting

[Philippe Mathieu-Daudé]

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/exynos4_boards.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index ac0b0dc2a9..5dd53d2a23 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -129,8 +129,8 @@ exynos4_boards_init_common(MachineState *machine,
     exynos4_boards_init_ram(s, get_system_memory(),
                             exynos4_board_ram_size[board_type]);
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
-    qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default());
+    sysbus_init_child_obj(OBJECT(machine), "soc",
+                          &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
     object_property_set_bool(OBJECT(&s->soc), true, "realized",
                              &error_fatal);
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 3/6] hw/arm: Use sysbus_init_child_obj for correct reference counting

[Peter Maydell]

On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> 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.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/exynos4_boards.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
> index ac0b0dc2a9..5dd53d2a23 100644
> --- a/hw/arm/exynos4_boards.c
> +++ b/hw/arm/exynos4_boards.c
> @@ -129,8 +129,8 @@ exynos4_boards_init_common(MachineState *machine,
>      exynos4_boards_init_ram(s, get_system_memory(),
>                              exynos4_board_ram_size[board_type]);
>
> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
> -    qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default());
> +    sysbus_init_child_obj(OBJECT(machine), "soc",
> +                          &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
>      object_property_set_bool(OBJECT(&s->soc), true, "realized",
>                               &error_fatal);

I suspect the code change here is correct but it doesn't seem
to match the commit message -- the old code is not calling
object_property_add_child() at all, and the new code does not
call object_initialize_child()...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/6] hw/arm: Use sysbus_init_child_obj for correct reference counting

[Philippe Mathieu-Daudé]

On 7/29/19 3:03 PM, Peter Maydell wrote:
> On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> 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.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/arm/exynos4_boards.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
>> index ac0b0dc2a9..5dd53d2a23 100644
>> --- a/hw/arm/exynos4_boards.c
>> +++ b/hw/arm/exynos4_boards.c
>> @@ -129,8 +129,8 @@ exynos4_boards_init_common(MachineState *machine,
>>      exynos4_boards_init_ram(s, get_system_memory(),
>>                              exynos4_board_ram_size[board_type]);
>>
>> -    object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
>> -    qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default());
>> +    sysbus_init_child_obj(OBJECT(machine), "soc",
>> +                          &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC);
>>      object_property_set_bool(OBJECT(&s->soc), true, "realized",
>>                               &error_fatal);
> 
> I suspect the code change here is correct but it doesn't seem
> to match the commit message -- the old code is not calling
> object_property_add_child() at all, and the new code does not
> call object_initialize_child()...

OK, will improve, thanks!

[Qemu-devel] [PATCH 4/6] hw/arm/fsl-imx: Add the cpu as child of the SoC object

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/fsl-imx25.c | 4 +++-
 hw/arm/fsl-imx31.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index 869ee89b15..a237e967e4 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -36,7 +36,9 @@ static void fsl_imx25_init(Object *obj)
     FslIMX25State *s = FSL_IMX25(obj);
     int i;
 
-    object_initialize(&s->cpu, sizeof(s->cpu), "arm926-" TYPE_ARM_CPU);
+    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
+                            ARM_CPU_TYPE_NAME("arm926"),
+                            &error_abort, NULL);
 
     sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic),
                           TYPE_IMX_AVIC);
diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
index 662fe78f1b..423d9ef076 100644
--- a/hw/arm/fsl-imx31.c
+++ b/hw/arm/fsl-imx31.c
@@ -33,7 +33,9 @@ static void fsl_imx31_init(Object *obj)
     FslIMX31State *s = FSL_IMX31(obj);
     int i;
 
-    object_initialize(&s->cpu, sizeof(s->cpu), "arm1136-" TYPE_ARM_CPU);
+    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
+                            ARM_CPU_TYPE_NAME("arm1136"),
+                            &error_abort, NULL);
 
     sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic),
                           TYPE_IMX_AVIC);
-- 
2.20.1

Re: [Qemu-devel] [PATCH 4/6] hw/arm/fsl-imx: Add the cpu as child of the SoC object

[Peter Maydell]

On Mon, 1 Jul 2019 at 13:31, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/fsl-imx25.c | 4 +++-
>  hw/arm/fsl-imx31.c | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
> index 869ee89b15..a237e967e4 100644
> --- a/hw/arm/fsl-imx25.c
> +++ b/hw/arm/fsl-imx25.c
> @@ -36,7 +36,9 @@ static void fsl_imx25_init(Object *obj)
>      FslIMX25State *s = FSL_IMX25(obj);
>      int i;
>
> -    object_initialize(&s->cpu, sizeof(s->cpu), "arm926-" TYPE_ARM_CPU);
> +    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
> +                            ARM_CPU_TYPE_NAME("arm926"),
> +                            &error_abort, NULL);
>
>      sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic),
>                            TYPE_IMX_AVIC);
> diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
> index 662fe78f1b..423d9ef076 100644
> --- a/hw/arm/fsl-imx31.c
> +++ b/hw/arm/fsl-imx31.c
> @@ -33,7 +33,9 @@ static void fsl_imx31_init(Object *obj)
>      FslIMX31State *s = FSL_IMX31(obj);
>      int i;
>
> -    object_initialize(&s->cpu, sizeof(s->cpu), "arm1136-" TYPE_ARM_CPU);
> +    object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu),
> +                            ARM_CPU_TYPE_NAME("arm1136"),
> +                            &error_abort, NULL);
>
>      sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic),
>                            TYPE_IMX_AVIC);
> --
> 2.20.1

Really the ARM_CPU_TYPE_NAME part of this change should be
in patch 1 I think...

Could you expand the commit message a little to explain why
we want to make the CPU a child of the SoC object?

thanks
-- PMM

[Qemu-devel] [PATCH 5/6] hw/dma/xilinx_axi: Use object_initialize_child for correct ref. counting

[Philippe Mathieu-Daudé]

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/dma/xilinx_axidma.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index 921be178d9..91f5ec587f 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -563,14 +563,14 @@ static void xilinx_axidma_init(Object *obj)
     XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
-    object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
-                      TYPE_XILINX_AXI_DMA_DATA_STREAM);
-    object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
-                      TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
-    object_property_add_child(OBJECT(s), "axistream-connected-target",
-                              (Object *)&s->rx_data_dev, &error_abort);
-    object_property_add_child(OBJECT(s), "axistream-control-connected-target",
-                              (Object *)&s->rx_control_dev, &error_abort);
+    object_initialize_child(OBJECT(s), "axistream-connected-target",
+                            &s->rx_data_dev, sizeof(s->rx_data_dev),
+                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
+                            NULL);
+    object_initialize_child(OBJECT(s), "axistream-control-connected-target",
+                            &s->rx_control_dev, sizeof(s->rx_control_dev),
+                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort,
+                            NULL);
 
     sysbus_init_irq(sbd, &s->streams[0].irq);
     sysbus_init_irq(sbd, &s->streams[1].irq);
-- 
2.20.1

Re: [Qemu-devel] [PATCH 5/6] hw/dma/xilinx_axi: Use object_initialize_child for correct ref. counting

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:32 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> 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.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/dma/xilinx_axidma.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> index 921be178d9..91f5ec587f 100644
> --- a/hw/dma/xilinx_axidma.c
> +++ b/hw/dma/xilinx_axidma.c
> @@ -563,14 +563,14 @@ static void xilinx_axidma_init(Object *obj)
>      XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>
> -    object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
> -                      TYPE_XILINX_AXI_DMA_DATA_STREAM);
> -    object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
> -                      TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
> -    object_property_add_child(OBJECT(s), "axistream-connected-target",
> -                              (Object *)&s->rx_data_dev, &error_abort);
> -    object_property_add_child(OBJECT(s), "axistream-control-connected-target",
> -                              (Object *)&s->rx_control_dev, &error_abort);
> +    object_initialize_child(OBJECT(s), "axistream-connected-target",
> +                            &s->rx_data_dev, sizeof(s->rx_data_dev),
> +                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
> +                            NULL);
> +    object_initialize_child(OBJECT(s), "axistream-control-connected-target",
> +                            &s->rx_control_dev, sizeof(s->rx_control_dev),
> +                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort,
> +                            NULL);
>
>      sysbus_init_irq(sbd, &s->streams[0].irq);
>      sysbus_init_irq(sbd, &s->streams[1].irq);
> --
> 2.20.1
>
>

Re: [Qemu-devel] [PATCH 5/6] hw/dma/xilinx_axi: Use object_initialize_child for correct ref. counting

[Philippe Mathieu-Daudé]

On 7/2/19 6:18 PM, Alistair Francis wrote:
> On Mon, Jul 1, 2019 at 5:32 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> 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.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Alistair
> 
>> ---
>>  hw/dma/xilinx_axidma.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
>> index 921be178d9..91f5ec587f 100644
>> --- a/hw/dma/xilinx_axidma.c
>> +++ b/hw/dma/xilinx_axidma.c
>> @@ -563,14 +563,14 @@ static void xilinx_axidma_init(Object *obj)
>>      XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
>>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>>
>> -    object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
>> -                      TYPE_XILINX_AXI_DMA_DATA_STREAM);
>> -    object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
>> -                      TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
>> -    object_property_add_child(OBJECT(s), "axistream-connected-target",
>> -                              (Object *)&s->rx_data_dev, &error_abort);
>> -    object_property_add_child(OBJECT(s), "axistream-control-connected-target",
>> -                              (Object *)&s->rx_control_dev, &error_abort);
>> +    object_initialize_child(OBJECT(s), "axistream-connected-target",
>> +                            &s->rx_data_dev, sizeof(s->rx_data_dev),
>> +                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
>> +                            NULL);
>> +    object_initialize_child(OBJECT(s), "axistream-control-connected-target",
>> +                            &s->rx_control_dev, sizeof(s->rx_control_dev),
>> +                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort,
>> +                            NULL);
>>
>>      sysbus_init_irq(sbd, &s->streams[0].irq);
>>      sysbus_init_irq(sbd, &s->streams[1].irq);
>> --
>> 2.20.1

As GCC suggested, this patch requires this snippet squashed:

-- >8 --
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
@@ -565,11 +565,11 @@ static void xilinx_axidma_init(Object *obj)

     object_initialize_child(OBJECT(s), "axistream-connected-target",
                             &s->rx_data_dev, sizeof(s->rx_data_dev),
-                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
+                            TYPE_XILINX_AXI_DMA_DATA_STREAM, &error_abort,
                             NULL);
     object_initialize_child(OBJECT(s),
"axistream-control-connected-target",
                             &s->rx_control_dev, sizeof(s->rx_control_dev),
-                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM,
&error_abort,
+                            TYPE_XILINX_AXI_DMA_CONTROL_STREAM,
&error_abort,
                             NULL);

     sysbus_init_irq(sbd, &s->streams[0].irq);
---

Since it was commented in reply to the cover, I assume your R-b stands
with the snippet applied.

Thanks for reviewing,

Phil.

Re: [Qemu-devel] [PATCH 5/6] hw/dma/xilinx_axi: Use object_initialize_child for correct ref. counting

[Alistair Francis]

On Tue, Jul 2, 2019 at 10:00 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 7/2/19 6:18 PM, Alistair Francis wrote:
> > On Mon, Jul 1, 2019 at 5:32 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >>
> >> 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.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >
> > Alistair
> >
> >> ---
> >>  hw/dma/xilinx_axidma.c | 16 ++++++++--------
> >>  1 file changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> >> index 921be178d9..91f5ec587f 100644
> >> --- a/hw/dma/xilinx_axidma.c
> >> +++ b/hw/dma/xilinx_axidma.c
> >> @@ -563,14 +563,14 @@ static void xilinx_axidma_init(Object *obj)
> >>      XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
> >>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> >>
> >> -    object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
> >> -                      TYPE_XILINX_AXI_DMA_DATA_STREAM);
> >> -    object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
> >> -                      TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
> >> -    object_property_add_child(OBJECT(s), "axistream-connected-target",
> >> -                              (Object *)&s->rx_data_dev, &error_abort);
> >> -    object_property_add_child(OBJECT(s), "axistream-control-connected-target",
> >> -                              (Object *)&s->rx_control_dev, &error_abort);
> >> +    object_initialize_child(OBJECT(s), "axistream-connected-target",
> >> +                            &s->rx_data_dev, sizeof(s->rx_data_dev),
> >> +                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
> >> +                            NULL);
> >> +    object_initialize_child(OBJECT(s), "axistream-control-connected-target",
> >> +                            &s->rx_control_dev, sizeof(s->rx_control_dev),
> >> +                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort,
> >> +                            NULL);
> >>
> >>      sysbus_init_irq(sbd, &s->streams[0].irq);
> >>      sysbus_init_irq(sbd, &s->streams[1].irq);
> >> --
> >> 2.20.1
>
> As GCC suggested, this patch requires this snippet squashed:
>
> -- >8 --
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> @@ -565,11 +565,11 @@ static void xilinx_axidma_init(Object *obj)
>
>      object_initialize_child(OBJECT(s), "axistream-connected-target",
>                              &s->rx_data_dev, sizeof(s->rx_data_dev),
> -                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
> +                            TYPE_XILINX_AXI_DMA_DATA_STREAM, &error_abort,
>                              NULL);
>      object_initialize_child(OBJECT(s),
> "axistream-control-connected-target",
>                              &s->rx_control_dev, sizeof(s->rx_control_dev),
> -                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM,
> &error_abort,
> +                            TYPE_XILINX_AXI_DMA_CONTROL_STREAM,
> &error_abort,
>                              NULL);
>
>      sysbus_init_irq(sbd, &s->streams[0].irq);
> ---

Ah I missed that

>
> Since it was commented in reply to the cover, I assume your R-b stands
> with the snippet applied.

Yeah it does.

Alistair

>
> Thanks for reviewing,
>
> Phil.

[Qemu-devel] [PATCH 6/6] hw/net/xilinx_axi: Use object_initialize_child for correct ref. counting

[Philippe Mathieu-Daudé]

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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/net/xilinx_axienet.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index feeaca680e..aa0ae3c013 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -990,15 +990,14 @@ static void xilinx_enet_init(Object *obj)
     XilinxAXIEnet *s = XILINX_AXI_ENET(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
-    object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
-                      TYPE_XILINX_AXI_ENET_DATA_STREAM);
-    object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
-                      TYPE_XILINX_AXI_ENET_CONTROL_STREAM);
-    object_property_add_child(OBJECT(s), "axistream-connected-target",
-                              (Object *)&s->rx_data_dev, &error_abort);
-    object_property_add_child(OBJECT(s), "axistream-control-connected-target",
-                              (Object *)&s->rx_control_dev, &error_abort);
-
+    object_initialize_child(OBJECT(s), "axistream-connected-target",
+                            &s->rx_data_dev, sizeof(s->rx_data_dev),
+                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
+                            NULL);
+    object_initialize_child(OBJECT(s), "axistream-control-connected-target",
+                            &s->rx_control_dev, sizeof(s->rx_control_dev),
+                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort,
+                            NULL);
     sysbus_init_irq(sbd, &s->irq);
 
     memory_region_init_io(&s->iomem, OBJECT(s), &enet_ops, s, "enet", 0x40000);
-- 
2.20.1

Re: [Qemu-devel] [PATCH 6/6] hw/net/xilinx_axi: Use object_initialize_child for correct ref. counting

[Alistair Francis]

On Mon, Jul 1, 2019 at 5:38 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> 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.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/net/xilinx_axienet.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> index feeaca680e..aa0ae3c013 100644
> --- a/hw/net/xilinx_axienet.c
> +++ b/hw/net/xilinx_axienet.c
> @@ -990,15 +990,14 @@ static void xilinx_enet_init(Object *obj)
>      XilinxAXIEnet *s = XILINX_AXI_ENET(obj);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>
> -    object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
> -                      TYPE_XILINX_AXI_ENET_DATA_STREAM);
> -    object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
> -                      TYPE_XILINX_AXI_ENET_CONTROL_STREAM);
> -    object_property_add_child(OBJECT(s), "axistream-connected-target",
> -                              (Object *)&s->rx_data_dev, &error_abort);
> -    object_property_add_child(OBJECT(s), "axistream-control-connected-target",
> -                              (Object *)&s->rx_control_dev, &error_abort);
> -
> +    object_initialize_child(OBJECT(s), "axistream-connected-target",
> +                            &s->rx_data_dev, sizeof(s->rx_data_dev),
> +                            TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
> +                            NULL);
> +    object_initialize_child(OBJECT(s), "axistream-control-connected-target",
> +                            &s->rx_control_dev, sizeof(s->rx_control_dev),
> +                            TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort,
> +                            NULL);
>      sysbus_init_irq(sbd, &s->irq);
>
>      memory_region_init_io(&s->iomem, OBJECT(s), &enet_ops, s, "enet", 0x40000);
> --
> 2.20.1
>
>

Re: [Qemu-devel] [PATCH 0/6] hw/arm: Use ARM_CPU_TYPE_NAME() and object_initialize_child()

[Philippe Mathieu-Daudé]

Buh I forgot to Cc Eduardo.

On 7/1/19 2:31 PM, Philippe Mathieu-Daudé wrote:
> First we use ARM_CPU_TYPE_NAME() when we should.
> 
> Then is follow up of
> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg01492.html
> 
>   This series looks at Eduardo suggestions from [1]
>   and Thomas commit aff39be0ed97 to replace various
>   object_initialize + qdev_set_parent_bus calls by
>   sysbus_init_child_obj().
> 
> Finally, some devices are declared orphean while they have a parent,
> let them be together again.
> 
> [1] https://patchwork.ozlabs.org/patch/943333/#1953608
> 
> Philippe Mathieu-Daudé (6):
>   hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate
>   hw/arm: Use object_initialize_child for correct reference counting
>   hw/arm: Use sysbus_init_child_obj for correct reference counting
>   hw/arm/fsl-imx: Add the cpu as child of the SoC object
>   hw/dma/xilinx_axi: Use object_initialize_child for correct ref.
>     counting
>   hw/net/xilinx_axi: Use object_initialize_child for correct ref.
>     counting

Re: [Qemu-devel] [PATCH 0/6] hw/arm: Use ARM_CPU_TYPE_NAME() and object_initialize_child()

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190701123108.12493-1-philmd@redhat.com/



Hi,

This series failed build test on s390x host. Please find the details below.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e

echo
echo "=== ENV ==="
env

echo
echo "=== PACKAGES ==="
rpm -qa

echo
echo "=== UNAME ==="
uname -a

CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

  CC      hw/dma/i8257.o
  CC      hw/dma/xilinx_axidma.o
/var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c: In function ‘xilinx_axidma_init’:
/var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c:568:29: error: ‘TYPE_XILINX_AXI_ENET_DATA_STREAM’ undeclared (first use in this function); did you mean ‘TYPE_XILINX_AXI_DMA_DATA_STREAM’?
  568 |                             TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                             TYPE_XILINX_AXI_DMA_DATA_STREAM
/var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c:568:29: note: each undeclared identifier is reported only once for each function it appears in
/var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c:572:29: error: ‘TYPE_XILINX_AXI_ENET_CONTROL_STREAM’ undeclared (first use in this function); did you mean ‘TYPE_XILINX_AXI_DMA_CONTROL_STREAM’?
  572 |                             TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort,
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                             TYPE_XILINX_AXI_DMA_CONTROL_STREAM


The full log is available at
http://patchew.org/logs/20190701123108.12493-1-philmd@redhat.com/testing.s390x/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/6] hw/arm: Use ARM_CPU_TYPE_NAME() and object_initialize_child()

[Philippe Mathieu-Daudé]

On 7/1/19 7:56 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20190701123108.12493-1-philmd@redhat.com/
> 
> Hi,
> 
> This series failed build test on s390x host. Please find the details below.
> 
>   CC      hw/dma/i8257.o
>   CC      hw/dma/xilinx_axidma.o
> /var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c: In function ‘xilinx_axidma_init’:
> /var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c:568:29: error: ‘TYPE_XILINX_AXI_ENET_DATA_STREAM’ undeclared (first use in this function); did you mean ‘TYPE_XILINX_AXI_DMA_DATA_STREAM’?
>   568 |                             TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
>       |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |                             TYPE_XILINX_AXI_DMA_DATA_STREAM
> /var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c:568:29: note: each undeclared identifier is reported only once for each function it appears in
> /var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c:572:29: error: ‘TYPE_XILINX_AXI_ENET_CONTROL_STREAM’ undeclared (first use in this function); did you mean ‘TYPE_XILINX_AXI_DMA_CONTROL_STREAM’?
>   572 |                             TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort,
>       |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |                             TYPE_XILINX_AXI_DMA_CONTROL_STREAM

It would be nice if GCC directly fix the patch instead of embarrassing
the author for obvious copy/paste mistakes...

Re: [Qemu-devel] [PATCH 0/6] hw/arm: Use ARM_CPU_TYPE_NAME() and object_initialize_child()

[Peter Maydell]

On Mon, 1 Jul 2019 at 19:13, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 7/1/19 7:56 PM, no-reply@patchew.org wrote:
> > Patchew URL: https://patchew.org/QEMU/20190701123108.12493-1-philmd@redhat.com/
> >
> > Hi,
> >
> > This series failed build test on s390x host. Please find the details below.
> >
> >   CC      hw/dma/i8257.o
> >   CC      hw/dma/xilinx_axidma.o
> > /var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c: In function ‘xilinx_axidma_init’:
> > /var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c:568:29: error: ‘TYPE_XILINX_AXI_ENET_DATA_STREAM’ undeclared (first use in this function); did you mean ‘TYPE_XILINX_AXI_DMA_DATA_STREAM’?
> >   568 |                             TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort,
> >       |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >       |                             TYPE_XILINX_AXI_DMA_DATA_STREAM
> > /var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c:568:29: note: each undeclared identifier is reported only once for each function it appears in
> > /var/tmp/patchew-tester-tmp-6umc5eop/src/hw/dma/xilinx_axidma.c:572:29: error: ‘TYPE_XILINX_AXI_ENET_CONTROL_STREAM’ undeclared (first use in this function); did you mean ‘TYPE_XILINX_AXI_DMA_CONTROL_STREAM’?
> >   572 |                             TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort,
> >       |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >       |                             TYPE_XILINX_AXI_DMA_CONTROL_STREAM
>
> It would be nice if GCC directly fix the patch instead of embarrassing
> the author for obvious copy/paste mistakes...

Sufficiently new gcc supports -fdiagnostics-generate-patch, and you could
probably script up a wrapper to automatically apply the patches ;-)

thanks
-- PMM