Re: [RFC PATCH 0/3] dp8393x: Reviewing CRC code

[Finn Thain]

On Sat, 3 Jul 2021, Philippe Mathieu-Daudé wrote:

> Hi Mark, few more patches while reviewing.
> 
> 
> 
> Again, not tested (yet)... Simply compiled.
> 
> 
> 
> Please tell me what you think of them.
> 
> 

I think these 3 patches can be reduced to this theoretical bug fix:

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index db9cfd786f..e278daebc5 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -802,7 +802,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));
@@ -812,8 +812,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 */

> 
> Regards,
> 
> 
> 
> Phil.
> 
> 
> 
> Philippe Mathieu-Daudé (3):
> 
>   dp8393x: Store CRC using address_space_stl_le()
> 
>   dp8393x: Do not amend CRC if it is inhibited (CRCI bit set)
> 
>   dp8393x: Store CRC using device configured endianess
> 
> 
> 
>  hw/net/dp8393x.c | 26 ++++++++++++++++++--------
> 
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> 
> 
> -- 
> 
> 2.31.1
> 
> 
> 
>