On Mon, 21 Mar 2022 at 18:55, Cédric Le Goater <c...@kaod.org> wrote: > I introduced quite a few of these calls, > > hw/ppc/pnv_lpc.c: irqs = qemu_allocate_irqs(handler, lpc, ISA_NUM_IRQS); > hw/ppc/pnv_psi.c: psi->qirqs = qemu_allocate_irqs(ics_set_irq, ics, > ics->nr_irqs); > hw/ppc/pnv_psi.c: psi->qirqs = qemu_allocate_irqs(xive_source_set_irq, > xsrc, xsrc->nr_irqs); > hw/ppc/ppc.c: env->irq_inputs = (void > **)qemu_allocate_irqs(&ppc6xx_set_irq, cpu, > hw/ppc/ppc.c: env->irq_inputs = (void > **)qemu_allocate_irqs(&ppc970_set_irq, cpu, > hw/ppc/ppc.c: env->irq_inputs = (void > **)qemu_allocate_irqs(&power7_set_irq, cpu, > hw/ppc/ppc.c: env->irq_inputs = (void > **)qemu_allocate_irqs(&power9_set_irq, cpu, > hw/ppc/ppc.c: env->irq_inputs = (void > **)qemu_allocate_irqs(&ppc40x_set_irq, > hw/ppc/ppc.c: env->irq_inputs = (void > **)qemu_allocate_irqs(&ppce500_set_irq, > hw/ppc/spapr_irq.c: spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, > spapr, > > and may be I can remove some. What's the best practice ?
The 'best practice' is that if you have an irq line it should be because it is the input (gpio or sysbus irq) or output (gpio) of some device, ie something that is a subtype of TYPE_DEVICE. For the ones in hw/ppc/ppc.c: we used to need to write code like that because CPU objects weren't TYPE_DEVICE; now they are, and so you can give them inbound gpio lines using qdev_init_gpio_in(), typically in the cpu initfn. (See target/riscv for an example, or grep for that function name in target/ for others.) Then the board code needs to wire up to those IRQs in the usual way for GPIO lines, ie using qdev_get_gpio_in(cpudev, ...), instead of directly reaching into the CPU struct env->irq_inputs. (There's probably a way to structure this change to avoid having to change the CPU and all the board code at once, but I haven't thought it through.) For the spapr one: this is in machine model code, and currently machines aren't subtypes of TYPE_DEVICE. I'd leave this one alone for now; we can come back and think about it later. For pnv_psi.c: these appear to be because the PnvPsi device is allocating irq lines which really should belong to the ICSState object (and as a result the ICSState code is having to expose ics->nr_irqs and the ics_set_irq function when they could be internal to the ICSState code). The ICSState's init function should be creating these as qdev gpio lines. pnv_lpc.c seems to be ISA related. hw/isa/lpc_ich9.c is an example of setting up IRQs for isa_bus_irqs() without using qemu_allocate_irqs(), but there may be some more generalised ISA cleanup possible here. thanks -- PMM