[PULL,for-2.0,2/7] raven: Implement non-contiguous I/O region

Am 07.04.2014 21:32, schrieb Andreas Färber:
> Am 05.04.2014 22:26, schrieb Hervé Poussineau:
>> Hi Andreas,
>>
>> Le sam. 05 avril 2014 17:41:43 CEST, Andreas Färber a écrit :
>>> Hi Hervé,
>>>
>>> Am 20.03.2014 00:36, schrieb Andreas Färber:
>>>> From: Hervé Poussineau <hpoussin@reactos.org>
>>>>
>>>> Remove now duplicated code from prep board.
>>>>
>>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>>> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>>> ---
>>>>   hw/pci-host/prep.c | 85
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   hw/ppc/prep.c      | 94
>>>> ++----------------------------------------------------
>>>>   2 files changed, 88 insertions(+), 91 deletions(-)
>>>
>>> I'm facing endianness-test failures in -rc1 on both openSUSE ppc/ppc64
>>> and OSX ppc64 (below) as well as "broken pipe" on OSX ppc.
>>>
>>> $ make check-qtest-ppc V=1
>>> [...]
>>>    /ppc/endianness/prep:                                              **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:131:test_endianness:
>>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 ==
>>> 0x00008765)
>>> FAIL
>>> [...]
>>>    /ppc/endianness/split/prep:                                        **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:206:test_endianness_split:
>>>
>>> assertion failed (isa_inw(test, 0xe2) == 0x8765): (0x00004321 ==
>>> 0x00008765)
>>> FAIL
>>> [...]
>>>    /ppc/endianness/combine/prep:                                      **
>>> ERROR:/Users/andreas/QEMU/tests/endianness-test.c:253:test_endianness_combine:
>>>
>>> assertion failed (isa_inw(test, 0xea) == 0x8765): (0x00004321 ==
>>> 0x00008765)
>>> FAIL
>>> [...]
>>> FAIL: tests/endianness-test
>>>
>>> On x86 everything is fine. git-bisect points to this commit.
>>>
>>> There is one "FIXME: handle endianness switch" in here, but I don't spot
>>> such code where it's being moved from either.
>>>
>>> My suspect is the cpu_inw() -> ldl_p() change, but I'm unsure whether
>>> the code or the test is wrong...
>>
>> Code removed in this commit was using DEVICE_NATIVE_ENDIAN, and then
>> using cpu_inl, which does a ldl_p.
>> Code added in this commit is using DEVICE_LITTLE_ENDIAN, and then is
>> using ldl_p.
>> So, yes, it seems that endianness of memory region does change things.
>> Native endian means native endian of the guest of of the host?
>>
>> I also checked tests/endianness-test.c.
>> The failing test is:
>>    isa_outl(test, 0xe0, 0x87654321);
>>    g_assert_cmphex(isa_inl(test, 0xe0), ==, 0x87654321);
>>    g_assert_cmphex(isa_inw(test, 0xe2), ==, 0x8765);
>>    g_assert_cmphex(isa_inw(test, 0xe0), ==, 0x4321);
>> which seems perfectly valid as ISA is little-endian.
>>
>> However, PReP is marked as bswap= true in this file, which means that
>> values are inverted before writing and after reading them.
> 
> I tested .bswap = false - that fixes ppc64 host but breaks x86_64 host.

Same results for the following patch (x86_64 broken, ppc64 fixed):


Andreas

On 7 April 2014 21:40, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 07.04.2014 21:32, schrieb Andreas Färber:
>> I tested .bswap = false - that fixes ppc64 host but breaks x86_64 host.
>
> Same results for the following patch (x86_64 broken, ppc64 fixed):
>
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index d3e746c..fd3956f 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -177,7 +177,7 @@ static void raven_io_write(void *opaque, hwaddr addr,
>  static const MemoryRegionOps raven_io_ops = {
>      .read = raven_io_read,
>      .write = raven_io_write,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
>      .impl.max_access_size = 4,
>      .valid.unaligned = true,
>  };

Unsurprisingly, since both of those changes add/remove an
extra endianness swap for all host systems.

What you're looking for is the point in the chain where
we do something which is different depending on the
endianness of the host. You could stick in debug printfs
or just look around in gdb, to find out whether, for instance,
the values being passed into the raven_io_read/write
functions are different on the two hosts: if so then the
problem is somewhere further up the callstack...

thanks
-- PMM