From 62f8dc529c76aca43113312079299d40741407dd Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Fri, 1 Jun 2012 00:56:22 +0100 Subject: [PATCH 01/10] sfc: Work around bogus 'uninitialised variable' warning With some gcc versions & optimisations, the compiler will warn that 'depth' in efx_filter_insert_filter() may be used without being initialised, although this is not the case. This is related to inlining of efx_filter_search(), which only has one caller since commit 8db182f4a8a6e2dcb8b65905ea4af56210e65430 ('sfc: Remove now-unused filter function'). Shut the compiler up by initialising it to 0. Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/sfc/filter.c b/drivers/net/ethernet/sfc/filter.c index fea7f7300675..c3fd61f0a95c 100644 --- a/drivers/net/ethernet/sfc/filter.c +++ b/drivers/net/ethernet/sfc/filter.c @@ -662,7 +662,7 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec, struct efx_filter_table *table = efx_filter_spec_table(state, spec); struct efx_filter_spec *saved_spec; efx_oword_t filter; - unsigned int filter_idx, depth; + unsigned int filter_idx, depth = 0; u32 key; int rc; From 0e33d87033d84768ae700217e7c52dfb0c3399ca Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 17 May 2012 17:46:55 +0100 Subject: [PATCH 02/10] sfc: Use generic DMA API, not PCI-DMA API Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/efx.c | 10 ++-- drivers/net/ethernet/sfc/net_driver.h | 2 +- drivers/net/ethernet/sfc/nic.c | 8 +-- drivers/net/ethernet/sfc/rx.c | 22 +++---- drivers/net/ethernet/sfc/tx.c | 83 +++++++++++++-------------- 5 files changed, 62 insertions(+), 63 deletions(-) diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c index b95f2e1b33f0..70554a1b2b02 100644 --- a/drivers/net/ethernet/sfc/efx.c +++ b/drivers/net/ethernet/sfc/efx.c @@ -1103,8 +1103,8 @@ static int efx_init_io(struct efx_nic *efx) * masks event though they reject 46 bit masks. */ while (dma_mask > 0x7fffffffUL) { - if (pci_dma_supported(pci_dev, dma_mask)) { - rc = pci_set_dma_mask(pci_dev, dma_mask); + if (dma_supported(&pci_dev->dev, dma_mask)) { + rc = dma_set_mask(&pci_dev->dev, dma_mask); if (rc == 0) break; } @@ -1117,10 +1117,10 @@ static int efx_init_io(struct efx_nic *efx) } netif_dbg(efx, probe, efx->net_dev, "using DMA mask %llx\n", (unsigned long long) dma_mask); - rc = pci_set_consistent_dma_mask(pci_dev, dma_mask); + rc = dma_set_coherent_mask(&pci_dev->dev, dma_mask); if (rc) { - /* pci_set_consistent_dma_mask() is not *allowed* to - * fail with a mask that pci_set_dma_mask() accepted, + /* dma_set_coherent_mask() is not *allowed* to + * fail with a mask that dma_set_mask() accepted, * but just in case... */ netif_err(efx, probe, efx->net_dev, diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h index 0e575359af17..8a9f6d48214d 100644 --- a/drivers/net/ethernet/sfc/net_driver.h +++ b/drivers/net/ethernet/sfc/net_driver.h @@ -100,7 +100,7 @@ struct efx_special_buffer { * @len: Length of this fragment. * This field is zero when the queue slot is empty. * @continuation: True if this fragment is not the end of a packet. - * @unmap_single: True if pci_unmap_single should be used. + * @unmap_single: True if dma_unmap_single should be used. * @unmap_len: Length of this fragment to unmap */ struct efx_tx_buffer { diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c index 4a9a5beec8fc..287738db24e5 100644 --- a/drivers/net/ethernet/sfc/nic.c +++ b/drivers/net/ethernet/sfc/nic.c @@ -308,8 +308,8 @@ efx_free_special_buffer(struct efx_nic *efx, struct efx_special_buffer *buffer) int efx_nic_alloc_buffer(struct efx_nic *efx, struct efx_buffer *buffer, unsigned int len) { - buffer->addr = pci_alloc_consistent(efx->pci_dev, len, - &buffer->dma_addr); + buffer->addr = dma_alloc_coherent(&efx->pci_dev->dev, len, + &buffer->dma_addr, GFP_ATOMIC); if (!buffer->addr) return -ENOMEM; buffer->len = len; @@ -320,8 +320,8 @@ int efx_nic_alloc_buffer(struct efx_nic *efx, struct efx_buffer *buffer, void efx_nic_free_buffer(struct efx_nic *efx, struct efx_buffer *buffer) { if (buffer->addr) { - pci_free_consistent(efx->pci_dev, buffer->len, - buffer->addr, buffer->dma_addr); + dma_free_coherent(&efx->pci_dev->dev, buffer->len, + buffer->addr, buffer->dma_addr); buffer->addr = NULL; } } diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c index 243e91f3dff9..6d1c6cfd6ba8 100644 --- a/drivers/net/ethernet/sfc/rx.c +++ b/drivers/net/ethernet/sfc/rx.c @@ -155,11 +155,11 @@ static int efx_init_rx_buffers_skb(struct efx_rx_queue *rx_queue) rx_buf->len = skb_len - NET_IP_ALIGN; rx_buf->flags = 0; - rx_buf->dma_addr = pci_map_single(efx->pci_dev, + rx_buf->dma_addr = dma_map_single(&efx->pci_dev->dev, skb->data, rx_buf->len, - PCI_DMA_FROMDEVICE); - if (unlikely(pci_dma_mapping_error(efx->pci_dev, - rx_buf->dma_addr))) { + DMA_FROM_DEVICE); + if (unlikely(dma_mapping_error(&efx->pci_dev->dev, + rx_buf->dma_addr))) { dev_kfree_skb_any(skb); rx_buf->u.skb = NULL; return -EIO; @@ -200,10 +200,10 @@ static int efx_init_rx_buffers_page(struct efx_rx_queue *rx_queue) efx->rx_buffer_order); if (unlikely(page == NULL)) return -ENOMEM; - dma_addr = pci_map_page(efx->pci_dev, page, 0, + dma_addr = dma_map_page(&efx->pci_dev->dev, page, 0, efx_rx_buf_size(efx), - PCI_DMA_FROMDEVICE); - if (unlikely(pci_dma_mapping_error(efx->pci_dev, dma_addr))) { + DMA_FROM_DEVICE); + if (unlikely(dma_mapping_error(&efx->pci_dev->dev, dma_addr))) { __free_pages(page, efx->rx_buffer_order); return -EIO; } @@ -247,14 +247,14 @@ static void efx_unmap_rx_buffer(struct efx_nic *efx, state = page_address(rx_buf->u.page); if (--state->refcnt == 0) { - pci_unmap_page(efx->pci_dev, + dma_unmap_page(&efx->pci_dev->dev, state->dma_addr, efx_rx_buf_size(efx), - PCI_DMA_FROMDEVICE); + DMA_FROM_DEVICE); } } else if (!(rx_buf->flags & EFX_RX_BUF_PAGE) && rx_buf->u.skb) { - pci_unmap_single(efx->pci_dev, rx_buf->dma_addr, - rx_buf->len, PCI_DMA_FROMDEVICE); + dma_unmap_single(&efx->pci_dev->dev, rx_buf->dma_addr, + rx_buf->len, DMA_FROM_DEVICE); } } diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c index 94d0365b31cd..18860f241bc5 100644 --- a/drivers/net/ethernet/sfc/tx.c +++ b/drivers/net/ethernet/sfc/tx.c @@ -36,15 +36,15 @@ static void efx_dequeue_buffer(struct efx_tx_queue *tx_queue, unsigned int *bytes_compl) { if (buffer->unmap_len) { - struct pci_dev *pci_dev = tx_queue->efx->pci_dev; + struct device *dma_dev = &tx_queue->efx->pci_dev->dev; dma_addr_t unmap_addr = (buffer->dma_addr + buffer->len - buffer->unmap_len); if (buffer->unmap_single) - pci_unmap_single(pci_dev, unmap_addr, buffer->unmap_len, - PCI_DMA_TODEVICE); + dma_unmap_single(dma_dev, unmap_addr, buffer->unmap_len, + DMA_TO_DEVICE); else - pci_unmap_page(pci_dev, unmap_addr, buffer->unmap_len, - PCI_DMA_TODEVICE); + dma_unmap_page(dma_dev, unmap_addr, buffer->unmap_len, + DMA_TO_DEVICE); buffer->unmap_len = 0; buffer->unmap_single = false; } @@ -138,7 +138,7 @@ efx_max_tx_len(struct efx_nic *efx, dma_addr_t dma_addr) netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb) { struct efx_nic *efx = tx_queue->efx; - struct pci_dev *pci_dev = efx->pci_dev; + struct device *dma_dev = &efx->pci_dev->dev; struct efx_tx_buffer *buffer; skb_frag_t *fragment; unsigned int len, unmap_len = 0, fill_level, insert_ptr; @@ -167,17 +167,17 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb) fill_level = tx_queue->insert_count - tx_queue->old_read_count; q_space = efx->txq_entries - 1 - fill_level; - /* Map for DMA. Use pci_map_single rather than pci_map_page + /* Map for DMA. Use dma_map_single rather than dma_map_page * since this is more efficient on machines with sparse * memory. */ unmap_single = true; - dma_addr = pci_map_single(pci_dev, skb->data, len, PCI_DMA_TODEVICE); + dma_addr = dma_map_single(dma_dev, skb->data, len, PCI_DMA_TODEVICE); /* Process all fragments */ while (1) { - if (unlikely(pci_dma_mapping_error(pci_dev, dma_addr))) - goto pci_err; + if (unlikely(dma_mapping_error(dma_dev, dma_addr))) + goto dma_err; /* Store fields for marking in the per-fragment final * descriptor */ @@ -246,7 +246,7 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb) i++; /* Map for DMA */ unmap_single = false; - dma_addr = skb_frag_dma_map(&pci_dev->dev, fragment, 0, len, + dma_addr = skb_frag_dma_map(dma_dev, fragment, 0, len, DMA_TO_DEVICE); } @@ -261,7 +261,7 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb) return NETDEV_TX_OK; - pci_err: + dma_err: netif_err(efx, tx_err, efx->net_dev, " TX queue %d could not map skb with %d bytes %d " "fragments for DMA\n", tx_queue->queue, skb->len, @@ -284,11 +284,11 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb) /* Free the fragment we were mid-way through pushing */ if (unmap_len) { if (unmap_single) - pci_unmap_single(pci_dev, unmap_addr, unmap_len, - PCI_DMA_TODEVICE); + dma_unmap_single(dma_dev, unmap_addr, unmap_len, + DMA_TO_DEVICE); else - pci_unmap_page(pci_dev, unmap_addr, unmap_len, - PCI_DMA_TODEVICE); + dma_unmap_page(dma_dev, unmap_addr, unmap_len, + DMA_TO_DEVICE); } return rc; @@ -684,20 +684,19 @@ static __be16 efx_tso_check_protocol(struct sk_buff *skb) */ static int efx_tsoh_block_alloc(struct efx_tx_queue *tx_queue) { - - struct pci_dev *pci_dev = tx_queue->efx->pci_dev; + struct device *dma_dev = &tx_queue->efx->pci_dev->dev; struct efx_tso_header *tsoh; dma_addr_t dma_addr; u8 *base_kva, *kva; - base_kva = pci_alloc_consistent(pci_dev, PAGE_SIZE, &dma_addr); + base_kva = dma_alloc_coherent(dma_dev, PAGE_SIZE, &dma_addr, GFP_ATOMIC); if (base_kva == NULL) { netif_err(tx_queue->efx, tx_err, tx_queue->efx->net_dev, "Unable to allocate page for TSO headers\n"); return -ENOMEM; } - /* pci_alloc_consistent() allocates pages. */ + /* dma_alloc_coherent() allocates pages. */ EFX_BUG_ON_PARANOID(dma_addr & (PAGE_SIZE - 1u)); for (kva = base_kva; kva < base_kva + PAGE_SIZE; kva += TSOH_STD_SIZE) { @@ -714,7 +713,7 @@ static int efx_tsoh_block_alloc(struct efx_tx_queue *tx_queue) /* Free up a TSO header, and all others in the same page. */ static void efx_tsoh_block_free(struct efx_tx_queue *tx_queue, struct efx_tso_header *tsoh, - struct pci_dev *pci_dev) + struct device *dma_dev) { struct efx_tso_header **p; unsigned long base_kva; @@ -731,7 +730,7 @@ static void efx_tsoh_block_free(struct efx_tx_queue *tx_queue, p = &(*p)->next; } - pci_free_consistent(pci_dev, PAGE_SIZE, (void *)base_kva, base_dma); + dma_free_coherent(dma_dev, PAGE_SIZE, (void *)base_kva, base_dma); } static struct efx_tso_header * @@ -743,11 +742,11 @@ efx_tsoh_heap_alloc(struct efx_tx_queue *tx_queue, size_t header_len) if (unlikely(!tsoh)) return NULL; - tsoh->dma_addr = pci_map_single(tx_queue->efx->pci_dev, + tsoh->dma_addr = dma_map_single(&tx_queue->efx->pci_dev->dev, TSOH_BUFFER(tsoh), header_len, - PCI_DMA_TODEVICE); - if (unlikely(pci_dma_mapping_error(tx_queue->efx->pci_dev, - tsoh->dma_addr))) { + DMA_TO_DEVICE); + if (unlikely(dma_mapping_error(&tx_queue->efx->pci_dev->dev, + tsoh->dma_addr))) { kfree(tsoh); return NULL; } @@ -759,9 +758,9 @@ efx_tsoh_heap_alloc(struct efx_tx_queue *tx_queue, size_t header_len) static void efx_tsoh_heap_free(struct efx_tx_queue *tx_queue, struct efx_tso_header *tsoh) { - pci_unmap_single(tx_queue->efx->pci_dev, + dma_unmap_single(&tx_queue->efx->pci_dev->dev, tsoh->dma_addr, tsoh->unmap_len, - PCI_DMA_TODEVICE); + DMA_TO_DEVICE); kfree(tsoh); } @@ -892,13 +891,13 @@ static void efx_enqueue_unwind(struct efx_tx_queue *tx_queue) unmap_addr = (buffer->dma_addr + buffer->len - buffer->unmap_len); if (buffer->unmap_single) - pci_unmap_single(tx_queue->efx->pci_dev, + dma_unmap_single(&tx_queue->efx->pci_dev->dev, unmap_addr, buffer->unmap_len, - PCI_DMA_TODEVICE); + DMA_TO_DEVICE); else - pci_unmap_page(tx_queue->efx->pci_dev, + dma_unmap_page(&tx_queue->efx->pci_dev->dev, unmap_addr, buffer->unmap_len, - PCI_DMA_TODEVICE); + DMA_TO_DEVICE); buffer->unmap_len = 0; } buffer->len = 0; @@ -954,9 +953,9 @@ static int tso_get_head_fragment(struct tso_state *st, struct efx_nic *efx, int hl = st->header_len; int len = skb_headlen(skb) - hl; - st->unmap_addr = pci_map_single(efx->pci_dev, skb->data + hl, - len, PCI_DMA_TODEVICE); - if (likely(!pci_dma_mapping_error(efx->pci_dev, st->unmap_addr))) { + st->unmap_addr = dma_map_single(&efx->pci_dev->dev, skb->data + hl, + len, DMA_TO_DEVICE); + if (likely(!dma_mapping_error(&efx->pci_dev->dev, st->unmap_addr))) { st->unmap_single = true; st->unmap_len = len; st->in_len = len; @@ -1008,7 +1007,7 @@ static int tso_fill_packet_with_fragment(struct efx_tx_queue *tx_queue, buffer->continuation = !end_of_packet; if (st->in_len == 0) { - /* Transfer ownership of the pci mapping */ + /* Transfer ownership of the DMA mapping */ buffer->unmap_len = st->unmap_len; buffer->unmap_single = st->unmap_single; st->unmap_len = 0; @@ -1181,18 +1180,18 @@ static int efx_enqueue_skb_tso(struct efx_tx_queue *tx_queue, mem_err: netif_err(efx, tx_err, efx->net_dev, - "Out of memory for TSO headers, or PCI mapping error\n"); + "Out of memory for TSO headers, or DMA mapping error\n"); dev_kfree_skb_any(skb); unwind: /* Free the DMA mapping we were in the process of writing out */ if (state.unmap_len) { if (state.unmap_single) - pci_unmap_single(efx->pci_dev, state.unmap_addr, - state.unmap_len, PCI_DMA_TODEVICE); + dma_unmap_single(&efx->pci_dev->dev, state.unmap_addr, + state.unmap_len, DMA_TO_DEVICE); else - pci_unmap_page(efx->pci_dev, state.unmap_addr, - state.unmap_len, PCI_DMA_TODEVICE); + dma_unmap_page(&efx->pci_dev->dev, state.unmap_addr, + state.unmap_len, DMA_TO_DEVICE); } efx_enqueue_unwind(tx_queue); @@ -1216,5 +1215,5 @@ static void efx_fini_tso(struct efx_tx_queue *tx_queue) while (tx_queue->tso_headers_free != NULL) efx_tsoh_block_free(tx_queue, tx_queue->tso_headers_free, - tx_queue->efx->pci_dev); + &tx_queue->efx->pci_dev->dev); } From e718905c4b02bfc650f383e02b643b7e60b9407e Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Tue, 19 Jun 2012 19:57:22 +0100 Subject: [PATCH 03/10] sfc: Remove dead write to tso_state::packet_space tso_state::packet_space is always set in tso_start_packet(); the value set in tso_start() is not used, and is also incorrect. Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/tx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c index 18860f241bc5..cfa5f6db20bd 100644 --- a/drivers/net/ethernet/sfc/tx.c +++ b/drivers/net/ethernet/sfc/tx.c @@ -926,7 +926,6 @@ static void tso_start(struct tso_state *st, const struct sk_buff *skb) EFX_BUG_ON_PARANOID(tcp_hdr(skb)->syn); EFX_BUG_ON_PARANOID(tcp_hdr(skb)->rst); - st->packet_space = st->full_packet_size; st->out_len = skb->len - st->header_len; st->unmap_len = 0; st->unmap_single = false; From f7cbb163d980fc79f47a3cba63dd130b1d655d1d Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 28 Jun 2012 21:55:15 +0100 Subject: [PATCH 04/10] sfc: Stop changing header offsets on TX There is nothing in the VLAN driver or core VLAN support that invalidates the TCP and IP header offsets. Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/tx.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c index cfa5f6db20bd..9b225a7769f7 100644 --- a/drivers/net/ethernet/sfc/tx.c +++ b/drivers/net/ethernet/sfc/tx.c @@ -651,17 +651,8 @@ static __be16 efx_tso_check_protocol(struct sk_buff *skb) EFX_BUG_ON_PARANOID(((struct ethhdr *)skb->data)->h_proto != protocol); if (protocol == htons(ETH_P_8021Q)) { - /* Find the encapsulated protocol; reset network header - * and transport header based on that. */ struct vlan_ethhdr *veh = (struct vlan_ethhdr *)skb->data; protocol = veh->h_vlan_encapsulated_proto; - skb_set_network_header(skb, sizeof(*veh)); - if (protocol == htons(ETH_P_IP)) - skb_set_transport_header(skb, sizeof(*veh) + - 4 * ip_hdr(skb)->ihl); - else if (protocol == htons(ETH_P_IPV6)) - skb_set_transport_header(skb, sizeof(*veh) + - sizeof(struct ipv6hdr)); } if (protocol == htons(ETH_P_IP)) { From a4ed2d4cd96dd71b0f2e04a88f83abcab6ddd023 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Mon, 2 Jul 2012 21:36:59 +0100 Subject: [PATCH 05/10] sfc: Use strlcpy() to copy ethtool stats names Fix CID 113703 in the Coverity report on Linux. ethtool stats names are limited to 32 bytes including a null terminator. Use strlcpy() to ensure that we will always include the null terminator even if a source string becomes longer than this. Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/ethtool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c index 03ded364c8da..10536f93b561 100644 --- a/drivers/net/ethernet/sfc/ethtool.c +++ b/drivers/net/ethernet/sfc/ethtool.c @@ -453,7 +453,7 @@ static void efx_ethtool_get_strings(struct net_device *net_dev, switch (string_set) { case ETH_SS_STATS: for (i = 0; i < EFX_ETHTOOL_NUM_STATS; i++) - strncpy(ethtool_strings[i].name, + strlcpy(ethtool_strings[i].name, efx_ethtool_stats[i].name, sizeof(ethtool_strings[i].name)); break; From e3ed2bdfc4a5dcdcdf74141e88af9990dc141a4b Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Mon, 2 Jul 2012 23:03:02 +0100 Subject: [PATCH 06/10] sfc: Use dev_kfree_skb() in efx_end_loopback() Fix CID 102619 in the Coverity report on Linux. efx_end_loopback() iterates over an array of skb pointers of which some may be null (if efx_begin_loopback() failed). It should not use dev_kfree_skb_irq(), which requires non-null pointers. In practice this is safe because it does not run in interrupt context and therefore always ends up calling dev_kfree_skb(), which does allow null pointers. But we should make that explicit. Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/selftest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/sfc/selftest.c b/drivers/net/ethernet/sfc/selftest.c index de4c0069f5b2..ccc428fc267b 100644 --- a/drivers/net/ethernet/sfc/selftest.c +++ b/drivers/net/ethernet/sfc/selftest.c @@ -488,7 +488,7 @@ static int efx_end_loopback(struct efx_tx_queue *tx_queue, skb = state->skbs[i]; if (skb && !skb_shared(skb)) ++tx_done; - dev_kfree_skb_any(skb); + dev_kfree_skb(skb); } netif_tx_unlock_bh(efx->net_dev); From 0f1e54ae52b950ed79074ae794d027d6c97fd34e Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Mon, 2 Jul 2012 23:37:40 +0100 Subject: [PATCH 07/10] sfc: Explain why efx_mcdi_exit_assertion() ignores result of efx_mcdi_rpc() Fix CID 113952 in Coverity report on Linux. This is the one instance where we don't, and shouldn't, check the return code from efx_mcdi_rpc(). It wasn't immediately obvious to me why we didn't, so I think an explanation is in order. Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/mcdi.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c index 17b6463e459c..fc5e7bbcbc9e 100644 --- a/drivers/net/ethernet/sfc/mcdi.c +++ b/drivers/net/ethernet/sfc/mcdi.c @@ -1001,12 +1001,17 @@ static void efx_mcdi_exit_assertion(struct efx_nic *efx) { u8 inbuf[MC_CMD_REBOOT_IN_LEN]; - /* Atomically reboot the mcfw out of the assertion handler */ + /* If the MC is running debug firmware, it might now be + * waiting for a debugger to attach, but we just want it to + * reboot. We set a flag that makes the command a no-op if it + * has already done so. We don't know what return code to + * expect (0 or -EIO), so ignore it. + */ BUILD_BUG_ON(MC_CMD_REBOOT_OUT_LEN != 0); MCDI_SET_DWORD(inbuf, REBOOT_IN_FLAGS, MC_CMD_REBOOT_FLAGS_AFTER_ASSERTION); - efx_mcdi_rpc(efx, MC_CMD_REBOOT, inbuf, MC_CMD_REBOOT_IN_LEN, - NULL, 0, NULL); + (void) efx_mcdi_rpc(efx, MC_CMD_REBOOT, inbuf, MC_CMD_REBOOT_IN_LEN, + NULL, 0, NULL); } int efx_mcdi_handle_assertion(struct efx_nic *efx) From d4f2cecce138c34960c467d0ae38a6d4bcd6af7b Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Wed, 4 Jul 2012 03:58:33 +0100 Subject: [PATCH 08/10] sfc: Disable VF queues during register self-test Currently VF queues and drivers may remain active during this test. This could cause memory corruption or spurious test failures. Therefore we reset the port/function before running these tests on Siena. On Falcon this doesn't work: we have to do some additional initialisation before some blocks will work again. So refactor the reset/register-test sequence into an efx_nic_type method so efx_selftest() doesn't have to consider such quirks. In the process, fix another minor bug: Siena does not have an 'invisible' reset and the self-test currently fails to push the PHY configuration after resetting. Passing RESET_TYPE_ALL to efx_reset_{down,up}() fixes this. Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/falcon.c | 35 +++++++++++++-- drivers/net/ethernet/sfc/net_driver.h | 7 ++- drivers/net/ethernet/sfc/nic.c | 3 -- drivers/net/ethernet/sfc/selftest.c | 62 +++++++-------------------- drivers/net/ethernet/sfc/siena.c | 29 +++++++++++-- 5 files changed, 76 insertions(+), 60 deletions(-) diff --git a/drivers/net/ethernet/sfc/falcon.c b/drivers/net/ethernet/sfc/falcon.c index 3a1ca2bd1548..12b573a8e82b 100644 --- a/drivers/net/ethernet/sfc/falcon.c +++ b/drivers/net/ethernet/sfc/falcon.c @@ -25,9 +25,12 @@ #include "io.h" #include "phy.h" #include "workarounds.h" +#include "selftest.h" /* Hardware control for SFC4000 (aka Falcon). */ +static int falcon_reset_hw(struct efx_nic *efx, enum reset_type method); + static const unsigned int /* "Large" EEPROM device: Atmel AT25640 or similar * 8 KB, 16-bit address, 32 B write block */ @@ -1034,10 +1037,34 @@ static const struct efx_nic_register_test falcon_b0_register_tests[] = { EFX_OWORD32(0x0003FF0F, 0x00000000, 0x00000000, 0x00000000) }, }; -static int falcon_b0_test_registers(struct efx_nic *efx) +static int +falcon_b0_test_chip(struct efx_nic *efx, struct efx_self_tests *tests) { - return efx_nic_test_registers(efx, falcon_b0_register_tests, - ARRAY_SIZE(falcon_b0_register_tests)); + enum reset_type reset_method = RESET_TYPE_INVISIBLE; + int rc, rc2; + + mutex_lock(&efx->mac_lock); + if (efx->loopback_modes) { + /* We need the 312 clock from the PHY to test the XMAC + * registers, so move into XGMII loopback if available */ + if (efx->loopback_modes & (1 << LOOPBACK_XGMII)) + efx->loopback_mode = LOOPBACK_XGMII; + else + efx->loopback_mode = __ffs(efx->loopback_modes); + } + __efx_reconfigure_port(efx); + mutex_unlock(&efx->mac_lock); + + efx_reset_down(efx, reset_method); + + tests->registers = + efx_nic_test_registers(efx, falcon_b0_register_tests, + ARRAY_SIZE(falcon_b0_register_tests)) + ? -1 : 1; + + rc = falcon_reset_hw(efx, reset_method); + rc2 = efx_reset_up(efx, reset_method, rc == 0); + return rc ? rc : rc2; } /************************************************************************** @@ -1818,7 +1845,7 @@ const struct efx_nic_type falcon_b0_nic_type = { .get_wol = falcon_get_wol, .set_wol = falcon_set_wol, .resume_wol = efx_port_dummy_op_void, - .test_registers = falcon_b0_test_registers, + .test_chip = falcon_b0_test_chip, .test_nvram = falcon_test_nvram, .revision = EFX_REV_FALCON_B0, diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h index 8a9f6d48214d..55be2fdb0e62 100644 --- a/drivers/net/ethernet/sfc/net_driver.h +++ b/drivers/net/ethernet/sfc/net_driver.h @@ -68,6 +68,8 @@ #define EFX_TXQ_TYPES 4 #define EFX_MAX_TX_QUEUES (EFX_TXQ_TYPES * EFX_MAX_CHANNELS) +struct efx_self_tests; + /** * struct efx_special_buffer - An Efx special buffer * @addr: CPU base address of the buffer @@ -901,7 +903,8 @@ static inline unsigned int efx_port_num(struct efx_nic *efx) * @get_wol: Get WoL configuration from driver state * @set_wol: Push WoL configuration to the NIC * @resume_wol: Synchronise WoL state between driver and MC (e.g. after resume) - * @test_registers: Test read/write functionality of control registers + * @test_chip: Test registers. Should use efx_nic_test_registers(), and is + * expected to reset the NIC. * @test_nvram: Test validity of NVRAM contents * @revision: Hardware architecture revision * @mem_map_size: Memory BAR mapped size @@ -946,7 +949,7 @@ struct efx_nic_type { void (*get_wol)(struct efx_nic *efx, struct ethtool_wolinfo *wol); int (*set_wol)(struct efx_nic *efx, u32 type); void (*resume_wol)(struct efx_nic *efx); - int (*test_registers)(struct efx_nic *efx); + int (*test_chip)(struct efx_nic *efx, struct efx_self_tests *tests); int (*test_nvram)(struct efx_nic *efx); int revision; diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c index 287738db24e5..326d799762d6 100644 --- a/drivers/net/ethernet/sfc/nic.c +++ b/drivers/net/ethernet/sfc/nic.c @@ -124,9 +124,6 @@ int efx_nic_test_registers(struct efx_nic *efx, unsigned address = 0, i, j; efx_oword_t mask, imask, original, reg, buf; - /* Falcon should be in loopback to isolate the XMAC from the PHY */ - WARN_ON(!LOOPBACK_INTERNAL(efx)); - for (i = 0; i < n_regs; ++i) { address = regs[i].address; mask = imask = regs[i].mask; diff --git a/drivers/net/ethernet/sfc/selftest.c b/drivers/net/ethernet/sfc/selftest.c index ccc428fc267b..96068d15b601 100644 --- a/drivers/net/ethernet/sfc/selftest.c +++ b/drivers/net/ethernet/sfc/selftest.c @@ -120,19 +120,6 @@ static int efx_test_nvram(struct efx_nic *efx, struct efx_self_tests *tests) return rc; } -static int efx_test_chip(struct efx_nic *efx, struct efx_self_tests *tests) -{ - int rc = 0; - - /* Test register access */ - if (efx->type->test_registers) { - rc = efx->type->test_registers(efx); - tests->registers = rc ? -1 : 1; - } - - return rc; -} - /************************************************************************** * * Interrupt and event queue testing @@ -699,8 +686,7 @@ int efx_selftest(struct efx_nic *efx, struct efx_self_tests *tests, { enum efx_loopback_mode loopback_mode = efx->loopback_mode; int phy_mode = efx->phy_mode; - enum reset_type reset_method = RESET_TYPE_INVISIBLE; - int rc_test = 0, rc_reset = 0, rc; + int rc_test = 0, rc_reset, rc; efx_selftest_async_cancel(efx); @@ -737,44 +723,26 @@ int efx_selftest(struct efx_nic *efx, struct efx_self_tests *tests, */ netif_device_detach(efx->net_dev); - mutex_lock(&efx->mac_lock); - if (efx->loopback_modes) { - /* We need the 312 clock from the PHY to test the XMAC - * registers, so move into XGMII loopback if available */ - if (efx->loopback_modes & (1 << LOOPBACK_XGMII)) - efx->loopback_mode = LOOPBACK_XGMII; - else - efx->loopback_mode = __ffs(efx->loopback_modes); + if (efx->type->test_chip) { + rc_reset = efx->type->test_chip(efx, tests); + if (rc_reset) { + netif_err(efx, hw, efx->net_dev, + "Unable to recover from chip test\n"); + efx_schedule_reset(efx, RESET_TYPE_DISABLE); + return rc_reset; + } + + if ((tests->registers < 0) && !rc_test) + rc_test = -EIO; } - __efx_reconfigure_port(efx); - mutex_unlock(&efx->mac_lock); - - /* free up all consumers of SRAM (including all the queues) */ - efx_reset_down(efx, reset_method); - - rc = efx_test_chip(efx, tests); - if (rc && !rc_test) - rc_test = rc; - - /* reset the chip to recover from the register test */ - rc_reset = efx->type->reset(efx, reset_method); - /* Ensure that the phy is powered and out of loopback * for the bist and loopback tests */ + mutex_lock(&efx->mac_lock); efx->phy_mode &= ~PHY_MODE_LOW_POWER; efx->loopback_mode = LOOPBACK_NONE; - - rc = efx_reset_up(efx, reset_method, rc_reset == 0); - if (rc && !rc_reset) - rc_reset = rc; - - if (rc_reset) { - netif_err(efx, drv, efx->net_dev, - "Unable to recover from chip test\n"); - efx_schedule_reset(efx, RESET_TYPE_DISABLE); - return rc_reset; - } + __efx_reconfigure_port(efx); + mutex_unlock(&efx->mac_lock); rc = efx_test_phy(efx, tests, flags); if (rc && !rc_test) diff --git a/drivers/net/ethernet/sfc/siena.c b/drivers/net/ethernet/sfc/siena.c index 9f8d7cea3967..2354886293db 100644 --- a/drivers/net/ethernet/sfc/siena.c +++ b/drivers/net/ethernet/sfc/siena.c @@ -25,10 +25,12 @@ #include "workarounds.h" #include "mcdi.h" #include "mcdi_pcol.h" +#include "selftest.h" /* Hardware control for SFC9000 family including SFL9021 (aka Siena). */ static void siena_init_wol(struct efx_nic *efx); +static int siena_reset_hw(struct efx_nic *efx, enum reset_type method); static void siena_push_irq_moderation(struct efx_channel *channel) @@ -154,10 +156,29 @@ static const struct efx_nic_register_test siena_register_tests[] = { EFX_OWORD32(0xFFFFFFFF, 0xFFFFFFFF, 0x00000007, 0x00000000) }, }; -static int siena_test_registers(struct efx_nic *efx) +static int siena_test_chip(struct efx_nic *efx, struct efx_self_tests *tests) { - return efx_nic_test_registers(efx, siena_register_tests, - ARRAY_SIZE(siena_register_tests)); + enum reset_type reset_method = reset_method; + int rc, rc2; + + efx_reset_down(efx, reset_method); + + /* Reset the chip immediately so that it is completely + * quiescent regardless of what any VF driver does. + */ + rc = siena_reset_hw(efx, reset_method); + if (rc) + goto out; + + tests->registers = + efx_nic_test_registers(efx, siena_register_tests, + ARRAY_SIZE(siena_register_tests)) + ? -1 : 1; + + rc = siena_reset_hw(efx, reset_method); +out: + rc2 = efx_reset_up(efx, reset_method, rc == 0); + return rc ? rc : rc2; } /************************************************************************** @@ -649,7 +670,7 @@ const struct efx_nic_type siena_a0_nic_type = { .get_wol = siena_get_wol, .set_wol = siena_set_wol, .resume_wol = siena_init_wol, - .test_registers = siena_test_registers, + .test_chip = siena_test_chip, .test_nvram = efx_mcdi_nvram_test_all, .revision = EFX_REV_SIENA_A0, From b7f514af7d6f36bc2f683e04a909c95e4744b95b Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Wed, 4 Jul 2012 22:25:07 +0100 Subject: [PATCH 09/10] sfc: Fix interface statistics running backward Some interface statistics are computed in such a way that they can sometimes decrease (and even underflow). Since the computed value will never be greater than the true value, we fix this by only storing the computed value when it increases. Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/falcon_xmac.c | 12 ++++++------ drivers/net/ethernet/sfc/nic.h | 18 ++++++++++++++++++ drivers/net/ethernet/sfc/siena.c | 8 ++++---- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/sfc/falcon_xmac.c b/drivers/net/ethernet/sfc/falcon_xmac.c index 6106ef15dee3..8333865d4c95 100644 --- a/drivers/net/ethernet/sfc/falcon_xmac.c +++ b/drivers/net/ethernet/sfc/falcon_xmac.c @@ -341,12 +341,12 @@ void falcon_update_stats_xmac(struct efx_nic *efx) FALCON_STAT(efx, XgTxIpSrcErrPkt, tx_ip_src_error); /* Update derived statistics */ - mac_stats->tx_good_bytes = - (mac_stats->tx_bytes - mac_stats->tx_bad_bytes - - mac_stats->tx_control * 64); - mac_stats->rx_bad_bytes = - (mac_stats->rx_bytes - mac_stats->rx_good_bytes - - mac_stats->rx_control * 64); + efx_update_diff_stat(&mac_stats->tx_good_bytes, + mac_stats->tx_bytes - mac_stats->tx_bad_bytes - + mac_stats->tx_control * 64); + efx_update_diff_stat(&mac_stats->rx_bad_bytes, + mac_stats->rx_bytes - mac_stats->rx_good_bytes - + mac_stats->rx_control * 64); } void falcon_poll_xmac(struct efx_nic *efx) diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h index f48ccf6bb3b9..bab5cd9f5740 100644 --- a/drivers/net/ethernet/sfc/nic.h +++ b/drivers/net/ethernet/sfc/nic.h @@ -294,6 +294,24 @@ extern bool falcon_xmac_check_fault(struct efx_nic *efx); extern int falcon_reconfigure_xmac(struct efx_nic *efx); extern void falcon_update_stats_xmac(struct efx_nic *efx); +/* Some statistics are computed as A - B where A and B each increase + * linearly with some hardware counter(s) and the counters are read + * asynchronously. If the counters contributing to B are always read + * after those contributing to A, the computed value may be lower than + * the true value by some variable amount, and may decrease between + * subsequent computations. + * + * We should never allow statistics to decrease or to exceed the true + * value. Since the computed value will never be greater than the + * true value, we can achieve this by only storing the computed value + * when it increases. + */ +static inline void efx_update_diff_stat(u64 *stat, u64 diff) +{ + if ((s64)(diff - *stat) > 0) + *stat = diff; +} + /* Interrupts and test events */ extern int efx_nic_init_interrupt(struct efx_nic *efx); extern void efx_nic_enable_interrupts(struct efx_nic *efx); diff --git a/drivers/net/ethernet/sfc/siena.c b/drivers/net/ethernet/sfc/siena.c index 2354886293db..6bafd216e55e 100644 --- a/drivers/net/ethernet/sfc/siena.c +++ b/drivers/net/ethernet/sfc/siena.c @@ -458,8 +458,8 @@ static int siena_try_update_nic_stats(struct efx_nic *efx) MAC_STAT(tx_bytes, TX_BYTES); MAC_STAT(tx_bad_bytes, TX_BAD_BYTES); - mac_stats->tx_good_bytes = (mac_stats->tx_bytes - - mac_stats->tx_bad_bytes); + efx_update_diff_stat(&mac_stats->tx_good_bytes, + mac_stats->tx_bytes - mac_stats->tx_bad_bytes); MAC_STAT(tx_packets, TX_PKTS); MAC_STAT(tx_bad, TX_BAD_FCS_PKTS); MAC_STAT(tx_pause, TX_PAUSE_PKTS); @@ -492,8 +492,8 @@ static int siena_try_update_nic_stats(struct efx_nic *efx) MAC_STAT(tx_ip_src_error, TX_IP_SRC_ERR_PKTS); MAC_STAT(rx_bytes, RX_BYTES); MAC_STAT(rx_bad_bytes, RX_BAD_BYTES); - mac_stats->rx_good_bytes = (mac_stats->rx_bytes - - mac_stats->rx_bad_bytes); + efx_update_diff_stat(&mac_stats->rx_good_bytes, + mac_stats->rx_bytes - mac_stats->rx_bad_bytes); MAC_STAT(rx_packets, RX_PKTS); MAC_STAT(rx_good, RX_GOOD_PKTS); MAC_STAT(rx_bad, RX_BAD_FCS_PKTS); From c2dbab39db1c3c2ccbdbb2c6bac6f07cc7a7c1f6 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 5 Jul 2012 17:31:04 +0100 Subject: [PATCH 10/10] sfc: Correct some comments on enum reset_type Signed-off-by: Ben Hutchings --- drivers/net/ethernet/sfc/enum.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/sfc/enum.h b/drivers/net/ethernet/sfc/enum.h index d725a8fbe1a6..182dbe2cc6e4 100644 --- a/drivers/net/ethernet/sfc/enum.h +++ b/drivers/net/ethernet/sfc/enum.h @@ -136,10 +136,10 @@ enum efx_loopback_mode { * * Reset methods are numbered in order of increasing scope. * - * @RESET_TYPE_INVISIBLE: don't reset the PHYs or interrupts - * @RESET_TYPE_ALL: reset everything but PCI core blocks - * @RESET_TYPE_WORLD: reset everything, save & restore PCI config - * @RESET_TYPE_DISABLE: disable NIC + * @RESET_TYPE_INVISIBLE: Reset datapath and MAC (Falcon only) + * @RESET_TYPE_ALL: Reset datapath, MAC and PHY + * @RESET_TYPE_WORLD: Reset as much as possible + * @RESET_TYPE_DISABLE: Reset datapath, MAC and PHY; leave NIC disabled * @RESET_TYPE_TX_WATCHDOG: reset due to TX watchdog * @RESET_TYPE_INT_ERROR: reset due to internal error * @RESET_TYPE_RX_RECOVERY: reset to recover from RX datapath errors