Re: Problems with irq mapping in qemu v5.2

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 23/12/2020 15:21, Philippe Mathieu-Daudé wrote:

> 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(-)

I think the original version of the patch broke fuloong2e, however that should have 
been fixed by my patchset here: 
https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg03936.html. It might be that 
there are multiple regressions located during a full bisect :/


ATB,

Mark.