Commit 07681e21 authored by Michael Buesch's avatar Michael Buesch Committed by John W. Linville
Browse files

b43: Rewrite DMA Tx status handling sanity checks



This rewrites the error handling policies in the TX status handler.
It tries to be error-tolerant as in "try hard to not crash the machine".
It won't recover from errors (that are bugs in the firmware or driver),
because that's impossible. However, it will return a more or less useful
error message and bail out. It also tries hard to use rate-limited messages
to not flood the syslog in case of a failure.
Signed-off-by: default avatarMichael Buesch <mb@bu3sch.de>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent 8c35024a
...@@ -874,7 +874,7 @@ static void free_all_descbuffers(struct b43_dmaring *ring) ...@@ -874,7 +874,7 @@ static void free_all_descbuffers(struct b43_dmaring *ring)
for (i = 0; i < ring->nr_slots; i++) { for (i = 0; i < ring->nr_slots; i++) {
desc = ring->ops->idx2desc(ring, i, &meta); desc = ring->ops->idx2desc(ring, i, &meta);
if (!meta->skb) { if (!meta->skb || b43_dma_ptr_is_poisoned(meta->skb)) {
B43_WARN_ON(!ring->tx); B43_WARN_ON(!ring->tx);
continue; continue;
} }
...@@ -926,7 +926,7 @@ struct b43_dmaring *b43_setup_dmaring(struct b43_wldev *dev, ...@@ -926,7 +926,7 @@ struct b43_dmaring *b43_setup_dmaring(struct b43_wldev *dev,
enum b43_dmatype type) enum b43_dmatype type)
{ {
struct b43_dmaring *ring; struct b43_dmaring *ring;
int err; int i, err;
dma_addr_t dma_test; dma_addr_t dma_test;
ring = kzalloc(sizeof(*ring), GFP_KERNEL); ring = kzalloc(sizeof(*ring), GFP_KERNEL);
...@@ -941,6 +941,8 @@ struct b43_dmaring *b43_setup_dmaring(struct b43_wldev *dev, ...@@ -941,6 +941,8 @@ struct b43_dmaring *b43_setup_dmaring(struct b43_wldev *dev,
GFP_KERNEL); GFP_KERNEL);
if (!ring->meta) if (!ring->meta)
goto err_kfree_ring; goto err_kfree_ring;
for (i = 0; i < ring->nr_slots; i++)
ring->meta->skb = B43_DMA_PTR_POISON;
ring->type = type; ring->type = type;
ring->dev = dev; ring->dev = dev;
...@@ -1251,11 +1253,13 @@ struct b43_dmaring *parse_cookie(struct b43_wldev *dev, u16 cookie, int *slot) ...@@ -1251,11 +1253,13 @@ struct b43_dmaring *parse_cookie(struct b43_wldev *dev, u16 cookie, int *slot)
case 0x5000: case 0x5000:
ring = dma->tx_ring_mcast; ring = dma->tx_ring_mcast;
break; break;
default:
B43_WARN_ON(1);
} }
*slot = (cookie & 0x0FFF); *slot = (cookie & 0x0FFF);
B43_WARN_ON(!(ring && *slot >= 0 && *slot < ring->nr_slots)); if (unlikely(!ring || *slot < 0 || *slot >= ring->nr_slots)) {
b43dbg(dev->wl, "TX-status contains "
"invalid cookie: 0x%04X\n", cookie);
return NULL;
}
return ring; return ring;
} }
...@@ -1494,19 +1498,40 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev, ...@@ -1494,19 +1498,40 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev,
struct b43_dmaring *ring; struct b43_dmaring *ring;
struct b43_dmadesc_generic *desc; struct b43_dmadesc_generic *desc;
struct b43_dmadesc_meta *meta; struct b43_dmadesc_meta *meta;
int slot; int slot, firstused;
bool frame_succeed; bool frame_succeed;
ring = parse_cookie(dev, status->cookie, &slot); ring = parse_cookie(dev, status->cookie, &slot);
if (unlikely(!ring)) if (unlikely(!ring))
return; return;
B43_WARN_ON(!ring->tx); B43_WARN_ON(!ring->tx);
/* Sanity check: TX packets are processed in-order on one ring.
* Check if the slot deduced from the cookie really is the first
* used slot. */
firstused = ring->current_slot - ring->used_slots + 1;
if (firstused < 0)
firstused = ring->nr_slots + firstused;
if (unlikely(slot != firstused)) {
/* This possibly is a firmware bug and will result in
* malfunction, memory leaks and/or stall of DMA functionality. */
b43dbg(dev->wl, "Out of order TX status report on DMA ring %d. "
"Expected %d, but got %d\n",
ring->index, firstused, slot);
return;
}
ops = ring->ops; ops = ring->ops;
while (1) { while (1) {
B43_WARN_ON(!(slot >= 0 && slot < ring->nr_slots)); B43_WARN_ON(slot < 0 || slot >= ring->nr_slots);
desc = ops->idx2desc(ring, slot, &meta); desc = ops->idx2desc(ring, slot, &meta);
if (b43_dma_ptr_is_poisoned(meta->skb)) {
b43dbg(dev->wl, "Poisoned TX slot %d (first=%d) "
"on ring %d\n",
slot, firstused, ring->index);
break;
}
if (meta->skb) { if (meta->skb) {
struct b43_private_tx_info *priv_info = struct b43_private_tx_info *priv_info =
b43_get_priv_tx_info(IEEE80211_SKB_CB(meta->skb)); b43_get_priv_tx_info(IEEE80211_SKB_CB(meta->skb));
...@@ -1522,7 +1547,14 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev, ...@@ -1522,7 +1547,14 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev,
if (meta->is_last_fragment) { if (meta->is_last_fragment) {
struct ieee80211_tx_info *info; struct ieee80211_tx_info *info;
BUG_ON(!meta->skb); if (unlikely(!meta->skb)) {
/* This is a scatter-gather fragment of a frame, so
* the skb pointer must not be NULL. */
b43dbg(dev->wl, "TX status unexpected NULL skb "
"at slot %d (first=%d) on ring %d\n",
slot, firstused, ring->index);
break;
}
info = IEEE80211_SKB_CB(meta->skb); info = IEEE80211_SKB_CB(meta->skb);
...@@ -1540,20 +1572,29 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev, ...@@ -1540,20 +1572,29 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev,
#endif /* DEBUG */ #endif /* DEBUG */
ieee80211_tx_status(dev->wl->hw, meta->skb); ieee80211_tx_status(dev->wl->hw, meta->skb);
/* skb is freed by ieee80211_tx_status() */ /* skb will be freed by ieee80211_tx_status().
meta->skb = NULL; * Poison our pointer. */
meta->skb = B43_DMA_PTR_POISON;
} else { } else {
/* No need to call free_descriptor_buffer here, as /* No need to call free_descriptor_buffer here, as
* this is only the txhdr, which is not allocated. * this is only the txhdr, which is not allocated.
*/ */
B43_WARN_ON(meta->skb); if (unlikely(meta->skb)) {
b43dbg(dev->wl, "TX status unexpected non-NULL skb "
"at slot %d (first=%d) on ring %d\n",
slot, firstused, ring->index);
break;
}
} }
/* Everything unmapped and free'd. So it's not used anymore. */ /* Everything unmapped and free'd. So it's not used anymore. */
ring->used_slots--; ring->used_slots--;
if (meta->is_last_fragment) if (meta->is_last_fragment) {
/* This is the last scatter-gather
* fragment of the frame. We are done. */
break; break;
}
slot = next_slot(ring, slot); slot = next_slot(ring, slot);
} }
if (ring->stopped) { if (ring->stopped) {
......
#ifndef B43_DMA_H_ #ifndef B43_DMA_H_
#define B43_DMA_H_ #define B43_DMA_H_
#include <linux/ieee80211.h> #include <linux/err.h>
#include "b43.h" #include "b43.h"
...@@ -164,6 +164,10 @@ struct b43_dmadesc_generic { ...@@ -164,6 +164,10 @@ struct b43_dmadesc_generic {
#define B43_RXRING_SLOTS 64 #define B43_RXRING_SLOTS 64
#define B43_DMA0_RX_BUFFERSIZE IEEE80211_MAX_FRAME_LEN #define B43_DMA0_RX_BUFFERSIZE IEEE80211_MAX_FRAME_LEN
/* Pointer poison */
#define B43_DMA_PTR_POISON ((void *)ERR_PTR(-ENOMEM))
#define b43_dma_ptr_is_poisoned(ptr) (unlikely((ptr) == B43_DMA_PTR_POISON))
struct sk_buff; struct sk_buff;
struct b43_private; struct b43_private;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment