Am 23. Februar 2023 23:47:58 UTC schrieb BALATON Zoltan <bala...@eik.bme.hu>:
>On Thu, 23 Feb 2023, Bernhard Beschow wrote:
>> Am 23. Februar 2023 21:11:23 UTC schrieb BALATON Zoltan <bala...@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 <shen...@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'm OK with doing audio as said and already did the PIRQ and pegaosos2 changes
>(patch 2 and 3 in my series), just take those without deleting half of them.
I can only take the three VT82xx patches as I proposed since I don't know the
Pegasos2 board as well as you do and I don't want to iterate on any review
comments for the other patches. I'll send my series soonish.
Best regards,
Bernhard
>So drop the last two via-ac97 patches and do the IRQ fixes in your way but
>keep working what now works.
>
>>> 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.
>
>I did, see patch 3 in my series.
>
>>> 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 did fix the USB just haven't sent a v2 yet due to this thread but it's just
>the change I've sent yesterday, just add this before qemu_set_irq at the end
>of via_isa_set_irq() in my series. This is what I have now:
>
>+ uint16_t old_state;
>+ if (old_state && s->isa_irq_state[isa_irq]) {
>+ /* FIXME: i8259 model does not support level sensitive mode */
>+ qemu_set_irq(s->isa_irqs[isa_irq], 0);
>+ }
>
>How to do that with your version I have no idea but this fixed the problem
>with my series. I can send a v2 of this patch with this change if it's not
>clear from this and the other message what I did.
>
>>> I'll try this one in the meantime
>>
>> Sounds good to me -- that's what I wanted to achieve ;) Thanks!
>
>I've answered about that in the other message, I've tried with AmigaOS, Debian
>Linux 8.11.0 netboot iso and MorphOS and they still boot but couldn't test
>them much yet. MorphOS works on my series with sound and USB and does not hang
>with the above workaround but found now it still hangs if I send something to
>it over serial (e.g. press space or enter where you've typed boot cd boot.img
>after it starts playing sound). This happens on both of our series but only
>with the via-ac97 patch so probably related to that. This could easily be a
>guest bug too so I don't care that much, the pegasos2 changes are more
>interesting to get AmigaOS run well so that's my main focus now, MorphOS
>already runs on other QEMU machines well. I'll still try to find this out but
>AmigaOS can use other sound card so as long as the IRQ problems are fixed it
>would work but we need more than one PCI cards working as we'd need sound card
>and network card for it to be usable. This was tested to work with my series,
>if you give alternative series I can ask to have those tested too.
>
>Regards,
>BALATON Zoltan