All of lore.kernel.org
 help / color / mirror / Atom feed
* Problems with irq mapping in qemu v5.2
@ 2020-12-22 16:16 Guenter Roeck
  2020-12-22 17:55 ` BALATON Zoltan via
                   ` (2 more replies)
  0 siblings, 3 replies; 36+ messages in thread
From: Guenter Roeck @ 2020-12-22 16:16 UTC (permalink / raw)
  To: QEMU Developers
  Cc: Mark Cave-Ayland, Philippe Mathieu-Daudé, Michael S. Tsirkin

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 ?

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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 18:23 ` Mark Cave-Ayland
  2020-12-23 15:21 ` Philippe Mathieu-Daudé
  2 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-22 17:55 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: qemu-ppc, Michael S. Tsirkin, Mark Cave-Ayland, QEMU Developers,
	Philippe Mathieu-Daudé

Hello,

On Tue, 22 Dec 2020, 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.

Is that with sam460ex? This ppc4xx_pci_map_irq appears in 
ppc4xx-host-bridge and hw/ppc/ppc440_pcix.c (which is used by sam460ex) 
has a ppc4xx-host-bridge. Other user of it may be the bamboo 440 board 
where this host bridge appears too. I did not see asserts with sam460ex 
but I've only tested it with AmigaOS which may use it in an odd way not 
tripping the check.

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

The current irq mapping may not match real hardware but unfortunately I 
don't know what real hardware does, neither for sam460ex nor for bamboo. 
I've implemented ppc440_pcix mostly by guessing and doing what made 
clients happy because I could not get docs for that PCI implementation so 
it's possible there are some bugs left. Since these are IBM parts (PCI 
parts of 440 SoCs) maybe there's some info available from them (but this 
is likely board specific) or someone may have some info here. I'm adding 
the qemu-ppc list in any case, hoping someone knowing PPC440 SoC might 
read that list. Also the u-boot source of the sam460ex firmware may have 
some info if somebody knows where to look for it in there.

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

There were some patches in the past few days on the list improving 
fuloong2e emulation so that the board's firmware can be used, but I don't 
know if any of those are relevant. Philippe may know more about that.

Regards,
BALATON Zoltan


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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 18:23 ` Mark Cave-Ayland
  2020-12-22 21:23   ` Guenter Roeck
  2020-12-23 15:21 ` Philippe Mathieu-Daudé
  2 siblings, 1 reply; 36+ messages in thread
From: Mark Cave-Ayland @ 2020-12-22 18:23 UTC (permalink / raw)
  To: Guenter Roeck, QEMU Developers
  Cc: Philippe Mathieu-Daudé, Michael S. Tsirkin

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.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-22 18:23 ` Mark Cave-Ayland
@ 2020-12-22 21:23   ` Guenter Roeck
  2020-12-22 22:57     ` BALATON Zoltan via
                       ` (3 more replies)
  0 siblings, 4 replies; 36+ messages in thread
From: Guenter Roeck @ 2020-12-22 21:23 UTC (permalink / raw)
  To: Mark Cave-Ayland, QEMU Developers
  Cc: Philippe Mathieu-Daudé, Michael S. Tsirkin

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


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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
  0 siblings, 2 replies; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-22 22:23 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Philippe Mathieu-Daudé,
	qemu-ppc, QEMU Developers, Michael S. Tsirkin

On Tue, 22 Dec 2020, BALATON Zoltan via wrote:
> Hello,
>
> On Tue, 22 Dec 2020, 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.
>
> Is that with sam460ex? This ppc4xx_pci_map_irq appears in ppc4xx-host-bridge 
> and hw/ppc/ppc440_pcix.c (which is used by sam460ex) has a 
> ppc4xx-host-bridge. Other user of it may be the bamboo 440 board where this 
> host bridge appears too. I did not see asserts with sam460ex but I've only 
> tested it with AmigaOS which may use it in an odd way not tripping the check.

I've just remembered that for sam460ex we had this commit: 484ab3dffadc 
(sam460ex: Fix PCI interrupts with multiple devices) that changed that 
mapping for that machine so I guess you got the exception with the bamboo 
board then. I'm not sure though that similar fix is applicable fot that or 
even that this fix is correct for sam460ex but appears to work so far.

Regards,
BALATON Zoltan

>> - 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 ?
>
> The current irq mapping may not match real hardware but unfortunately I don't 
> know what real hardware does, neither for sam460ex nor for bamboo. I've 
> implemented ppc440_pcix mostly by guessing and doing what made clients happy 
> because I could not get docs for that PCI implementation so it's possible 
> there are some bugs left. Since these are IBM parts (PCI parts of 440 SoCs) 
> maybe there's some info available from them (but this is likely board 
> specific) or someone may have some info here. I'm adding the qemu-ppc list in 
> any case, hoping someone knowing PPC440 SoC might read that list. Also the 
> u-boot source of the sam460ex firmware may have some info if somebody knows 
> where to look for it in there.
>
>> 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 ?
>
> There were some patches in the past few days on the list improving fuloong2e 
> emulation so that the board's firmware can be used, but I don't know if any 
> of those are relevant. Philippe may know more about that.
>
> Regards,
> BALATON Zoltan
>
>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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 10:17     ` Mark Cave-Ayland
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-22 22:57 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Michael S. Tsirkin, Mark Cave-Ayland, QEMU Developers,
	Philippe Mathieu-Daudé

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-22 22:23   ` BALATON Zoltan via
@ 2020-12-22 23:12     ` Guenter Roeck
  2020-12-23 10:31     ` Mark Cave-Ayland
  1 sibling, 0 replies; 36+ messages in thread
From: Guenter Roeck @ 2020-12-22 23:12 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: Philippe Mathieu-Daudé,
	qemu-ppc, QEMU Developers, Michael S. Tsirkin

On 12/22/20 2:23 PM, BALATON Zoltan wrote:
> On Tue, 22 Dec 2020, BALATON Zoltan via wrote:
>> Hello,
>>
>> On Tue, 22 Dec 2020, 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.
>>
>> Is that with sam460ex? This ppc4xx_pci_map_irq appears in ppc4xx-host-bridge and hw/ppc/ppc440_pcix.c (which is used by sam460ex) has a ppc4xx-host-bridge. Other user of it may be the bamboo 440 board where this host bridge appears too. I did not see asserts with sam460ex but I've only tested it with AmigaOS which may use it in an odd way not tripping the check.
> 
> I've just remembered that for sam460ex we had this commit: 484ab3dffadc (sam460ex: Fix PCI interrupts with multiple devices) that changed that mapping for that machine so I guess you got the exception with the bamboo board then. I'm not sure though that similar fix is applicable fot that or even that this fix is correct for sam460ex but appears to work so far.
> 
Yes, it is for bamboo. I think I'll just keep the mapping I came up with;
that seems to work for me.

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-22 22:57     ` BALATON Zoltan via
@ 2020-12-23  1:01       ` Guenter Roeck
  2020-12-23 13:35         ` BALATON Zoltan via
  0 siblings, 1 reply; 36+ messages in thread
From: Guenter Roeck @ 2020-12-23  1:01 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: Michael S. Tsirkin, Mark Cave-Ayland, QEMU Developers,
	Philippe Mathieu-Daudé

On 12/22/20 2:57 PM, BALATON Zoltan wrote:

[ ... ]

> 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
> 
This set of patches is in v5.2, yet I can still not instantiate an IDE drive, at least
not with "-drive file=<filename>,if=ide". Is there some other means to instantiate
an ide drive with the fuloong2e emulation ?

For reference, here is my qemu command line:

qemu-system-mips64el -M fulong2e \
    -kernel vmlinux -no-reboot -m 256 \
    -snapshot -drive file=rootfs.mipsel.ext3,format=raw,if=ide \
    -vga none \
    --append "root=/dev/sda console=ttyS0" \
    -nographic -serial stdio -monitor none

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-22 21:23   ` Guenter Roeck
  2020-12-22 22:57     ` BALATON Zoltan via
@ 2020-12-23 10:17     ` Mark Cave-Ayland
  2020-12-23 10:24     ` Mark Cave-Ayland
  2020-12-25 23:43     ` BALATON Zoltan via
  3 siblings, 0 replies; 36+ messages in thread
From: Mark Cave-Ayland @ 2020-12-23 10:17 UTC (permalink / raw)
  To: Guenter Roeck, QEMU Developers
  Cc: Philippe Mathieu-Daudé, Michael S. Tsirkin

On 22/12/2020 21:23, Guenter Roeck wrote:

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

That's interesting. I had a look at bamboo.dts in Linux and the interrupt-map 
property suggests there are 4 fixed PCI slots available 1 to 4, so it shouldn't be 
possible for slot 0 to generate an IRQ as nothing can be plugged there.

Can you share your reproducer? Feel free to send me links off-list if you prefer.

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

Again, can you share the files that you are using here? I think it's worth adding 
Jiaxun Yang to this thread since the original cover letter at 
https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg04293.html states that this 
latest series allows the Debian installer to boot.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-22 21:23   ` Guenter Roeck
  2020-12-22 22:57     ` 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-25 23:43     ` BALATON Zoltan via
  3 siblings, 1 reply; 36+ messages in thread
From: Mark Cave-Ayland @ 2020-12-23 10:24 UTC (permalink / raw)
  To: Guenter Roeck, QEMU Developers
  Cc: Philippe Mathieu-Daudé, Michael S. Tsirkin

On 22/12/2020 21:23, Guenter Roeck wrote:

(Added jiaxun.yang@flygoat.com as CC)

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

Jiaxun: Guenter is reporting that even with your latest series at 
https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg04293.html he is unable to 
boot from an IDE drive. Your cover letter suggests that it should be possible to boot 
the Debian installer: can you provide any insight here?


ATB,

Mark.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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
  1 sibling, 1 reply; 36+ messages in thread
From: Mark Cave-Ayland @ 2020-12-23 10:31 UTC (permalink / raw)
  To: BALATON Zoltan, Guenter Roeck
  Cc: Michael S. Tsirkin, qemu-ppc, Philippe Mathieu-Daudé,
	QEMU Developers

On 22/12/2020 22:23, BALATON Zoltan via wrote:

> I've just remembered that for sam460ex we had this commit: 484ab3dffadc (sam460ex: 
> Fix PCI interrupts with multiple devices) that changed that mapping for that machine 
> so I guess you got the exception with the bamboo board then. I'm not sure though that 
> similar fix is applicable fot that or even that this fix is correct for sam460ex but 
> appears to work so far.

FWIW you might want to review this commit: as Peter noticed it is possible to lose 
interrupts here since if one PCI interrupt is already asserted and then another comes 
along, the second PCI interrupt will unintentionally clear the first which could 
cause problems.

You probably want to keep the 4 separate PCI interrupts but feed them into an OR IRQ, 
the output of which gets fed into the UIC. Have a look at 
https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg05503.html for the sun4m 
variant of this based upon Peter's original example.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-23 10:24     ` Mark Cave-Ayland
@ 2020-12-23 13:17       ` BALATON Zoltan via
  2020-12-23 18:15         ` Mark Cave-Ayland
  0 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-23 13:17 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: Michael S. Tsirkin, QEMU Developers, Guenter Roeck,
	Philippe Mathieu-Daudé

On Wed, 23 Dec 2020, Mark Cave-Ayland wrote:
> On 22/12/2020 21:23, Guenter Roeck wrote:
>
> (Added jiaxun.yang@flygoat.com as CC)

Are you sure? It does not show up on cc list for me so unless the list ate 
it you might have forgotten to copy the address there. Done now just in 
case, sorry if this resulted in double post.

Regards,
BALATON Zoltan

