[PATCH 00/10] Fixes for DP8393X SONIC device emulation

[PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

Hi All,

There is a bug in the DP8393X emulation that can stop packet reception.

Whilst debugging that issue I found that the receiver algorithm differs
from the one described in the National Semiconductor datasheet.

These issues and others are addressed by this patch series.

This series has only been tested with Linux/m68k guests. It needs further
testing with MIPS Jazz guests such as NetBSD or Windows NT.

Thanks.


Finn Thain (10):
  dp8393x: Mask EOL bit from descriptor addresses
  dp8393x: Clean up endianness hacks
  dp8393x: Have dp8393x_receive() return the packet size
  dp8393x: Don't advance RX descriptor twice
  dp8393x: Update LLFA register
  dp8393x: Clear RRRA command register bit only when appropriate
  dp8393x: Implement TBWC0 and TBWC1 registers to restore buffer state
  dp8393x: Implement packet size limit and RBAE interrupt
  dp8393x: Don't stop reception upon RBE interrupt assertion
  dp8393x: Don't clobber packet checksum

 hw/net/dp8393x.c | 80 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 28 deletions(-)

-- 
2.23.0

[PATCH 08/10] dp8393x: Implement packet size limit and RBAE interrupt

[Finn Thain]

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 <fthain@telegraphics.com.au>
---
QEMU passes short packets to dp8393x_receive(). But a real SONIC rejects
packets shorter than 64 bytes.

For dp8393x, the effective limit is 60 bytes because packets passed to
dp8393x_receive() lack a frame checksum. However, even 60 bytes proved
too high and blocked ARP packets when I tried it.

So the SONIC's mechanism for detecting runt packets (the RNT bit in the
RCR register) remains unimplemented in dp8393x (i.e. they are still
accepted).

This packet length issue may have implications for guests that depend on
the chip honouring the packet length threshold set in the EOBC register.
---
 hw/net/dp8393x.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 5e4494a945..24f2e19f07 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -139,6 +139,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
@@ -751,6 +752,14 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     uint32_t checksum;
     int size;
 
+    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;
+    }
+
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
 
     s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER |
-- 
2.23.0

[PATCH 09/10] dp8393x: Don't stop reception upon RBE interrupt assertion

[Finn Thain]

Section 5.4.7 of the datasheet explains that the RBE interrupt is an
"early warning". It indicates that the last RBA is still available to
receive a packet. It doesn't imply actual receive buffer exhaustion.

This is an important distinction 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. So the SONIC becomes deaf (until
reset).

Fix this with a new flag to indicate actual receive buffer exhaustion.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 hw/net/dp8393x.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 24f2e19f07..8e66b1f5de 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -157,6 +157,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;
@@ -311,6 +312,12 @@ static void dp8393x_do_read_rra(dp8393xState *s)
 {
     int width, size;
 
+    /* Already on the last RBA? */
+    s->last_rba_is_full = s->regs[SONIC_ISR] & SONIC_ISR_RBE;
+    if (s->last_rba_is_full) {
+        return;
+    }
+
     /* Read memory */
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
     size = sizeof(uint16_t) * 4 * width;
@@ -334,7 +341,7 @@ static void dp8393x_do_read_rra(dp8393xState *s)
         s->regs[SONIC_RRP] = s->regs[SONIC_RSA];
     }
 
-    /* Check resource exhaustion */
+    /* Warn the host if this entry is the last one */
     if (s->regs[SONIC_RRP] == s->regs[SONIC_RWP])
     {
         s->regs[SONIC_ISR] |= SONIC_ISR_RBE;
@@ -703,8 +710,9 @@ 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)
+    if (s->last_rba_is_full) {
         return 0;
+    }
     return 1;
 }
 
-- 
2.23.0

[PATCH 06/10] dp8393x: Clear RRRA command register bit only when appropriate

[Finn Thain]

It doesn't make sense to clear the command register bit unless the
command was actually issued.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 hw/net/dp8393x.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 494deb42bf..3fdc6cc6f9 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -337,9 +337,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)
@@ -548,8 +545,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);
 }
-- 
2.23.0

Re: [PATCH 06/10] dp8393x: Clear RRRA command register bit only when appropriate

[Philippe Mathieu-Daudé]

On 12/14/19 2:25 AM, Finn Thain wrote:
> It doesn't make sense to clear the command register bit unless the
> command was actually issued.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   hw/net/dp8393x.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 494deb42bf..3fdc6cc6f9 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -337,9 +337,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)
> @@ -548,8 +545,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);
>   }
> 

[PATCH 02/10] dp8393x: Clean up endianness hacks

[Finn Thain]

The in_use field is no different to the other words handled using
dp8393x_put() and dp8393x_get(). Use the same technique for in_use
that is used everywhere else.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
Laurent tells me that this clean-up has been tried before. He referred
me to commit c744cf7879 ("dp8393x: fix dp8393x_receive()") and
commit 409b52bfe1 ("net/dp8393x: correctly reset in_use field").

Both of those patches look wrong to me because they both pass the wrong
byte count to address_space_rw(). It's possible that those patches were
needed to work around some kind of bug elsewhere, for example, an
off-by-one result from dp8393x_crda(). The preceding patch in this series
might help there.

This needs testing with the little endian MIPS Jazz emulation.
---
 hw/net/dp8393x.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 164311c055..3fd59bc1d4 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -760,8 +760,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] & 0x1) {
         /* Are we still in resource exhaustion? */
@@ -831,15 +829,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 */
+        /* Clear in_use */
         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;
+        size = sizeof(uint16_t) * width;
+        dp8393x_put(s, width, 0, 0);
         address_space_rw(&s->as, offset, MEMTXATTRS_UNSPECIFIED,
-                         (uint8_t *)s->data, sizeof(uint16_t), 1);
+                         (uint8_t *)s->data, size, 1);
         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);
-- 
2.23.0

[PATCH 05/10] dp8393x: Update LLFA register

[Finn Thain]

If the chip used QEMU's algorithm it would need an extra word-sized
register just to recheck the EOL bit, which would be a waste of silicon.
Do it the way the chip would do it. No functional change.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 hw/net/dp8393x.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 49d7d9769e..494deb42bf 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -768,7 +768,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
         address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width;
         address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED,
                          (uint8_t *)s->data, size, 0);
-        if (dp8393x_get(s, width, 0) & 0x1) {
+        s->regs[SONIC_LLFA] = dp8393x_get(s, width, 0);
+        if (s->regs[SONIC_LLFA] & 0x1) {
             /* Still EOL ; stop reception */
             return -1;
         }
-- 
2.23.0

[PATCH 10/10] dp8393x: Don't clobber packet checksum

[Finn Thain]

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 <fthain@telegraphics.com.au>
---
 hw/net/dp8393x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 8e66b1f5de..9f4162c98c 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -810,6 +810,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     address += rx_len;
     address_space_rw(&s->as, address,
         MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1);
+    address += 4;
     rx_len += 4;
     s->regs[SONIC_CRBA1] = address >> 16;
     s->regs[SONIC_CRBA0] = address & 0xffff;
-- 
2.23.0

Re: [PATCH 10/10] dp8393x: Don't clobber packet checksum

[Philippe Mathieu-Daudé]

On 12/14/19 2:25 AM, Finn Thain wrote:
> 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 <fthain@telegraphics.com.au>
> ---
>   hw/net/dp8393x.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 8e66b1f5de..9f4162c98c 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -810,6 +810,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
>       address += rx_len;
>       address_space_rw(&s->as, address,
>           MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1);
> +    address += 4;
>       rx_len += 4;
>       s->regs[SONIC_CRBA1] = address >> 16;
>       s->regs[SONIC_CRBA0] = address & 0xffff;
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 07/10] dp8393x: Implement TBWC0 and TBWC1 registers to restore buffer state

[Finn Thain]

