From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: "Guenter Roeck" <linux@roeck-us.net>,
"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
"QEMU Developers" <qemu-devel@nongnu.org>,
"BALATON Zoltan" <balaton@eik.bme.hu>,
"John Snow" <jsnow@redhat.com>,
"Jiaxun Yang" <jiaxun.yang@flygoat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: Problems with irq mapping in qemu v5.2
Date: Wed, 23 Dec 2020 18:01:25 +0000 [thread overview]
Message-ID: <0c8d9c4f-cfd4-0df9-a722-14d624bfd646@ilande.co.uk> (raw)
In-Reply-To: <1d33fa46-2079-ad40-d579-6e827543de50@roeck-us.net>
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.
next prev parent reply other threads:[~2020-12-23 18:03 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-22 16:16 Problems with irq mapping in qemu v5.2 Guenter Roeck
2020-12-22 17:55 ` BALATON Zoltan via
2020-12-22 22:23 ` BALATON Zoltan via
2020-12-22 23:12 ` Guenter Roeck
2020-12-23 10:31 ` Mark Cave-Ayland
2020-12-23 13:39 ` BALATON Zoltan via
2020-12-22 18:23 ` Mark Cave-Ayland
2020-12-22 21:23 ` Guenter Roeck
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 [this message]
2020-12-23 20:20 ` BALATON Zoltan via
2020-12-23 21:01 ` Guenter Roeck
2020-12-23 22:05 ` Mark Cave-Ayland
2020-12-23 22:47 ` Guenter Roeck
2020-12-23 23:05 ` Philippe Mathieu-Daudé
2020-12-23 23:56 ` BALATON Zoltan via
2020-12-24 1:34 ` BALATON Zoltan via
2020-12-24 2:29 ` Jiaxun Yang
2020-12-24 5:32 ` Guenter Roeck
2020-12-24 8:11 ` BALATON Zoltan via
2020-12-24 10:50 ` Philippe Mathieu-Daudé
2020-12-24 17:09 ` BALATON Zoltan via
2020-12-28 19:26 ` Mark Cave-Ayland
2020-12-28 21:18 ` BALATON Zoltan via
2020-12-23 19:49 ` BALATON Zoltan via
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0c8d9c4f-cfd4-0df9-a722-14d624bfd646@ilande.co.uk \
--to=mark.cave-ayland@ilande.co.uk \
--cc=balaton@eik.bme.hu \
--cc=f4bug@amsat.org \
--cc=jiaxun.yang@flygoat.com \
--cc=jsnow@redhat.com \
--cc=linux@roeck-us.net \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).