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

[David Gibson]

On Tue, Oct 11, 2016 at 09:55:54AM +0100, Peter Maydell wrote:
> On 11 October 2016 at 02:24, David Gibson <address@hidden> wrote:
> > On Mon, Oct 10, 2016 at 03:10:33PM +0100, Peter Maydell wrote:
> >> On 10 October 2016 at 14:39, David Gibson <address@hidden> wrote:
> >> > In the overwhelming majority of cases the endianness of the device is
> >> > known independent of the guest CPU and board.
> >>
> >> We don't seem to model things that way:
> >>
> >> $ git grep '.endianness = DEVICE_NATIVE' |wc -l
> >> 341
> >> $ git grep '.endianness = DEVICE_BIG' |wc -l
> >> 18
> >> $ git grep '.endianness = DEVICE_LITTLE' |wc -l
> >> 172
> >
> > Sigh.  So, most of these are themselves mistakes.
> >
> > A lot of these are target specific devices that should be tagged with
> > their target's (notional) endianness, rather than NATIVE.  That
> > covers:
> 
> > MICROBLAZE
> >         hw/dma/xilinx_axidma.c                            1
> >         hw/char/xilinx_uartlite.c                         1
> >         hw/intc/xilinx_intc.c                             1
> >         hw/net/xilinx_ethlite.c                           1
> >         hw/timer/xilinx_timer.c                           1
> >         hw/ssi/xilinx_spi.c                               1
> >                                                         ---
> >                 TOTAL                                     6
> 
> We build microblaze in both big and little endian configs...

And the on-board devices really have different endianness in those
cases?  Yuck.  I guess it's kind of plausible here since microblaze is
a kind of weird arch built around FPGA devi

> 
> > MIPS
> >         hw/mips                                           8
> >         hw/*/mips*                                        5
> >         hw/display/jazz_led.c                             1
> >         hw/dma/rc4030.c                                   2
> >         hw/net/dp8393x.c                                  1
> >         hw/pci-host/bonito.c                              5
> >                                                         ---
> >                 TOTAL                                    22
> 
> Ditto MIPS.

Ok, some of those devices (e.g. bonito and the other fulong bits) are
only for specific machines types which are BE or LE, not both.

> > SH
> >         hw/sh4                                            4
> >         hw/*/sh_*                                         3
> >         hw/display/sm501.c                                4
> >                                                         ---
> >                 TOTAL                                    11
> 
> And SH.

Hrm.. the kernel driver appears to treat sm501 on SH as LE always.
Has it actually been tested with sh4eb?

> > XTENSA
> >         hw/xtensa                                         3
> 
> And Xtensa.

Ok, so there are a few more "native endian" devices out there than I
thought.  For the FPGA-centric platforms like Microblaze, I guess that
means you can compile the peripherals, like the core, as LE or BE.
Kind of pointless, but whatever.  For the rest (assuming the real
hardware really truly exists in both endian variants) that almost
certainly means some of the boards have bridges which don't preserve
byte-order invariance.  That is horrifically broken.

Even so, it's still a small fraction of all devices.  We should be
designing our interfaces around the modern sane case, and having extra
workarounds for horrifically broken bridges, not the other way around.

I'm happy enough to retain "native endian" readw/writew operations in
qtest for such devices.  I just want there to be fixed-endian variants
*as well* - and I'm pretty sure those will be the predominantly useful
ones for any vaguely modern device.

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

signature.asc
Description: PGP signature