Re: [Qemu-devel] [PATCH] Add rtl8139b

[takasi-y]

Hi,
"Alexey Eremenko" <address@hidden> wrote:
> What is the reason behind implementing two versions of the same NIC in Qemu ?
Simple answer is because it is on "r2d".
(r2d(hw/r2d.c in qemu) is a board which has sh4 CPU and rtl8139 and others on 
it)

More precisely, because..
1. rtl8139 is supported by two different drivers in Linux kernel.
  Devices RevID<0x20 are supported by "8139too", others are by "8139cp".
2. Not like PC in qemu which has very flexible model, r2d is more
 specifically modeled. That is not only because HW is not flexible very much,
but also because software is built statically, as is common for embed devices.
Usually, linux kernel for r2d statically includes 8139too, but 8139cp.

"Igor Kovalenko" <address@hidden> wrote:
> Maybe original poster's rationale is required to continue :)

As far as my understanding, C+ is "B + new functions".
B's device drivers can handle it in same way, though RevID differs.
That's why my patch changes only RevID. Actually, "change only RevID" was
the starting point. I did it because I thought I can support B with small
modifications, and with only little side effect.

> -net nic,model=rtl8139[,quirks=8139cplus|8139b] default to 8139cplus
I think this change is a bit heavy.
Now, qemu code requires different name for different NIC, because differences
 of NICs are handled by NICInfo.model string.

Updated patch attached. Still no documentation (only enumerating a name)
 because I can't find suitable place.
How about using somewhat more self descriptive name like "rtl8139old" ?
/yoshii

This adds support for RTL8139 below Rev.C.
Name is "rtl8139b".
"rtl8139" is still RTL8139C and above as it was.

Signed-off-by: Takashi YOSHII <address@hidden>
---
 hw/pci.c      |    4 +++-
 hw/pci.h      |    1 +
 hw/r2d.c      |    2 +-
 hw/rtl8139.c  |   33 +++++++++++++++++++++------------
 qemu-doc.texi |    2 +-
 5 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 8252d21..9950d29 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -663,6 +663,8 @@ void pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn)
         pci_i82557b_init(bus, nd, devfn);
     } else if (strcmp(nd->model, "i82559er") == 0) {
         pci_i82559er_init(bus, nd, devfn);
+    } else if (strcmp(nd->model, "rtl8139b") == 0) {
+        pci_rtl8139b_init(bus, nd, devfn);
     } else if (strcmp(nd->model, "rtl8139") == 0) {
         pci_rtl8139_init(bus, nd, devfn);
     } else if (strcmp(nd->model, "e1000") == 0) {
@@ -673,7 +675,7 @@ void pci_nic_init(PCIBus *bus, NICInfo *nd, int devfn)
         virtio_net_init(bus, nd, devfn);
     } else if (strcmp(nd->model, "?") == 0) {
         fprintf(stderr, "qemu: Supported PCI NICs: i82551 i82557b i82559er"
-                        " ne2k_pci pcnet rtl8139 e1000 virtio\n");
+                        " ne2k_pci pcnet rtl8139b rtl8139 e1000 virtio\n");
         exit (1);
     } else {
         fprintf(stderr, "qemu: Unsupported NIC: %s\n", nd->model);
diff --git a/hw/pci.h b/hw/pci.h
index 3b1caf5..daca180 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -157,6 +157,7 @@ void pci_ne2000_init(PCIBus *bus, NICInfo *nd, int devfn);
 /* rtl8139.c */
 
 void pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn);
+void pci_rtl8139b_init(PCIBus *bus, NICInfo *nd, int devfn);
 
 /* e1000.c */
 void pci_e1000_init(PCIBus *bus, NICInfo *nd, int devfn);
diff --git a/hw/r2d.c b/hw/r2d.c
index 5d5eb1e..e2496fe 100644
--- a/hw/r2d.c
+++ b/hw/r2d.c
@@ -230,7 +230,7 @@ static void r2d_init(ram_addr_t ram_size, int vga_ram_size,
         drives_table[drive_get_index(IF_IDE, 0, 0)].bdrv, NULL);
 
     /* NIC: rtl8139 on-board, and 2 slots. */
