[PATCH net-next v2 00/10] amd-xgbe: AMD XGBE driver updates 2015-03-19

[PATCH net-next v2 00/10] amd-xgbe: AMD XGBE driver updates 2015-03-19

[Tom Lendacky]

The following series of patches includes functional updates and changes
to the driver.

- Use the phydev->advertising field instead of the phydev->supported
  field when configuring for auto-negotiation, etc.
- Use the phy_driver flags field for setting the transceiver type
  instead of hardcoding it in the ethtool support.
- Provide an auto-negotiation timeout check
- Clarify the Tx/Rx queue information messages
- Use the new DMA memory barrier operations
- Set the device DMA mask based on what the hardware reports
- Remove the software implementation of Tx coalescing
- Fix the reporting of the Rx coalescing value
- Use napi_alloc_skb when allocating an SKB in softirq
- Rework the Rx path SKB allocation to make it cleaner and be able
  to possibly inline more data

This patch series is based on net-next.

---

Changes from v1:
- Default to 32-bit DMA width (minimum supported) if hardware returns
  an unexpected DMA width value

Tom Lendacky (10):
      amd-xgbe-phy: Use phydev advertising field vs supported
      amd-xgbe-phy: Use the phy_driver flags field
      amd-xgbe-phy: Provide support for auto-negotiation timeout
      amd-xgbe: Clarify output message about queues
      amd-xgbe: Use the new DMA memory barriers where appropriate
      amd-xgbe: Set DMA mask based on hardware register value
      amd-xgbe: Remove Tx coalescing
      amd-xgbe: Fix Rx coalescing reporting
      amd-xgbe: Use napi_alloc_skb when allocating skb in softirq
      amd-xgbe: Rework the Rx path SKB allocation


 drivers/net/ethernet/amd/xgbe/xgbe-common.h  |   16 ++-
 drivers/net/ethernet/amd/xgbe/xgbe-desc.c    |    2 
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c     |   45 +++++----
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c     |  124 +++++++++++++++++---------
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c |   19 +---
 drivers/net/ethernet/amd/xgbe/xgbe-main.c    |   19 ++--
 drivers/net/ethernet/amd/xgbe/xgbe.h         |    8 +-
 drivers/net/phy/amd-xgbe-phy.c               |   98 ++++++++++++---------
 8 files changed, 197 insertions(+), 134 deletions(-)

-- 
Tom Lendacky

[PATCH net-next v2 01/10] amd-xgbe-phy: Use phydev advertising field vs supported

[Tom Lendacky]

With ethtool being able to control what is advertised, the advertising
field is what should be used for priming the auto-negotiation registers
and for various other checks, instead of the supported field.

Also, move the initial setting of the supported and advertising fields
into the probe function so that they are not reset each time the device
is brought up, thus allowing the user to set as desired before bringing
the device up.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/phy/amd-xgbe-phy.c |   77 ++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 39 deletions(-)

diff --git a/drivers/net/phy/amd-xgbe-phy.c b/drivers/net/phy/amd-xgbe-phy.c
index 32efbd4..acb4449 100644
--- a/drivers/net/phy/amd-xgbe-phy.c
+++ b/drivers/net/phy/amd-xgbe-phy.c
@@ -932,8 +932,8 @@ static enum amd_xgbe_phy_an amd_xgbe_an_incompat_link(struct phy_device *phydev)
 	if (amd_xgbe_phy_in_kr_mode(phydev)) {
 		priv->kr_state = AMD_XGBE_RX_ERROR;
 
-		if (!(phydev->supported & SUPPORTED_1000baseKX_Full) &&
-		    !(phydev->supported & SUPPORTED_2500baseX_Full))
+		if (!(phydev->advertising & SUPPORTED_1000baseKX_Full) &&
+		    !(phydev->advertising & SUPPORTED_2500baseX_Full))
 			return AMD_XGBE_AN_NO_LINK;
 
 		if (priv->kx_state != AMD_XGBE_RX_BPA)
@@ -941,7 +941,7 @@ static enum amd_xgbe_phy_an amd_xgbe_an_incompat_link(struct phy_device *phydev)
 	} else {
 		priv->kx_state = AMD_XGBE_RX_ERROR;
 
-		if (!(phydev->supported & SUPPORTED_10000baseKR_Full))
+		if (!(phydev->advertising & SUPPORTED_10000baseKR_Full))
 			return AMD_XGBE_AN_NO_LINK;
 
 		if (priv->kr_state != AMD_XGBE_RX_BPA)
@@ -1101,7 +1101,7 @@ static int amd_xgbe_an_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	if (phydev->supported & SUPPORTED_10000baseR_FEC)
+	if (phydev->advertising & SUPPORTED_10000baseR_FEC)
 		ret |= 0xc000;
 	else
 		ret &= ~0xc000;
@@ -1113,13 +1113,13 @@ static int amd_xgbe_an_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	if (phydev->supported & SUPPORTED_10000baseKR_Full)
+	if (phydev->advertising & SUPPORTED_10000baseKR_Full)
 		ret |= 0x80;
 	else
 		ret &= ~0x80;
 
-	if ((phydev->supported & SUPPORTED_1000baseKX_Full) ||
-	    (phydev->supported & SUPPORTED_2500baseX_Full))
+	if ((phydev->advertising & SUPPORTED_1000baseKX_Full) ||
+	    (phydev->advertising & SUPPORTED_2500baseX_Full))
 		ret |= 0x20;
 	else
 		ret &= ~0x20;
@@ -1131,12 +1131,12 @@ static int amd_xgbe_an_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	if (phydev->supported & SUPPORTED_Pause)
+	if (phydev->advertising & SUPPORTED_Pause)
 		ret |= 0x400;
 	else
 		ret &= ~0x400;
 
-	if (phydev->supported & SUPPORTED_Asym_Pause)
+	if (phydev->advertising & SUPPORTED_Asym_Pause)
 		ret |= 0x800;
 	else
 		ret &= ~0x800;
@@ -1212,38 +1212,14 @@ static int amd_xgbe_phy_config_init(struct phy_device *phydev)
 		priv->an_irq_allocated = 1;
 	}
 
-	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_10GBR_FEC_ABILITY);
-	if (ret < 0)
-		return ret;
-	priv->fec_ability = ret & XGBE_PHY_FEC_MASK;
-
-	/* Initialize supported features */
-	phydev->supported = SUPPORTED_Autoneg;
-	phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
-	phydev->supported |= SUPPORTED_Backplane;
-	phydev->supported |= SUPPORTED_10000baseKR_Full;
-	switch (priv->speed_set) {
-	case AMD_XGBE_PHY_SPEEDSET_1000_10000:
-		phydev->supported |= SUPPORTED_1000baseKX_Full;
-		break;
-	case AMD_XGBE_PHY_SPEEDSET_2500_10000:
-		phydev->supported |= SUPPORTED_2500baseX_Full;
-		break;
-	}
-
-	if (priv->fec_ability & XGBE_PHY_FEC_ENABLE)
-		phydev->supported |= SUPPORTED_10000baseR_FEC;
-
-	phydev->advertising = phydev->supported;
-
 	/* Set initial mode - call the mode setting routines
 	 * directly to insure we are properly configured
 	 */
-	if (phydev->supported & SUPPORTED_10000baseKR_Full)
+	if (phydev->advertising & SUPPORTED_10000baseKR_Full)
 		ret = amd_xgbe_phy_xgmii_mode(phydev);
-	else if (phydev->supported & SUPPORTED_1000baseKX_Full)
+	else if (phydev->advertising & SUPPORTED_1000baseKX_Full)
 		ret = amd_xgbe_phy_gmii_mode(phydev);
-	else if (phydev->supported & SUPPORTED_2500baseX_Full)
+	else if (phydev->advertising & SUPPORTED_2500baseX_Full)
 		ret = amd_xgbe_phy_gmii_2500_mode(phydev);
 	else
 		ret = -EINVAL;
