On Fri, 7 Oct 2016 14:56:26 +0200 Laurent Vivier <lviv...@redhat.com> wrote:
> On 07/10/2016 14:52, Peter Maydell wrote: > > On 7 October 2016 at 13:45, Greg Kurz <gr...@kaod.org> wrote: > >> On Fri, 7 Oct 2016 13:31:10 +0100 > >> Peter Maydell <peter.mayd...@linaro.org> wrote: > >> > >>> On 7 October 2016 at 13:27, Greg Kurz <gr...@kaod.org> wrote: > >>>> 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); > >>> > >>> Not really the same thing though -- virtio_is_big_endian > >>> in QEMU is indeed used only in virtio, because it makes > >>> dubious use of the internals of the CPU state. The > >>> equivalent of this proposed qtest function is the #define > >>> TARGET_BIG_ENDIAN, which is global to all of QEMU and > >>> reasonably widely used (because it's not a property of > >>> the CPU's internals). > >>> > >> > >> Indeed but is it expected to be used in other tests than > >> virtio ? > > > > Well, that's where we came in. > > > > Personally I'd rather see this patch purely fix the current > > rather dodgy implementation of the existing qtest_big_endian() > > function, which seems to be non-controversial, rather than > > getting bogged down too much in the questions about what the > > function name should be and how widely it should be used, etc. > > I'd rather too.. > Fair enough. > And I can rework this part later, as I've a series to enable virtio > tests for SPAPR. > > So if v2 covers all non virtio naming space issues, is it acceptable as-is? > What I said with v1 still stands: this is an improvement over what we currently have. Reviewed-by: Greg Kurz <gr...@kaod.org> > Thanks, > Laurent Cheers. -- Greg