-    pci_rtl8139_init(pci, &nd_table[0], 2 << 3);
+    pci_rtl8139b_init(pci, &nd_table[0], 2 << 3);
     for (i = 1; i < nb_nics; i++)
         pci_nic_init(pci, &nd_table[i], -1);
 
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index c3ab854..2aa889c 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -349,8 +349,6 @@ enum chip_flags {
 #define RTL8139_PCI_REVID_8139      0x10
 #define RTL8139_PCI_REVID_8139CPLUS 0x20
 
-#define RTL8139_PCI_REVID           RTL8139_PCI_REVID_8139CPLUS
-
 /* Size is 64 * 16bit words */
 #define EEPROM_9346_ADDR_BITS 6
 #define EEPROM_9346_SIZE  (1 << EEPROM_9346_ADDR_BITS)
@@ -1213,13 +1211,13 @@ static void rtl8139_reset(RTL8139State *s)
     /* ACK the reset */
     s->TxConfig = 0;
 
-#if 0
-//    s->TxConfig |= HW_REVID(1, 0, 0, 0, 0, 0, 0); // RTL-8139  HasHltClk
-    s->clock_enabled = 0;
-#else
-    s->TxConfig |= HW_REVID(1, 1, 1, 0, 1, 1, 0); // RTL-8139C+ HasLWake
-    s->clock_enabled = 1;
-#endif
+    if (s->pci_dev->config[0x08] == RTL8139_PCI_REVID_8139) {
+        s->TxConfig |= HW_REVID(1, 0, 0, 0, 0, 0, 0); // RTL-8139  HasHltClk
+        s->clock_enabled = 0;
+    } else {
+        s->TxConfig |= HW_REVID(1, 1, 1, 0, 1, 1, 0); // RTL-8139C+ HasLWake
+        s->clock_enabled = 1;
+    }
 
     s->bChipCmdState = CmdReset; /* RxBufEmpty bit is calculated on read from 
ChipCmd */;
 
@@ -2856,7 +2854,7 @@ static uint32_t rtl8139_io_readb(void *opaque, uint8_t 
addr)
             break;
 
         case PCIRevisionID:
-            ret = RTL8139_PCI_REVID;
+            ret = s->pci_dev->config[0x08];
             DEBUG_PRINT(("RTL8139: PCI Revision ID read 0x%x\n", ret));
             break;
 
@@ -3400,7 +3398,8 @@ static void rtl8139_timer(void *opaque)
 }
 #endif /* RTL8139_ONBOARD_TIMER */
 
-void pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn)
+void pci_rtl8139_common_init(PCIBus *bus, NICInfo *nd, int devfn,
+                                      uint8_t revid)
 {
     PCIRTL8139State *d;
     RTL8139State *s;
@@ -3416,7 +3415,7 @@ void pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn)
     pci_conf[0x02] = 0x39;
     pci_conf[0x03] = 0x81;
     pci_conf[0x04] = 0x05; /* command = I/O space, Bus Master */
-    pci_conf[0x08] = RTL8139_PCI_REVID; /* PCI revision ID; >=0x20 is for 
8139C+ */
+    pci_conf[0x08] = revid; /* PCI revision ID; >=0x20 is for 8139C+ */
     pci_conf[0x0a] = 0x00; /* ethernet network controller */
     pci_conf[0x0b] = 0x02;
     pci_conf[0x0e] = 0x00; /* header_type */
@@ -3463,3 +3462,13 @@ void pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int 
devfn)
         rtl8139_get_next_tctr_time(s,qemu_get_clock(vm_clock)));
 #endif /* RTL8139_ONBOARD_TIMER */
 }
+
+void pci_rtl8139b_init(PCIBus *bus, NICInfo *nd, int devfn)
+{
+    pci_rtl8139_common_init(bus, nd, devfn, RTL8139_PCI_REVID_8139);
+}
+
+void pci_rtl8139_init(PCIBus *bus, NICInfo *nd, int devfn)
+{
+    pci_rtl8139_common_init(bus, nd, devfn, RTL8139_PCI_REVID_8139CPLUS);
+}
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 698e0d5..b826e01 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -614,7 +614,7 @@ target. Optionally, the MAC address can be changed. If no
 Qemu can emulate several different models of network card.
 Valid values for @var{type} are
 @code{i82551}, @code{i82557b}, @code{i82559er},
address@hidden, @code{ne2k_isa}, @code{pcnet}, @code{rtl8139},
address@hidden, @code{ne2k_isa}, @code{pcnet}, @code{rtl8139b}, @code{rtl8139},
 @code{e1000}, @code{smc91c111}, @code{lance} and @code{mcf_fec}.
 Not all devices are supported on all targets.  Use -net nic,model=?
 for a list of available devices for your target.
-- 
1.5.6.3