From d68ab591f874cf752101ac77b08c01123b6f3a2e Mon Sep 17 00:00:00 2001 From: "Maciej W. Rozycki" Date: Sat, 5 Jul 2014 15:14:22 +0100 Subject: [PATCH 1/5] defxx: Correct the receive DMA map size 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 Tested-by: Robert Coerver Signed-off-by: Maciej W. Rozycki Signed-off-by: David S. Miller --- drivers/net/fddi/defxx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c index eb78203cd58e..4dcfb32983d9 100644 --- a/drivers/net/fddi/defxx.c +++ b/drivers/net/fddi/defxx.c @@ -2936,7 +2936,7 @@ static int dfx_rcv_init(DFX_board_t *bp, int get_buffers) 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 @@ -3053,14 +3053,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; From 6329fe5c4e61655bcb8456805d2c485f791b50cc Mon Sep 17 00:00:00 2001 From: "Maciej W. Rozycki" Date: Sat, 5 Jul 2014 15:14:30 +0100 Subject: [PATCH 2/5] defxx: Discard DMA maps on buffer deallocation 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 Tested-by: Robert Coerver Signed-off-by: Maciej W. Rozycki Signed-off-by: David S. Miller --- drivers/net/fddi/defxx.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c index 4dcfb32983d9..0b2e80940b95 100644 --- a/drivers/net/fddi/defxx.c +++ b/drivers/net/fddi/defxx.c @@ -3447,8 +3447,13 @@ static void dfx_rcv_flush( DFX_board_t *bp ) { 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; } From a630be70771ddbe8253f619ac4b9ca4c3277b13b Mon Sep 17 00:00:00 2001 From: "Maciej W. Rozycki" Date: Sat, 5 Jul 2014 15:14:36 +0100 Subject: [PATCH 3/5] defxx: Use netdev_alloc_skb consistently 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 Tested-by: Robert Coerver Signed-off-by: Maciej W. Rozycki Signed-off-by: David S. Miller --- drivers/net/fddi/defxx.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c index 0b2e80940b95..4c5a2fe67dfb 100644 --- a/drivers/net/fddi/defxx.c +++ b/drivers/net/fddi/defxx.c @@ -3045,7 +3045,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; @@ -3066,7 +3067,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); From b37cccf031bdcaba2f461cdb5a2b93ebbd0af03c Mon Sep 17 00:00:00 2001 From: "Maciej W. Rozycki" Date: Sat, 5 Jul 2014 15:14:40 +0100 Subject: [PATCH 4/5] defxx: Handle DMA mapping errors 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 Tested-by: Robert Coerver Signed-off-by: Maciej W. Rozycki Signed-off-by: David S. Miller --- drivers/net/fddi/defxx.c | 84 +++++++++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 26 deletions(-) diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c index 4c5a2fe67dfb..ed23288d1c55 100644 --- a/drivers/net/fddi/defxx.c +++ b/drivers/net/fddi/defxx.c @@ -2923,21 +2923,35 @@ static int dfx_rcv_init(DFX_board_t *bp, int get_buffers) 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. @@ -3004,7 +3018,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 */ @@ -3042,15 +3056,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, @@ -3058,14 +3087,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. */ @@ -3185,6 +3210,7 @@ static netdev_tx_t dfx_xmt_queue_pkt(struct sk_buff *skb, 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); @@ -3232,6 +3258,20 @@ static netdev_tx_t dfx_xmt_queue_pkt(struct sk_buff *skb, } } + /* 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 */ @@ -3252,13 +3292,6 @@ static netdev_tx_t dfx_xmt_queue_pkt(struct sk_buff *skb, 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 * @@ -3287,8 +3320,7 @@ static netdev_tx_t dfx_xmt_queue_pkt(struct sk_buff *skb, */ 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 From 8848761f9432160ad63e28b16f5c4516683ef905 Mon Sep 17 00:00:00 2001 From: "Maciej W. Rozycki" Date: Sat, 5 Jul 2014 15:14:46 +0100 Subject: [PATCH 5/5] defxx: Add missing DMA synchronisation calls 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 Tested-by: Robert Coerver Signed-off-by: Maciej W. Rozycki Signed-off-by: David S. Miller --- drivers/net/fddi/defxx.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c index ed23288d1c55..3d1a878a4ab0 100644 --- a/drivers/net/fddi/defxx.c +++ b/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 @@ -3026,7 +3027,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; @@ -3035,6 +3036,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) @@ -3082,7 +3088,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); @@ -3108,6 +3114,12 @@ static void dfx_rcv_queue_process( #endif { /* 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,