qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	QEMU Developers <qemu-devel@nongnu.org>
Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: Problems with irq mapping in qemu v5.2
Date: Tue, 22 Dec 2020 13:23:48 -0800	[thread overview]
Message-ID: <5849da05-a063-cd56-7709-c4760c8aa71f@roeck-us.net> (raw)
In-Reply-To: <5ef852ee-8a53-df9d-82f4-33a68c05f53a@ilande.co.uk>

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]

Guenter


  reply	other threads:[~2020-12-22 21:24 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 [this message]
2020-12-22 22:57     ` BALATON Zoltan via
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=5849da05-a063-cd56-7709-c4760c8aa71f@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=f4bug@amsat.org \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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).