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

[Bernhard Beschow]

Am 1. März 2023 11:15:02 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>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.)

I'm not sure what you mean. The 0x3c regs aren't related to the PCI IRQ routing 
regs.

Moreover, as I wrote in my other mail, I wonder what the datasheet tries to 
tell us here at all. The information there partly contradicts itself.

Can you please clarify?

Thanks,
Bernhard

>
>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;
>>>     }
>> 
>>