Re: [PATCH v2 2/3] hw/mips/mips_int: Use qdev gpio rather than qemu_allocate_irqs()

[Philippe Mathieu-Daudé]

On 5/14/20 3:24 PM, Peter Maydell wrote:
> On Tue, 12 May 2020 at 08:48, Philippe Mathieu-Daudé <address@hidden> wrote:
> > Switch to using the qdev gpio API which is preferred over
> > qemu_allocate_irqs(). One step to eventually deprecate and
> > remove qemu_allocate_irqs() one day.
>
> > diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
> > index 796730b11d..91788c51a9 100644
> > --- a/hw/mips/mips_int.c
> > +++ b/hw/mips/mips_int.c
> > @@ -74,14 +74,12 @@ static void cpu_mips_irq_request(void *opaque, int irq, int 
> > level)
> >   void cpu_mips_irq_init_cpu(MIPSCPU *cpu)
> >   {
> >       CPUMIPSState *env = &cpu->env;
> > -    qemu_irq *qi;
> >       int i;
> >
> > -    qi = qemu_allocate_irqs(cpu_mips_irq_request, cpu, 8);
> > +    qdev_init_gpio_in(DEVICE(cpu), cpu_mips_irq_request, 8);
> >       for (i = 0; i < 8; i++) {
> > -        env->irq[i] = qi[i];
> > +        env->irq[i] = qdev_get_gpio_in(DEVICE(cpu), i);
> >       }
> > -    g_free(qi);
> >   }
>
> Similar comments as with the openrisc patch apply here:
>   * ideally this code should be in target/mips/, not in hw/mips
>   * board code should call qdev_get_gpio_in() to get the IRQ
>     line, rather than fishing env->irq[] out of the CPU object
>     directly
> This is a bit more complicated than with openrisc, because there's
> more than just a single board model, and not all MIPS boards call
> cpu_mips_irq_init_cpu() so figuring out whether MIPS CPUs should
> always provide inbound CPU lines, or only those with some
> particular feature, or something else, would need some
> investigation or MIPS knowledge.

Yes, I'm aware of at least 3 different to map interrupts to a MIPS core.
QEMU models at least 2.

As X86, MIPS code use old QEMU API, I don't see the codebase being 
upgraded anytime soon.

This is another borderline case between architecture and hardware, as 
the cache units, or the ARM NVIC. Ideally we should keep target/* free 
of references to hw/* code. In my experience it often give troubles.

> But this is an OK first
> step anyway, so
>
> Reviewed-by: Peter Maydell <address@hidden>

Thanks. The idea to keep qemu_allocate_irqs() as internal as possible, 
and have devices use the qdev GPIO API.

> thanks
> -- PMM