[Qemu-devel] [PATCH] lance.c emulation corrections

[Qemu-devel] [PATCH] lance.c emulation corrections

[Igor Kovalenko]

[-- Attachment #1.1: Type: text/plain, Size: 690 bytes --]

Hi!

lance.c emulation implements incorrect algorithm to scan receive/transmit
descriptors:
 - it scans a whole ring of descriptors instead of stopping at first owned
by host
 - it skips buffers in corner cases
 - card is not reset, current rx/tx descriptor number is not reset when card
is stopped by driver

Attached patch should fix these problems.
On-demand packet transmission is also implemented.

Without this patch I had ~500 byte/second transfers with qemu-system-sparc,
now it network speed is much better :)

Looking at 7990 chip docs (main chip on lance card) it seems like there are
more features
not implemented; e.g. 1.6ms timer is needed to implement "normal"
transmission.

[-- Attachment #1.2: Type: text/html, Size: 768 bytes --]

[-- Attachment #2: lance-20060810-1.diff --]
[-- Type: text/x-patch, Size: 8776 bytes --]

Index: hw/lance.c
===================================================================
RCS file: /cvsroot/qemu/qemu/hw/lance.c,v
retrieving revision 1.9
diff -u -r1.9 lance.c
--- hw/lance.c	9 Aug 2006 22:38:19 -0000	1.9
+++ hw/lance.c	9 Aug 2006 23:14:37 -0000
@@ -21,6 +21,20 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+
+/*
+ * Modifications:
+ *  2006-Aug-10  Igor Kovalenko :   Corrected STOP procedure to reset chip
+ *                                  Corrected transmission routine to stop ring polling
+ *                                  if OWN bit is not set in current descriptor
+ *                                  Implemented TDMD bit of CSR0 for transmit on demand
+ *                                  More debug statements in routines
+ *
+ * TODO list:
+ *      Implement block chaining for send and receive rings.
+ *      lance_can_receive() : implement checking if card owns current rx ring descriptor
+ *
+ */
 #include "vl.h"
 
 /* debug LANCE card */
@@ -176,6 +190,8 @@
     memset(s->regs, 0, LE_NREGS * 2);
     s->regs[LE_CSR0] = LE_C0_STOP;
     memset(s->ledmaregs, 0, LEDMA_REGS * 4);
+
+    DPRINTF("done reset\n");
 }
 
 static uint32_t lance_mem_readw(void *opaque, target_phys_addr_t addr)
@@ -204,6 +220,7 @@
     LANCEState *s = opaque;
     uint32_t saddr;
     uint16_t reg;
+    int lance_transmit_on_demand = 0;
 
     saddr = addr & LE_MAXREG;
     switch (saddr >> 1) {
@@ -212,13 +229,23 @@
         switch (s->addr) {
         case LE_CSR0:
             if (val & LE_C0_STOP) {
-                s->regs[LE_CSR0] = LE_C0_STOP;
+                DPRINTF("CSR0 write STOP: chip reset\n");
+                lance_reset(s);
                 break;
             }
 
+            // Transmit demand bit
+            if (val & LE_C0_TDMD)
+            {
+                DPRINTF("CSR0 write %08x : TDMD transmit demand\n", val);
+                lance_transmit_on_demand = 1;
+
+                // this bit is automatically cleared, see below
+            }
+
             reg = s->regs[LE_CSR0];
 
-            // 1 = clear for some bits
+            // 1 = clear for some bits, including TDMD
             reg &= ~(val & 0x7f00);
 
             // generated bits
@@ -265,7 +292,12 @@
         DPRINTF("write unknown(%d) = %4.4x\n", saddr >> 1, val);
         break;
     }
-    lance_send(s);
+
+    if (lance_transmit_on_demand)
+    {
+        DPRINTF("=== Transmit on demand\n");
+        lance_send(s);
+    }
 }
 
 static CPUReadMemoryFunc *lance_mem_read[3] = {
@@ -285,6 +317,16 @@
 
 static int lance_can_receive(void *opaque)
 {
+    LANCEState *s = opaque;
+
+    if ((s->regs[LE_CSR0] & LE_C0_STOP) == LE_C0_STOP)
+    {
+        DPRINTF("lance_can_receive() : STOP\n");
+        return 1;
+    }
+
+    // TODO: Need to check if card owns current rx ring descriptor
+
     return 1;
 }
 
@@ -299,33 +341,47 @@
 
     DPRINTF("receive size %d\n", size);
     if ((s->regs[LE_CSR0] & LE_C0_STOP) == LE_C0_STOP)
+    {
+        DPRINTF("lance_receive() : STOP\n");
         return;
+    }
 
     ib = (void *) iommu_translate(dmaptr);
 
+    DPRINTF("--- lance_receive --- scanning...\n");
+
     old_rxptr = s->rxptr;
     for (i = s->rxptr; i != ((old_rxptr - 1) & RX_RING_MOD_MASK);
          i = (i + 1) & RX_RING_MOD_MASK) {
         cpu_physical_memory_read((uint32_t) & ib->brx_ring[i].rmd1_bits,
                                  (void *) &temp8, 1);
-        if (temp8 == (LE_R1_OWN)) {
-            s->rxptr = (s->rxptr + 1) & RX_RING_MOD_MASK;
-            temp16 = size + 4;
-            bswap16s(&temp16);
-            cpu_physical_memory_write((uint32_t) & ib->brx_ring[i].
-                                      mblength, (void *) &temp16, 2);
-            cpu_physical_memory_write((uint32_t) & ib->rx_buf[i], buf,
-                                      size);
-            temp8 = LE_R1_POK;
-            cpu_physical_memory_write((uint32_t) & ib->brx_ring[i].
-                                      rmd1_bits, (void *) &temp8, 1);
-            s->regs[LE_CSR0] |= LE_C0_RINT | LE_C0_INTR;
-            if (s->regs[LE_CSR0] & LE_C0_INEA)
-                pic_set_irq(s->irq, 1);
-            DPRINTF("got packet, len %d\n", size);
-            return;
+
+        if (!(temp8 & LE_R1_OWN))
+        {
+            DPRINTF("### lance_receive() : OWN not set RMD1 bits=%02x in rx descriptor %u\n", temp8, i);
+            break;
         }
+
+        // TODO: implement receive buffer chaining
+
+        s->rxptr = (s->rxptr + 1) & RX_RING_MOD_MASK;
+        temp16 = size + 4;
+        bswap16s(&temp16);
+        cpu_physical_memory_write((uint32_t) & ib->brx_ring[i].
+                                  mblength, (void *) &temp16, 2);
+        cpu_physical_memory_write((uint32_t) & ib->rx_buf[i], buf,
+                                  size);
+        temp8 = LE_R1_POK;
+        cpu_physical_memory_write((uint32_t) & ib->brx_ring[i].
+                                  rmd1_bits, (void *) &temp8, 1);
+        s->regs[LE_CSR0] |= LE_C0_RINT | LE_C0_INTR;
+        DPRINTF("got packet, len %d\n", size);
+        if (s->regs[LE_CSR0] & LE_C0_INEA)
+            pic_set_irq(s->irq, 1);
+        break;
     }
+
+    DPRINTF("lance_receive() : leave\n");
 }
 
 static void lance_send(void *opaque)
@@ -340,35 +396,82 @@
 
     DPRINTF("sending packet? (csr0 %4.4x)\n", s->regs[LE_CSR0]);
     if ((s->regs[LE_CSR0] & LE_C0_STOP) == LE_C0_STOP)
+    {
+        DPRINTF("lance_send() : STOP\n");
         return;
+    }
 
     ib = (void *) iommu_translate(dmaptr);
 
     DPRINTF("sending packet? (dmaptr %8.8x) (ib %p) (btx_ring %p)\n",
             dmaptr, ib, &ib->btx_ring);
     old_txptr = s->txptr;
-    for (i = s->txptr; i != ((old_txptr - 1) & TX_RING_MOD_MASK);
-         i = (i + 1) & TX_RING_MOD_MASK) {
+
+    // transmit descriptor polling loop
+    // terminates ONLY if lance does not own current transmit descriptor
+    while (1)
+    {
+        i = s->txptr;
+
         cpu_physical_memory_read((uint32_t) & ib->btx_ring[i].tmd1_bits,
                                  (void *) &temp8, 1);
-        if (temp8 == (LE_T1_POK | LE_T1_OWN)) {
+
+        if (!(temp8 & LE_T1_OWN))
+        {
+            DPRINTF("### OWN bit not set in tx descriptor %u : stop polling\n", i);
+            break;
+        }
+        else if (temp8 == (LE_T1_POK|LE_T1_OWN))
+        {
+            // TODO: NODE this is a probe for complete packet in single tx buffer
+
+            DPRINTF("+++ OWN bit set in tx descriptor %u\n", i);
             cpu_physical_memory_read((uint32_t) & ib->btx_ring[i].length,
                                      (void *) &temp16, 2);
             bswap16s(&temp16);
             temp16 = (~temp16) + 1;
             cpu_physical_memory_read((uint32_t) & ib->tx_buf[i], pkt_buf,
                                      temp16);
-            DPRINTF("sending packet, len %d\n", temp16);
-            qemu_send_packet(s->vc, pkt_buf, temp16);
             temp8 = LE_T1_POK;
             cpu_physical_memory_write((uint32_t) & ib->btx_ring[i].
                                       tmd1_bits, (void *) &temp8, 1);
+
+            // advance to next transmit descriptor
             s->txptr = (s->txptr + 1) & TX_RING_MOD_MASK;
             s->regs[LE_CSR0] |= LE_C0_TINT | LE_C0_INTR;
+
+            // notify about complete packet transmission
+            if ((s->regs[LE_CSR0] & LE_C0_INTR) && (s->regs[LE_CSR0] & LE_C0_INEA))
+            {
+                DPRINTF("--- lance_send +++ raising interrupt for TINT\n");
+                pic_set_irq(s->irq, 1);
+            }
+
+            // actually send packet a bit later, to prevent possible recursion (tx->rx->tx)
+            DPRINTF("--- lance_send +++ sending packet, len %d\n", temp16);
+            qemu_send_packet(s->vc, pkt_buf, temp16);
+        }
+        else
+        {
+            DPRINTF("### not implemented buffer chaining, flags %02x in tx descriptor %u\n", temp8, i);
+
+            // TODO: must handle buffer chaining
+
+            temp8 = 0;
+
+            cpu_physical_memory_write((uint32_t)&ib->btx_ring[i].tmd1_bits, (void *) &temp8, 1);
+            s->regs[LE_CSR0] |= LE_C0_TINT | LE_C0_INTR;
+
+            // notify about complete packet transmission
+            if ((s->regs[LE_CSR0] & LE_C0_INTR) && (s->regs[LE_CSR0] & LE_C0_INEA))
+            {
+                DPRINTF("--- lance_send +++ raising interrupt for TINT\n");
+                pic_set_irq(s->irq, 1);
+            }
         }
     }
-    if ((s->regs[LE_CSR0] & LE_C0_INTR) && (s->regs[LE_CSR0] & LE_C0_INEA))
-        pic_set_irq(s->irq, 1);
+
+    DPRINTF("lance_send() leave\n");
 }
 
 static uint32_t ledma_mem_readl(void *opaque, target_phys_addr_t addr)

RE: [Qemu-devel] [PATCH] lance.c emulation corrections

[Blue Swirl]

>Without this patch I had ~500 byte/second transfers with qemu-system-sparc,
>now it network speed is much better :)

Good work!

>Looking at 7990 chip docs (main chip on lance card) it seems like there are
>more features
>not implemented; e.g. 1.6ms timer is needed to implement "normal"
>transmission.

It could be worthwhile to check if lance (Am7990) and pcnet (Am79C970A) 
drivers could be merged. This was discussed briefly some years ago:
http://lists.gnu.org/archive/html/qemu-devel/2004-10/msg00364.html

_________________________________________________________________
Don't just search. Find. Check out the new MSN Search! 
http://search.msn.com/

Re: [Qemu-devel] [PATCH] lance.c emulation corrections

[Igor Kovalenko]

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

>
> >Looking at 7990 chip docs (main chip on lance card) it seems like there
> are
> >more features
> >not implemented; e.g. 1.6ms timer is needed to implement "normal"
> >transmission.
>
> It could be worthwhile to check if lance (Am7990) and pcnet (Am79C970A)
> drivers could be merged. This was discussed briefly some years ago:
> http://lists.gnu.org/archive/html/qemu-devel/2004-10/msg00364.html
>

Yes, a quick look shows 79c970a has one rx/tx buffer management mode which
is software-compatible with 7990 chip (SWMODE=0) There may be more
backward-compatibility bits to be unified. Anyone? :)

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

Re: [Qemu-devel] [PATCH] lance.c emulation corrections

[Blue Swirl]

>>It could be worthwhile to check if lance (Am7990) and pcnet (Am79C970A)
>>drivers could be merged. This was discussed briefly some years ago:
>>http://lists.gnu.org/archive/html/qemu-devel/2004-10/msg00364.html
>>
>
>Yes, a quick look shows 79c970a has one rx/tx buffer management mode which
>is software-compatible with 7990 chip (SWMODE=0) There may be more
>backward-compatibility bits to be unified. Anyone? :)

