On Mon, Feb 27, 2023 at 1:57 PM BALATON Zoltan <balaton@eik.bme.hu
<mailto:balaton@eik.bme.hu>> wrote:
On Mon, 27 Feb 2023, BALATON Zoltan wrote:
> On Mon, 27 Feb 2023, BALATON Zoltan wrote:
>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>> Am 26. Februar 2023 23:33:20 UTC schrieb BALATON Zoltan
>>> <balaton@eik.bme.hu <mailto:balaton@eik.bme.hu>>:
>>>> On Sun, 26 Feb 2023, Bernhard Beschow wrote:
>>>>> Am 25. Februar 2023 18:11:49 UTC schrieb BALATON Zoltan
>>>>> <balaton@eik.bme.hu <mailto:balaton@eik.bme.hu>>:
>>>>>> From: Bernhard Beschow <shentey@gmail.com
<mailto:shentey@gmail.com>>
>>>>>>
>>>>>> 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
<mailto:shentey@gmail.com>>
>>>>>> [balaton: declare gpio inputs instead of changing pci bus irqs so it
>>>>>> can
>>>>>> be connected in board code; remove some empty lines]
>>>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu
<mailto:balaton@eik.bme.hu>>
>>>>>> Tested-by: Rene Engel <ReneEngel80@emailn.de
<mailto:ReneEngel80@emailn.de>>
>>>>>> ---
>>>>>> hw/isa/vt82c686.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 39 insertions(+)
>>>>>>
>>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>>>> index 3f9bd0c04d..4025f9bcdc 100644
>>>>>> --- a/hw/isa/vt82c686.c
>>>>>> +++ b/hw/isa/vt82c686.c
>>>>>> @@ -604,6 +604,44 @@ 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);
>>>>>> @@ -614,6 +652,7 @@ static void via_isa_realize(PCIDevice *d, Error
>>>>>> **errp)
>>>>>> int i;
>>>>>>
>>>>>> qdev_init_gpio_out(dev, &s->cpu_intr, 1);
>>>>>> + qdev_init_gpio_in_named(dev, via_isa_set_pci_irq, "pirq",
>>>>>> PCI_NUM_PINS);
>>>>>
>>>>> This line is a Pegasos2 specific addition for fixing its IRQ
handling.
>>>>> Since this code must also work with the Fuloong2e board we should aim
>>>>> for a minimal changeset here which renders this line out of scope.
>>>>>
>>>>> Let's keep the two series separate since now I need to watch two
series
>>>>> for comments. Please use Based-on: tag next time instead.
>>>>
>>>> Well, it's not. It's part of the QDev model for VT8231 that allows it
to
>>>> be connected by boards so I think this belongs here otherwise this
won't
>>>> even compile because the function you've added would be unused and
bail
>>>> on -Werror. Let's not make this more difficult than it is. I'm OK with
>>>> reasonable changes but what's your goal now? You can't get rid of this
>>>> line as it's how QDev can model it. Either I have to call into this
model
>>>> or have to export these pins as gpios.
>>>
>>> Exporting the pins is a separate aspect on top of implementing PCI IRQ
>>> routing. To make this clear and obvious this should be a dedicated
patch.
>>> In case either patch has an issue this would also ease and therefore
>>> acellerate discussions.
>>
>> How would you do that? Introduce via_isa_set_pci_irq() as unused in this
>> patch then have a one line patch to add connecting it to gpio pins? That
>> just makes no sense. This is not modeling the chip with QDev and then
the
>> boatd
>
> This is now modeling...
>
>> can connect it as appropriate modeling the board that the chip is in.
So if
>> fuloon2e needs to do that then it should. I'll check that as I was
focusing
>
> fuloong2e
I've checked fuloong2e and it still works as before. PCI bus is handled by
bonito on that board so your patch would actually break it. The VIA chip
is a PCIDevice. You're not supposed to replace the interrupts of the bus
it's connected to from this model as that should be done by the pci-host
or the board. Therefore modeling the chip's PIRQ/PINT pins as gpios which
is the QDev concept for that is right and your usage of pci_set_irq here
is wrong.
Works for me:
(08/84) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_mips64el_fuloong2e:
PASS(2.77 s)