Restore the receive buffer state when the SONIC runs out of receive
descriptors. Otherwise it may write the next packet past the end of the
buffer and corrupt guest memory. This implements behaviour described
in section 3.4.6.2 in the datasheet.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 hw/net/dp8393x.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 3fdc6cc6f9..5e4494a945 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -39,7 +39,7 @@ static const char* reg_names[] = {
     "CR", "DCR", "RCR", "TCR", "IMR", "ISR", "UTDA", "CTDA",
     "TPS", "TFC", "TSA0", "TSA1", "TFS", "URDA", "CRDA", "CRBA0",
     "CRBA1", "RBWC0", "RBWC1", "EOBC", "URRA", "RSA", "REA", "RRP",
-    "RWP", "TRBA0", "TRBA1", "0x1b", "0x1c", "0x1d", "0x1e", "LLFA",
+    "RWP", "TRBA0", "TRBA1", "TBWC0", "TBWC1", "0x1d", "0x1e", "LLFA",
     "TTDA", "CEP", "CAP2", "CAP1", "CAP0", "CE", "CDP", "CDC",
     "SR", "WT0", "WT1", "RSC", "CRCT", "FAET", "MPT", "MDT",
     "0x30", "0x31", "0x32", "0x33", "0x34", "0x35", "0x36", "0x37",
@@ -78,6 +78,8 @@ do { printf("sonic ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0)
 #define SONIC_RWP    0x18
 #define SONIC_TRBA0  0x19
 #define SONIC_TRBA1  0x1a
+#define SONIC_TBWC0  0x1b
+#define SONIC_TBWC1  0x1c
 #define SONIC_LLFA   0x1f
 #define SONIC_TTDA   0x20
 #define SONIC_CEP    0x21
@@ -777,6 +779,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     /* Save current position */
     s->regs[SONIC_TRBA1] = s->regs[SONIC_CRBA1];
     s->regs[SONIC_TRBA0] = s->regs[SONIC_CRBA0];
+    s->regs[SONIC_TBWC1] = s->regs[SONIC_RBWC1];
+    s->regs[SONIC_TBWC0] = s->regs[SONIC_RBWC0];
 
     /* Calculate the ethernet checksum */
     checksum = cpu_to_le32(crc32(0, buf, rx_len));
@@ -827,6 +831,12 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     if (s->regs[SONIC_LLFA] & 0x1) {
         /* EOL detected */
         s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
+
+        /* Restore buffer state */
+        s->regs[SONIC_CRBA1] = s->regs[SONIC_TRBA1];
+        s->regs[SONIC_CRBA0] = s->regs[SONIC_TRBA0];
+        s->regs[SONIC_RBWC1] = s->regs[SONIC_TBWC1];
+        s->regs[SONIC_RBWC0] = s->regs[SONIC_TBWC0];
     } else {
         /* Clear in_use */
         int offset = dp8393x_crda(s) + sizeof(uint16_t) * 6 * width;
-- 
2.23.0

[PATCH 01/10] dp8393x: Mask EOL bit from descriptor addresses

[Finn Thain]

The LSB of descriptor address registers is used as an EOL flag.
It has to be masked when those registers are to be used as actual
addresses for copying memory around. But when the registers are
to be updated the EOL bit should not be masked.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 hw/net/dp8393x.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 3d991af163..164311c055 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -197,7 +197,7 @@ 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] & 0xfffe);
 }
 
 static uint32_t dp8393x_rbwc(dp8393xState *s)
@@ -217,7 +217,7 @@ 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] & 0xfffe);
 }
 
 static uint32_t dp8393x_wt(dp8393xState *s)
@@ -506,8 +506,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
                              sizeof(uint16_t) *
                              (4 + 3 * s->regs[SONIC_TFC]) * width,
                 MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0);
-            s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1;
-            if (dp8393x_get(s, width, 0) & 0x1) {
+            s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0);
+            if (s->regs[SONIC_CTDA] & 0x1) {
                 /* EOL detected */
                 break;
             }
-- 
2.23.0

Re: [PATCH 01/10] dp8393x: Mask EOL bit from descriptor addresses

[Philippe Mathieu-Daudé]

Hi Finn,

On 12/14/19 2:25 AM, Finn Thain wrote:
> The LSB of descriptor address registers is used as an EOL flag.
> It has to be masked when those registers are to be used as actual
> addresses for copying memory around. But when the registers are
> to be updated the EOL bit should not be masked.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
>   hw/net/dp8393x.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 3d991af163..164311c055 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -197,7 +197,7 @@ 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] & 0xfffe);
>   }
>   
>   static uint32_t dp8393x_rbwc(dp8393xState *s)
> @@ -217,7 +217,7 @@ 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] & 0xfffe);
>   }
>   
>   static uint32_t dp8393x_wt(dp8393xState *s)
> @@ -506,8 +506,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
>                                sizeof(uint16_t) *
>                                (4 + 3 * s->regs[SONIC_TFC]) * width,
>                   MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0);
> -            s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1;
> -            if (dp8393x_get(s, width, 0) & 0x1) {
> +            s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0);
> +            if (s->regs[SONIC_CTDA] & 0x1) {

Can you add a definition for the EOL bit and use it, instead of these 
magic 0x1/0xfffe values? That way the meaning will be obvious for future 
reviewers.

Thanks,

Phil.

>                   /* EOL detected */
>                   break;
>               }
> 

Re: [PATCH 01/10] dp8393x: Mask EOL bit from descriptor addresses

[Finn Thain]

On Sat, 14 Dec 2019, Philippe Mathieu-Daud? wrote:

> Hi Finn,
> 
> On 12/14/19 2:25 AM, Finn Thain wrote:
> > The LSB of descriptor address registers is used as an EOL flag.
> > It has to be masked when those registers are to be used as actual
> > addresses for copying memory around. But when the registers are
> > to be updated the EOL bit should not be masked.
> > 
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> > ---
> >   hw/net/dp8393x.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> > index 3d991af163..164311c055 100644
> > --- a/hw/net/dp8393x.c
> > +++ b/hw/net/dp8393x.c
> > @@ -197,7 +197,7 @@ 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] & 0xfffe);
> >   }
> >     static uint32_t dp8393x_rbwc(dp8393xState *s)
> > @@ -217,7 +217,7 @@ 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] & 0xfffe);
> >   }
> >     static uint32_t dp8393x_wt(dp8393xState *s)
> > @@ -506,8 +506,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
> >                                sizeof(uint16_t) *
> >                                (4 + 3 * s->regs[SONIC_TFC]) * width,
> >                   MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0);
> > -            s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1;
> > -            if (dp8393x_get(s, width, 0) & 0x1) {
> > +            s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0);
> > +            if (s->regs[SONIC_CTDA] & 0x1) {
> 
> Can you add a definition for the EOL bit and use it, instead of these magic
> 0x1/0xfffe values? That way the meaning will be obvious for future reviewers.
> 

Sure. I'll prepare v2.

Thanks for your review.

[PATCH 04/10] dp8393x: Don't advance RX descriptor twice

[Finn Thain]

Follow the algorithm given in the National Semiconductor DP83932C
datasheet in sections 3.4.5 and 3.4.6.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 hw/net/dp8393x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 462f8646e0..49d7d9769e 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -771,8 +771,6 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
         if (dp8393x_get(s, width, 0) & 0x1) {
             /* Still EOL ; stop reception */
             return -1;
-        } else {
-            s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
         }
     }
 
@@ -821,7 +819,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     address_space_rw(&s->as, dp8393x_crda(s),
         MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 1);
 
-    /* Move to next descriptor */
+    /* Check link field */
     size = sizeof(uint16_t) * width;
     address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width,
         MEMTXATTRS_UNSPECIFIED, (uint8_t *)s->data, size, 0);
@@ -836,6 +834,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
         dp8393x_put(s, width, 0, 0);
         address_space_rw(&s->as, offset, 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;
         s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff);
-- 
2.23.0

[PATCH 03/10] dp8393x: Have dp8393x_receive() return the packet size

[Finn Thain]

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 <fthain@telegraphics.com.au>
---
 hw/net/dp8393x.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 3fd59bc1d4..462f8646e0 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -741,20 +741,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;
@@ -848,7 +849,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)
-- 
2.23.0

Re: [PATCH 03/10] dp8393x: Have dp8393x_receive() return the packet size

[Philippe Mathieu-Daudé]

Hi Finn,

On 12/14/19 2:25 AM, Finn Thain wrote:
> 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 <fthain@telegraphics.com.au>
> ---
>   hw/net/dp8393x.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 3fd59bc1d4..462f8646e0 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -741,20 +741,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;

I'm tempted to remove rx_len and use pkt_size, pkt_size + 
sizeof(checksum) in place.

Anyway:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>       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;
> @@ -848,7 +849,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)
> 

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[no-reply]

Patchew URL: https://patchew.org/QEMU/cover.1576286757.git.fthain@telegraphics.com.au/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH 00/10] Fixes for DP8393X SONIC device emulation
Type: series
Message-id: cover.1576286757.git.fthain@telegraphics.com.au

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
702b708 dp8393x: Don't clobber packet checksum
a6efce5 dp8393x: Don't stop reception upon RBE interrupt assertion
8f04361 dp8393x: Implement packet size limit and RBAE interrupt
19bdaec dp8393x: Implement TBWC0 and TBWC1 registers to restore buffer state
d4634fd dp8393x: Clear RRRA command register bit only when appropriate
39e35db dp8393x: Update LLFA register
240cef4 dp8393x: Don't advance RX descriptor twice
ba2922d dp8393x: Have dp8393x_receive() return the packet size
8e1c5a6 dp8393x: Clean up endianness hacks
9c9ffc3 dp8393x: Mask EOL bit from descriptor addresses

=== OUTPUT BEGIN ===
1/10 Checking commit 9c9ffc38e9b9 (dp8393x: Mask EOL bit from descriptor addresses)
ERROR: return is not a function, parentheses are not required
#24: FILE: hw/net/dp8393x.c:200:
+    return (s->regs[SONIC_URDA] << 16) | (s->regs[SONIC_CRDA] & 0xfffe);

