Re: [PATCH v8 3/6] hw/isa/vt82c686: Implement PCI IRQ routing

[Mark Cave-Ayland]

On 06/03/2023 12:33, BALATON Zoltan wrote:

> 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>
> Tested-by: Rene Engel <ReneEngel80@emailn.de>
> ---
>   hw/isa/vt82c686.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 8900d87f59..3383ab7e2d 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -600,6 +600,46 @@ void via_isa_set_irq(PCIDevice *d, int n, int level)
>       qemu_set_irq(s->isa_irqs_in[n], 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);
> +
> +    /* IRQ 0: disabled, IRQ 2,8,13: reserved */
> +    if (!pic_irq) {
> +        return;
> +    }
> +    if (unlikely(pic_irq == 2 || pic_irq == 8 || pic_irq == 13)) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "Invalid ISA IRQ routing");
> +    }
> +
> +    /* 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);
> @@ -620,6 +660,8 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>       i8254_pit_init(isa_bus, 0x40, 0, NULL);
>       i8257_dma_init(isa_bus, 0);
>   
> +    qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq", PCI_NUM_PINS);
> +
>       /* RTC */
>       qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
>       if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {

Writing my previous response where I asked about the additional PCI interrupt 
connections to the MV64361, I realised that if you forget about the Northbridge 
component for a moment then things start to make a bit more sense.

At the moment we have 3 different scenarios:

   1) ISA IRQ -> 8259 for all ISA devices

   2) ISA IRQ -> 8259 *OR*
      PCI IRQ -> PCI IRQ router -> 8259 for the PCI-IDE device

   3) PCI IRQ -> PCI IRQ router -> 8259 for PCI devices

When you look at it this way it is easy to see for case 3 that the PCI IRQ routing 
mechanism is handled by *_set_irq() and *_map_irq() callbacks. And so with that in 
mind, re-reading the VIA datasheet I came up with the following PoC for discussion: 
https://github.com/mcayland/qemu/commits/via-poc.

It needs a bit of work, but at first glance it ticks all the boxes in that the PCI 
bus IRQs are all internal to the VIA southbridge (no global via_isa_set_irq() 
function and no overriding of PCI bus IRQs), there are separate legacy and PCI IRQs 
for the via-ide device, and the PCI IRQ routing bears at least a passing resemblance 
to the datasheet.


ATB,

Mark.