Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 1/6] libqos: add PPC64 PCI support

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2463 bytes --]

On Wed, Oct 05, 2016 at 09:33:06PM +0200, Greg Kurz wrote:
> On Wed, 5 Oct 2016 09:34:05 +0200
> Greg Kurz <groug@kaod.org> wrote:
> > On Wed, 5 Oct 2016 12:14:05 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > [...]
> > You convinced me. The tswaps in qtest.c are toxic and should be removed.
> > 
> > Thanks for the clarification.
> > 
> 
> Rewind. Cedric and I spent the whole day thinking about that, based on
> Peter's inputs. The conclusion is: the qtest accelerator replaces the
> real world CPU and and the test program simulates what the CPU actually
> does when running the guest driver code in a specific situation.

No, I still think Peter is dead wrong, as you can see in my various
replies

"What the CPU actually does" depends on a number of factors which
simply aren't modelled when qtest replaces the cpu, so it's not a
meaningful concept.

> If the guest driver performs a store to the device, and the CPU and
> device have different endianness, cpu_to_xxYY() in the driver code
> boils down to bswapYY(). Doing things like writel(cpu_to_beYY()) in
> the test program is thus wrong since it involves the host endianness,
> and the test program no longer simulates what the real CPU would do.
> The test program must hence do writel(bswapYY()) and send that to
> qtest.

Except that the bswap has to be conditional on whether the guest CPU's
notional endianness is the same as the device or not.  So, for say an
LE device that can appear on several platforms what we actually need
writel(guestcpu_to_le32(val)).  Except that if host and guest
(notional) endianness are different the result of that conversion
won't *actually* be LE32, it will be a swapped value in anticipation
of the swap back to guest endianness within the writel().

Why not just say what you actually want.  We want to write to an LE
device, so writel_le(val);

> If the host endianness differs from the simulated CPU, the value is in
> wrong order and must be byteswapped before being handed over to the memory
> layer.
> 
> This explains why qtest calls tswapYY() before cpu_physical_memory_write().

Yes, the tswap() correctly implements an operation with semantics that
are approximately never what you want.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]