ERROR: return is not a function, parentheses are not required
#33: FILE: hw/net/dp8393x.c:220:
+    return (s->regs[SONIC_UTDA] << 16) | (s->regs[SONIC_TTDA] & 0xfffe);

total: 2 errors, 0 warnings, 26 lines checked

Patch 1/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/10 Checking commit 8e1c5a688838 (dp8393x: Clean up endianness hacks)
3/10 Checking commit ba2922dc3e93 (dp8393x: Have dp8393x_receive() return the packet size)
4/10 Checking commit 240cef4caaee (dp8393x: Don't advance RX descriptor twice)
5/10 Checking commit 39e35db107bd (dp8393x: Update LLFA register)
6/10 Checking commit d4634fdd244c (dp8393x: Clear RRRA command register bit only when appropriate)
7/10 Checking commit 19bdaec299f1 (dp8393x: Implement TBWC0 and TBWC1 registers to restore buffer state)
8/10 Checking commit 8f04361b6bc8 (dp8393x: Implement packet size limit and RBAE interrupt)
9/10 Checking commit a6efce5b17d0 (dp8393x: Don't stop reception upon RBE interrupt assertion)
10/10 Checking commit 702b708cf099 (dp8393x: Don't clobber packet checksum)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/cover.1576286757.git.fthain@telegraphics.com.au/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Fri, 13 Dec 2019, no-reply@patchew.org wrote:

> === OUTPUT BEGIN ===
> 1/10 Checking commit 9c9ffc38e9b9 (dp8393x: Mask EOL bit from descriptor addresses)
> ERROR: return is not a function, parentheses are not required
> #24: FILE: hw/net/dp8393x.c:200:
> +    return (s->regs[SONIC_URDA] << 16) | (s->regs[SONIC_CRDA] & 0xfffe);
> 
> ERROR: return is not a function, parentheses are not required
> #33: FILE: hw/net/dp8393x.c:220:
> +    return (s->regs[SONIC_UTDA] << 16) | (s->regs[SONIC_TTDA] & 0xfffe);
> 

I expect that checkpatch.pl has no idea about operator precedence, but 
these parentheses could actually be omitted.

I kept them because I don't want readers to have to remember that bit 
shift operator has higher precedence than bitwise OR operator, or look it 
up if they don't.

The existing code also has those unnecessary parentheses.

Please let me know if this patch should include a code style change.

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Philippe Mathieu-Daudé]

On 12/14/19 3:52 AM, Finn Thain wrote:
> On Fri, 13 Dec 2019, no-reply@patchew.org wrote:
> 
>> === OUTPUT BEGIN ===
>> 1/10 Checking commit 9c9ffc38e9b9 (dp8393x: Mask EOL bit from descriptor addresses)
>> ERROR: return is not a function, parentheses are not required
>> #24: FILE: hw/net/dp8393x.c:200:
>> +    return (s->regs[SONIC_URDA] << 16) | (s->regs[SONIC_CRDA] & 0xfffe);
>>
>> ERROR: return is not a function, parentheses are not required
>> #33: FILE: hw/net/dp8393x.c:220:
>> +    return (s->regs[SONIC_UTDA] << 16) | (s->regs[SONIC_TTDA] & 0xfffe);
>>
> 
> I expect that checkpatch.pl has no idea about operator precedence, but
> these parentheses could actually be omitted.
> 
> I kept them because I don't want readers to have to remember that bit
> shift operator has higher precedence than bitwise OR operator, or look it
> up if they don't.
> 
> The existing code also has those unnecessary parentheses.
> 
> Please let me know if this patch should include a code style change.

This is a bug in checkpatch. Since this script doesn't have dedicated 
maintainer, I Cc'ed the recent contributors:

$ ./scripts/get_maintainer.pl -f scripts/checkpatch.pl
get_maintainer.pl: No maintainers found, printing recent contributors.

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Eric Blake]

On 12/14/19 7:38 AM, Philippe Mathieu-Daudé wrote:
> On 12/14/19 3:52 AM, Finn Thain wrote:
>> On Fri, 13 Dec 2019, no-reply@patchew.org wrote:
>>
>>> === OUTPUT BEGIN ===
>>> 1/10 Checking commit 9c9ffc38e9b9 (dp8393x: Mask EOL bit from 
>>> descriptor addresses)
>>> ERROR: return is not a function, parentheses are not required
>>> #24: FILE: hw/net/dp8393x.c:200:
>>> +    return (s->regs[SONIC_URDA] << 16) | (s->regs[SONIC_CRDA] & 
>>> 0xfffe);
>>>
>>> ERROR: return is not a function, parentheses are not required
>>> #33: FILE: hw/net/dp8393x.c:220:
>>> +    return (s->regs[SONIC_UTDA] << 16) | (s->regs[SONIC_TTDA] & 
>>> 0xfffe);
>>>
>>
>> I expect that checkpatch.pl has no idea about operator precedence, but
>> these parentheses could actually be omitted.

You are correct: It's a false positive; you can safely ignore it.

>>
>> I kept them because I don't want readers to have to remember that bit
>> shift operator has higher precedence than bitwise OR operator, or look it
>> up if they don't.
>>
>> The existing code also has those unnecessary parentheses.
>>
>> Please let me know if this patch should include a code style change.
> 
> This is a bug in checkpatch. Since this script doesn't have dedicated 
> maintainer, I Cc'ed the recent contributors:

However, it's complex enough, and the false positive occurs infrequently 
enough, that just ignoring it (instead of trying to patch checkpatch) is 
also fine, and probably what will happen.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Aleksandar Markovic]

[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]

On Saturday, December 14, 2019, Finn Thain <fthain@telegraphics.com.au>
wrote:

> Hi All,
>
> There is a bug in the DP8393X emulation that can stop packet reception.
>
>
Can you provide the details of your test scenario?

Thanks,
Aleksandar




> Whilst debugging that issue I found that the receiver algorithm differs
> from the one described in the National Semiconductor datasheet.
>
> These issues and others are addressed by this patch series.
>
> This series has only been tested with Linux/m68k guests. It needs further
> testing with MIPS Jazz guests such as NetBSD or Windows NT.
>
> Thanks.
>
>
> Finn Thain (10):
>   dp8393x: Mask EOL bit from descriptor addresses
>   dp8393x: Clean up endianness hacks
>   dp8393x: Have dp8393x_receive() return the packet size
>   dp8393x: Don't advance RX descriptor twice
>   dp8393x: Update LLFA register
>   dp8393x: Clear RRRA command register bit only when appropriate
>   dp8393x: Implement TBWC0 and TBWC1 registers to restore buffer state
>   dp8393x: Implement packet size limit and RBAE interrupt
>   dp8393x: Don't stop reception upon RBE interrupt assertion
>   dp8393x: Don't clobber packet checksum
>
>  hw/net/dp8393x.c | 80 +++++++++++++++++++++++++++++++-----------------
>  1 file changed, 52 insertions(+), 28 deletions(-)
>
> --
> 2.23.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 1845 bytes --]

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Sat, 14 Dec 2019, Aleksandar Markovic wrote:

> On Saturday, December 14, 2019, Finn Thain <fthain@telegraphics.com.au>
> wrote:
> 
> > Hi All,
> >
> > There is a bug in the DP8393X emulation that can stop packet reception.
> >
> >
> Can you provide the details of your test scenario?
> 
> Thanks,
> Aleksandar
> 

I test the qemu build like this,

qemu-system-m68k -M q800 -m 512M -serial none -serial mon:stdio -g 800x600x4
-net nic,model=dp83932,addr=00:00:00:01:02:03
-net bridge,helper=/opt/qemu/libexec/qemu-bridge-helper,br=br0
-append "fbcon=font:ProFont6x11 console=tty0 console=ttyS0 ignore_loglevel"
-kernel vmlinux-4.14.157-mac-backport+
-initrd /mnt/loop/install/cdrom/initrd.gz

You can obtain this kernel binary from the linux-mac68k project on 
sourceforge. (I usually use a mainline Linux build but it makes no 
difference.)

I normally use a disk image with Debian/m68k SID rootfs but in this 
example I've used the initrd that you can find on the Debian/m68k 
installer ISO.

Once the guest starts, switch to a different virtual console and bring up 
the SONIC:

<ctrl>-<a> <ctrl>-<a> <2>
# ip addr add dev eth0 192.168.65.2/24
# ip link set dev eth0 up

On the host, send a ping flood (with preload) to the guest:

# ifconfig br0 192.168.65.1/24
# ping 192.168.65.2 -f -l 20

The packet reception ("deaf sonic") issue is reproduced immediately.

This has been observed in both qemu-m68k and mainline qemu. See also,
https://github.com/vivier/qemu-m68k/commit/0a45280c9fa40da8d5f30b1bb3d0513db91c3909

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Aleksandar Markovic]

