[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 00/11] Fix versatile_pci

From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v4 00/11] Fix versatile_pci
Date: Mon, 8 Jul 2013 08:46:21 +0100

On 8 July 2013 07:43, Rob Landley <address@hidden> wrote:
> On 06/29/2013 06:03:26 AM, Peter Maydell wrote:
>> The "with the patch" case is uninteresting, because it's not
>> actually a fix for anything, it's just a change that happened
>> to work with old QEMU, and it's not a change that is in upstream
>> Linux.
> I want to create a kernel image that works with the existing QEMU image
> people are likely to have installed on their systems, _and_ with current
> versions of QEMU.
> Is that what you consider "uninteresting"?

 * kernels which actually work on real hardware
 * mainline kernels which worked on old QEMU, because they're
   widely used
 * local hacks which worked by fluke on old QEMU but won't
   work on the hardware
 * making mainline kernels which didn't work on old QEMU work
   by adapting to their bugginess

(current vanilla mainline falls into the last of these categories)

> I am trying to produce "an image that runs under qemu". That level of
> genericity worked for several years, at least ever since 1.0. When it
> stopped, I complained. You seem to be expressing incredulity at the concept.

QEMU has always been free to improve its emulation to be
closer to what the hardware does. If your guest doesn't
work on the hardware it is always going to be liable to

It's really common for new kernel changes to require new QEMU,
because they tickle some bit of hardware emulation we had omitted
or hadn't got right. If you want a kernel image that runs on
older QEMU, stick with an older kernel.

> I tried building a vanilla linux 3.10 kernel, and it doesn't work on qemu
> 1.5. I confirmed their current interrupt mapping does not match any known
> hardware or emulator, and that it changed more than once in incompatible
> ways as they did wild-ass-guess du jour about what it should be now.

Current vanilla works for the first PCI card, not for others.
It behaves like this on both QEMU 1.4 and 1.5, ie these changes
have not broken it further than it is already.

> So I did the obvious-to-me thing and patched it back to how it _was_, which
> is the only thing that works with previous versions of QEMU. You seem to
> find this approach shocking and unexpected.

You haven't patched it back like how it was, or it would work;
old 2.x kernels work fine. Old kernels wrote 27 to PCI_INTERRUPT_LINE,
not 59.

>> "would work on real hardware".
> The stuff that's so rare the kernel guys couldn't find any to test on?

They could, if they asked, if they cared. I actually went
around and tested Arnd's patchset back in 2011. Nobody's
ever asked me since. The point is that this patchset is a
line in the sand -- we now have a defensible set of behaviour,
because it's what the hardware does. Any future "hey, the kernel
changed and broke things" can be directed straight at the kernel
guys, because we're not changing any further unless it can be
demonstrated that we don't run a kernel that's been tested on h/w.

> Maybe I haven't been clear: I want something that works with current QEMU
> _and_ old QEMU.
> Your paragraph above implies that there's smoething _different_ I can write
> to PCI_INTERRUPT_LINE that will trigger the old behavior. So my patch to do
> the old IRQ (which worked fine with qemu 1.2) needs to be bigger to humor
> QEMU's heuristic. I'll try to figure out how to extend my patch to avoid
> triggering your new "does not work" mode.

The old mode is the broken one...

> Let's see, stick a printf into qemu, and:
>   =====Write 60=12,59=====
> So it's requesting IRQ 59, and QEMU's reponse is to _not_ give it IRQ 59
> unless we trigger the "broken" flag.
> So... real hardware doesn't get the IRQ it requests? Using the reqested IRQ
> is "broken"? (Apparently your patch for a real hardware kernel doesn't
> request the IRQ it's going to use? I'm confused...)

On real hardware, PCI_INTERRUPT_LINE has no effect -- all it
does is store a value for the kernel to read back. It has
no effect on the interrupt routing. (This is true for any
PCI controller.) However, the kernel happens to write a value
which corresponds to what it *thinks* the hardware is wired up to.
So we can use this to identify some of the mainline cases which
expect old broken behaviour.

> Right, easy enough to fix:
> --- a/arch/arm/mach-versatile/pci.c
> +++ b/arch/arm/mach-versatile/pci.c
> @@ -333,7 +333,13 @@ static int __init versatile_map_irq(const struct
> pci_dev *dev, u8 slot, u8 pin)
>          *  26     1     IRQ_SIC_PCI2
>          *  27     1     IRQ_SIC_PCI3
>          */
> -       irq = IRQ_SIC_PCI0 + ((slot - 24 + pin - 1) & 3);
> +       // Hit QEMU 1.5.0 and later with a brick so it uses the IRQ we say.

This comment is totally wrong. More accurate would be:
    // This code won't work on hardware because we still assume
    // the broken interrupt mapping used by old (pre 1.5.0)
    // versions of QEMU. Ensure that QEMU 1.5 is in its backward
    // compatibility mode by writing a definitely wrong number

Also you're probably better off doing this write in the
initialization code, rather than in versatile_map_irq().

> +       dev->bus->ops->write(dev->bus, dev->devfn, PCI_INTERRUPT_LINE, 1,
> 27);
> +
> +       // The kernel has no clue where IRQs are, and its current
> assignments
> +       // match neither the hardware nor historic QEMU. Use historic QEMU
> +       // for compatability with old versions.
> +       irq = 59; //IRQ_SIC_PCI0 + ((slot - 24 + pin - 1) & 3);
>         return irq;
>  }
> That should work with new _and_ old QEMU.

>> I tested these changes against a patchset which Arnd wrote which
>> I can guarantee was tested on real hardware because I did the
>> testing myself.
> Where is this patch? (Was it submitted upstream to linux-kernel?)

Yes. http://comments.gmane.org/gmane.linux.ports.arm.kernel/93393

>> > Do you have a kernel that runs under current qemu arm versatile
>> > emulation? I
>> > can poke around and figure out which irqs it expects _now_, but it's not
>> > "right" and presumably you're just going to change it again when you
>> > realize
>> > what qemu is doing and what the unpatched kernel is doing don't match.
>> The ones on http://people.debian.org/~aurel32/qemu/armel/
>> should work.
> I'm not seeing a patch there.

You asked for a kernel, not a patch. They're Debian's standard

-- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]