On 04/08/2014 08:51 AM, Peter Maydell wrote: > The raven_io_read() and raven_io_write() functions pass and > return values in little-endian format (since the IO op struct > is marked DEVICE_LITTLE_ENDIAN); however they were storing the > values in the buffer to pass to address_space_read/write() > in host-endian order, which meant that on big-endian hosts > the values were inadvertently reversed. Use the *_le_p() > accessors instead so that we are consistent regardless of > host endianness. > > Strictly speaking the byte order of the buffer for > address_space_rw() is target byte order (which for PPC > will be BE) but it doesn't actually matter as long as we > are consistent about the marking on the IO op struct and > which stl_*_p(). > > This bug was probably introduced due to confusion caused by > the two different versions of ldl_p() and friends: > bswap.h defines versions meaning "host endianness access" > cpu-all.h defines versions meaning "target endianness access" > As a target-independent source file prep.c gets the bswap.h > versions; the very similar looking code in ioport.c is > compiled per-target and gets the cpu-all.h versions. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > "Why is a raven like a writing desk? > Because it is nevar put with the wrong end in front!" > -- Lewis Carroll
Ha ha. > > This fixes the endianness test failure on bigendian hosts. > HOWEVER I have not actually tested it with a guest :-) > and endianness issues are notoriously hard to reason about > correctly. Review appreciated. > > RTH suggests that we rename the cpu-all.h ldl_p &c to > ldl_te_p() &c (for 'target endianness') to reduce confusion; > I agree but this probably also requires some auditing of > users to check for other mistaken uses and in any case is > 2.1 material. > > hw/pci-host/prep.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson <r...@twiddle.net> r~
