DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
@ 2023-02-08  9:15 Chaoyong He
  2023-02-15 13:42 ` Ferruh Yigit
  2023-02-20  9:02 ` [PATCH v2] net/nfp: fix 48-bit DMA address support for NFDk Chaoyong He
  0 siblings, 2 replies; 13+ messages in thread
From: Chaoyong He @ 2023-02-08  9:15 UTC (permalink / raw)
  To: dev
  Cc: oss-drivers, niklas.soderlund, Peng Zhang, jin.liu, stable, Chaoyong He

From: Peng Zhang <peng.zhang@corigine.com>

48-bit DMA address is supported in the firmware with NFDk, so enable
this feature in PMD now. But the firmware with NFD3 still just
support 40-bit DMA address.

RX free list descriptor, used by both NFD3 and NFDk, is also modified
to support 48-bit DMA address. That's OK because the top bits is always
set to 0 when assigned with 40-bit DMA address.

Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
Cc: jin.liu@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 drivers/net/nfp/flower/nfp_flower.c      | 12 ++++--------
 drivers/net/nfp/flower/nfp_flower_ctrl.c |  2 +-
 drivers/net/nfp/nfp_common.c             | 18 ++++++++++++++++++
 drivers/net/nfp/nfp_common.h             |  1 +
 drivers/net/nfp/nfp_ethdev.c             | 11 +++--------
 drivers/net/nfp/nfp_ethdev_vf.c          | 11 +++--------
 drivers/net/nfp/nfp_rxtx.c               |  4 ++--
 drivers/net/nfp/nfp_rxtx.h               |  4 ++--
 8 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index 7b46dc0f6a..7302d39af5 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -452,7 +452,7 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 		rxds->vals[1] = 0;
 		dma_addr = rte_cpu_to_le_64(RTE_MBUF_DMA_ADDR_DEFAULT(new_mb));
 		rxds->fld.dd = 0;
-		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
 		rxds->fld.dma_addr_lo = dma_addr & 0xffffffff;
 		nb_hold++;
 
@@ -632,13 +632,6 @@ nfp_flower_init_vnic_common(struct nfp_net_hw *hw, const char *vnic_type)
 	pf_dev = hw->pf_dev;
 	pci_dev = hw->pf_dev->pci_dev;
 
-	/* NFP can not handle DMA addresses requiring more than 40 bits */
-	if (rte_mem_check_dma_mask(40)) {
-		PMD_INIT_LOG(ERR, "Device %s can not be used: restricted dma mask to 40 bits!\n",
-				pci_dev->device.name);
-		return -ENODEV;
-	};
-
 	hw->device_id = pci_dev->id.device_id;
 	hw->vendor_id = pci_dev->id.vendor_id;
 	hw->subsystem_device_id = pci_dev->id.subsystem_device_id;
@@ -667,6 +660,9 @@ nfp_flower_init_vnic_common(struct nfp_net_hw *hw, const char *vnic_type)
 	hw->mtu = hw->max_mtu;
 	hw->flbufsz = DEFAULT_FLBUF_SIZE;
 
+	if (nfp_net_check_dma_mask(hw, pci_dev->name) != 0)
+		return -ENODEV;
+
 	/* read the Rx offset configured from firmware */
 	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2)
 		hw->rx_offset = NFP_NET_RX_OFFSET;
