diff mbox series

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

Message ID 20201212001537.24520-4-peter.maydell@linaro.org (mailing list archive)
State New, archived
Headers show
Series hw/ppc: Convert UIC device to QOM | expand

Commit Message

Peter Maydell Dec. 12, 2020, 12:15 a.m. UTC
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(-)

Comments

Edgar E. Iglesias Dec. 13, 2020, 2:32 p.m. UTC | #1
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
>
David Gibson Dec. 14, 2020, 5:55 a.m. UTC | #2
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;
>  }
>
BALATON Zoltan Jan. 11, 2021, 7:04 p.m. UTC | #3
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
>
>
>
diff mbox series

Patch

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;
 }