From: Bill Paul <wpaul@windriver.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Peter Maydell <peter.maydell@linaro.org>,
QEMU Developers <qemu-devel@nongnu.org>,
Andrey Smirnov <andrew.smirnov@gmail.com>,
Chris Healy <cphealy@gmail.com>, Jason Wang <jasowang@redhat.com>,
Jean-Christophe Dubois <jcd@tribudubois.net>
Subject: Re: [Qemu-devel] [PATCH] fsl-imx6: Swap Ethernet interrupt defines
Date: Fri, 9 Mar 2018 10:57:02 -0800 [thread overview]
Message-ID: <201803091057.02505.wpaul@windriver.com> (raw)
In-Reply-To: <201803091053.43494.wpaul@windriver.com>
Of all the gin joints in all the towns in all the world, Bill Paul had to walk
into mine at 10:53 on Friday 09 March 2018 and say:
> Of all the gin joints in all the towns in all the world, Guenter Roeck had
> to
>
> walk into mine at 10:20 on Friday 09 March 2018 and say:
> > On Fri, Mar 09, 2018 at 05:47:16PM +0000, Peter Maydell wrote:
> > > On 8 March 2018 at 18:28, Bill Paul <wpaul@windriver.com> wrote:
> > > > Anyway, this means that the only reason older Linux kernels worked in
> > > > QEMU with the broken interrupt configuration is that they also
> > > > registered a handler on vector 151 (119). Even though QEMU could not
> > > > send events via GPIO6, it was mistakenly sending them via vector 151,
> > > > so it looked like things worked. On real hardware, the older kernels
> > > > would have gotten their interrupts via GPIO6 and also worked. The
> > > > older kernels would _not_ work if you fix QEMU because now they would
> > > > never get interrupts on either vector, unless you fudge things so
> > > > that the ENET module triggers both vector 150 and the vector for
> > > > GPIO6 in the GIC or patch them to back out the erratum 6678
> > > > workaround as later kernels do.
> > >
> > > Thanks for that really useful writeup. So if I understand correctly
> > >
> > > we have several choices here:
> > > (1) we could implement a model of the IOMUX block that is at least
> > > sufficient to support guests that configure it to route the ENET
> > > interrupt line to a GPIO pin. Then we could apply this patch that
> > > fixes the ENET line definitions. Old kernels would continue to work
> > > (for the same reason they worked on hardware), and new ones would
> > > work now too. This is in some ways the preferred option, but it's
> > > possibly a lot of code and we're nearly in freeze for 2.12.
> > >
> > > (2) we could leave everything as it is for 2.12. This would mean that
> > > at least we don't regress setups that used to work on older QEMU
> > > versions. Downside is that we wouldn't be able to run Linux v4.15+, or
> > > other guest OSes that don't have the bug that older Linux kernels do.
> > > (Presumably we'd only do this on the understanding that we were going
> > > to go down route (1) for 2.13.)
> > >
> > > (3) we could apply this patch for 2.12. Linux v4.15+ now works, as
> > > do other guest OSes that use the ENET interrupt. v4.1 and older Linux
> > > guests that used to boot in QEMU stop doing so, and 4.2-4.9 boot but
> > > lose the ethernet device support. Perhaps for 2.13 we might
> > > take route (1) to make those older guests start working again.
> > >
> > > Do I have that right?
> >
> > Pretty much.
>
> There may be a 4th option.
>
> Since older kernels work because they were looking at vector 151, you could
> patch the imx_fec.c module so that when it signals an event for vector 150,
> it also signals vector 151 too. This would keep newer versions of QEMU
> "bug- compatible" with older versions while also fixing support for newer
> Linux kernels and other guests (e.g. VxWorks) that follow the hardware
> spec correctly.
Oops, just to be clear: I mean that the vector definitions should be fixed and
this backward-compatibility change should be applied at the same time.
-Bill
> I think this would require only a small modification to this function:
>
> static void imx_eth_update(IMXFECState *s)
> {
> if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_TS_TIMER) {
> qemu_set_irq(s->irq[1], 1);
> } else {
> qemu_set_irq(s->irq[1], 0);
> }
>
> if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] & ENET_INT_MAC) {
> qemu_set_irq(s->irq[0], 1);
> } else {
> qemu_set_irq(s->irq[0], 0);
> }
> }
>
> (For ENET_INT_MAC events, just signal both s->irq[0] and s->irq[1]).
>
> This of course means signaling spurious events on vector 151, but you're
> doing that now anyway. :)
>
> This is much less work than implementing an IOMUX module. Creating an IOMUX
> module for QEMU just for this one fixup would probably be the most elegant
> solution, but adding IOMUX support to QEMU also doesn't make much sense
> since QEMU has no actual pins.
>
> -Bill
>
> > > None of these options seems especially palatable to me, so we're
> > > choosing the lesser evil, I think... (unless somebody wants to say
> > > that option (1) would be 20 lines of code and here's the patch :-))
> > > I guess in the absence of (1) that (3) is better than (2) ?
> >
> > I would prefer (2). This is what I decided to use in my "local"
> > version of qemu. Older versions of Linux can be fixed by applying one
> > (4.2..4.9) or two (4.1 and older) upstream patches; anyone interested
> > running those kernels in qemu with Ethernet working should apply those
> > patches (or, alternatively, provide a patch adding IOMUX support to
> > qemu).
> >
> > Thanks,
> > Guenter
--
=============================================================================
-Bill Paul (510) 749-2329 | Senior Member of Technical Staff,
wpaul@windriver.com | Master of Unix-Fu - Wind River Systems
=============================================================================
"I put a dollar in a change machine. Nothing changed." - George Carlin
=============================================================================
next prev parent reply other threads:[~2018-03-09 18:58 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-07 17:37 [Qemu-devel] [PATCH] fsl-imx6: Swap Ethernet interrupt defines Guenter Roeck
2018-03-08 10:50 ` Peter Maydell
2018-03-08 14:47 ` Guenter Roeck
2018-03-08 14:51 ` Peter Maydell
2018-03-08 15:05 ` Guenter Roeck
2018-03-08 17:12 ` Guenter Roeck
2018-03-08 18:28 ` Bill Paul
2018-03-08 19:22 ` Guenter Roeck
2018-03-09 17:47 ` Peter Maydell
2018-03-09 18:20 ` Guenter Roeck
2018-03-09 18:48 ` Peter Maydell
2018-03-09 20:19 ` Guenter Roeck
2018-03-09 18:53 ` Bill Paul
2018-03-09 18:57 ` Bill Paul [this message]
2018-03-09 21:38 ` Guenter Roeck
2018-03-09 23:03 ` Bill Paul
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=201803091057.02505.wpaul@windriver.com \
--to=wpaul@windriver.com \
--cc=andrew.smirnov@gmail.com \
--cc=cphealy@gmail.com \
--cc=jasowang@redhat.com \
--cc=jcd@tribudubois.net \
--cc=linux@roeck-us.net \
--cc=peter.maydell@linaro.org \
--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).