From 88f632fbb1b3d31d5b6978d28f8735a6ed18b8f5 Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Mon, 20 Jan 2020 09:59:21 +1100 Subject: dp8393x: Mask EOL bit from descriptor addresses The Least Significant bit of a descriptor address register is used as an EOL flag. It has to be masked when the register value is to be used as an actual address for copying memory around. But when the registers are to be updated the EOL bit should not be masked. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 70451934ae..216d44bd46 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -145,6 +145,9 @@ do { printf("sonic ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0) #define SONIC_ISR_PINT 0x0800 #define SONIC_ISR_LCD 0x1000 +#define SONIC_DESC_EOL 0x0001 +#define SONIC_DESC_ADDR 0xFFFE + #define TYPE_DP8393X "dp8393x" #define DP8393X(obj) OBJECT_CHECK(dp8393xState, (obj), TYPE_DP8393X) @@ -197,7 +200,8 @@ static uint32_t dp8393x_crba(dp8393xState *s) static uint32_t dp8393x_crda(dp8393xState *s) { - return (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]; + return (s->regs[SONIC_URDA] << 16) | + (s->regs[SONIC_CRDA] & SONIC_DESC_ADDR); } static uint32_t dp8393x_rbwc(dp8393xState *s) @@ -217,7 +221,8 @@ static uint32_t dp8393x_tsa(dp8393xState *s) static uint32_t dp8393x_ttda(dp8393xState *s) { - return (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]; + return (s->regs[SONIC_UTDA] << 16) | + (s->regs[SONIC_TTDA] & SONIC_DESC_ADDR); } static uint32_t dp8393x_wt(dp8393xState *s) @@ -509,7 +514,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) MEMTXATTRS_UNSPECIFIED, s->data, size); s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1; - if (dp8393x_get(s, width, 0) & 0x1) { + if (dp8393x_get(s, width, 0) & SONIC_DESC_EOL) { /* EOL detected */ break; } @@ -765,13 +770,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, /* XXX: Check byte ordering */ /* Check for EOL */ - if (s->regs[SONIC_LLFA] & 0x1) { + if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { /* Are we still in resource exhaustion? */ size = sizeof(uint16_t) * 1 * width; address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, s->data, size); - if (dp8393x_get(s, width, 0) & 0x1) { + if (dp8393x_get(s, width, 0) & SONIC_DESC_EOL) { /* Still EOL ; stop reception */ return -1; } else { @@ -831,7 +836,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, MEMTXATTRS_UNSPECIFIED, s->data, size); s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); - if (s->regs[SONIC_LLFA] & 0x1) { + if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { /* EOL detected */ s->regs[SONIC_ISR] |= SONIC_ISR_RDE; } else { -- cgit v1.2.3-55-g7522 From 3fe9a838ec3eae1374ced16b63bf56894b2ffbe6 Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Always use 32-bit accesses The DP83932 and DP83934 have 32 data lines. The datasheet says, Data Bus: These bidirectional lines are used to transfer data on the system bus. When the SONIC is a bus master, 16-bit data is transferred on D15-D0 and 32-bit data is transferred on D31-D0. When the SONIC is accessed as a slave, register data is driven onto lines D15-D0. D31-D16 are held TRI-STATE if SONIC is in 16-bit mode. If SONIC is in 32-bit mode, they are driven, but invalid. Always use 32-bit accesses both as bus master and bus slave. Force the MSW to zero in bus master mode. This gets the Linux 'jazzsonic' driver working, and avoids the need for prior hacks to make the NetBSD 'sn' driver work. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 47 +++++++++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 18 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 216d44bd46..51b71dac1e 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -246,9 +246,19 @@ static void dp8393x_put(dp8393xState *s, int width, int offset, uint16_t val) { if (s->big_endian) { - s->data[offset * width + width - 1] = cpu_to_be16(val); + if (width == 2) { + s->data[offset * 2] = 0; + s->data[offset * 2 + 1] = cpu_to_be16(val); + } else { + s->data[offset] = cpu_to_be16(val); + } } else { - s->data[offset * width] = cpu_to_le16(val); + if (width == 2) { + s->data[offset * 2] = cpu_to_le16(val); + s->data[offset * 2 + 1] = 0; + } else { + s->data[offset] = cpu_to_le16(val); + } } } @@ -590,7 +600,7 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size) DPRINTF("read 0x%04x from reg %s\n", val, reg_names[reg]); - return val; + return s->big_endian ? val << 16 : val; } static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data, @@ -598,13 +608,14 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data, { dp8393xState *s = opaque; int reg = addr >> s->it_shift; + uint32_t val = s->big_endian ? data >> 16 : data; - DPRINTF("write 0x%04x to reg %s\n", (uint16_t)data, reg_names[reg]); + DPRINTF("write 0x%04x to reg %s\n", (uint16_t)val, reg_names[reg]); switch (reg) { /* Command register */ case SONIC_CR: - dp8393x_do_command(s, data); + dp8393x_do_command(s, val); break; /* Prevent write to read-only registers */ case SONIC_CAP2: @@ -617,36 +628,36 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data, /* Accept write to some registers only when in reset mode */ case SONIC_DCR: if (s->regs[SONIC_CR] & SONIC_CR_RST) { - s->regs[reg] = data & 0xbfff; + s->regs[reg] = val & 0xbfff; } else { DPRINTF("writing to DCR invalid\n"); } break; case SONIC_DCR2: if (s->regs[SONIC_CR] & SONIC_CR_RST) { - s->regs[reg] = data & 0xf017; + s->regs[reg] = val & 0xf017; } else { DPRINTF("writing to DCR2 invalid\n"); } break; /* 12 lower bytes are Read Only */ case SONIC_TCR: - s->regs[reg] = data & 0xf000; + s->regs[reg] = val & 0xf000; break; /* 9 lower bytes are Read Only */ case SONIC_RCR: - s->regs[reg] = data & 0xffe0; + s->regs[reg] = val & 0xffe0; break; /* Ignore most significant bit */ case SONIC_IMR: - s->regs[reg] = data & 0x7fff; + s->regs[reg] = val & 0x7fff; dp8393x_update_irq(s); break; /* Clear bits by writing 1 to them */ case SONIC_ISR: - data &= s->regs[reg]; - s->regs[reg] &= ~data; - if (data & SONIC_ISR_RBE) { + val &= s->regs[reg]; + s->regs[reg] &= ~val; + if (val & SONIC_ISR_RBE) { dp8393x_do_read_rra(s); } dp8393x_update_irq(s); @@ -659,17 +670,17 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data, case SONIC_REA: case SONIC_RRP: case SONIC_RWP: - s->regs[reg] = data & 0xfffe; + s->regs[reg] = val & 0xfffe; break; /* Invert written value for some registers */ case SONIC_CRCT: case SONIC_FAET: case SONIC_MPT: - s->regs[reg] = data ^ 0xffff; + s->regs[reg] = val ^ 0xffff; break; /* All other registers have no special contrainst */ default: - s->regs[reg] = data; + s->regs[reg] = val; } if (reg == SONIC_WT0 || reg == SONIC_WT1) { @@ -680,8 +691,8 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data, static const MemoryRegionOps dp8393x_ops = { .read = dp8393x_read, .write = dp8393x_write, - .impl.min_access_size = 2, - .impl.max_access_size = 2, + .impl.min_access_size = 4, + .impl.max_access_size = 4, .endianness = DEVICE_NATIVE_ENDIAN, }; -- cgit v1.2.3-55-g7522 From 46ffee9ad43185cbee4182c208bbd534814086ca Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Clean up endianness hacks According to the datasheet, section 3.4.4, "in 32-bit mode ... the SONIC always writes long words". Therefore, use the same technique for the 'in_use' field that is used everywhere else, and write the full long word. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 51b71dac1e..1844482b3d 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -778,8 +778,6 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, return -1; } - /* XXX: Check byte ordering */ - /* Check for EOL */ if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { /* Are we still in resource exhaustion? */ @@ -851,15 +849,12 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, /* EOL detected */ s->regs[SONIC_ISR] |= SONIC_ISR_RDE; } else { - /* Clear in_use, but it is always 16bit wide */ - int offset = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; - if (s->big_endian && width == 2) { - /* we need to adjust the offset of the 16bit field */ - offset += sizeof(uint16_t); - } - s->data[0] = 0; - address_space_write(&s->as, offset, MEMTXATTRS_UNSPECIFIED, - s->data, sizeof(uint16_t)); + /* Clear in_use */ + size = sizeof(uint16_t) * width; + address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; + dp8393x_put(s, width, 0, 0); + address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, + s->data, size); s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); -- cgit v1.2.3-55-g7522 From 9e3cd456d85ad45e72bdba99203302342ce29b3b Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Have dp8393x_receive() return the packet size This function re-uses its 'size' argument as a scratch variable. Instead, declare a local 'size' variable for that purpose so that the function result doesn't get messed up. Signed-off-by: Finn Thain Reviewed-by: Philippe Mathieu-Daudé Tested-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 1844482b3d..ed57f5f326 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -759,20 +759,21 @@ static int dp8393x_receive_filter(dp8393xState *s, const uint8_t * buf, } static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, - size_t size) + size_t pkt_size) { dp8393xState *s = qemu_get_nic_opaque(nc); int packet_type; uint32_t available, address; - int width, rx_len = size; + int width, rx_len = pkt_size; uint32_t checksum; + int size; width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER | SONIC_RCR_CRCR | SONIC_RCR_LPKT | SONIC_RCR_BC | SONIC_RCR_MC); - packet_type = dp8393x_receive_filter(s, buf, size); + packet_type = dp8393x_receive_filter(s, buf, pkt_size); if (packet_type < 0) { DPRINTF("packet not for netcard\n"); return -1; @@ -868,7 +869,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, /* Done */ dp8393x_update_irq(s); - return size; + return pkt_size; } static void dp8393x_reset(DeviceState *dev) -- cgit v1.2.3-55-g7522 From 5b0c98fcb7ac006bd8efe0e0fecba52c43a9d028 Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Update LLFA and CRDA registers from rx descriptor Follow the algorithm given in the National Semiconductor DP83932C datasheet in section 3.4.7: At the next reception, the SONIC re-reads the last RXpkt.link field, and updates its CRDA register to point to the next descriptor. The chip is designed to allow the host to provide a new list of descriptors in this way. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index ed57f5f326..960462397e 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -786,12 +786,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED, s->data, size); - if (dp8393x_get(s, width, 0) & SONIC_DESC_EOL) { + s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0); + if (s->regs[SONIC_LLFA] & SONIC_DESC_EOL) { /* Still EOL ; stop reception */ return -1; - } else { - s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; } + /* Link has been updated by host */ + s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; } /* Save current position */ @@ -840,7 +841,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, MEMTXATTRS_UNSPECIFIED, s->data, size); - /* Move to next descriptor */ + /* Check link field */ size = sizeof(uint16_t) * width; address_space_read(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, @@ -856,6 +857,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, dp8393x_put(s, width, 0, 0); address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, s->data, size); + + /* Move to next descriptor */ s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); -- cgit v1.2.3-55-g7522 From a3cce2825a0b12bb717a5106daaca245557cc9ae Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Clear RRRA command register bit only when appropriate It doesn't make sense to clear the command register bit unless the command was actually issued. Signed-off-by: Finn Thain Reviewed-by: Philippe Mathieu-Daudé Tested-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 960462397e..b5a9c6ac0f 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -352,9 +352,6 @@ static void dp8393x_do_read_rra(dp8393xState *s) s->regs[SONIC_ISR] |= SONIC_ISR_RBE; dp8393x_update_irq(s); } - - /* Done */ - s->regs[SONIC_CR] &= ~SONIC_CR_RRRA; } static void dp8393x_do_software_reset(dp8393xState *s) @@ -565,8 +562,10 @@ static void dp8393x_do_command(dp8393xState *s, uint16_t command) dp8393x_do_start_timer(s); if (command & SONIC_CR_RST) dp8393x_do_software_reset(s); - if (command & SONIC_CR_RRRA) + if (command & SONIC_CR_RRRA) { dp8393x_do_read_rra(s); + s->regs[SONIC_CR] &= ~SONIC_CR_RRRA; + } if (command & SONIC_CR_LCAM) dp8393x_do_load_cam(s); } -- cgit v1.2.3-55-g7522 From ada74315270d1dcabf4c9d4fece19df7ef5b9577 Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Implement packet size limit and RBAE interrupt Add a bounds check to prevent a large packet from causing a buffer overflow. This is defensive programming -- I haven't actually tried sending an oversized packet or a jumbo ethernet frame. The SONIC handles packets that are too big for the buffer by raising the RBAE interrupt and dropping them. Linux uses that interrupt to count dropped packets. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index b5a9c6ac0f..911f59e0c9 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -137,6 +137,7 @@ do { printf("sonic ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0) #define SONIC_TCR_CRCI 0x2000 #define SONIC_TCR_PINT 0x8000 +#define SONIC_ISR_RBAE 0x0010 #define SONIC_ISR_RBE 0x0020 #define SONIC_ISR_RDE 0x0040 #define SONIC_ISR_TC 0x0080 @@ -772,6 +773,14 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER | SONIC_RCR_CRCR | SONIC_RCR_LPKT | SONIC_RCR_BC | SONIC_RCR_MC); + if (pkt_size + 4 > dp8393x_rbwc(s) * 2) { + DPRINTF("oversize packet, pkt_size is %d\n", pkt_size); + s->regs[SONIC_ISR] |= SONIC_ISR_RBAE; + dp8393x_update_irq(s); + dp8393x_do_read_rra(s); + return pkt_size; + } + packet_type = dp8393x_receive_filter(s, buf, pkt_size); if (packet_type < 0) { DPRINTF("packet not for netcard\n"); -- cgit v1.2.3-55-g7522 From bae112b80c9c42cea21ee7623c283668c3451c2e Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Don't clobber packet checksum A received packet consumes pkt_size bytes in the buffer and the frame checksum that's appended to it consumes another 4 bytes. The Receive Buffer Address register takes the former quantity into account but not the latter. So the next packet written to the buffer overwrites the frame checksum. Fix this. Signed-off-by: Finn Thain Reviewed-by: Philippe Mathieu-Daudé Tested-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 1 + 1 file changed, 1 insertion(+) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 911f59e0c9..63293411db 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -818,6 +818,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, address += rx_len; address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, &checksum, 4); + address += 4; rx_len += 4; s->regs[SONIC_CRBA1] = address >> 16; s->regs[SONIC_CRBA0] = address & 0xffff; -- cgit v1.2.3-55-g7522 From ea2270279bc2e1635cb6e909e22e17e630198773 Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Use long-word-aligned RRA pointers in 32-bit mode Section 3.4.1 of the datasheet says, The alignment of the RRA is confined to either word or long word boundaries, depending upon the data width mode. In 16-bit mode, the RRA must be aligned to a word boundary (A0 is always zero) and in 32-bit mode, the RRA is aligned to a long word boundary (A0 and A1 are always zero). This constraint has been implemented for 16-bit mode; implement it for 32-bit mode too. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 63293411db..d8bf248bc6 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -665,12 +665,16 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data, qemu_flush_queued_packets(qemu_get_queue(s->nic)); } break; - /* Ignore least significant bit */ + /* The guest is required to store aligned pointers here */ case SONIC_RSA: case SONIC_REA: case SONIC_RRP: case SONIC_RWP: - s->regs[reg] = val & 0xfffe; + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { + s->regs[reg] = val & 0xfffc; + } else { + s->regs[reg] = val & 0xfffe; + } break; /* Invert written value for some registers */ case SONIC_CRCT: -- cgit v1.2.3-55-g7522 From 350e7d9a77d3b9ac74d240e4b232db1ebe5c05bc Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Pad frames to word or long word boundary The existing code has a bug where the Remaining Buffer Word Count (RBWC) is calculated with a truncating division, which gives the wrong result for odd-sized packets. Section 1.4.1 of the datasheet says, Once the end of the packet has been reached, the serializer will fill out the last word (16-bit mode) or long word (32-bit mode) if the last byte did not end on a word or long word boundary respectively. The fill byte will be 0FFh. Implement buffer padding so that buffer limits are correctly enforced. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index d8bf248bc6..22b4d362e2 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -768,16 +768,23 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, dp8393xState *s = qemu_get_nic_opaque(nc); int packet_type; uint32_t available, address; - int width, rx_len = pkt_size; + int width, rx_len, padded_len; uint32_t checksum; int size; - width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; - s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER | SONIC_RCR_CRCR | SONIC_RCR_LPKT | SONIC_RCR_BC | SONIC_RCR_MC); - if (pkt_size + 4 > dp8393x_rbwc(s) * 2) { + rx_len = pkt_size + sizeof(checksum); + if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { + width = 2; + padded_len = ((rx_len - 1) | 3) + 1; + } else { + width = 1; + padded_len = ((rx_len - 1) | 1) + 1; + } + + if (padded_len > dp8393x_rbwc(s) * 2) { DPRINTF("oversize packet, pkt_size is %d\n", pkt_size); s->regs[SONIC_ISR] |= SONIC_ISR_RBAE; dp8393x_update_irq(s); @@ -812,22 +819,32 @@ 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, rx_len)); + checksum = cpu_to_le32(crc32(0, buf, pkt_size)); /* Put packet into RBA */ DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); address = dp8393x_crba(s); address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, - buf, rx_len); - address += rx_len; + buf, pkt_size); + address += pkt_size; + + /* Put frame checksum into RBA */ address_space_write(&s->as, address, MEMTXATTRS_UNSPECIFIED, - &checksum, 4); - address += 4; - rx_len += 4; + &checksum, sizeof(checksum)); + address += sizeof(checksum); + + /* Pad short packets to keep pointers aligned */ + if (rx_len < padded_len) { + size = padded_len - rx_len; + address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)"\xFF\xFF\xFF", size, 1); + address += size; + } + s->regs[SONIC_CRBA1] = address >> 16; s->regs[SONIC_CRBA0] = address & 0xffff; available = dp8393x_rbwc(s); - available -= rx_len / 2; + available -= padded_len >> 1; s->regs[SONIC_RBWC1] = available >> 16; s->regs[SONIC_RBWC0] = available & 0xffff; -- cgit v1.2.3-55-g7522 From d9fae13196a31716f45dcddcdd958fbb8e59b35a Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Clear descriptor in_use field to release packet When the SONIC receives a packet into the last available descriptor, it retains ownership of that descriptor for as long as necessary. Section 3.4.7 of the datasheet says, When the system appends more descriptors, the SONIC releases ownership of the descriptor after writing 0000h to the RXpkt.in_use field. The packet can now be processed by the host, so raise a PKTRX interrupt, just like the normal case. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 22b4d362e2..08194a4968 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -811,7 +811,17 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, return -1; } /* Link has been updated by host */ + + /* Clear in_use */ + size = sizeof(uint16_t) * width; + address = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width; + dp8393x_put(s, width, 0, 0); + address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, + (uint8_t *)s->data, size, 1); + + /* Move to next descriptor */ s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; + s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; } /* Save current position */ -- cgit v1.2.3-55-g7522 From 80b60673ea598869050c66d95d8339480e4cefd0 Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Always update RRA pointers and sequence numbers These operations need to take place regardless of whether or not rx descriptors have been used up (that is, EOL flag was observed). The algorithm is now the same for a packet that was withheld as for a packet that was not. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 08194a4968..cfbc2ee552 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -901,12 +901,14 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, /* Move to next descriptor */ s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; - s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff); + } - if (s->regs[SONIC_RCR] & SONIC_RCR_LPKT) { - /* Read next RRA */ - dp8393x_do_read_rra(s); - } + s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | + ((s->regs[SONIC_RSC] + 1) & 0x00ff); + + if (s->regs[SONIC_RCR] & SONIC_RCR_LPKT) { + /* Read next RRA */ + dp8393x_do_read_rra(s); } /* Done */ -- cgit v1.2.3-55-g7522 From 083e21bbdde7dbd326baf29d21f49fc3f5614496 Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Don't reset Silicon Revision register The jazzsonic driver in Linux uses the Silicon Revision register value to probe the chip. The driver fails unless the SR register contains 4. Unfortunately, reading this register in QEMU usually returns 0 because the s->regs[] array gets wiped after a software reset. Fixes: bd8f1ebce4 ("net/dp8393x: fix hardware reset") Suggested-by: Philippe Mathieu-Daudé Signed-off-by: Finn Thain Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index cfbc2ee552..c2ac2a110e 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -923,6 +923,7 @@ static void dp8393x_reset(DeviceState *dev) timer_del(s->watchdog); memset(s->regs, 0, sizeof(s->regs)); + s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux/mips */ s->regs[SONIC_CR] = SONIC_CR_RST | SONIC_CR_STP | SONIC_CR_RXDIS; s->regs[SONIC_DCR] &= ~(SONIC_DCR_EXBUS | SONIC_DCR_LBR); s->regs[SONIC_RCR] &= ~(SONIC_RCR_LB0 | SONIC_RCR_LB1 | SONIC_RCR_BRD | SONIC_RCR_RNT); @@ -975,7 +976,6 @@ static void dp8393x_realize(DeviceState *dev, Error **errp) qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s); - s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */ memory_region_init_ram(&s->prom, OBJECT(dev), "dp8393x-prom", SONIC_PROM_SIZE, &local_err); -- cgit v1.2.3-55-g7522 From c2279bd0a19b35057f2e4c3b4df9a915717d1142 Mon Sep 17 00:00:00 2001 From: Finn Thain Date: Wed, 29 Jan 2020 20:27:49 +1100 Subject: dp8393x: Don't stop reception upon RBE interrupt assertion Section 3.4.7 of the datasheet explains that, The RBE bit in the Interrupt Status register is set when the SONIC finishes using the second to last receive buffer and reads the last RRA descriptor. Actually, the SONIC is not truly out of resources, but gives the system an early warning of an impending out of resources condition. RBE does not mean actual receive buffer exhaustion, and reception should not be stopped. This is important because Linux will not check and clear the RBE interrupt until it receives another packet. But that won't happen if can_receive returns false. This bug causes the SONIC to become deaf (until reset). Fix this with a new flag to indicate actual receive buffer exhaustion. Signed-off-by: Finn Thain Tested-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) (limited to 'hw/net/dp8393x.c') diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index c2ac2a110e..8a3504d962 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -158,6 +158,7 @@ typedef struct dp8393xState { /* Hardware */ uint8_t it_shift; bool big_endian; + bool last_rba_is_full; qemu_irq irq; #ifdef DEBUG_SONIC int irq_level; @@ -347,12 +348,15 @@ static void dp8393x_do_read_rra(dp8393xState *s) s->regs[SONIC_RRP] = s->regs[SONIC_RSA]; } - /* Check resource exhaustion */ + /* Warn the host if CRBA now has the last available resource */ if (s->regs[SONIC_RRP] == s->regs[SONIC_RWP]) { s->regs[SONIC_ISR] |= SONIC_ISR_RBE; dp8393x_update_irq(s); } + + /* Allow packet reception */ + s->last_rba_is_full = false; } static void dp8393x_do_software_reset(dp8393xState *s) @@ -661,9 +665,6 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data, dp8393x_do_read_rra(s); } dp8393x_update_irq(s); - if (dp8393x_can_receive(s->nic->ncs)) { - qemu_flush_queued_packets(qemu_get_queue(s->nic)); - } break; /* The guest is required to store aligned pointers here */ case SONIC_RSA: @@ -723,8 +724,6 @@ static int dp8393x_can_receive(NetClientState *nc) if (!(s->regs[SONIC_CR] & SONIC_CR_RXEN)) return 0; - if (s->regs[SONIC_ISR] & SONIC_ISR_RBE) - return 0; return 1; } @@ -775,6 +774,10 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER | SONIC_RCR_CRCR | SONIC_RCR_LPKT | SONIC_RCR_BC | SONIC_RCR_MC); + if (s->last_rba_is_full) { + return pkt_size; + } + rx_len = pkt_size + sizeof(checksum); if (s->regs[SONIC_DCR] & SONIC_DCR_DW) { width = 2; @@ -788,8 +791,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, DPRINTF("oversize packet, pkt_size is %d\n", pkt_size); s->regs[SONIC_ISR] |= SONIC_ISR_RBAE; dp8393x_update_irq(s); - dp8393x_do_read_rra(s); - return pkt_size; + s->regs[SONIC_RCR] |= SONIC_RCR_LPKT; + goto done; } packet_type = dp8393x_receive_filter(s, buf, pkt_size); @@ -903,17 +906,23 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; } + dp8393x_update_irq(s); + s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | ((s->regs[SONIC_RSC] + 1) & 0x00ff); +done: + if (s->regs[SONIC_RCR] & SONIC_RCR_LPKT) { - /* Read next RRA */ - dp8393x_do_read_rra(s); + if (s->regs[SONIC_RRP] == s->regs[SONIC_RWP]) { + /* Stop packet reception */ + s->last_rba_is_full = true; + } else { + /* Read next resource */ + dp8393x_do_read_rra(s); + } } - /* Done */ - dp8393x_update_irq(s); - return pkt_size; } -- cgit v1.2.3-55-g7522