From: "Michael S. Tsirkin" <mst@redhat.com>
To: Thomas Huth <thuth@redhat.com>
Cc: qemu-devel@nongnu.org, Jason Wang <jasowang@redhat.com>,
Stefan Weil <sw@weilnetz.de>,
Michael Nawrocki <michael.nawrocki@gtri.gatech.edu>,
qemu-s390x@nongnu.org
Subject: Re: [Qemu-devel] [PATCH for-2.11] hw/net/eepro100: Fix endianness problem on big endian hosts
Date: Fri, 17 Nov 2017 06:14:48 +0200 [thread overview]
Message-ID: <20171117061341-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <1510867014-13423-1-git-send-email-thuth@redhat.com>
On Thu, Nov 16, 2017 at 10:16:54PM +0100, Thomas Huth wrote:
> Since commit 1865e288a823c764cd4344d ("Fix eepro100 simple transmission
> mode"), the test/pxe-test is broken for the eepro100 device on big
> endian hosts. However, it seems like that commit did not introduce the
> problem, but just uncovered it: The EEPRO100State->tx.tbd_array_addr and
> EEPRO100State->tx.tcb_bytes fields are already in host byte order, since
> they have already been byte-swapped in the read_cb() function.
> Thus byte-swapping them in tx_command() again results in the wrong
> endianness. Removing the byte-swapping here fixes the pxe-test.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Thomas, how about adding sparse endian-ness annotations
a la le32 in Linux? We could then use static checkers to
catch these bugs at build time.
> ---
> hw/net/eepro100.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> index a63ed2c..03e00f7 100644
> --- a/hw/net/eepro100.c
> +++ b/hw/net/eepro100.c
> @@ -756,8 +756,8 @@ static void read_cb(EEPRO100State *s)
>
> static void tx_command(EEPRO100State *s)
> {
> - uint32_t tbd_array = le32_to_cpu(s->tx.tbd_array_addr);
> - uint16_t tcb_bytes = (le16_to_cpu(s->tx.tcb_bytes) & 0x3fff);
> + uint32_t tbd_array = s->tx.tbd_array_addr;
> + uint16_t tcb_bytes = s->tx.tcb_bytes & 0x3fff;
> /* Sends larger than MAX_ETH_FRAME_SIZE are allowed, up to 2600 bytes. */
> uint8_t buf[2600];
> uint16_t size = 0;
> --
> 1.8.3.1
next prev parent reply other threads:[~2017-11-17 4:14 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-16 21:16 [Qemu-devel] [PATCH for-2.11] hw/net/eepro100: Fix endianness problem on big endian hosts Thomas Huth
2017-11-17 2:44 ` Jason Wang
2017-11-17 4:14 ` Michael S. Tsirkin [this message]
2017-11-17 5:35 ` Stefan Weil
2017-11-17 7:20 ` Thomas Huth
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=20171117061341-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=jasowang@redhat.com \
--cc=michael.nawrocki@gtri.gatech.edu \
--cc=qemu-devel@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=sw@weilnetz.de \
--cc=thuth@redhat.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.