[PATCH 0/5] defxx: Fixes for 64-bit host support

[PATCH 0/5] defxx: Fixes for 64-bit host support

[Maciej W. Rozycki]

Hi,

 This mini patch series addresses issues with 64-bit host support for FDDI 
interface boards supported by the defxx driver where DMA mapping 
synchronisation is required on swiotlb systems.  While PDQ, the DMA engine 
chip used with these boards, supports 48-bit addressing that would 
normally suffice for typical 64-bit systems in existence, the host bus 
interface chips used by individual implementations have their limitations 
as follows:

* DEFTA or DEC FDDIcontroller/TURBOchannel -- there's no host bus 
  interface chip, the PDQ connects to TURBOchannel directly; TURBOchannel 
  supports DMA addressing of up to 16GB (34-bit addressing), however no 
  TURBOchannel system has ever been made that supports more than 1GB of 
  RAM, so in reality no remapping is ever required,

* DEFEA or DEC FDDIcontroller/EISA -- the ESIC EISA interface chip only 
  supports 32-bit addressing, all accesses beyond 4GB have to be remapped,

* DEFPA or DEC FDDIcontroller/PCI -- the PFI PCI interface chip rev. 1 & 2 
  only support 32-bit addressing, they have 32 AD lines only both on the 
  PDQ and the PCI side, and consequently no Dual Address Cycle support, so 
  all accesses beyond 4GB have to be remapped; the range of addressing 
  supported by PFI rev. 3 is currently not certain, however the chip is 
  backwards compatible with earlier revisions and will work with code that 
  supports them.

Some other issues discovered in the course of correcting 64-bit support 
have been fixed as well.  Each of the patches is functionally 
self-contained and can be applied independentely, although there may be 
mechanical dependencies making it necessary to apply patches in order.

 The driver suffers from non-standard formatting and while I did my best 
with these bug fixes to follow our coding style, I found some pieces 
hopeless, checkpatch.pl will complain.  I plan to reformat the whole 
driver, that will inevitably require factoring out some pieces into 
separate functions, but that's going to be a major effort and therefore I 
want to do this separately, with no functional changes made at the same 
time.  If anyone has specific suggestions as to how to reformat any of the 
pieces submitted here for a better layout, then I'll be happy to take them 
into account.

 And last but not least many thanks to Robert Coerver, who was the most 
recent person to report this problem with the driver and was kind enough 
to patiently try a few revisions of the driver update on his system as I 
was finding and addressing issues.

  Maciej

[PATCH 1/5] defxx: Correct the receive DMA map size

[Maciej W. Rozycki]

Receive DMA maps are oversized, they include EISA legacy 128-byte
alignment padding in size calculation whereas this padding is never used
for data.  Worse yet, if the skb's data area has been realigned indeed,
then data beyond the end of the buffer will be synchronised from the
receive DMA bounce buffer, possibly corrupting data structures residing
in memory beyond the actual end of this data buffer.

Therefore switch to using PI_RCV_DATA_K_SIZE_MAX rather than NEW_SKB_SIZE
in DMA mapping, the value the former macro expands to is written to the
receive ring DMA descriptor of the PDQ DMA chip and determines the
maximum amount of data PDQ will ever transfer to the corresponding data
buffer, including all headers and padding.

Reported-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Tested-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
 Please apply,

  Maciej