>>>> 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
>
> Jiaxun: Guenter is reporting that even with your latest series at 
> https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg04293.html he is 
> unable to boot from an IDE drive. Your cover letter suggests that it should 
> be possible to boot the Debian installer: can you provide any insight here?
>
>
> ATB,
>
> Mark.
>
>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-23  1:01       ` Guenter Roeck
@ 2020-12-23 13:35         ` BALATON Zoltan via
  0 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-23 13:35 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Michael S. Tsirkin, Mark Cave-Ayland, QEMU Developers,
	Philippe Mathieu-Daudé

On Tue, 22 Dec 2020, Guenter Roeck wrote:
> On 12/22/20 2:57 PM, BALATON Zoltan wrote:
>
> [ ... ]
>
>> 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
>>
> This set of patches is in v5.2, yet I can still not instantiate an IDE drive, at least
> not with "-drive file=<filename>,if=ide". Is there some other means to instantiate
> an ide drive with the fuloong2e emulation ?
>
> For reference, here is my qemu command line:
>
> qemu-system-mips64el -M fulong2e \
>    -kernel vmlinux -no-reboot -m 256 \
>    -snapshot -drive file=rootfs.mipsel.ext3,format=raw,if=ide \
>    -vga none \
>    --append "root=/dev/sda console=ttyS0" \
>    -nographic -serial stdio -monitor none

This is what worked the last time we've tried:

https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04086.html

and this was my understanding of the issues with this VIA IDE function of 
these integrated southbridge/superio chips and Linux's approach to them 
that may be relevant to debug this:

https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg00019.html

We've found that pegasos2 PPC board uses this "half-native" mode where 
IRQs are using ISA IRQs but not sure what the fuloong2e has. Linux did not 
seem to care as long as there's no mismatch between config bits and IRQ 
routing. Although we've also found some comments in Linux may be 
misleading:

https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg02349.html

For the command line I never know which of these work for which machines 
so I just avoid using -drive if=... If the machine has 4 IDE ports then 
-hda -hdb -cdrom usually works, otherwise the most verbose form should 
always work which is:

-drive if=none,id=cd,file=some.iso,format=raw \
-device ide-cd,drive=cd,bus=ide.1

and similar for HD image with ide-hd and bus=ide.0 or wherever you want to 
connect it.

Regards,
BALATON Zoltan


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-23 10:31     ` Mark Cave-Ayland
@ 2020-12-23 13:39       ` BALATON Zoltan via
  0 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-23 13:39 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: QEMU Developers, qemu-ppc, Philippe Mathieu-Daudé,
	Guenter Roeck, Michael S. Tsirkin

On Wed, 23 Dec 2020, Mark Cave-Ayland wrote:
> On 22/12/2020 22:23, BALATON Zoltan via wrote:
>> I've just remembered that for sam460ex we had this commit: 484ab3dffadc 
>> (sam460ex: Fix PCI interrupts with multiple devices) that changed that 
>> mapping for that machine so I guess you got the exception with the bamboo 
>> board then. I'm not sure though that similar fix is applicable fot that or 
>> even that this fix is correct for sam460ex but appears to work so far.
>
> FWIW you might want to review this commit: as Peter noticed it is possible to 
> lose interrupts here since if one PCI interrupt is already asserted and then 
> another comes along, the second PCI interrupt will unintentionally clear the 
> first which could cause problems.

Yes, thanks for reminding. I think this was also raised back then but for 
some reason we thought this should not be an issue in this case but I'll 
give it a try and see if using an or-gate would be better. The only 
possible IRQ related problem I know about is choppy sound with an emulated 
PCI sound card but that may be because some other audio related problem as 
well.

Regards,
BALATON Zoltan

> You probably want to keep the 4 separate PCI interrupts but feed them into an 
> OR IRQ, the output of which gets fed into the UIC. Have a look at 
> https://lists.gnu.org/archive/html/qemu-devel/2020-12/msg05503.html for the 
> sun4m variant of this based upon Peter's original example.
>
>
> ATB,
>
> Mark.
>
>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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 18:23 ` Mark Cave-Ayland
@ 2020-12-23 15:21 ` Philippe Mathieu-Daudé
  2020-12-23 16:09   ` Mark Cave-Ayland
  2020-12-23 19:49   ` BALATON Zoltan via
  2 siblings, 2 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-23 15:21 UTC (permalink / raw)
  To: Guenter Roeck, QEMU Developers, BALATON Zoltan, John Snow
  Cc: Mark Cave-Ayland, Michael S. Tsirkin

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


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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 19:49   ` BALATON Zoltan via
  1 sibling, 1 reply; 36+ messages in thread
From: Mark Cave-Ayland @ 2020-12-23 16:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé,
	Guenter Roeck, QEMU Developers, BALATON Zoltan, John Snow
  Cc: Michael S. Tsirkin

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.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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
  0 siblings, 2 replies; 36+ messages in thread
From: Guenter Roeck @ 2020-12-23 17:01 UTC (permalink / raw)
  To: Mark Cave-Ayland, Philippe Mathieu-Daudé,
	QEMU Developers, BALATON Zoltan, John Snow
  Cc: Michael S. Tsirkin

On 12/23/20 8:09 AM, Mark Cave-Ayland wrote:
> 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 :/
> 

Not really. The following patch on top of qemu 5.2 results in the ide drive
being detected and working.

diff --git a/hw/ide/via.c b/hw/ide/via.c
index be09912b33..1bfdc422ee 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -186,11 +186,14 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
     pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);

     bmdma_setup_bar(d);
+#if 0
     pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+#endif

     qdev_init_gpio_in(ds, via_ide_set_irq, 2);
     for (i = 0; i < 2; i++) {
         ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
+        ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0, i ? 0x376 : 0x3f6);
         ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));

         bmdma_init(&d->bus[i], &d->bmdma[i], d);

With the added ide_init_ioport(), the drive is detected. With the #if 0,
it actually starts working. So there are two problems: 1) The qemu ide
subsystem isn't informed about the io addresses, and 2) bmdma isn't working.

Guenter


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-23 17:01     ` Guenter Roeck
@ 2020-12-23 18:01       ` Mark Cave-Ayland
  2020-12-23 20:20       ` BALATON Zoltan via
  1 sibling, 0 replies; 36+ messages in thread
From: Mark Cave-Ayland @ 2020-12-23 18:01 UTC (permalink / raw)
  To: Guenter Roeck, Philippe Mathieu-Daudé,
	QEMU Developers, BALATON Zoltan, John Snow, Jiaxun Yang
  Cc: Michael S. Tsirkin

On 23/12/2020 17:01, Guenter Roeck wrote:

> On 12/23/20 8:09 AM, Mark Cave-Ayland wrote:
>> 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 :/
>>
> 
> Not really. The following patch on top of qemu 5.2 results in the ide drive
> being detected and working.
> 
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index be09912b33..1bfdc422ee 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -186,11 +186,14 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>       pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
> 
>       bmdma_setup_bar(d);
> +#if 0
>       pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
> +#endif
> 
>       qdev_init_gpio_in(ds, via_ide_set_irq, 2);
>       for (i = 0; i < 2; i++) {
>           ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
> +        ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0, i ? 0x376 : 0x3f6);
>           ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
> 
>           bmdma_init(&d->bus[i], &d->bmdma[i], d);
> 
> With the added ide_init_ioport(), the drive is detected. With the #if 0,
> it actually starts working. So there are two problems: 1) The qemu ide
> subsystem isn't informed about the io addresses, and 2) bmdma isn't working.

Interesting: that's basically disabling BMDMA and switching back to use legacy IDE 
again. Regardless of this it seems I need a better test case for this, and some input 
from people who know the hardware better :(


ATB,

Mark.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-23 13:17       ` BALATON Zoltan via
@ 2020-12-23 18:15         ` Mark Cave-Ayland
  0 siblings, 0 replies; 36+ messages in thread
From: Mark Cave-Ayland @ 2020-12-23 18:15 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: Philippe Mathieu-Daudé,
	QEMU Developers, Guenter Roeck, Michael S. Tsirkin

On 23/12/2020 13:17, BALATON Zoltan via wrote:

> On Wed, 23 Dec 2020, Mark Cave-Ayland wrote:
>> On 22/12/2020 21:23, Guenter Roeck wrote:
>>
>> (Added jiaxun.yang@flygoat.com as CC)
> 
> Are you sure? It does not show up on cc list for me so unless the list ate it you 
> might have forgotten to copy the address there. Done now just in case, sorry if this 
> resulted in double post.
> 
> Regards,
> BALATON Zoltan

I've added the address to the CC (and I can confirm from the exim logs that it gets 
accepted), however the list copy removes it. Perhaps Jiaxun isn't subscribed to 
qemu-devel?


ATB,

Mark.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-23 15:21 ` Philippe Mathieu-Daudé
  2020-12-23 16:09   ` Mark Cave-Ayland
@ 2020-12-23 19:49   ` BALATON Zoltan via
  1 sibling, 0 replies; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-23 19:49 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Michael S. Tsirkin, Mark Cave-Ayland, QEMU Developers, John Snow,
	Guenter Roeck

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

On Wed, 23 Dec 2020, 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(-)

How did you test? What guest OS and version have you used?

Regards,
BALATON Zoltan

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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
  1 sibling, 1 reply; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-23 20:20 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Michael S. Tsirkin, Mark Cave-Ayland, Philippe Mathieu-Daudé,
	QEMU Developers, John Snow

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

On Wed, 23 Dec 2020, Guenter Roeck wrote:
> On 12/23/20 8:09 AM, Mark Cave-Ayland wrote:
>> 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 :/
>>
>
> Not really. The following patch on top of qemu 5.2 results in the ide drive
> being detected and working.
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index be09912b33..1bfdc422ee 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -186,11 +186,14 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>     pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
>
>     bmdma_setup_bar(d);
> +#if 0
>     pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
> +#endif
>
>     qdev_init_gpio_in(ds, via_ide_set_irq, 2);
>     for (i = 0; i < 2; i++) {
>         ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
> +        ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0, i ? 0x376 : 0x3f6);
>         ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
>
>         bmdma_init(&d->bus[i], &d->bmdma[i], d);
>
> With the added ide_init_ioport(), the drive is detected. With the #if 0,

This breaks MorphOS on pegasos2 so it's not acceptable for me as a fix. 
(Actually this just reverts my commit in a cryptic way.)

> it actually starts working. So there are two problems: 1) The qemu ide
> subsystem isn't informed about the io addresses, and 2) bmdma isn't working.

The problem rather seems to be that whatever you're trying to run can only 
handle legacy mode and does not correctly detect or work with native mode 
of this IDE controller. The real chip can switch between these modes and 
starts in legacy mode but most OSes with a better driver will switch to 
native mode during boot (in some cases the firmware will switch already). 
But we can't emulate that in QEMU easily because of how the IDE emulation 
is implemented: we either set up legacy ioports or use PCI BMDMA, I don't 
see a way to deregister legacy ports and irqs once the config reg is 
flipped to native mode. Therefore I've chosen to only emulate native mode 
which is what most guests want to use and some only work with that and 
I've tested this with the previously mentioned Linux version that it still 
detected and worked with the IDE ports. During testing I've found that 
Linux will use either native or legacy modes if the appropriate config 
bits are set but for some boards there may be work arounds for specific 
quirks such as the case for pegasos2 with IRQs hardwired to legacy 
interrupts even in native mode where we need to follow what hardware does 
otherwise one or the other guest breaks. Maybe there's a similar quirk for 
the fuloong2e?

What guest OS are you running and did you confirm that it runs on the real 
machine? If you run recent Linux kernels and don't know if those still 
work with real hardware could this be a bug in the guest driver and not in 
QEMU? We know that we don't fully emulate this controller but there should 
be a way to set things up in a way that satisfies all guests and I've 
tried to do that when touching this part but possibly I did not have the 
right Linux version for the real machine as it was hard to find one distro 
that worked with it. Maybe Jiaxun has a known working Linux distro or 
kernel that we can use to check emulation with or knows more about how the 
VIA IDE port IRQs are wired on this board. (I've added Jiaxun again but 
the list seems to strip his addess.)

Regards,
BALATON Zoltan

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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 23:56           ` BALATON Zoltan via
  0 siblings, 2 replies; 36+ messages in thread
