Re: [Qemu-devel] [PATCH] qtest: add read/write accessors with a specific endianness

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

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

On Thu, Oct 06, 2016 at 04:11:42PM +0200, Greg Kurz wrote:
> On Thu, 6 Oct 2016 12:03:34 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> > On 6 October 2016 at 04:38, David Gibson <david@gibson.dropbear.id.au> wrote:
> > > On Wed, Oct 05, 2016 at 05:31:07AM -0700, Peter Maydell wrote:  
> > >> On 4 October 2016 at 16:43, David Gibson <david@gibson.dropbear.id.au> 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.  
> > >
> > > It's adding 1 swap on top of the memread/memwrite path - that's the
> > > path which had no existing swaps (intended primarily for bag-o'-bytes
> > > block access AFAICT).  
> > 
> > Yeah, I hadn't noticed it was using the memread/memwrite path.
> > I disagree with using that code path for what ought to be
> > register read/writes (among other things, it's not clear to me
> > that it guarantees that a 4-byte access by the test code is
> > always a 4-byte access on the device, etc).
> > 
> 
> FWIW, Cedric had another proposal which apparently went unnoticed:
> 
> <fc24ad74-da26-a713-9312-a2c2d07fb6a7@kaod.org>
> 
> The idea is to add an optional endianness argument to the read*/write*
> commands in the qtest protocol:
> - libqtest then provides explicit _le and _be APIs
> - no extra byteswap is performed on the test program side: qtest
>   actually handles that and does exactly 1 or 0 byteswap.
> - it does not use memread/memwrite
> - the current 'guest native' API where qtest tswaps is preserved

That would also be fine by me.

-- 
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 --]