linux-defxx-rx-dma-map-size.patch
Index: linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
===================================================================
--- linux-20140623-swarm64-eb.orig/drivers/net/fddi/defxx.c
+++ linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
@@ -2940,7 +2940,7 @@ static int dfx_rcv_init(DFX_board_t *bp,
 			my_skb_align(newskb, 128);
 			bp->descr_block_virt->rcv_data[i + j].long_1 =
 				(u32)dma_map_single(bp->bus_dev, newskb->data,
-						    NEW_SKB_SIZE,
+						    PI_RCV_DATA_K_SIZE_MAX,
 						    DMA_FROM_DEVICE);
 			/*
 			 * p_rcv_buff_va is only used inside the
@@ -3057,14 +3057,14 @@ static void dfx_rcv_queue_process(
 						skb = (struct sk_buff *)bp->p_rcv_buff_va[entry];
 						dma_unmap_single(bp->bus_dev,
 							bp->descr_block_virt->rcv_data[entry].long_1,
-							NEW_SKB_SIZE,
+							PI_RCV_DATA_K_SIZE_MAX,
 							DMA_FROM_DEVICE);
 						skb_reserve(skb, RCV_BUFF_K_PADDING);
 						bp->p_rcv_buff_va[entry] = (char *)newskb;
 						bp->descr_block_virt->rcv_data[entry].long_1 =
 							(u32)dma_map_single(bp->bus_dev,
 								newskb->data,
-								NEW_SKB_SIZE,
+								PI_RCV_DATA_K_SIZE_MAX,
 								DMA_FROM_DEVICE);
 					} else
 						skb = NULL;

[PATCH 2/5] defxx: Discard DMA maps on buffer deallocation

[Maciej W. Rozycki]

Prearranged receive DMA bounce buffer mappings are not released in the
card reboot/shutdown path.  That does not affect frame reception, but
probably explains the random segmentation fault I observed the other day
on interface shutdown.  Card is rebooted as required by the spec in the
process of ring fault recovery when a PC Trace signal has been received.

This change fixes the problem in an obvious manner.

Reported-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Tested-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
 Please apply,

  Maciej

linux-defxx-rcv-flush-unmap.patch
Index: linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
===================================================================
--- linux-20140623-swarm64-eb.orig/drivers/net/fddi/defxx.c
+++ linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
@@ -3448,8 +3448,13 @@ static void dfx_rcv_flush( DFX_board_t *
 		{
 			struct sk_buff *skb;
 			skb = (struct sk_buff *)bp->p_rcv_buff_va[i+j];
-			if (skb)
+			if (skb) {
+				dma_unmap_single(bp->bus_dev,
+						 bp->descr_block_virt->rcv_data[i+j].long_1,
+						 PI_RCV_DATA_K_SIZE_MAX,
+						 DMA_FROM_DEVICE);
 				dev_kfree_skb(skb);
+			}
 			bp->p_rcv_buff_va[i+j] = NULL;
 		}
 

[PATCH 3/5] defxx: Use netdev_alloc_skb consistently

[Maciej W. Rozycki]

Switch the two remaining places across the driver that use dev_alloc_skb 
to netdev_alloc_skb.  Another place has already been converted to use 
__netdev_alloc_skb, no idea why these two have been left behind.

Reported-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Tested-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
 Please apply,

  Maciej

linux-defxx-netdev-alloc-skb.patch
Index: linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
===================================================================
--- linux-20140623-swarm64-eb.orig/drivers/net/fddi/defxx.c
+++ linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
@@ -3049,7 +3049,8 @@ static void dfx_rcv_queue_process(
 				if (pkt_len > SKBUFF_RX_COPYBREAK) {
 					struct sk_buff *newskb;
 
-					newskb = dev_alloc_skb(NEW_SKB_SIZE);
+					newskb = netdev_alloc_skb(bp->dev,
+								  NEW_SKB_SIZE);
 					if (newskb){
 						rx_in_place = 1;
 
@@ -3070,7 +3071,10 @@ static void dfx_rcv_queue_process(
 						skb = NULL;
 				} else
 #endif
-					skb = dev_alloc_skb(pkt_len+3);	/* alloc new buffer to pass up, add room for PRH */
+					/* Alloc new buffer to pass up,
+					 * add room for PRH. */
+					skb = netdev_alloc_skb(bp->dev,
+							       pkt_len + 3);
 				if (skb == NULL)
 					{
 					printk("%s: Could not allocate receive buffer.  Dropping packet.\n", bp->dev->name);

[PATCH 4/5] defxx: Handle DMA mapping errors

[Maciej W. Rozycki]

This adds error handling for DMA mapping requests; I think there isn't
much else to say about it.

A good side-effect is the mapping in the transmit path is now made with
the board lock released.  Also if DMA mapping fails for a newly
allocated receive buffer, then data from the old buffer will be copied
out (as is presently done for small frames only whose size does not
exceed SKBUFF_RX_COPYBREAK) and the original buffer returned, with its 
mapping unchanged, to the DMA descriptor ring.

Reported-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Tested-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
 Please apply,

  Maciej

linux-defxx-dma-map-error.patch
Index: linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
===================================================================
--- linux-20140623-swarm64-eb.orig/drivers/net/fddi/defxx.c
+++ linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
@@ -2927,21 +2927,35 @@ static int dfx_rcv_init(DFX_board_t *bp,
 	for (i = 0; i < (int)(bp->rcv_bufs_to_post); i++)
 		for (j = 0; (i + j) < (int)PI_RCV_DATA_K_NUM_ENTRIES; j += bp->rcv_bufs_to_post)
 		{
-			struct sk_buff *newskb = __netdev_alloc_skb(bp->dev, NEW_SKB_SIZE, GFP_NOIO);
+			struct sk_buff *newskb;
+			dma_addr_t dma_addr;
+
+			newskb = __netdev_alloc_skb(bp->dev, NEW_SKB_SIZE,
+						    GFP_NOIO);
 			if (!newskb)
 				return -ENOMEM;
-			bp->descr_block_virt->rcv_data[i+j].long_0 = (u32) (PI_RCV_DESCR_M_SOP |
-				((PI_RCV_DATA_K_SIZE_MAX / PI_ALIGN_K_RCV_DATA_BUFF) << PI_RCV_DESCR_V_SEG_LEN));
 			/*
 			 * align to 128 bytes for compatibility with
 			 * the old EISA boards.
 			 */
 
 			my_skb_align(newskb, 128);
+			dma_addr = dma_map_single(bp->bus_dev,
+						  newskb->data,
+						  PI_RCV_DATA_K_SIZE_MAX,
+						  DMA_FROM_DEVICE);
+			if (dma_mapping_error(bp->bus_dev, dma_addr)) {
+				dev_kfree_skb(newskb);
+				return -ENOMEM;
+			}
+			bp->descr_block_virt->rcv_data[i + j].long_0 =
+				(u32)(PI_RCV_DESCR_M_SOP |
+				      ((PI_RCV_DATA_K_SIZE_MAX /
+					PI_ALIGN_K_RCV_DATA_BUFF) <<
+				       PI_RCV_DESCR_V_SEG_LEN));
 			bp->descr_block_virt->rcv_data[i + j].long_1 =
-				(u32)dma_map_single(bp->bus_dev, newskb->data,
-						    PI_RCV_DATA_K_SIZE_MAX,
-						    DMA_FROM_DEVICE);
+				(u32)dma_addr;
+
 			/*
 			 * p_rcv_buff_va is only used inside the
 			 * kernel so we put the skb pointer here.
@@ -3008,7 +3022,7 @@ static void dfx_rcv_queue_process(
 	PI_TYPE_2_CONSUMER	*p_type_2_cons;		/* ptr to rcv/xmt consumer block register */
 	char				*p_buff;			/* ptr to start of packet receive buffer (FMC descriptor) */
 	u32					descr, pkt_len;		/* FMC descriptor field and packet length */
-	struct sk_buff		*skb;				/* pointer to a sk_buff to hold incoming packet data */
+	struct sk_buff		*skb = NULL;			/* pointer to a sk_buff to hold incoming packet data */
 
 	/* Service all consumed LLC receive frames */
 
@@ -3046,15 +3060,30 @@ static void dfx_rcv_queue_process(
 				bp->rcv_length_errors++;
 			else{
 #ifdef DYNAMIC_BUFFERS
+				struct sk_buff *newskb = NULL;
+
 				if (pkt_len > SKBUFF_RX_COPYBREAK) {
-					struct sk_buff *newskb;
+					dma_addr_t new_dma_addr;
 
 					newskb = netdev_alloc_skb(bp->dev,
 								  NEW_SKB_SIZE);
 					if (newskb){
+						my_skb_align(newskb, 128);
+						new_dma_addr = dma_map_single(
+								bp->bus_dev,
+								newskb->data,
+								PI_RCV_DATA_K_SIZE_MAX,
+								DMA_FROM_DEVICE);
+						if (dma_mapping_error(
+								bp->bus_dev,
+								new_dma_addr)) {
+							dev_kfree_skb(newskb);
+							newskb = NULL;
+						}
+					}
+					if (newskb) {
 						rx_in_place = 1;
 
-						my_skb_align(newskb, 128);
 						skb = (struct sk_buff *)bp->p_rcv_buff_va[entry];
 						dma_unmap_single(bp->bus_dev,
 							bp->descr_block_virt->rcv_data[entry].long_1,
@@ -3062,14 +3091,10 @@ static void dfx_rcv_queue_process(
 							DMA_FROM_DEVICE);
 						skb_reserve(skb, RCV_BUFF_K_PADDING);
 						bp->p_rcv_buff_va[entry] = (char *)newskb;
-						bp->descr_block_virt->rcv_data[entry].long_1 =
-							(u32)dma_map_single(bp->bus_dev,
-								newskb->data,
-								PI_RCV_DATA_K_SIZE_MAX,
-								DMA_FROM_DEVICE);
-					} else
-						skb = NULL;
-				} else
+						bp->descr_block_virt->rcv_data[entry].long_1 = (u32)new_dma_addr;
+					}
+				}
+				if (!newskb)
 #endif
 					/* Alloc new buffer to pass up,
 					 * add room for PRH. */
@@ -3186,6 +3211,7 @@ static netdev_tx_t dfx_xmt_queue_pkt(str
 	u8			prod;				/* local transmit producer index */
 	PI_XMT_DESCR		*p_xmt_descr;		/* ptr to transmit descriptor block entry */
 	XMT_DRIVER_DESCR	*p_xmt_drv_descr;	/* ptr to transmit driver descriptor */
+	dma_addr_t		dma_addr;
 	unsigned long		flags;
 
 	netif_stop_queue(dev);
@@ -3233,6 +3259,20 @@ static netdev_tx_t dfx_xmt_queue_pkt(str
 			}
 		}
 
+	/* Write the three PRH bytes immediately before the FC byte */
+
+	skb_push(skb, 3);
+	skb->data[0] = DFX_PRH0_BYTE;	/* these byte values are defined */
+	skb->data[1] = DFX_PRH1_BYTE;	/* in the Motorola FDDI MAC chip */
+	skb->data[2] = DFX_PRH2_BYTE;	/* specification */
+
+	dma_addr = dma_map_single(bp->bus_dev, skb->data, skb->len,
+				  DMA_TO_DEVICE);
+	if (dma_mapping_error(bp->bus_dev, dma_addr)) {
+		skb_pull(skb, 3);
+		return NETDEV_TX_BUSY;
+	}
+
 	spin_lock_irqsave(&bp->lock, flags);
 
 	/* Get the current producer and the next free xmt data descriptor */
@@ -3253,13 +3293,6 @@ static netdev_tx_t dfx_xmt_queue_pkt(str
 
 	p_xmt_drv_descr = &(bp->xmt_drv_descr_blk[prod++]);	/* also bump producer index */
 
-	/* Write the three PRH bytes immediately before the FC byte */
-
-	skb_push(skb,3);
-	skb->data[0] = DFX_PRH0_BYTE;	/* these byte values are defined */
-	skb->data[1] = DFX_PRH1_BYTE;	/* in the Motorola FDDI MAC chip */
-	skb->data[2] = DFX_PRH2_BYTE;	/* specification */
-
 	/*
 	 * Write the descriptor with buffer info and bump producer
 	 *
@@ -3288,8 +3321,7 @@ static netdev_tx_t dfx_xmt_queue_pkt(str
 	 */
 
 	p_xmt_descr->long_0	= (u32) (PI_XMT_DESCR_M_SOP | PI_XMT_DESCR_M_EOP | ((skb->len) << PI_XMT_DESCR_V_SEG_LEN));
-	p_xmt_descr->long_1 = (u32)dma_map_single(bp->bus_dev, skb->data,
-						  skb->len, DMA_TO_DEVICE);
+	p_xmt_descr->long_1 = (u32)dma_addr;
 
 	/*
 	 * Verify that descriptor is actually available

[PATCH 5/5] defxx: Add missing DMA synchronisation calls

[Maciej W. Rozycki]

This adds DMA synchronisation calls needed in the receive path:

1. To retrieve the Receive Status word that is prepended by the PDQ DMA
   engine in the receive buffer, and provides information about the
   frame received, including its size and any errors.

2. To make data received available for copying in the small-frame case
   (size <= SKBUFF_RX_COPYBREAK) where the original DMA buffer will be
   returned to the receive descriptor ring and therefore its mapping
   retained.

   With DMA mapping error handling in place, added by the other patch,
   this may now also trigger where an attempt to map a newly allocated
   buffer for DMA has failed.  In that case data from the original buffer
   will be copied out and the buffer returned to the DMA descriptor ring.

These calls may do nothing when data is in the host DMA addressing range
of the FDDI interface, such as always on 32-bit systems, however their
absence makes frame reception stop functioning reliably on systems that
have memory beyond the low 4GB of the address space.

Reported-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Tested-by: Robert Coerver <Robert.Coerver@ll.mit.edu>
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
 Please apply,

  Maciej

linux-defxx-dma-map-sync.patch
Index: linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
===================================================================
--- linux-20140623-swarm64-eb.orig/drivers/net/fddi/defxx.c
+++ linux-20140623-swarm64-eb/drivers/net/fddi/defxx.c
@@ -196,6 +196,7 @@
  *		14 Jun 2005	macro		Use irqreturn_t.
  *		23 Oct 2006	macro		Big-endian host support.
  *		14 Dec 2006	macro		TURBOchannel support.
+ *		01 Jul 2014	macro		Fixes for DMA on 64-bit hosts.
  */
 
 /* Include files */
@@ -224,8 +225,8 @@
 
 /* Version information string should be updated prior to each new release!  */
 #define DRV_NAME "defxx"
-#define DRV_VERSION "v1.10"
-#define DRV_RELDATE "2006/12/14"
+#define DRV_VERSION "v1.11"
+#define DRV_RELDATE "2014/07/01"
 
 static char version[] =
 	DRV_NAME ": " DRV_VERSION " " DRV_RELDATE
@@ -3030,7 +3031,7 @@ static void dfx_rcv_queue_process(
 	while (bp->rcv_xmt_reg.index.rcv_comp != p_type_2_cons->index.rcv_cons)
 		{
 		/* Process any errors */
-
+		dma_addr_t dma_addr;
 		int entry;
 
 		entry = bp->rcv_xmt_reg.index.rcv_comp;
@@ -3039,6 +3040,11 @@ static void dfx_rcv_queue_process(
 #else
 		p_buff = bp->p_rcv_buff_va[entry];
 #endif
+		dma_addr = bp->descr_block_virt->rcv_data[entry].long_1;
+		dma_sync_single_for_cpu(bp->bus_dev,
+					dma_addr + RCV_BUFF_K_DESCR,
+					sizeof(u32),
+					DMA_FROM_DEVICE);
 		memcpy(&descr, p_buff + RCV_BUFF_K_DESCR, sizeof(u32));
 
 		if (descr & PI_FMC_DESCR_M_RCC_FLUSH)
@@ -3086,7 +3092,7 @@ static void dfx_rcv_queue_process(
 
 						skb = (struct sk_buff *)bp->p_rcv_buff_va[entry];
 						dma_unmap_single(bp->bus_dev,
-							bp->descr_block_virt->rcv_data[entry].long_1,
+							dma_addr,
 							PI_RCV_DATA_K_SIZE_MAX,
 							DMA_FROM_DEVICE);
 						skb_reserve(skb, RCV_BUFF_K_PADDING);
@@ -3109,6 +3115,12 @@ static void dfx_rcv_queue_process(
 				else {
 					if (!rx_in_place) {
 						/* Receive buffer allocated, pass receive packet up */
+						dma_sync_single_for_cpu(
+							bp->bus_dev,
+							dma_addr +
+							RCV_BUFF_K_PADDING,
+							pkt_len + 3,
+							DMA_FROM_DEVICE);
 
 						skb_copy_to_linear_data(skb,
 							       p_buff + RCV_BUFF_K_PADDING,

Re: [PATCH 0/5] defxx: Fixes for 64-bit host support

[David Miller]

From: "Maciej W. Rozycki" <macro@linux-mips.org>
Date: Sat, 5 Jul 2014 15:14:13 +0100 (BST)

>  This mini patch series addresses issues with 64-bit host support for FDDI 
> interface boards supported by the defxx driver where DMA mapping 
> synchronisation is required on swiotlb systems.  While PDQ, the DMA engine 
> chip used with these boards, supports 48-bit addressing that would 
> normally suffice for typical 64-bit systems in existence, the host bus 
> interface chips used by individual implementations have their limitations 
> as follows:

Series applied to net-next, thanks.