qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: BALATON Zoltan via <qemu-devel@nongnu.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: Re: Problems with irq mapping in qemu v5.2
Date: Tue, 22 Dec 2020 23:57:28 +0100 (CET)	[thread overview]
Message-ID: <e2a1818e-366d-8a58-ce-e3eacb6787d7@eik.bme.hu> (raw)
In-Reply-To: <5849da05-a063-cd56-7709-c4760c8aa71f@roeck-us.net>

[-- Attachment #1: Type: text/plain, Size: 6331 bytes --]

On Tue, 22 Dec 2020, Guenter Roeck wrote:
> On 12/22/20 10:23 AM, Mark Cave-Ayland wrote:
>> On 22/12/2020 16:16, Guenter Roeck wrote:
>>
>>> Hi,
>>>
>>> commit 459ca8bfa41 ("pci: Assert irqnum is between 0 and bus->nirqs in
>>> pci_bus_change_irq_level") added sanity checks to the interrupt number passed
>>> to pci_bus_change_irq_level(). That makes sense, given that bus->irq_count
>>> is indexed and sized by the number of interrupts.
>>>
>>> However, as it turns out, the interrupt number passed to this function
>>> is the _mapped_ interrupt number. The result in assertion failures for various
>>> emulations.
>>
>> That doesn't sound quite right. My understanding from the other boards I have been working on is that they use the map_irq() functions recursively so that the final set_irq() is on the physical pin, so it might just be that the assert() is simply exposing an existing bug.
>>
>>> Examples (I don't know if there are others):
>>>
>>> - ppc4xx_pci_map_irq() maps the interrupt number to "slot - 1". Obviously
>>>    that isn't a good thing to do for slot 0, and indeed results in an
>>>    assertion as soon as slot 0 is initialized (presumably that is the root
>>>    bridge). Changing the mapping to "slot" doesn't help because valid slots
>>>    are 0..4, and only four interrupts are allocated.
>>> - pci_bonito_map_irq() changes the mapping all over the place. Whatever
>>>    it does, it returns numbers starting with 32 for slots 5..12. With
>>>    a total number of 32 interrupts, this again results in an assertion
>>>    failure.
>>>
>>> ppc4xx_pci_map_irq() is definitely buggy. I just don't know what the
>>> correct mapping should be. slot  & 3, maybe ?
>>
>> Yeah that doesn't look right. Certainly both the Mac PPC machines use ((pci_dev->devfn >> 3)) & 3) plus the interrupt pin so I think you're right that this is missing an & 3 here. Does adding this allow your image to boot?
>>
>
> Actually, it does not help. This does:
>
> @@ -247,7 +247,7 @@ static int ppc4xx_pci_map_irq(PCIDevice *pci_dev, int irq_num)
>
>     trace_ppc4xx_pci_map_irq(pci_dev->devfn, irq_num, slot);
>
> -    return slot - 1;
> +    return slot ? slot - 1 : slot;
> }
>
> but I have no idea why.
>
>>> I don't really have a good solution for pci_bonito_map_irq(). It may not
>>> matter much - I have not been able to boot fuloong_2e since qemu v4.0,
>>> and afaics that is the only platform using it. Maybe it is just completely
>>> broken ?
>>
>> It looks like you want this patchset posted last week: https://patchew.org/QEMU/20201216022513.89451-1-jiaxun.yang@flygoat.com/ (specifically: https://patchew.org/QEMU/20201216022513.89451-1-jiaxun.yang@flygoat.com/20201216022513.89451-4-jiaxun.yang@flygoat.com/). Zoltan was working on the VIA southbridge wiring at the start of the year and provided me a test case that would boot Linux on the fulong2e machine, so at that point in time it wasn't completely broken.
>>
> Those patches don't help for my tests. Problem is that I try to boot from ide drive.
>
> qemu-system-mips64el -M fulong2e \
>    -kernel vmlinux -no-reboot -m 256 -snapshot \
>    -drive file=rootfs.mipsel.ext3,format=raw,if=ide \
>    -vga none -nographic \
>    --append "root=/dev/sda console=ttyS0"
>    -serial stdio -monitor none
>
> This works just fine with qemu v3.1. With qemu v5.2 (after applying the
> fuloong patch series), I get:
>
> VFS: Cannot open root device "sda" or unknown-block(0,0): error -6
>
> This used to work up to qemu v3.1. Since qemu v4.0, there has been a variety
> of failures. Common denominator is that the ide drive is no longer recognized,
> presumably due to related changes in the via and/or pci code between v3.1
> and v4.0.
>
> Difference in log messages:
>
> v3.1:
>
> pci 0000:00:05.1: [Firmware Bug]: reg 0x10: invalid BAR (can't size)
> pci 0000:00:05.1: [Firmware Bug]: reg 0x14: invalid BAR (can't size)
> pci 0000:00:05.1: [Firmware Bug]: reg 0x18: invalid BAR (can't size)
> pci 0000:00:05.1: reg 0x1c: [mem 0x100000370-0x10000037f 64bit]
> ...
> pata_via 0000:00:05.1: BMDMA: BAR4 is zero, falling back to PIO
> ata1: PATA max PIO4 cmd 0x1f0 ctl 0x3f6 irq 14
> ata2: PATA max PIO4 cmd 0x170 ctl 0x376 irq 15
> ata1.00: ATA-7: QEMU HARDDISK, 2.5+, max UDMA/100
> ...
>
> ----
>
> v5.2:
>
> pci 0000:00:05.1: reg 0x10: [io  0x0000-0x0007]
> pci 0000:00:05.1: reg 0x14: [io  0x0000-0x0003]
> pci 0000:00:05.1: reg 0x18: [io  0x0000-0x0007]
> pci 0000:00:05.1: reg 0x1c: [io  0x0000-0x0003]
> pci 0000:00:05.1: reg 0x20: [io  0x0000-0x000f]
> pci 0000:00:05.1: BAR 4: assigned [io  0x4440-0x444f]
> ...
> ata1: PATA max UDMA/100 cmd 0x1f0 ctl 0x3f6 bmdma 0x4440 irq 14
> ata2: PATA max UDMA/100 cmd 0x170 ctl 0x376 bmdma 0x4448 irq 15
> [and nothing else]

I've already forgot about the details but we have analysed it quite 
throughly back when the via ide changes were made. Here are some random 
pointers to threads that could have some info:
This was the final solution that was merged as the simplest that worked 
for all cases we've tried to fix:
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg03977.html

Before that we went through other solutions that worked too but may not 
have matched hardware enough or something:
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg02324.html
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg02831.html
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg02817.html

Some messages in those threads might explain these but AFAIR the main 
problem was that the IDE controller embedded in the VIA southbridge chip 
can either use PCI IRQ or legacy ISA IRQs and it probably depends on the 
boards and their firmware so Linux and other OSes running on these boards 
have different workarounds to guess what's the correct IRQ routing, which 
wasn't emulated correctly. With the current version we could boot Linux 
kernel known to work on real hardware on both fuloong2e and the yet 
off-branch pegasos2 I'm working on but not having access to the real 
boards we are not 100% sure everything is correct but did seem to work. 
(The tests we've used are also documented in one of those threads.)

This may not help much but that's all I remember now without going through 
the threads again still may give you a place to start.

Regards,
BALATON Zoltan

  reply	other threads:[~2020-12-22 22:59 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-22 16:16 Problems with irq mapping in qemu v5.2 Guenter Roeck
2020-12-22 17:55 ` BALATON Zoltan via
2020-12-22 22:23   ` BALATON Zoltan via
2020-12-22 23:12     ` Guenter Roeck
2020-12-23 10:31     ` Mark Cave-Ayland
2020-12-23 13:39       ` BALATON Zoltan via
2020-12-22 18:23 ` Mark Cave-Ayland
2020-12-22 21:23   ` Guenter Roeck
2020-12-22 22:57     ` BALATON Zoltan via [this message]
2020-12-23  1:01       ` Guenter Roeck
2020-12-23 13:35         ` BALATON Zoltan via
2020-12-23 10:17     ` Mark Cave-Ayland
2020-12-23 10:24     ` Mark Cave-Ayland
2020-12-23 13:17       ` BALATON Zoltan via
2020-12-23 18:15         ` Mark Cave-Ayland
2020-12-25 23:43     ` BALATON Zoltan via
2020-12-31 15:34       ` Peter Maydell
2020-12-23 15:21 ` Philippe Mathieu-Daudé
2020-12-23 16:09   ` Mark Cave-Ayland
2020-12-23 17:01     ` Guenter Roeck
2020-12-23 18:01       ` Mark Cave-Ayland
2020-12-23 20:20       ` BALATON Zoltan via
2020-12-23 21:01         ` Guenter Roeck
2020-12-23 22:05           ` Mark Cave-Ayland
2020-12-23 22:47             ` Guenter Roeck
2020-12-23 23:05               ` Philippe Mathieu-Daudé
2020-12-23 23:56           ` BALATON Zoltan via
2020-12-24  1:34             ` BALATON Zoltan via
2020-12-24  2:29               ` Jiaxun Yang
2020-12-24  5:32               ` Guenter Roeck
2020-12-24  8:11                 ` BALATON Zoltan via
2020-12-24 10:50                   ` Philippe Mathieu-Daudé
2020-12-24 17:09                     ` BALATON Zoltan via
2020-12-28 19:26                   ` Mark Cave-Ayland
2020-12-28 21:18                     ` BALATON Zoltan via
2020-12-23 19:49   ` BALATON Zoltan via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e2a1818e-366d-8a58-ce-e3eacb6787d7@eik.bme.hu \
    --to=qemu-devel@nongnu.org \
    --cc=balaton@eik.bme.hu \
    --cc=f4bug@amsat.org \
    --cc=linux@roeck-us.net \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=mst@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).