Skip to content

Commit

Permalink
sh_eth: Ensure DMA engines are stopped before freeing buffers
Browse files Browse the repository at this point in the history
Currently we try to clear EDRRR and EDTRR and immediately continue to
free buffers.  This is unsafe because:

- In general, register writes are not serialised with DMA, so we still
  have to wait for DMA to complete somehow
- The R8A7790 (R-Car H2) manual states that the TX running flag cannot
  be cleared by writing to EDTRR
- The same manual states that clearing the RX running flag only stops
  RX DMA at the next packet boundary

I applied this patch to the driver to detect DMA writes to freed
buffers:

> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -1098,7 +1098,14 @@ static void sh_eth_ring_free(struct net_device *ndev)
>  	/* Free Rx skb ringbuffer */
>  	if (mdp->rx_skbuff) {
>  		for (i = 0; i < mdp->num_rx_ring; i++)
> +			memcpy(mdp->rx_skbuff[i]->data,
> +			       "Hello, world", 12);
> +		msleep(100);
> +		for (i = 0; i < mdp->num_rx_ring; i++) {
> +			WARN_ON(memcmp(mdp->rx_skbuff[i]->data,
> +				       "Hello, world", 12));
>  			dev_kfree_skb(mdp->rx_skbuff[i]);
> +		}
>  	}
>  	kfree(mdp->rx_skbuff);
>  	mdp->rx_skbuff = NULL;

then ran the loop:

    while ethtool -G eth0 rx 128 ; ethtool -G eth0 rx 64; do echo -n .; done

and 'ping -f' toward the sh_eth port from another machine.  The
warning fired several times a minute.

To fix these issues:

- Deactivate all TX descriptors rather than writing to EDTRR
- As there seems to be no way of telling when RX DMA is stopped,
  perform a soft reset to ensure that both DMA enginess are stopped
- To reduce the possibility of the reset truncating a transmitted
  frame, disable egress and wait a reasonable time to reach a
  packet boundary before resetting
- Update statistics before resetting

(The 'reasonable time' does not allow for CS/CD in half-duplex
mode, but half-duplex no longer seems reasonable!)

Signed-off-by: Ben Hutchings <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
bwh-ct authored and davem330 committed Jan 27, 2015
1 parent dc1d0e6 commit 740c7f3
Showing 1 changed file with 32 additions and 7 deletions.
39 changes: 32 additions & 7 deletions drivers/net/ethernet/renesas/sh_eth.c
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,9 @@ static const u16 sh_eth_offset_fast_sh3_sh2[SH_ETH_MAX_REGISTER_OFFSET] = {
[TSU_ADRL31] = 0x01fc,
};

static void sh_eth_rcv_snd_disable(struct net_device *ndev);
static struct net_device_stats *sh_eth_get_stats(struct net_device *ndev);

static bool sh_eth_is_gether(struct sh_eth_private *mdp)
{
return mdp->reg_offset == sh_eth_offset_gigabit;
Expand Down Expand Up @@ -1358,6 +1361,33 @@ static int sh_eth_dev_init(struct net_device *ndev, bool start)
return ret;
}

static void sh_eth_dev_exit(struct net_device *ndev)
{
struct sh_eth_private *mdp = netdev_priv(ndev);
int i;

/* Deactivate all TX descriptors, so DMA should stop at next
* packet boundary if it's currently running
*/
for (i = 0; i < mdp->num_tx_ring; i++)
mdp->tx_ring[i].status &= ~cpu_to_edmac(mdp, TD_TACT);

/* Disable TX FIFO egress to MAC */
sh_eth_rcv_snd_disable(ndev);

/* Stop RX DMA at next packet boundary */
sh_eth_write(ndev, 0, EDRRR);

/* Aside from TX DMA, we can't tell when the hardware is
* really stopped, so we need to reset to make sure.
* Before doing that, wait for long enough to *probably*
* finish transmitting the last packet and poll stats.
*/
msleep(2); /* max frame time at 10 Mbps < 1250 us */
sh_eth_get_stats(ndev);
sh_eth_reset(ndev);
}

/* free Tx skb function */
static int sh_eth_txfree(struct net_device *ndev)
{
Expand Down Expand Up @@ -1986,9 +2016,7 @@ static int sh_eth_set_ringparam(struct net_device *ndev,
napi_synchronize(&mdp->napi);
sh_eth_write(ndev, 0x0000, EESIPR);

/* Stop the chip's Tx and Rx processes. */
sh_eth_write(ndev, 0, EDTRR);
sh_eth_write(ndev, 0, EDRRR);
sh_eth_dev_exit(ndev);

/* Free all the skbuffs in the Rx queue. */
sh_eth_ring_free(ndev);
Expand Down Expand Up @@ -2207,11 +2235,8 @@ static int sh_eth_close(struct net_device *ndev)
napi_disable(&mdp->napi);
sh_eth_write(ndev, 0x0000, EESIPR);

/* Stop the chip's Tx and Rx processes. */
sh_eth_write(ndev, 0, EDTRR);
sh_eth_write(ndev, 0, EDRRR);
sh_eth_dev_exit(ndev);

sh_eth_get_stats(ndev);
/* PHY Disconnect */
if (mdp->phydev) {
phy_stop(mdp->phydev);
Expand Down

0 comments on commit 740c7f3

Please sign in to comment.