diff --git a/drivers/net/nfp/flower/nfp_flower_ctrl.c b/drivers/net/nfp/flower/nfp_flower_ctrl.c
index 03a2e2e622..b134a74bd8 100644
--- a/drivers/net/nfp/flower/nfp_flower_ctrl.c
+++ b/drivers/net/nfp/flower/nfp_flower_ctrl.c
@@ -122,7 +122,7 @@ nfp_flower_ctrl_vnic_recv(void *rx_queue,
 		rxds->vals[1] = 0;
 		dma_addr = rte_cpu_to_le_64(RTE_MBUF_DMA_ADDR_DEFAULT(new_mb));
 		rxds->fld.dd = 0;
-		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
 		rxds->fld.dma_addr_lo = dma_addr & 0xffffffff;
 		nb_hold++;
 
diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index 4f21d9978d..4b36861b21 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -1563,6 +1563,24 @@ nfp_net_set_vxlan_port(struct nfp_net_hw *hw,
 	return ret;
 }
 
+/*
+ * The firmware with NFD3 can not handle DMA address requiring more
+ * than 40 bits
+ */
+int
+nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name)
+{
+	if (NFD_CFG_CLASS_VER_of(hw->ver) == NFP_NET_CFG_VERSION_DP_NFD3 &&
+			rte_mem_check_dma_mask(40) != 0) {
+		PMD_DRV_LOG(ERR,
+			"The device %s can't be used: restricted dma mask to 40 bits!",
+			name);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 RTE_LOG_REGISTER_SUFFIX(nfp_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(nfp_logtype_driver, driver, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(nfp_logtype_cpp, cpp, NOTICE);
diff --git a/drivers/net/nfp/nfp_common.h b/drivers/net/nfp/nfp_common.h
index 56b7edc951..980f3cad89 100644
--- a/drivers/net/nfp/nfp_common.h
+++ b/drivers/net/nfp/nfp_common.h
@@ -454,6 +454,7 @@ int nfp_net_rx_desc_limits(struct nfp_net_hw *hw,
 int nfp_net_tx_desc_limits(struct nfp_net_hw *hw,
 		uint16_t *min_tx_desc,
 		uint16_t *max_tx_desc);
+int nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name);
 
 #define NFP_NET_DEV_PRIVATE_TO_HW(adapter)\
 	(&((struct nfp_net_adapter *)adapter)->hw)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 31201c0197..8ac2a4b5cd 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -517,14 +517,6 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	/* Use backpointer to the CoreNIC app struct */
 	app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
 
-	/* NFP can not handle DMA addresses requiring more than 40 bits */
-	if (rte_mem_check_dma_mask(40)) {
-		RTE_LOG(ERR, PMD,
-			"device %s can not be used: restricted dma mask to 40 bits!\n",
-			pci_dev->device.name);
-		return -ENODEV;
-	}
-
 	port = ((struct nfp_net_hw *)eth_dev->data->dev_private)->idx;
 	if (port < 0 || port > 7) {
 		PMD_DRV_LOG(ERR, "Port value is wrong");
@@ -572,6 +564,9 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 
 	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
 
+	if (nfp_net_check_dma_mask(hw, pci_dev->name) != 0)
+		return -ENODEV;
+
 	if (nfp_net_ethdev_ops_mount(hw, eth_dev))
 		return -EINVAL;
 
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index 4eea197f85..e60e9f0f15 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -291,14 +291,6 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 
 	pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 
-	/* NFP can not handle DMA addresses requiring more than 40 bits */
-	if (rte_mem_check_dma_mask(40)) {
-		RTE_LOG(ERR, PMD,
-			"device %s can not be used: restricted dma mask to 40 bits!\n",
-			pci_dev->device.name);
-		return -ENODEV;
-	}
-
 	hw = NFP_NET_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
 
 	hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
@@ -312,6 +304,9 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 
 	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
 
+	if (nfp_net_check_dma_mask(hw, pci_dev->name) != 0)
+		return -ENODEV;
+
 	if (nfp_netvf_ethdev_ops_mount(hw, eth_dev))
 		return -EINVAL;
 
diff --git a/drivers/net/nfp/nfp_rxtx.c b/drivers/net/nfp/nfp_rxtx.c
index 4a7574fd65..5ca3aef7e1 100644
--- a/drivers/net/nfp/nfp_rxtx.c
+++ b/drivers/net/nfp/nfp_rxtx.c
@@ -48,7 +48,7 @@ nfp_net_rx_fill_freelist(struct nfp_net_rxq *rxq)
 
 		rxd = &rxq->rxds[i];
 		rxd->fld.dd = 0;
-		rxd->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+		rxd->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
 		rxd->fld.dma_addr_lo = dma_addr & 0xffffffff;
 		rxe[i].mbuf = mbuf;
 		PMD_RX_LOG(DEBUG, "[%d]: %" PRIx64, i, dma_addr);
@@ -454,7 +454,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		rxds->vals[1] = 0;
 		dma_addr = rte_cpu_to_le_64(RTE_MBUF_DMA_ADDR_DEFAULT(new_mb));
 		rxds->fld.dd = 0;
-		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
 		rxds->fld.dma_addr_lo = dma_addr & 0xffffffff;
 		nb_hold++;
 
diff --git a/drivers/net/nfp/nfp_rxtx.h b/drivers/net/nfp/nfp_rxtx.h
index 8a29adbd73..765717e8dc 100644
--- a/drivers/net/nfp/nfp_rxtx.h
+++ b/drivers/net/nfp/nfp_rxtx.h
@@ -287,8 +287,8 @@ struct nfp_net_rx_desc {
 	union {
 		/* Freelist descriptor */
 		struct {
-			uint8_t dma_addr_hi;
-			__le16 spare;
+			__le16 dma_addr_hi;
+			uint8_t spare;
 			uint8_t dd;
 
 			__le32 dma_addr_lo;
-- 
2.29.3


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-08  9:15 [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk Chaoyong He
@ 2023-02-15 13:42 ` Ferruh Yigit
  2023-02-15 17:47   ` Niklas Söderlund
  2023-02-20  9:02 ` [PATCH v2] net/nfp: fix 48-bit DMA address support for NFDk Chaoyong He
  1 sibling, 1 reply; 13+ messages in thread
From: Ferruh Yigit @ 2023-02-15 13:42 UTC (permalink / raw)
  To: Chaoyong He, dev, Luca Boccassi, Kevin Traynor
  Cc: oss-drivers, niklas.soderlund, Peng Zhang, jin.liu, stable

On 2/8/2023 9:15 AM, Chaoyong He wrote:
> From: Peng Zhang <peng.zhang@corigine.com>
> 
> 48-bit DMA address is supported in the firmware with NFDk, so enable
> this feature in PMD now. But the firmware with NFD3 still just
> support 40-bit DMA address.
> 
> RX free list descriptor, used by both NFD3 and NFDk, is also modified
> to support 48-bit DMA address. That's OK because the top bits is always
> set to 0 when assigned with 40-bit DMA address.
> 
> Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
> Cc: jin.liu@corigine.com
> Cc: stable@dpdk.org
> 

Why a backport is requested? As far as I understand this is not fixing
anything but extending device capability. Is this a fix?

> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-15 13:42 ` Ferruh Yigit
@ 2023-02-15 17:47   ` Niklas Söderlund
  2023-02-15 18:28     ` Ferruh Yigit
  0 siblings, 1 reply; 13+ messages in thread
From: Niklas Söderlund @ 2023-02-15 17:47 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Chaoyong He, dev, Luca Boccassi, Kevin Traynor, oss-drivers,
	Peng Zhang, jin.liu, stable

Hi Ferruh,

Thanks for your continues effort in dealing with NFP patches.

On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
> On 2/8/2023 9:15 AM, Chaoyong He wrote:
> > From: Peng Zhang <peng.zhang@corigine.com>
> > 
> > 48-bit DMA address is supported in the firmware with NFDk, so enable
> > this feature in PMD now. But the firmware with NFD3 still just
> > support 40-bit DMA address.
> > 
> > RX free list descriptor, used by both NFD3 and NFDk, is also modified
> > to support 48-bit DMA address. That's OK because the top bits is always
> > set to 0 when assigned with 40-bit DMA address.
> > 
> > Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
> > Cc: jin.liu@corigine.com
> > Cc: stable@dpdk.org
> > 
> 
> Why a backport is requested? As far as I understand this is not fixing
> anything but extending device capability. Is this a fix?

I agree this is a bit of a grey zone. We reasoned this was a fix as we 
should have done this from the start in the commit that added support 
for NFDk. Are you OK moving forward with this as a fix or would you 
prefer we resubmit without the request to backport?

> 
> > Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> 

-- 
Kind Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-15 17:47   ` Niklas Söderlund
@ 2023-02-15 18:28     ` Ferruh Yigit
  2023-02-16 10:28       ` Kevin Traynor
  0 siblings, 1 reply; 13+ messages in thread
From: Ferruh Yigit @ 2023-02-15 18:28 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Chaoyong He, dev, Luca Boccassi, Kevin Traynor, oss-drivers,
	Peng Zhang, jin.liu, stable

On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
> Hi Ferruh,
> 
> Thanks for your continues effort in dealing with NFP patches.
> 
> On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
>> On 2/8/2023 9:15 AM, Chaoyong He wrote:
>>> From: Peng Zhang <peng.zhang@corigine.com>
>>>
>>> 48-bit DMA address is supported in the firmware with NFDk, so enable
>>> this feature in PMD now. But the firmware with NFD3 still just
>>> support 40-bit DMA address.
>>>
>>> RX free list descriptor, used by both NFD3 and NFDk, is also modified
>>> to support 48-bit DMA address. That's OK because the top bits is always
>>> set to 0 when assigned with 40-bit DMA address.
>>>
>>> Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
>>> Cc: jin.liu@corigine.com
>>> Cc: stable@dpdk.org
>>>
>>
>> Why a backport is requested? As far as I understand this is not fixing
>> anything but extending device capability. Is this a fix?
> 
> I agree this is a bit of a grey zone. We reasoned this was a fix as we 
> should have done this from the start in the commit that added support 
> for NFDk. Are you OK moving forward with this as a fix or would you 
> prefer we resubmit without the request to backport?
> 

I am not sure, is this change have any potential to change behavior for
existing users?
Like if one of your user is using 22.11.1 release, and if this patch
backported to next LTS version, 22.11.2, will user notice any difference?


@Luca, @Kevin, what is your comment as LTS maintainers?

>>
>>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
>>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-15 18:28     ` Ferruh Yigit
@ 2023-02-16 10:28       ` Kevin Traynor
  2023-02-16 10:37         ` Niklas Söderlund
  0 siblings, 1 reply; 13+ messages in thread
From: Kevin Traynor @ 2023-02-16 10:28 UTC (permalink / raw)
  To: Ferruh Yigit, Niklas Söderlund, Xueming(Steven) Li
  Cc: Chaoyong He, dev, Luca Boccassi, oss-drivers, Peng Zhang,
	jin.liu, stable

On 15/02/2023 18:28, Ferruh Yigit wrote:
> On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
>> Hi Ferruh,
>>
>> Thanks for your continues effort in dealing with NFP patches.
>>
>> On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
>>> On 2/8/2023 9:15 AM, Chaoyong He wrote:
>>>> From: Peng Zhang <peng.zhang@corigine.com>
>>>>
>>>> 48-bit DMA address is supported in the firmware with NFDk, so enable
>>>> this feature in PMD now. But the firmware with NFD3 still just
>>>> support 40-bit DMA address.
>>>>
>>>> RX free list descriptor, used by both NFD3 and NFDk, is also modified
>>>> to support 48-bit DMA address. That's OK because the top bits is always
>>>> set to 0 when assigned with 40-bit DMA address.
>>>>
>>>> Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
>>>> Cc: jin.liu@corigine.com
>>>> Cc: stable@dpdk.org
>>>>
>>>
>>> Why a backport is requested? As far as I understand this is not fixing
>>> anything but extending device capability. Is this a fix?
>>
>> I agree this is a bit of a grey zone. We reasoned this was a fix as we
>> should have done this from the start in the commit that added support
>> for NFDk. Are you OK moving forward with this as a fix or would you
>> prefer we resubmit without the request to backport?
>>
> 
> I am not sure, is this change have any potential to change behavior for
> existing users?
> Like if one of your user is using 22.11.1 release, and if this patch
> backported to next LTS version, 22.11.2, will user notice any difference?
> 
> 
> @Luca, @Kevin, what is your comment as LTS maintainers?
> 

A bit difficult to know. If NFDk is not practicably usable without it, 
then it could be considered a fix. If it's just extending to add 
nice-to-have functionality then probably it is not a fix.

It would need to ensure that it is tested on 22.11 branch and there are 
no regressions. It is only relevant to DPDK 22.11 LTS so Cc Xueming who 
will ultimately decide.

A guide below on some things to consider for this type of backport is here:
http://doc.dpdk.org/guides/contributing/stable.html#what-changes-should-be-backported

>>>
>>>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
>>>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>>
>>
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-16 10:28       ` Kevin Traynor
@ 2023-02-16 10:37         ` Niklas Söderlund
  2023-02-16 10:41           ` Chaoyong He
  0 siblings, 1 reply; 13+ messages in thread
From: Niklas Söderlund @ 2023-02-16 10:37 UTC (permalink / raw)
  To: Kevin Traynor
  Cc: Ferruh Yigit, Xueming(Steven) Li, Chaoyong He, dev,
	Luca Boccassi, oss-drivers, Peng Zhang, jin.liu, stable

Hi Kevin,

Thanks for your input.

On 2023-02-16 10:28:34 +0000, Kevin Traynor wrote:
> On 15/02/2023 18:28, Ferruh Yigit wrote:
> > On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
> > > Hi Ferruh,
> > > 
> > > Thanks for your continues effort in dealing with NFP patches.
> > > 
> > > On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
> > > > On 2/8/2023 9:15 AM, Chaoyong He wrote:
> > > > > From: Peng Zhang <peng.zhang@corigine.com>
> > > > > 
> > > > > 48-bit DMA address is supported in the firmware with NFDk, so enable
> > > > > this feature in PMD now. But the firmware with NFD3 still just
> > > > > support 40-bit DMA address.
> > > > > 
> > > > > RX free list descriptor, used by both NFD3 and NFDk, is also modified
> > > > > to support 48-bit DMA address. That's OK because the top bits is always
> > > > > set to 0 when assigned with 40-bit DMA address.
> > > > > 
> > > > > Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
> > > > > Cc: jin.liu@corigine.com
> > > > > Cc: stable@dpdk.org
> > > > > 
> > > > 
> > > > Why a backport is requested? As far as I understand this is not fixing
> > > > anything but extending device capability. Is this a fix?
> > > 
> > > I agree this is a bit of a grey zone. We reasoned this was a fix as we
> > > should have done this from the start in the commit that added support
> > > for NFDk. Are you OK moving forward with this as a fix or would you
> > > prefer we resubmit without the request to backport?
> > > 
> > 
> > I am not sure, is this change have any potential to change behavior for
> > existing users?
> > Like if one of your user is using 22.11.1 release, and if this patch
> > backported to next LTS version, 22.11.2, will user notice any difference?
> > 
> > 
> > @Luca, @Kevin, what is your comment as LTS maintainers?
> > 
> 
> A bit difficult to know. If NFDk is not practicably usable without it, then
> it could be considered a fix. If it's just extending to add nice-to-have
> functionality then probably it is not a fix.

I think we can treat this as a nice-to-have and not something that makes 
NFDk unusable. As stated above, we marked this as a Fix as we *really* 
should have done this in the commit which added NFDk support.

@Ferruh, would you prefer we send a v2 or will you drop the Fixes and CC 
tags when/if applying?

> 
> It would need to ensure that it is tested on 22.11 branch and there are no
> regressions. It is only relevant to DPDK 22.11 LTS so Cc Xueming who will
> ultimately decide.
> 
> A guide below on some things to consider for this type of backport is here:
> http://doc.dpdk.org/guides/contributing/stable.html#what-changes-should-be-backported
> 
> > > > 
> > > > > Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> > > > > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> > > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> > > > 
> > > 
> > 
> 

-- 
Kind Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-16 10:37         ` Niklas Söderlund
@ 2023-02-16 10:41           ` Chaoyong He
  2023-02-16 10:55             ` Niklas Soderlund
  2023-02-16 10:59             ` Ferruh Yigit
  0 siblings, 2 replies; 13+ messages in thread
From: Chaoyong He @ 2023-02-16 10:41 UTC (permalink / raw)
  To: Niklas Soderlund, Kevin Traynor
  Cc: Ferruh Yigit, Xueming(Steven) Li, dev, Luca Boccassi,
	oss-drivers, Nole Zhang, Kevin Liu, stable



> -----Original Message-----
> From: Niklas Soderlund <niklas.soderlund@corigine.com>
> Sent: Thursday, February 16, 2023 6:37 PM
> To: Kevin Traynor <ktraynor@redhat.com>
> Cc: Ferruh Yigit <ferruh.yigit@amd.com>; Xueming(Steven) Li
> <xuemingl@nvidia.com>; Chaoyong He <chaoyong.he@corigine.com>;
> dev@dpdk.org; Luca Boccassi <bluca@debian.org>; oss-drivers <oss-
> drivers@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Kevin Liu
> <jin.liu@corigine.com>; stable@dpdk.org
> Subject: Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with
> NFDk
> 
> Hi Kevin,
> 
> Thanks for your input.
> 
> On 2023-02-16 10:28:34 +0000, Kevin Traynor wrote:
> > On 15/02/2023 18:28, Ferruh Yigit wrote:
> > > On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
> > > > Hi Ferruh,
> > > >
> > > > Thanks for your continues effort in dealing with NFP patches.
> > > >
> > > > On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
> > > > > On 2/8/2023 9:15 AM, Chaoyong He wrote:
> > > > > > From: Peng Zhang <peng.zhang@corigine.com>
> > > > > >
> > > > > > 48-bit DMA address is supported in the firmware with NFDk, so
> > > > > > enable this feature in PMD now. But the firmware with NFD3
> > > > > > still just support 40-bit DMA address.
> > > > > >
> > > > > > RX free list descriptor, used by both NFD3 and NFDk, is also
> > > > > > modified to support 48-bit DMA address. That's OK because the
> > > > > > top bits is always set to 0 when assigned with 40-bit DMA address.
> > > > > >
> > > > > > Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
> > > > > > Cc: jin.liu@corigine.com
> > > > > > Cc: stable@dpdk.org
> > > > > >
> > > > >
> > > > > Why a backport is requested? As far as I understand this is not
> > > > > fixing anything but extending device capability. Is this a fix?
> > > >
> > > > I agree this is a bit of a grey zone. We reasoned this was a fix
> > > > as we should have done this from the start in the commit that
> > > > added support for NFDk. Are you OK moving forward with this as a
> > > > fix or would you prefer we resubmit without the request to backport?
> > > >
> > >
> > > I am not sure, is this change have any potential to change behavior
> > > for existing users?
> > > Like if one of your user is using 22.11.1 release, and if this patch
> > > backported to next LTS version, 22.11.2, will user notice any difference?
> > >
> > >
> > > @Luca, @Kevin, what is your comment as LTS maintainers?
> > >
> >
> > A bit difficult to know. If NFDk is not practicably usable without it,
> > then it could be considered a fix. If it's just extending to add
> > nice-to-have functionality then probably it is not a fix.
> 
> I think we can treat this as a nice-to-have and not something that makes
> NFDk unusable. As stated above, we marked this as a Fix as we *really*
> should have done this in the commit which added NFDk support.
> 
> @Ferruh, would you prefer we send a v2 or will you drop the Fixes and CC
> tags when/if applying?
> 

Actually, the DPDK app using the nfp card with a firmware of NFDk will coredump without this patch.
And that's the directly reason we consider backport this patch.

> >
> > It would need to ensure that it is tested on 22.11 branch and there
> > are no regressions. It is only relevant to DPDK 22.11 LTS so Cc
> > Xueming who will ultimately decide.
> >
> > A guide below on some things to consider for this type of backport is here:
> > http://doc.dpdk.org/guides/contributing/stable.html#what-changes-shoul
> > d-be-backported
> >
> > > > >
> > > > > > Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> > > > > > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> > > > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> > > > >
> > > >
> > >
> >
> 
> --
> Kind Regards,
> Niklas Söderlund

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-16 10:41           ` Chaoyong He
@ 2023-02-16 10:55             ` Niklas Soderlund
  2023-02-16 10:59             ` Ferruh Yigit
  1 sibling, 0 replies; 13+ messages in thread
From: Niklas Soderlund @ 2023-02-16 10:55 UTC (permalink / raw)
  To: Chaoyong He
  Cc: Kevin Traynor, Ferruh Yigit, Xueming(Steven) Li, dev,
	Luca Boccassi, oss-drivers, Nole Zhang, Kevin Liu, stable

Hello again,

On 2023-02-16 11:41:13 +0100, Chaoyong He wrote:
> 
> 
> > -----Original Message-----
> > From: Niklas Soderlund <niklas.soderlund@corigine.com>
> > Sent: Thursday, February 16, 2023 6:37 PM
> > To: Kevin Traynor <ktraynor@redhat.com>
> > Cc: Ferruh Yigit <ferruh.yigit@amd.com>; Xueming(Steven) Li
> > <xuemingl@nvidia.com>; Chaoyong He <chaoyong.he@corigine.com>;
> > dev@dpdk.org; Luca Boccassi <bluca@debian.org>; oss-drivers <oss-
> > drivers@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Kevin Liu
> > <jin.liu@corigine.com>; stable@dpdk.org
> > Subject: Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with
> > NFDk
> > 
> > Hi Kevin,
> > 
> > Thanks for your input.
> > 
> > On 2023-02-16 10:28:34 +0000, Kevin Traynor wrote:
> > > On 15/02/2023 18:28, Ferruh Yigit wrote:
> > > > On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
> > > > > Hi Ferruh,
> > > > >
> > > > > Thanks for your continues effort in dealing with NFP patches.
> > > > >
> > > > > On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
> > > > > > On 2/8/2023 9:15 AM, Chaoyong He wrote:
> > > > > > > From: Peng Zhang <peng.zhang@corigine.com>
> > > > > > >
> > > > > > > 48-bit DMA address is supported in the firmware with NFDk, so
> > > > > > > enable this feature in PMD now. But the firmware with NFD3
> > > > > > > still just support 40-bit DMA address.
> > > > > > >
> > > > > > > RX free list descriptor, used by both NFD3 and NFDk, is also
> > > > > > > modified to support 48-bit DMA address. That's OK because the
> > > > > > > top bits is always set to 0 when assigned with 40-bit DMA address.
> > > > > > >
> > > > > > > Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
> > > > > > > Cc: jin.liu@corigine.com
> > > > > > > Cc: stable@dpdk.org
> > > > > > >
> > > > > >
> > > > > > Why a backport is requested? As far as I understand this is not
> > > > > > fixing anything but extending device capability. Is this a fix?
> > > > >
> > > > > I agree this is a bit of a grey zone. We reasoned this was a fix
> > > > > as we should have done this from the start in the commit that
> > > > > added support for NFDk. Are you OK moving forward with this as a
> > > > > fix or would you prefer we resubmit without the request to backport?
> > > > >
> > > >
> > > > I am not sure, is this change have any potential to change behavior
> > > > for existing users?
> > > > Like if one of your user is using 22.11.1 release, and if this patch
> > > > backported to next LTS version, 22.11.2, will user notice any difference?
> > > >
> > > >
> > > > @Luca, @Kevin, what is your comment as LTS maintainers?
> > > >
> > >
> > > A bit difficult to know. If NFDk is not practicably usable without it,
> > > then it could be considered a fix. If it's just extending to add
> > > nice-to-have functionality then probably it is not a fix.
> > 
> > I think we can treat this as a nice-to-have and not something that makes
> > NFDk unusable. As stated above, we marked this as a Fix as we *really*
> > should have done this in the commit which added NFDk support.
> > 
> > @Ferruh, would you prefer we send a v2 or will you drop the Fixes and CC
> > tags when/if applying?
> > 
> 
> Actually, the DPDK app using the nfp card with a firmware of NFDk will coredump without this patch.
> And that's the directly reason we consider backport this patch.

Wops, you are right I was thinking of a different patch when replying.  
Sorry about the confusion.

Yes this is a bug fix that should be back ported. Thanks Chaoyong for 
correcting me.

> 
> > >
> > > It would need to ensure that it is tested on 22.11 branch and there
> > > are no regressions. It is only relevant to DPDK 22.11 LTS so Cc
> > > Xueming who will ultimately decide.
> > >
> > > A guide below on some things to consider for this type of backport is here:
> > > http://doc.dpdk.org/guides/contributing/stable.html#what-changes-shoul
> > > d-be-backported
> > >
> > > > > >
> > > > > > > Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> > > > > > > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> > > > > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> > > > > >
> > > > >
> > > >
> > >
> > 
> > --
> > Kind Regards,
> > Niklas Söderlund

-- 
Kind Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-16 10:41           ` Chaoyong He
  2023-02-16 10:55             ` Niklas Soderlund
@ 2023-02-16 10:59             ` Ferruh Yigit
  2023-02-16 11:11               ` Nole Zhang
  1 sibling, 1 reply; 13+ messages in thread
From: Ferruh Yigit @ 2023-02-16 10:59 UTC (permalink / raw)
  To: Chaoyong He, Niklas Soderlund, Kevin Traynor
  Cc: Xueming(Steven) Li, dev, Luca Boccassi, oss-drivers, Nole Zhang,
	Kevin Liu, stable

On 2/16/2023 10:41 AM, Chaoyong He wrote:
> 
> 
>> -----Original Message-----
>> From: Niklas Soderlund <niklas.soderlund@corigine.com>
>> Sent: Thursday, February 16, 2023 6:37 PM
>> To: Kevin Traynor <ktraynor@redhat.com>
>> Cc: Ferruh Yigit <ferruh.yigit@amd.com>; Xueming(Steven) Li
>> <xuemingl@nvidia.com>; Chaoyong He <chaoyong.he@corigine.com>;
>> dev@dpdk.org; Luca Boccassi <bluca@debian.org>; oss-drivers <oss-
>> drivers@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Kevin Liu
>> <jin.liu@corigine.com>; stable@dpdk.org
>> Subject: Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with
>> NFDk
>>
>> Hi Kevin,
>>
>> Thanks for your input.
>>
>> On 2023-02-16 10:28:34 +0000, Kevin Traynor wrote:
>>> On 15/02/2023 18:28, Ferruh Yigit wrote:
>>>> On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
>>>>> Hi Ferruh,
>>>>>
>>>>> Thanks for your continues effort in dealing with NFP patches.
>>>>>
>>>>> On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
>>>>>> On 2/8/2023 9:15 AM, Chaoyong He wrote:
>>>>>>> From: Peng Zhang <peng.zhang@corigine.com>
>>>>>>>
>>>>>>> 48-bit DMA address is supported in the firmware with NFDk, so
>>>>>>> enable this feature in PMD now. But the firmware with NFD3
>>>>>>> still just support 40-bit DMA address.
>>>>>>>
>>>>>>> RX free list descriptor, used by both NFD3 and NFDk, is also
>>>>>>> modified to support 48-bit DMA address. That's OK because the
>>>>>>> top bits is always set to 0 when assigned with 40-bit DMA address.
>>>>>>>
>>>>>>> Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
>>>>>>> Cc: jin.liu@corigine.com
>>>>>>> Cc: stable@dpdk.org
>>>>>>>
>>>>>>
>>>>>> Why a backport is requested? As far as I understand this is not
>>>>>> fixing anything but extending device capability. Is this a fix?
>>>>>
>>>>> I agree this is a bit of a grey zone. We reasoned this was a fix
>>>>> as we should have done this from the start in the commit that
>>>>> added support for NFDk. Are you OK moving forward with this as a
>>>>> fix or would you prefer we resubmit without the request to backport?
>>>>>
>>>>
>>>> I am not sure, is this change have any potential to change behavior
>>>> for existing users?
>>>> Like if one of your user is using 22.11.1 release, and if this patch
>>>> backported to next LTS version, 22.11.2, will user notice any difference?
>>>>
>>>>
>>>> @Luca, @Kevin, what is your comment as LTS maintainers?
>>>>
>>>
>>> A bit difficult to know. If NFDk is not practicably usable without it,
>>> then it could be considered a fix. If it's just extending to add
>>> nice-to-have functionality then probably it is not a fix.
>>
>> I think we can treat this as a nice-to-have and not something that makes
>> NFDk unusable. As stated above, we marked this as a Fix as we *really*
>> should have done this in the commit which added NFDk support.
>>
>> @Ferruh, would you prefer we send a v2 or will you drop the Fixes and CC
>> tags when/if applying?
>>
> 
> Actually, the DPDK app using the nfp card with a firmware of NFDk will coredump without this patch.
> And that's the directly reason we consider backport this patch.
> 

It has been long since NFDk FW support added, how a crash missed until
this point, is it crashing in a edge case or something?

>>>
>>> It would need to ensure that it is tested on 22.11 branch and there
>>> are no regressions. It is only relevant to DPDK 22.11 LTS so Cc
>>> Xueming who will ultimately decide.
>>>
>>> A guide below on some things to consider for this type of backport is here:
>>> http://doc.dpdk.org/guides/contributing/stable.html#what-changes-shoul
>>> d-be-backported
>>>
>>>>>>
>>>>>>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
>>>>>>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>>>>>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>>>>>
>>>>>
>>>>
>>>
>>
>> --
>> Kind Regards,
>> Niklas Söderlund


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-16 10:59             ` Ferruh Yigit
@ 2023-02-16 11:11               ` Nole Zhang
  2023-02-16 11:17                 ` Ferruh Yigit
  0 siblings, 1 reply; 13+ messages in thread
From: Nole Zhang @ 2023-02-16 11:11 UTC (permalink / raw)
  To: Ferruh Yigit, Chaoyong He, Niklas Soderlund, Kevin Traynor
  Cc: Xueming(Steven) Li, dev, Luca Boccassi, oss-drivers, Kevin Liu, stable



> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Thursday, February 16, 2023 7:00 PM
> To: Chaoyong He <chaoyong.he@corigine.com>; Niklas Soderlund
> <niklas.soderlund@corigine.com>; Kevin Traynor <ktraynor@redhat.com>
> Cc: Xueming(Steven) Li <xuemingl@nvidia.com>; dev@dpdk.org; Luca
> Boccassi <bluca@debian.org>; oss-drivers <oss-drivers@corigine.com>; Nole
> Zhang <peng.zhang@corigine.com>; Kevin Liu <jin.liu@corigine.com>;
> stable@dpdk.org
> Subject: Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with
> NFDk
> 
> On 2/16/2023 10:41 AM, Chaoyong He wrote:
> >
> >
> >> -----Original Message-----
> >> From: Niklas Soderlund <niklas.soderlund@corigine.com>
> >> Sent: Thursday, February 16, 2023 6:37 PM
> >> To: Kevin Traynor <ktraynor@redhat.com>
> >> Cc: Ferruh Yigit <ferruh.yigit@amd.com>; Xueming(Steven) Li
> >> <xuemingl@nvidia.com>; Chaoyong He <chaoyong.he@corigine.com>;
> >> dev@dpdk.org; Luca Boccassi <bluca@debian.org>; oss-drivers <oss-
> >> drivers@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Kevin
> >> Liu <jin.liu@corigine.com>; stable@dpdk.org
> >> Subject: Re: [PATCH] net/nfp: support 48-bit DMA address for firmware
> >> with NFDk
> >>
> >> Hi Kevin,
> >>
> >> Thanks for your input.
> >>
> >> On 2023-02-16 10:28:34 +0000, Kevin Traynor wrote:
> >>> On 15/02/2023 18:28, Ferruh Yigit wrote:
> >>>> On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
> >>>>> Hi Ferruh,
> >>>>>
> >>>>> Thanks for your continues effort in dealing with NFP patches.
> >>>>>
> >>>>> On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
> >>>>>> On 2/8/2023 9:15 AM, Chaoyong He wrote:
> >>>>>>> From: Peng Zhang <peng.zhang@corigine.com>
> >>>>>>>
> >>>>>>> 48-bit DMA address is supported in the firmware with NFDk, so
> >>>>>>> enable this feature in PMD now. But the firmware with NFD3 still
> >>>>>>> just support 40-bit DMA address.
> >>>>>>>
> >>>>>>> RX free list descriptor, used by both NFD3 and NFDk, is also
> >>>>>>> modified to support 48-bit DMA address. That's OK because the
> >>>>>>> top bits is always set to 0 when assigned with 40-bit DMA address.
> >>>>>>>
> >>>>>>> Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
> >>>>>>> Cc: jin.liu@corigine.com
> >>>>>>> Cc: stable@dpdk.org
> >>>>>>>
> >>>>>>
> >>>>>> Why a backport is requested? As far as I understand this is not
> >>>>>> fixing anything but extending device capability. Is this a fix?
> >>>>>
> >>>>> I agree this is a bit of a grey zone. We reasoned this was a fix
> >>>>> as we should have done this from the start in the commit that
> >>>>> added support for NFDk. Are you OK moving forward with this as a
> >>>>> fix or would you prefer we resubmit without the request to backport?
> >>>>>
> >>>>
> >>>> I am not sure, is this change have any potential to change behavior
> >>>> for existing users?
> >>>> Like if one of your user is using 22.11.1 release, and if this
> >>>> patch backported to next LTS version, 22.11.2, will user notice any
> difference?
> >>>>
> >>>>
> >>>> @Luca, @Kevin, what is your comment as LTS maintainers?
> >>>>
> >>>
> >>> A bit difficult to know. If NFDk is not practicably usable without
> >>> it, then it could be considered a fix. If it's just extending to add
> >>> nice-to-have functionality then probably it is not a fix.
> >>
> >> I think we can treat this as a nice-to-have and not something that
> >> makes NFDk unusable. As stated above, we marked this as a Fix as we
> >> *really* should have done this in the commit which added NFDk support.
> >>
> >> @Ferruh, would you prefer we send a v2 or will you drop the Fixes and
> >> CC tags when/if applying?
> >>
> >
> > Actually, the DPDK app using the nfp card with a firmware of NFDk will
> coredump without this patch.
> > And that's the directly reason we consider backport this patch.
> >
> 
> It has been long since NFDk FW support added, how a crash missed until this
> point, is it crashing in a edge case or something?
> 
Yes, this occur in the server with CPU FT-2000/64, it has 2 PCIE1 x8 and 1 PCIE0 x16,
Pcie x8 can only support 48 bit, but the pcie16 can support 40bit.
> >>>
> >>> It would need to ensure that it is tested on 22.11 branch and there
> >>> are no regressions. It is only relevant to DPDK 22.11 LTS so Cc
> >>> Xueming who will ultimately decide.
> >>>
> >>> A guide below on some things to consider for this type of backport is
> here:
> >>> http://doc.dpdk.org/guides/contributing/stable.html#what-changes-sho
> >>> ul
> >>> d-be-backported
> >>>
> >>>>>>
> >>>>>>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> >>>>>>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> >>>>>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >> --
> >> Kind Regards,
> >> Niklas Söderlund


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk
  2023-02-16 11:11               ` Nole Zhang
@ 2023-02-16 11:17                 ` Ferruh Yigit
  0 siblings, 0 replies; 13+ messages in thread
From: Ferruh Yigit @ 2023-02-16 11:17 UTC (permalink / raw)
  To: Nole Zhang, Chaoyong He, Niklas Soderlund, Kevin Traynor
  Cc: Xueming(Steven) Li, dev, Luca Boccassi, oss-drivers, Kevin Liu, stable

On 2/16/2023 11:11 AM, Nole Zhang wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Thursday, February 16, 2023 7:00 PM
>> To: Chaoyong He <chaoyong.he@corigine.com>; Niklas Soderlund
>> <niklas.soderlund@corigine.com>; Kevin Traynor <ktraynor@redhat.com>
>> Cc: Xueming(Steven) Li <xuemingl@nvidia.com>; dev@dpdk.org; Luca
>> Boccassi <bluca@debian.org>; oss-drivers <oss-drivers@corigine.com>; Nole
>> Zhang <peng.zhang@corigine.com>; Kevin Liu <jin.liu@corigine.com>;
>> stable@dpdk.org
>> Subject: Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with
>> NFDk
>>
>> On 2/16/2023 10:41 AM, Chaoyong He wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Niklas Soderlund <niklas.soderlund@corigine.com>
>>>> Sent: Thursday, February 16, 2023 6:37 PM
>>>> To: Kevin Traynor <ktraynor@redhat.com>
>>>> Cc: Ferruh Yigit <ferruh.yigit@amd.com>; Xueming(Steven) Li
>>>> <xuemingl@nvidia.com>; Chaoyong He <chaoyong.he@corigine.com>;
>>>> dev@dpdk.org; Luca Boccassi <bluca@debian.org>; oss-drivers <oss-
>>>> drivers@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Kevin
>>>> Liu <jin.liu@corigine.com>; stable@dpdk.org
>>>> Subject: Re: [PATCH] net/nfp: support 48-bit DMA address for firmware
>>>> with NFDk
>>>>
>>>> Hi Kevin,
>>>>
>>>> Thanks for your input.
>>>>
>>>> On 2023-02-16 10:28:34 +0000, Kevin Traynor wrote:
>>>>> On 15/02/2023 18:28, Ferruh Yigit wrote:
>>>>>> On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
>>>>>>> Hi Ferruh,
>>>>>>>
>>>>>>> Thanks for your continues effort in dealing with NFP patches.
>>>>>>>
>>>>>>> On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
>>>>>>>> On 2/8/2023 9:15 AM, Chaoyong He wrote:
>>>>>>>>> From: Peng Zhang <peng.zhang@corigine.com>
>>>>>>>>>
>>>>>>>>> 48-bit DMA address is supported in the firmware with NFDk, so
>>>>>>>>> enable this feature in PMD now. But the firmware with NFD3 still
>>>>>>>>> just support 40-bit DMA address.
>>>>>>>>>
>>>>>>>>> RX free list descriptor, used by both NFD3 and NFDk, is also
>>>>>>>>> modified to support 48-bit DMA address. That's OK because the
>>>>>>>>> top bits is always set to 0 when assigned with 40-bit DMA address.
>>>>>>>>>
>>>>>>>>> Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
>>>>>>>>> Cc: jin.liu@corigine.com
>>>>>>>>> Cc: stable@dpdk.org
>>>>>>>>>
>>>>>>>>
>>>>>>>> Why a backport is requested? As far as I understand this is not
>>>>>>>> fixing anything but extending device capability. Is this a fix?
>>>>>>>
>>>>>>> I agree this is a bit of a grey zone. We reasoned this was a fix
>>>>>>> as we should have done this from the start in the commit that
>>>>>>> added support for NFDk. Are you OK moving forward with this as a
>>>>>>> fix or would you prefer we resubmit without the request to backport?
>>>>>>>
>>>>>>
>>>>>> I am not sure, is this change have any potential to change behavior
>>>>>> for existing users?
>>>>>> Like if one of your user is using 22.11.1 release, and if this
>>>>>> patch backported to next LTS version, 22.11.2, will user notice any
>> difference?
>>>>>>
>>>>>>
>>>>>> @Luca, @Kevin, what is your comment as LTS maintainers?
>>>>>>
>>>>>
>>>>> A bit difficult to know. If NFDk is not practicably usable without
>>>>> it, then it could be considered a fix. If it's just extending to add
>>>>> nice-to-have functionality then probably it is not a fix.
>>>>
>>>> I think we can treat this as a nice-to-have and not something that
>>>> makes NFDk unusable. As stated above, we marked this as a Fix as we
>>>> *really* should have done this in the commit which added NFDk support.
>>>>
>>>> @Ferruh, would you prefer we send a v2 or will you drop the Fixes and
>>>> CC tags when/if applying?
>>>>
>>>
>>> Actually, the DPDK app using the nfp card with a firmware of NFDk will
>> coredump without this patch.
>>> And that's the directly reason we consider backport this patch.
>>>
>>
>> It has been long since NFDk FW support added, how a crash missed until this
>> point, is it crashing in a edge case or something?
>>
> Yes, this occur in the server with CPU FT-2000/64, it has 2 PCIE1 x8 and 1 PCIE0 x16,
> Pcie x8 can only support 48 bit, but the pcie16 can support 40bit.

OK, can you please send a new version of the patch updating patch title
and commit log to highlight that patch is fixing a crash.

It can be helpful to detail the reason of crash in commit log.

>>>>>
>>>>> It would need to ensure that it is tested on 22.11 branch and there
>>>>> are no regressions. It is only relevant to DPDK 22.11 LTS so Cc
>>>>> Xueming who will ultimately decide.
>>>>>
>>>>> A guide below on some things to consider for this type of backport is
>> here:
>>>>> http://doc.dpdk.org/guides/contributing/stable.html#what-changes-sho
>>>>> ul
>>>>> d-be-backported
>>>>>
>>>>>>>>
>>>>>>>>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
>>>>>>>>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>>>>>>>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>> --
>>>> Kind Regards,
>>>> Niklas Söderlund
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] net/nfp: fix 48-bit DMA address support for NFDk
  2023-02-08  9:15 [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk Chaoyong He
  2023-02-15 13:42 ` Ferruh Yigit
@ 2023-02-20  9:02 ` Chaoyong He
  2023-02-20 14:02   ` Ferruh Yigit
  1 sibling, 1 reply; 13+ messages in thread
From: Chaoyong He @ 2023-02-20  9:02 UTC (permalink / raw)
  To: dev
  Cc: oss-drivers, niklas.soderlund, Peng Zhang, jin.liu, stable, Chaoyong He

From: Peng Zhang <peng.zhang@corigine.com>

Initializing of the NFP PMD fails when a NFDk device is used in a PCIe
slot that supports 48-bit DMA address. The failure is due to an
incorrect check by the PMD that limits the support to 40-bit DMA
address. While this check is correct for NFD3 devices, it is incorrect
for NFDk that can support 48-bit DMA address.

Fix this by correcting the DMA mask check at initialization to allow for
different DMA address masks for NFD3 and NFDk.

The RX free list descriptor code is also updated to allow for 48-bit DMA
address. While this code is shared by the NFD3 and NFDk code paths,
this is not an issue as for 40-bit address the top bits are always 0.

Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
Cc: jin.liu@corigine.com
Cc: stable@dpdk.org

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>

---
v2:
* Rewrite the commit message.

---
 drivers/net/nfp/flower/nfp_flower.c      | 12 ++++--------
 drivers/net/nfp/flower/nfp_flower_ctrl.c |  2 +-
 drivers/net/nfp/nfp_common.c             | 18 ++++++++++++++++++
 drivers/net/nfp/nfp_common.h             |  1 +
 drivers/net/nfp/nfp_ethdev.c             | 11 +++--------
 drivers/net/nfp/nfp_ethdev_vf.c          | 11 +++--------
 drivers/net/nfp/nfp_rxtx.c               |  4 ++--
 drivers/net/nfp/nfp_rxtx.h               |  4 ++--
 8 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/drivers/net/nfp/flower/nfp_flower.c b/drivers/net/nfp/flower/nfp_flower.c
index f1424a010d..11bf277f42 100644
--- a/drivers/net/nfp/flower/nfp_flower.c
+++ b/drivers/net/nfp/flower/nfp_flower.c
@@ -452,7 +452,7 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
 		rxds->vals[1] = 0;
 		dma_addr = rte_cpu_to_le_64(RTE_MBUF_DMA_ADDR_DEFAULT(new_mb));
 		rxds->fld.dd = 0;
-		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
 		rxds->fld.dma_addr_lo = dma_addr & 0xffffffff;
 		nb_hold++;
 
@@ -629,13 +629,6 @@ nfp_flower_init_vnic_common(struct nfp_net_hw *hw, const char *vnic_type)
 	pf_dev = hw->pf_dev;
 	pci_dev = hw->pf_dev->pci_dev;
 
-	/* NFP can not handle DMA addresses requiring more than 40 bits */
-	if (rte_mem_check_dma_mask(40)) {
-		PMD_INIT_LOG(ERR, "Device %s can not be used: restricted dma mask to 40 bits!\n",
-				pci_dev->device.name);
-		return -ENODEV;
-	};
-
 	hw->device_id = pci_dev->id.device_id;
 	hw->vendor_id = pci_dev->id.vendor_id;
 	hw->subsystem_device_id = pci_dev->id.subsystem_device_id;
@@ -664,6 +657,9 @@ nfp_flower_init_vnic_common(struct nfp_net_hw *hw, const char *vnic_type)
 	hw->mtu = hw->max_mtu;
 	hw->flbufsz = DEFAULT_FLBUF_SIZE;
 
+	if (nfp_net_check_dma_mask(hw, pci_dev->name) != 0)
+		return -ENODEV;
+
 	/* read the Rx offset configured from firmware */
 	if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2)
 		hw->rx_offset = NFP_NET_RX_OFFSET;
diff --git a/drivers/net/nfp/flower/nfp_flower_ctrl.c b/drivers/net/nfp/flower/nfp_flower_ctrl.c
index 03a2e2e622..b134a74bd8 100644
--- a/drivers/net/nfp/flower/nfp_flower_ctrl.c
+++ b/drivers/net/nfp/flower/nfp_flower_ctrl.c
@@ -122,7 +122,7 @@ nfp_flower_ctrl_vnic_recv(void *rx_queue,
 		rxds->vals[1] = 0;
 		dma_addr = rte_cpu_to_le_64(RTE_MBUF_DMA_ADDR_DEFAULT(new_mb));
 		rxds->fld.dd = 0;
-		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
 		rxds->fld.dma_addr_lo = dma_addr & 0xffffffff;
 		nb_hold++;
 
diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
index 5d6e1a742d..4d36adc377 100644
--- a/drivers/net/nfp/nfp_common.c
+++ b/drivers/net/nfp/nfp_common.c
@@ -1566,6 +1566,24 @@ nfp_net_set_vxlan_port(struct nfp_net_hw *hw,
 	return ret;
 }
 
+/*
+ * The firmware with NFD3 can not handle DMA address requiring more
+ * than 40 bits
+ */
+int
+nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name)
+{
+	if (NFD_CFG_CLASS_VER_of(hw->ver) == NFP_NET_CFG_VERSION_DP_NFD3 &&
+			rte_mem_check_dma_mask(40) != 0) {
+		PMD_DRV_LOG(ERR,
+			"The device %s can't be used: restricted dma mask to 40 bits!",
+			name);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 RTE_LOG_REGISTER_SUFFIX(nfp_logtype_init, init, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(nfp_logtype_driver, driver, NOTICE);
 RTE_LOG_REGISTER_SUFFIX(nfp_logtype_cpp, cpp, NOTICE);
diff --git a/drivers/net/nfp/nfp_common.h b/drivers/net/nfp/nfp_common.h
index 56b7edc951..980f3cad89 100644
--- a/drivers/net/nfp/nfp_common.h
+++ b/drivers/net/nfp/nfp_common.h
@@ -454,6 +454,7 @@ int nfp_net_rx_desc_limits(struct nfp_net_hw *hw,
 int nfp_net_tx_desc_limits(struct nfp_net_hw *hw,
 		uint16_t *min_tx_desc,
 		uint16_t *max_tx_desc);
+int nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name);
 
 #define NFP_NET_DEV_PRIVATE_TO_HW(adapter)\
 	(&((struct nfp_net_adapter *)adapter)->hw)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 290e2fcb41..fed7b1ab13 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -519,14 +519,6 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 	/* Use backpointer to the CoreNIC app struct */
 	app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
 
-	/* NFP can not handle DMA addresses requiring more than 40 bits */
-	if (rte_mem_check_dma_mask(40)) {
-		RTE_LOG(ERR, PMD,
-			"device %s can not be used: restricted dma mask to 40 bits!\n",
-			pci_dev->device.name);
-		return -ENODEV;
-	}
-
 	port = ((struct nfp_net_hw *)eth_dev->data->dev_private)->idx;
 	if (port < 0 || port > 7) {
 		PMD_DRV_LOG(ERR, "Port value is wrong");
@@ -574,6 +566,9 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 
 	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
 
+	if (nfp_net_check_dma_mask(hw, pci_dev->name) != 0)
+		return -ENODEV;
+
 	if (nfp_net_ethdev_ops_mount(hw, eth_dev))
 		return -EINVAL;
 
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index 07a2e17ef8..c1f8a0fa0f 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -291,14 +291,6 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 
 	pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 
-	/* NFP can not handle DMA addresses requiring more than 40 bits */
-	if (rte_mem_check_dma_mask(40)) {
-		RTE_LOG(ERR, PMD,
-			"device %s can not be used: restricted dma mask to 40 bits!\n",
-			pci_dev->device.name);
-		return -ENODEV;
-	}
-
 	hw = NFP_NET_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
 
 	hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
@@ -312,6 +304,9 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
 
 	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
 
+	if (nfp_net_check_dma_mask(hw, pci_dev->name) != 0)
+		return -ENODEV;
+
 	if (nfp_netvf_ethdev_ops_mount(hw, eth_dev))
 		return -EINVAL;
 
diff --git a/drivers/net/nfp/nfp_rxtx.c b/drivers/net/nfp/nfp_rxtx.c
index cfc1a784b1..12ca8381e4 100644
--- a/drivers/net/nfp/nfp_rxtx.c
+++ b/drivers/net/nfp/nfp_rxtx.c
@@ -48,7 +48,7 @@ nfp_net_rx_fill_freelist(struct nfp_net_rxq *rxq)
 
 		rxd = &rxq->rxds[i];
 		rxd->fld.dd = 0;
-		rxd->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+		rxd->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
 		rxd->fld.dma_addr_lo = dma_addr & 0xffffffff;
 		rxe[i].mbuf = mbuf;
 		PMD_RX_LOG(DEBUG, "[%d]: %" PRIx64, i, dma_addr);
@@ -454,7 +454,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		rxds->vals[1] = 0;
 		dma_addr = rte_cpu_to_le_64(RTE_MBUF_DMA_ADDR_DEFAULT(new_mb));
 		rxds->fld.dd = 0;
-		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+		rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
 		rxds->fld.dma_addr_lo = dma_addr & 0xffffffff;
 		nb_hold++;
 
diff --git a/drivers/net/nfp/nfp_rxtx.h b/drivers/net/nfp/nfp_rxtx.h
index cb67657014..5e651518ed 100644
--- a/drivers/net/nfp/nfp_rxtx.h
+++ b/drivers/net/nfp/nfp_rxtx.h
@@ -288,8 +288,8 @@ struct nfp_net_rx_desc {
 	union {
 		/* Freelist descriptor */
 		struct {
-			uint8_t dma_addr_hi;
-			__le16 spare;
+			__le16 dma_addr_hi;
+			uint8_t spare;
 			uint8_t dd;
 
 			__le32 dma_addr_lo;
-- 
2.29.3


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] net/nfp: fix 48-bit DMA address support for NFDk
  2023-02-20  9:02 ` [PATCH v2] net/nfp: fix 48-bit DMA address support for NFDk Chaoyong He
@ 2023-02-20 14:02   ` Ferruh Yigit
  0 siblings, 0 replies; 13+ messages in thread
From: Ferruh Yigit @ 2023-02-20 14:02 UTC (permalink / raw)
  To: Chaoyong He, dev
  Cc: oss-drivers, niklas.soderlund, Peng Zhang, jin.liu, stable

On 2/20/2023 9:02 AM, Chaoyong He wrote:
> From: Peng Zhang <peng.zhang@corigine.com>
> 
> Initializing of the NFP PMD fails when a NFDk device is used in a PCIe
> slot that supports 48-bit DMA address. The failure is due to an
> incorrect check by the PMD that limits the support to 40-bit DMA
> address. While this check is correct for NFD3 devices, it is incorrect
> for NFDk that can support 48-bit DMA address.
> 
> Fix this by correcting the DMA mask check at initialization to allow for
> different DMA address masks for NFD3 and NFDk.
> 
> The RX free list descriptor code is also updated to allow for 48-bit DMA
> address. While this code is shared by the NFD3 and NFDk code paths,
> this is not an issue as for 40-bit address the top bits are always 0.
> 
> Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
> Cc: jin.liu@corigine.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>

Applied to dpdk-next-net/main, thanks.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-02-20 14:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-08  9:15 [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk Chaoyong He
2023-02-15 13:42 ` Ferruh Yigit
2023-02-15 17:47   ` Niklas Söderlund
2023-02-15 18:28     ` Ferruh Yigit
2023-02-16 10:28       ` Kevin Traynor
2023-02-16 10:37         ` Niklas Söderlund
2023-02-16 10:41           ` Chaoyong He
2023-02-16 10:55             ` Niklas Soderlund
2023-02-16 10:59             ` Ferruh Yigit
2023-02-16 11:11               ` Nole Zhang
2023-02-16 11:17                 ` Ferruh Yigit
2023-02-20  9:02 ` [PATCH v2] net/nfp: fix 48-bit DMA address support for NFDk Chaoyong He
2023-02-20 14:02   ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).