Re: [Qemu-devel] PowerPC 4xx EMAC emulation

[Hollis Blanchard <hollisb@us.ibm.com>]

On Fri, 2008-11-21 at 01:53 +0000, Salvatore Lionetti wrote:

> Index: hw/emac.c
> ===================================================================
> --- hw/emac.c	(revision 0)
> +++ hw/emac.c	(revision 0)

You should name this ppc4xx_emac.c to avoid confusion.

> @@ -0,0 +1,797 @@
> +/*
> + * QEMU EMAC emulation
> + *
> + * Copyright (c) 2008, Lionetti Salvatore salvatorelionetti@yahoo.it
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +/* This module provide an emulation for ethernet onchip controller on ppc4xx cpu.
> + * Tested on
> + * - emulator of a NSN proprietary board, for 4G
> + * - walnut board/u-boot-1.1.6 (whith some patch)
> + *
> + * TODO:
> + * - >1 {tx, rx} channel 
> + * - can_receive() and buffer_full() too simple.
> + */
> +#include "hw.h"
> +#include "ppc.h"
> +#include "ppc405.h"
> +#include "pc.h"
> +#include "qemu-timer.h"
> +#include "sysemu.h"
> +#include "net.h"
> +
> +/*#define DEBUG_EMAC*/
> +#ifdef DEBUG_EMAC
> +#define DPRINT(fmt, args...)                           \
> +    do { printf("%" PRIu64 " %s: " fmt , qemu_get_clock(rt_clock), __func__, ##args); } while (0)
> +#else
> +#define DPRINT(fmt, args...)
> +#endif 
> +
> +/* 
> + * Redefined because if !exist, linker gives a warning but produce qemu,
> + * but on execution this call cause problem
> + * (could be cygwin & mingw cohesistence?)
> + */
> +#if (defined(WORDS_BIGENDIAN) && defined(TARGET_WORDS_BIGENDIAN)) || ((!defined(WORDS_BIGENDIAN) && !defined(TARGET_WORDS_BIGENDIAN)))
> +#if 0
> +static unsigned long int htonl(unsigned long int hostlong);
> +static unsigned short int htons(unsigned short int hostshort);
> +#endif
> +static unsigned long int ntohl(unsigned long int ing) {
> +	return ing;
> +}
> +static unsigned short int ntohs(unsigned short int ing) {
> +	return ing;
> +}
> +#else
> +static unsigned long int ntohl(unsigned long int ing) {
> +	unsigned char tmp;
> +	unsigned char* tmpp = (unsigned char*)&ing;
> +	tmp=tmpp[0]; tmpp[0]=tmpp[3]; tmpp[3]=tmp;
> +	tmp=tmpp[1]; tmpp[1]=tmpp[2]; tmpp[2]=tmp;
> +	return *(unsigned long int*)tmpp;
> +}
> +static unsigned short int ntohs(unsigned short int ing) {
> +	return ((ing&0xFF)<<8) | ((ing&0xFF00)>>8);
> +}
> +#endif

That comment makes me very nervous. At any rate, you don't need to
invent your own byteswapping macros here; use the standard qemu ones.

> +/* qemu initialization:
> + *
> + * parse command line:	call net_client_init() for each -net, either for {if, collision domain} options.
> + * 			==========					================
> + * 			IF MAC=...	<==== Vlan{Id,Ptr} ====>	COLLISION DOMAIN (mean N-1 other IF MAC=...)
> + * 			========== 					================
> + * 				-net nic[,...]:	fill field onto nd_table[] {MAC(cl|auto), model, vlanPtr=f(vlanId)}
> + * 						vlanPtr=f(vlanId)->guest++
> + * 				-net tap[,...]: vlanPtr=f(vlanId)->host ++
> + * 						tap_win32_init(vlanPtr, "ifname=tap..."):
> + * 							qemu_new_vlan_client(vlanPtr,tap_receive, tap* opaque)		RECV
> + * 							qemu_add_wait_object(tap_sem, tap_w32_send, tap* opaque)	SEND
> + * 			
> + * 			For each vlan created,
> + * 				exit if no GUEST & some HOST
> + * 				warn if some GUEST & no HOST
> + * 											  
> + * start machine:	Es ppc (also pc use n2000) call machine->init():
> + * 			isa_ne2000_init(base, irqno, &nd_table[i])
> + * 				register_ioport_write(base + ..., ne2000_asic_ioport_write, opaque NE2000State*);	SEND
> + * 				qemu_new_vlan_client(nd->vlan, ne2000_receive, ne2000_can_receive, opaque NE2000State*) RECV
> + *
> + * Done!!!

This comment is way over 80 chars wide, and should be removed anyways.
This is not the appropriate place to document qemu's initialization
sequence. I also don't understand why you're talking about NE2000 in
emac.c.

> + * qemu runtime scenario:
> + *
> + * packet send from HOST:	a signal (after exec() cycle) is sent to tap_sem => tap_w32_send() => tap_win32_read()
> + * 				if some bytes returned => qemu_send_packet() => send to all 'client' (better peer) of
> + * 				such vlan, different from itself => ne2000_receive()
> + *
> + * packet send from GUEST:	in machine code we call qemu_send_packet() => (prec) => tap_receive() => tap_w32_write()
> + *
> + *
> + * So onto VLAN actors always call qemu_send_packet() that dispatch packet to all peer attached to same vlan.
> + *
> + */
> +/* PPC405 layer 2, reversed MII mode:
> + *
> + * Register@0xEF600000 already mapped.
> + * Configuration RW:
> + *
> + * Read(ind, data):
> + * 	[EMAC0_STACR] <= (STACR_STAC_R | ind)
> + *	[EMAC0_STACR] & STACR_OC must goes to 0
> + *	[EMAC0_STACR] => >>16 => data
> + *
> + * Write(ind, data)
> + * 	[EMAC0_STACR] <= (STACR_STAC_W | ind | data<<16)
> + * 	[EMAC0_STACR] & STACR_OC must goes to 0
> + *
> + * Physical access:
> + * Read(ind_phy, ind_reg, data)
> + * 	[EMAC0_STACR] <= (STACR_STAC_W | TANTOS_REG_MIIAC | (ind_reg&0x1F | OP_READ | (ind_phy&0x1F)<<5))>>16
> + * 	[EMAC0_STACR] & STACR_OC must goes to 0
> + *	
> + *	Verify:
> + *	[EMAC0_STACR] <= (STACR_STAC_R | TANTOS_REG_MIIRD)
> + * 	[EMAC0_STACR] & STACR_OC must goes to 0
> + * 	[EMAC0_STACR] => data
> + * 	data & STACR_PHYE should be 0
> + * 	data=data>>16
> + *
> + * Tantos Stub Layer: (needed?)
> + *
> + * From mal we need
> + * - MAL0_TXCTPxR
> + * - MAL0_RXCTPxR
> + * - MAL0_RCBS0
> + */
> +/* TODO: need interaction between device!
> + * very difficult since all struct is in .c file */

Refactor it then.

> +extern uint32_t txctpr[4];
> +extern uint32_t rxctpr[2];
> +extern uint32_t rcbs[2];
> +extern uint32_t *rxeobisr; /* With namespace, use & to automatize correlation.*/
> +extern uint32_t *txeobisr;

These should not be not global. Pass a pointer to ppc40x_mal_t into the
EMAC init code and access them that way.

> +// For PPC405
> +#define EMAC0_BASE        0xEF600800
> +#define EMAC0_STACR_DISP  0x35
> +#define EMAC0_STACR      (EMAC0_BASE + EMAC0_STACR_DISP) /* 4 byte addressing */
> +#define STACR_OC          0x00008000 /* Occupied flag */
> +#define STACR_PHYE        0x00004000
> +#define STACR_STAC_W      0x00002000
> +#define STACR_STAC_R      0x00001000
> +
> +#define TANTOS_REG_MIIAC    0x120      /// TANTOS3G MII indirect acess registers
> +#define TANTOS_REG_MIIWD    0x121
> +#define TANTOS_REG_MIIRD    0x122
> +#define OP_WRITE	0x0400
> +#define OP_READ		0x0800
> +#define MBUSY 		0x8000

Probably all these Tantos references should be removed; they're not used
in the code.

> +/* WORDS_BIGENDIAN = FALSE su HOST i386, GUEST ppc
> + * per cui e' l'architetura HOST
> + */

Some Italian was left here.

> +struct EmacMalRxDes {
> +#ifndef WORDS_BIGENDIAN
> +    union {
> +	    struct { /* Mal */
> +		    unsigned int emac1_nu	: 2;
> +		    unsigned int bit5_intr	: 1;
> +		    unsigned int bit4_first	: 1;
> +		    unsigned int bit3_last	: 1;
> +		    unsigned int bit2_contm	: 1;
> +		    unsigned int bit1_wrap	: 1;
> +		    unsigned int bit0_empty	: 1;
> +		    unsigned int emac2_nu	: 8; 
> +	    } __attribute__ ((packed)) mal;
> +	    struct { /* Status: Read access, error cause */
> +		    unsigned int bit7_pausepacket	: 1;
> +		    unsigned int bit6_overrun		: 1;
> +		    unsigned int mal_nu			: 6;
> +		    unsigned int bit15_inrange		: 1;
> +		    unsigned int bit14_outrange		: 1;
> +		    unsigned int bit13_longpacket	: 1;
> +		    unsigned int bit12_fcs		: 1;
> +		    unsigned int bit11_alignment	: 1;
> +		    unsigned int bit10_shortevent	: 1;
> +		    unsigned int bit9_runtpacket	: 1;
> +		    unsigned int bit8_badpacket		: 1;
> +	    } __attribute__ ((packed)) emac_errstatus;
> +    } __attribute__ ((packed));
> +#else
> +#error "Host endianism BUG: To Test!!!"
> +#endif
> +    unsigned short len;
> +    unsigned char* buf;
> +} __attribute__ ((packed)); /* Host endianism, to be converted */

I worry a lot about this endianness handling. You should probably
byteswap whole words *before* storing into this structure, so that you
don't need to define two structures with different layouts.

> +struct EmacMalTxDes {
> +#ifndef WORDS_BIGENDIAN
> +    /* IN A HALF-WORD (16bits)
> +     * position		position
> +     * in bitfield	in value
> +     * from high	from MSb
> +     * =========================
> +     * 0		8
> +     * 1		9
> +     * 2		10
> +     * 3		11
> +     * 4		12
> +     * 5		13
> +     * 6		14
> +     * 7		15
> +     * 8		0
> +     * 9		1
> +     * 10		2
> +     * 11		3
> +     * 12		4
> +     * 13		5
> +     * 14		6
> +     * 15		7
> +     *
> +     * ===HOST LE, TARGET BE===
> +     * IN A BYTE (8bits)
> +     *
> +     * IN A HALF-WORD (16bits)
> +     * 0-7		8-15
> +     * 8-15		0-7
> +     */
> +    union {
> +	    struct { /* Mal */
> +		    unsigned int emac1_nu	: 2;
> +		    unsigned int bit5_intr	: 1;
> +		    unsigned int bit4_resv	: 1;
> +		    unsigned int bit3_last	: 1;
> +		    unsigned int bit2_contm	: 1;
> +		    unsigned int bit1_wrap	: 1;
> +		    unsigned int bit0_ready	: 1;
> +		    unsigned int emac2_nu	: 8; 
> +	    } __attribute__ ((packed)) mal;
> +	    struct { /* Status: Read access, error cause */
> +		    unsigned int bit7_badpacket		: 1;
> +		    unsigned int bit6_badfsc		: 1;
> +		    unsigned int mal_nu			: 6;
> +		    unsigned int bit15_sqe		: 1;
> +		    unsigned int bit14_underrun		: 1;
> +		    unsigned int bit13_singlecoll	: 1;
> +		    unsigned int bit12_multiplecoll	: 1;
> +		    unsigned int bit11_latecoll		: 1;
> +		    unsigned int bit10_excessivecoll	: 1;
> +		    unsigned int bit9_excessivedeferral : 1;
> +		    unsigned int bit8_lossofcarrier	: 1;
> +	    } __attribute__ ((packed)) emac_errorstatus;
> +	    struct { /* Control: Write access */
> +		    unsigned int bit7_generatepad	: 1;
> +		    unsigned int bit6_generatefcs	: 1;
> +		    unsigned int mal_nu			: 6;
> +		    unsigned int emac_nu		: 4;
> +		    unsigned int bit11_vlantag_replace	: 1;
> +		    unsigned int bit10_vlantag_insert	: 1;
> +		    unsigned int bit9_sourceaddr_insert	: 1;
> +		    unsigned int bit8_sourceaddr_replace: 1;
> +	    } __attribute__ ((packed)) emac_control;
> +    };
> +#else
> +#error "Host endianism BUG: To Test!!!"
> +#endif
> +    unsigned short len;
> +    unsigned char* buf;
> +}__attribute__ ((packed)); /* Host endianism, to be converted */
> +
> +/* Controlo Register definition */
> +struct EmacRegs {
> +#if 0
> +#ifndef WORDS_BIGENDIAN
> +	struct Emac0_mr0 {
> +		unsigned int ;
> +	} __attribute__((packed)); 
> +#else
> +#error "Host endianism BUG: To Test!!!"
> +#endif
> +#else
> +	/* 0 */
> +	uint32_t mr0;
> +	uint32_t mr1;
> +	uint32_t tmr0;
> +	uint32_t tmr1;
> +	uint32_t rmr;
> +	uint32_t isr;
> +	uint32_t isre;
> +	uint32_t iahr;
> +	uint32_t ialr;
> +	uint32_t vtpid;
> +	/* 10 */
> +	uint32_t vtci;
> +	uint32_t ptr;
> +	uint32_t iaht1;
> +	uint32_t iaht2;
> +	uint32_t iaht3;
> +	uint32_t iaht4;
> +	uint32_t gaht1;
> +	uint32_t gaht2;
> +	uint32_t gaht3;
> +	uint32_t gaht4;
> +	/* 20 */
> +	uint32_t lsah;
> +	uint32_t lsal;
> +	uint32_t ipgvr;
> +	uint32_t stacr;
> +	uint32_t trtr;
> +	uint32_t rwmr;
> +	uint32_t octx;
> +	uint32_t ocrx;
> +	/* 28 reg */
> +
> +#define RSTA  mr0 & 0x20000000 
> +#define TXEN  mr0 & 0x10000000 
> +#define RXEN  mr0 & 0x08000000 
> +#endif	
> +} __attribute__ ((packed));
> +
> +typedef struct ppc4xx_emac_t ppc4xx_emac_t;
> +struct ppc4xx_emac_t {
> +    /* Guest related */
> +    target_phys_addr_t base;
> +    NICInfo nic;
> +    /* To remove, already defined in mal */
> +    qemu_irq mal_irqs_txeob;
> +    qemu_irq mal_irqs_rxeob;
> +    qemu_irq mal_irqs_txerr;
> +    qemu_irq mal_irqs_rxerr;
> +    qemu_irq mal_irqs_syserr;
> +
> +    struct EmacRegs reg, _reg;
> +
> +    struct EmacMalTxDes* tx;
> +    struct EmacMalRxDes* rx;
> +
> +    int txPos, rxPos;
> +    int txNum, rxNum;
> +
> +    int rxLostNum, rxLostLimit;
> +
> +    /* Host related */
> +    VLANClientState *vc;
> +    QEMUTimer* tx_timer;
> +};
> +
> +#if 0
> +#define EMACMAL_EMPTY 0x8000
> +#define EMACMAL_WRAP  0x8000
> +#define EMACMAL_EMPTY 0x8000
> +#define EMACMAL_EMPTY 0x8000
> +void EmacMalDes_readFromTarget(struct EmacMalDes* host, struct EmacMalDes* guest) {
> +
> +    host->status = ntohs(guest->status);
> +    host->len = ntohs(guest->len);
> +    host->buf = htol(guest->buf);
> +}
> +void EmacMalDes_writeToTarget(struct EmacMalDes* host, struct EmacMalDes* guest) {
> +}
> +#endif

Remove #if 0 code.

> +void EmacMalRxDes_dump(struct EmacMalRxDes* des) {
> +#ifdef DEBUG_EMAC
> +	unsigned char* tdes = (unsigned char*) ((unsigned char*)des - phys_ram_base);
> +	printf("RXdes@%p={len%d,buf%p,empty%d wrap%d contm %d last%d first%d intr%d\n",
> +			tdes, ntohs(des->len), (unsigned char*) ntohl((unsigned long int)des->buf),
> +			des->mal.bit0_empty, des->mal.bit1_wrap, des->mal.bit2_contm, des->mal.bit3_last, des->mal.bit4_first, des->mal.bit5_intr);
> +#endif
> +}
> +void EmacMalTxDes_dump(struct EmacMalTxDes* des) {
> +#ifdef DEBUG_EMAC
> +	unsigned char* tdes = (unsigned char*)((unsigned char*)des - phys_ram_base);
> +	printf("TXdes@%p={len%d,buf%p,ready%d wrap%d contm %d last%d first%d intr%d\n",
> +			tdes, ntohs(des->len), (unsigned char*)ntohl((unsigned long int)des->buf),
> +			des->mal.bit0_ready, des->mal.bit1_wrap, des->mal.bit2_contm, des->mal.bit3_last, des->mal.bit4_resv, des->mal.bit5_intr);
> +#endif
> +}
> +#if 0
> +static int emac_buffer_full(ppc4xx_emac_t *s)
> +{
> +#if 0
> +    int avail, index, boundary;
> +
> +    index = s->curpag << 8;
> +    boundary = s->boundary << 8;
> +    if (index < boundary)
> +        avail = boundary - index;
> +    else
> +        avail = (s->stop - s->start) - (index - boundary);
> +    if (avail < (MAX_ETH_FRAME_SIZE + 4))
> +        return 1;
> +#endif
> +    return 0;
> +}
> +
> +static int emac_can_receive(void *opaque)
> +{
> +#if 0
> +    NE2000State *s = opaque;
> +
> +    if (s->cmd & E8390_STOP)
> +        return 1;
> +    return !ne2000_buffer_full(s);
> +#endif
> +    return 0;
> +}
> +#endif

Remove lots more #if 0 code.

> +static void emac_receive(void *opaque, const uint8_t *buf, int size)
> +{
> +    ppc4xx_emac_t* emac = (ppc4xx_emac_t*) opaque;
> +
> +    DPRINT("%d bytes\n", size);
> +    if (emac->reg.RXEN) {
> +    if (emac->rx->buf) {
> +    if (size<1520) {
> +	    if (emac->rx[emac->rxPos].mal.bit0_empty) {
> +	    /* data is allocated with the maximum possible length, on eth = 1520Bytes
> +	     * len field represent packet size.*/
> +		    /* We !use Continuos mode, first & last field ignored */
> +		    unsigned char* dstG = (unsigned char*) ntohl((unsigned long int)emac->rx[emac->rxPos].buf);
> +		    unsigned char* dstH= dstG+(unsigned int)phys_ram_base;
> +		    unsigned short size16 = size;
> +		    DPRINT(" Delivery message HOST %p-> GUEST %p,pos%d\n", dstH, dstG, emac->rxPos);
> +		    EmacMalRxDes_dump(&emac->rx[emac->rxPos]);
> +		    emac->reg.ocrx += size16;
> +		    memcpy(dstH, buf, size);	
> +		    emac->rx[emac->rxPos].len = ntohs(size16);
> +		    emac->rx[emac->rxPos].mal.bit0_empty = 0;
> +		    if (emac->rx[emac->rxPos].mal.bit5_intr) {
> +			    /* TODO: use appropriate channel */
> +			    *rxeobisr = 0xC0000000;
> +			    qemu_irq_raise(emac->mal_irqs_rxeob);
> +		    }
> +
> +		    emac->rxPos += (emac->rx[emac->rxPos].mal.bit1_wrap)? -(emac->rxPos) : 1;
> +	    } else {
> +		    if (++emac->rxLostNum == emac->rxLostLimit) {
> +			    printf(" %d Message lost, board seem hung up!\n", emac->rxLostNum);
> +			    emac->rxLostLimit*=10;
> +		    }
> +	    }
> +    } else {
> +	    printf(" Message size > 1520, discarding packet\n");
> +    }

The indentation here looks pretty messed up. Also, what is the 1520 byte
limitation? From a quick skim, I don't see mention of that in the user
manual.

> +    } else {
> +    }
> +    } else {
> +	    static int sndOrMore=0;
> +	    if (!sndOrMore) {
> +		    printf("emac/mal: Driver !command start yet\n");
> +		    sndOrMore=1;
> +	    }
> +    }
> +#if 0
> +#define MIN_BUF_SIZE 60
> +    NE2000State *s = opaque;
> +    uint8_t *p;
> +    unsigned int total_len, next, avail, len, index, mcast_idx;
> +    uint8_t buf1[60];
> +    static const uint8_t broadcast_macaddr[6] =
> +        { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
> +
> +#if defined(DEBUG_NE2000)
> +    printf("NE2000: received len=%d\n", size);
> +#endif
> +
> +    if (s->cmd & E8390_STOP || ne2000_bufstruct mal_emac_bdfer_full(s))
> +        return;
> +
> +    /* XXX: check this */
> +    if (s->rxcr & 0x10) {
> +        /* promiscuous: receive all */
> +    } else {
> +        if (!memcmp(buf,  broadcast_macaddr, 6)) {
> +            /* broadcast address */
> +            if (!(s->rxcr & 0x04))
> +                return;
> +        } else if (buf[0] & 0x01) {
> +            /* multicast */
> +            if (!(s->rxcr & 0x08))
> +                return;
> +            mcast_idx = compute_mcast_idx(buf);
> +            if (!(s->mult[mcast_idx >> 3] & (1 << (mcast_idx & 7))))
> +                return;
> +        } else if (s->mem[0] == buf[0] &&
> +                   s->mem[2] == buf[1] &&
> +                   s->mem[4] == buf[2] &&
> +                   s->mem[6] == buf[3] &&
> +                   s->mem[8] == buf[4] &&
> +                   s->mem[10] == buf[5]) {
> +            /* match */
> +        } else {
> +            return;
> +        }
> +    }
> +
> +
> +    /* if too small buffer, then expand it */
> +    if (size < MIN_BUF_SIZE) {
> +        memcpy(buf1, buf, size);
> +        memset(buf1 + size, 0, MIN_BUF_SIZE - size);
> +        buf = buf1;
> +        size = MIN_BUF_SIZE;
> +    }
> +
> +    index = s->curpag << 8;
> +    /* 4 bytes for header */
> +    total_len = size + 4;
> +    /* address for next packet (4 bytes for CRC) */
> +    next = index + ((total_len + 4 + 255) & ~0xff);
> +    if (next >= s->stop)
> +        next -= (s->stop - s->start);
> +    /* prepare packet header */
> +    p = s->mem + index;
> +    s->rsr = ENRSR_RXOK; /* receive status */
> +    /* XXX: check this */
> +    if (buf[0] & 0x01)
> +        s->rsr |= ENRSR_PHY;
> +    p[0] = s->rsr;
> +    p[1] = next >> 8;
> +    p[2] = total_len;
> +    p[3] = total_len >> 8;
> +    index += 4;
> +
> +    /* write packet data */
> +    while (size > 0) {
> +        if (index <= s->stop)
> +            avail = s->stop - index;
> +        else
> +            avail = 0;
> +        len = size;
> +        if (len > avail)
> +            len = avail;
> +        memcpy(s->mem + index, buf, len);
> +        buf += len;
> +        index += len;
> +        if (index == s->stop)
> +            index = s->start;
> +        size -= len;
> +    }
> +    s->curpag = next >> 8;
> +
> +    /* now we can signal we have received something */
> +    s->isr |= ENISR_RX;
> +    ne2000_update_irq(s);
> +#endif

All that #if 0 code looks like it was copied directly from ne2000
emulation. Get rid of it.

> +}
> +
> +void emac_ppc405_init(ppc4xx_emac_t* emac, NICInfo *nd)
> +{
> +    memcpy(&emac->nic, nd, sizeof(NICInfo));
> +    /* We are only be able to receive. Sending packet mean receiver action get up */

I don't understand this comment at all.

Is this function really specific to ppc405, or could it also apply to
440 SoCs with EMAC?

> +    emac->vc = qemu_new_vlan_client(nd->vlan, emac_receive,
> +                                 NULL/*emac_can_receive*/, emac);
> +
> +    snprintf(emac->vc->info_str, sizeof(emac->vc->info_str),
> +             "emac macaddr=%02x:%02x:%02x:%02x:%02x:%02x",
> +             emac->nic.macaddr[0],
> +             emac->nic.macaddr[1],
> +             emac->nic.macaddr[2],
> +             emac->nic.macaddr[3],
> +             emac->nic.macaddr[4],
> +             emac->nic.macaddr[5]);
> +    printf("%s\n", emac->vc->info_str);
> +}
> +
> +static void ppc4xx_emac_reset (void *opaque)
> +{
> +    ppc4xx_emac_t *emac;
> +
> +    emac = opaque;
> +    emac->rx = NULL;
> +    emac->tx = NULL;
> +    emac->txPos = emac->rxPos = 0;
> +    emac->txNum = emac->rxNum = 0;
> +    emac->rxLostNum=0;
> +    emac->rxLostLimit=1;
> +
> +    memset(&emac->reg, 0, sizeof(emac->reg));
> +    emac->reg.mr0   = 0xC0000000;
> +    emac->reg.tmr1  = 0x380F0000;
> +    emac->reg.vtpid = 0x00008808;
> +    emac->reg.ptr   = 0x0000FFFF;
> +    emac->reg.ipgvr = 0x00000004;
> +    emac->reg.stacr = 0x00008000;
> +    emac->reg.rwmr  = 0x04001000;
> +
> +    /* Now ready to issue mii control transfer */
> +    emac->reg.stacr = 0;
> +}
> +/* It looks like a lot of Linux programs assume page size
> + * is 4kB long. This is evil, but we have to deal with it...
> + *
> +#define TARGET_PAGE_BITS 12

TARGET_PAGE_BITS has no place here at all, and you never even use it
anyways.

> + * Here we have many register.
> + * Do action only for reset or transmit command.
> + * Other will be simple configuration latched during device emulation
> + * es stop rx, allow pause packet.
> + *
> + * Another tip:
> + * When possible only implemente the read of value, since using var read/write is target indep
> + */
> +static uint32_t emac_readl (void *opaque, target_phys_addr_t addr)
> +{
> +	uint32_t value;
> +	ppc4xx_emac_t* emac = (ppc4xx_emac_t*)opaque;
> +	uint32_t ind = (addr-emac->base)>>2;
> +	/* Keep in a separate way, as stackable code */
> +	if (ind>=28) {
> +		printf("emac: Out of range register %d!\n", ind);
> +		return 0;
> +	}
> +	value = ((uint32_t*)&emac->reg)[ind];
> +	DPRINT("emac: [%d] => %x\n", ind, value);
> +	return value/*STACR_OC*/;
> +}
> +
> +static void emac_writel (void *opaque, target_phys_addr_t addr, uint32_t value)
> +{
> +	ppc4xx_emac_t* emac = (ppc4xx_emac_t*)opaque;
> +	uint32_t ind = (addr-emac->base)>>2;
> +
> +/*	printf("%s: " PADDRX " val %x\n",__func__, addr, value);*/
> +	/* Exist some common code for this? */
> +	if (ind>=28) {
> +		printf("emac: Out of range register %d!\n", ind);
> +		return;
> +	}
> +	switch (ind) {
> +		case 20:
> +		case 21:
> +		case 26:
> +		case 27:
> +			printf("emac: Attemping to write to read only reg!");
> +			return;
> +	}
> +	DPRINT("emac: [%d] <= %x\n", ind, value);
> +	/* Keep in a separate way, as stackable code */
> +	((uint32_t*)&emac->reg)[ind] = value;
> +	if (ind==0) {
> +		emac->_reg.mr0 = value;
> +		/* soft reset */
> +		if (emac->_reg.RSTA) {
> +			printf("emac: soft reset commanded!\n");
> +			ppc4xx_emac_reset(emac);/* Should be protected with a semaphore, also in emac_receive()? */
> +			value &= ~(emac->_reg.RSTA);	/* Sw look for this bit go down */
> +		}
> +
> +		DPRINT("emac: St%s tx channel!\n", emac->_reg.TXEN?"arting":"opping");
> +		DPRINT("emac: St%s rx channel!\n", emac->_reg.RXEN?"arting":"opping");
> +
> +		/* Suppose the mal reg was updated, seem a good approximation */
> +		if (emac->_reg.TXEN && emac->tx==NULL) {
> +			int l=0;
> +			emac->tx = (struct EmacMalTxDes*) (txctpr[0] + phys_ram_base);
> +			do { EmacMalTxDes_dump(&emac->tx[l]); } while (!emac->tx[l].mal.bit1_wrap && l++<50);
> +		}
> +		if (emac->_reg.RXEN && emac->rx==NULL) {
> +			int l=0;
> +			emac->rx = (struct EmacMalRxDes*) (rxctpr[0] + phys_ram_base);
> +			do { EmacMalRxDes_dump(&emac->rx[l]); } while (!emac->rx[l].mal.bit1_wrap && l++<50);
> +		}
> +		/* A timer that periodically check packet to send 
> +		emac->tx_timer = qemu_new_timer();*/
> +	}
> +	if (ind==2) {
> +		if (value & 0xC0000000) { /* both channel */
> +			if (emac->reg.TXEN) {
> +			if (emac->tx[emac->txPos].mal.bit0_ready) {
> +				unsigned char* dstG = (unsigned char*) ntohl((unsigned long int)emac->tx[emac->txPos].buf);
> +				unsigned char* dstH= dstG+(unsigned int)phys_ram_base;
> +				unsigned short size16 = ntohs(emac->tx[emac->txPos].len);
> +
> +				DPRINT("Sending packet on pos %d\n", emac->txPos);
> +				qemu_send_packet(emac->vc, dstH, size16);
> +				emac->reg.octx += size16;
> +				if (emac->tx[emac->txPos].mal.bit5_intr) {
> +					/* TODO: use appropriate channel */
> +					*txeobisr = 0xC0000000;
> +					qemu_irq_raise(emac->mal_irqs_txeob);
> +				}
> +
> +				/* Clear status */
> +				emac->tx[emac->txPos].mal.bit0_ready = 0;
> +				/* 0x808 is stucked at STACR_OC */
> +				emac->txPos += (emac->tx[emac->txPos].mal.bit1_wrap)? -(emac->txPos) : 1;
> +
> +				/* 
> +				 * u-boot driver (no os=>no task) poll this bit until became 0,
> +				 * but in PPC405GPr User manual nothing said about this
> +				 */
> +				value &= ~0xC0000000;
> +			} else {printf("Processor goes crazy(ready bit was 0)!!! Stopping tx\n");}
> +			} else {DPRINT("Tx is disabled\n");}
> +		}
> +	}
> +	if (ind==23) {
> +		/* PPC405GPr_UM2004 say, on page 568:
> +		 * 'EMAC sets EMAC0_STACR[OC] = 0 when the EMAC0_STACR is written to.'
> +		 * 'EMAC then sets EMAC0_STACR[OC] = 1 to indicate that the data has been written to the PHY, or the data'
> +		 * 'read from the PHY is valid. The device driver should poll for EMAC0_STACR[OC] = 1 before issuing a new'
> +		 * 'command, or before using data read from the PHY'
> +		 * 
> +		 * our approximation:
> +		 * - initial condition			0
> +		 * - write(sw start some MII command)	1 (we end in the same cycle)
> +		 * - read(poll status)			1
> +		 * - write(sw start new cmd | ...)	0
> +		 *
> +		 * reg write clear (bit index is opposite then those reported by official manual)
> +		 * PHYD	bit 31:16	Data (RW)
> +		 * OC	bit 15		0 when this reg is addressed, 1 data written to PHY/data readed correctly from PHY
> +		 * PHYE	bit 14		1 PhyError during read
> +		 * STAC bit 13:12	00 Reserved/01 Read/10 Write/11 Reserved
> +		 * OPBC	bit 11:10	OPB Bus clock freq. signal EMCMDCIk
> +		 * PCDA	bit 9:5		PHY Command destination address
> +		 * PRA	bit 4:0		PHY Register address
> +		 *
> +		value &= ~(1<<15);*/
> +		value ^=  (1<<15);
> +		value &= ~(1<<14);
> +	}
> +	/* Keep in a separate way, as stackable code */
> +	((uint32_t*)&emac->reg)[ind] = value;
> +}
> +static uint32_t emac_readw (void *opaque, target_phys_addr_t addr)
> +{
> +	printf("Lettura a granularita' word su EMAC " PADDRX "\n",addr);
> +	return 0;
> +}
> +
> +static void emac_writew (void *opaque, target_phys_addr_t addr, uint32_t value)
> +{
> +	printf("Scrittura a granularita' word su EMAC " PADDRX "\n",addr);
> +}
> +
> +static uint32_t emac_readb (void *opaque, target_phys_addr_t addr)
> +{
> +	printf("Lettura a granularita' byte su EMAC " PADDRX "\n",addr);
> +	return 0;
> +}
> +
> +static void emac_writeb (void *opaque, target_phys_addr_t addr, uint32_t value)
> +{
> +	printf("Scrittura a granularita' byte su EMAC " PADDRX "\n",addr);
> +}

A lot of Italian in here. :)

> +
> +static CPUReadMemoryFunc *emac_read[] = {
> +    &emac_readb,
> +    &emac_readw,
> +    &emac_readl,
> +};
> +
> +static CPUWriteMemoryFunc *emac_write[] = {
> +    &emac_writeb,
> +    &emac_writew,
> +    &emac_writel,
> +};
> +void ppc4xx_emac_init (CPUState *env, ppc4xx_mmio_t *mmio, target_phys_addr_t offset, NICInfo *nic, qemu_irq mal_irqs[4])
> +{
> +    ppc4xx_emac_t *emac;
> +
> +    emac = qemu_mallocz(sizeof(ppc4xx_emac_t));
> +    if (emac != NULL) {
> +        emac->base = offset;
> +#ifdef DEBUG_OPBA
> +        printf("%s: offset " PADDRX "\n", __func__, offset);
> +#endif
> +        ppc4xx_mmio_register(env, mmio, offset, 0x70,
> +                             emac_read, emac_write, emac);
> +        qemu_register_reset(ppc4xx_emac_reset, emac);
> +
> +	/* One time configuration, HOST related */
> +	emac_ppc405_init(emac, nic);
> +
> +	emac->mal_irqs_txeob = mal_irqs[0];
> +	emac->mal_irqs_rxeob = mal_irqs[1];
> +	emac->mal_irqs_txerr = mal_irqs[2];
> +	emac->mal_irqs_rxerr = mal_irqs[3];
> +
> +	/* N Time configuration, GUEST related */
> +        ppc4xx_emac_reset(emac);
> +    }
> +}

I'm more interested in your EMAC work, so if you split that into a
separate patch as you revise it, we may be able to get it cleaned up and
usable sooner than later.

-- 
Hollis Blanchard
IBM Linux Technology Center