@@ -1315,10 +1291,10 @@ static int __amd_xgbe_phy_config_aneg(struct phy_device *phydev)
 	disable_irq(priv->an_irq);
 
 	/* Start auto-negotiation in a supported mode */
-	if (phydev->supported & SUPPORTED_10000baseKR_Full)
+	if (phydev->advertising & SUPPORTED_10000baseKR_Full)
 		ret = amd_xgbe_phy_set_mode(phydev, AMD_XGBE_MODE_KR);
-	else if ((phydev->supported & SUPPORTED_1000baseKX_Full) ||
-		 (phydev->supported & SUPPORTED_2500baseX_Full))
+	else if ((phydev->advertising & SUPPORTED_1000baseKX_Full) ||
+		 (phydev->advertising & SUPPORTED_2500baseX_Full))
 		ret = amd_xgbe_phy_set_mode(phydev, AMD_XGBE_MODE_KX);
 	else
 		ret = -EINVAL;
@@ -1746,6 +1722,29 @@ static int amd_xgbe_phy_probe(struct phy_device *phydev)
 		       sizeof(priv->serdes_dfe_tap_ena));
 	}
 
+	/* Initialize supported features */
+	phydev->supported = SUPPORTED_Autoneg;
+	phydev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+	phydev->supported |= SUPPORTED_Backplane;
+	phydev->supported |= SUPPORTED_10000baseKR_Full;
+	switch (priv->speed_set) {
+	case AMD_XGBE_PHY_SPEEDSET_1000_10000:
+		phydev->supported |= SUPPORTED_1000baseKX_Full;
+		break;
+	case AMD_XGBE_PHY_SPEEDSET_2500_10000:
+		phydev->supported |= SUPPORTED_2500baseX_Full;
+		break;
+	}
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_10GBR_FEC_ABILITY);
+	if (ret < 0)
+		return ret;
+	priv->fec_ability = ret & XGBE_PHY_FEC_MASK;
+	if (priv->fec_ability & XGBE_PHY_FEC_ENABLE)
+		phydev->supported |= SUPPORTED_10000baseR_FEC;
+
+	phydev->advertising = phydev->supported;
+
 	phydev->priv = priv;
 
 	if (!priv->adev || acpi_disabled)

[PATCH net-next v2 02/10] amd-xgbe-phy: Use the phy_driver flags field

[Tom Lendacky]

Remove the setting of the transceiver type when retrieving the device
settings using ethtool and instead set the transceiver type in the
phy_driver structure flags field. Change the transceiver type to be
internal, also.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c |    1 -
 drivers/net/phy/amd-xgbe-phy.c               |    1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
