qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: Guenter Roeck <linux@roeck-us.net>,
	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 18:23:09 +0000	[thread overview]
Message-ID: <5ef852ee-8a53-df9d-82f4-33a68c05f53a@ilande.co.uk> (raw)
In-Reply-To: <3f0f8fc6-6148-a76e-1088-b7882b0bbcaf@roeck-us.net>

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?

> 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.

So far it does seem like these are existing bugs being exposed, but please do report 
back on whether the above suggestions fix the problems you are experiencing.


ATB,

Mark.


  parent reply	other threads:[~2020-12-22 18: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 [this message]
2020-12-22 21:23   ` Guenter Roeck
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=5ef852ee-8a53-df9d-82f4-33a68c05f53a@ilande.co.uk \
    --to=mark.cave-ayland@ilande.co.uk \
    --cc=f4bug@amsat.org \
    --cc=linux@roeck-us.net \
    --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).