Re: [PATCH 1/2] target/nios2: Move cpu_pic code into CPU object proper

[Philippe Mathieu-Daudé]

On 11/27/20 8:12 PM, Peter Maydell wrote:
> The nios2 code uses an old style of interrupt handling, where a
> separate standalone set of qemu_irqs invoke a function
> nios2_pic_cpu_handler() which signals the interrupt to the CPU proper
> by directly calling cpu_interrupt() and cpu_reset_interrupt().
> Because CPU objects now inherit (indirectly) from TYPE_DEVICE, they
> can have GPIO input lines themselves, and the neater modern way to
> implement this is to simply have the CPU object itself provide the
> input IRQ lines.
> 
> Create named "NMI" and "IRQ" GPIO inputs to the Nios2 CPU object, and
> make the only user of nios2_cpu_pic_init() wire up directly to those
> instead.
> 
> This fixes a Coverity-reported trivial memory leak of the IRQ array
> allocated in nios2_cpu_pic_init().
> 
> Fixes: Coverity CID 1421916
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/nios2/cpu.h        |  1 -
>  hw/nios2/10m50_devboard.c |  8 +++-----
>  hw/nios2/cpu_pic.c        | 31 -------------------------------
>  target/nios2/cpu.c        | 34 ++++++++++++++++++++++++++++++++++
>  4 files changed, 37 insertions(+), 37 deletions(-)
> 
> diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
> index 86bbe1d8670..b7efb54ba7e 100644
> --- a/target/nios2/cpu.h
> +++ b/target/nios2/cpu.h
> @@ -201,7 +201,6 @@ void nios2_cpu_do_unaligned_access(CPUState *cpu, vaddr 
> addr,
>                                     MMUAccessType access_type,
>                                     int mmu_idx, uintptr_t retaddr);
>  
> -qemu_irq *nios2_cpu_pic_init(Nios2CPU *cpu);
>  void nios2_check_interrupts(CPUNios2State *env);
>  
>  void do_nios2_semihosting(CPUNios2State *env);
> diff --git a/hw/nios2/10m50_devboard.c b/hw/nios2/10m50_devboard.c
> index 5c13b74306f..ac1993e8c08 100644
> --- a/hw/nios2/10m50_devboard.c
> +++ b/hw/nios2/10m50_devboard.c
> @@ -52,7 +52,7 @@ static void nios2_10m50_ghrd_init(MachineState *machine)
>      ram_addr_t tcm_size = 0x1000;    /* 1 kiB, but QEMU limit is 4 kiB */
>      ram_addr_t ram_base = 0x08000000;
>      ram_addr_t ram_size = 0x08000000;
> -    qemu_irq *cpu_irq, irq[32];
> +    qemu_irq irq[32];
>      int i;
>  
>      /* Physical TCM (tb_ram_1k) with alias at 0xc0000000 */
> @@ -76,14 +76,12 @@ static void nios2_10m50_ghrd_init(MachineState *machine)
>      /* Create CPU -- FIXME */
>      cpu = NIOS2_CPU(cpu_create(TYPE_NIOS2_CPU));
>  
> -    /* Register: CPU interrupt controller (PIC) */
> -    cpu_irq = nios2_cpu_pic_init(cpu);
> -
>      /* Register: Internal Interrupt Controller (IIC) */
>      dev = qdev_new("altera,iic");
>      object_property_add_const_link(OBJECT(dev), "cpu", OBJECT(cpu));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> -    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, cpu_irq[0]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> +                       qdev_get_gpio_in_named(DEVICE(cpu), "IRQ", 0));

Ah, NMI is never used.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>      for (i = 0; i < 32; i++) {
>          irq[i] = qdev_get_gpio_in(dev, i);
>      }
...