Re: [PATCH 2/5] hw/isa/vt82c686: Implement PCI IRQ routing

[Bernhard Beschow]

Am 23. Februar 2023 21:11:23 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Thu, 23 Feb 2023, Bernhard Beschow 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.
>> 
>> Note: The implementation was taken from piix4_set_irq() in hw/isa/piix4.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/isa/vt82c686.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 44 insertions(+)
>> 
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index 3f9bd0c04d..f24e387d63 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -604,6 +604,48 @@ 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 pic_irq;
>> +
>> +    /* now we change the pic irq level according to the via irq mappings */
>> +    /* XXX: optimize */
>> +    pic_irq = via_isa_get_pci_irq(s, irq_num);
>> +    if (pic_irq < ISA_NUM_IRQS) {
>> +        int i, pic_level;
>> +
>> +        /* 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);
>> +            }
>> +        }
>> +        qemu_set_irq(s->isa_irqs[pic_irq], pic_level);
>> +    }
>> +}
>> +
>> static void via_isa_realize(PCIDevice *d, Error **errp)
>> {
>>     ViaISAState *s = VIA_ISA(d);
>> @@ -676,6 +718,8 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>     if (!qdev_realize(DEVICE(&s->mc97), BUS(pci_bus), errp)) {
>>         return;
>>     }
>> +
>> +    pci_bus_irqs(pci_bus, via_isa_set_pci_irq, s, PCI_NUM_PINS);
>
>Please no oversimplification. This replaces the connections to mv64361 gpp 
>pins made in mv64361_realize() and breaks the interrupts that can be enabled 
>in mv64361.

Let's split our work as follows: You'll do the audio and pegasos2 changes 
including exporting the pirq properties, I'll do the first three routing 
patches of this series as the base.

> I've implemented that for something but can't remember now which guest 
> exactly,

Could you please put this information into the commit message or into the code? 
That would ease maintainance a lot.

> but this would be needed so please restore my pegasos2 patch and move this 
> there connecting both mv64361 and via-isa to PCI interrupts as shown in the 
> schematics. That means you also need the PIRQ pins here. Can you do a new 
> version with that?

As proposed above I'd fold the first three patches into a separate series which 
you can build upon. I have no way to test the pegasos2 IRQ changes since the 
test cases I'm aware of either work or we agreed that they can be fixed later 
(-> USB).

> I'll try this one in the meantime

Sounds good to me -- that's what I wanted to achieve ;) Thanks!

Best regards,
Bernhard

> but I'm quite sure this is wrong as it is. You can drop the via-ac97 patches 
> from this series, I can submit them separately rebased on the final IRQ 
> series we come up with.
>
>Regards,
>BALATON Zoltan
>
>> }
>> 
>> /* TYPE_VT82C686B_ISA */
>>