From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>, qemu-devel@nongnu.org
Cc: "Hervé Poussineau" <hpoussin@reactos.org>,
"Laurent Vivier" <laurent@vivier.eu>,
"Finn Thain" <fthain@linux-m68k.org>
Subject: Re: [PATCH v3 6/8] dp8393x: Store CRC using device configured endianess
Date: Sat, 10 Jul 2021 22:28:58 +0100 [thread overview]
Message-ID: <36de5aff-cbf5-6398-774c-db482e1f1d35@ilande.co.uk> (raw)
In-Reply-To: <20210710174954.2577195-7-f4bug@amsat.org>
On 10/07/2021 18:49, Philippe Mathieu-Daudé wrote:
> Little-Endian CRC is dubious. The datasheet does not
> specify it being little-endian. Use big-endian access
> when the device is configured in such endianess.
> (This is a theoretical bug fix.)
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/net/dp8393x.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 68516241a1f..ac93412f70b 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -827,7 +827,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
> s->regs[SONIC_TRBA0] = s->regs[SONIC_CRBA0];
>
> /* Calculate the ethernet checksum */
> - checksum = cpu_to_le32(crc32(0, buf, pkt_size));
> + checksum = crc32(0, buf, pkt_size);
>
> /* Put packet into RBA */
> trace_dp8393x_receive_packet(dp8393x_crba(s));
> @@ -837,8 +837,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
> address += pkt_size;
>
> /* Put frame checksum into RBA */
> - address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED,
> - &checksum, sizeof(checksum));
> + if (s->big_endian) {
> + address_space_stl_be(&s->as, address, checksum,
> + MEMTXATTRS_UNSPECIFIED, NULL);
> + } else {
> + address_space_stl_le(&s->as, address, checksum,
> + MEMTXATTRS_UNSPECIFIED, NULL);
> + }
> address += sizeof(checksum);
>
> /* Pad short packets to keep pointers aligned */
This is obviously new to the series: I can test this on big endian m68k but are you
sure that this won't break big endian MIPS? Or do we not care for now since we don't
have a working test image?
ATB,
Mark.
next prev parent reply other threads:[~2021-07-10 21:30 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-10 17:49 [PATCH v3 0/8] dp8393x: fixes and improvements Philippe Mathieu-Daudé
2021-07-10 17:49 ` [PATCH v3 1/8] dp8393x: Replace address_space_rw(is_write=1) by address_space_write() Philippe Mathieu-Daudé
2021-07-10 21:16 ` Mark Cave-Ayland
2021-07-10 17:49 ` [PATCH v3 2/8] dp8393x: Replace 0x40 magic value by SONIC_REG16_COUNT definition Philippe Mathieu-Daudé
2021-07-10 17:53 ` Philippe Mathieu-Daudé
2021-07-10 21:17 ` Mark Cave-Ayland
2021-07-10 17:49 ` [PATCH v3 3/8] dp8393x: Only shift the device registers mapping by 1 bit Philippe Mathieu-Daudé
2021-07-10 21:23 ` Mark Cave-Ayland
2021-07-10 17:49 ` [PATCH v3 4/8] dp8393x: Store CAM registers as 16-bit Philippe Mathieu-Daudé
2021-07-10 21:24 ` Mark Cave-Ayland
2021-07-10 17:49 ` [PATCH v3 5/8] dp8393x: Migrate registers as array of uint16 Philippe Mathieu-Daudé
2021-07-10 21:25 ` Mark Cave-Ayland
2021-07-10 17:49 ` [PATCH v3 6/8] dp8393x: Store CRC using device configured endianess Philippe Mathieu-Daudé
2021-07-10 21:28 ` Mark Cave-Ayland [this message]
2021-07-10 17:49 ` [NOTFORMERGE PATCH v3 7/8] dp8393x: Rewrite dp8393x_get() / dp8393x_put() Philippe Mathieu-Daudé
2021-07-10 21:31 ` Mark Cave-Ayland
2021-07-10 17:49 ` [NOTFORMERGE PATCH v3 8/8] dp8393x: don't force 32-bit register access Philippe Mathieu-Daudé
2021-07-10 21:48 ` Mark Cave-Ayland
2021-07-11 2:08 ` [PATCH v3 0/8] dp8393x: fixes and improvements Finn Thain
2021-07-11 10:33 ` Philippe Mathieu-Daudé
2021-07-12 7:09 ` Finn Thain
2021-07-13 7:08 ` Finn Thain
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=36de5aff-cbf5-6398-774c-db482e1f1d35@ilande.co.uk \
--to=mark.cave-ayland@ilande.co.uk \
--cc=f4bug@amsat.org \
--cc=fthain@linux-m68k.org \
--cc=hpoussin@reactos.org \
--cc=laurent@vivier.eu \
--cc=qemu-devel@nongnu.org \
/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).