[Qemu-devel] eepro100: Improve emulation and portability

[Qemu-devel] eepro100: Improve emulation and portability

[Stefan Weil]

These patches clean some parts of the code, improve the code for
big endian hosts and guests, and add some more control / status
register access methods.

They also add the usual workaround for non-standard short frames
which are generated by QEMU's current networking code.

The new register access methods were required for a proprietary
OS (prologue), partially also for windows guests.

[PATCH 1/9] eepro100: Avoid duplicate debug messages
[PATCH 2/9] eepro100: Fix endianness issues
[PATCH 3/9] eepro100: Support byte/word writes to port address
[PATCH 4/9] eepro100: Support byte/word writes to pointer register
[PATCH 5/9] eepro100: Support byte/word read/write access to MDI control register
[PATCH 6/9] eepro100: Support byte read access to general control register
[PATCH 7/9] eepro100: Support 32 bit read access to flash register
[PATCH 8/9] eepro100: Pad received short frames
[PATCH 9/9] eepro100: Simplify receive data structure

[Qemu-devel] [PATCH 1/9] eepro100: Avoid duplicate debug messages

[Stefan Weil]

When DEBUG_EEPRO100 was enabled, unsupported writes were logged twice.
Now logging in eepro100_write1 and eepro100_write2 is similar to the
logging in eepro100_write4 (which already was correct).

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index edf48f6..f89ff17 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -1,7 +1,7 @@
 /*
  * QEMU i8255x (PRO100) emulation
  *
- * Copyright (C) 2006-2010 Stefan Weil
+ * Copyright (C) 2006-2011 Stefan Weil
  *
  * Portions of the code are copies from grub / etherboot eepro100.c
  * and linux e100.c.
@@ -1393,18 +1393,20 @@ static void eepro100_write1(EEPRO100State * s, uint32_t addr, uint8_t val)
         memcpy(&s->mem[addr], &val, sizeof(val));
     }
 
-    TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
-
     switch (addr) {
     case SCBStatus:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         break;
     case SCBAck:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         eepro100_acknowledge(s);
         break;
     case SCBCmd:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         eepro100_write_command(s, val);
         break;
     case SCBIntmask:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         if (val & BIT(1)) {
             eepro100_swi_interrupt(s);
         }
@@ -1418,6 +1420,7 @@ static void eepro100_write1(EEPRO100State * s, uint32_t addr, uint8_t val)
         TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         break;
     case SCBeeprom:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         eepro100_write_eeprom(s->eeprom, val);
         break;
     default:
@@ -1433,18 +1436,19 @@ static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
         memcpy(&s->mem[addr], &val, sizeof(val));
     }
 
-    TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
-
     switch (addr) {
     case SCBStatus:
+        TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
         s->mem[SCBAck] = (val >> 8);
         eepro100_acknowledge(s);
         break;
     case SCBCmd:
+        TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
         eepro100_write_command(s, val);
         eepro100_write1(s, SCBIntmask, val >> 8);
         break;
     case SCBeeprom:
+        TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
         eepro100_write_eeprom(s->eeprom, val);
         break;
     default:
-- 
1.7.2.5

[Qemu-devel] [PATCH 2/9] eepro100: Fix endianness issues

[Stefan Weil]

Like other Intel devices, e100 (eepro100) uses little endian byte order.

This patch was tested with these combinations:

i386 host, i386 + mipsel guests (le-le)
mipsel host, i386 guest (le-le)
i386 host, mips + ppc guests (le-be)
mips host, i386 guest (be-le)

mips and mipsel hosts were emulated machines.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |  113 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 80 insertions(+), 33 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index f89ff17..c789767 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -20,11 +20,10 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Tested features (i82559):
- *      PXE boot (i386) ok
+ *      PXE boot (i386 guest, i386 / mips / mipsel / ppc host) ok
  *      Linux networking (i386) ok
  *
  * Untested:
- *      non-i386 platforms
  *      Windows networking
  *
  * References:
@@ -130,7 +129,7 @@ typedef struct {
 
 /* Offsets to the various registers.
    All accesses need not be longword aligned. */
