Re: [Qemu-devel] [PATCH v2 05/20] arm: add Faraday FTGMAC100 1Gbps ethernet support

2013-01-27 Thread Kuo-Jung Su
2013/1/26 Paul Brook p...@codesourcery.com

  In order to reduce the processing load of the host CPU, the FTGMAC100
  implements TCP, UDP, and IP V4 checksum generation and validation, and
  supports VLAN tagging.

 I see no evidence of these features in the code.


My bad  and yes, the VLAN and checksum offload are not yet implemented.
I simply fotget to remove these description ... :P
but since the checksum offload feature of the real hardware actually
malfunctioned.
(becuase of the alignment issue).
I'll only implement the VLAN tagging feature in the next version.

 +static void ftgmac100_read_desc(hwaddr addr, void *desc)
  +{
  +int i;
  +uint32_t *p = desc;
  +
  +cpu_physical_memory_read(addr, desc, 16);
  +
  +for (i = 0; i  16; i += 4) {
  +*p = le32_to_cpu(*p);
  +}
  +}

 You're relying on the compiler choosing a particular bitfield and structure
 layout. Don't do that.  Especially when one of the fields is a void*.
  Clearly
 never been tested on a 64-bit host. void *desc is just plain lazy.


yes, it's my bad. It'll be fixed later


  +buf = s-txbuff.buf + s-txbuff.len;
  +cpu_physical_memory_read(txd.buf, (uint8_t *)buf, txd.len);

 Buffer overflow.  In at least two differnt ways.