From: Guenter Roeck @ 2020-12-23 21:01 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: Michael S. Tsirkin, Mark Cave-Ayland, Philippe Mathieu-Daudé,
	QEMU Developers, John Snow

On 12/23/20 12:20 PM, BALATON Zoltan wrote:
> On Wed, 23 Dec 2020, Guenter Roeck wrote:
>> On 12/23/20 8:09 AM, Mark Cave-Ayland wrote:
>>> 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 :/
>>>
>>
>> Not really. The following patch on top of qemu 5.2 results in the ide drive
>> being detected and working.
>>
>> diff --git a/hw/ide/via.c b/hw/ide/via.c
>> index be09912b33..1bfdc422ee 100644
>> --- a/hw/ide/via.c
>> +++ b/hw/ide/via.c
>> @@ -186,11 +186,14 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>>     pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
>>
>>     bmdma_setup_bar(d);
>> +#if 0
>>     pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
>> +#endif
>>
>>     qdev_init_gpio_in(ds, via_ide_set_irq, 2);
>>     for (i = 0; i < 2; i++) {
>>         ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
>> +        ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0, i ? 0x376 : 0x3f6);
>>         ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
>>
>>         bmdma_init(&d->bus[i], &d->bmdma[i], d);
>>
>> With the added ide_init_ioport(), the drive is detected. With the #if 0,
> 
> This breaks MorphOS on pegasos2 so it's not acceptable for me as a fix. (Actually this just reverts my commit in a cryptic way.)
> 
>> it actually starts working. So there are two problems: 1) The qemu ide
>> subsystem isn't informed about the io addresses, and 2) bmdma isn't working.
> 
> The problem rather seems to be that whatever you're trying to run can only handle legacy mode and does not correctly detect or work with native mode of this IDE controller. The real chip can switch between these modes and starts in legacy mode but most OSes with a better driver will switch to native mode during boot (in some cases the firmware will switch already). But we can't emulate that in QEMU easily because of how the IDE emulation is implemented: we either set up legacy ioports or use PCI BMDMA, I don't see a way to deregister legacy ports and irqs once the config reg is flipped to native mode. Therefore I've chosen to only emulate native mode which is what most guests want to use and some only work with that and I've tested this with the previously mentioned Linux version that it still detected and worked with the IDE ports. During testing I've found that Linux will use either native or legacy modes if the appropriate config bits are set but for some boards there may
> be work arounds for specific quirks such as the case for pegasos2 with IRQs hardwired to legacy interrupts even in native mode where we need to follow what hardware does otherwise one or the other guest breaks. Maybe there's a similar quirk for the fuloong2e?
> 
> What guest OS are you running and did you confirm that it runs on the real machine? If you run recent Linux kernels and don't know if those still work with real hardware could this be a bug in the guest driver and not in QEMU? We know that we don't fully emulate this controller but there should be a way to set things up in a way that satisfies all guests and I've tried to do that when touching this part but possibly I did not have the right Linux version for the real machine as it was hard to find one distro that worked with it. Maybe Jiaxun has a known working Linux distro or kernel that we can use to check emulation with or knows more about how the VIA IDE port IRQs are wired on this board. (I've added Jiaxun again but the list seems to strip his addess.)
> 

I don't have a real machine, and therefore did not test it on one.

I tried with Linux mainline (v5.10-12913-g614cb5894306), v3.16.85, v4.4.248,
and v4.14.212. I can't test older version because my cross compiler is too
new. Each of those kernel versions shows exactly the same behavior.

Guenter

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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:56           ` BALATON Zoltan via
  1 sibling, 1 reply; 36+ messages in thread
From: Mark Cave-Ayland @ 2020-12-23 22:05 UTC (permalink / raw)
  To: Guenter Roeck, BALATON Zoltan
  Cc: Michael S. Tsirkin, John Snow, Philippe Mathieu-Daudé,
	QEMU Developers

On 23/12/2020 21:01, Guenter Roeck wrote:

> I don't have a real machine, and therefore did not test it on one.
> 
> I tried with Linux mainline (v5.10-12913-g614cb5894306), v3.16.85, v4.4.248,
> and v4.14.212. I can't test older version because my cross compiler is too
> new. Each of those kernel versions shows exactly the same behavior.

Is it possible for you to provide links to your drive image and kernel so that we can 
reproduce the same environment to investigate?


ATB,

Mark.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-23 22:05           ` Mark Cave-Ayland
@ 2020-12-23 22:47             ` Guenter Roeck
  2020-12-23 23:05               ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 36+ messages in thread
From: Guenter Roeck @ 2020-12-23 22:47 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: Michael S. Tsirkin, QEMU Developers, Philippe Mathieu-Daudé,
	John Snow

On Wed, Dec 23, 2020 at 10:05:12PM +0000, Mark Cave-Ayland wrote:
> On 23/12/2020 21:01, Guenter Roeck wrote:
> 
> > I don't have a real machine, and therefore did not test it on one.
> > 
> > I tried with Linux mainline (v5.10-12913-g614cb5894306), v3.16.85, v4.4.248,
> > and v4.14.212. I can't test older version because my cross compiler is too
> > new. Each of those kernel versions shows exactly the same behavior.
> 
> Is it possible for you to provide links to your drive image and kernel so
> that we can reproduce the same environment to investigate?
> 

The root file system is available from
https://github.com/groeck/linux-build-test/blob/master/rootfs/mipsel64/rootfs.mipsel.ext3.gz

The script used to build the kernel is in available in
the same directory, though building fuloong2e_defconfig
should do it, possibly with CONFIG_DEVTMPFS=y added.

Guenter


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-23 22:47             ` Guenter Roeck
@ 2020-12-23 23:05               ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-23 23:05 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Michael S. Tsirkin, Mark Cave-Ayland, QEMU Developers, John Snow

On Wed, Dec 23, 2020 at 11:47 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Wed, Dec 23, 2020 at 10:05:12PM +0000, Mark Cave-Ayland wrote:
> > On 23/12/2020 21:01, Guenter Roeck wrote:
> >
> > > I don't have a real machine, and therefore did not test it on one.
> > >
> > > I tried with Linux mainline (v5.10-12913-g614cb5894306), v3.16.85, v4.4.248,
> > > and v4.14.212. I can't test older version because my cross compiler is too
> > > new. Each of those kernel versions shows exactly the same behavior.
> >
> > Is it possible for you to provide links to your drive image and kernel so
> > that we can reproduce the same environment to investigate?
> >
>
> The root file system is available from
> https://github.com/groeck/linux-build-test/blob/master/rootfs/mipsel64/rootfs.mipsel.ext3.gz
>
> The script used to build the kernel is in available in
> the same directory, though building fuloong2e_defconfig
> should do it, possibly with CONFIG_DEVTMPFS=y added.

I directly used the prebuilt Debian kernel Jiaxun recently shared in:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg768711.html

vmlinux-3.16.0-6-loongson-2e from
http://archive.debian.org/debian/pool/main/l/linux/linux-image-3.16.0-6-loongson-2e_3.16.56-1+deb8u1_mipsel.deb


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-23 21:01         ` Guenter Roeck
  2020-12-23 22:05           ` Mark Cave-Ayland
@ 2020-12-23 23:56           ` BALATON Zoltan via
  2020-12-24  1:34             ` BALATON Zoltan via
  1 sibling, 1 reply; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-23 23:56 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Michael S. Tsirkin, chenhuacai, Mark Cave-Ayland,
	Philippe Mathieu-Daudé,
	QEMU Developers, John Snow

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

On Wed, 23 Dec 2020, Guenter Roeck wrote:
> On 12/23/20 12:20 PM, BALATON Zoltan wrote:
>> On Wed, 23 Dec 2020, Guenter Roeck wrote:
>>> On 12/23/20 8:09 AM, Mark Cave-Ayland wrote:
>>>> On 23/12/2020 15:21, Philippe Mathieu-Daudé wrote:
>>>>> 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 :/
>>>>
>>>
>>> Not really. The following patch on top of qemu 5.2 results in the ide drive
>>> being detected and working.
>>>
>>> diff --git a/hw/ide/via.c b/hw/ide/via.c
>>> index be09912b33..1bfdc422ee 100644
>>> --- a/hw/ide/via.c
>>> +++ b/hw/ide/via.c
>>> @@ -186,11 +186,14 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>>>     pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
>>>
>>>     bmdma_setup_bar(d);
>>> +#if 0
>>>     pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
>>> +#endif
>>>
>>>     qdev_init_gpio_in(ds, via_ide_set_irq, 2);
>>>     for (i = 0; i < 2; i++) {
>>>         ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
>>> +        ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0, i ? 0x376 : 0x3f6);
>>>         ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
>>>
>>>         bmdma_init(&d->bus[i], &d->bmdma[i], d);
>>>
>>> With the added ide_init_ioport(), the drive is detected. With the #if 0,
>>
>> This breaks MorphOS on pegasos2 so it's not acceptable for me as a fix. (Actually this just reverts my commit in a cryptic way.)
>>
>>> it actually starts working. So there are two problems: 1) The qemu ide
>>> subsystem isn't informed about the io addresses, and 2) bmdma isn't working.
>>
>> The problem rather seems to be that whatever you're trying to run can only handle legacy mode and does not correctly detect or work with native mode of this IDE controller. The real chip can switch between these modes and starts in legacy mode but most OSes with a better driver will switch to native mode during boot (in some cases the firmware will switch already). But we can't emulate that in QEMU easily because of how the IDE emulation is implemented: we either set up legacy ioports or use PCI BMDMA, I don't see a way to deregister legacy ports and irqs once the config reg is flipped to native mode. Therefore I've chosen to only emulate native mode which is what most guests want to use and some only work with that and I've tested this with the previously mentioned Linux version that it still detected and worked with the IDE ports. During testing I've found that Linux will use either native or legacy modes if the appropriate config bits are set but for some boards there may
>> be work arounds for specific quirks such as the case for pegasos2 with IRQs hardwired to legacy interrupts even in native mode where we need to follow what hardware does otherwise one or the other guest breaks. Maybe there's a similar quirk for the fuloong2e?
>>
>> What guest OS are you running and did you confirm that it runs on the real machine? If you run recent Linux kernels and don't know if those still work with real hardware could this be a bug in the guest driver and not in QEMU? We know that we don't fully emulate this controller but there should be a way to set things up in a way that satisfies all guests and I've tried to do that when touching this part but possibly I did not have the right Linux version for the real machine as it was hard to find one distro that worked with it. Maybe Jiaxun has a known working Linux distro or kernel that we can use to check emulation with or knows more about how the VIA IDE port IRQs are wired on this board. (I've added Jiaxun again but the list seems to strip his addess.)
>>
>
> I don't have a real machine, and therefore did not test it on one.
>
> I tried with Linux mainline (v5.10-12913-g614cb5894306), v3.16.85, v4.4.248,
> and v4.14.212. I can't test older version because my cross compiler is too
> new. Each of those kernel versions shows exactly the same behavior.

I think the original author of this device was Huacai so adding him to the 
thread too in case he remembers anything relevant, but code there now is 
not what he wrote because that only emulated legacy mode and after my 
changes (reworked by Mark) we only emulate native mode with optional quirk 
using legacy IRQs for pegasos2. But maybe he has some images that were 
known to work on the real machine that we could test now to see where's 
the problem.

On Tue, 22 Dec 2020, Guenter Roeck wrote:
> 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
> ...

