[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness |
Date: |
Wed, 5 Oct 2016 15:49:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 10/05/2016 02:31 PM, Peter Maydell wrote:
> On 4 October 2016 at 16:43, David Gibson <address@hidden> wrote:
>> On Tue, Oct 04, 2016 at 01:36:09PM +0100, Peter Maydell wrote:
>>> The difficulty with this patch is that it's hard to tell whether
>>> it's really required, or if this is just adding an extra layer
>>> of byteswapping that should really be done in some other location
>>
>> Actually, it's neither. It's not essential for anything, but it
>> *removes* an extra layer of byteswapping that really never should have
>> been done in the first place.
>
> The patch is very clearly adding calls to swapping functions.
> It looks like it's mostly convenience functions for not doing
> those swaps explicitly in the test cases.
>
>>> in the stack. What's the actual test case here?
>>
>> The current readw, readl, etc. all work in "guest endianness". But
>> guest endianness is not well defined - there are a number of targets
>> which can support either.
>
> It's guest bus endianness, and it's pretty well defined I think.
> (ARM for instance is LE bus even if the CPU is doing BE writes.)
>
>> And it's doubly meaningless since it's a
>> property of the guest cpu, which we're essentially replacing with the
>> qtest stub anyway.
>
> The stub sits on the same bus the guest cpu would.
>
>> Furthermore "guest endianness" isn't useful. With a tiny handful of
>> exceptions, all peripherals have their own endianness which is known
>> independent of the target. It makes more sense for test cases to
>> explicitly do their accesses in the correct endianness for the device,
>> without having to compensate for the fact that it'll be swapped into
>> the essentially arbitrary "guest endianness" along the way.
>
> Here I definitely disagree. I think it makes much more sense
> for writes to be "what the guest CPU would write", because that's
> where we're injecting them. If we had a test framework where the
> test was talking directly to the device, then maybe, but we don't.
This is how it all started with the test I wrote. Quite ignorant
of the middle layers, I used cpu_to_be32() as I would have done
on the guest and then realized that qtest was byte-swapping also
in some cases and so the test failed on a BE host.
>> Basically, the existing byteswaps, instead of removing the need for
>> them in the testcase code, instead mean that target-conditional
>> byteswaps will be *required* in nearly every testcase. It's a recipe
>> for endianness-unsafe testcases.
>
> I prefer the swaps to be explicit in the test cases. If your
> test case running on the real CPU would have had to do
> "swap to LE and then write this word", so does the test
> case in our test framework. If your test case just does
> "write the word", then so does the test.
>
> Most devices IME do not require byteswaps by guest code,
> and I think these functions are confusing -- if you try
> to use them to write tests for ARM devices, for instance,
> the _le versions of these functions will introduce an
> incorrect byteswap on a BE host, even though ARM CPUs and
> devices are typically all LE.
hmmm, yes I agree :/ My scenario is for a ARM SoC running LE
on a BE host and accessing a BE device.
So how do we handle the possible bswap due to the host and
guest endianness being different ? I am confused. Should we
try to remove the tswap from qtest ?
Thanks
C.
- [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Cédric Le Goater, 2016/10/04
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Peter Maydell, 2016/10/04
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Laurent Vivier, 2016/10/04
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Cédric Le Goater, 2016/10/04
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Paolo Bonzini, 2016/10/04
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, David Gibson, 2016/10/04
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Cédric Le Goater, 2016/10/05
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Peter Maydell, 2016/10/05
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness,
Cédric Le Goater <=
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Peter Maydell, 2016/10/05
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Cédric Le Goater, 2016/10/05
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Peter Maydell, 2016/10/05
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Cédric Le Goater, 2016/10/05
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Peter Maydell, 2016/10/05
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, David Gibson, 2016/10/05
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Paolo Bonzini, 2016/10/06
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, David Gibson, 2016/10/06
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Paolo Bonzini, 2016/10/06
- Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness, Cédric Le Goater, 2016/10/06