Re: [Qemu-devel] [PATCH v2] qtest: ask endianness of the target in qtest_init()

[Greg Kurz <groug@kaod.org>]

On Fri, 7 Oct 2016 12:57:41 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 07/10/2016 12:48, Greg Kurz wrote:
> > On Fri,  7 Oct 2016 12:14:27 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> The target endianness is not deduced anymore from
> >> the architecture name but asked directly to the guest,
> >> using a new qtest command: "endianness". As it can't
> >> change (this is the value of TARGET_WORDS_BIGENDIAN),
> >> we store it to not have to ask every time we want to
> >> know if we have to byte-swap a value.
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> CC: Greg Kurz <groug@kaod.org>
> >> CC: David Gibson <david@gibson.dropbear.id.au>
> >> CC: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >> v2:
> >> - move the "endianness" command to a function and
> >>   don't move the qtest_init()/qtest_quit() functions
> >>  
> > 
> > Not speaking about the current discussion on TARGET_WORDS_BIGENDIAN,
> > I guess a consensus could be that this only makes sense when testing
> > legacy virtio. People should not be tempted to use this anywhere else
> > actually.  
> 
> I can rename target_big_endian() into qvirtio_is_big_endian() on the
> test side (and put it in libqos/virtio.h).
> 

Yes.

> I'd like to keep the qtest_big_endian() as it returns
> TARGET_WORDS_BIGENDIAN, and qvirtio_is_big_endian() will depend also on
> virtio-1.0 or not.
> 

Indeed but my suggestion is to open code this in qvirtio_is_big_endian(),
and even rename QTestState::big_endian to virtio_big_endian to make it
really obvious it should not be used elsewhere.

I now remember this is what I was resolutely suggested to do in
include/qom/cpu.h at the time we started to support ppc64le:

    bool (*virtio_is_big_endian)(CPUState *cpu);

Cheers.

--
Greg

> Laurent