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

[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