On 11/28/20 3:13 AM, 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.mayd...@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)); > for (i = 0; i < 32; i++) { > irq[i] = qdev_get_gpio_in(dev, i); > } > diff --git a/hw/nios2/cpu_pic.c b/hw/nios2/cpu_pic.c index > 5ea7e52ab83..3fb621c5c85 100644 > --- a/hw/nios2/cpu_pic.c > +++ b/hw/nios2/cpu_pic.c > @@ -26,32 +26,6 @@ > > #include "boot.h" > > -static void nios2_pic_cpu_handler(void *opaque, int irq, int level) -{ > - Nios2CPU *cpu = opaque; > - CPUNios2State *env = &cpu->env; > - CPUState *cs = CPU(cpu); > - int type = irq ? CPU_INTERRUPT_NMI : CPU_INTERRUPT_HARD; > - > - if (type == CPU_INTERRUPT_HARD) { > - env->irq_pending = level; > - > - if (level && (env->regs[CR_STATUS] & CR_STATUS_PIE)) { > - env->irq_pending = 0; > - cpu_interrupt(cs, type); > - } else if (!level) { > - env->irq_pending = 0; > - cpu_reset_interrupt(cs, type); > - } > - } else { > - if (level) { > - cpu_interrupt(cs, type); > - } else { > - cpu_reset_interrupt(cs, type); > - } > - } > -} > - > void nios2_check_interrupts(CPUNios2State *env) { > if (env->irq_pending && > @@ -60,8 +34,3 @@ void nios2_check_interrupts(CPUNios2State *env) > cpu_interrupt(env_cpu(env), CPU_INTERRUPT_HARD); > } > } > - > -qemu_irq *nios2_cpu_pic_init(Nios2CPU *cpu) -{ > - return qemu_allocate_irqs(nios2_pic_cpu_handler, cpu, 2); > -} > diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c index > 8f7011fcb92..4b21e7c6d1c 100644 > --- a/target/nios2/cpu.c > +++ b/target/nios2/cpu.c > @@ -64,6 +64,37 @@ static void nios2_cpu_reset(DeviceState *dev) #endif } > > +#ifndef CONFIG_USER_ONLY > +static void nios2_cpu_set_nmi(void *opaque, int irq, int level) { > + Nios2CPU *cpu = opaque; > + CPUState *cs = CPU(cpu); > + > + if (level) { > + cpu_interrupt(cs, CPU_INTERRUPT_NMI); > + } else { > + cpu_reset_interrupt(cs, CPU_INTERRUPT_NMI); > + } > +} > + > +static void nios2_cpu_set_irq(void *opaque, int irq, int level) { > + Nios2CPU *cpu = opaque; > + CPUNios2State *env = &cpu->env; > + CPUState *cs = CPU(cpu); + + env->irq_pending = level; + + if (level && (env->regs[CR_STATUS] & CR_STATUS_PIE)) { + env->irq_pending = 0; + cpu_interrupt(cs, CPU_INTERRUPT_HARD); + } else if (!level) { + env->irq_pending = 0; + cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD); + } +} +#endif + static void nios2_cpu_initfn(Object *obj) { Nios2CPU *cpu = NIOS2_CPU(obj); @@ -72,6 +103,9 @@ static void nios2_cpu_initfn(Object *obj) #if !defined(CONFIG_USER_ONLY) mmu_init(&cpu->env); + + qdev_init_gpio_in_named(DEVICE(cpu), nios2_cpu_set_nmi, "NMI", 1); + qdev_init_gpio_in_named(DEVICE(cpu), nios2_cpu_set_irq, "IRQ", 1);
The code looks ok to me, and I tested the changes on Zephyr project, it works well. But, according https://www.intel.com/content/dam/www/programmable/us/en/pdfs/literature/hb/nios2/n2sw_nii52006.pdf , The Nios II processor offers two distinct approaches to handling hardware interrupts: ■ The internal interrupt controller (IIC) ■ The external interrupt controller (EIC) interface We have already defined TypeInfo named "altera,iic" , and others can also define EIC, so IMHO I don't think we should replace the internal interrupt controller with GPIO. > #endif > } > > -- > 2.20.1