| Message ID | 20210710174954.2577195-7-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | dp8393x: fixes and improvements | expand |
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.
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 */
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(-)