This is the previous state only emulating legacy mode and since none of 
the native mode BARs are there Linux fails to enable native mode and falls 
back to legacy so it ends up working but probably not how this should work 
on real machine.

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

Now we emulate native mode and Linux seems to program the BARs (although 
I'm not sure all these should be starting at 0) but then still tries to 
access the device in legacy mode as shown by ports and IRQs.

If someone has logs from original machine it would be interesting to see 
how IDE ports are detected there. I'll try with the kernel from debian and 
see what that does but maybe it tries to use legacy mode too then it 
won't work.

With the original image I used for testing described here:
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04086.html
I now get:

$ qemu-system-mips64el -M fuloong2e -serial stdio -net none -vga none -kernel gentoo-loongson-2.6.22.6-20070902 -cdrom debian-8.11.0-mipsel-netinst.iso

Linux version 2.6.22.6-mipsgit-20070902-lm2e-liveusb (stuartl@zhenghe) 
(gcc version 4.1.2 (Gentoo 4.1.2 p1.0.1)) #5 Fri Jan 25 11:19:12 EST 2008

[...]

via686b fix: ISA bridge
via686b fix: ISA bridge done
via686b fix: IDE
via686b fix: IDE done
PCI quirk: region eee0-eeef claimed by vt82c686 SMB
ac97 interrupt = 9
ac97 rev=80
Setting sub-vendor ID & device ID
sub vendor-device id=11001af4
pci_update_mappings: adding bar 4 to io @ 0x4040

[note the IDE fixup which some board specific quirk coming from
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/mips/pci/fixup-fuloong2e.c]

serial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
io_map_base of root PCI bus 0000:00 unset.  Trying to continue but you better
fix this issue or report it to linux-mips@linux-mips.org or your vendor.
scsi0 : pata_via
scsi1 : pata_via
ata1: PATA max UDMA/100 cmd 0xffffffffbfd001f0 ctl 0xffffffffbfd003f6 bmdma 0xffffffffbfd04040 irq 14
ata2: PATA max UDMA/100 cmd 0xffffffffbfd00170 ctl 0xffffffffbfd00376 bmdma 0xffffffffbfd04048 irq 15
[...]
NET: Registered protocol family 17
Freeing unused kernel memory: 8832k freed


Loading drivers...
usbcore: registered new interface driver usbfs
[...]
USB Universal Host Controller Interface driver v3.0
PCI: Enabling device 0000:00:05.2 (0000 -> 0001)
pci_update_mappings: adding bar 4 to io @ 0x4000
uhci_hcd 0000:00:05.2: UHCI Host Controller
uhci_hcd 0000:00:05.2: new USB bus registered, assigned bus number 1
uhci_hcd 0000:00:05.2: irq 10, io base 0x00004000
qemu-system-mips64el: ../hw/pci/pci.c:255: pci_bus_change_irq_level: 
Assertion `irq_num < bus->nirq' failed.
Aborted (core dumped)

I think this is the problem you've reported originally exposed by 
459ca8bfa41, reverting that commit gives me:

USB Universal Host Controller Interface driver v3.0
PCI: Enabling device 0000:00:05.2 (0000 -> 0001)
uhci_hcd 0000:00:05.2: UHCI Host Controller
uhci_hcd 0000:00:05.2: new USB bus registered, assigned bus number 1
uhci_hcd 0000:00:05.2: irq 10, io base 0x00004000
usb usb1: configuration #1 chosen from 1 choice
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 2 ports detected
PCI: Enabling device 0000:00:05.3 (0000 -> 0001)
uhci_hcd 0000:00:05.3: UHCI Host Controller
uhci_hcd 0000:00:05.3: new USB bus registered, assigned bus number 2
uhci_hcd 0000:00:05.3: irq 11, io base 0x00004020
usb usb2: configuration #1 chosen from 1 choice
hub 2-0:1.0: USB hub found
hub 2-0:1.0: 2 ports detected
loop: module loaded
Registering unionfs 2.2.2 (for 2.6.22.15)
squashfs: version 3.2-r2 (2007/01/15) Phillip Lougher
Waiting 10 seconds for devices to settle.

Gentoo/MIPS 2007.1 Netboot Image
--------------------------------

Kernel 2.6.22.6-mipsgit-20070902-lm2e-liveusb compiled #5 Fri Jan 25 
11:19:12 EST 2008
Running on platform lemote-fulong

but so does removing USB such as:

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 45c596f4fe..26c3438729 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -255,10 +255,10 @@ static void vt82c686b_southbridge_init(PCIBus 
*pci_bus, int slot, qemu_irq intc,

      dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
      pci_ide_create_devs(dev);
-
+#if 0
      pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
      pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
-
+#endif
      *i2c_bus = vt82c686b_pm_init(pci_bus, PCI_DEVFN(slot, 4), 0xeee1, NULL);

      /* Audio support */

which also avoids the the problem without reverting 459ca8bfa41 so I think 
there are two problems:

1. IRQ mapping is somehow wrong on fuloong2, this is evidenced by the USB 
part above and also by kernel panics if I remove -net none or -vga none 
from the command line. I think this is not related to IDE emulation, maybe 
only exposed by it first.

2. The Linux driver you use wants to use legacy mode of the IDE that we 
don't emulate. The linux/arch/mips/pci/fixup-fuloong2e.c does mention 
legacy mode but I think I've found previously that if we hard code native 
mode, Linux would detect it and use it anyway. I think this worked with my 
original series but may have been broken during the rework. I'd have to 
dig up those patches and see what's the difference. Probably hardcoding 
legacy IRQ's instead of allowing true native mode and maybe handling of 
the native mode bit are different in my original patches but I don't 
really want to go over this again after we had a long discussion 
previously and I did describe everything I've found in detail back then 
but forgotten by now and would have to discover it again.

Neither of the above problems should be fixed by reverting my via-ide 
changes that are needed for pegasos2 emulation. If Linux can't be changed 
to work with native mode of the controller then maybe emulating both 
legacy and native mode could help but that does not seem to be simple 
without changing low level IDE emulation that has a chance to break 
something else so I did not try to do that. That's why I chose to emulate 
native mode only which I did test to work with both fuloong2e and 
pegasos2. I did spend quite some time with this back then. I think the 
problem with the current version is that it only emulates half-native mode 
now which is OK for pegasos2 but confuses Linux on fuloong2e. On fuloong 
full native mode would probably work where IRQ is settable by register as 
described in datasheet if we fix native mode bit so it can't be set to 
legacy mode which we don't emulate. I think I did this in original series 
and it worked but needed a bit to select between this mode and half-native 
mode for pegasos2 which Mark disliked so he twisted the patches as long as 
he could get rid of that bit but still allow the test cases to pass.

I know my original proposal may not match real hardware completely but we 
have to decide what we want: faithful emulation of all the quirks of this 
chip or getting an OS running so the user space could be tested. A lot of 
machines in QEMU do the latter (e.g. mac99 or e500 and maybe really all 
machines as they don't aim to be a simulator just emulate enough of the 
machine to get OSes running) so I think it would be OK for this case as 
well if fully emulating the chip is much more trouble than it's worth.

Regards,
BALATON Zoltan

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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
  0 siblings, 2 replies; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-24  1:34 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Michael S. Tsirkin, chenhuacai, Mark Cave-Ayland,
	Philippe Mathieu-Daudé,
	QEMU Developers, John Snow

On Thu, 24 Dec 2020, BALATON Zoltan wrote:
> On Wed, 23 Dec 2020, Guenter Roeck wrote:
>> 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
>> ...
>
> This is the previous state only emulating legacy mode and since none of the 
> native mode BARs are there Linux fails to enable native mode and falls back 
> to legacy so it ends up working but probably not how this should work on real 
> machine.
>
>> ----
>> 
>> 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]
>
> Now we emulate native mode and Linux seems to program the BARs (although I'm 
> not sure all these should be starting at 0) but then still tries to access 
> the device in legacy mode as shown by ports and IRQs.
>
> If someone has logs from original machine it would be interesting to see how 
> IDE ports are detected there. I'll try with the kernel from debian and see 
> what that does but maybe it tries to use legacy mode too then it won't work.
>
> With the original image I used for testing described here:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04086.html
> I now get:
>
> $ qemu-system-mips64el -M fuloong2e -serial stdio -net none -vga none -kernel 
> gentoo-loongson-2.6.22.6-20070902 -cdrom debian-8.11.0-mipsel-netinst.iso

> scsi0 : pata_via
> scsi1 : pata_via
> ata1: PATA max UDMA/100 cmd 0xffffffffbfd001f0 ctl 0xffffffffbfd003f6 bmdma 0xffffffffbfd04040 irq 14
> ata2: PATA max UDMA/100 cmd 0xffffffffbfd00170 ctl 0xffffffffbfd00376 bmdma 0xffffffffbfd04048 irq 15

> 2. The Linux driver you use wants to use legacy mode of the IDE that we don't 
> emulate. The linux/arch/mips/pci/fixup-fuloong2e.c does mention legacy mode 
> but I think I've found previously that if we hard code native mode, Linux 
> would detect it and use it anyway. I think this worked with my original 
> series but may have been broken during the rework. I'd have to dig up those

Here's my original series from March 10:
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=163521
this applies on a checkout from that time such as 7f368aed672117
and with that it works with the gentoo kernel above:

Linux version 2.6.22.6-mipsgit-20070902-lm2e-liveusb (stuartl@zhenghe) (gcc version 4.1.2 (Gentoo 4.1.2 p1.0.1)) #5 Fri Jan 25 11:19:12 EST 2008
[...]
SCSI subsystem initialized
via686b fix: ISA bridge
via686b fix: ISA bridge done
via686b fix: IDE
via686b fix: IDE done
PCI quirk: region eee0-eeef claimed by vt82c686 SMB
ac97 interrupt = 9
[...]
scsi0 : pata_via
scsi1 : pata_via
ata1: PATA max UDMA/100 cmd 0xffffffffbfd04050 ctl 0xffffffffbfd04062 bmdma 0xffffffffbfd04040 irq 14
ata2: PATA max UDMA/100 cmd 0xffffffffbfd04058 ctl 0xffffffffbfd04066 bmdma 0xffffffffbfd04048 irq 14
ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100
ata2.00: limited to UDMA/33 due to 40-wire cable
ata2.00: configured for UDMA/33
scsi 1:0:0:0: CD-ROM            QEMU     QEMU DVD-ROM     2.5+ PQ: 0 ANSI: 5

Note that both channels using irq 14 which means fully native mode but 
that would not work for pegasos2 where we need half-native mode (PCI BARs 
with IRQ 14/15) hence we need the property and flag to enable that mode 
for pegasos2. Without that flag we now only really emulate half-native 
mode which is fine for pegasos2 but seems Linux on fuloong2e does not like 
that. The gentoo kernel seems to like either legacy or full native mode, 
however with the debian kernel from Philippe even full native mode fails:

[    0.000000] Linux version 3.16.0-6-loongson-2e (debian-kernel@lists.debian.org) (gcc version 4.8.4 (Debian 4.8.4-1) ) #1 Debian 3.16.56-1+deb8u1 (2018-05-08)
[...]
[    0.196000] SCSI subsystem initialized
[    0.200000] PCI host bridge to bus 0000:00
[    0.200000] pci_bus 0000:00: root bus resource [mem 0x14000000-0x1c000000]
[    0.200000] pci_bus 0000:00: root bus resource [io  0x4000-0xffff]
[    0.200000] pci_bus 0000:00: No busn resource found for root bus, will use [bus 00-ff]
[    0.208000] via686b fix: ISA bridge
[    0.208000] via686b fix: ISA bridge done
[    0.208000] via686b fix: IDE
[    0.208000] via686b fix: IDE done
[    0.208000] pci 0000:00:05.4: quirk: [io  0xeee0-0xeeef] claimed by vt82c686 SMB
[    0.212000] pci 0000:00:05.2: BAR 4: assigned [io  0x4000-0x401f]
[    0.216000] pci 0000:00:05.3: BAR 4: assigned [io  0x4020-0x403f]
[    0.216000] pci 0000:00:05.1: BAR 4: assigned [io  0x4040-0x404f]
[    0.216000] pci 0000:00:05.1: BAR 0: assigned [io  0x4050-0x4057]
[    0.216000] pci 0000:00:05.1: BAR 2: assigned [io  0x4058-0x405f]
[    0.216000] pci 0000:00:05.1: BAR 1: assigned [io  0x4060-0x4063]
[    0.216000] pci 0000:00:05.1: BAR 3: assigned [io  0x4064-0x4067]
[    0.224000] Switched to clocksource MIPS
[...]
[    0.404000] scsi0 : pata_via
[    0.404000] scsi1 : pata_via
[    0.404000] ata1: PATA max UDMA/100 cmd 0x4050 ctl 0x4060 bmdma 0x4040
[    0.404000] ata2: PATA max UDMA/100 cmd 0x4058 ctl 0x4064 bmdma 0x4048
[...]
[    0.728000] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100
[    0.728000] ata2.00: limited to UDMA/33 due to 40-wire cable
[    0.732000] ata2.00: configured for UDMA/33
[    1.356000] input: ImExPS/2 Generic Explorer Mouse as /devices/platform/i8042/serio1/input/input2
[    5.732000] ata2.00: qc timeout (cmd 0xa0)
[    5.732000] ata2.00: TEST_UNIT_READY failed (err_mask=0x4)
[    5.888000] ata2.00: configured for UDMA/33
[   10.888000] ata2.00: qc timeout (cmd 0xa0)
[   10.888000] ata2.00: TEST_UNIT_READY failed (err_mask=0x4)
[   10.888000] ata2.00: limiting speed to UDMA/33:PIO3
[   11.044000] ata2.00: configured for UDMA/33
[   16.044000] ata2.00: qc timeout (cmd 0xa0)
[   16.044000] ata2.00: TEST_UNIT_READY failed (err_mask=0x4)
[   16.044000] ata2.00: disabled
[   16.044000] ata2: soft resetting link
[   16.200000] ata2: EH complete

It does not say which interrupts are used so can't tell if it's trying 
native or legacy mode but I think it may want to use legacy mode (based on 
the comment in the fixup-fulong2 function) but then I'm not sure why it 
sets up BMDMA and PCI BARs which is an indication of using native mode. 
Something seems to be mixed up here. The comment in the qtest this kernel 
comes from says it's trusted because it comes from Debian. Well, I'd only 
trust it if it was confirmed to run on real hardware but unfortunately we 
don't have that confirmation for any of these kernels so we really don't 
know what we're doing here. I think we need a known working, tested on 
real machine kernel so we know that our test case is correct in the first 
place. Does anybody have access to real hardware to test?

Now I remember that the issue here was basically the following:

- The chip can be used in 3 modes on different boards:
1. legacy ports and IRQ 14/15
2. native mode in which it behaves as a PCI IDE controller with BARs and 
IRQ set via a register
3. but on some machines like pegasos2 there's half-native or non-100% 
native mode as Linux calls it which is like native but IRQs hard wired to 
ISA 14/15 regardless of the register that select IRQ line in native mode.

- Guest OSes on these machines have all kinds of fixups and assumptions on 
how the chip works on that machine and so only work if we emulate that 
closely enough because they don't read config bits of the device just 
program them or expect some specific behaviour.

- It's not easy to fully emulate all this in QEMU given the current low 
level IDE emulation, in particular changing between legacy and IDE modes 
is not supported by low level functions and so instead of risking breaking 
low level IDE code used by almost all machines I've opted to try only 
partially implement it so guests would run. (Besides it's not enough to 
switch between legacy and native modes but also need the 3rd mode.) This 
seemed to work for the test cases we had but now we have Linux kernels 
that have different assumptions so now we have a conflict: fuloong2e 
needing legacy mode and pegasos2 needing half-native mode.

To resolve this conflict I'd like to know if the assumption of needing 
legacy mode on fuloong is valid or could it be lifted in Linux so it works 
with half-native or native mode as that would avoid the need to emulate 
legacy mode as well in QEMU. (Although I'm not sure about that, maybe on 
real hardware legacy mode is used as some of these VIA chips had known 
problems with DMA so to avoid that it's not used but those problems don't 
affect QEMU. But this is not confirmed either so only guessing here.)

If we need legacy mode then we may be able to emulate that by setting BARs 
to legacy ports ignoring what values are written to them if legacy mode 
config is set (which may be what the real chip does) and we already have 
IRQs hard wired to legacy values so that would give us legacy and 
half-native mode which is enough for both fuloong2e and pegasos2 but I'm 
not sure how can we fix BARs in QEMU because that's also handled by 
generic PCI code which I also don't want to break.

Regards,
BALATON Zoltan


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-24  1:34             ` BALATON Zoltan via
@ 2020-12-24  2:29               ` Jiaxun Yang
  2020-12-24  5:32               ` Guenter Roeck
  1 sibling, 0 replies; 36+ messages in thread
From: Jiaxun Yang @ 2020-12-24  2:29 UTC (permalink / raw)
  To: BALATON Zoltan, Guenter Roeck
  Cc: Michael S. Tsirkin, chenhuacai, Mark Cave-Ayland,
	Philippe Mathieu-Daudé,
	QEMU Developers, John Snow

在 2020/12/24 上午9:34, BALATON Zoltan 写道:
> On Thu, 24 Dec 2020, BALATON Zoltan wrote:
>> On Wed, 23 Dec 2020, Guenter Roeck wrote:
>>> 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
>>> ...
>>
>> This is the previous state only emulating legacy mode and since none 
>> of the native mode BARs are there Linux fails to enable native mode 
>> and falls back to legacy so it ends up working but probably not how 
>> this should work on real machine.
>>
>>> ----
>>>
>>> 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]
>>
>> Now we emulate native mode and Linux seems to program the BARs 
>> (although I'm not sure all these should be starting at 0) but then 
>> still tries to access the device in legacy mode as shown by ports and 
>> IRQs.
>>
>> If someone has logs from original machine it would be interesting to 
>> see how IDE ports are detected there. I'll try with the kernel from 
>> debian and see what that does but maybe it tries to use legacy mode 
>> too then it won't work.
>>
>> With the original image I used for testing described here:
>> https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg04086.html
>> I now get:
>>
>> $ qemu-system-mips64el -M fuloong2e -serial stdio -net none -vga none 
>> -kernel gentoo-loongson-2.6.22.6-20070902 -cdrom 
>> debian-8.11.0-mipsel-netinst.iso
>
>> scsi0 : pata_via
>> scsi1 : pata_via
>> ata1: PATA max UDMA/100 cmd 0xffffffffbfd001f0 ctl 0xffffffffbfd003f6 
>> bmdma 0xffffffffbfd04040 irq 14
>> ata2: PATA max UDMA/100 cmd 0xffffffffbfd00170 ctl 0xffffffffbfd00376 
>> bmdma 0xffffffffbfd04048 irq 15
>
>> 2. The Linux driver you use wants to use legacy mode of the IDE that 
>> we don't emulate. The linux/arch/mips/pci/fixup-fuloong2e.c does 
>> mention legacy mode but I think I've found previously that if we hard 
>> code native mode, Linux would detect it and use it anyway. I think 
>> this worked with my original series but may have been broken during 
>> the rework. I'd have to dig up those
>
> Here's my original series from March 10:
> http://patchwork.ozlabs.org/project/qemu-devel/list/?series=163521
> this applies on a checkout from that time such as 7f368aed672117
> and with that it works with the gentoo kernel above:
>
> Linux version 2.6.22.6-mipsgit-20070902-lm2e-liveusb (stuartl@zhenghe) 
> (gcc version 4.1.2 (Gentoo 4.1.2 p1.0.1)) #5 Fri Jan 25 11:19:12 EST 2008
> [...]
> SCSI subsystem initialized
> via686b fix: ISA bridge
> via686b fix: ISA bridge done
> via686b fix: IDE
> via686b fix: IDE done
> PCI quirk: region eee0-eeef claimed by vt82c686 SMB
> ac97 interrupt = 9
> [...]
> scsi0 : pata_via
> scsi1 : pata_via
> ata1: PATA max UDMA/100 cmd 0xffffffffbfd04050 ctl 0xffffffffbfd04062 
> bmdma 0xffffffffbfd04040 irq 14
> ata2: PATA max UDMA/100 cmd 0xffffffffbfd04058 ctl 0xffffffffbfd04066 
> bmdma 0xffffffffbfd04048 irq 14
> ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100
> ata2.00: limited to UDMA/33 due to 40-wire cable
> ata2.00: configured for UDMA/33
> scsi 1:0:0:0: CD-ROM            QEMU     QEMU DVD-ROM     2.5+ PQ: 0 
> ANSI: 5
>
> Note that both channels using irq 14 which means fully native mode but 
> that would not work for pegasos2 where we need half-native mode (PCI 
> BARs with IRQ 14/15) hence we need the property and flag to enable 
> that mode for pegasos2. Without that flag we now only really emulate 
> half-native mode which is fine for pegasos2 but seems Linux on 
> fuloong2e does not like that. The gentoo kernel seems to like either 
> legacy or full native mode, however with the debian kernel from 
> Philippe even full native mode fails:
>
> [    0.000000] Linux version 3.16.0-6-loongson-2e 
> (debian-kernel@lists.debian.org) (gcc version 4.8.4 (Debian 4.8.4-1) ) 
> #1 Debian 3.16.56-1+deb8u1 (2018-05-08)
> [...]
> [    0.196000] SCSI subsystem initialized
> [    0.200000] PCI host bridge to bus 0000:00
> [    0.200000] pci_bus 0000:00: root bus resource [mem 
> 0x14000000-0x1c000000]
> [    0.200000] pci_bus 0000:00: root bus resource [io 0x4000-0xffff]
> [    0.200000] pci_bus 0000:00: No busn resource found for root bus, 
> will use [bus 00-ff]
> [    0.208000] via686b fix: ISA bridge
> [    0.208000] via686b fix: ISA bridge done
> [    0.208000] via686b fix: IDE
> [    0.208000] via686b fix: IDE done
> [    0.208000] pci 0000:00:05.4: quirk: [io  0xeee0-0xeeef] claimed by 
> vt82c686 SMB
> [    0.212000] pci 0000:00:05.2: BAR 4: assigned [io 0x4000-0x401f]
> [    0.216000] pci 0000:00:05.3: BAR 4: assigned [io 0x4020-0x403f]
> [    0.216000] pci 0000:00:05.1: BAR 4: assigned [io 0x4040-0x404f]
> [    0.216000] pci 0000:00:05.1: BAR 0: assigned [io 0x4050-0x4057]
> [    0.216000] pci 0000:00:05.1: BAR 2: assigned [io 0x4058-0x405f]
> [    0.216000] pci 0000:00:05.1: BAR 1: assigned [io 0x4060-0x4063]
> [    0.216000] pci 0000:00:05.1: BAR 3: assigned [io 0x4064-0x4067]
> [    0.224000] Switched to clocksource MIPS
> [...]
> [    0.404000] scsi0 : pata_via
> [    0.404000] scsi1 : pata_via
> [    0.404000] ata1: PATA max UDMA/100 cmd 0x4050 ctl 0x4060 bmdma 0x4040
> [    0.404000] ata2: PATA max UDMA/100 cmd 0x4058 ctl 0x4064 bmdma 0x4048
> [...]
> [    0.728000] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100
> [    0.728000] ata2.00: limited to UDMA/33 due to 40-wire cable
> [    0.732000] ata2.00: configured for UDMA/33
> [    1.356000] input: ImExPS/2 Generic Explorer Mouse as 
> /devices/platform/i8042/serio1/input/input2
> [    5.732000] ata2.00: qc timeout (cmd 0xa0)
> [    5.732000] ata2.00: TEST_UNIT_READY failed (err_mask=0x4)
> [    5.888000] ata2.00: configured for UDMA/33
> [   10.888000] ata2.00: qc timeout (cmd 0xa0)
> [   10.888000] ata2.00: TEST_UNIT_READY failed (err_mask=0x4)
> [   10.888000] ata2.00: limiting speed to UDMA/33:PIO3
> [   11.044000] ata2.00: configured for UDMA/33
> [   16.044000] ata2.00: qc timeout (cmd 0xa0)
> [   16.044000] ata2.00: TEST_UNIT_READY failed (err_mask=0x4)
> [   16.044000] ata2.00: disabled
> [   16.044000] ata2: soft resetting link
> [   16.200000] ata2: EH complete
>
> It does not say which interrupts are used so can't tell if it's trying 
> native or legacy mode but I think it may want to use legacy mode 
> (based on the comment in the fixup-fulong2 function) but then I'm not 
> sure why it sets up BMDMA and PCI BARs which is an indication of using 
> native mode. Something seems to be mixed up here. The comment in the 
> qtest this kernel comes from says it's trusted because it comes from 
> Debian. Well, I'd only trust it if it was confirmed to run on real 
> hardware but unfortunately we don't have that confirmation for any of 
> these kernels so we really don't know what we're doing here. I think 
> we need a known working, tested on real machine kernel so we know that 
> our test case is correct in the first place. Does anybody have access 
> to real hardware to test?
>
> Now I remember that the issue here was basically the following:
>
> - The chip can be used in 3 modes on different boards:
> 1. legacy ports and IRQ 14/15
> 2. native mode in which it behaves as a PCI IDE controller with BARs 
> and IRQ set via a register
> 3. but on some machines like pegasos2 there's half-native or non-100% 
> native mode as Linux calls it which is like native but IRQs hard wired 
> to ISA 14/15 regardless of the register that select IRQ line in native 
> mode.
>
> - Guest OSes on these machines have all kinds of fixups and 
> assumptions on how the chip works on that machine and so only work if 
> we emulate that closely enough because they don't read config bits of 
> the device just program them or expect some specific behaviour.
>
> - It's not easy to fully emulate all this in QEMU given the current 
> low level IDE emulation, in particular changing between legacy and IDE 
> modes is not supported by low level functions and so instead of 
> risking breaking low level IDE code used by almost all machines I've 
> opted to try only partially implement it so guests would run. (Besides 
> it's not enough to switch between legacy and native modes but also 
> need the 3rd mode.) This seemed to work for the test cases we had but 
> now we have Linux kernels that have different assumptions so now we 
> have a conflict: fuloong2e needing legacy mode and pegasos2 needing 
> half-native mode.
>
> To resolve this conflict I'd like to know if the assumption of needing 
> legacy mode on fuloong is valid or could it be lifted in Linux so it 
> works with half-native or native mode as that would avoid the need to 
> emulate legacy mode as well in QEMU. (Although I'm not sure about 
> that, maybe on real hardware legacy mode is used as some of these VIA 
> chips had known problems with DMA so to avoid that it's not used but 
> those problems don't affect QEMU. But this is not confirmed either so 
> only guessing here.)
>
> If we need legacy mode then we may be able to emulate that by setting 
> BARs to legacy ports ignoring what values are written to them if 
> legacy mode config is set (which may be what the real chip does) and 
> we already have IRQs hard wired to legacy values so that would give us 
> legacy and half-native mode which is enough for both fuloong2e and 
> pegasos2 but I'm not sure how can we fix BARs in QEMU because that's 
> also handled by generic PCI code which I also don't want to break.

Hi all,

For Fuloong2E I think we can simply emulate legacy mode and provide a 
fake BAR in bonito.c
to workaround that?

Thanks.

- Jiaxun

>
> Regards,
> BALATON Zoltan



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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
  1 sibling, 1 reply; 36+ messages in thread
From: Guenter Roeck @ 2020-12-24  5:32 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: Michael S. Tsirkin, chenhuacai, Mark Cave-Ayland,
	Philippe Mathieu-Daudé,
	QEMU Developers, John Snow

On Thu, Dec 24, 2020 at 02:34:07AM +0100, BALATON Zoltan wrote:
[ ... ]
> 
> If we need legacy mode then we may be able to emulate that by setting BARs
> to legacy ports ignoring what values are written to them if legacy mode
> config is set (which may be what the real chip does) and we already have
> IRQs hard wired to legacy values so that would give us legacy and
> half-native mode which is enough for both fuloong2e and pegasos2 but I'm not
> sure how can we fix BARs in QEMU because that's also handled by generic PCI
> code which I also don't want to break.

The code below works for booting Linux while at the same time not affecting
any other emulation. I don't claim it to be a perfect fix, and overloading
the existing property is a bit hackish, but it does work.

Guenter

---
From cf2d1d655f3fe4f88dc435a3ac4e1e6b6040d08b Mon Sep 17 00:00:00 2001
From: Guenter Roeck <linux@roeck-us.net>
Date: Wed, 23 Dec 2020 09:12:37 -0800
Subject: [PATCH] via-ide: Fix fuloong2 support

Fuloong2 needs to use legacy mode for IDE support to work with Linux.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 hw/ide/via.c        | 16 ++++++++++++++--
 hw/mips/fuloong2e.c |  4 +++-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index be09912b33..9e55e717e8 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -26,6 +26,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "sysemu/dma.h"
@@ -185,12 +186,17 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
                           &d->bus[1], "via-ide1-cmd", 4);
     pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
 