-enum speedo_offsets {
+typedef enum {
     SCBStatus = 0,              /* Status Word. */
     SCBAck = 1,
     SCBCmd = 2,                 /* Rx/Command Unit command and status. */
@@ -145,7 +144,7 @@ enum speedo_offsets {
     SCBpmdr = 27,               /* Power Management Driver. */
     SCBgctrl = 28,              /* General Control. */
     SCBgstat = 29,              /* General Status. */
-};
+} E100RegisterOffset;
 
 /* A speedo3 transmit buffer descriptor with two buffers... */
 typedef struct {
@@ -307,7 +306,32 @@ static const uint16_t eepro100_mdi_mask[] = {
     0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
 };
 
-/* XXX: optimize */
+/* Read a 16 bit little endian value from physical memory. */
+static uint16_t lduw_le_phys(target_phys_addr_t addr)
+{
+    /* Load 16 bit (little endian) word from emulated hardware. */
+    uint16_t val;
+    cpu_physical_memory_read(addr, (uint8_t *)&val, sizeof(val));
+    return le16_to_cpu(val);
+}
+
+/* Read a 32 bit little endian value from physical memory. */
+static uint32_t ldl_le_phys(target_phys_addr_t addr)
+{
+    /* Load 32 bit (little endian) word from emulated hardware. */
+    uint32_t val;
+    cpu_physical_memory_read(addr, (uint8_t *)&val, sizeof(val));
+    return le32_to_cpu(val);
+}
+
+/* Write a 16 bit little endian value to physical memory. */
+static void stw_le_phys(target_phys_addr_t addr, uint16_t val)
+{
+    val = cpu_to_le16(val);
+    cpu_physical_memory_write(addr, (const uint8_t *)&val, sizeof(val));
+}
+
+/* Write a 32 bit little endian value to physical memory. */
 static void stl_le_phys(target_phys_addr_t addr, uint32_t val)
 {
     val = cpu_to_le32(val);
@@ -339,6 +363,32 @@ static unsigned compute_mcast_idx(const uint8_t * ep)
     return (crc & BITS(7, 2)) >> 2;
 }
 
+/* Read a 16 bit control/status (CSR) register. */
+static uint16_t e100_read_reg2(EEPRO100State *s, E100RegisterOffset addr)
+{
+    return le16_to_cpup((uint16_t *)&s->mem[addr]);
+}
+
+/* Read a 32 bit control/status (CSR) register. */
+static uint32_t e100_read_reg4(EEPRO100State *s, E100RegisterOffset addr)
+{
+    return le32_to_cpup((uint32_t *)&s->mem[addr]);
+}
+
+/* Write a 16 bit control/status (CSR) register. */
+static void e100_write_reg2(EEPRO100State *s, E100RegisterOffset addr,
+                            uint16_t val)
+{
+    cpu_to_le16w((uint16_t *)&s->mem[addr], val);
+}
+
+/* Read a 32 bit control/status (CSR) register. */
+static void e100_write_reg4(EEPRO100State *s, E100RegisterOffset addr,
+                            uint32_t val)
+{
+    cpu_to_le32w((uint32_t *)&s->mem[addr], val);
+}
+
 #if defined(DEBUG_EEPRO100)
 static const char *nic_dump(const uint8_t * buf, unsigned size)
 {
@@ -590,8 +640,7 @@ static void nic_selective_reset(EEPRO100State * s)
     TRACE(EEPROM, logout("checksum=0x%04x\n", eeprom_contents[EEPROM_SIZE - 1]));
 
     memset(s->mem, 0, sizeof(s->mem));
-    uint32_t val = BIT(21);
-    memcpy(&s->mem[SCBCtrlMDI], &val, sizeof(val));
+    e100_write_reg4(s, SCBCtrlMDI, BIT(21));
 
     assert(sizeof(s->mdimem) == sizeof(eepro100_mdi_default));
     memcpy(&s->mdimem[0], &eepro100_mdi_default[0], sizeof(s->mdimem));
@@ -739,10 +788,10 @@ static void tx_command(EEPRO100State *s)
     }
     assert(tcb_bytes <= sizeof(buf));
     while (size < tcb_bytes) {
-        uint32_t tx_buffer_address = ldl_phys(tbd_address);
-        uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
+        uint32_t tx_buffer_address = ldl_le_phys(tbd_address);
+        uint16_t tx_buffer_size = lduw_le_phys(tbd_address + 4);
 #if 0
-        uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
+        uint16_t tx_buffer_el = lduw_le_phys(tbd_address + 6);
 #endif
         tbd_address += 8;
         TRACE(RXTX, logout
@@ -761,9 +810,9 @@ static void tx_command(EEPRO100State *s)
         if (s->has_extended_tcb_support && !(s->configuration[6] & BIT(4))) {
             /* Extended Flexible TCB. */
             for (; tbd_count < 2; tbd_count++) {
-                uint32_t tx_buffer_address = ldl_phys(tbd_address);
-                uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
-                uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
+                uint32_t tx_buffer_address = ldl_le_phys(tbd_address);
+                uint16_t tx_buffer_size = lduw_le_phys(tbd_address + 4);
+                uint16_t tx_buffer_el = lduw_le_phys(tbd_address + 6);
                 tbd_address += 8;
                 TRACE(RXTX, logout
                     ("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
@@ -779,9 +828,9 @@ static void tx_command(EEPRO100State *s)
         }
         tbd_address = tbd_array;
         for (; tbd_count < s->tx.tbd_count; tbd_count++) {
-            uint32_t tx_buffer_address = ldl_phys(tbd_address);
-            uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
-            uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
+            uint32_t tx_buffer_address = ldl_le_phys(tbd_address);
+            uint16_t tx_buffer_size = lduw_le_phys(tbd_address + 4);
+            uint16_t tx_buffer_el = lduw_le_phys(tbd_address + 6);
             tbd_address += 8;
             TRACE(RXTX, logout
                 ("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
@@ -889,7 +938,7 @@ static void action_command(EEPRO100State *s)
             break;
         }
         /* Write new status. */
-        stw_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
+        stw_le_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
         if (bit_i) {
             /* CU completed action. */
             eepro100_cx_interrupt(s);
@@ -1050,8 +1099,7 @@ static void eepro100_write_command(EEPRO100State * s, uint8_t val)
 
 static uint16_t eepro100_read_eeprom(EEPRO100State * s)
 {
-    uint16_t val;
-    memcpy(&val, &s->mem[SCBeeprom], sizeof(val));
+    uint16_t val = e100_read_reg4(s, SCBeeprom);
     if (eeprom93xx_read(s->eeprom)) {
         val |= EEPROM_DO;
     } else {
@@ -1121,8 +1169,7 @@ static const char *reg2name(uint8_t reg)
 
 static uint32_t eepro100_read_mdi(EEPRO100State * s)
 {
-    uint32_t val;
-    memcpy(&val, &s->mem[0x10], sizeof(val));
+    uint32_t val = e100_read_reg4(s, SCBCtrlMDI);
 
 #ifdef DEBUG_EEPRO100
     uint8_t raiseint = (val & BIT(29)) >> 29;
@@ -1231,7 +1278,7 @@ static void eepro100_write_mdi(EEPRO100State * s, uint32_t val)
         }
     }
     val = (val & 0xffff0000) + data;
-    memcpy(&s->mem[0x10], &val, sizeof(val));
+    e100_write_reg4(s, SCBCtrlMDI, val);
 }
 
 /*****************************************************************************
@@ -1258,7 +1305,6 @@ static uint32_t eepro100_read_port(EEPRO100State * s)
 
 static void eepro100_write_port(EEPRO100State * s, uint32_t val)
 {
-    val = le32_to_cpu(val);
     uint32_t address = (val & ~PORT_SELECTION_MASK);
     uint8_t selection = (val & PORT_SELECTION_MASK);
     switch (selection) {
@@ -1293,7 +1339,7 @@ static uint8_t eepro100_read1(EEPRO100State * s, uint32_t addr)
 {
     uint8_t val = 0;
     if (addr <= sizeof(s->mem) - sizeof(val)) {
-        memcpy(&val, &s->mem[addr], sizeof(val));
+        val = s->mem[addr];
     }
 
     switch (addr) {
@@ -1336,7 +1382,7 @@ static uint16_t eepro100_read2(EEPRO100State * s, uint32_t addr)
 {
     uint16_t val = 0;
     if (addr <= sizeof(s->mem) - sizeof(val)) {
-        memcpy(&val, &s->mem[addr], sizeof(val));
+        val = e100_read_reg2(s, addr);
     }
 
     switch (addr) {
@@ -1359,7 +1405,7 @@ static uint32_t eepro100_read4(EEPRO100State * s, uint32_t addr)
 {
     uint32_t val = 0;
     if (addr <= sizeof(s->mem) - sizeof(val)) {
-        memcpy(&val, &s->mem[addr], sizeof(val));
+        val = e100_read_reg4(s, addr);
     }
 
     switch (addr) {
@@ -1390,7 +1436,7 @@ static void eepro100_write1(EEPRO100State * s, uint32_t addr, uint8_t val)
 {
     /* SCBStatus is readonly. */
     if (addr > SCBStatus && addr <= sizeof(s->mem) - sizeof(val)) {
-        memcpy(&s->mem[addr], &val, sizeof(val));
+        s->mem[addr] = val;
     }
 
     switch (addr) {
@@ -1433,7 +1479,7 @@ static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
 {
     /* SCBStatus is readonly. */
     if (addr > SCBStatus && addr <= sizeof(s->mem) - sizeof(val)) {
-        memcpy(&s->mem[addr], &val, sizeof(val));
+        e100_write_reg2(s, addr, val);
     }
 
     switch (addr) {
@@ -1460,7 +1506,7 @@ static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
 static void eepro100_write4(EEPRO100State * s, uint32_t addr, uint32_t val)
 {
     if (addr <= sizeof(s->mem) - sizeof(val)) {
-        memcpy(&s->mem[addr], &val, sizeof(val));
+        e100_write_reg4(s, addr, val);
     }
 
     switch (addr) {
@@ -1753,9 +1799,10 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
     }
     TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
           rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
-    stw_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, status),
-             rfd_status);
-    stw_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, count), size);
+    stw_le_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, status),
+                rfd_status);
+    stw_le_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, count),
+                size);
     /* Early receive interrupt not supported. */
 #if 0
     eepro100_er_interrupt(s);
@@ -1884,7 +1931,7 @@ static int e100_nic_init(PCIDevice *pci_dev)
     /* Handler for memory-mapped I/O */
     s->mmio_index =
         cpu_register_io_memory(pci_mmio_read, pci_mmio_write, s,
-                               DEVICE_NATIVE_ENDIAN);
+                               DEVICE_LITTLE_ENDIAN);
 
     pci_register_bar(&s->dev, 0, PCI_MEM_SIZE,
                            PCI_BASE_ADDRESS_SPACE_MEMORY |
-- 
1.7.2.5

[Qemu-devel] [PATCH 3/9] eepro100: Support byte/word writes to port address

[Stefan Weil]

port is a 32 bit register, but may be written using 8 or 16 bit writes.
Add support for byte/word writes.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index c789767..47ef78c 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -1303,8 +1303,9 @@ static uint32_t eepro100_read_port(EEPRO100State * s)
     return 0;
 }
 
-static void eepro100_write_port(EEPRO100State * s, uint32_t val)
+static void eepro100_write_port(EEPRO100State * s)
 {
+    uint32_t val = e100_read_reg4(s, SCBPort);
     uint32_t address = (val & ~PORT_SELECTION_MASK);
     uint8_t selection = (val & PORT_SELECTION_MASK);
     switch (selection) {
@@ -1458,7 +1459,15 @@ static void eepro100_write1(EEPRO100State * s, uint32_t addr, uint8_t val)
         }
         eepro100_interrupt(s, 0);
         break;
+    case SCBPort:
+    case SCBPort + 1:
+    case SCBPort + 2:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
+        break;
     case SCBPort + 3:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
+        eepro100_write_port(s);
+        break;
     case SCBFlow:       /* does not exist on 82557 */
     case SCBFlow + 1:
     case SCBFlow + 2:
@@ -1493,6 +1502,13 @@ static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
         eepro100_write_command(s, val);
         eepro100_write1(s, SCBIntmask, val >> 8);
         break;
+    case SCBPort:
+        TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
+        break;
+    case SCBPort + 2:
+        TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
+        eepro100_write_port(s);
+        break;
     case SCBeeprom:
         TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
         eepro100_write_eeprom(s->eeprom, val);
@@ -1515,7 +1531,7 @@ static void eepro100_write4(EEPRO100State * s, uint32_t addr, uint32_t val)
         break;
     case SCBPort:
         TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
-        eepro100_write_port(s, val);
+        eepro100_write_port(s);
         break;
     case SCBCtrlMDI:
         eepro100_write_mdi(s, val);
-- 
1.7.2.5

[Qemu-devel] [PATCH 4/9] eepro100: Support byte/word writes to pointer register

[Stefan Weil]

pointer is a 32 bit register, but may be written using 8 or 16 bit writes.
Add support for byte/word writes.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |   33 ++++++++++++++++-----------------
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index 47ef78c..df96394 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -231,7 +231,6 @@ typedef struct {
     uint16_t mdimem[32];
     eeprom_t *eeprom;
     uint32_t device;            /* device variant */
-    uint32_t pointer;
     /* (cu_base + cu_offset) address the next command block in the command block list. */
     uint32_t cu_base;           /* CU base address */
     uint32_t cu_offset;         /* CU address offset */
@@ -977,7 +976,7 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
             logout("unexpected CU state is %u\n", cu_state);
         }
         set_cu_state(s, cu_active);
-        s->cu_offset = s->pointer;
+        s->cu_offset = e100_read_reg4(s, SCBPointer);
         action_command(s);
         break;
     case CU_RESUME:
@@ -998,7 +997,7 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
         break;
     case CU_STATSADDR:
         /* Load dump counters address. */
-        s->statsaddr = s->pointer;
+        s->statsaddr = e100_read_reg4(s, SCBPointer);
         TRACE(OTHER, logout("val=0x%02x (status address)\n", val));
         break;
     case CU_SHOWSTATS:
@@ -1010,7 +1009,7 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
     case CU_CMD_BASE:
         /* Load CU base. */
         TRACE(OTHER, logout("val=0x%02x (CU base address)\n", val));
-        s->cu_base = s->pointer;
+        s->cu_base = e100_read_reg4(s, SCBPointer);
         break;
     case CU_DUMPSTATS:
         /* Dump and reset statistical counters. */
@@ -1043,7 +1042,7 @@ static void eepro100_ru_command(EEPRO100State * s, uint8_t val)
 #endif
         }
         set_ru_state(s, ru_ready);
-        s->ru_offset = s->pointer;
+        s->ru_offset = e100_read_reg4(s, SCBPointer);
         TRACE(OTHER, logout("val=0x%02x (rx start)\n", val));
         break;
     case RX_RESUME:
@@ -1067,7 +1066,7 @@ static void eepro100_ru_command(EEPRO100State * s, uint8_t val)
     case RX_ADDR_LOAD:
         /* Load RU base. */
         TRACE(OTHER, logout("val=0x%02x (RU base address)\n", val));
-        s->ru_base = s->pointer;
+        s->ru_base = e100_read_reg4(s, SCBPointer);
         break;
     default:
         logout("val=0x%02x (undefined RU command)\n", val);
@@ -1124,12 +1123,6 @@ static void eepro100_write_eeprom(eeprom_t * eeprom, uint8_t val)
     eeprom93xx_write(eeprom, eecs, eesk, eedi);
 }
 
-static void eepro100_write_pointer(EEPRO100State * s, uint32_t val)
-{
-    s->pointer = le32_to_cpu(val);
-    TRACE(OTHER, logout("val=0x%08x\n", val));
-}
-
 /*****************************************************************************
  *
  * MDI emulation.
@@ -1414,9 +1407,6 @@ static uint32_t eepro100_read4(EEPRO100State * s, uint32_t addr)
         TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
         break;
     case SCBPointer:
-#if 0
-        val = eepro100_read_pointer(s);
-#endif
         TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
         break;
     case SCBPort:
@@ -1459,6 +1449,12 @@ static void eepro100_write1(EEPRO100State * s, uint32_t addr, uint8_t val)
         }
         eepro100_interrupt(s, 0);
         break;
+    case SCBPointer:
+    case SCBPointer + 1:
+    case SCBPointer + 2:
+    case SCBPointer + 3:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
+        break;
     case SCBPort:
     case SCBPort + 1:
     case SCBPort + 2:
@@ -1502,6 +1498,10 @@ static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
         eepro100_write_command(s, val);
         eepro100_write1(s, SCBIntmask, val >> 8);
         break;
+    case SCBPointer:
+    case SCBPointer + 2:
+        TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
+        break;
     case SCBPort:
         TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
         break;
@@ -1527,7 +1527,7 @@ static void eepro100_write4(EEPRO100State * s, uint32_t addr, uint32_t val)
 
     switch (addr) {
     case SCBPointer:
-        eepro100_write_pointer(s, val);
+        TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
         break;
     case SCBPort:
         TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
@@ -1868,7 +1868,6 @@ static const VMStateDescription vmstate_eepro100 = {
         /* The eeprom should be saved and restored by its own routines. */
         VMSTATE_UINT32(device, EEPRO100State),
         /* TODO check device. */
-        VMSTATE_UINT32(pointer, EEPRO100State),
         VMSTATE_UINT32(cu_base, EEPRO100State),
         VMSTATE_UINT32(cu_offset, EEPRO100State),
         VMSTATE_UINT32(ru_base, EEPRO100State),
-- 
1.7.2.5

[Qemu-devel] [PATCH 5/9] eepro100: Support byte/word read/write access to MDI control register

[Stefan Weil]

MDI control is a 32 bit register, but may be read or written using
8 or 16 bit access. Data is latched when the MSB is written.

Add support for byte/word read/write access.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |   34 ++++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index df96394..2332556 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -1179,8 +1179,9 @@ static uint32_t eepro100_read_mdi(EEPRO100State * s)
     return val;
 }
 
-static void eepro100_write_mdi(EEPRO100State * s, uint32_t val)
+static void eepro100_write_mdi(EEPRO100State * s)
 {
+    uint32_t val = e100_read_reg4(s, SCBCtrlMDI);
     uint8_t raiseint = (val & BIT(29)) >> 29;
     uint8_t opcode = (val & BITS(27, 26)) >> 26;
     uint8_t phy = (val & BITS(25, 21)) >> 21;
@@ -1356,6 +1357,13 @@ static uint8_t eepro100_read1(EEPRO100State * s, uint32_t addr)
     case SCBeeprom:
         val = eepro100_read_eeprom(s);
         break;
+    case SCBCtrlMDI:
+    case SCBCtrlMDI + 1:
+    case SCBCtrlMDI + 2:
+    case SCBCtrlMDI + 3:
+        val = (uint8_t)(eepro100_read_mdi(s) >> (8 * (addr & 3)));
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
+        break;
     case SCBpmdr:       /* Power Management Driver Register */
         val = 0;
         TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
@@ -1388,6 +1396,11 @@ static uint16_t eepro100_read2(EEPRO100State * s, uint32_t addr)
         val = eepro100_read_eeprom(s);
         TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
         break;
+    case SCBCtrlMDI:
+    case SCBCtrlMDI + 2:
+        val = (uint16_t)(eepro100_read_mdi(s) >> (8 * (addr & 3)));
+        TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
+        break;
     default:
         logout("addr=%s val=0x%04x\n", regname(addr), val);
         missing("unknown word read");
@@ -1474,6 +1487,15 @@ static void eepro100_write1(EEPRO100State * s, uint32_t addr, uint8_t val)
         TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         eepro100_write_eeprom(s->eeprom, val);
         break;
+    case SCBCtrlMDI:
+    case SCBCtrlMDI + 1:
+    case SCBCtrlMDI + 2:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
+        break;
+    case SCBCtrlMDI + 3:
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
+        eepro100_write_mdi(s);
+        break;
     default:
         logout("addr=%s val=0x%02x\n", regname(addr), val);
         missing("unknown byte write");
@@ -1513,6 +1535,13 @@ static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
         TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
         eepro100_write_eeprom(s->eeprom, val);
         break;
+    case SCBCtrlMDI:
+        TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
+        break;
+    case SCBCtrlMDI + 2:
+        TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val));
+        eepro100_write_mdi(s);
+        break;
     default:
         logout("addr=%s val=0x%04x\n", regname(addr), val);
         missing("unknown word write");
@@ -1534,7 +1563,8 @@ static void eepro100_write4(EEPRO100State * s, uint32_t addr, uint32_t val)
         eepro100_write_port(s);
         break;
     case SCBCtrlMDI:
-        eepro100_write_mdi(s, val);
+        TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
+        eepro100_write_mdi(s);
         break;
     default:
         logout("addr=%s val=0x%08x\n", regname(addr), val);
-- 
1.7.2.5

[Qemu-devel] [PATCH 6/9] eepro100: Support byte read access to general control register

[Stefan Weil]

The general control register is a byte register.
Add support for byte reads.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index 2332556..b51e391 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -1368,6 +1368,9 @@ static uint8_t eepro100_read1(EEPRO100State * s, uint32_t addr)
         val = 0;
         TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
         break;
+    case SCBgctrl:      /* General Control Register */
+        TRACE(OTHER, logout("addr=%s val=0x%02x\n", regname(addr), val));
+        break;
     case SCBgstat:      /* General Status Register */
         /* 100 Mbps full duplex, valid link */
         val = 0x07;
-- 
1.7.2.5

[Qemu-devel] [PATCH 7/9] eepro100: Support 32 bit read access to flash register

[Stefan Weil]

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index b51e391..500a3af 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -1429,6 +1429,10 @@ static uint32_t eepro100_read4(EEPRO100State * s, uint32_t addr)
         val = eepro100_read_port(s);
         TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
         break;
+    case SCBflash:
+        val = eepro100_read_eeprom(s);
+        TRACE(OTHER, logout("addr=%s val=0x%08x\n", regname(addr), val));
+        break;
     case SCBCtrlMDI:
         val = eepro100_read_mdi(s);
         break;
-- 
1.7.2.5

[Qemu-devel] [PATCH 8/9] eepro100: Pad received short frames

[Stefan Weil]

QEMU sends frames smaller than 60 bytes to ethernet nics.
This should be fixed in the networking code because normally
such frames are rejected by real NICs and their emulations.
To avoid this behaviour, other NIC emulations pad received
frames. This patch enables this workaround for eepro100, too.

All related code is marked with CONFIG_PAD_RECEIVED_FRAMES,
so emulation of the correct handling for short frames can
be restored as soon as QEMU's networking code is fixed.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index 500a3af..a740d2e 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -47,6 +47,14 @@
 #include "eeprom93xx.h"
 #include "sysemu.h"
 
+/* QEMU sends frames smaller than 60 bytes to ethernet nics.
+ * This should be fixed in the networking code because normally
+ * such frames are rejected by real nics and their emulations.
+ * To avoid this behaviour, other nic emulations pad received
+ * frames. The following definition enables this workaround for
+ * eepro100, too. */
+#define CONFIG_PAD_RECEIVED_FRAMES
+
 #define KiB 1024
 
 /* Debug EEPRO100 card. */
@@ -1756,19 +1764,32 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
      */
     EEPRO100State *s = DO_UPCAST(NICState, nc, nc)->opaque;
     uint16_t rfd_status = 0xa000;
+#if defined(CONFIG_PAD_RECEIVED_FRAMES)
+    uint8_t min_buf[60];
+#endif
     static const uint8_t broadcast_macaddr[6] =
         { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
+#if defined(CONFIG_PAD_RECEIVED_FRAMES)
+    /* Pad to minimum Ethernet frame length */
+    if (size < sizeof(min_buf)) {
+        memcpy(min_buf, buf, size);
+        memset(&min_buf[size], 0, sizeof(min_buf) - size);
+        buf = min_buf;
+        size = sizeof(min_buf);
+    }
+#endif
+
     if (s->configuration[8] & 0x80) {
         /* CSMA is disabled. */
         logout("%p received while CSMA is disabled\n", s);
         return -1;
+#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
     } else if (size < 64 && (s->configuration[7] & BIT(0))) {
         /* Short frame and configuration byte 7/0 (discard short receive) set:
          * Short frame is discarded */
         logout("%p received short frame (%zu byte)\n", s, size);
         s->statistics.rx_short_frame_errors++;
-#if 0
         return -1;
 #endif
     } else if ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] & BIT(3))) {
@@ -1847,9 +1868,11 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
             "(%zu bytes); data truncated\n", rfd_size, size);
         size = rfd_size;
     }