[-- Attachment #1: Type: text/plain, Size: 1745 bytes --]

On Sunday, December 15, 2019, Finn Thain <fthain@telegraphics.com.au> wrote:

> On Sat, 14 Dec 2019, Aleksandar Markovic wrote:
>
> > On Saturday, December 14, 2019, Finn Thain <fthain@telegraphics.com.au>
> > wrote:
> >
> > > Hi All,
> > >
> > > There is a bug in the DP8393X emulation that can stop packet reception.
> > >
> > >
> > Can you provide the details of your test scenario?
> >
> > Thanks,
> > Aleksandar
> >
>
> I test the qemu build like this,
>
> qemu-system-m68k -M q800 -m 512M -serial none -serial mon:stdio -g
> 800x600x4
> -net nic,model=dp83932,addr=00:00:00:01:02:03
> -net bridge,helper=/opt/qemu/libexec/qemu-bridge-helper,br=br0
> -append "fbcon=font:ProFont6x11 console=tty0 console=ttyS0 ignore_loglevel"
> -kernel vmlinux-4.14.157-mac-backport+
> -initrd /mnt/loop/install/cdrom/initrd.gz
>
> You can obtain this kernel binary from the linux-mac68k project on
> sourceforge. (I usually use a mainline Linux build but it makes no
> difference.)
>
> I normally use a disk image with Debian/m68k SID rootfs but in this
> example I've used the initrd that you can find on the Debian/m68k
> installer ISO.
>
> Once the guest starts, switch to a different virtual console and bring up
> the SONIC:
>
> <ctrl>-<a> <ctrl>-<a> <2>
> # ip addr add dev eth0 192.168.65.2/24
> # ip link set dev eth0 up
>
> On the host, send a ping flood (with preload) to the guest:
>
> # ifconfig br0 192.168.65.1/24
> # ping 192.168.65.2 -f -l 20
>
> The packet reception ("deaf sonic") issue is reproduced immediately.
>
> This has been observed in both qemu-m68k and mainline qemu. See also,
> https://github.com/vivier/qemu-m68k/commit/0a45280c9fa40da8d5f30b1bb3d051
> 3db91c3909
>
>
Finn,

I appreciate your detailed response!

Aleksandar

[-- Attachment #2: Type: text/html, Size: 2592 bytes --]

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Aleksandar Markovic]

[-- Attachment #1: Type: text/plain, Size: 2051 bytes --]

On Sunday, December 15, 2019, Aleksandar Markovic <
aleksandar.m.mail@gmail.com> wrote:

>
>
> On Sunday, December 15, 2019, Finn Thain <fthain@telegraphics.com.au>
> wrote:
>
>> On Sat, 14 Dec 2019, Aleksandar Markovic wrote:
>>
>> > On Saturday, December 14, 2019, Finn Thain <fthain@telegraphics.com.au>
>> > wrote:
>> >
>> > > Hi All,
>> > >
>> > > There is a bug in the DP8393X emulation that can stop packet
>> reception.
>> > >
>> > >
>> > Can you provide the details of your test scenario?
>> >
>> > Thanks,
>> > Aleksandar
>> >
>>
>> I test the qemu build like this,
>>
>> qemu-system-m68k -M q800 -m 512M -serial none -serial mon:stdio -g
>> 800x600x4
>> -net nic,model=dp83932,addr=00:00:00:01:02:03
>> -net bridge,helper=/opt/qemu/libexec/qemu-bridge-helper,br=br0
>> -append "fbcon=font:ProFont6x11 console=tty0 console=ttyS0
>> ignore_loglevel"
>> -kernel vmlinux-4.14.157-mac-backport+
>> -initrd /mnt/loop/install/cdrom/initrd.gz
>>
>> You can obtain this kernel binary from the linux-mac68k project on
>> sourceforge. (I usually use a mainline Linux build but it makes no
>> difference.)
>>
>> I normally use a disk image with Debian/m68k SID rootfs but in this
>> example I've used the initrd that you can find on the Debian/m68k
>> installer ISO.
>>
>> Once the guest starts, switch to a different virtual console and bring up
>> the SONIC:
>>
>> <ctrl>-<a> <ctrl>-<a> <2>
>> # ip addr add dev eth0 192.168.65.2/24
>> # ip link set dev eth0 up
>>
>> On the host, send a ping flood (with preload) to the guest:
>>
>> # ifconfig br0 192.168.65.1/24
>> # ping 192.168.65.2 -f -l 20
>>
>> The packet reception ("deaf sonic") issue is reproduced immediately.
>>
>> This has been observed in both qemu-m68k and mainline qemu. See also,
>> https://github.com/vivier/qemu-m68k/commit/0a45280c9fa40da8d
>> 5f30b1bb3d0513db91c3909
>>
>>
> Finn,
>
> I appreciate your detailed response!
>
> Aleksandar
>
>


Herve,

Is there any way for us to come up with an equivalent or at least
approximate scenario for Jazz machines?

Regards,
Aleksandar

[-- Attachment #2: Type: text/html, Size: 3117 bytes --]

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Sun, 15 Dec 2019, Aleksandar Markovic wrote:

> 
> Herve,
> 
> Is there any way for us to come up with an equivalent or at least
> approximate scenario for Jazz machines?
> 
> Regards,
> Aleksandar
> 

That would be useful in general, but in this case I think it might be 
better to test NetBSD, since I have already tested Linux. (I had to fix 
some bugs in the Linux sonic driver.)

I tried to boot NetBSD/arc but failed. I got a blue screen when I typed 
"cd:boot" at the "Run A Program" prompt in the ARC menu.

$ ln -s NTPROM.RAW mipsel_bios.bin
$ mips64el-softmmu/qemu-system-mips64el -M magnum -L . 
-drive if=scsi,unit=2,media=cdrom,format=raw,file=NetBSD-8.1-arc.iso 
-global ds1225y.filename=nvram -global ds1225y.size=8200
qemu-system-mips64el: g364: invalid read at [0000000000102000]
$ 

Any help would be appreciated.

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Philippe Mathieu-Daudé]

Hi Finn,

On 12/20/19 5:24 AM, Finn Thain wrote:
> On Sun, 15 Dec 2019, Aleksandar Markovic wrote:
> 
>>
>> Herve,
>>
>> Is there any way for us to come up with an equivalent or at least
>> approximate scenario for Jazz machines?
>>
>> Regards,
>> Aleksandar
>>
> 
> That would be useful in general, but in this case I think it might be
> better to test NetBSD, since I have already tested Linux. (I had to fix
> some bugs in the Linux sonic driver.)
> 
> I tried to boot NetBSD/arc but failed. I got a blue screen when I typed
> "cd:boot" at the "Run A Program" prompt in the ARC menu.
> 
> $ ln -s NTPROM.RAW mipsel_bios.bin
> $ mips64el-softmmu/qemu-system-mips64el -M magnum -L .
> -drive if=scsi,unit=2,media=cdrom,format=raw,file=NetBSD-8.1-arc.iso
> -global ds1225y.filename=nvram -global ds1225y.size=8200
> qemu-system-mips64el: g364: invalid read at [0000000000102000]
> $
> 
> Any help would be appreciated.

Please open a new bug entry with this information at 
https://bugs.launchpad.net/qemu/+filebug

Thanks,

Phil.

NetBSD/arc on MIPS Magnum, was Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Mon, 23 Dec 2019, Philippe Mathieu-Daud? wrote:

> Hi Finn,
> 
> On 12/20/19 5:24 AM, Finn Thain wrote:
> > On Sun, 15 Dec 2019, Aleksandar Markovic wrote:
> > 
> > > 
> > > Herve,
> > > 
> > > Is there any way for us to come up with an equivalent or at least
> > > approximate scenario for Jazz machines?
> > > 
> > > Regards,
> > > Aleksandar
> > > 
> > 
> > That would be useful in general, but in this case I think it might be
> > better to test NetBSD, since I have already tested Linux. (I had to fix
> > some bugs in the Linux sonic driver.)
> > 
> > I tried to boot NetBSD/arc but failed. I got a blue screen when I typed
> > "cd:boot" at the "Run A Program" prompt in the ARC menu.
> > 
> > $ ln -s NTPROM.RAW mipsel_bios.bin
> > $ mips64el-softmmu/qemu-system-mips64el -M magnum -L .
> > -drive if=scsi,unit=2,media=cdrom,format=raw,file=NetBSD-8.1-arc.iso
> > -global ds1225y.filename=nvram -global ds1225y.size=8200
> > qemu-system-mips64el: g364: invalid read at [0000000000102000]
> > $
> > 
> > Any help would be appreciated.
> 
> Please open a new bug entry with this information at
> https://bugs.launchpad.net/qemu/+filebug
> 

I know precious little about NetBSD installation and MIPS Magnum. What I 
wrote above was guesswork. Hence this could be a NetBSD bug or user error.

Does there exist a known-good combination of NetBSD/arc and 
qemu-system-mips64el releases?

If so, I could use that to check for user error or possible NetBSD issue.

> Thanks,
> 
> Phil.
> 
> 

Re: NetBSD/arc on MIPS Magnum, was Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Tue, 24 Dec 2019, Finn Thain wrote:

> 
> I know precious little about NetBSD installation and MIPS Magnum. What I 
> wrote above was guesswork. Hence this could be a NetBSD bug or user 
> error.
> 

It was bugs and user error.

The user error was not using the serial console. The NetBSD/arc 
installation guide says that only serial console is supported for MIPS 
Magnum.

The bugs include regressions in NetBSD. (See below.)

The other issue is that the ARC firmware didn't work properly until I 
defined one or more 'boot selections', even though none of these will ever 
be selected.

> Does there exist a known-good combination of NetBSD/arc and 
> qemu-system-mips64el releases?
> 

The commit log says that Herv? Poussineau used NetBSD 5.1 with dp8393x in 
the past, so I tried that.

Here are the steps I used:

./mips64el-softmmu/qemu-system-mips64el -M magnum -L .  
-drive if=scsi,unit=2,media=cdrom,format=raw,file=arccd-5.1.iso
-global ds1225y.filename=nvram -global ds1225y.size=8200
-serial stdio -serial null
-nic bridge,model=dp83932,mac=00:00:00:02:03:04

-> Run setup -> Initialize system -> Set default configurations
	800x688
	3.5 1.44 M
	No
	7

-> Set default environment
	CD-ROM
	2

-> Set environment variables
	CONSOLEIN
	multi()serial(0)term()
	CONSOLEOUT
	multi()serial(0)term()

-> Exit

Now restart QEMU. The ARC menu should appear on the tty.

-> Run a program

	scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)netbsd

That doesn't work. Add a boot selection.

-> Run setup -> Manage startup -> Add a boot selection -> Scsi CD-ROM 0
	\os\nt\osloader.exe
	Yes
	\winnt
	Windows NT
	No

Somehow, that seems to help. Now restart QEMU.

-> Run a program

        scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)netbsd