-    bmdma_setup_bar(d);
-    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+    if (!d->secondary) {
+        bmdma_setup_bar(d);
+        pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+    }
 
     qdev_init_gpio_in(ds, via_ide_set_irq, 2);
     for (i = 0; i < 2; i++) {
         ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
+        if (d->secondary) {
+            ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0, i ? 0x376 : 0x3f6);
+        }
         ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
@@ -210,6 +216,11 @@ static void via_ide_exitfn(PCIDevice *dev)
     }
 }
 
+static Property via_ide_properties[] = {
+    DEFINE_PROP_UINT32("legacy_mode", PCIIDEState, secondary, 0), /* hijacked */
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void via_ide_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -223,6 +234,7 @@ static void via_ide_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_VIA_IDE;
     k->revision = 0x06;
     k->class_id = PCI_CLASS_STORAGE_IDE;
+    device_class_set_props(dc, via_ide_properties);
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
 
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 23c526c69d..d0398d6266 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -245,7 +245,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
     /* Super I/O */
     isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
 
-    dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
+    dev = pci_new(PCI_DEVFN(slot, 1), "via-ide");
+    qdev_prop_set_uint32(&dev->qdev, "legacy_mode", 1);
+    pci_realize_and_unref(dev, pci_bus, &error_fatal);
     pci_ide_create_devs(dev);
 
     pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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-28 19:26                   ` Mark Cave-Ayland
  0 siblings, 2 replies; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-24  8:11 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Michael S. Tsirkin, chenhuacai, Mark Cave-Ayland,
	Philippe Mathieu-Daudé,
	QEMU Developers, John Snow

On Wed, 23 Dec 2020, Guenter Roeck wrote:
> On Thu, Dec 24, 2020 at 02:34:07AM +0100, BALATON Zoltan wrote:
> [ ... ]
>>
>> If we need legacy mode then we may be able to emulate that by setting BARs
>> to legacy ports ignoring what values are written to them if legacy mode
>> config is set (which may be what the real chip does) and we already have
>> IRQs hard wired to legacy values so that would give us legacy and
>> half-native mode which is enough for both fuloong2e and pegasos2 but I'm not
>> sure how can we fix BARs in QEMU because that's also handled by generic PCI
>> code which I also don't want to break.
>
> The code below works for booting Linux while at the same time not affecting
> any other emulation. I don't claim it to be a perfect fix, and overloading
> the existing property is a bit hackish, but it does work.

Yes, maybe combining it with my original patch 1 to change secondary to 
flags to make it a bit cleaner would work for me. Then we would either 
only emulate legacy or half-native mode which is sufficient for these two 
machines we have. If Mark or others do not object it this time, I can 
update my patch and resubmit with this one to fix this issue, otherwise 
let's wait what idea do they have because I hate to spend time with 
something only to be discarded again. I think we don't need more complete 
emulation of this chip than this for now but if somebody wants to attempt 
that I don't mind as long as it does not break pegasos2.

Regards,
BALATON Zoltan

> Guenter
>
> ---
> From cf2d1d655f3fe4f88dc435a3ac4e1e6b6040d08b Mon Sep 17 00:00:00 2001
> From: Guenter Roeck <linux@roeck-us.net>
> Date: Wed, 23 Dec 2020 09:12:37 -0800
> Subject: [PATCH] via-ide: Fix fuloong2 support
>
> Fuloong2 needs to use legacy mode for IDE support to work with Linux.
> Add property to via-ide driver to make the mode configurable, and set
> legacy mode for Fuloong2.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> hw/ide/via.c        | 16 ++++++++++++++--
> hw/mips/fuloong2e.c |  4 +++-
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index be09912b33..9e55e717e8 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -26,6 +26,7 @@
>
> #include "qemu/osdep.h"
> #include "hw/pci/pci.h"
> +#include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> #include "sysemu/dma.h"
> @@ -185,12 +186,17 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>                           &d->bus[1], "via-ide1-cmd", 4);
>     pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
>
> -    bmdma_setup_bar(d);
> -    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
> +    if (!d->secondary) {
> +        bmdma_setup_bar(d);
> +        pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
> +    }
>
>     qdev_init_gpio_in(ds, via_ide_set_irq, 2);
>     for (i = 0; i < 2; i++) {
>         ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
> +        if (d->secondary) {
> +            ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0, i ? 0x376 : 0x3f6);
> +        }
>         ide_init2(&d->bus[i], qdev_get_gpio_in(ds, i));
>
>         bmdma_init(&d->bus[i], &d->bmdma[i], d);
> @@ -210,6 +216,11 @@ static void via_ide_exitfn(PCIDevice *dev)
>     }
> }
>
> +static Property via_ide_properties[] = {
> +    DEFINE_PROP_UINT32("legacy_mode", PCIIDEState, secondary, 0), /* hijacked */
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void via_ide_class_init(ObjectClass *klass, void *data)
> {
>     DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -223,6 +234,7 @@ static void via_ide_class_init(ObjectClass *klass, void *data)
>     k->device_id = PCI_DEVICE_ID_VIA_IDE;
>     k->revision = 0x06;
>     k->class_id = PCI_CLASS_STORAGE_IDE;
> +    device_class_set_props(dc, via_ide_properties);
>     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> }
>
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 23c526c69d..d0398d6266 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -245,7 +245,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>     /* Super I/O */
>     isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
>
> -    dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
> +    dev = pci_new(PCI_DEVFN(slot, 1), "via-ide");
> +    qdev_prop_set_uint32(&dev->qdev, "legacy_mode", 1);
> +    pci_realize_and_unref(dev, pci_bus, &error_fatal);
>     pci_ide_create_devs(dev);
>
>     pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  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
  1 sibling, 1 reply; 36+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-24 10:50 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: Michael S. Tsirkin, Alex Bennée, Huacai Chen,
	Mark Cave-Ayland, QEMU Developers, John Snow, Guenter Roeck

On Thu, Dec 24, 2020 at 9:11 AM BALATON Zoltan <balaton@eik.bme.hu> wrote:
> On Wed, 23 Dec 2020, Guenter Roeck wrote:
> > On Thu, Dec 24, 2020 at 02:34:07AM +0100, BALATON Zoltan wrote:
> > [ ... ]
> >>
> >> If we need legacy mode then we may be able to emulate that by setting BARs
> >> to legacy ports ignoring what values are written to them if legacy mode
> >> config is set (which may be what the real chip does) and we already have
> >> IRQs hard wired to legacy values so that would give us legacy and
> >> half-native mode which is enough for both fuloong2e and pegasos2 but I'm not
> >> sure how can we fix BARs in QEMU because that's also handled by generic PCI
> >> code which I also don't want to break.
> >
> > The code below works for booting Linux while at the same time not affecting
> > any other emulation. I don't claim it to be a perfect fix, and overloading
> > the existing property is a bit hackish, but it does work.
[...]
> I think we don't need more complete
> emulation of this chip than this for now but if somebody wants to attempt
> that I don't mind as long as it does not break pegasos2.

Fine by me as long as pegasos2 doesn't break other OSes :)

Can we have integration tests of pegasos2 so we can modify the device models
without introducing regressions?
If it is not open-source, you could still contribute tests with hash
of tested binary
and provide the binary file to test on demand off-list.

Regards,

Phil.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-24 10:50                   ` Philippe Mathieu-Daudé
@ 2020-12-24 17:09                     ` BALATON Zoltan via
  0 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-24 17:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Michael S. Tsirkin, Alex Bennée, Huacai Chen,
	Mark Cave-Ayland, QEMU Developers, John Snow, Guenter Roeck

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

On Thu, 24 Dec 2020, Philippe Mathieu-Daudé wrote:
> On Thu, Dec 24, 2020 at 9:11 AM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>> On Wed, 23 Dec 2020, Guenter Roeck wrote:
>>> On Thu, Dec 24, 2020 at 02:34:07AM +0100, BALATON Zoltan wrote:
>>> [ ... ]
>>>>
>>>> If we need legacy mode then we may be able to emulate that by setting BARs
>>>> to legacy ports ignoring what values are written to them if legacy mode
>>>> config is set (which may be what the real chip does) and we already have
>>>> IRQs hard wired to legacy values so that would give us legacy and
>>>> half-native mode which is enough for both fuloong2e and pegasos2 but I'm not
>>>> sure how can we fix BARs in QEMU because that's also handled by generic PCI
>>>> code which I also don't want to break.
>>>
>>> The code below works for booting Linux while at the same time not affecting
>>> any other emulation. I don't claim it to be a perfect fix, and overloading
>>> the existing property is a bit hackish, but it does work.
> [...]
>> I think we don't need more complete
>> emulation of this chip than this for now but if somebody wants to attempt
>> that I don't mind as long as it does not break pegasos2.
>
> Fine by me as long as pegasos2 doesn't break other OSes :)

Sure, I did try to avoid breaking fuloong2e last time too and tested with 
the kernel I could dig up. Unfortunately that does not seem to be the 
right test for that machine. The fuloong2e model wasn't in very good shape 
back then. Now we have better test cases for it.

> Can we have integration tests of pegasos2 so we can modify the device models
> without introducing regressions?
> If it is not open-source, you could still contribute tests with hash
> of tested binary
> and provide the binary file to test on demand off-list.

We're not there yet when I can submit pegasos2 patches for merging because 
I'll need to make more clean ups to via model and also have a replacement 
for the firmware binary that I plan to do as time permits. For OS there 
may be some older PPC Linux distros that used to work with pegasos2 and 
MorphOS demo is freely downloadable but not redistributable so maybe 
possible to use as test but I'll need some help with the python module to 
integrate it in QEMU tests. I'll keep you cc-d about this anyway as the 
MIPS maintainer.

Regards,
BALATON Zoltan

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-22 21:23   ` Guenter Roeck
                       ` (2 preceding siblings ...)
  2020-12-23 10:24     ` Mark Cave-Ayland
@ 2020-12-25 23:43     ` BALATON Zoltan via
  2020-12-31 15:34       ` Peter Maydell
  3 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-25 23:43 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Michael S. Tsirkin, Mark Cave-Ayland, QEMU Developers,
	Philippe Mathieu-Daudé

[-- Attachment #1: Type: text/plain, Size: 3971 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 had a look if similar fix would work for this as for sam460ex but I'm 
not sure. The real Sam460EX only has one PCI slot so no need to map slots 
and according to U-Boot sources all PCI INT pins are connected to single 
IRQ on the interrupt controller. In QEMU we can have multiple PCI devices 
but just connecting everything up to a single interrupt seems to work. 
(Previously we did that in map_irq of the pci host, after clean up we 
model the 4 PCI interrupt lines that are then or-ed in the board code. I 
did not find a difference in working but the model may be a bit cleaner 
this way and allow reusing the pci controller in a board that may have 
different mapping.)

For the Bamboo board we have 4 interrupts connected to the PCI bus in the 
board but also have a comment in ppc4xx_pci.c near the above function 
saying:

/* On Bamboo, all pins from each slot are tied to a single board IRQ. This
  * may need further refactoring for other boards. */
static int ppc4xx_pci_map_irq(PCIDevice *pci_dev, int irq_num)
{
     int slot = pci_dev->devfn >> 3;

     trace_ppc4xx_pci_map_irq(pci_dev->devfn, irq_num, slot);

     return slot - 1;
}

Now I'm not sure what that comment means:

1. All PCI INTA from all slots go to one irq, PCI INTB to another and so on

or

2. All PCI interrupts (INTA-D) from first slot are connected to one irq on 
the board, then those from next slot are to another irq and so on

The slot - 1 mapping seems to correspond more to 2. but that also means we 
can only have 4 slots. I did not find a picture of the real board so don't 
know how many slots that has or how they are connected. I think we need 
more info on the real hardware to tell what's the correct mapping here.

Regards,
BALATON Zoltan

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-24  8:11                 ` BALATON Zoltan via
  2020-12-24 10:50                   ` Philippe Mathieu-Daudé
@ 2020-12-28 19:26                   ` Mark Cave-Ayland
  2020-12-28 21:18                     ` BALATON Zoltan via
  1 sibling, 1 reply; 36+ messages in thread
From: Mark Cave-Ayland @ 2020-12-28 19:26 UTC (permalink / raw)
  To: BALATON Zoltan, Guenter Roeck
  Cc: QEMU Developers, chenhuacai, John Snow,
	Philippe Mathieu-Daudé,
	Michael S. Tsirkin

On 24/12/2020 08:11, BALATON Zoltan via wrote:

> On Wed, 23 Dec 2020, Guenter Roeck wrote:
>> On Thu, Dec 24, 2020 at 02:34:07AM +0100, BALATON Zoltan wrote:
>> [ ... ]
>>>
>>> If we need legacy mode then we may be able to emulate that by setting BARs
>>> to legacy ports ignoring what values are written to them if legacy mode
>>> config is set (which may be what the real chip does) and we already have
>>> IRQs hard wired to legacy values so that would give us legacy and
>>> half-native mode which is enough for both fuloong2e and pegasos2 but I'm not
>>> sure how can we fix BARs in QEMU because that's also handled by generic PCI
>>> code which I also don't want to break.
>>
>> The code below works for booting Linux while at the same time not affecting
>> any other emulation. I don't claim it to be a perfect fix, and overloading
>> the existing property is a bit hackish, but it does work.
> 
> Yes, maybe combining it with my original patch 1 to change secondary to flags to make 
> it a bit cleaner would work for me. Then we would either only emulate legacy or 
> half-native mode which is sufficient for these two machines we have. If Mark or 
> others do not object it this time, I can update my patch and resubmit with this one 
> to fix this issue, otherwise let's wait what idea do they have because I hate to 
> spend time with something only to be discarded again. I think we don't need more 
> complete emulation of this chip than this for now but if somebody wants to attempt 
> that I don't mind as long as it does not break pegasos2.

I had a play with your patches this afternoon, and spent some time performing some 
experiments and also reading various PCI bus master specifications and datasheets: 
this helped me understand a lot more about the theory of IRQ routing and compatible 
vs. legacy mode.

 From reading all the documentation (including the VIA and other datasheets) I cannot 
find any reference to a half-native mode which makes me think something else is wrong 
here. At the simplest level it could simply be that the VIA doesn't tri-state its 
legacy IRQ lines whilst the device is in native mode (the SI controller has an option 
for this), or it could indicate there is a PCI IRQ routing problem somewhere else 
that hasn't been picked up yet.

All of the datasheets suggest that legacy vs. native mode is selected by setting the 
correct bits in PCI_CLASS_PROG, and Linux reads this byte and configures itself to 
use legacy or native mode accordingly. Since the current default for the VIA is 0x8a 
then it should default to legacy mode, but we're immediately hitting some issues 
here: I've summarised my notes below for those interested.


1) PCI bus reset loses the default BAR addresses

The first problem we find is that the initialisation of the PCI bus erases the 
default BAR addresses: that's to say lines 133-137 in hw/ide/via.c will in effect do 
nothing:

  133     pci_set_long(pci_conf + PCI_BASE_ADDRESS_0, 0x000001f0);
  134     pci_set_long(pci_conf + PCI_BASE_ADDRESS_1, 0x000003f4);
  135     pci_set_long(pci_conf + PCI_BASE_ADDRESS_2, 0x00000170);
  136     pci_set_long(pci_conf + PCI_BASE_ADDRESS_3, 0x00000374);
  137     pci_set_long(pci_conf + PCI_BASE_ADDRESS_4, 0x0000cc01); /* BMIBA: 20-23h */

The lifecycle of the VIA IDE device goes like this: init() -> realize() -> reset() 
but then the PCI bus reset in pci_do_device_reset() immediately wipes the BAR 
addresses. This is why the legacy IDE ports currently don't appear at startup. Note I 
do see that other devices do try this e.g. gt64120_pci_realize() so it's an easy 
mistake to make.


2) -kernel doesn't initialise the VIA device