+#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
     if (size < 64) {
         rfd_status |= 0x0080;
     }
+#endif
     TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
           rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
     stw_le_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, status),
-- 
1.7.2.5

[Qemu-devel] [PATCH 9/9] eepro100: Simplify receive data structure

[Stefan Weil]

The packet data part of this structure was only used to calculate
the size of the preceding data. This can be simplified.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index a740d2e..d4d315b 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -180,7 +180,7 @@ typedef struct {
     uint32_t rx_buf_addr;       /* void * */
     uint16_t count;
     uint16_t size;
-    char packet[MAX_ETH_FRAME_SIZE + 4];
+    /* Ethernet frame data follows. */
 } eepro100_rx_t;
 
 typedef enum {
@@ -1859,7 +1859,7 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
     /* !!! */
     eepro100_rx_t rx;
     cpu_physical_memory_read(s->ru_base + s->ru_offset, (uint8_t *) & rx,
-                             offsetof(eepro100_rx_t, packet));
+                             sizeof(eepro100_rx_t));
     uint16_t rfd_command = le16_to_cpu(rx.command);
     uint16_t rfd_size = le16_to_cpu(rx.size);
 
@@ -1893,7 +1893,7 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
     assert(!(s->configuration[17] & BIT(0)));
 #endif
     cpu_physical_memory_write(s->ru_base + s->ru_offset +
-                              offsetof(eepro100_rx_t, packet), buf, size);
+                              sizeof(eepro100_rx_t), buf, size);
     s->statistics.rx_good_frames++;
     eepro100_fr_interrupt(s);
     s->ru_offset = le32_to_cpu(rx.link);