index ebf4893..43e5571 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
@@ -291,7 +291,6 @@ static int xgbe_get_settings(struct net_device *netdev,
 		return -ENODEV;
 
 	ret = phy_ethtool_gset(pdata->phydev, cmd);
-	cmd->transceiver = XCVR_EXTERNAL;
 
 	DBGPR("<--xgbe_get_settings\n");
 
diff --git a/drivers/net/phy/amd-xgbe-phy.c b/drivers/net/phy/amd-xgbe-phy.c
index acb4449..4b95caf 100644
--- a/drivers/net/phy/amd-xgbe-phy.c
+++ b/drivers/net/phy/amd-xgbe-phy.c
@@ -1816,6 +1816,7 @@ static struct phy_driver amd_xgbe_phy_driver[] = {
 		.phy_id_mask		= XGBE_PHY_MASK,
 		.name			= "AMD XGBE PHY",
 		.features		= 0,
+		.flags			= PHY_IS_INTERNAL,
 		.probe			= amd_xgbe_phy_probe,
 		.remove			= amd_xgbe_phy_remove,
 		.soft_reset		= amd_xgbe_phy_soft_reset,

[PATCH net-next v2 03/10] amd-xgbe-phy: Provide support for auto-negotiation timeout

[Tom Lendacky]

Currently, there is no interrupt code that indicates auto-negotiation
has timed out. If the auto-negotiation has timed out then the start of
a new auto-negotiation will begin again with a new base page being
received. The state machine could be in a state that is not expecting
this interrupt code which results in an error during auto-negotiation.

Update the code to timestamp when the auto-negotiation starts.  Should
another page received interrupt code occur before auto-negotiation has
completed but after the auto-negotiation timeout, then reset the state
machine to allow the auto-negotiation to continue.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/phy/amd-xgbe-phy.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/phy/amd-xgbe-phy.c b/drivers/net/phy/amd-xgbe-phy.c
index 4b95caf..823b316 100644
--- a/drivers/net/phy/amd-xgbe-phy.c
+++ b/drivers/net/phy/amd-xgbe-phy.c
@@ -78,6 +78,7 @@
 #include <linux/bitops.h>
 #include <linux/property.h>
 #include <linux/acpi.h>
+#include <linux/time.h>
 
 MODULE_AUTHOR("Tom Lendacky <thomas.lendacky@amd.com>");
 MODULE_LICENSE("Dual BSD/GPL");
@@ -100,6 +101,8 @@ MODULE_DESCRIPTION("AMD 10GbE (amd-xgbe) PHY driver");
 #define XGBE_PHY_SPEED_2500		1
 #define XGBE_PHY_SPEED_10000		2
 
+#define XGBE_AN_NS_TIMEOUT		(500 * NSEC_PER_MSEC)	/* 500 ms */
+
 #define XGBE_AN_INT_CMPLT		0x01
 #define XGBE_AN_INC_LINK		0x02
 #define XGBE_AN_PG_RCV			0x04
@@ -434,6 +437,7 @@ struct amd_xgbe_phy_priv {
 	unsigned int an_supported;
 	unsigned int parallel_detect;
 	unsigned int fec_ability;
+	struct timespec64 an_start;
 
 	unsigned int lpm_ctrl;		/* CTRL1 for resume */
 };
@@ -902,8 +906,23 @@ static enum amd_xgbe_phy_an amd_xgbe_an_page_received(struct phy_device *phydev)
 {
 	struct amd_xgbe_phy_priv *priv = phydev->priv;
 	enum amd_xgbe_phy_rx *state;
+	struct timespec64 rcv_time, diff_time;
 	int ret;
 
+	getnstimeofday64(&rcv_time);
+	if (!timespec64_to_ns(&priv->an_start)) {
+		priv->an_start = rcv_time;
+	} else {
+		diff_time = timespec64_sub(rcv_time, priv->an_start);
+		if (timespec64_to_ns(&diff_time) > XGBE_AN_NS_TIMEOUT) {
+			/* Auto-negotiation timed out, reset state */
+			priv->kr_state = AMD_XGBE_RX_BPA;
+			priv->kx_state = AMD_XGBE_RX_BPA;
+
+			priv->an_start = rcv_time;
+		}
+	}
+
 	state = amd_xgbe_phy_in_kr_mode(phydev) ? &priv->kr_state
 						: &priv->kx_state;
 
@@ -1078,6 +1097,7 @@ again:
 		priv->an_state = AMD_XGBE_AN_READY;
 		priv->kr_state = AMD_XGBE_RX_BPA;
 		priv->kx_state = AMD_XGBE_RX_BPA;
+		priv->an_start = ns_to_timespec64(0);
 	}
 
 	if (cur_state != priv->an_state)

Re: [PATCH net-next v2 03/10] amd-xgbe-phy: Provide support for auto-negotiation timeout

[David Miller]

From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Thu, 19 Mar 2015 15:08:26 -0500

> @@ -902,8 +906,23 @@ static enum amd_xgbe_phy_an amd_xgbe_an_page_received(struct phy_device *phydev)
>  {
>  	struct amd_xgbe_phy_priv *priv = phydev->priv;
>  	enum amd_xgbe_phy_rx *state;
> +	struct timespec64 rcv_time, diff_time;
>  	int ret;
>  
> +	getnstimeofday64(&rcv_time);
> +	if (!timespec64_to_ns(&priv->an_start)) {
> +		priv->an_start = rcv_time;
> +	} else {
> +		diff_time = timespec64_sub(rcv_time, priv->an_start);
> +		if (timespec64_to_ns(&diff_time) > XGBE_AN_NS_TIMEOUT) {
> +			/* Auto-negotiation timed out, reset state */
> +			priv->kr_state = AMD_XGBE_RX_BPA;
> +			priv->kx_state = AMD_XGBE_RX_BPA;
> +
> +			priv->an_start = rcv_time;
> +		}
> +	}
> +

timespec is a little bit heavyweight for something like this, you just
want to snapshot a point in time then later check if a certain number
of ms have passed or not.

For that, plain 'jiffies' is sufficient.

Re: [PATCH net-next v2 03/10] amd-xgbe-phy: Provide support for auto-negotiation timeout

[Tom Lendacky]

On 03/19/2015 03:16 PM, David Miller wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Date: Thu, 19 Mar 2015 15:08:26 -0500
>
>> @@ -902,8 +906,23 @@ static enum amd_xgbe_phy_an amd_xgbe_an_page_received(struct phy_device *phydev)
>>   {
>>   	struct amd_xgbe_phy_priv *priv = phydev->priv;
>>   	enum amd_xgbe_phy_rx *state;
>> +	struct timespec64 rcv_time, diff_time;
>>   	int ret;
>>
>> +	getnstimeofday64(&rcv_time);
>> +	if (!timespec64_to_ns(&priv->an_start)) {
>> +		priv->an_start = rcv_time;
>> +	} else {
>> +		diff_time = timespec64_sub(rcv_time, priv->an_start);
>> +		if (timespec64_to_ns(&diff_time) > XGBE_AN_NS_TIMEOUT) {
>> +			/* Auto-negotiation timed out, reset state */
>> +			priv->kr_state = AMD_XGBE_RX_BPA;
>> +			priv->kx_state = AMD_XGBE_RX_BPA;
>> +
>> +			priv->an_start = rcv_time;
>> +		}
>> +	}
>> +
>
> timespec is a little bit heavyweight for something like this, you just
> want to snapshot a point in time then later check if a certain number
> of ms have passed or not.
>
> For that, plain 'jiffies' is sufficient.
>

Sounds good, I'll rework it to use jiffies.

Thanks,
Tom

[PATCH net-next v2 04/10] amd-xgbe: Clarify output message about queues

[Tom Lendacky]

Clarify that the queues referred to in a message when the device is
brought up are hardware queues and not necessarily related to the
Linux network queues.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index 400757b..29137c9 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -2004,7 +2004,8 @@ static void xgbe_config_tx_fifo_size(struct xgbe_prv_data *pdata)
 	for (i = 0; i < pdata->tx_q_count; i++)
 		XGMAC_MTL_IOWRITE_BITS(pdata, i, MTL_Q_TQOMR, TQS, fifo_size);
 
-	netdev_notice(pdata->netdev, "%d Tx queues, %d byte fifo per queue\n",
+	netdev_notice(pdata->netdev,
+		      "%d Tx hardware queues, %d byte fifo per queue\n",
 		      pdata->tx_q_count, ((fifo_size + 1) * 256));
 }
 
@@ -2019,7 +2020,8 @@ static void xgbe_config_rx_fifo_size(struct xgbe_prv_data *pdata)
 	for (i = 0; i < pdata->rx_q_count; i++)
 		XGMAC_MTL_IOWRITE_BITS(pdata, i, MTL_Q_RQOMR, RQS, fifo_size);
 
-	netdev_notice(pdata->netdev, "%d Rx queues, %d byte fifo per queue\n",
+	netdev_notice(pdata->netdev,
+		      "%d Rx hardware queues, %d byte fifo per queue\n",
 		      pdata->rx_q_count, ((fifo_size + 1) * 256));
 }
 

[PATCH net-next v2 05/10] amd-xgbe: Use the new DMA memory barriers where appropriate

[Tom Lendacky]

Use the new lighter weight memory barriers when working with the device
descriptors.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c |   15 +++++++++------
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c |    5 ++++-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index 29137c9..e165d06 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -1068,7 +1068,7 @@ static void xgbe_tx_desc_reset(struct xgbe_ring_data *rdata)
 	rdesc->desc3 = 0;
 
 	/* Make sure ownership is written to the descriptor */
-	wmb();
+	dma_wmb();
 }
 
 static void xgbe_tx_desc_init(struct xgbe_channel *channel)
@@ -1124,12 +1124,12 @@ static void xgbe_rx_desc_reset(struct xgbe_ring_data *rdata)
 	 * is written to the descriptor(s) before setting the OWN bit
 	 * for the descriptor
 	 */
-	wmb();
+	dma_wmb();
 
 	XGMAC_SET_BITS_LE(rdesc->desc3, RX_NORMAL_DESC3, OWN, 1);
 
 	/* Make sure ownership is written to the descriptor */
-	wmb();
+	dma_wmb();
 }
 
 static void xgbe_rx_desc_init(struct xgbe_channel *channel)
@@ -1358,6 +1358,9 @@ static void xgbe_tx_start_xmit(struct xgbe_channel *channel,
 	struct xgbe_prv_data *pdata = channel->pdata;
 	struct xgbe_ring_data *rdata;
 
+	/* Make sure everything is written before the register write */
+	wmb();
+
 	/* Issue a poll command to Tx DMA by writing address
 	 * of next immediate free descriptor */
 	rdata = XGBE_GET_DESC_DATA(ring, ring->cur);
@@ -1565,7 +1568,7 @@ static void xgbe_dev_xmit(struct xgbe_channel *channel)
 	 * is written to the descriptor(s) before setting the OWN bit
 	 * for the first descriptor
 	 */
-	wmb();
+	dma_wmb();
 
 	/* Set OWN bit for the first descriptor */
 	rdata = XGBE_GET_DESC_DATA(ring, start_index);
@@ -1577,7 +1580,7 @@ static void xgbe_dev_xmit(struct xgbe_channel *channel)
 #endif
 
 	/* Make sure ownership is written to the descriptor */
-	wmb();
+	dma_wmb();
 
 	ring->cur = cur_index + 1;
 	if (!packet->skb->xmit_more ||
@@ -1613,7 +1616,7 @@ static int xgbe_dev_read(struct xgbe_channel *channel)
 		return 1;
 
 	/* Make sure descriptor fields are read after reading the OWN bit */
-	rmb();
+	dma_rmb();
 
 #ifdef XGMAC_ENABLE_RX_DESC_DUMP
 	xgbe_dump_rx_desc(ring, rdesc, ring->cur);
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 885b02b..8635c94 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -1800,6 +1800,9 @@ static void xgbe_rx_refresh(struct xgbe_channel *channel)
 		ring->dirty++;
 	}
 
+	/* Make sure everything is written before the register write */
+	wmb();
+
 	/* Update the Rx Tail Pointer Register with address of
 	 * the last cleaned entry */
 	rdata = XGBE_GET_DESC_DATA(ring, ring->dirty - 1);
@@ -1863,7 +1866,7 @@ static int xgbe_tx_poll(struct xgbe_channel *channel)
 
 		/* Make sure descriptor fields are read after reading the OWN
 		 * bit */
-		rmb();
+		dma_rmb();
 
 #ifdef XGMAC_ENABLE_TX_DESC_DUMP
 		xgbe_dump_tx_desc(ring, ring->dirty, 1, 0);

[PATCH net-next v2 06/10] amd-xgbe: Set DMA mask based on hardware register value

[Tom Lendacky]

The hardware supplies a value that indicates the DMA range that it
is capable of using. Use this value rather than hard-coding it in
the driver.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-common.h |    2 ++
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c    |   16 ++++++++++++++++
 drivers/net/ethernet/amd/xgbe/xgbe-main.c   |   19 ++++++++++---------
 drivers/net/ethernet/amd/xgbe/xgbe.h        |    1 +
 4 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
index 29a0927..34c28aa 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
@@ -365,6 +365,8 @@
 #define MAC_HWF0R_TXCOESEL_WIDTH	1
 #define MAC_HWF0R_VLHASH_INDEX		4
 #define MAC_HWF0R_VLHASH_WIDTH		1
+#define MAC_HWF1R_ADDR64_INDEX		14
+#define MAC_HWF1R_ADDR64_WIDTH		2
 #define MAC_HWF1R_ADVTHWORD_INDEX	13
 #define MAC_HWF1R_ADVTHWORD_WIDTH	1
 #define MAC_HWF1R_DBGMEMA_INDEX		19
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 8635c94..ef4625e 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -519,6 +519,7 @@ void xgbe_get_all_hw_features(struct xgbe_prv_data *pdata)
 						RXFIFOSIZE);
 	hw_feat->tx_fifo_size  = XGMAC_GET_BITS(mac_hfr1, MAC_HWF1R,
 						TXFIFOSIZE);
+	hw_feat->dma_width     = XGMAC_GET_BITS(mac_hfr1, MAC_HWF1R, ADDR64);
 	hw_feat->dcb           = XGMAC_GET_BITS(mac_hfr1, MAC_HWF1R, DCBEN);
 	hw_feat->sph           = XGMAC_GET_BITS(mac_hfr1, MAC_HWF1R, SPHEN);
 	hw_feat->tso           = XGMAC_GET_BITS(mac_hfr1, MAC_HWF1R, TSOEN);
@@ -553,6 +554,21 @@ void xgbe_get_all_hw_features(struct xgbe_prv_data *pdata)
 		break;
 	}
 
+	/* Translate the address width setting into actual number */
+	switch (hw_feat->dma_width) {
+	case 0:
+		hw_feat->dma_width = 32;
+		break;
+	case 1:
+		hw_feat->dma_width = 40;
+		break;
+	case 2:
+		hw_feat->dma_width = 48;
+		break;
+	default:
+		hw_feat->dma_width = 32;
+	}
+
 	/* The Queue, Channel and TC counts are zero based so increment them
 	 * to get the actual number
 	 */
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-main.c b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
index 32dd651..2e4c22d 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-main.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
@@ -374,15 +374,6 @@ static int xgbe_probe(struct platform_device *pdev)
 		pdata->awcache = XGBE_DMA_SYS_AWCACHE;
 	}
 