NetBSD/arc Bootstrap, Revision 1.1
(builds@b7.netbsd.org, Sat Nov  6 14:06:36 UTC 2010)
devopen: scsi(0)cdrom(2)fdisk(0) type disk file netbsd
5502064+289092=0x5860e0
Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
    2006, 2007, 2008, 2009, 2010
    The NetBSD Foundation, Inc.  All rights reserved.
Copyright (c) 1982, 1986, 1989, 1991, 1993
    The Regents of the University of California.  All rights reserved.

NetBSD 5.1 (RAMDISK) #0: Sat Nov  6 14:17:36 UTC 2010
        builds@b7.netbsd.org:/home/builds/ab/netbsd-5-1-RELEASE/arc/201011061943Z-obj/home/builds/ab/netbsd-5-1-RELEASE/src/sys/arch/arc/compile/RAMDISK
MIPS Magnum
total memory = 128 MB
avail memory = 117 MB
mainbus0 (root)
cpu0 at mainbus0: MIPS R4000 CPU (0x400) Rev. 0.0 with MIPS R4010 FPC Rev. 0.0
cpu0: 8KB/16B direct-mapped L1 Instruction cache, 48 TLB entries
cpu0: 8KB/16B direct-mapped write-back L1 Data cache
jazzio0 at mainbus0
timer0 at jazzio0 addr 0xe0000228
mcclock0 at jazzio0 addr 0xe0004000: mc146818 compatible time-of-day clock
LPT1 at jazzio0 addr 0xe0008000 intr 0 not configured
fdc0 at jazzio0 addr 0xe0003000 intr 1
fd0 at fdc0 drive 1: 1.44MB, 80 cyl, 2 head, 18 sec
MAGNUM at jazzio0 addr 0xe000c000 intr 2 not configured
VXL at jazzio0 addr 0xe0800000 intr 3 not configured
sn0 at jazzio0 addr 0xe0001000 intr 4: SONIC Ethernet
sonic: write 0x0015 to reg CR
sonic: write 0x0080 to reg CR
sonic: write 0x0000 to reg IMR
sonic: write 0x7fff to reg ISR
sonic: write 0x0000 to reg CR
sn0: Ethernet address 00:00:00:00:00:00
asc0 at jazzio0 addr 0xe0002000 intr 5: NCR53C94, 25MHz, SCSI ID 7
scsibus0 at asc0: 8 targets, 8 luns per target
pckbc0 at jazzio0 addr 0xe0005000 intr 6
pckbd0 at pckbc0 (kbd slot)
wskbd0 at pckbd0 (mux ignored)
pms at jazzio0 addr 0xe0005000 intr 7 not configured
com0 at jazzio0 addr 0xe0006000 intr 8: ns16550a, working fifo
com0: txfifo disabled
com0: console
com1 at jazzio0 addr 0xe0007000 intr 9: ns16550a, working fifo
com1: txfifo disabled
jazzisabr0 at mainbus0
isa0 at jazzisabr0
isapnp0 at isa0 port 0x279: ISA Plug 'n Play device support
scsibus0: waiting 2 seconds for devices to settle...
cd0 at scsibus0 target 2 lun 0: <QEMU, QEMU CD-ROM, 2.5+> cdrom removable
cd1 at scsibus0 target 4 lun 0: <QEMU, QEMU CD-ROM, 2.5+> cdrom removable
boot device: <unknown>
root on md0a dumps on md0b
root file system type: ffs
WARNING: preposterous TOD clock time
WARNING: using filesystem time
WARNING: CHECK AND RESET THE DATE!
erase ^H, werase ^W, kill ^U, intr ^C, status ^T
Terminal type? [vt100] 
Erase is backspace.                                                     
(I)nstall, (S)hell or (H)alt ? s
# ifconfig sn0 10.2.3.4/24
# ping
usage: 
ping [-adDfLnoPqQrRv] [-c count] [-g gateway] [-h host] [-i interval] [-I addr]
     [-l preload] [-p pattern] [-s size] [-t tos] [-T ttl] [-w maxwait] host


My initial testing shows that NetBSD 5.1 doesn't like my v2 patch series. 
I'll debug that before I send v3.

BTW, there seem to be regressions in NetBSD 8.1 compared to NetBSD 5.1. 

The 'boot' program on the 8.1 ISO just hangs.

If I use the 'boot' program from the 5.1 ISO to load the 'netbsd' 
binary from the 8.1 ISO, I get a crash:

-> Run a program

	scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(4)fdisk(0)netbsd

NetBSD/arc Bootstrap, Revision 1.1
(builds@b7.netbsd.org, Sat Nov  6 14:06:36 UTC 2010)
devopen: scsi(0)cdrom(4)fdisk(0) type disk file netbsd
7902064|
Jazz Monitor. Version 174
Press H for help, Q to quit.
AdEL exception occurred.
 at=149f4800 v0=00000000 v1=8003d698 a0=807f3d40 a1=80eff158 a2=00000800
 a3=80eff188 t0=00000001 t1=00000001 t2=80045400 t3=800232d8 t4=000a4fa4
 t5=00000000 t6=00746669 t7=00746685 s0=80f0cab8 s1=00000800 s2=80eff1d0
 s3=80f10000 s4=80f0ee18 s5=80f0ee18 s6=00028eef s7=00000000 t8=00746685
 t9=c0000000 k0=80041f50 k1=80000194 gp=80f0c540 sp=80eff128 s8=80f0ca30
 ra=80023310 psr=20000803 epc=8002331c cause=00004010 errorepc=00000000
 badvaddr=00746689
>

I haven't tried the latest iso (9.0-rc1).

Re: NetBSD/arc on MIPS Magnum, was Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Hervé Poussineau]

