1. Patchew
  2. QEMU
  3. [PATCH 0/8] hw/ppc: Convert UIC device to QOM
  4. View patch

[PATCH 3/8] hw/ppc/virtex_ml507: Drop use of ppcuic_init()

Peter Maydell posted 8 patches 5 years, 1 month ago
Maintainers: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, BALATON Zoltan <balaton@eik.bme.hu>
There is a newer version of this series
[PATCH 3/8] hw/ppc/virtex_ml507: Drop use of ppcuic_init()
Posted by Peter Maydell 5 years, 1 month ago 
Switch the virtex_ml507 board to directly creating and
configuring the UIC, rather than doing it via the old
ppcuic_init() helper function.

This fixes a trivial Coverity-detected memory leak where
we were leaking the array of IRQs returned by ppcuic_init().

Fixes: Coverity CID 1421992
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/ppc/virtex_ml507.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 7f1bca928c1..34767b11cad 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -43,6 +43,7 @@
 #include "qemu/option.h"
 #include "exec/address-spaces.h"
 
+#include "hw/intc/ppc-uic.h"
 #include "hw/ppc/ppc.h"
 #include "hw/ppc/ppc4xx.h"
 #include "hw/qdev-properties.h"
@@ -95,7 +96,8 @@ static PowerPCCPU *ppc440_init_xilinx(const char *cpu_type, uint32_t sysclk)
 {
     PowerPCCPU *cpu;
     CPUPPCState *env;
-    qemu_irq *irqs;
+    DeviceState *uicdev;
+    SysBusDevice *uicsbd;
 
     cpu = POWERPC_CPU(cpu_create(cpu_type));
     env = &cpu->env;
@@ -105,10 +107,19 @@ static PowerPCCPU *ppc440_init_xilinx(const char *cpu_type, uint32_t sysclk)
     ppc_dcr_init(env, NULL, NULL);
 
     /* interrupt controller */
-    irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
-    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
-    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
-    ppcuic_init(env, irqs, 0x0C0, 0, 1);
+    uicdev = qdev_new(TYPE_PPC_UIC);
+    uicsbd = SYS_BUS_DEVICE(uicdev);
+
+    object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu),
+                             &error_fatal);
+    sysbus_realize_and_unref(uicsbd, &error_fatal);
+
+    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT,
+                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT]);
+    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT,
+                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT]);
+
+    /* This board doesn't wire anything up to the inputs of the UIC. */
     return cpu;
 }
 
-- 
2.20.1
Re: [PATCH 3/8] hw/ppc/virtex_ml507: Drop use of ppcuic_init()
Posted by Edgar E. Iglesias 5 years, 1 month ago 
On Sat, Dec 12, 2020 at 12:15:32AM +0000, Peter Maydell wrote:
> Switch the virtex_ml507 board to directly creating and
> configuring the UIC, rather than doing it via the old
> ppcuic_init() helper function.
> 
> This fixes a trivial Coverity-detected memory leak where
> we were leaking the array of IRQs returned by ppcuic_init().
> 
> Fixes: Coverity CID 1421992
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> ---
>  hw/ppc/virtex_ml507.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index 7f1bca928c1..34767b11cad 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -43,6 +43,7 @@
>  #include "qemu/option.h"
>  #include "exec/address-spaces.h"
>  
> +#include "hw/intc/ppc-uic.h"
>  #include "hw/ppc/ppc.h"
>  #include "hw/ppc/ppc4xx.h"
>  #include "hw/qdev-properties.h"
> @@ -95,7 +96,8 @@ static PowerPCCPU *ppc440_init_xilinx(const char *cpu_type, uint32_t sysclk)
>  {
>      PowerPCCPU *cpu;
>      CPUPPCState *env;
> -    qemu_irq *irqs;
> +    DeviceState *uicdev;
> +    SysBusDevice *uicsbd;
>  
>      cpu = POWERPC_CPU(cpu_create(cpu_type));
>      env = &cpu->env;
> @@ -105,10 +107,19 @@ static PowerPCCPU *ppc440_init_xilinx(const char *cpu_type, uint32_t sysclk)
>      ppc_dcr_init(env, NULL, NULL);
>  
>      /* interrupt controller */
> -    irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
> -    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
> -    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
> -    ppcuic_init(env, irqs, 0x0C0, 0, 1);
> +    uicdev = qdev_new(TYPE_PPC_UIC);
> +    uicsbd = SYS_BUS_DEVICE(uicdev);
> +
> +    object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu),
> +                             &error_fatal);
> +    sysbus_realize_and_unref(uicsbd, &error_fatal);
> +
> +    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT,
> +                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT]);
> +    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT,
> +                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT]);
> +
> +    /* This board doesn't wire anything up to the inputs of the UIC. */
>      return cpu;
>  }
>  
> -- 
> 2.20.1
>
Re: [PATCH 3/8] hw/ppc/virtex_ml507: Drop use of ppcuic_init()
Posted by David Gibson 5 years, 1 month ago 
On Sat, Dec 12, 2020 at 12:15:32AM +0000, Peter Maydell wrote:
> Switch the virtex_ml507 board to directly creating and
> configuring the UIC, rather than doing it via the old
> ppcuic_init() helper function.
> 
> This fixes a trivial Coverity-detected memory leak where
> we were leaking the array of IRQs returned by ppcuic_init().
> 
> Fixes: Coverity CID 1421992
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Applied to ppc-for-6.0.