Looks like only 32-bit modes are implemented in pcnet.c and endianness 
issues should be checked.

The files could be reorganised to for example pcnet-common.c, pcnet-pci.c, 
and pcnet-lance.c or just throw everything together with small performance 
penalty and unwanted code for both lance and pcnet. Opinions?

_________________________________________________________________
Express yourself instantly with MSN Messenger! Download today it's FREE! 
http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/

Re: [Qemu-devel] [PATCH] lance.c emulation corrections

[Fabrice Bellard]

Blue Swirl wrote:
>>> It could be worthwhile to check if lance (Am7990) and pcnet (Am79C970A)
>>> drivers could be merged. This was discussed briefly some years ago:
>>> http://lists.gnu.org/archive/html/qemu-devel/2004-10/msg00364.html
>>>
>>
>> Yes, a quick look shows 79c970a has one rx/tx buffer management mode 
>> which
>> is software-compatible with 7990 chip (SWMODE=0) There may be more
>> backward-compatibility bits to be unified. Anyone? :)
> 
> 
> Looks like only 32-bit modes are implemented in pcnet.c and endianness 
> issues should be checked.
> 
> The files could be reorganised to for example pcnet-common.c, 
> pcnet-pci.c, and pcnet-lance.c or just throw everything together with 
> small performance penalty and unwanted code for both lance and pcnet. 
> Opinions?

Make a single file. It will always be possible to split it if it becomes 
too big.

Fabrice.