Le 24/12/2019 à 05:33, Finn Thain a écrit :
> On Tue, 24 Dec 2019, Finn Thain wrote:
> 
>>
>> I know precious little about NetBSD installation and MIPS Magnum. What I
>> wrote above was guesswork. Hence this could be a NetBSD bug or user
>> error.
>>
> 
> It was bugs and user error.
> 
> The user error was not using the serial console. The NetBSD/arc
> installation guide says that only serial console is supported for MIPS
> Magnum.
> 
> The bugs include regressions in NetBSD. (See below.)
> 
> The other issue is that the ARC firmware didn't work properly until I
> defined one or more 'boot selections', even though none of these will ever
> be selected.
> 
>> Does there exist a known-good combination of NetBSD/arc and
>> qemu-system-mips64el releases?
>>
> 
> The commit log says that Herv? Poussineau used NetBSD 5.1 with dp8393x in
> the past, so I tried that.
> 
> Here are the steps I used:
> 
> ./mips64el-softmmu/qemu-system-mips64el -M magnum -L .
> -drive if=scsi,unit=2,media=cdrom,format=raw,file=arccd-5.1.iso
> -global ds1225y.filename=nvram -global ds1225y.size=8200
> -serial stdio -serial null
> -nic bridge,model=dp83932,mac=00:00:00:02:03:04
> 
> -> Run setup -> Initialize system -> Set default configurations
> 	800x688
> 	3.5 1.44 M
> 	No
> 	7
> 
> -> Set default environment
> 	CD-ROM
> 	2
> 
> -> Set environment variables
> 	CONSOLEIN
> 	multi()serial(0)term()
> 	CONSOLEOUT
> 	multi()serial(0)term()
> 
> -> Exit
> 
> Now restart QEMU. The ARC menu should appear on the tty.
> 
> -> Run a program
> 
> 	scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)netbsd
> 
> That doesn't work. Add a boot selection.
> 
> -> Run setup -> Manage startup -> Add a boot selection -> Scsi CD-ROM 0
> 	\os\nt\osloader.exe
> 	Yes
> 	\winnt
> 	Windows NT
> 	No
> 
> Somehow, that seems to help. Now restart QEMU.
> 
> -> Run a program
> 
>          scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(2)fdisk(0)netbsd
> 
> NetBSD/arc Bootstrap, Revision 1.1
> (builds@b7.netbsd.org, Sat Nov  6 14:06:36 UTC 2010)
> devopen: scsi(0)cdrom(2)fdisk(0) type disk file netbsd
> 5502064+289092=0x5860e0
> Copyright (c) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
>      2006, 2007, 2008, 2009, 2010
>      The NetBSD Foundation, Inc.  All rights reserved.
> Copyright (c) 1982, 1986, 1989, 1991, 1993
>      The Regents of the University of California.  All rights reserved.
> 
> NetBSD 5.1 (RAMDISK) #0: Sat Nov  6 14:17:36 UTC 2010
>          builds@b7.netbsd.org:/home/builds/ab/netbsd-5-1-RELEASE/arc/201011061943Z-obj/home/builds/ab/netbsd-5-1-RELEASE/src/sys/arch/arc/compile/RAMDISK
> MIPS Magnum
> total memory = 128 MB
> avail memory = 117 MB
> mainbus0 (root)
> cpu0 at mainbus0: MIPS R4000 CPU (0x400) Rev. 0.0 with MIPS R4010 FPC Rev. 0.0
> cpu0: 8KB/16B direct-mapped L1 Instruction cache, 48 TLB entries
> cpu0: 8KB/16B direct-mapped write-back L1 Data cache
> jazzio0 at mainbus0
> timer0 at jazzio0 addr 0xe0000228
> mcclock0 at jazzio0 addr 0xe0004000: mc146818 compatible time-of-day clock
> LPT1 at jazzio0 addr 0xe0008000 intr 0 not configured
> fdc0 at jazzio0 addr 0xe0003000 intr 1
> fd0 at fdc0 drive 1: 1.44MB, 80 cyl, 2 head, 18 sec
> MAGNUM at jazzio0 addr 0xe000c000 intr 2 not configured
> VXL at jazzio0 addr 0xe0800000 intr 3 not configured
> sn0 at jazzio0 addr 0xe0001000 intr 4: SONIC Ethernet
> sonic: write 0x0015 to reg CR
> sonic: write 0x0080 to reg CR
> sonic: write 0x0000 to reg IMR
> sonic: write 0x7fff to reg ISR
> sonic: write 0x0000 to reg CR
> sn0: Ethernet address 00:00:00:00:00:00
> asc0 at jazzio0 addr 0xe0002000 intr 5: NCR53C94, 25MHz, SCSI ID 7
> scsibus0 at asc0: 8 targets, 8 luns per target
> pckbc0 at jazzio0 addr 0xe0005000 intr 6
> pckbd0 at pckbc0 (kbd slot)
> wskbd0 at pckbd0 (mux ignored)
> pms at jazzio0 addr 0xe0005000 intr 7 not configured
> com0 at jazzio0 addr 0xe0006000 intr 8: ns16550a, working fifo
> com0: txfifo disabled
> com0: console
> com1 at jazzio0 addr 0xe0007000 intr 9: ns16550a, working fifo
> com1: txfifo disabled
> jazzisabr0 at mainbus0
> isa0 at jazzisabr0
> isapnp0 at isa0 port 0x279: ISA Plug 'n Play device support
> scsibus0: waiting 2 seconds for devices to settle...
> cd0 at scsibus0 target 2 lun 0: <QEMU, QEMU CD-ROM, 2.5+> cdrom removable
> cd1 at scsibus0 target 4 lun 0: <QEMU, QEMU CD-ROM, 2.5+> cdrom removable
> boot device: <unknown>
> root on md0a dumps on md0b
> root file system type: ffs
> WARNING: preposterous TOD clock time
> WARNING: using filesystem time
> WARNING: CHECK AND RESET THE DATE!
> erase ^H, werase ^W, kill ^U, intr ^C, status ^T
> Terminal type? [vt100]
> Erase is backspace.
> (I)nstall, (S)hell or (H)alt ? s
> # ifconfig sn0 10.2.3.4/24
> # ping
> usage:
> ping [-adDfLnoPqQrRv] [-c count] [-g gateway] [-h host] [-i interval] [-I addr]
>       [-l preload] [-p pattern] [-s size] [-t tos] [-T ttl] [-w maxwait] host
> 
> 
> My initial testing shows that NetBSD 5.1 doesn't like my v2 patch series.
> I'll debug that before I send v3.
> 
> BTW, there seem to be regressions in NetBSD 8.1 compared to NetBSD 5.1.
> 
> The 'boot' program on the 8.1 ISO just hangs.
> 
> If I use the 'boot' program from the 5.1 ISO to load the 'netbsd'
> binary from the 8.1 ISO, I get a crash:
> 
> -> Run a program
> 
> 	scsi(0)cdrom(2)fdisk(0)boot scsi(0)cdrom(4)fdisk(0)netbsd
> 
> NetBSD/arc Bootstrap, Revision 1.1
> (builds@b7.netbsd.org, Sat Nov  6 14:06:36 UTC 2010)
> devopen: scsi(0)cdrom(4)fdisk(0) type disk file netbsd
> 7902064|
> Jazz Monitor. Version 174
> Press H for help, Q to quit.
> AdEL exception occurred.
>   at=149f4800 v0=00000000 v1=8003d698 a0=807f3d40 a1=80eff158 a2=00000800
>   a3=80eff188 t0=00000001 t1=00000001 t2=80045400 t3=800232d8 t4=000a4fa4
>   t5=00000000 t6=00746669 t7=00746685 s0=80f0cab8 s1=00000800 s2=80eff1d0
>   s3=80f10000 s4=80f0ee18 s5=80f0ee18 s6=00028eef s7=00000000 t8=00746685
>   t9=c0000000 k0=80041f50 k1=80000194 gp=80f0c540 sp=80eff128 s8=80f0ca30
>   ra=80023310 psr=20000803 epc=8002331c cause=00004010 errorepc=00000000
>   badvaddr=00746689
>>
> 
> I haven't tried the latest iso (9.0-rc1).
> 

Hello Finn,

Thanks for finding the required steps to boot NetBSD on MIPS Magnum. I was trying to find how to let it work on non-configured machine since some days!
You indeed need my patch at https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg05037.html

My test for networking is:
- Terminal type = "vt100"
- (S)hell
- ifconfig sn0 10.0.2.15 netmask 255.255.255.0
- route add default 10.0.2.2
- connect to ftp.intel.com: ftp 192.198.164.82

Regards,

Hervé

Re: NetBSD/arc on MIPS Magnum, was Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Tue, 24 Dec 2019, hpoussin@reactos.org wrote:

> > 
> > I haven't tried the latest iso (9.0-rc1).
> > 

I found that NetBSD 9.0-rc1 has the same regressions.

> 
> Hello Finn,
> 
> Thanks for finding the required steps to boot NetBSD on MIPS Magnum. I 
> was trying to find how to let it work on non-configured machine since 
> some days!
> You indeed need my patch at
> https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg05037.html
> 

Thanks. As it turned out, I didn't need that patch in order to get 
NetBSD/arc 5.1 working. But I applied the patch anyway after I found that 
a Linux/mips kernel produced DMA errors. But the errors remain (see 
below).

With a few minor Linux patches and a mipsel busybox build, I was able to 
boot to a prompt. ESP SCSI works, but not SONIC ethernet. My dp8393x patch 
series was not sufficient to make ethernet work (no regression though).

