qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-devel@nongnu.org
Cc: patches@linaro.org, "Alexander Graf" <agraf@suse.de>,
	"Hervé Poussineau" <hpoussin@reactos.org>,
	qemu-ppc@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
	"Richard Henderson" <rth@twiddle.net>
Subject: [Qemu-devel] [PATCH for-2.0] hw/pci-host/prep: Don't reverse IO accesses on bigendian hosts
Date: Tue,  8 Apr 2014 16:51:11 +0100	[thread overview]
Message-ID: <1396972271-22660-1-git-send-email-peter.maydell@linaro.org> (raw)

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.maydell@linaro.org>
---
"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(-)

diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index d3e746c..4014540 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -145,9 +145,9 @@ static uint64_t raven_io_read(void *opaque, hwaddr addr,
     if (size == 1) {
         return buf[0];
     } else if (size == 2) {
-        return lduw_p(buf);
+        return lduw_le_p(buf);
     } else if (size == 4) {
-        return ldl_p(buf);
+        return ldl_le_p(buf);
     } else {
         g_assert_not_reached();
     }
@@ -164,9 +164,9 @@ static void raven_io_write(void *opaque, hwaddr addr,
     if (size == 1) {
         buf[0] = val;
     } else if (size == 2) {
-        stw_p(buf, val);
+        stw_le_p(buf, val);
     } else if (size == 4) {
-        stl_p(buf, val);
+        stl_le_p(buf, val);
     } else {
         g_assert_not_reached();
     }
-- 
1.9.1

             reply	other threads:[~2014-04-08 15:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-08 15:51 Peter Maydell [this message]
2014-04-08 17:08 ` [Qemu-devel] [PATCH for-2.0] hw/pci-host/prep: Don't reverse IO accesses on bigendian hosts Peter Maydell
2014-04-08 17:24 ` Richard Henderson
2014-04-08 17:53   ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1396972271-22660-1-git-send-email-peter.maydell@linaro.org \
    --to=peter.maydell@linaro.org \
    --cc=agraf@suse.de \
    --cc=hpoussin@reactos.org \
    --cc=patches@linaro.org \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).