Re: [PATCH v2 33/43] hw/intc: Add LoongArch ls7a msi interrupt controller support(PCH-MSI)

[Mark Cave-Ayland]

On 27/04/2022 03:01, Richard Henderson wrote:

> On 4/25/22 02:10, Xiaojuan Yang wrote:
> > +static void loongarch_msi_mem_write(void *opaque, hwaddr addr,
> > +                                    uint64_t val, unsigned size)
> > +{
> > +    LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque);
> > +    int irq_num = val & 0xff;
> > +
> > +    trace_loongarch_msi_set_irq(irq_num);
> > +    qemu_set_irq(s->pch_msi_irq[irq_num - PCH_PIC_IRQ_NUM], 1);
> > +}
>
> This needs to be bounded properly -- irq_num >= PCH_PIC_IRQ_NUM -- since 'val' is 
> under the control of the guest.
>
>
> > +static void pch_msi_irq_handler(void *opaque, int irq, int level)
> > +{
> > +    LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque);
> > +
> > +    qemu_set_irq(s->pch_msi_irq[irq], level);
> > +}
>
> You should be able to connect the gpio lines directly, rather than having a 
> pass-through function like this.  I think this is sysbus_pass_irq.

The *_pass() functions don't work for this case because they directly alias the gpios 
onto another device i.e. an input still remains an input on the target device, rather 
than becoming an output. So I think this is still required here.

However your comment above about PCH_PIC_IRQ_NUM has made me realise that it is 
possible to improve the wiring of the IRQs here, so I'll follow up with replies on a 
few more patches separately.


ATB,

Mark.