[Qemu-devel] [PATCH 0/4] hw/net: fix m68/ColdFire ethernet fec support

[Qemu-devel] [PATCH 0/4] hw/net: fix m68/ColdFire ethernet fec support

[gerg]

The following set of patches fixes the emulated ColdFire ethernet fec
driver. There is primarily two problems that need to be fixed.

1. The emulated driver needs to support probing of an attached phy.
   It is strait forward to emulate an attached phy, but to avoid using
   magic numbers I have factored out the common MII register and value
   definitions into their own mii.h file first.

2. Fix the fec driver receiver to return the correct value.

With these changes in place the qemu m5208evb board emulation can probe,
identify and use the fec ethernet running a Linux guest.


 hw/net/mcf_fec.c                |   54 ++++++++++++++++++++++++++--
 include/hw/net/allwinner_emac.h |   40 ---------------------
 include/hw/net/mii.h            |   76 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+), 42 deletions(-)

[Qemu-devel] [PATCH 1/4] hw/net: create common collection of MII definitions

[gerg]

From: Greg Ungerer <gerg@uclinux.org>

Create a common set of definitions of address and register values for
ethernet MII phys. A few of the current ethernet drivers have at least
a partial set of these definitions. Others just use hard coded raw
constant numbers.

This initial set is copied directly from the allwinner_emac code.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 include/hw/net/allwinner_emac.h | 40 +-------------------------
 include/hw/net/mii.h            | 64 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 39 deletions(-)
 create mode 100644 include/hw/net/mii.h

diff --git a/include/hw/net/allwinner_emac.h b/include/hw/net/allwinner_emac.h
index 5ae7717..9f21aa7 100644
--- a/include/hw/net/allwinner_emac.h
+++ b/include/hw/net/allwinner_emac.h
@@ -24,6 +24,7 @@
 
 #include "net/net.h"
 #include "qemu/fifo8.h"
+#include "hw/net/mii.h"
 
 #define TYPE_AW_EMAC "allwinner-emac"
 #define AW_EMAC(obj) OBJECT_CHECK(AwEmacState, (obj), TYPE_AW_EMAC)
@@ -118,45 +119,6 @@
 #define EMAC_RX_IO_DATA_STATUS_OK       (1 << 7)
 #define EMAC_UNDOCUMENTED_MAGIC         0x0143414d  /* header for RX frames */
 
-/* PHY registers */
-#define MII_BMCR            0
-#define MII_BMSR            1
-#define MII_PHYID1          2
-#define MII_PHYID2          3
-#define MII_ANAR            4
-#define MII_ANLPAR          5
-#define MII_ANER            6
-#define MII_NSR             16
-#define MII_LBREMR          17
-#define MII_REC             18
-#define MII_SNRDR           19
-#define MII_TEST            25
-
-/* PHY registers fields */
-#define MII_BMCR_RESET      (1 << 15)
-#define MII_BMCR_LOOPBACK   (1 << 14)
-#define MII_BMCR_SPEED      (1 << 13)
-#define MII_BMCR_AUTOEN     (1 << 12)
-#define MII_BMCR_FD         (1 << 8)
-
-#define MII_BMSR_100TX_FD   (1 << 14)
-#define MII_BMSR_100TX_HD   (1 << 13)
-#define MII_BMSR_10T_FD     (1 << 12)
-#define MII_BMSR_10T_HD     (1 << 11)
-#define MII_BMSR_MFPS       (1 << 6)
-#define MII_BMSR_AN_COMP    (1 << 5)
-#define MII_BMSR_AUTONEG    (1 << 3)
-#define MII_BMSR_LINK_ST    (1 << 2)
-
-#define MII_ANAR_TXFD       (1 << 8)
-#define MII_ANAR_TX         (1 << 7)
-#define MII_ANAR_10FD       (1 << 6)
-#define MII_ANAR_10         (1 << 5)
-#define MII_ANAR_CSMACD     (1 << 0)
-
-#define RTL8201CP_PHYID1    0x0000
-#define RTL8201CP_PHYID2    0x8201
-
 /* INT CTL and INT STA registers fields */
 #define EMAC_INT_TX_CHAN(x) (1 << (x))
 #define EMAC_INT_RX         (1 << 8)
