[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH for-2.0] hw/pci-host/prep: Don't reverse IO access
Re: [Qemu-ppc] [PATCH for-2.0] hw/pci-host/prep: Don't reverse IO accesses on bigendian hosts
Tue, 08 Apr 2014 10:24:10 -0700
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0
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 <address@hidden>
> "Why is a raven like a writing desk?
> Because it is nevar put with the wrong end in front!"
> -- Lewis Carroll
> 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 <address@hidden>