If you take a look at the PMON source it is possible to see that the firmware 
explicitly sets the PCI_CLASS_PROG to compatibility mode and disables the native PCI 
interrupt 
(https://github.com/loongson-community/pmon-2ef/blob/master/sys/dev/pci/vt82c686.c#L82).

Since Linux reads this byte on startup then this is why the kernel switches to 
compatibility mode by default. However the point here is that booting a kernel 
directly without firmware means the VIA IDE device isn't initialised as it would be 
in real life, and that's why there are attempts to pre-configure the device 
accordingly in via_ide_realize()/via_ide_reset().


3) QEMU doesn't (easily) enable a BAR to be disabled

The ideal situation would be for QEMU's VIA IDE device to check PCI_CLASS_PROG and 
configure itself dynamically: with PCI_CLASS_PROG set for legacy mode by default, the 
device can disable its BARs until they are explicitly enabled.

According to the PCI bus master specification the recommended behaviour for a device 
in compatible mode is to ignore all writes to the BARs, and for all BAR reads to 
return 0. This fits nicely with Guenter's finding that the BMDMA BAR should not 
return a value in order for Linux to boot correctly in legacy mode.

Unfortunately there is no existing functionality for this in QEMU which means you 
would have to do this manually by overriding the PCI config read/write functions. 
This is trickier than it sounds because the reads/writes don't necessarily have to be 
aligned to the BAR addresses in configuration space.


