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

["Andreas Färber" <andreas.faerber@web.de>]

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):

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,
 };

Andreas