> ---
>  hw/ppc/virtex_ml507.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index 7f1bca928c1..34767b11cad 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -43,6 +43,7 @@
>  #include "qemu/option.h"
>  #include "exec/address-spaces.h"
>  
> +#include "hw/intc/ppc-uic.h"
>  #include "hw/ppc/ppc.h"
>  #include "hw/ppc/ppc4xx.h"
>  #include "hw/qdev-properties.h"
> @@ -95,7 +96,8 @@ static PowerPCCPU *ppc440_init_xilinx(const char *cpu_type, uint32_t sysclk)
>  {
>      PowerPCCPU *cpu;
>      CPUPPCState *env;
> -    qemu_irq *irqs;
> +    DeviceState *uicdev;
> +    SysBusDevice *uicsbd;
>  
>      cpu = POWERPC_CPU(cpu_create(cpu_type));
>      env = &cpu->env;
> @@ -105,10 +107,19 @@ static PowerPCCPU *ppc440_init_xilinx(const char *cpu_type, uint32_t sysclk)
>      ppc_dcr_init(env, NULL, NULL);
>  
>      /* interrupt controller */
> -    irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
> -    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
> -    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
> -    ppcuic_init(env, irqs, 0x0C0, 0, 1);
> +    uicdev = qdev_new(TYPE_PPC_UIC);
> +    uicsbd = SYS_BUS_DEVICE(uicdev);
> +
> +    object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu),
> +                             &error_fatal);
> +    sysbus_realize_and_unref(uicsbd, &error_fatal);
> +
> +    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT,
> +                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT]);
> +    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT,
> +                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT]);
> +
> +    /* This board doesn't wire anything up to the inputs of the UIC. */
>      return cpu;
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [PATCH 3/8] hw/ppc/virtex_ml507: Drop use of ppcuic_init()
Posted by BALATON Zoltan 5 years ago 
On Sat, 12 Dec 2020, Peter Maydell wrote:
> Switch the virtex_ml507 board to directly creating and
> configuring the UIC, rather than doing it via the old
> ppcuic_init() helper function.
>
> This fixes a trivial Coverity-detected memory leak where
> we were leaking the array of IRQs returned by ppcuic_init().
>
> Fixes: Coverity CID 1421992
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/ppc/virtex_ml507.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index 7f1bca928c1..34767b11cad 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -43,6 +43,7 @@
> #include "qemu/option.h"
> #include "exec/address-spaces.h"
>
> +#include "hw/intc/ppc-uic.h"
> #include "hw/ppc/ppc.h"
> #include "hw/ppc/ppc4xx.h"
> #include "hw/qdev-properties.h"
> @@ -95,7 +96,8 @@ static PowerPCCPU *ppc440_init_xilinx(const char *cpu_type, uint32_t sysclk)
> {
>     PowerPCCPU *cpu;
>     CPUPPCState *env;
> -    qemu_irq *irqs;
> +    DeviceState *uicdev;
> +    SysBusDevice *uicsbd;
>
>     cpu = POWERPC_CPU(cpu_create(cpu_type));
>     env = &cpu->env;
> @@ -105,10 +107,19 @@ static PowerPCCPU *ppc440_init_xilinx(const char *cpu_type, uint32_t sysclk)
>     ppc_dcr_init(env, NULL, NULL);
>
>     /* interrupt controller */
> -    irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
> -    irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
> -    irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT];
> -    ppcuic_init(env, irqs, 0x0C0, 0, 1);
> +    uicdev = qdev_new(TYPE_PPC_UIC);
> +    uicsbd = SYS_BUS_DEVICE(uicdev);
> +
> +    object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu),
> +                             &error_fatal);
> +    sysbus_realize_and_unref(uicsbd, &error_fatal);

This also uses 0xc0 which is not set. I don't know where's your default 
value comes from but I all these boards seem to use 0xc0 rather than that 
default. So maybe this is better fixed in the UIC dev to change the 
default to 0xc0?

Regards,
BALATON Zoltan

> +
> +    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT,
> +                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT]);
> +    sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT,
> +                       ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT]);
> +
> +    /* This board doesn't wire anything up to the inputs of the UIC. */
>     return cpu;
> }
>
> -- 
> 2.20.1
>
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.