diff --git a/include/hw/net/mii.h b/include/hw/net/mii.h
new file mode 100644
index 0000000..4d93114
--- /dev/null
+++ b/include/hw/net/mii.h
@@ -0,0 +1,64 @@
+/*
+ * Common network MII address and register definitions.
+ *
+ * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.com>
+ *
+ * Allwinner EMAC register definitions from Linux kernel are:
+ *   Copyright 2012 Stefan Roese <sr@denx.de>
+ *   Copyright 2013 Maxime Ripard <maxime.ripard@free-electrons.com>
+ *   Copyright 1997 Sten Wang
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef MII_H
+#define MII_H
+
+/* PHY registers */
+#define MII_BMCR            0
+#define MII_BMSR            1
+#define MII_PHYID1          2
+#define MII_PHYID2          3
+#define MII_ANAR            4
+#define MII_ANLPAR          5
+#define MII_ANER            6
+#define MII_NSR             16
+#define MII_LBREMR          17
+#define MII_REC             18
+#define MII_SNRDR           19
+#define MII_TEST            25
+
+/* PHY registers fields */
+#define MII_BMCR_RESET      (1 << 15)
+#define MII_BMCR_LOOPBACK   (1 << 14)
+#define MII_BMCR_SPEED      (1 << 13)
+#define MII_BMCR_AUTOEN     (1 << 12)
+#define MII_BMCR_FD         (1 << 8)
+
+#define MII_BMSR_100TX_FD   (1 << 14)
+#define MII_BMSR_100TX_HD   (1 << 13)
+#define MII_BMSR_10T_FD     (1 << 12)
+#define MII_BMSR_10T_HD     (1 << 11)
+#define MII_BMSR_MFPS       (1 << 6)
+#define MII_BMSR_AN_COMP    (1 << 5)
+#define MII_BMSR_AUTONEG    (1 << 3)
+#define MII_BMSR_LINK_ST    (1 << 2)
+
+#define MII_ANAR_TXFD       (1 << 8)
+#define MII_ANAR_TX         (1 << 7)
+#define MII_ANAR_10FD       (1 << 6)
+#define MII_ANAR_10         (1 << 5)
+#define MII_ANAR_CSMACD     (1 << 0)
+
+/* List of vendor identifiers */
+#define RTL8201CP_PHYID1    0x0000
+#define RTL8201CP_PHYID2    0x8201
+
+#endif /* MII_H */
-- 
1.9.1

[Qemu-devel] [PATCH 2/4] hw/net: add ANLPAR bit definitions to generic mii

[gerg]

From: Greg Ungerer <gerg@uclinux.org>

Add a base set of bit definitions for the standard MII phy "Auto-Negotiation
Link Partner Ability Register" (ANLPAR).

The original definitions moved into mii.h from the allwinner_emac driver
did not define these.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 include/hw/net/mii.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/hw/net/mii.h b/include/hw/net/mii.h
index 4d93114..cd2a4e2 100644
--- a/include/hw/net/mii.h
+++ b/include/hw/net/mii.h
@@ -57,6 +57,13 @@
 #define MII_ANAR_10         (1 << 5)
 #define MII_ANAR_CSMACD     (1 << 0)
 
+#define MII_ANLPAR_ACK      (1 << 14)
+#define MII_ANLPAR_TXFD     (1 << 8)
+#define MII_ANLPAR_TX       (1 << 7)
+#define MII_ANLPAR_10FD     (1 << 6)
+#define MII_ANLPAR_10       (1 << 5)
+#define MII_ANLPAR_CSMACD   (1 << 0)
+
 /* List of vendor identifiers */
 #define RTL8201CP_PHYID1    0x0000
 #define RTL8201CP_PHYID2    0x8201