In summary whilst I'm not keen on the series in its current form, it seems the best 
solution for now. I've got a few comments on the latest version of the series which I 
will send along shortly.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-28 19:26                   ` Mark Cave-Ayland
@ 2020-12-28 21:18                     ` BALATON Zoltan via
  0 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan via @ 2020-12-28 21:18 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: Michael S. Tsirkin, chenhuacai, QEMU Developers,
	Philippe Mathieu-Daudé,
	John Snow, Guenter Roeck

On Mon, 28 Dec 2020, Mark Cave-Ayland wrote:
> On 24/12/2020 08:11, BALATON Zoltan via wrote:
>
>> On Wed, 23 Dec 2020, Guenter Roeck wrote:
>>> On Thu, Dec 24, 2020 at 02:34:07AM +0100, BALATON Zoltan wrote:
>>> [ ... ]
>>>> 
>>>> If we need legacy mode then we may be able to emulate that by setting 
>>>> BARs
>>>> to legacy ports ignoring what values are written to them if legacy mode
>>>> config is set (which may be what the real chip does) and we already have
>>>> IRQs hard wired to legacy values so that would give us legacy and
>>>> half-native mode which is enough for both fuloong2e and pegasos2 but I'm 
>>>> not
>>>> sure how can we fix BARs in QEMU because that's also handled by generic 
>>>> PCI
>>>> code which I also don't want to break.
>>> 
>>> The code below works for booting Linux while at the same time not 
>>> affecting
>>> any other emulation. I don't claim it to be a perfect fix, and overloading
>>> the existing property is a bit hackish, but it does work.
>> 
>> Yes, maybe combining it with my original patch 1 to change secondary to 
>> flags to make it a bit cleaner would work for me. Then we would either only 
>> emulate legacy or half-native mode which is sufficient for these two 
>> machines we have. If Mark or others do not object it this time, I can 
>> update my patch and resubmit with this one to fix this issue, otherwise 
>> let's wait what idea do they have because I hate to spend time with 
>> something only to be discarded again. I think we don't need more complete 
>> emulation of this chip than this for now but if somebody wants to attempt 
>> that I don't mind as long as it does not break pegasos2.
>
> I had a play with your patches this afternoon, and spent some time performing 
> some experiments and also reading various PCI bus master specifications and 
> datasheets: this helped me understand a lot more about the theory of IRQ 
> routing and compatible vs. legacy mode.
>
> From reading all the documentation (including the VIA and other datasheets) I 
> cannot find any reference to a half-native mode which makes me think

