Re: Problems with irq mapping in qemu v5.2 - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: Guenter Roeck <linux@roeck-us.net>,
	QEMU Developers <qemu-devel@nongnu.org>,
	BALATON Zoltan <balaton@eik.bme.hu>, John Snow <jsnow@redhat.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: Problems with irq mapping in qemu v5.2
Date: Wed, 23 Dec 2020 16:21:37 +0100	[thread overview]
Message-ID: <e90b9df0-5173-6e2b-3572-4f21ac318231@amsat.org> (raw)
In-Reply-To: <3f0f8fc6-6148-a76e-1088-b7882b0bbcaf@roeck-us.net>

On 12/22/20 5:16 PM, 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.
> 
> 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 ?
> 
> 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 ?

FWIW bisecting Fuloong2E starts failing here:

4ea98d317eb442c738f898f16cfdd47a18b7ca49 is the first bad commit
commit 4ea98d317eb442c738f898f16cfdd47a18b7ca49
Author: BALATON Zoltan <balaton@eik.bme.hu>
Date:   Fri Jan 25 14:52:12 2019 -0500

    ide/via: Implement and use native PCI IDE mode

    This device only implemented ISA compatibility mode and native PCI IDE
    mode was missing but no clients actually need ISA mode but to the
    contrary, they usually want to switch to and use device in native
    PCI IDE mode. Therefore implement native PCI mode and switch default
    to that.

    Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
    Message-id:
c323f08c59b9931310c5d92503d370f77ce3a557.1548160772.git.balaton@eik.bme.hu
    Signed-off-by: John Snow <jsnow@redhat.com>

 hw/ide/via.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 14 deletions(-)