NetBSD/arc Bootstrap, Revision 1.1
(builds@b7.netbsd.org, Sat Nov  6 14:06:36 UTC 2010)
devopen: scsi(0)cdrom(4)fdisk(0) type disk file vmlinux
5991052+141348 [656192+872841]=0x74eb98
Linux version 5.4.0-00003-g34add35b08c0 (fthain@nippy) (gcc version 4.6.4 (btc)) #24 Mon Jan 6 20:10:57 AEDT 2020
ARCH: Microsoft-Jazz
PROMLIB: ARC firmware Version 1 Revision 2
CPU0 revision is: 00000400 (R4000PC)
FPU revision is: 00000500
printk: debug: ignoring loglevel setting.
Primary instruction cache 8kB, VIPT, direct mapped, linesize 16 bytes.
Primary data cache 8kB, direct mapped, VIPT, cache aliases, linesize 16 bytes
Zone ranges:
  DMA      [mem 0x0000000000000000-0x0000000000ffffff]
  Normal   [mem 0x0000000001000000-0x0000000007ffffff]
Movable zone start for each node
Early memory node ranges
  node   0: [mem 0x0000000000000000-0x0000000007ffffff]
Initmem setup node 0 [mem 0x0000000000000000-0x0000000007ffffff]
On node 0 totalpages: 32768
  DMA zone: 32 pages used for memmap
  DMA zone: 0 pages reserved
  DMA zone: 4096 pages, LIFO batch:0
  Normal zone: 224 pages used for memmap
  Normal zone: 28672 pages, LIFO batch:7
pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
pcpu-alloc: [0] 0
Built 1 zonelists, mobility grouping on.  Total pages: 32512
Kernel command line: scsi(0)cdrom(4)fdisk(0)vmlinux root=/dev/sda rw ignore_loglevel ip=192.168.66.11 init=/bin/sh
Dentry cache hash table entries: 16384 (order: 4, 65536 bytes, linear)
Inode-cache hash table entries: 8192 (order: 3, 32768 bytes, linear)
mem auto-init: stack:off, heap alloc:off, heap free:off
Memory: 123396K/131072K available (4716K kernel code, 168K rwdata, 812K rodata, 184K init, 100K bss, 7676K reserved, 0K cma-reserved)
NR_IRQS: 128
random: get_random_bytes called from start_kernel+0x32c/0x4e0 with crng_init=0
Console: colour dummy device 80x25
sched_clock: 32 bits at 100 Hz, resolution 10000000ns, wraps every 21474836475000000ns
Calibrating delay loop... 990.41 BogoMIPS (lpj=4952064)
pid_max: default: 32768 minimum: 301
Mount-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
devtmpfs: initialized
clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
futex hash table entries: 256 (order: -1, 3072 bytes, linear)
NET: Registered protocol family 16
VDMA: R4030 DMA pagetables initialized.
SCSI subsystem initialized
NET: Registered protocol family 2
tcp_listen_portaddr_hash hash table entries: 512 (order: 0, 4096 bytes, linear)
TCP established hash table entries: 1024 (order: 0, 4096 bytes, linear)
TCP bind hash table entries: 1024 (order: 0, 4096 bytes, linear)
TCP: Hash tables configured (established 1024 bind 1024)
UDP hash table entries: 256 (order: 0, 4096 bytes, linear)
UDP-Lite hash table entries: 256 (order: 0, 4096 bytes, linear)
NET: Registered protocol family 1
workingset: timestamp_bits=30 max_order=15 bucket_order=0
Block layer SCSI generic (bsg) driver version 0.4 loaded (major 254)
io scheduler mq-deadline registered
io scheduler kyber registered
Console: switching to colour frame buffer device 100x37
Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
printk: console [ttyS0] disabled
serial8250.0: ttyS0 at MMIO 0xe0006000 (irq = 32, base_baud = 115200) is a 16550A
printk: console [ttyS0] enabled
serial8250.0: ttyS1 at MMIO 0xe0007000 (irq = 33, base_baud = 115200) is a 16550A
loop: module loaded
jazz_esp jazz_esp.0: esp0: regs[(ptrval):(ptrval)] irq[29]
jazz_esp jazz_esp.0: esp0: is a FAS100A, 40 MHz (ccf=0), SCSI ID 7
random: fast init done
scsi host0: esp
scsi 0:0:0:0: Direct-Access     QEMU     QEMU HARDDISK    2.5+ PQ: 0 ANSI: 5
scsi target0:0:0: Beginning Domain Validation
scsi target0:0:0: Domain Validation skipping write tests
scsi target0:0:0: Ending Domain Validation
scsi 0:0:2:0: CD-ROM            QEMU     QEMU CD-ROM      2.5+ PQ: 0 ANSI: 5
scsi target0:0:2: Beginning Domain Validation
scsi target0:0:2: Domain Validation skipping write tests
scsi target0:0:2: Ending Domain Validation
scsi 0:0:4:0: CD-ROM            QEMU     QEMU CD-ROM      2.5+ PQ: 0 ANSI: 5
scsi target0:0:4: Beginning Domain Validation
scsi target0:0:4: Domain Validation skipping write tests
scsi target0:0:4: Ending Domain Validation
sd 0:0:0:0: [sda] 120000 512-byte logical blocks: (61.4 MB/58.6 MiB)
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] Mode Sense: 63 00 00 08
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
SONIC ethernet @e0001000, MAC 01:00:1c:00:01:00, IRQ 28
serio: i8042 KBD port at 0xe0005000,0xe0005001 irq 30
serio: i8042 AUX port at 0xe0005000,0xe0005001 irq 31
NET: Registered protocol family 10
Segment Routing with IPv6
sd 0:0:0:0: [sda] Attached SCSI disk
sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver
input: AT Raw Set 2 keyboard as /devices/platform/i8042/serio0/input/input0
input: ImExPS/2 Generic Explorer Mouse as /devices/platform/i8042/serio1/input/input2
EXT4-fs (sda): warning: mounting unchecked fs, running e2fsck is recommended
EXT4-fs (sda): mounted filesystem without journal. Opts: (null)
VFS: Mounted root (ext4 filesystem) on device 8:0.
Freeing prom memory: 376k freed
Freeing prom memory: 60k freed
Freeing prom memory: 4k freed
Freeing unused kernel memory: 184K
This architecture does not have kernel memory protection.
Run /bin/sh as init process
VDMA: Channel 0: Address error!
VDMA: Channel 0: Memory error!

The patches may be found at,

https://github.com/fthain/linux/commits/magnum 
https://github.com/fthain/linux/commits/mac68k
https://github.com/fthain/qemu/commits/sonic

I'll post them when I've finished testing.

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Sun, 15 Dec 2019, Finn Thain wrote:

> I test the qemu build like this,
> 
> qemu-system-m68k -M q800 -m 512M -serial none -serial mon:stdio -g 800x600x4
> -net nic,model=dp83932,addr=00:00:00:01:02:03
> -net bridge,helper=/opt/qemu/libexec/qemu-bridge-helper,br=br0
> -append "fbcon=font:ProFont6x11 console=tty0 console=ttyS0 ignore_loglevel"
> -kernel vmlinux-4.14.157-mac-backport+
> -initrd /mnt/loop/install/cdrom/initrd.gz
> 
> You can obtain this kernel binary from the linux-mac68k project on 
> sourceforge. (I usually use a mainline Linux build but it makes no 
> difference.)
> 

One difficulty with testing these patches with Linux guests is some old 
bugs in drivers/net/ethernet/natsemi/sonic.c that can cause tx watchdog 
timeouts on real hardware.

I have some patches for that driver which may be useful when testing 
QEMU's hw/net/dp8393x.c device. (I've pushed those patches to my github 
repo.)

The second obstacle I have involves testing the dp8393x device with a 
bridge device on a Linux/i686 host.

Running tcpdump in the Linux/m68k guest showed these two ping packets from 
the host,

00:15:28.480164 IP 192.168.66.1 > 192.168.66.111: ICMP echo request, id 23957, seq 11, length 64
        0x0000:  0800 0702 0304 fe16 d9ae 6943 0800 4500  ..........iC..E.
        0x0010:  0054 ff4d 4000 4001 359a c0a8 4201 c0a8  .T.M@.@.5...B...
        0x0020:  426f 0800 4243 5d95 000b a0cc f65d cfee  Bo..BC]......]..
        0x0030:  0600 0809 0a0b 0c0d 0e0f 1011 1213 1415  ................
        0x0040:  1617 1819 1a1b 1c1d 1e1f 2021 2223 2425  ...........!"#$%
        0x0050:  2627 2829 2a2b 2c2d 2e2f 3031 3233 3435  &'()*+,-./012345
        0x0060:  3637 33e0 14c7                           673...
00:15:29.341601 IP truncated-ip - 52 bytes missing! 192.168.66.1 > 192.168.66.111: ICMP echo request, id 23957, seq 12, length 64
        0x0000:  0800 0702 0304 fe16 d9ae 6943 0800 4500  ..........iC..E.
        0x0010:  0054 ff4e 4000 4001 3599 c0a8 4201 c0a8  .T.N@.@.5...B...
        0x0020:  426f 0800 d61a 5d95 000c a0cc f65d        Bo....]......]

Sniffing br0 on the host shows no sign of the truncated packet at all 
which leaves a gap in the packet sequence numbers captured on the host. 
Weird.

When I log the calls to,

