mirror of
https://github.com/AsahiLinux/u-boot
synced 2025-03-16 23:07:00 +00:00
net: macb: Fix race caused by flushing unwanted descriptors
The rx descriptor list is in cached memory, and there may be multiple descriptors per cache-line. After reclaim_rx_buffers marks a descriptor as unused it does a cache flush, which causes the entire cache-line to be written to memory, which may override other descriptors in the same cache-line that the controller may have written to. The fix skips freeing descriptors that are not the last in a cache-line, and if the freed descriptor is the last one in a cache-line, it marks all the descriptors in the cache-line as unused. This is similarly to what is done in drivers/net/fec_mxc.c In my case this bug caused tftpboot to fail some times when other packets are sent to u-boot in addition to the ongoing tftp (e.g. ping). The driver would stop receiving new packets because it is waiting on a descriptor that is marked unused, when in reality the descriptor contains a new unprocessed packet but while freeing the previous buffer descriptor & flushing the cache, the driver accidentally marked the descriptor as unused. Signed-off-by: Yaron Micher <yaronm@hailo.ai>
This commit is contained in:
parent
d6abc7e2e0
commit
d1559435d7
1 changed files with 39 additions and 12 deletions
|
@ -98,6 +98,9 @@ struct macb_dma_desc_64 {
|
|||
#define MACB_RX_DMA_DESC_SIZE (DMA_DESC_BYTES(MACB_RX_RING_SIZE))
|
||||
#define MACB_TX_DUMMY_DMA_DESC_SIZE (DMA_DESC_BYTES(1))
|
||||
|
||||
#define DESC_PER_CACHELINE_32 (ARCH_DMA_MINALIGN/sizeof(struct macb_dma_desc))
|
||||
#define DESC_PER_CACHELINE_64 (ARCH_DMA_MINALIGN/DMA_DESC_SIZE)
|
||||
|
||||
#define RXBUF_FRMLEN_MASK 0x00000fff
|
||||
#define TXBUF_FRMLEN_MASK 0x000007ff
|
||||
|
||||
|
@ -401,32 +404,56 @@ static int _macb_send(struct macb_device *macb, const char *name, void *packet,
|
|||
return 0;
|
||||
}
|
||||
|
||||
static void reclaim_rx_buffer(struct macb_device *macb,
|
||||
unsigned int idx)
|
||||
{
|
||||
unsigned int mask;
|
||||
unsigned int shift;
|
||||
unsigned int i;
|
||||
|
||||
/*
|
||||
* There may be multiple descriptors per CPU cacheline,
|
||||
* so a cache flush would flush the whole line, meaning the content of other descriptors
|
||||
* in the cacheline would also flush. If one of the other descriptors had been
|
||||
* written to by the controller, the flush would cause those changes to be lost.
|
||||
*
|
||||
* To circumvent this issue, we do the actual freeing only when we need to free
|
||||
* the last descriptor in the current cacheline. When the current descriptor is the
|
||||
* last in the cacheline, we free all the descriptors that belong to that cacheline.
|
||||
*/
|
||||
if (macb->config->hw_dma_cap & HW_DMA_CAP_64B) {
|
||||
mask = DESC_PER_CACHELINE_64 - 1;
|
||||
shift = 1;
|
||||
} else {
|
||||
mask = DESC_PER_CACHELINE_32 - 1;
|
||||
shift = 0;
|
||||
}
|
||||
|
||||
/* we exit without freeing if idx is not the last descriptor in the cacheline */
|
||||
if ((idx & mask) != mask)
|
||||
return;
|
||||
|
||||
for (i = idx & (~mask); i <= idx; i++)
|
||||
macb->rx_ring[i << shift].addr &= ~MACB_BIT(RX_USED);
|
||||
}
|
||||
|
||||
static void reclaim_rx_buffers(struct macb_device *macb,
|
||||
unsigned int new_tail)
|
||||
{
|
||||
unsigned int i;
|
||||
unsigned int count;
|
||||
|
||||
i = macb->rx_tail;
|
||||
|
||||
macb_invalidate_ring_desc(macb, RX);
|
||||
while (i > new_tail) {
|
||||
if (macb->config->hw_dma_cap & HW_DMA_CAP_64B)
|
||||
count = i * 2;
|
||||
else
|
||||
count = i;
|
||||
macb->rx_ring[count].addr &= ~MACB_BIT(RX_USED);
|
||||
reclaim_rx_buffer(macb, i);
|
||||
i++;
|
||||
if (i > MACB_RX_RING_SIZE)
|
||||
if (i >= MACB_RX_RING_SIZE)
|
||||
i = 0;
|
||||
}
|
||||
|
||||
while (i < new_tail) {
|
||||
if (macb->config->hw_dma_cap & HW_DMA_CAP_64B)
|
||||
count = i * 2;
|
||||
else
|
||||
count = i;
|
||||
macb->rx_ring[count].addr &= ~MACB_BIT(RX_USED);
|
||||
reclaim_rx_buffer(macb, i);
|
||||
i++;
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Reference in a new issue