-- 
1.7.2.5

[Qemu-devel] Re: [PATCH 8/9] eepro100: Pad received short frames

[Michael S. Tsirkin]

On Thu, Mar 31, 2011 at 10:33:30PM +0200, Stefan Weil wrote:
> QEMU sends frames smaller than 60 bytes to ethernet nics.
> This should be fixed in the networking code because normally
> such frames are rejected by real NICs and their emulations.
> To avoid this behaviour, other NIC emulations pad received
> frames. This patch enables this workaround for eepro100, too.
> 
> All related code is marked with CONFIG_PAD_RECEIVED_FRAMES,
> so emulation of the correct handling for short frames can
> be restored as soon as QEMU's networking code is fixed.
> 
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>

qemu networking core isn't ethernet-specific
(slirp is). That's why we don't do this in core,
so the uglification isn't worth it.

> ---
>  hw/eepro100.c |   25 ++++++++++++++++++++++++-
>  1 files changed, 24 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index 500a3af..a740d2e 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -47,6 +47,14 @@
>  #include "eeprom93xx.h"
>  #include "sysemu.h"
>  
> +/* QEMU sends frames smaller than 60 bytes to ethernet nics.
> + * This should be fixed in the networking code because normally
> + * such frames are rejected by real nics and their emulations.
> + * To avoid this behaviour, other nic emulations pad received
> + * frames. The following definition enables this workaround for
> + * eepro100, too. */
> +#define CONFIG_PAD_RECEIVED_FRAMES
> +
>  #define KiB 1024
>  
>  /* Debug EEPRO100 card. */
> @@ -1756,19 +1764,32 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
>       */
>      EEPRO100State *s = DO_UPCAST(NICState, nc, nc)->opaque;
>      uint16_t rfd_status = 0xa000;
> +#if defined(CONFIG_PAD_RECEIVED_FRAMES)
> +    uint8_t min_buf[60];
> +#endif
>      static const uint8_t broadcast_macaddr[6] =
>          { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>  
> +#if defined(CONFIG_PAD_RECEIVED_FRAMES)
> +    /* Pad to minimum Ethernet frame length */
> +    if (size < sizeof(min_buf)) {
> +        memcpy(min_buf, buf, size);
> +        memset(&min_buf[size], 0, sizeof(min_buf) - size);
> +        buf = min_buf;
> +        size = sizeof(min_buf);
> +    }
> +#endif
> +
>      if (s->configuration[8] & 0x80) {
>          /* CSMA is disabled. */
>          logout("%p received while CSMA is disabled\n", s);
>          return -1;
> +#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
>      } else if (size < 64 && (s->configuration[7] & BIT(0))) {
>          /* Short frame and configuration byte 7/0 (discard short receive) set:
>           * Short frame is discarded */
>          logout("%p received short frame (%zu byte)\n", s, size);
>          s->statistics.rx_short_frame_errors++;
> -#if 0
>          return -1;
>  #endif
>      } else if ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] & BIT(3))) {
> @@ -1847,9 +1868,11 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
>              "(%zu bytes); data truncated\n", rfd_size, size);
>          size = rfd_size;
>      }
> +#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
>      if (size < 64) {
>          rfd_status |= 0x0080;
>      }
> +#endif
>      TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
>            rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
>      stw_le_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, status),
> -- 
> 1.7.2.5