-	/* Set the DMA mask */
-	if (!dev->dma_mask)
-		dev->dma_mask = &dev->coherent_dma_mask;
-	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
-	if (ret) {
-		dev_err(dev, "dma_set_mask_and_coherent failed\n");
-		goto err_io;
-	}
-
 	/* Get the device interrupt */
 	ret = platform_get_irq(pdev, 0);
 	if (ret < 0) {
@@ -409,6 +400,16 @@ static int xgbe_probe(struct platform_device *pdev)
 	/* Set default configuration data */
 	xgbe_default_config(pdata);
 
+	/* Set the DMA mask */
+	if (!dev->dma_mask)
+		dev->dma_mask = &dev->coherent_dma_mask;
+	ret = dma_set_mask_and_coherent(dev,
+					DMA_BIT_MASK(pdata->hw_feat.dma_width));
+	if (ret) {
+		dev_err(dev, "dma_set_mask_and_coherent failed\n");
+		goto err_io;
+	}
+
 	/* Calculate the number of Tx and Rx rings to be created
 	 *  -Tx (DMA) Channels map 1-to-1 to Tx Queues so set
 	 *   the number of Tx queues to the number of Tx channels
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index 1eea3e5..bfe11fb 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -632,6 +632,7 @@ struct xgbe_hw_features {
 	unsigned int rx_fifo_size;	/* MTL Receive FIFO Size */
 	unsigned int tx_fifo_size;	/* MTL Transmit FIFO Size */
 	unsigned int adv_ts_hi;		/* Advance Timestamping High Word */
+	unsigned int dma_width;		/* DMA width */
 	unsigned int dcb;		/* DCB Feature */
 	unsigned int sph;		/* Split Header Feature */
 	unsigned int tso;		/* TCP Segmentation Offload */

[PATCH net-next v2 07/10] amd-xgbe: Remove Tx coalescing

[Tom Lendacky]

The Tx coalescing support in the driver was a software implementation
for something lacking in the hardware. Using hrtimers, the idea was to
trigger a timer interrupt after having queued a packet for transmit.
Unfortunately, as the timer value was lowered, the timer expired before
the hardware actually did the transmit and so it was racey and resulted
in unnecessary interrupts.

Remove the Tx coalescing support and hrtimer and replace with a Tx timer
that is used as a reclaim timer in case of inactivity.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c     |    7 +++----
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c     |   16 +++++-----------
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c |    6 ++----
 drivers/net/ethernet/amd/xgbe/xgbe.h         |    4 ++--
 4 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index e165d06..80dd7a9 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -1367,12 +1367,11 @@ static void xgbe_tx_start_xmit(struct xgbe_channel *channel,
 	XGMAC_DMA_IOWRITE(channel, DMA_CH_TDTR_LO,
 			  lower_32_bits(rdata->rdesc_dma));
 
-	/* Start the Tx coalescing timer */
+	/* Start the Tx timer */
 	if (pdata->tx_usecs && !channel->tx_timer_active) {
 		channel->tx_timer_active = 1;
-		hrtimer_start(&channel->tx_timer,
-			      ktime_set(0, pdata->tx_usecs * NSEC_PER_USEC),
-			      HRTIMER_MODE_REL);
+		mod_timer(&channel->tx_timer,
+			  jiffies + usecs_to_jiffies(pdata->tx_usecs));
 	}
 
 	ring->tx.xmit_more = 0;
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index ef4625e..122a631 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -411,11 +411,9 @@ static irqreturn_t xgbe_dma_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static enum hrtimer_restart xgbe_tx_timer(struct hrtimer *timer)
+static void xgbe_tx_timer(unsigned long data)
 {
-	struct xgbe_channel *channel = container_of(timer,
-						    struct xgbe_channel,
-						    tx_timer);
+	struct xgbe_channel *channel = (struct xgbe_channel *)data;
 	struct xgbe_prv_data *pdata = channel->pdata;
 	struct napi_struct *napi;
 
@@ -437,8 +435,6 @@ static enum hrtimer_restart xgbe_tx_timer(struct hrtimer *timer)
 	channel->tx_timer_active = 0;
 
 	DBGPR("<--xgbe_tx_timer\n");
-
-	return HRTIMER_NORESTART;
 }
 
 static void xgbe_init_tx_timers(struct xgbe_prv_data *pdata)
@@ -454,9 +450,8 @@ static void xgbe_init_tx_timers(struct xgbe_prv_data *pdata)
 			break;
 
 		DBGPR("  %s adding tx timer\n", channel->name);
-		hrtimer_init(&channel->tx_timer, CLOCK_MONOTONIC,
-			     HRTIMER_MODE_REL);
-		channel->tx_timer.function = xgbe_tx_timer;
+		setup_timer(&channel->tx_timer, xgbe_tx_timer,
+			    (unsigned long)channel);
 	}
 
 	DBGPR("<--xgbe_init_tx_timers\n");
@@ -475,8 +470,7 @@ static void xgbe_stop_tx_timers(struct xgbe_prv_data *pdata)
 			break;
 
 		DBGPR("  %s deleting tx timer\n", channel->name);
-		channel->tx_timer_active = 0;
-		hrtimer_cancel(&channel->tx_timer);
+		del_timer_sync(&channel->tx_timer);
 	}
 
 	DBGPR("<--xgbe_stop_tx_timers\n");
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
index 43e5571..376d6a5 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
@@ -388,7 +388,6 @@ static int xgbe_get_coalesce(struct net_device *netdev,
 	ec->rx_coalesce_usecs = hw_if->riwt_to_usec(pdata, riwt);
 	ec->rx_max_coalesced_frames = pdata->rx_frames;
 
-	ec->tx_coalesce_usecs = pdata->tx_usecs;
 	ec->tx_max_coalesced_frames = pdata->tx_frames;
 
 	DBGPR("<--xgbe_get_coalesce\n");
@@ -402,13 +401,14 @@ static int xgbe_set_coalesce(struct net_device *netdev,
 	struct xgbe_prv_data *pdata = netdev_priv(netdev);
 	struct xgbe_hw_if *hw_if = &pdata->hw_if;
 	unsigned int rx_frames, rx_riwt, rx_usecs;
-	unsigned int tx_frames, tx_usecs;
+	unsigned int tx_frames;
 
 	DBGPR("-->xgbe_set_coalesce\n");
 
 	/* Check for not supported parameters  */
 	if ((ec->rx_coalesce_usecs_irq) ||
 	    (ec->rx_max_coalesced_frames_irq) ||
+	    (ec->tx_coalesce_usecs) ||
 	    (ec->tx_coalesce_usecs_irq) ||
 	    (ec->tx_max_coalesced_frames_irq) ||
 	    (ec->stats_block_coalesce_usecs) ||
@@ -457,7 +457,6 @@ static int xgbe_set_coalesce(struct net_device *netdev,
 		return -EINVAL;
 	}
 
-	tx_usecs = ec->tx_coalesce_usecs;
 	tx_frames = ec->tx_max_coalesced_frames;
 
 	/* Check the bounds of values for Tx */
@@ -471,7 +470,6 @@ static int xgbe_set_coalesce(struct net_device *netdev,
 	pdata->rx_frames = rx_frames;
 	hw_if->config_rx_coalesce(pdata);
 
-	pdata->tx_usecs = tx_usecs;
 	pdata->tx_frames = tx_frames;
 	hw_if->config_tx_coalesce(pdata);
 
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index bfe11fb..72b8f27 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -222,7 +222,7 @@
 	 ((_idx) & ((_ring)->rdesc_count - 1)))
 
 /* Default coalescing parameters */
-#define XGMAC_INIT_DMA_TX_USECS		50
+#define XGMAC_INIT_DMA_TX_USECS		1000
 #define XGMAC_INIT_DMA_TX_FRAMES	25
 
 #define XGMAC_MAX_DMA_RIWT		0xff
@@ -410,7 +410,7 @@ struct xgbe_channel {
 	unsigned int saved_ier;
 
 	unsigned int tx_timer_active;
-	struct hrtimer tx_timer;
+	struct timer_list tx_timer;
 
 	struct xgbe_ring *tx_ring;
 	struct xgbe_ring *rx_ring;

[PATCH net-next v2 08/10] amd-xgbe: Fix Rx coalescing reporting

[Tom Lendacky]

The Rx coalescing value is internally converted from usecs to a value
that the hardware can use. When reporting the Rx coalescing value, this
internal value is converted back to usecs. During the conversion from
and back to usecs some rounding occurs. So, for example, when setting an
Rx usec of 30, it will be reported as 29. Fix this reporting issue by
keeping the original usec value and using that during reporting.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c     |    1 +
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c |   12 +++++-------
 drivers/net/ethernet/amd/xgbe/xgbe.h         |    1 +
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 122a631..fc362be 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -702,6 +702,7 @@ void xgbe_init_rx_coalesce(struct xgbe_prv_data *pdata)
 	DBGPR("-->xgbe_init_rx_coalesce\n");
 
 	pdata->rx_riwt = hw_if->usec_to_riwt(pdata, XGMAC_INIT_DMA_RX_USECS);
+	pdata->rx_usecs = XGMAC_INIT_DMA_RX_USECS;
 	pdata->rx_frames = XGMAC_INIT_DMA_RX_FRAMES;
 
 	hw_if->config_rx_coalesce(pdata);
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
index 376d6a5..b4f6eaa 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
@@ -377,15 +377,12 @@ static int xgbe_get_coalesce(struct net_device *netdev,
 			     struct ethtool_coalesce *ec)
 {
 	struct xgbe_prv_data *pdata = netdev_priv(netdev);
-	struct xgbe_hw_if *hw_if = &pdata->hw_if;
-	unsigned int riwt;
 
 	DBGPR("-->xgbe_get_coalesce\n");
 
 	memset(ec, 0, sizeof(struct ethtool_coalesce));
 
-	riwt = pdata->rx_riwt;
-	ec->rx_coalesce_usecs = hw_if->riwt_to_usec(pdata, riwt);
+	ec->rx_coalesce_usecs = pdata->rx_usecs;
 	ec->rx_max_coalesced_frames = pdata->rx_frames;
 
 	ec->tx_max_coalesced_frames = pdata->tx_frames;
@@ -438,17 +435,17 @@ static int xgbe_set_coalesce(struct net_device *netdev,
 	}
 
 	rx_riwt = hw_if->usec_to_riwt(pdata, ec->rx_coalesce_usecs);
+	rx_usecs = ec->rx_coalesce_usecs;
 	rx_frames = ec->rx_max_coalesced_frames;
 
 	/* Use smallest possible value if conversion resulted in zero */
-	if (ec->rx_coalesce_usecs && !rx_riwt)
+	if (rx_usecs && !rx_riwt)
 		rx_riwt = 1;
 
 	/* Check the bounds of values for Rx */
 	if (rx_riwt > XGMAC_MAX_DMA_RIWT) {
-		rx_usecs = hw_if->riwt_to_usec(pdata, XGMAC_MAX_DMA_RIWT);
 		netdev_alert(netdev, "rx-usec is limited to %d usecs\n",
-			     rx_usecs);
+			     hw_if->riwt_to_usec(pdata, XGMAC_MAX_DMA_RIWT));
 		return -EINVAL;
 	}
 	if (rx_frames > pdata->rx_desc_count) {
@@ -467,6 +464,7 @@ static int xgbe_set_coalesce(struct net_device *netdev,
 	}
 
 	pdata->rx_riwt = rx_riwt;
+	pdata->rx_usecs = rx_usecs;
 	pdata->rx_frames = rx_frames;
 	hw_if->config_rx_coalesce(pdata);
 
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index 72b8f27..dd74242 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -716,6 +716,7 @@ struct xgbe_prv_data {
 
 	/* Rx coalescing settings */
 	unsigned int rx_riwt;
+	unsigned int rx_usecs;
 	unsigned int rx_frames;
 
 	/* Current Rx buffer size */

[PATCH net-next v2 09/10] amd-xgbe: Use napi_alloc_skb when allocating skb in softirq

[Tom Lendacky]

Use the napi_alloc_skb function to allocate an skb when running within
the softirq context to avoid calls to local_irq_save/restore.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index fc362be..347fe24 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -1821,16 +1821,15 @@ static void xgbe_rx_refresh(struct xgbe_channel *channel)
 			  lower_32_bits(rdata->rdesc_dma));
 }
 
-static struct sk_buff *xgbe_create_skb(struct xgbe_prv_data *pdata,
+static struct sk_buff *xgbe_create_skb(struct napi_struct *napi,
 				       struct xgbe_ring_data *rdata,
 				       unsigned int *len)
 {
-	struct net_device *netdev = pdata->netdev;
 	struct sk_buff *skb;
 	u8 *packet;
 	unsigned int copy_len;
 
-	skb = netdev_alloc_skb_ip_align(netdev, rdata->rx.hdr.dma_len);
+	skb = napi_alloc_skb(napi, rdata->rx.hdr.dma_len);
 	if (!skb)
 		return NULL;
 
@@ -2000,7 +1999,7 @@ read_again:
 							rdata->rx.hdr.dma_len,
 							DMA_FROM_DEVICE);
 
-				skb = xgbe_create_skb(pdata, rdata, &put_len);
+				skb = xgbe_create_skb(napi, rdata, &put_len);
 				if (!skb) {
 					error = 1;
 					goto skip_data;

[PATCH net-next v2 10/10] amd-xgbe: Rework the Rx path SKB allocation

[Tom Lendacky]

When the driver creates an SKB it currently only copies the header
buffer data (which can be just the header if split header processing
succeeded or header plus data if split header processing did not
succeed) into the SKB. The receive buffer data is always added as a
frag, even if it could fit in the SKB. As part of SKB creation, inline
the receive buffer data if it will fit in the the SKB, otherwise add it
as a frag during SKB creation.

Also, Update the code to trigger off of the first/last descriptor
indicators and remove the incomplete indicator.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-common.h |   14 ++--
 drivers/net/ethernet/amd/xgbe/xgbe-desc.c   |    2 -
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c    |   17 +++--
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c    |   87 +++++++++++++++++----------
 drivers/net/ethernet/amd/xgbe/xgbe.h        |    2 -
 5 files changed, 74 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
index 34c28aa..44dded5 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h
@@ -871,15 +871,17 @@
 #define RX_PACKET_ATTRIBUTES_CSUM_DONE_WIDTH	1
 #define RX_PACKET_ATTRIBUTES_VLAN_CTAG_INDEX	1
 #define RX_PACKET_ATTRIBUTES_VLAN_CTAG_WIDTH	1
-#define RX_PACKET_ATTRIBUTES_INCOMPLETE_INDEX	2
-#define RX_PACKET_ATTRIBUTES_INCOMPLETE_WIDTH	1
-#define RX_PACKET_ATTRIBUTES_CONTEXT_NEXT_INDEX	3
+#define RX_PACKET_ATTRIBUTES_FIRST_DESC_INDEX	2
+#define RX_PACKET_ATTRIBUTES_FIRST_DESC_WIDTH	1
+#define RX_PACKET_ATTRIBUTES_LAST_DESC_INDEX	3
+#define RX_PACKET_ATTRIBUTES_LAST_DESC_WIDTH	1
+#define RX_PACKET_ATTRIBUTES_CONTEXT_NEXT_INDEX	4
 #define RX_PACKET_ATTRIBUTES_CONTEXT_NEXT_WIDTH	1
-#define RX_PACKET_ATTRIBUTES_CONTEXT_INDEX	4
+#define RX_PACKET_ATTRIBUTES_CONTEXT_INDEX	5
 #define RX_PACKET_ATTRIBUTES_CONTEXT_WIDTH	1
-#define RX_PACKET_ATTRIBUTES_RX_TSTAMP_INDEX	5
+#define RX_PACKET_ATTRIBUTES_RX_TSTAMP_INDEX	6
 #define RX_PACKET_ATTRIBUTES_RX_TSTAMP_WIDTH	1
-#define RX_PACKET_ATTRIBUTES_RSS_HASH_INDEX	6
+#define RX_PACKET_ATTRIBUTES_RSS_HASH_INDEX	7
 #define RX_PACKET_ATTRIBUTES_RSS_HASH_WIDTH	1
 
 #define RX_NORMAL_DESC0_OVT_INDEX		0
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-desc.c b/drivers/net/ethernet/amd/xgbe/xgbe-desc.c
index d81fc6b..585ee66 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-desc.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-desc.c
@@ -476,8 +476,6 @@ static void xgbe_unmap_rdata(struct xgbe_prv_data *pdata,
 
 	if (rdata->state_saved) {
 		rdata->state_saved = 0;
-		rdata->state.incomplete = 0;
-		rdata->state.context_next = 0;
 		rdata->state.skb = NULL;
 		rdata->state.len = 0;
 		rdata->state.error = 0;
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index 80dd7a9..7f6f0ff 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -1641,9 +1641,16 @@ static int xgbe_dev_read(struct xgbe_channel *channel)
 			       CONTEXT_NEXT, 1);
 
 	/* Get the header length */
-	if (XGMAC_GET_BITS_LE(rdesc->desc3, RX_NORMAL_DESC3, FD))
+	if (XGMAC_GET_BITS_LE(rdesc->desc3, RX_NORMAL_DESC3, FD)) {
+		XGMAC_SET_BITS(packet->attributes, RX_PACKET_ATTRIBUTES,
+			       FIRST_DESC, 1);
 		rdata->rx.hdr_len = XGMAC_GET_BITS_LE(rdesc->desc2,
 						      RX_NORMAL_DESC2, HL);
+	} else {
+		XGMAC_SET_BITS(packet->attributes, RX_PACKET_ATTRIBUTES,
+			       FIRST_DESC, 0);
+		rdata->rx.hdr_len = 0;
+	}
 
 	/* Get the RSS hash */
 	if (XGMAC_GET_BITS_LE(rdesc->desc3, RX_NORMAL_DESC3, RSV)) {
@@ -1668,16 +1675,12 @@ static int xgbe_dev_read(struct xgbe_channel *channel)
 	/* Get the packet length */
 	rdata->rx.len = XGMAC_GET_BITS_LE(rdesc->desc3, RX_NORMAL_DESC3, PL);
 
-	if (!XGMAC_GET_BITS_LE(rdesc->desc3, RX_NORMAL_DESC3, LD)) {
+	if (!XGMAC_GET_BITS_LE(rdesc->desc3, RX_NORMAL_DESC3, LD))
 		/* Not all the data has been transferred for this packet */
-		XGMAC_SET_BITS(packet->attributes, RX_PACKET_ATTRIBUTES,
-			       INCOMPLETE, 1);
 		return 0;
-	}
 
 	/* This is the last of the data for this packet */
-	XGMAC_SET_BITS(packet->attributes, RX_PACKET_ATTRIBUTES,
-		       INCOMPLETE, 0);
+	XGMAC_SET_BITS(packet->attributes, RX_PACKET_ATTRIBUTES, LAST_DESC, 1);
 
 	/* Set checksum done indicator as appropriate */
 	if (channel->pdata->netdev->features & NETIF_F_RXCSUM)
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 347fe24..e357b69 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -1821,26 +1821,59 @@ static void xgbe_rx_refresh(struct xgbe_channel *channel)
 			  lower_32_bits(rdata->rdesc_dma));
 }
 
-static struct sk_buff *xgbe_create_skb(struct napi_struct *napi,
-				       struct xgbe_ring_data *rdata,
-				       unsigned int *len)
+static struct sk_buff *xgbe_create_skb(struct xgbe_prv_data *pdata,
+				       struct napi_struct *napi,
+				       struct xgbe_ring_data *rdata)
 {
 	struct sk_buff *skb;
 	u8 *packet;
-	unsigned int copy_len;
+	unsigned int skb_len, hdr_len, data_len, copy_len;
 
-	skb = napi_alloc_skb(napi, rdata->rx.hdr.dma_len);
+	skb_len = rdata->rx.hdr.dma_len;
+	skb = napi_alloc_skb(napi, skb_len);
 	if (!skb)
 		return NULL;
 
+	hdr_len = rdata->rx.hdr_len;
+	data_len = rdata->rx.len;
+
+	/* Start with the header buffer which may contain
+	 * just the header or the header plus data
+	 */
+	dma_sync_single_for_cpu(pdata->dev, rdata->rx.hdr.dma,
+				rdata->rx.hdr.dma_len, DMA_FROM_DEVICE);
+
+	copy_len = (hdr_len) ? hdr_len : data_len;
+	copy_len = min(copy_len, skb_len);
+
 	packet = page_address(rdata->rx.hdr.pa.pages) +
 		 rdata->rx.hdr.pa.pages_offset;
-	copy_len = (rdata->rx.hdr_len) ? rdata->rx.hdr_len : *len;
-	copy_len = min(rdata->rx.hdr.dma_len, copy_len);
 	skb_copy_to_linear_data(skb, packet, copy_len);
 	skb_put(skb, copy_len);
 
-	*len -= copy_len;
+	data_len -= copy_len;
+	if (!data_len)
+		return skb;
+
+	/* More data, see if it can be inlined */
+	dma_sync_single_for_cpu(pdata->dev, rdata->rx.buf.dma,
+				rdata->rx.buf.dma_len, DMA_FROM_DEVICE);
+
+	skb_len -= copy_len;
+	if (data_len < skb_len) {
+		/* Inline the remaining data */
+		packet = page_address(rdata->rx.buf.pa.pages) +
+			 rdata->rx.buf.pa.pages_offset;
+		skb_copy_to_linear_data_offset(skb, copy_len, packet, data_len);
+		skb_put(skb, data_len);
+	} else {
+		/* Add the remaining data as a frag */
+		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+				rdata->rx.buf.pa.pages,
+				rdata->rx.buf.pa.pages_offset,
+				data_len, rdata->rx.buf.dma_len);
+		rdata->rx.buf.pa.pages = NULL;
+	}
 
 	return skb;
 }
@@ -1922,7 +1955,7 @@ static int xgbe_rx_poll(struct xgbe_channel *channel, int budget)
 	struct napi_struct *napi;
 	struct sk_buff *skb;
 	struct skb_shared_hwtstamps *hwtstamps;
-	unsigned int incomplete, error, context_next, context;
+	unsigned int first_desc, last_desc, error, context_next, context;
 	unsigned int len, put_len, max_len;
 	unsigned int received = 0;
 	int packet_count = 0;
@@ -1935,6 +1968,9 @@ static int xgbe_rx_poll(struct xgbe_channel *channel, int budget)
 
 	napi = (pdata->per_channel_irq) ? &channel->napi : &pdata->napi;
 
+	last_desc = 0;
+	context_next = 0;
+
 	rdata = XGBE_GET_DESC_DATA(ring, ring->cur);
 	packet = &ring->packet_data;
 	while (packet_count < budget) {
@@ -1942,15 +1978,11 @@ static int xgbe_rx_poll(struct xgbe_channel *channel, int budget)
 
 		/* First time in loop see if we need to restore state */
 		if (!received && rdata->state_saved) {
-			incomplete = rdata->state.incomplete;
-			context_next = rdata->state.context_next;
 			skb = rdata->state.skb;
 			error = rdata->state.error;
 			len = rdata->state.len;
 		} else {
 			memset(packet, 0, sizeof(*packet));
-			incomplete = 0;
-			context_next = 0;
 			skb = NULL;
 			error = 0;
 			len = 0;
@@ -1968,9 +2000,10 @@ read_again:
 		received++;
 		ring->cur++;
 
-		incomplete = XGMAC_GET_BITS(packet->attributes,
-					    RX_PACKET_ATTRIBUTES,
-					    INCOMPLETE);
+		first_desc = XGMAC_GET_BITS(packet->attributes,
+					    RX_PACKET_ATTRIBUTES, FIRST_DESC);
+		last_desc = XGMAC_GET_BITS(packet->attributes,
+					   RX_PACKET_ATTRIBUTES, LAST_DESC);
 		context_next = XGMAC_GET_BITS(packet->attributes,
 					      RX_PACKET_ATTRIBUTES,
 					      CONTEXT_NEXT);
@@ -1979,7 +2012,7 @@ read_again:
 					 CONTEXT);
 
 		/* Earlier error, just drain the remaining data */
-		if ((incomplete || context_next) && error)
+		if ((!last_desc || context_next) && error)
 			goto read_again;
 
 		if (error || packet->errors) {
@@ -1990,23 +2023,17 @@ read_again:
 		}
 
 		if (!context) {
+			/* Returned data length is cumulative */
 			put_len = rdata->rx.len - len;
 			len += put_len;
 
-			if (!skb) {
-				dma_sync_single_for_cpu(pdata->dev,
-							rdata->rx.hdr.dma,
-							rdata->rx.hdr.dma_len,
-							DMA_FROM_DEVICE);
-
-				skb = xgbe_create_skb(napi, rdata, &put_len);
+			if (first_desc) {
+				skb = xgbe_create_skb(pdata, napi, rdata);
 				if (!skb) {
 					error = 1;
 					goto skip_data;
 				}
-			}
-
-			if (put_len) {
+			} else {
 				dma_sync_single_for_cpu(pdata->dev,
 							rdata->rx.buf.dma,
 							rdata->rx.buf.dma_len,
@@ -2021,7 +2048,7 @@ read_again:
 		}
 
 skip_data:
-		if (incomplete || context_next)
+		if (!last_desc || context_next)
 			goto read_again;
 
 		if (!skb)
@@ -2081,11 +2108,9 @@ next_packet:
 	}
 
 	/* Check if we need to save state before leaving */
-	if (received && (incomplete || context_next)) {
+	if (received && (!last_desc || context_next)) {
 		rdata = XGBE_GET_DESC_DATA(ring, ring->cur);
 		rdata->state_saved = 1;
-		rdata->state.incomplete = incomplete;
-		rdata->state.context_next = context_next;
 		rdata->state.skb = skb;
 		rdata->state.len = len;
 		rdata->state.error = error;
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
index dd74242..296ad26 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe.h
+++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
@@ -336,8 +336,6 @@ struct xgbe_ring_data {
 	 */
 	unsigned int state_saved;
 	struct {
-		unsigned int incomplete;
-		unsigned int context_next;
 		struct sk_buff *skb;
 		unsigned int len;
 		unsigned int error;

Re: [PATCH net-next v2 10/10] amd-xgbe: Rework the Rx path SKB allocation

[David Miller]

From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Thu, 19 Mar 2015 15:09:08 -0500

> When the driver creates an SKB it currently only copies the header
> buffer data (which can be just the header if split header processing
> succeeded or header plus data if split header processing did not
> succeed) into the SKB. The receive buffer data is always added as a
> frag, even if it could fit in the SKB. As part of SKB creation, inline
> the receive buffer data if it will fit in the the SKB, otherwise add it
> as a frag during SKB creation.
> 
> Also, Update the code to trigger off of the first/last descriptor
> indicators and remove the incomplete indicator.
> 
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>

I do not understand the motivation for this, could you explain?

The less copying you do the better, just having the headers in the
linear area is the most optimal situation, and have all the data
in page frag references.

Re: [PATCH net-next v2 10/10] amd-xgbe: Rework the Rx path SKB allocation

[Tom Lendacky]

On 03/19/2015 03:20 PM, David Miller wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Date: Thu, 19 Mar 2015 15:09:08 -0500
>
>> When the driver creates an SKB it currently only copies the header
>> buffer data (which can be just the header if split header processing
>> succeeded or header plus data if split header processing did not
>> succeed) into the SKB. The receive buffer data is always added as a
>> frag, even if it could fit in the SKB. As part of SKB creation, inline
>> the receive buffer data if it will fit in the the SKB, otherwise add it
>> as a frag during SKB creation.
>>
>> Also, Update the code to trigger off of the first/last descriptor
>> indicators and remove the incomplete indicator.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>
> I do not understand the motivation for this, could you explain?
>
> The less copying you do the better, just having the headers in the
> linear area is the most optimal situation, and have all the data
> in page frag references.
>

I was trying to make the Rx path more logical from a first / last
descriptor point of view.  If it's the first descriptor allocate the
SKB, otherwise add the data as a frag. Compared to the current code:
check for null skb pointer, allocate the SKB and if there's data left
add it as a frag.

I could keep the first / last descriptor methodology and in the
xgbe_create_skb routine avoid the second copy and just always add the
other buffer as a frag. That will eliminate the extra copying. Would
that be ok or would you prefer that I just drop this patch?

Thanks,
Tom

Re: [PATCH net-next v2 10/10] amd-xgbe: Rework the Rx path SKB allocation

[David Miller]

From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Thu, 19 Mar 2015 15:54:24 -0500

> On 03/19/2015 03:20 PM, David Miller wrote:
>> From: Tom Lendacky <thomas.lendacky@amd.com>
>> Date: Thu, 19 Mar 2015 15:09:08 -0500
>>
>>> When the driver creates an SKB it currently only copies the header
>>> buffer data (which can be just the header if split header processing
>>> succeeded or header plus data if split header processing did not
>>> succeed) into the SKB. The receive buffer data is always added as a
>>> frag, even if it could fit in the SKB. As part of SKB creation, inline
>>> the receive buffer data if it will fit in the the SKB, otherwise add
>>> it
>>> as a frag during SKB creation.
>>>
>>> Also, Update the code to trigger off of the first/last descriptor
>>> indicators and remove the incomplete indicator.
>>>
>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>
>> I do not understand the motivation for this, could you explain?
>>
>> The less copying you do the better, just having the headers in the
>> linear area is the most optimal situation, and have all the data
>> in page frag references.
>>
> 
> I was trying to make the Rx path more logical from a first / last
> descriptor point of view.  If it's the first descriptor allocate the
> SKB, otherwise add the data as a frag. Compared to the current code:
> check for null skb pointer, allocate the SKB and if there's data left
> add it as a frag.
> 
> I could keep the first / last descriptor methodology and in the
> xgbe_create_skb routine avoid the second copy and just always add the
> other buffer as a frag. That will eliminate the extra copying. Would
> that be ok or would you prefer that I just drop this patch?

The point is, the data might not even be touched by the cpu so copying
it into the linear SKB data area could be a complete waste of cpu
cycles.

Only the headers will be touched by the cpu in the packet processing
paths.

And when we copy the packet data into userspace, that might even occur
on another cpu, so the data will just thrash between the individual
cpu's caches.

Re: [PATCH net-next v2 10/10] amd-xgbe: Rework the Rx path SKB allocation

[Tom Lendacky]

On 03/19/2015 04:51 PM, David Miller wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Date: Thu, 19 Mar 2015 15:54:24 -0500
>
>> On 03/19/2015 03:20 PM, David Miller wrote:
>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>> Date: Thu, 19 Mar 2015 15:09:08 -0500
>>>
>>>> When the driver creates an SKB it currently only copies the header
>>>> buffer data (which can be just the header if split header processing
>>>> succeeded or header plus data if split header processing did not
>>>> succeed) into the SKB. The receive buffer data is always added as a
>>>> frag, even if it could fit in the SKB. As part of SKB creation, inline
>>>> the receive buffer data if it will fit in the the SKB, otherwise add
>>>> it
>>>> as a frag during SKB creation.
>>>>
>>>> Also, Update the code to trigger off of the first/last descriptor
>>>> indicators and remove the incomplete indicator.
>>>>
>>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>>
>>> I do not understand the motivation for this, could you explain?
>>>
>>> The less copying you do the better, just having the headers in the
>>> linear area is the most optimal situation, and have all the data
>>> in page frag references.
>>>
>>
>> I was trying to make the Rx path more logical from a first / last
>> descriptor point of view.  If it's the first descriptor allocate the
>> SKB, otherwise add the data as a frag. Compared to the current code:
>> check for null skb pointer, allocate the SKB and if there's data left
>> add it as a frag.
>>
>> I could keep the first / last descriptor methodology and in the
>> xgbe_create_skb routine avoid the second copy and just always add the
>> other buffer as a frag. That will eliminate the extra copying. Would
>> that be ok or would you prefer that I just drop this patch?
>
> The point is, the data might not even be touched by the cpu so copying
> it into the linear SKB data area could be a complete waste of cpu
> cycles.

I understood that point, sorry if I wasn't clear.  I'd would remove the
copying of the data and just always add it as a frag in xgbe_create_skb
routine so that only the headers are in the linear area.  What I'd like
to do though is keep the overall changes of how I determine when to
call the xgbe_create_skb routine so that it appears a bit cleaner,
more logical.  The net effect is that the behavior of the code would
remain the same (headers in the linear area, data as frags), but I feel
it reads better and is easier to understand.

Thanks,
Tom

>
> Only the headers will be touched by the cpu in the packet processing
> paths.
>
> And when we copy the packet data into userspace, that might even occur
> on another cpu, so the data will just thrash between the individual
> cpu's caches.
>

Re: [PATCH net-next v2 10/10] amd-xgbe: Rework the Rx path SKB allocation

[Tom Lendacky]

On 03/19/2015 05:07 PM, Tom Lendacky wrote:
> On 03/19/2015 04:51 PM, David Miller wrote:
>> From: Tom Lendacky <thomas.lendacky@amd.com>
>> Date: Thu, 19 Mar 2015 15:54:24 -0500
>>
>>> On 03/19/2015 03:20 PM, David Miller wrote:
>>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>>> Date: Thu, 19 Mar 2015 15:09:08 -0500
>>>>
>>>>> When the driver creates an SKB it currently only copies the header
>>>>> buffer data (which can be just the header if split header processing
>>>>> succeeded or header plus data if split header processing did not
>>>>> succeed) into the SKB. The receive buffer data is always added as a
>>>>> frag, even if it could fit in the SKB. As part of SKB creation, inline
>>>>> the receive buffer data if it will fit in the the SKB, otherwise add
>>>>> it
>>>>> as a frag during SKB creation.
>>>>>
>>>>> Also, Update the code to trigger off of the first/last descriptor
>>>>> indicators and remove the incomplete indicator.
>>>>>
>>>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>>>
>>>> I do not understand the motivation for this, could you explain?
>>>>
>>>> The less copying you do the better, just having the headers in the
>>>> linear area is the most optimal situation, and have all the data
>>>> in page frag references.
>>>>
>>>
>>> I was trying to make the Rx path more logical from a first / last
>>> descriptor point of view.  If it's the first descriptor allocate the
>>> SKB, otherwise add the data as a frag. Compared to the current code:
>>> check for null skb pointer, allocate the SKB and if there's data left
>>> add it as a frag.
>>>
>>> I could keep the first / last descriptor methodology and in the
>>> xgbe_create_skb routine avoid the second copy and just always add the
>>> other buffer as a frag. That will eliminate the extra copying. Would
>>> that be ok or would you prefer that I just drop this patch?
>>
>> The point is, the data might not even be touched by the cpu so copying
>> it into the linear SKB data area could be a complete waste of cpu
>> cycles.
>
> I understood that point, sorry if I wasn't clear.  I'd would remove the
> copying of the data and just always add it as a frag in xgbe_create_skb
> routine so that only the headers are in the linear area.  What I'd like
> to do though is keep the overall changes of how I determine when to
> call the xgbe_create_skb routine so that it appears a bit cleaner,
> more logical.  The net effect is that the behavior of the code would
> remain the same (headers in the linear area, data as frags), but I feel
> it reads better and is easier to understand.

Actually, going back to the comment you made from one of the other
patches about programming defensively, I believe the current code is
safer since it will check for a NULL skb pointer and allocate one vs
this patch assuming the first descriptor bit will be set. Should the
hardware have a bug and not set that bit then the driver would try to
add a frag using a NULL skb pointer. I'm going to drop this patch in
the next version of the series I send out.

Thanks,
Tom

>
> Thanks,
> Tom
>
>>
>> Only the headers will be touched by the cpu in the packet processing
>> paths.
>>
>> And when we copy the packet data into userspace, that might even occur
>> on another cpu, so the data will just thrash between the individual
>> cpu's caches.
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html