Re: [PATCH v5 3/7] hw/isa/vt82c686: Implement PCI IRQ routing

[BALATON Zoltan]

On Wed, 1 Mar 2023, Bernhard Beschow wrote:
> Am 1. März 2023 00:17:09 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
> > The real VIA south bridges implement a PCI IRQ router which is configured
> > by the BIOS or the OS. In order to respect these configurations, QEMU
> > needs to implement it as well. The real chip may allow routing IRQs from
> > internal functions independently of PCI interrupts but since guests
> > usually configute it to a single shared interrupt we don't model that
> > here for simplicity.
> >
> > Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
> >
> > Suggested-by: Bernhard Beschow <shentey@gmail.com>
> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > ---
> > hw/isa/vt82c686.c | 38 +++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 37 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> > index 01e0148967..018a119964 100644
> > --- a/hw/isa/vt82c686.c
> > +++ b/hw/isa/vt82c686.c
> > @@ -604,6 +604,42 @@ static void via_isa_request_i8259_irq(void *opaque, int 
> > irq, int level)
> >     qemu_set_irq(s->cpu_intr, level);
> > }
> >
> > +static int via_isa_get_pci_irq(const ViaISAState *s, int irq_num)
> > +{
> > +    switch (irq_num) {
> > +    case 0:
> > +        return s->dev.config[0x55] >> 4;
> > +    case 1:
> > +        return s->dev.config[0x56] & 0xf;
> > +    case 2:
> > +        return s->dev.config[0x56] >> 4;
> > +    case 3:
> > +        return s->dev.config[0x57] >> 4;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static void via_isa_set_pci_irq(void *opaque, int irq_num, int level)
> > +{
> > +    ViaISAState *s = opaque;
> > +    PCIBus *bus = pci_get_bus(&s->dev);
> > +    int i, pic_level, pic_irq = via_isa_get_pci_irq(s, irq_num);
> > +
> > +    if (unlikely(pic_irq == 0 || pic_irq == 2 || pic_irq > 14)) {
>
> Where does the "pic_irq > 14" come from? It's not mentioned in the datasheet.

Check at 0x3c register of USB and AC97 functions. For the others it may be 
valid but unlikely to be used hence we just disallow it. (In my version 
which also mapped IDE here I've checkrf for each source but there's no way 
to do that in this version.)

Regards,
BALATON Zoltan

> > +        return;
> > +    }
> > +
> > +    /* The pic level is the logical OR of all the PCI irqs mapped to it. */
> > +    pic_level = 0;
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> > +        if (pic_irq == via_isa_get_pci_irq(s, i)) {
> > +            pic_level |= pci_bus_get_irq_level(bus, i);
> > +        }
> > +    }
> > +    /* Now we change the pic irq level according to the via irq mappings. */
> > +    qemu_set_irq(s->isa_irqs_in[pic_irq], pic_level);
> > +}
> > +
> > static void via_isa_realize(PCIDevice *d, Error **errp)
> > {
> >     ViaISAState *s = VIA_ISA(d);
> > @@ -615,9 +651,9 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
> >
> >     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
> >     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
> > +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", PCI_NUM_PINS);
> >     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
> >                           errp);
> > -
> >     if (!isa_bus) {
> >         return;
> >     }