Re: [Qemu-devel] [PATCH 00/28] Memory API for 1.6: fix I/O port endianness mess

[Paolo Bonzini <pbonzini@redhat.com>]

Il 25/07/2013 07:47, Benjamin Herrenschmidt ha scritto:
> On Thu, 2013-07-25 at 15:26 +1000, Benjamin Herrenschmidt wrote:
>> On Mon, 2013-07-22 at 10:34 -0500, Anthony Liguori wrote:
>>>
>>> Really nice series.  I'd prefer we simply got rid of the endianness
>>> flag
>>> entirely but this is a good step.
>>>
>>> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> Are you going to merge this ?

I guess so.

>> Afaik (Alexey just told me), pretty much anything IO is broken for
>> powerpc upstream and has been for weeks now ! It looks like the only
>> thing that got reverted was the VGA problem but everything else is still
>> busted including virtio.
>>
>> Why hasn't the original breakage been reverted immediately instead ?
> 
> It's actually worse than I thought. Alexey is showing me that in fact,
> even PCI MMIO is busted, using EHCI causes qemu to segfault for example.

The MMIO problems are unrelated.  They were reported by Nikunj in June
and they only happen with an unreleased version of SLOF that he's
working on (Linux works just fine).

Fixing it was on my todo list, then I had to do other stuff (like going
on holiday for 5 days and getting a thousand emails in your inbox) and I
prioritized the more general I/O port problem.  Anyhow the fix is in
patches 25-27 of this series, Nikunj just confirmed by testing them.

> This is a complete trainwreck. Why was that junk merged in the first
> place and why wasn't it immediately reverted ?

Perhaps you haven't noticed, but this list tends to have better social
standards than this.  Anyhow, here is an explanation.

The patches were absolutely not junk.  They removed a lot of messy old
code.  Unfortunately they didn't remove _enough_ messy old code.  This
series completes the work.

They were merged because:

1) we didn't know it broke all big endian targets.  There was some
suspicion that it broke pseries, but the idea was that removing the PCI
I/O hack would have fixed it.

2) In fact it did, but the dicussion derailed on endianness topics and
Alexey's patch to revert the PCI I/O hack wasn't accepted.

3) Sometimes we have to deal with old code written by people that aren't
contributors anymore and frankly no one understands it fully.  Breaking
stuff actually forces us to understand it and fix it the right way.  It
is not the first time it happens.


They weren't immediately reverted because:

4) the actual extent of the problem was not known.  Initial reports were
just about VGA, but that was not the case.

5) because we thought it was just about VGA, everybody thought that
Anthony's VGA fix would be the end of it.  (In the end Anthony's patch
was wrong and this series reverts it!)

6) the dicussion derailed on endianness topics and nobody posted a patch
to revert.


Fixing took a while because I originally thought I would have to rack
firmwares and images for all big-endian machine types.  Once it dawned
on me that I could just use a unit test, it took 5-6 hours to write the
testcase _and_ the fixes.

In any case, let me reinforce point 6 above.  The patches were not
reverted because nobody posted the patches to do so.  Until someone does
so, it is basically Anthony's call.  If he trusts people to fix the
whole thing, he has no reason to revert anything.  My track record of
fixing mess is probably better than my track record of not causing it,
so he didn't revert it.

Paolo