static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
                               size_t pkt_size)

the corresponding pkt_size values look like this,

pkt_size 98
pkt_size 42

So this seems to show that the bug is not in dp8393x. Possibly not in 
QEMU?

I don't see any options in 'man brctl' that might explain why the host and 
guest see different packets. I guess I'll have to find a way to avoid 
using bridge interfaces (?)

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Mon, 16 Dec 2019, Finn Thain wrote:

> 00:15:29.341601 IP truncated-ip - 52 bytes missing! 192.168.66.1 > 192.168.66.111: ICMP echo request, id 23957, seq 12, length 64

...

> 
> Sniffing br0 on the host shows no sign of the truncated packet at all 
> which leaves a gap in the packet sequence numbers captured on the host. 

I've fixed some bugs and I don't see these problems any more. I'll send v2.

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Aleksandar Markovic]

On Sat, Dec 14, 2019 at 2:29 AM Finn Thain <fthain@telegraphics.com.au> wrote:
>
> Hi All,
>
> There is a bug in the DP8393X emulation that can stop packet reception.
>
> Whilst debugging that issue I found that the receiver algorithm differs
> from the one described in the National Semiconductor datasheet.
>

Finn, could you please provide the link to the exact datasheet that
you used for reference, so that we are on the same page while looking
at your code?

Best regards,
Aleksandar

> These issues and others are addressed by this patch series.
>
> This series has only been tested with Linux/m68k guests. It needs further
> testing with MIPS Jazz guests such as NetBSD or Windows NT.
>
> Thanks.
>
>
> Finn Thain (10):
>   dp8393x: Mask EOL bit from descriptor addresses
>   dp8393x: Clean up endianness hacks
>   dp8393x: Have dp8393x_receive() return the packet size
>   dp8393x: Don't advance RX descriptor twice
>   dp8393x: Update LLFA register
>   dp8393x: Clear RRRA command register bit only when appropriate
>   dp8393x: Implement TBWC0 and TBWC1 registers to restore buffer state
>   dp8393x: Implement packet size limit and RBAE interrupt
>   dp8393x: Don't stop reception upon RBE interrupt assertion
>   dp8393x: Don't clobber packet checksum
>
>  hw/net/dp8393x.c | 80 +++++++++++++++++++++++++++++++-----------------
>  1 file changed, 52 insertions(+), 28 deletions(-)
>
> --
> 2.23.0
>
>

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Aleksandar Markovic]

On Fri, Dec 20, 2019 at 12:38 PM Aleksandar Markovic
<aleksandar.m.mail@gmail.com> wrote:
>
> On Sat, Dec 14, 2019 at 2:29 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> >
> > Hi All,
> >
> > There is a bug in the DP8393X emulation that can stop packet reception.
> >
> > Whilst debugging that issue I found that the receiver algorithm differs
> > from the one described in the National Semiconductor datasheet.
> >
>
> Finn, could you please provide the link to the exact datasheet that

or, several datasheets...

> you used for reference, so that we are on the same page while looking
> at your code?
>
> Best regards,
> Aleksandar
>
> > These issues and others are addressed by this patch series.
> >
> > This series has only been tested with Linux/m68k guests. It needs further
> > testing with MIPS Jazz guests such as NetBSD or Windows NT.
> >
> > Thanks.
> >
> >
> > Finn Thain (10):
> >   dp8393x: Mask EOL bit from descriptor addresses
> >   dp8393x: Clean up endianness hacks
> >   dp8393x: Have dp8393x_receive() return the packet size
> >   dp8393x: Don't advance RX descriptor twice
> >   dp8393x: Update LLFA register
> >   dp8393x: Clear RRRA command register bit only when appropriate
> >   dp8393x: Implement TBWC0 and TBWC1 registers to restore buffer state
> >   dp8393x: Implement packet size limit and RBAE interrupt
> >   dp8393x: Don't stop reception upon RBE interrupt assertion
> >   dp8393x: Don't clobber packet checksum
> >
> >  hw/net/dp8393x.c | 80 +++++++++++++++++++++++++++++++-----------------
> >  1 file changed, 52 insertions(+), 28 deletions(-)
> >
> > --
> > 2.23.0
> >
> >

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Laurent Vivier]

Le 20/12/2019 à 12:38, Aleksandar Markovic a écrit :
> On Sat, Dec 14, 2019 at 2:29 AM Finn Thain <fthain@telegraphics.com.au> wrote:
>>
>> Hi All,
>>
>> There is a bug in the DP8393X emulation that can stop packet reception.
>>
>> Whilst debugging that issue I found that the receiver algorithm differs
>> from the one described in the National Semiconductor datasheet.
>>
> 
> Finn, could you please provide the link to the exact datasheet that
> you used for reference, so that we are on the same page while looking
> at your code?

According to his comments ,"National Semiconductor DP83932C" and
sections seem to be the same, I think the datasheet is:

https://www.eit.lth.se/fileadmin/eit/courses/datablad/Periphery/Communication/DP83932C.pdf

Thanks,
Laurent

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Fri, 20 Dec 2019, Laurent Vivier wrote:

> Le 20/12/2019 ? 12:38, Aleksandar Markovic a ?crit?:
> > On Sat, Dec 14, 2019 at 2:29 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> >>
> >> Hi All,
> >>
> >> There is a bug in the DP8393X emulation that can stop packet reception.
> >>
> >> Whilst debugging that issue I found that the receiver algorithm differs
> >> from the one described in the National Semiconductor datasheet.
> >>
> > 
> > Finn, could you please provide the link to the exact datasheet that
> > you used for reference, so that we are on the same page while looking
> > at your code?
> 
> According to his comments ,"National Semiconductor DP83932C" and
> sections seem to be the same, I think the datasheet is:
> 
> https://www.eit.lth.se/fileadmin/eit/courses/datablad/Periphery/Communication/DP83932C.pdf
> 

Yes. I know of 3 datasheets from National Semiconductor,

11719  DP83934CVUL-20/25 MHz SONIC-T Systems-Oriented Network Interface 
Controller with Twisted Pair Interface
10492  DP83932C-20/25/33 MHz SONIC Systems-Oriented Network Interface 
Controller
11722  DP83916 SONIC-16 Systems-Oriented Network Interface Controller

The publication numbered 10492 is the one that Laurent linked to. It and 
11722 both have the same table of contents. The references I gave in the 
patch descriptions are applicable to these. (Having said that, I see now 
that I did mess up one reference. I'll fix it.)

The "1995 National Ethernet Databook" on bitsavers has more information. 
https://mirrorservice.org/sites/www.bitsavers.org/components/national/_dataBooks/1995_National_Ethernet_Databook.pdf

-- 

> Thanks,
> Laurent
> 

Re: [PATCH 00/10] Fixes for DP8393X SONIC device emulation

[Aleksandar Markovic]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

On Saturday, December 21, 2019, Finn Thain <fthain@telegraphics.com.au>
wrote:

> On Fri, 20 Dec 2019, Laurent Vivier wrote:
>
> > Le 20/12/2019 ? 12:38, Aleksandar Markovic a ?crit?:
> > > On Sat, Dec 14, 2019 at 2:29 AM Finn Thain <fthain@telegraphics.com.au>
> wrote:
> > >>
> > >> Hi All,
> > >>
> > >> There is a bug in the DP8393X emulation that can stop packet
> reception.
> > >>
> > >> Whilst debugging that issue I found that the receiver algorithm
> differs
> > >> from the one described in the National Semiconductor datasheet.
> > >>
> > >
> > > Finn, could you please provide the link to the exact datasheet that
> > > you used for reference, so that we are on the same page while looking
> > > at your code?
> >
> > According to his comments ,"National Semiconductor DP83932C" and
> > sections seem to be the same, I think the datasheet is:
> >
> > https://www.eit.lth.se/fileadmin/eit/courses/datablad/Periphery/
> Communication/DP83932C.pdf
> >
>
> Yes. I know of 3 datasheets from National Semiconductor,
>
> 11719  DP83934CVUL-20/25 MHz SONIC-T Systems-Oriented Network Interface
> Controller with Twisted Pair Interface
> 10492  DP83932C-20/25/33 MHz SONIC Systems-Oriented Network Interface
> Controller
> 11722  DP83916 SONIC-16 Systems-Oriented Network Interface Controller
>
> The publication numbered 10492 is the one that Laurent linked to. It and
> 11722 both have the same table of contents. The references I gave in the
> patch descriptions are applicable to these. (Having said that, I see now
> that I did mess up one reference. I'll fix it.)
>
> The "1995 National Ethernet Databook" on bitsavers has more information.
> https://mirrorservice.org/sites/www.bitsavers.org/components/national/_
> dataBooks/1995_National_Ethernet_Databook.pdf
>
> --


Finn,

I truly appreciate your detailed response.

Aleksandar


> > Thanks,
> > Laurent
> >
>

[-- Attachment #2: Type: text/html, Size: 2908 bytes --]