[Qemu-devel] Re: [PATCH 2/9] eepro100: Fix endianness issues

[Michael S. Tsirkin]

On Thu, Mar 31, 2011 at 10:33:24PM +0200, Stefan Weil wrote:
> Like other Intel devices, e100 (eepro100) uses little endian byte order.
> 
> This patch was tested with these combinations:
> 
> i386 host, i386 + mipsel guests (le-le)
> mipsel host, i386 guest (le-le)
> i386 host, mips + ppc guests (le-be)
> mips host, i386 guest (be-le)
> 
> mips and mipsel hosts were emulated machines.
> 
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  hw/eepro100.c |  113 ++++++++++++++++++++++++++++++++++++++++-----------------
>  1 files changed, 80 insertions(+), 33 deletions(-)
> 
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index f89ff17..c789767 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -20,11 +20,10 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   *
>   * Tested features (i82559):
> - *      PXE boot (i386) ok
> + *      PXE boot (i386 guest, i386 / mips / mipsel / ppc host) ok
>   *      Linux networking (i386) ok
>   *
>   * Untested:
> - *      non-i386 platforms
>   *      Windows networking
>   *
>   * References:
> @@ -130,7 +129,7 @@ typedef struct {
>  
>  /* Offsets to the various registers.
>     All accesses need not be longword aligned. */
> -enum speedo_offsets {
> +typedef enum {
>      SCBStatus = 0,              /* Status Word. */
>      SCBAck = 1,
>      SCBCmd = 2,                 /* Rx/Command Unit command and status. */
> @@ -145,7 +144,7 @@ enum speedo_offsets {
>      SCBpmdr = 27,               /* Power Management Driver. */
>      SCBgctrl = 28,              /* General Control. */
>      SCBgstat = 29,              /* General Status. */
> -};
> +} E100RegisterOffset;
>  
>  /* A speedo3 transmit buffer descriptor with two buffers... */
>  typedef struct {
> @@ -307,7 +306,32 @@ static const uint16_t eepro100_mdi_mask[] = {
>      0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
>  };
>  
> -/* XXX: optimize */
> +/* Read a 16 bit little endian value from physical memory. */
> +static uint16_t lduw_le_phys(target_phys_addr_t addr)
> +{
> +    /* Load 16 bit (little endian) word from emulated hardware. */
> +    uint16_t val;
> +    cpu_physical_memory_read(addr, (uint8_t *)&val, sizeof(val));
> +    return le16_to_cpu(val);
> +}
> +
> +/* Read a 32 bit little endian value from physical memory. */
> +static uint32_t ldl_le_phys(target_phys_addr_t addr)
> +{
> +    /* Load 32 bit (little endian) word from emulated hardware. */
> +    uint32_t val;
> +    cpu_physical_memory_read(addr, (uint8_t *)&val, sizeof(val));
> +    return le32_to_cpu(val);
> +}
> +
> +/* Write a 16 bit little endian value to physical memory. */
> +static void stw_le_phys(target_phys_addr_t addr, uint16_t val)
> +{
> +    val = cpu_to_le16(val);
> +    cpu_physical_memory_write(addr, (const uint8_t *)&val, sizeof(val));
> +}
> +
> +/* Write a 32 bit little endian value to physical memory. */

So why not opencode e.g.
	le32_to_cpu(ldl_phys(addr))

wrappers really worth it? What do I miss?

If you insist on these online wrappers, pls prefix
them with eepro100_.
Also, why not use lduw_phys and friends internally?
cpu_physical_  is slower ...

>  static void stl_le_phys(target_phys_addr_t addr, uint32_t val)
>  {
>      val = cpu_to_le32(val);
> @@ -339,6 +363,32 @@ static unsigned compute_mcast_idx(const uint8_t * ep)
>      return (crc & BITS(7, 2)) >> 2;
>  }
>  
> +/* Read a 16 bit control/status (CSR) register. */
> +static uint16_t e100_read_reg2(EEPRO100State *s, E100RegisterOffset addr)
> +{
> +    return le16_to_cpup((uint16_t *)&s->mem[addr]);
> +}
> +
> +/* Read a 32 bit control/status (CSR) register. */
> +static uint32_t e100_read_reg4(EEPRO100State *s, E100RegisterOffset addr)
> +{
> +    return le32_to_cpup((uint32_t *)&s->mem[addr]);
> +}
> +
> +/* Write a 16 bit control/status (CSR) register. */
> +static void e100_write_reg2(EEPRO100State *s, E100RegisterOffset addr,
> +                            uint16_t val)
> +{
> +    cpu_to_le16w((uint16_t *)&s->mem[addr], val);
> +}
> +
> +/* Read a 32 bit control/status (CSR) register. */
> +static void e100_write_reg4(EEPRO100State *s, E100RegisterOffset addr,
> +                            uint32_t val)
> +{
> +    cpu_to_le32w((uint32_t *)&s->mem[addr], val);
> +}
> +

Note that cpu_to_le32w requires an aligned address, unlike
memcpy, and there's no guarantee
addr is aligned apparently?

If true you need to memcpy to a 32 bit variable, then
cpu_to_le32w ther result.

>  #if defined(DEBUG_EEPRO100)
>  static const char *nic_dump(const uint8_t * buf, unsigned size)
>  {
> @@ -590,8 +640,7 @@ static void nic_selective_reset(EEPRO100State * s)
>      TRACE(EEPROM, logout("checksum=0x%04x\n", eeprom_contents[EEPROM_SIZE - 1]));
>  
>      memset(s->mem, 0, sizeof(s->mem));
> -    uint32_t val = BIT(21);
> -    memcpy(&s->mem[SCBCtrlMDI], &val, sizeof(val));
> +    e100_write_reg4(s, SCBCtrlMDI, BIT(21));
>  
>      assert(sizeof(s->mdimem) == sizeof(eepro100_mdi_default));
>      memcpy(&s->mdimem[0], &eepro100_mdi_default[0], sizeof(s->mdimem));
> @@ -739,10 +788,10 @@ static void tx_command(EEPRO100State *s)
>      }
>      assert(tcb_bytes <= sizeof(buf));
>      while (size < tcb_bytes) {
> -        uint32_t tx_buffer_address = ldl_phys(tbd_address);
> -        uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> +        uint32_t tx_buffer_address = ldl_le_phys(tbd_address);
> +        uint16_t tx_buffer_size = lduw_le_phys(tbd_address + 4);
>  #if 0
> -        uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> +        uint16_t tx_buffer_el = lduw_le_phys(tbd_address + 6);
>  #endif
>          tbd_address += 8;
>          TRACE(RXTX, logout
> @@ -761,9 +810,9 @@ static void tx_command(EEPRO100State *s)
>          if (s->has_extended_tcb_support && !(s->configuration[6] & BIT(4))) {
>              /* Extended Flexible TCB. */
>              for (; tbd_count < 2; tbd_count++) {
> -                uint32_t tx_buffer_address = ldl_phys(tbd_address);
> -                uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> -                uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> +                uint32_t tx_buffer_address = ldl_le_phys(tbd_address);
> +                uint16_t tx_buffer_size = lduw_le_phys(tbd_address + 4);
> +                uint16_t tx_buffer_el = lduw_le_phys(tbd_address + 6);
>                  tbd_address += 8;
>                  TRACE(RXTX, logout
>                      ("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
> @@ -779,9 +828,9 @@ static void tx_command(EEPRO100State *s)
>          }
>          tbd_address = tbd_array;
>          for (; tbd_count < s->tx.tbd_count; tbd_count++) {
> -            uint32_t tx_buffer_address = ldl_phys(tbd_address);
> -            uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> -            uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
> +            uint32_t tx_buffer_address = ldl_le_phys(tbd_address);
> +            uint16_t tx_buffer_size = lduw_le_phys(tbd_address + 4);
> +            uint16_t tx_buffer_el = lduw_le_phys(tbd_address + 6);
>              tbd_address += 8;
>              TRACE(RXTX, logout
>                  ("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
> @@ -889,7 +938,7 @@ static void action_command(EEPRO100State *s)
>              break;
>          }
>          /* Write new status. */
> -        stw_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
> +        stw_le_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
>          if (bit_i) {
>              /* CU completed action. */
>              eepro100_cx_interrupt(s);
> @@ -1050,8 +1099,7 @@ static void eepro100_write_command(EEPRO100State * s, uint8_t val)
>  
>  static uint16_t eepro100_read_eeprom(EEPRO100State * s)
>  {
> -    uint16_t val;
> -    memcpy(&val, &s->mem[SCBeeprom], sizeof(val));
> +    uint16_t val = e100_read_reg4(s, SCBeeprom);
>      if (eeprom93xx_read(s->eeprom)) {
>          val |= EEPROM_DO;
>      } else {
> @@ -1121,8 +1169,7 @@ static const char *reg2name(uint8_t reg)
>  
>  static uint32_t eepro100_read_mdi(EEPRO100State * s)
>  {
> -    uint32_t val;
> -    memcpy(&val, &s->mem[0x10], sizeof(val));
> +    uint32_t val = e100_read_reg4(s, SCBCtrlMDI);
>  
>  #ifdef DEBUG_EEPRO100
>      uint8_t raiseint = (val & BIT(29)) >> 29;
> @@ -1231,7 +1278,7 @@ static void eepro100_write_mdi(EEPRO100State * s, uint32_t val)
>          }
>      }
>      val = (val & 0xffff0000) + data;
> -    memcpy(&s->mem[0x10], &val, sizeof(val));
> +    e100_write_reg4(s, SCBCtrlMDI, val);
>  }
>  
>  /*****************************************************************************
> @@ -1258,7 +1305,6 @@ static uint32_t eepro100_read_port(EEPRO100State * s)
>  
>  static void eepro100_write_port(EEPRO100State * s, uint32_t val)
>  {
> -    val = le32_to_cpu(val);
>      uint32_t address = (val & ~PORT_SELECTION_MASK);
>      uint8_t selection = (val & PORT_SELECTION_MASK);
>      switch (selection) {
> @@ -1293,7 +1339,7 @@ static uint8_t eepro100_read1(EEPRO100State * s, uint32_t addr)
>  {
>      uint8_t val = 0;
>      if (addr <= sizeof(s->mem) - sizeof(val)) {
> -        memcpy(&val, &s->mem[addr], sizeof(val));
> +        val = s->mem[addr];
>      }
>  
>      switch (addr) {
> @@ -1336,7 +1382,7 @@ static uint16_t eepro100_read2(EEPRO100State * s, uint32_t addr)
>  {
>      uint16_t val = 0;
>      if (addr <= sizeof(s->mem) - sizeof(val)) {
> -        memcpy(&val, &s->mem[addr], sizeof(val));
> +        val = e100_read_reg2(s, addr);
>      }
>  
>      switch (addr) {
> @@ -1359,7 +1405,7 @@ static uint32_t eepro100_read4(EEPRO100State * s, uint32_t addr)
>  {
>      uint32_t val = 0;
>      if (addr <= sizeof(s->mem) - sizeof(val)) {
> -        memcpy(&val, &s->mem[addr], sizeof(val));
> +        val = e100_read_reg4(s, addr);
>      }
>  
>      switch (addr) {
> @@ -1390,7 +1436,7 @@ static void eepro100_write1(EEPRO100State * s, uint32_t addr, uint8_t val)
>  {
>      /* SCBStatus is readonly. */
>      if (addr > SCBStatus && addr <= sizeof(s->mem) - sizeof(val)) {
> -        memcpy(&s->mem[addr], &val, sizeof(val));
> +        s->mem[addr] = val;
>      }
>  
>      switch (addr) {
> @@ -1433,7 +1479,7 @@ static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
>  {
>      /* SCBStatus is readonly. */
>      if (addr > SCBStatus && addr <= sizeof(s->mem) - sizeof(val)) {
> -        memcpy(&s->mem[addr], &val, sizeof(val));
> +        e100_write_reg2(s, addr, val);
>      }
>  
>      switch (addr) {
> @@ -1460,7 +1506,7 @@ static void eepro100_write2(EEPRO100State * s, uint32_t addr, uint16_t val)
>  static void eepro100_write4(EEPRO100State * s, uint32_t addr, uint32_t val)
>  {
>      if (addr <= sizeof(s->mem) - sizeof(val)) {
> -        memcpy(&s->mem[addr], &val, sizeof(val));
> +        e100_write_reg4(s, addr, val);
>      }
>  
>      switch (addr) {
> @@ -1753,9 +1799,10 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
>      }
>      TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
>            rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
> -    stw_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, status),
> -             rfd_status);
> -    stw_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, count), size);
> +    stw_le_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, status),
> +                rfd_status);
> +    stw_le_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, count),
> +                size);
>      /* Early receive interrupt not supported. */
>  #if 0
>      eepro100_er_interrupt(s);
> @@ -1884,7 +1931,7 @@ static int e100_nic_init(PCIDevice *pci_dev)
>      /* Handler for memory-mapped I/O */
>      s->mmio_index =
>          cpu_register_io_memory(pci_mmio_read, pci_mmio_write, s,
> -                               DEVICE_NATIVE_ENDIAN);
> +                               DEVICE_LITTLE_ENDIAN);
>  
>      pci_register_bar(&s->dev, 0, PCI_MEM_SIZE,
>                             PCI_BASE_ADDRESS_SPACE_MEMORY |
> -- 
> 1.7.2.5

[Qemu-devel] Re: [PATCH 8/9] eepro100: Pad received short frames

[Stefan Weil]

Am 31.03.2011 23:41, schrieb Michael S. Tsirkin:
> On Thu, Mar 31, 2011 at 10:33:30PM +0200, Stefan Weil wrote:
>> QEMU sends frames smaller than 60 bytes to ethernet nics.
>> This should be fixed in the networking code because normally
>> such frames are rejected by real NICs and their emulations.
>> To avoid this behaviour, other NIC emulations pad received
>> frames. This patch enables this workaround for eepro100, too.
>>
>> All related code is marked with CONFIG_PAD_RECEIVED_FRAMES,
>> so emulation of the correct handling for short frames can
>> be restored as soon as QEMU's networking code is fixed.
>>
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>
> qemu networking core isn't ethernet-specific
> (slirp is). That's why we don't do this in core,
> so the uglification isn't worth it.

Technically it is possible to add the padding needed for ethernet frames
in slirp code (a patch was already sent to qemu-devel) and any other
code location. Ethernet devices could also set a flag about their padding
requirements, so unnecessary padding could be avoided for non-ethernet
devices like virtio.

The last time when padding was discussed here, not everybody was happy
with the current solution: it was simply the solution which required
the least efforts.

I don't think that the current solution will last forever, simply
because it does not allow testing systems which do create short frames
because they want to test the reaction on short frames or for any
other reason.

That's why I want to be able to disable the padding in the device
emulation.


>
>> ---
>> hw/eepro100.c | 25 ++++++++++++++++++++++++-
>> 1 files changed, 24 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/eepro100.c b/hw/eepro100.c
>> index 500a3af..a740d2e 100644
>> --- a/hw/eepro100.c
>> +++ b/hw/eepro100.c
>> @@ -47,6 +47,14 @@
>> #include "eeprom93xx.h"
>> #include "sysemu.h"
>>
>> +/* QEMU sends frames smaller than 60 bytes to ethernet nics.
>> + * This should be fixed in the networking code because normally
>> + * such frames are rejected by real nics and their emulations.
>> + * To avoid this behaviour, other nic emulations pad received
>> + * frames. The following definition enables this workaround for
>> + * eepro100, too. */
>> +#define CONFIG_PAD_RECEIVED_FRAMES
>> +
>> #define KiB 1024
>>
>> /* Debug EEPRO100 card. */
>> @@ -1756,19 +1764,32 @@ static ssize_t nic_receive(VLANClientState 
>> *nc, const uint8_t * buf, size_t size
>> */
>> EEPRO100State *s = DO_UPCAST(NICState, nc, nc)->opaque;
>> uint16_t rfd_status = 0xa000;
>> +#if defined(CONFIG_PAD_RECEIVED_FRAMES)
>> + uint8_t min_buf[60];
>> +#endif
>> static const uint8_t broadcast_macaddr[6] =
>> { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>>
>> +#if defined(CONFIG_PAD_RECEIVED_FRAMES)
>> + /* Pad to minimum Ethernet frame length */
>> + if (size < sizeof(min_buf)) {
>> + memcpy(min_buf, buf, size);
>> + memset(&min_buf[size], 0, sizeof(min_buf) - size);
>> + buf = min_buf;
>> + size = sizeof(min_buf);
>> + }
>> +#endif
>> +
>> if (s->configuration[8] & 0x80) {
>> /* CSMA is disabled. */
>> logout("%p received while CSMA is disabled\n", s);
>> return -1;
>> +#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
>> } else if (size < 64 && (s->configuration[7] & BIT(0))) {
>> /* Short frame and configuration byte 7/0 (discard short receive) set:
>> * Short frame is discarded */
>> logout("%p received short frame (%zu byte)\n", s, size);
>> s->statistics.rx_short_frame_errors++;
>> -#if 0
>> return -1;
>> #endif
>> } else if ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] 
>> & BIT(3))) {
>> @@ -1847,9 +1868,11 @@ static ssize_t nic_receive(VLANClientState 
>> *nc, const uint8_t * buf, size_t size
>> "(%zu bytes); data truncated\n", rfd_size, size);
>> size = rfd_size;
>> }
>> +#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
>> if (size < 64) {
>> rfd_status |= 0x0080;
>> }
>> +#endif
>> TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size 
>> %u\n",
>> rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
>> stw_le_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, status),
>> -- 
>> 1.7.2.5
>

[Qemu-devel] Re: [PATCH 2/9] eepro100: Fix endianness issues

[Stefan Weil]

Am 31.03.2011 23:52, schrieb Michael S. Tsirkin:
> On Thu, Mar 31, 2011 at 10:33:24PM +0200, Stefan Weil wrote:
>> Like other Intel devices, e100 (eepro100) uses little endian byte order.
>>
>> This patch was tested with these combinations:
>>
>> i386 host, i386 + mipsel guests (le-le)
>> mipsel host, i386 guest (le-le)
>> i386 host, mips + ppc guests (le-be)
>> mips host, i386 guest (be-le)
>>
>> mips and mipsel hosts were emulated machines.
>>
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>> ---
>> hw/eepro100.c | 113 
>> ++++++++++++++++++++++++++++++++++++++++-----------------
>> 1 files changed, 80 insertions(+), 33 deletions(-)
>>
>> diff --git a/hw/eepro100.c b/hw/eepro100.c
>> index f89ff17..c789767 100644
>> --- a/hw/eepro100.c
>> +++ b/hw/eepro100.c
>> @@ -20,11 +20,10 @@
>> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> *
>> * Tested features (i82559):
>> - * PXE boot (i386) ok
>> + * PXE boot (i386 guest, i386 / mips / mipsel / ppc host) ok
>> * Linux networking (i386) ok
>> *
>> * Untested:
>> - * non-i386 platforms
>> * Windows networking
>> *
>> * References:
>> @@ -130,7 +129,7 @@ typedef struct {
>>
>> /* Offsets to the various registers.
>> All accesses need not be longword aligned. */
>> -enum speedo_offsets {
>> +typedef enum {
>> SCBStatus = 0, /* Status Word. */
>> SCBAck = 1,
>> SCBCmd = 2, /* Rx/Command Unit command and status. */
>> @@ -145,7 +144,7 @@ enum speedo_offsets {
>> SCBpmdr = 27, /* Power Management Driver. */
>> SCBgctrl = 28, /* General Control. */
>> SCBgstat = 29, /* General Status. */
>> -};
>> +} E100RegisterOffset;
>>
>> /* A speedo3 transmit buffer descriptor with two buffers... */
>> typedef struct {
>> @@ -307,7 +306,32 @@ static const uint16_t eepro100_mdi_mask[] = {
>> 0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
>> };
>>
>> -/* XXX: optimize */
>> +/* Read a 16 bit little endian value from physical memory. */
>> +static uint16_t lduw_le_phys(target_phys_addr_t addr)
>> +{
>> + /* Load 16 bit (little endian) word from emulated hardware. */
>> + uint16_t val;
>> + cpu_physical_memory_read(addr, (uint8_t *)&val, sizeof(val));
>> + return le16_to_cpu(val);
>> +}
>> +
>> +/* Read a 32 bit little endian value from physical memory. */
>> +static uint32_t ldl_le_phys(target_phys_addr_t addr)
>> +{
>> + /* Load 32 bit (little endian) word from emulated hardware. */
>> + uint32_t val;
>> + cpu_physical_memory_read(addr, (uint8_t *)&val, sizeof(val));
>> + return le32_to_cpu(val);
>> +}
>> +
>> +/* Write a 16 bit little endian value to physical memory. */
>> +static void stw_le_phys(target_phys_addr_t addr, uint16_t val)
>> +{
>> + val = cpu_to_le16(val);
>> + cpu_physical_memory_write(addr, (const uint8_t *)&val, sizeof(val));
>> +}
>> +
>> +/* Write a 32 bit little endian value to physical memory. */
>
> So why not opencode e.g.
> le32_to_cpu(ldl_phys(addr))
>
> wrappers really worth it? What do I miss?
>
> If you insist on these online wrappers, pls prefix
> them with eepro100_.
> Also, why not use lduw_phys and friends internally?
> cpu_physical_ is slower ...
>
>> static void stl_le_phys(target_phys_addr_t addr, uint32_t val)
>> {
>> val = cpu_to_le32(val);
>> @@ -339,6 +363,32 @@ static unsigned compute_mcast_idx(const uint8_t 
>> * ep)
>> return (crc & BITS(7, 2)) >> 2;
>> }
>>
>> +/* Read a 16 bit control/status (CSR) register. */
>> +static uint16_t e100_read_reg2(EEPRO100State *s, E100RegisterOffset 
>> addr)
>> +{
>> + return le16_to_cpup((uint16_t *)&s->mem[addr]);
>> +}
>> +
>> +/* Read a 32 bit control/status (CSR) register. */
>> +static uint32_t e100_read_reg4(EEPRO100State *s, E100RegisterOffset 
>> addr)
>> +{
>> + return le32_to_cpup((uint32_t *)&s->mem[addr]);
>> +}
>> +
>> +/* Write a 16 bit control/status (CSR) register. */
>> +static void e100_write_reg2(EEPRO100State *s, E100RegisterOffset addr,
>> + uint16_t val)
>> +{
>> + cpu_to_le16w((uint16_t *)&s->mem[addr], val);
>> +}
>> +
>> +/* Read a 32 bit control/status (CSR) register. */
>> +static void e100_write_reg4(EEPRO100State *s, E100RegisterOffset addr,
>> + uint32_t val)
>> +{
>> + cpu_to_le32w((uint32_t *)&s->mem[addr], val);
>> +}
>> +
>
> Note that cpu_to_le32w requires an aligned address, unlike
> memcpy, and there's no guarantee
> addr is aligned apparently?
>
> If true you need to memcpy to a 32 bit variable, then
> cpu_to_le32w ther result.
>

[snip]

Thank you for your review, especially for the hints at lduw_phys
and potential alignment issues. I'll apply them to a new version
of this patch.

There was already a function without prefix (stl_le_phys),
and the new ones belong to the same "family". There is nothing
e100 specific in them, so they might be added to qemu-common.h
as well. That was (and is) the reason why I did not add a prefix.

[Qemu-devel] Re: [PATCH 2/9] eepro100: Fix endianness issues

[Michael S. Tsirkin]

On Fri, Apr 01, 2011 at 07:52:52PM +0200, Stefan Weil wrote:
> [snip]
> 
> Thank you for your review, especially for the hints at lduw_phys
> and potential alignment issues. I'll apply them to a new version
> of this patch.
> 
> There was already a function without prefix (stl_le_phys),
> and the new ones belong to the same "family". There is nothing
> e100 specific in them, so they might be added to qemu-common.h
> as well. That was (and is) the reason why I did not add a prefix.

Yes but they don't seem to do much useful,
IMO open-coding will be clearer, they are unlikely
to be generally useful. But if you like to
keep them pls add a eepro100 prefix.

-- 
MST