-- 
1.9.1

[Qemu-devel] [PATCH 3/4] hw/net: add simple phy support to mcf_fec driver

[gerg]

From: Greg Ungerer <gerg@uclinux.org>

The Linux fec driver needs at least basic phy support to probe and work.
The current qemu mcf_fec emulation has no support for the reading or
writing of the MDIO lines to access an attached phy.

This code adds a very simple set of register results for a fixed phy
setup - very similar to that used on an m5208evb board. This is enough
to probe and identify an emulated attached phy.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 hw/net/mcf_fec.c     | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
 include/hw/net/mii.h |  5 +++++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c
index 0255612..ea59017 100644
--- a/hw/net/mcf_fec.c
+++ b/hw/net/mcf_fec.c
@@ -8,6 +8,7 @@
 #include "hw/hw.h"
 #include "net/net.h"
 #include "hw/m68k/mcf.h"
+#include "hw/net/mii.h"
 /* For crc32 */
 #include <zlib.h>
 #include "exec/address-spaces.h"
@@ -216,6 +217,51 @@ static void mcf_fec_reset(mcf_fec_state *s)
     s->rfsr = 0x500;
 }
 
+#define MMFR_WRITE_OP	(1 << 28)
+#define MMFR_READ_OP	(2 << 28)
+#define MMFR_PHYADDR(v)	(((v) >> 23) & 0x1f)
+#define MMFR_REGNUM(v)	(((v) >> 18) & 0x1f)
+
+static uint64_t mcf_fec_read_mdio(mcf_fec_state *s)
+{
+    uint64_t v;
+
+    if (s->mmfr & MMFR_WRITE_OP)
+        return s->mmfr;
+    if (MMFR_PHYADDR(s->mmfr) != 1)
+        return s->mmfr |= 0xffff;
+
+    switch (MMFR_REGNUM(s->mmfr)) {
+    case MII_BMCR:
+        v = MII_BMCR_SPEED | MII_BMCR_AUTOEN | MII_BMCR_FD;
+        break;
+    case MII_BMSR:
+        v = MII_BMSR_100TX_FD | MII_BMSR_100TX_HD | MII_BMSR_10T_FD |
+            MII_BMSR_10T_HD | MII_BMSR_MFPS | MII_BMSR_AN_COMP |
+            MII_BMSR_AUTONEG | MII_BMSR_LINK_ST;
+        break;
+    case MII_PHYID1:
+        v = DP83848_PHYID1;
+        break;
+    case MII_PHYID2:
+        v = DP83848_PHYID2;
+        break;
+    case MII_ANAR:
+        v = MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD |
+            MII_ANAR_10 | MII_ANAR_CSMACD;
+        break;
+    case MII_ANLPAR:
+        v = MII_ANLPAR_ACK | MII_ANLPAR_TXFD | MII_ANLPAR_TX |
+            MII_ANLPAR_10FD | MII_ANLPAR_10 | MII_ANLPAR_CSMACD;
+        break;
+    default:
+        v = 0xffff;
+        break;
+    }
+    s->mmfr = (s->mmfr & ~0xffff) | v;
+    return s->mmfr;
+}
+
 static uint64_t mcf_fec_read(void *opaque, hwaddr addr,
                              unsigned size)
 {
@@ -226,7 +272,7 @@ static uint64_t mcf_fec_read(void *opaque, hwaddr addr,
     case 0x010: return s->rx_enabled ? (1 << 24) : 0; /* RDAR */
     case 0x014: return 0; /* TDAR */
     case 0x024: return s->ecr;
-    case 0x040: return s->mmfr;
+    case 0x040: return mcf_fec_read_mdio(s);
     case 0x044: return s->mscr;
     case 0x064: return 0; /* MIBC */
     case 0x084: return s->rcr;
@@ -287,8 +333,8 @@ static void mcf_fec_write(void *opaque, hwaddr addr,
         }
         break;
     case 0x040:
-        /* TODO: Implement MII.  */
         s->mmfr = value;
+        s->eir |= FEC_INT_MII;
         break;
     case 0x044:
         s->mscr = value & 0xfe;
diff --git a/include/hw/net/mii.h b/include/hw/net/mii.h
index cd2a4e2..9fdd7bb 100644
--- a/include/hw/net/mii.h
+++ b/include/hw/net/mii.h
@@ -65,7 +65,12 @@
 #define MII_ANLPAR_CSMACD   (1 << 0)
 
 /* List of vendor identifiers */
+/* RealTek 8201 */
 #define RTL8201CP_PHYID1    0x0000
 #define RTL8201CP_PHYID2    0x8201
 
+/* National Semiconductor DP83848 */
+#define DP83848_PHYID1      0x2000
+#define DP83848_PHYID2      0x5c90
+
 #endif /* MII_H */
-- 
1.9.1

[Qemu-devel] [PATCH 4/4] hw/net: fix mcf_fec driver receiver

[gerg]

From: Greg Ungerer <gerg@uclinux.org>

The network mcf_fec driver emulated receive side method is returning a
result of 0 causing the network layer to disable receive for this emulated
device. This results in the guest only ever receiving one packet.

Fix the recieve side processing to return the number of bytes that we
passed back through to the guest.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 hw/net/mcf_fec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c
index ea59017..e63af1b 100644
--- a/hw/net/mcf_fec.c
+++ b/hw/net/mcf_fec.c
@@ -413,6 +413,7 @@ static ssize_t mcf_fec_receive(NetClientState *nc, const uint8_t *buf, size_t si
     uint32_t buf_addr;
     uint8_t *crc_ptr;
     unsigned int buf_len;
+    size_t retsize;
 
     DPRINTF("do_rx len %d\n", size);
     if (!s->rx_enabled) {
@@ -432,6 +433,7 @@ static ssize_t mcf_fec_receive(NetClientState *nc, const uint8_t *buf, size_t si
         flags |= FEC_BD_LG;
     }
     addr = s->rx_descriptor;
+    retsize = size;
     while (size > 0) {
         mcf_fec_read_bd(&bd, addr);
         if ((bd.flags & FEC_BD_E) == 0) {
@@ -476,7 +478,7 @@ static ssize_t mcf_fec_receive(NetClientState *nc, const uint8_t *buf, size_t si
     s->rx_descriptor = addr;
     mcf_fec_enable_rx(s);
     mcf_fec_update(s);
-    return size;
+    return retsize;
 }
 
 static const MemoryRegionOps mcf_fec_ops = {
-- 
1.9.1

Re: [Qemu-devel] [PATCH 0/4] hw/net: fix m68/ColdFire ethernet fec support

[Stefan Hajnoczi]

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

On Fri, Jun 26, 2015 at 03:27:12PM +1000, gerg@uclinux.org wrote:
> 
> The following set of patches fixes the emulated ColdFire ethernet fec
> driver. There is primarily two problems that need to be fixed.
> 
> 1. The emulated driver needs to support probing of an attached phy.
>    It is strait forward to emulate an attached phy, but to avoid using
>    magic numbers I have factored out the common MII register and value
>    definitions into their own mii.h file first.
> 
> 2. Fix the fec driver receiver to return the correct value.
> 
> With these changes in place the qemu m5208evb board emulation can probe,
> identify and use the fec ethernet running a Linux guest.
> 
> 
>  hw/net/mcf_fec.c                |   54 ++++++++++++++++++++++++++--
>  include/hw/net/allwinner_emac.h |   40 ---------------------
>  include/hw/net/mii.h            |   76 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 128 insertions(+), 42 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

If this series has no maintainer to go through I'll merge it via my net
tree.  Please let me know if you'd like that.

mcf_fec is not fully correct yet, it needs to call
qemu_flush_queued_packets() when rx_enabled transitions from 0 to 1.
This will restart rx by sending any queued packets from the net
subsystem.

In order to get flow control between NetClientState peers working the
following is missing:

1. mcf_fec_receive()'s !s->rx_enabled case should return -1 to drop
   unexpected packets.

2. mcf_fec_receive() should return 0 in the (bd.flags & FEC_BD_E) == 0
   case where we've run out of rx buffers.  That way the net subsystem
   queues the packet and waits until the next RDAR write transitions
   rx_enabled from 0 to 1.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/4] hw/net: fix m68/ColdFire ethernet fec support

[Greg Ungerer]

Hi Stefan,

On 26/06/15 20:12, Stefan Hajnoczi wrote:
> On Fri, Jun 26, 2015 at 03:27:12PM +1000, gerg@uclinux.org wrote:
>> The following set of patches fixes the emulated ColdFire ethernet fec
>> driver. There is primarily two problems that need to be fixed.
>>
>> 1. The emulated driver needs to support probing of an attached phy.
>>     It is strait forward to emulate an attached phy, but to avoid using
>>     magic numbers I have factored out the common MII register and value
>>     definitions into their own mii.h file first.
>>
>> 2. Fix the fec driver receiver to return the correct value.
>>
>> With these changes in place the qemu m5208evb board emulation can probe,
>> identify and use the fec ethernet running a Linux guest.
>>
>>
>>   hw/net/mcf_fec.c                |   54 ++++++++++++++++++++++++++--
>>   include/hw/net/allwinner_emac.h |   40 ---------------------
>>   include/hw/net/mii.h            |   76 ++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 128 insertions(+), 42 deletions(-)
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> If this series has no maintainer to go through I'll merge it via my net
> tree.  Please let me know if you'd like that.

That would be great. Please do merge via your tree.
Thanks for the review.

> mcf_fec is not fully correct yet, it needs to call
> qemu_flush_queued_packets() when rx_enabled transitions from 0 to 1.
> This will restart rx by sending any queued packets from the net
> subsystem.
>
> In order to get flow control between NetClientState peers working the
> following is missing:
>
> 1. mcf_fec_receive()'s !s->rx_enabled case should return -1 to drop
>     unexpected packets.
>
> 2. mcf_fec_receive() should return 0 in the (bd.flags & FEC_BD_E) == 0
>     case where we've run out of rx buffers.  That way the net subsystem
>     queues the packet and waits until the next RDAR write transitions
>     rx_enabled from 0 to 1.

Ok, thanks for the pointers. I'll work up some additional changes
that handle these cases.

Regards
Greg

Re: [Qemu-devel] [PATCH 0/4] hw/net: fix m68/ColdFire ethernet fec support

[Greg Ungerer]

Hi Stefan,

On 26/06/15 20:12, Stefan Hajnoczi wrote:
> On Fri, Jun 26, 2015 at 03:27:12PM +1000, gerg@uclinux.org wrote:
>>
>> The following set of patches fixes the emulated ColdFire ethernet fec
>> driver. There is primarily two problems that need to be fixed.
>>
>> 1. The emulated driver needs to support probing of an attached phy.
>>    It is strait forward to emulate an attached phy, but to avoid using
>>    magic numbers I have factored out the common MII register and value
>>    definitions into their own mii.h file first.
>>
>> 2. Fix the fec driver receiver to return the correct value.
>>
>> With these changes in place the qemu m5208evb board emulation can probe,
>> identify and use the fec ethernet running a Linux guest.
>>
>>
>>  hw/net/mcf_fec.c                |   54 ++++++++++++++++++++++++++--
>>  include/hw/net/allwinner_emac.h |   40 ---------------------
>>  include/hw/net/mii.h            |   76 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 128 insertions(+), 42 deletions(-)
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> If this series has no maintainer to go through I'll merge it via my net
> tree.  Please let me know if you'd like that.
> 
> mcf_fec is not fully correct yet, it needs to call
> qemu_flush_queued_packets() when rx_enabled transitions from 0 to 1.
> This will restart rx by sending any queued packets from the net
> subsystem.
> 
> In order to get flow control between NetClientState peers working the
> following is missing:
> 
> 1. mcf_fec_receive()'s !s->rx_enabled case should return -1 to drop
>    unexpected packets.
> 
> 2. mcf_fec_receive() should return 0 in the (bd.flags & FEC_BD_E) == 0
>    case where we've run out of rx buffers.  That way the net subsystem
>    queues the packet and waits until the next RDAR write transitions
>    rx_enabled from 0 to 1.

Is the correct behavior in this case to check if we can write
out the whole packet data, or too partially write out what we can?

For example it is possible that we hit bd.flags & FEC_BD_E) == 0
having written some of the packet data but now we cannot fit the rest.
So is it correct to simply return what count we have written or
should we not write unless we can fit the whole packet and return 0.

Regards
Greg

Re: [Qemu-devel] [PATCH 0/4] hw/net: fix m68/ColdFire ethernet fec support

[Stefan Hajnoczi]

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

On Tue, Jun 30, 2015 at 03:38:11PM +1000, Greg Ungerer wrote:
> Hi Stefan,
> 
> On 26/06/15 20:12, Stefan Hajnoczi wrote:
> > On Fri, Jun 26, 2015 at 03:27:12PM +1000, gerg@uclinux.org wrote:
> >>
> >> The following set of patches fixes the emulated ColdFire ethernet fec
> >> driver. There is primarily two problems that need to be fixed.
> >>
> >> 1. The emulated driver needs to support probing of an attached phy.
> >>    It is strait forward to emulate an attached phy, but to avoid using
> >>    magic numbers I have factored out the common MII register and value
> >>    definitions into their own mii.h file first.
> >>
> >> 2. Fix the fec driver receiver to return the correct value.
> >>
> >> With these changes in place the qemu m5208evb board emulation can probe,
> >> identify and use the fec ethernet running a Linux guest.
> >>
> >>
> >>  hw/net/mcf_fec.c                |   54 ++++++++++++++++++++++++++--
> >>  include/hw/net/allwinner_emac.h |   40 ---------------------
> >>  include/hw/net/mii.h            |   76 ++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 128 insertions(+), 42 deletions(-)
> > 
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > If this series has no maintainer to go through I'll merge it via my net
> > tree.  Please let me know if you'd like that.
> > 
> > mcf_fec is not fully correct yet, it needs to call
> > qemu_flush_queued_packets() when rx_enabled transitions from 0 to 1.
> > This will restart rx by sending any queued packets from the net
> > subsystem.
> > 
> > In order to get flow control between NetClientState peers working the
> > following is missing:
> > 
> > 1. mcf_fec_receive()'s !s->rx_enabled case should return -1 to drop
> >    unexpected packets.
> > 
> > 2. mcf_fec_receive() should return 0 in the (bd.flags & FEC_BD_E) == 0
> >    case where we've run out of rx buffers.  That way the net subsystem
> >    queues the packet and waits until the next RDAR write transitions
> >    rx_enabled from 0 to 1.
> 
> Is the correct behavior in this case to check if we can write
> out the whole packet data, or too partially write out what we can?

It depends on the NIC you are emulation, but in all existing NICs we
either deliver the entire packet or we don't.

> For example it is possible that we hit bd.flags & FEC_BD_E) == 0
> having written some of the packet data but now we cannot fit the rest.
> So is it correct to simply return what count we have written or
> should we not write unless we can fit the whole packet and return 0.

Check the datasheet to see how the NIC handles that case.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/4] hw/net: fix m68/ColdFire ethernet fec support

[Stefan Hajnoczi]

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

On Fri, Jun 26, 2015 at 03:27:12PM +1000, gerg@uclinux.org wrote:
> 
> The following set of patches fixes the emulated ColdFire ethernet fec
> driver. There is primarily two problems that need to be fixed.
> 
> 1. The emulated driver needs to support probing of an attached phy.
>    It is strait forward to emulate an attached phy, but to avoid using
>    magic numbers I have factored out the common MII register and value
>    definitions into their own mii.h file first.
> 
> 2. Fix the fec driver receiver to return the correct value.
> 
> With these changes in place the qemu m5208evb board emulation can probe,
> identify and use the fec ethernet running a Linux guest.
> 
> 
>  hw/net/mcf_fec.c                |   54 ++++++++++++++++++++++++++--
>  include/hw/net/allwinner_emac.h |   40 ---------------------
>  include/hw/net/mii.h            |   76 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 128 insertions(+), 42 deletions(-)
> 
> 

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/4] hw/net: fix m68/ColdFire ethernet fec support

[Greg Ungerer]

Hi Stefan,

On 27/07/15 23:11, Stefan Hajnoczi wrote:
> On Tue, Jun 30, 2015 at 03:38:11PM +1000, Greg Ungerer wrote:
>> Hi Stefan,
>>
>> On 26/06/15 20:12, Stefan Hajnoczi wrote:
>>> On Fri, Jun 26, 2015 at 03:27:12PM +1000, gerg@uclinux.org wrote:
>>>>
>>>> The following set of patches fixes the emulated ColdFire ethernet fec
>>>> driver. There is primarily two problems that need to be fixed.
>>>>
>>>> 1. The emulated driver needs to support probing of an attached phy.
>>>>    It is strait forward to emulate an attached phy, but to avoid using
>>>>    magic numbers I have factored out the common MII register and value
>>>>    definitions into their own mii.h file first.
>>>>
>>>> 2. Fix the fec driver receiver to return the correct value.
>>>>
>>>> With these changes in place the qemu m5208evb board emulation can probe,
>>>> identify and use the fec ethernet running a Linux guest.
>>>>
>>>>
>>>>  hw/net/mcf_fec.c                |   54 ++++++++++++++++++++++++++--
>>>>  include/hw/net/allwinner_emac.h |   40 ---------------------
>>>>  include/hw/net/mii.h            |   76 ++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 128 insertions(+), 42 deletions(-)
>>>
>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>
>>> If this series has no maintainer to go through I'll merge it via my net
>>> tree.  Please let me know if you'd like that.
>>>
>>> mcf_fec is not fully correct yet, it needs to call
>>> qemu_flush_queued_packets() when rx_enabled transitions from 0 to 1.
>>> This will restart rx by sending any queued packets from the net
>>> subsystem.
>>>
>>> In order to get flow control between NetClientState peers working the
>>> following is missing:
>>>
>>> 1. mcf_fec_receive()'s !s->rx_enabled case should return -1 to drop
>>>    unexpected packets.
>>>
>>> 2. mcf_fec_receive() should return 0 in the (bd.flags & FEC_BD_E) == 0
>>>    case where we've run out of rx buffers.  That way the net subsystem
>>>    queues the packet and waits until the next RDAR write transitions
>>>    rx_enabled from 0 to 1.
>>
>> Is the correct behavior in this case to check if we can write
>> out the whole packet data, or too partially write out what we can?
> 
> It depends on the NIC you are emulation, but in all existing NICs we
> either deliver the entire packet or we don't.

Ok, that is good to know, thanks.


>> For example it is possible that we hit bd.flags & FEC_BD_E) == 0
>> having written some of the packet data but now we cannot fit the rest.
>> So is it correct to simply return what count we have written or
>> should we not write unless we can fit the whole packet and return 0.
> 
> Check the datasheet to see how the NIC handles that case.

I have a patch that addresses the above issues and uses an all-or-nothing
receive side processing. I'll send that through for review.

Regards
Greg