Subject: Re: CVS commit: src/sys/arch/sgimips
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
From: Jason Thorpe <thorpej@wasabisystems.com>
List: tech-kern
Date: 12/13/2003 16:53:08
--Apple-Mail-21-714191977
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII; format=flowed

Wow, I let this mail sit for too long.

On Oct 8, 2003, at 5:29 AM, Izumi Tsutsui wrote:

> With a quick glance, structure of wm(4) actually smells Intel-ism ;-p
> uint8_t/uint16_t values in 32 bit DMA descriptors always cause
> endian issue (and might cause race conditions on RISC CPUs.)

If you haven't already checked in this patch, please do, as long as you 
can confirm it still works on i386 :-)

> How about the attached patch?
> ---
> Izumi Tsutsui
> tsutsui@ceres.dti.ne.jp
>
> Index: if_wm.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/pci/if_wm.c,v
> retrieving revision 1.41
> diff -u -r1.41 if_wm.c
> --- if_wm.c	10 Sep 2003 04:02:17 -0000	1.41
> +++ if_wm.c	8 Oct 2003 12:25:00 -0000
> @@ -1074,36 +1074,36 @@
>
>  	if (m0->m_pkthdr.csum_flags & M_CSUM_IPv4) {
>  		WM_EVCNT_INCR(&sc->sc_ev_txipsum);
> -		fields |= htole32(WTX_IXSM);
> -		ipcs = htole32(WTX_TCPIP_IPCSS(offset) |
> +		fields |= WTX_IXSM;
> +		ipcs = WTX_TCPIP_IPCSS(offset) |
>  		    WTX_TCPIP_IPCSO(offset + offsetof(struct ip, ip_sum)) |
> -		    WTX_TCPIP_IPCSE(offset + iphl - 1));
> +		    WTX_TCPIP_IPCSE(offset + iphl - 1);
>  	} else if (__predict_true(sc->sc_txctx_ipcs != 0xffffffff)) {
>  		/* Use the cached value. */
>  		ipcs = sc->sc_txctx_ipcs;
>  	} else {
>  		/* Just initialize it to the likely value anyway. */
> -		ipcs = htole32(WTX_TCPIP_IPCSS(offset) |
> +		ipcs = WTX_TCPIP_IPCSS(offset) |
>  		    WTX_TCPIP_IPCSO(offset + offsetof(struct ip, ip_sum)) |
> -		    WTX_TCPIP_IPCSE(offset + iphl - 1));
> +		    WTX_TCPIP_IPCSE(offset + iphl - 1);
>  	}
>
>  	offset += iphl;
>
>  	if (m0->m_pkthdr.csum_flags & (M_CSUM_TCPv4|M_CSUM_UDPv4)) {
>  		WM_EVCNT_INCR(&sc->sc_ev_txtusum);
> -		fields |= htole32(WTX_TXSM);
> -		tucs = htole32(WTX_TCPIP_TUCSS(offset) |
> +		fields |= WTX_TXSM;
> +		tucs = WTX_TCPIP_TUCSS(offset) |
>  		    WTX_TCPIP_TUCSO(offset + m0->m_pkthdr.csum_data) |
> -		    WTX_TCPIP_TUCSE(0) /* rest of packet */);
> +		    WTX_TCPIP_TUCSE(0) /* rest of packet */;
>  	} else if (__predict_true(sc->sc_txctx_tucs != 0xffffffff)) {
>  		/* Use the cached value. */
>  		tucs = sc->sc_txctx_tucs;
>  	} else {
>  		/* Just initialize it to a valid TCP context. */
> -		tucs = htole32(WTX_TCPIP_TUCSS(offset) |
> +		tucs = WTX_TCPIP_TUCSS(offset) |
>  		    WTX_TCPIP_TUCSO(offset + offsetof(struct tcphdr, th_sum)) |
> -		    WTX_TCPIP_TUCSE(0) /* rest of packet */);
> +		    WTX_TCPIP_TUCSE(0) /* rest of packet */;
>  	}
>
>  	if (sc->sc_txctx_ipcs == ipcs &&
> @@ -1121,8 +1121,8 @@
>  #endif
>  		t = (struct livengood_tcpip_ctxdesc *)
>  		    &sc->sc_txdescs[sc->sc_txnext];
> -		t->tcpip_ipcs = ipcs;
> -		t->tcpip_tucs = tucs;
> +		t->tcpip_ipcs = htole32(ipcs);
> +		t->tcpip_tucs = htole32(tucs);
>  		t->tcpip_cmdlen =
>  		    htole32(WTX_CMD_DEXT | WTX_DTYP_C);
>  		t->tcpip_seg = 0;
> @@ -1297,7 +1297,7 @@
>  			cksumfields = 0;
>  		}
>
> -		cksumcmd |= htole32(WTX_CMD_IDE);
> +		cksumcmd |= WTX_CMD_IDE;
>
>  		/*
>  		 * Initialize the transmit descriptor.
> @@ -1312,17 +1312,17 @@
>  			sc->sc_txdescs[nexttx].wtx_addr.wa_high = 0;
>  			sc->sc_txdescs[nexttx].wtx_addr.wa_low =
>  			    htole32(dmamap->dm_segs[seg].ds_addr);
> -			sc->sc_txdescs[nexttx].wtx_cmdlen = cksumcmd |
> -			    htole32(dmamap->dm_segs[seg].ds_len);
> +			sc->sc_txdescs[nexttx].wtx_cmdlen =
> +			    htole32(cksumcmd | dmamap->dm_segs[seg].ds_len);
>  			sc->sc_txdescs[nexttx].wtx_fields.wtxu_bits =
> -			    cksumfields;
> +			    htole32(cksumfields);
>  			lasttx = nexttx;
>
>  			DPRINTF(WM_DEBUG_TX,
>  			    ("%s: TX: desc %d: low 0x%08x, len 0x%04x\n",
>  			    sc->sc_dev.dv_xname, nexttx,
> -			    (uint32_t) dmamap->dm_segs[seg].ds_addr,
> -			    (uint32_t) dmamap->dm_segs[seg].ds_len));
> +			    le32toh(dmamap->dm_segs[seg].ds_addr),
> +			    le32toh(dmamap->dm_segs[seg].ds_len)));
>  		}
>
>  		/*
> Index: if_wmreg.h
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/pci/if_wmreg.h,v
> retrieving revision 1.5
> diff -u -r1.5 if_wmreg.h
> --- if_wmreg.h	30 May 2003 02:08:55 -0000	1.5
> +++ if_wmreg.h	8 Oct 2003 12:25:01 -0000
> @@ -59,12 +59,23 @@
>  typedef struct wiseman_rxdesc {
>  	wiseman_addr_t	wrx_addr;	/* buffer address */
>
> +#if BYTE_ORDER == LITTLE_ENDIGN
>  	uint16_t	wrx_len;	/* buffer length */
>  	uint16_t	wrx_cksum;	/* checksum (starting at PCSS) */
> +#else
> +	uint16_t	wrx_cksum;	/* checksum (starting at PCSS) */
> +	uint16_t	wrx_len;	/* buffer length */
> +#endif
>
> +#if BYTE_ORDER == LITTLE_ENDIGN
>  	uint8_t		wrx_status;	/* Rx status */
>  	uint8_t		wrx_errors;	/* Rx errors */
>  	uint16_t	wrx_special;	/* special field (VLAN, etc.) */
> +#else
> +	uint16_t	wrx_special;	/* special field (VLAN, etc.) */
> +	uint8_t		wrx_errors;	/* Rx errors */
> +	uint8_t		wrx_status;	/* Rx status */
> +#endif
>  } __attribute__((__packed__)) wiseman_rxdesc_t;
>
>  /* wrx_status bits */
> @@ -101,9 +112,15 @@
>  typedef union wiseman_tx_fields {
>  	uint32_t	wtxu_bits;	/* bits; see below; */
>  	struct {
> +#if BYTE_ORDER == LITTLE_ENDIGN
>  		uint8_t wtxu_status;		/* Tx status */
>  		uint8_t wtxu_options;		/* options */
>  		uint16_t wtxu_vlan;		/* VLAN info */
> +#else
> +		uint16_t wtxu_vlan;		/* VLAN info */
> +		uint8_t wtxu_options;		/* options */
> +		uint8_t wtxu_status;		/* Tx status */
> +#endif
>  	} __attribute__((__packed__)) wtxu_fields;
>  } __attribute__((__packed__)) wiseman_txfields_t;
>  typedef struct wiseman_txdesc {
>
         -- Jason R. Thorpe <thorpej@wasabisystems.com>


--Apple-Mail-21-714191977
content-type: application/pgp-signature; x-mac-type=70674453;
	name=PGP.sig
content-description: This is a digitally signed message part
content-disposition: inline; filename=PGP.sig
content-transfer-encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (Darwin)

iD8DBQE/27R1OpVKkaBm8XkRAt+3AKCx0Zp17DtG5q2yY5ft2fe3iqsCxQCgkRcA
S7gX3rZRtPKO3YnhJKLxUpM=
=bqch
-----END PGP SIGNATURE-----

--Apple-Mail-21-714191977--