The half-native mode is my simpler term for Linux's "non 100% native 
mode". This may not exist in hardware but exists as a concept in some 
Linux (and maybe other) drivers so emulating it just means we do what 
these drivers expect to work correctly.

How this maps to hardware and what interactions are there with firmware 
may be interesting but I'm not interested to find out as long as all 
guests we care about work because adding more complexity just for the sake 
of correctly modeling hardware seems like a waste of time in this case. 
Thanks for taking the time to find and document these though, it may be 
useful if someone wants to clean this up further. I'm satisfied with 
getting it in good enough shape for fuloong2e and pegasos2 to boot the 
guests we want, because I'd rather spend time on other, more interesting 
stuff such as writing replacement firmware for pegasos2 to avoid needing 
an undistributable ROM, implementing missing sound support, improving 
ati-vga or getting the Mac ROM work with g3beige, and also FPU emulation 
on PPC (and these are just the QEMU related stuff, I can think of others 
too). All of those seem time better spent than beating this via-ide model 
further now just for the sake of perfection without any gain, because 
guests will not work better even after spending more time with this. 
That's why I call it a waste of time. I know you prefer perfect patches 
but as they say "Perfect is the enemy of good." (I could think of better 
use of your time too such as finishing your screamer patches or improving 
OpenBIOS or your original sparc interest but that's for you to decide what 
you do.)

I also try to improve these models and add missing stuff as needed but my 
goal is not perfection because I don't have that much time, just reaching 
good enough. It can always be improved later (or corrected if it turns out 
to be needed as in this case) but if we always hold back until getting it 
perfect we wont get anywhere. If your level of perfection was a 
requirement in QEMU a lot of devices would not be there as they could not 
get in in the first place which means other people cannot improve it as 
there's nothing there to start with. So I think something that is good 
enough is at least a good start towards perfection.

We can argue what level is good enough. I think if it makes guests work 
which seems to be the general approach in QEMU as a lot of devices don't 
actually model real hardware correctly but just so that guests run with 
it. Of course we should make it clean and follow hardware where possible 
but a lot of models don't do that (maybe actually very few are anywhere 
near perfect).

> something else is wrong here. At the simplest level it could simply be that 
> the VIA doesn't tri-state its legacy IRQ lines whilst the device is in native 
> mode (the SI controller has an option for this), or it could indicate there 
> is a PCI IRQ routing problem somewhere else that hasn't been picked up yet.
>
> All of the datasheets suggest that legacy vs. native mode is selected by 
> setting the correct bits in PCI_CLASS_PROG, and Linux reads this byte and 
> configures itself to use legacy or native mode accordingly. Since the current 
> default for the VIA is 0x8a then it should default to legacy mode, but we're 
> immediately hitting some issues here: I've summarised my notes below for 
> those interested.
>
>
> 1) PCI bus reset loses the default BAR addresses
>
> The first problem we find is that the initialisation of the PCI bus erases 
> the default BAR addresses: that's to say lines 133-137 in hw/ide/via.c will 
> in effect do nothing:
>
> 133     pci_set_long(pci_conf + PCI_BASE_ADDRESS_0, 0x000001f0);
> 134     pci_set_long(pci_conf + PCI_BASE_ADDRESS_1, 0x000003f4);
> 135     pci_set_long(pci_conf + PCI_BASE_ADDRESS_2, 0x00000170);
> 136     pci_set_long(pci_conf + PCI_BASE_ADDRESS_3, 0x00000374);
> 137     pci_set_long(pci_conf + PCI_BASE_ADDRESS_4, 0x0000cc01); /* BMIBA: 
> 20-23h */
>
> The lifecycle of the VIA IDE device goes like this: init() -> realize() -> 
> reset() but then the PCI bus reset in pci_do_device_reset() immediately wipes 
> the BAR addresses. This is why the legacy IDE ports currently don't appear at 
> startup. Note I do see that other devices do try this e.g. 
> gt64120_pci_realize() so it's an easy mistake to make.

This is from the original commit 10 years ago so I think QEMU may have 
worked differently back then and possibly this worked and just left there 
because nobody noticed until now. I did notice PCI config values are reset 
when starting to work on this and on your suggestion fixed the problem for 
that one register in PCI reset code that I've worked around first in this 
model.

> 2) -kernel doesn't initialise the VIA device
>
> If you take a look at the PMON source it is possible to see that the firmware 
> explicitly sets the PCI_CLASS_PROG to compatibility mode and disables the 
> native PCI interrupt 
> (https://github.com/loongson-community/pmon-2ef/blob/master/sys/dev/pci/vt82c686.c#L82).
>
> Since Linux reads this byte on startup then this is why the kernel switches 
> to compatibility mode by default. However the point here is that booting a 
> kernel directly without firmware means the VIA IDE device isn't initialised 
> as it would be in real life, and that's why there are attempts to 
> pre-configure the device accordingly in via_ide_realize()/via_ide_reset().

Isn't this worked around by setting the mode to legacy at start up? Maybe 
you could emulate firmware in load_kernel() but I leave that exercise to 
somebody who is interested in running Linux on fuloong2e.

> 3) QEMU doesn't (easily) enable a BAR to be disabled
>
> The ideal situation would be for QEMU's VIA IDE device to check 
> PCI_CLASS_PROG and configure itself dynamically: with PCI_CLASS_PROG set for 
> legacy mode by default, the device can disable its BARs until they are 
> explicitly enabled.
>
> According to the PCI bus master specification the recommended behaviour for a 
> device in compatible mode is to ignore all writes to the BARs, and for all 
> BAR reads to return 0. This fits nicely with Guenter's finding that the BMDMA 
> BAR should not return a value in order for Linux to boot correctly in legacy 
> mode.
>
> Unfortunately there is no existing functionality for this in QEMU which means 
> you would have to do this manually by overriding the PCI config read/write 
> functions. This is trickier than it sounds because the reads/writes don't 
> necessarily have to be aligned to the BAR addresses in configuration space.

I did go through this too when I've prepared my original patches and got 
to the same conclusion.

> In summary whilst I'm not keen on the series in its current form, it 
> seems the best solution for now. I've got a few comments on the latest 
> version of the series which I will send along shortly.

Glad you've got to this at last. Would have probably saved some time if 
you accepted it back in March but that's gone now.

Regards,
BALATON Zoltan


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: Problems with irq mapping in qemu v5.2
  2020-12-25 23:43     ` BALATON Zoltan via
@ 2020-12-31 15:34       ` Peter Maydell
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Maydell @ 2020-12-31 15:34 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: Philippe Mathieu-Daudé,
	Mark Cave-Ayland, QEMU Developers, Guenter Roeck,
	Michael S. Tsirkin

On Fri, 25 Dec 2020 at 23:45, BALATON Zoltan via <qemu-devel@nongnu.org> wrote:
> For the Bamboo board we have 4 interrupts connected to the PCI bus in the
> board but also have a comment in ppc4xx_pci.c near the above function
> saying:
>
> /* On Bamboo, all pins from each slot are tied to a single board IRQ. This
>   * may need further refactoring for other boards. */
> static int ppc4xx_pci_map_irq(PCIDevice *pci_dev, int irq_num)
> {
>      int slot = pci_dev->devfn >> 3;
>
>      trace_ppc4xx_pci_map_irq(pci_dev->devfn, irq_num, slot);
>
>      return slot - 1;
> }
>
> Now I'm not sure what that comment means:

It definitely doesn't match the code, anyway...

> 1. All PCI INTA from all slots go to one irq, PCI INTB to another and so on
>
> or
>
> 2. All PCI interrupts (INTA-D) from first slot are connected to one irq on
> the board, then those from next slot are to another irq and so on

I looked at the datasheet for the PPC440EP SoC itself, and it
has only a single PCIINT pin, so all boards using this CPU
must end up only asserting one interrupt line into the interrupt
controller (presumably via an on-board OR gate?)

> The slot - 1 mapping seems to correspond more to 2. but that also means we
> can only have 4 slots.

You can have more than 4 slots -- one common approach is to
stripe the 4 interrupts across slots, so that if you have
4 interrupts 0, 1, 2, 3 then in slot 0 they are wired
INTA=0, INTB=1, INTC=2, INTD=3, and in slot 1 they are
INTA=1, INTB=2, INTC=3, INTD=0, and in slot 2 they are
INTA=2, INTB=3, INTC=4, INTD=1, and so on.
This just helps to spread the interrupt usage out and
minimise sharing of interrupts between devices, because most
PCI cards only ever use INTA (though they are permitted to use
all 4 if they have a need for it, I think.) This kind of striping
is just implemented by how you connect the pins on the PCI slots.
(This striping is what function pci_swizzle_map_irq_fn() implements.)

In QEMU's PCI implementation, it's OK for the mapping function
to reuse IRQ numbers -- PCI IRQ line changes go via pci_change_irq_level(),
which first calls the controller-specific map_irq function, and then
in pci_bus_change_irq_level() keeps a count of how many different
devices have asserted the (mapped) IRQ, so it calls the
controller's set_irq method with effectively the logical-OR of
all the input levels. (I was wrong about this in another email I
just wrote -- I'll go back and correct that in a moment.)

> I did not find a picture of the real board so don't
> know how many slots that has or how they are connected. I think we need
> more info on the real hardware to tell what's the correct mapping here.

Yeah, I couldn't find any documentation for the board itself either :-(

Incidentally, if guest software like Linux has been written and
tested on QEMU and not on the real hardware, then bugs in the
mapping of IRQs can be painful to try to fix, because if the
kernel was written with the equal-but-opposite bug it will then
not run on a bugfixed QEMU. We had a bit of a mess with the
QEMU versatilepb PCI controller that way.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2020-12-31 15:36 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.