yes, it's my bad. It'll be fixed later


  +if (!(s-maccr  MACCR_HT_MULTI_EN)) {
  +printf([qemu] ftgmac100_receive: mcst filtered\n);
  +return -1;

 Looks like stray debug code.  Several other occurences.

  +case REG_TXPD:
  +case REG_HPTXPD:
  +qemu_mod_timer(s-qtimer, qemu_get_clock_ns(vm_clock) + 1);

 Using a timer here is wrong.  Either you should transmit immediately, or
 you
 should wait for something else to happen.  Delaying by 1ns is never the
 right
 answer.

 Paul


yes, it's my bad. I'll try to use mutex later

-- 
Best wishes,
Kuo-Jung Su


[Qemu-devel] [PATCH v2 05/20] arm: add Faraday FTGMAC100 1Gbps ethernet support

2013-01-25 Thread Kuo-Jung Su
From: Kuo-Jung Su dant...@faraday-tech.com

The FTGMAC100 is a high-quality Ethernet controller with DMA function.
It includes the AHB wrapper, DMA engine, on-chip memories (TX FIFO
and RX FIFO), MAC, and MII/GMII interfaces.

The FTGMAC100 is an Ethernet controller that provides AHB master capability
and full compliance with IEEE 802.3 specification for 10/100 Mbps Ethernet
and IEEE 802.3z specification for 1000 Mbps Ethernet. FTGMAC100 Ethernet
controller with DMA function handles all data transfers between the system
memory and on-chip memories. With the DMA engine, it can reduce the CPU
loading, maximize the performance and minimize the FIFO size. It also has
on-chip memories for buffering, so that the external local-buffer memory
is not needed. The MII interfaces support two specific data rates, 10 Mbps
and 100 Mbps. The GMII interfaces support 1000 Mbps data rate.

In order to reduce the processing load of the host CPU, the FTGMAC100
implements TCP, UDP, and IP V4 checksum generation and validation, and
supports VLAN tagging. For QoS and CoS requirements, it supports high-priority
queues to reduce the processing load of hosts CPU for transmitting packets.

The FTGMAC100 also provides a Wake-On-LAN function. It supports three
wake-up events: link status change, magic packet, and wake-up frame.

Signed-off-by: Kuo-Jung Su dant...@faraday-tech.com
---
 hw/ftgmac100.c |  647 
 hw/ftgmac100.h |  177 
 2 files changed, 824 insertions(+)
 create mode 100644 hw/ftgmac100.c
 create mode 100644 hw/ftgmac100.h

diff --git a/hw/ftgmac100.c b/hw/ftgmac100.c
new file mode 100644
index 000..48f10bb
--- /dev/null
+++ b/hw/ftgmac100.c
@@ -0,0 +1,647 @@
+/*
+ * QEMU model of the FTGMAC100 Controller
+ *
+ * Copyright (C) 2012 Faraday Technology
+ * Written by Dante Su dant...@faraday-tech.com
+ *
+ * This file is licensed under GNU GPL v2.
+ */
+
+#include sysbus.h
+#include sysemu/sysemu.h
+#include net/net.h
+
+#include faraday.h
+#include ftgmac100.h
+
+#define TYPE_FTGMAC100ftgmac100
+
+typedef struct Ftgmac100State {
+SysBusDevice busdev;
+MemoryRegion mmio;
+
+QEMUTimer *qtimer;
+qemu_irq irq;
+NICState *nic;
+NICConf conf;
+
+uint32_t isr;
+uint32_t ier;
+uint32_t mhash[2];
+uint32_t tx_bar;
+uint32_t rx_bar;
+uint32_t hptx_bar;
+uint32_t tx_idx;
+uint32_t rx_idx;
+uint32_t hptx_idx;
+uint32_t maccr;
+uint32_t macsr;
+uint32_t phycr;
+uint32_t phycr_rd;
+
+struct {
+uint8_t  buf[2048];
+uint32_t len;
+} txbuff;
+
+uint32_t rx_pkt;
+uint32_t rx_bcst;
+uint32_t rx_mcst;
+uint16_t rx_runt;
+uint16_t rx_drop;
+uint16_t rx_crc;
+uint16_t rx_ftl;
+uint32_t tx_pkt;
+
+} ftgmac100_state;
+
+#define FTGMAC100(obj) \
+OBJECT_CHECK(ftgmac100_state, obj, TYPE_FTGMAC100)
+
+static uint8_t bitrev8(uint8_t v)
+{
+int i;
+uint8_t r = 0;
+for (i = 0; i  8; ++i) {
+if (v  (1  i)) {
+r |= (1  (7 - i));
+}
+}
+return r;
+}
+
+static int ftgmac100_mcast_hash(ftgmac100_state *s, const uint8_t *data)
+{
+#define CRCPOLY_BE0x04c11db7
+int i, len;
+uint32_t crc = 0x;
+
+if (s-maccr  MACCR_GMODE) {
+len = 5;
+} else {
+len = 6;
+}
+
+while (len--) {
+uint32_t c = *(data++);
+for (i = 0; i  8; ++i) {
+crc = (crc  1) ^ crc  31) ^ c)  0x01) ? CRCPOLY_BE : 0);
+c = 1;
+}
+}
+crc = ~crc;
+
+/* Reverse CRC32 and return MSB 6 bits only */
+return bitrev8(crc  24)  2;
+}
+
+static void ftgmac100_read_desc(hwaddr addr, void *desc)
+{
+int i;
+uint32_t *p = desc;
+
+cpu_physical_memory_read(addr, desc, 16);
+
+for (i = 0; i  16; i += 4) {
+*p = le32_to_cpu(*p);
+}
+}
+
+static void ftgmac100_write_desc(hwaddr addr, void *desc)
+{
+int i;
+uint32_t *p = desc;
+
+for (i = 0; i  16; i += 4) {
+*p = cpu_to_le32(*p);
+}
+
+cpu_physical_memory_write(addr, desc, 16);
+}
+
+static void ftgmac100_update_irq(ftgmac100_state *s)
+{
+if (s-isr  s-ier) {
+qemu_set_irq(s-irq, 1);
+} else {
+qemu_set_irq(s-irq, 0);
+}
+}
+
+static int ftgmac100_can_receive(NetClientState *nc)
+{
+ftgmac100_state *s = FTGMAC100(DO_UPCAST(NICState, nc, nc)-opaque);
+ftgmac100_rxdesc_t rxd;
+hwaddr off = s-rx_bar + s-rx_idx * sizeof(rxd);
+
+if ((s-maccr  (MACCR_RCV_EN | MACCR_RDMA_EN))
+!= (MACCR_RCV_EN | MACCR_RDMA_EN)) {
+return 0;
+}
+
+ftgmac100_read_desc(off, rxd);
+
+return !rxd.owner;
+}
+
+static ssize_t ftgmac100_receive(NetClientState *nc,
+ const uint8_t  *buf,
+ size_t  size)
+{
+const uint8_t *ptr = buf;
+hwaddr off;
+size_t len;
+ftgmac100_rxdesc_t rxd;
+

Re: [Qemu-devel] [PATCH v2 05/20] arm: add Faraday FTGMAC100 1Gbps ethernet support

2013-01-25 Thread Paul Brook
 In order to reduce the processing load of the host CPU, the FTGMAC100
 implements TCP, UDP, and IP V4 checksum generation and validation, and
 supports VLAN tagging.

I see no evidence of these features in the code.

 +static void ftgmac100_read_desc(hwaddr addr, void *desc)
 +{
 +int i;
 +uint32_t *p = desc;
 +
 +cpu_physical_memory_read(addr, desc, 16);
 +
 +for (i = 0; i  16; i += 4) {
 +*p = le32_to_cpu(*p);
 +}
 +}

You're relying on the compiler choosing a particular bitfield and structure 
layout. Don't do that.  Especially when one of the fields is a void*.  Clearly 
never been tested on a 64-bit host. void *desc is just plain lazy.

 +buf = s-txbuff.buf + s-txbuff.len;
 +cpu_physical_memory_read(txd.buf, (uint8_t *)buf, txd.len);

Buffer overflow.  In at least two differnt ways.

 +if (!(s-maccr  MACCR_HT_MULTI_EN)) {
 +printf([qemu] ftgmac100_receive: mcst filtered\n);
 +return -1;

Looks like stray debug code.  Several other occurences.

 +case REG_TXPD:
 +case REG_HPTXPD:
 +qemu_mod_timer(s-qtimer, qemu_get_clock_ns(vm_clock) + 1);

Using a timer here is wrong.  Either you should transmit immediately, or you 
should wait for something else to happen.  Delaying by 1ns is never the right 
answer.

Paul