[PATCH net-next V2 00/13] net/mlx5e: Add netdev support for data direct

[PATCH net-next V2 00/13] net/mlx5e: Add netdev support for data direct

[Tariq Toukan]

Hi,

This patch series by Dragos adds data direct support for netdev devices,
allowing a mlx5 netdev to issue DMA commands through multiple paths.
This feature is critical for improving performance and reaching line
rate in certain environments where issuing PCI transactions over one
path may be significantly faster than over another. These differences
can arise from various PCI generations in the system or the specific
system topology.

Data direct for netdev will work with devmem by returning the data
direct DMA device instead of the DMA device of the NIC. Each PF netdev
registers its own data direct device and support for RX and TX datapath
is added.

The feature is enabled though a ethtool private flag. When this flag
is enabled, the driver will return the data direct device in the
ndo_queue_get_dma_dev op.

When the data direct device is unbound, the netdev is closed, the
memory providers and TX dmabuf bindings mapped against the device are
unmapped through the new netdev_unbind_dma_dev() core helper, the flag
is disabled and the netdev is reopened. The helper does not care which
memory provider is installed: devmem and io_uring zcrx are both
uninstalled through their mp_ops->uninstall callback, like on netdev
unregister.

Regards,
Tariq

V2:
- Patch 2 (new): Detach auxiliary devices on attach failure during load
  in mlx5_load_one_devl_locked().
- Patch 3: Move the data direct notifier unregister to after
  mlx5e_nic_set_rx_mode() in mlx5e_nic_disable(), alongside the other
  NIC-disable cleanups.
- Patch 4: Reword the commit message.
- Patch 6: Size the ICOSQ for the UMR mode of the RQ, since KSM UMR WQEs
  map half the pages of MTT ones.
- Patch 6: Initialize umr_mkey to avoid a maybe-uninitialized warning.
- Patch 7 (new): Use the page_pool DMA sync helper for SHAMPO SKB frags,
  as unreadable netmem is mapped against the data direct device rather
  than rq->pdev.
- Patch 8: Document why the data_direct private flag is not propagated
  to PTP SQs.
- Patch 9: Return the data direct device from ndo_queue_get_dma_dev and
  use it for the page_pool of data direct RQs. Fail with -ENODEV if the
  device is gone. Reject leased RX queues in data direct mode.
  Bound-check queue_index.
- Patch 10: Walk the bindings xarray and report only bindings that are
  still mapped, instead of tracking them on a separate list.
- Patch 11 (new): Add the netdev_unbind_dma_dev() core helper, which
  uninstalls the memory providers (devmem and io_uring zcrx) and unmaps
  TX bindings of the departing DMA device.
- Patch 12: Replace the channel re-creation on unbind with closing the
  netdev, calling netdev_unbind_dma_dev(), disabling the flag and
  reopening. Also release the providers on suspend. This addresses the
  dmabuf binding limitation noted in V1.
- Patch 13: Reject enabling data direct on uplink representors and in
  socket direct mode. Block flag changes while a memory provider is
  installed on an RX queue or RX queues are leased.
- Add Reviewed-by tags from Cosmin where applicable.
- Drop the "SHAMPO required" gap: only enable data direct on an RX queue
  with an unreadable memory provider when SHAMPO is active.

V1:
https://lore.kernel.org/all/20261008132815.654147-1-tariqt@nvidia.com/

Dragos Tatulea (13):
  net/mlx5: Log the data direct to PF device mapping
  net/mlx5: Detach auxiliary devices on attach failure during load
  net/mlx5e: Register supported netdevs as data direct users
  net/mlx5e: Pre-calculate UMR padding and entry size
  net/mlx5e: Add data direct ethtool private flag
  net/mlx5e: Add data direct RX infrastructure
  net/mlx5e: SHAMPO, Use page_pool DMA sync helper for SKB frags
  net/mlx5e: Add data direct TX infrastructure
  net/mlx5e: Use the correct DMA dev when data_direct is enabled
  net: devmem: add netdev_has_dmabuf_binding() helper
  net: add netdev_unbind_dma_dev() helper
  net/mlx5e: Disable data direct on data direct device unbind
  net/mlx5e: Enable the data direct netdev feature

 .../ethernet/mellanox/mlx5/core/data_direct.c |   9 +-
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  14 +
 .../ethernet/mellanox/mlx5/core/en/params.c   |  18 +-
 .../ethernet/mellanox/mlx5/core/en/params.h   |   2 +
 .../mellanox/mlx5/core/en/reporter_rx.c       |   1 +
 .../mellanox/mlx5/core/en/reporter_tx.c       |   1 +
 .../ethernet/mellanox/mlx5/core/en_common.c   |  38 ++-
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  |  67 +++++
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 253 +++++++++++++++++-
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  33 +--
 .../net/ethernet/mellanox/mlx5/core/en_tx.c   |   4 +-
 .../net/ethernet/mellanox/mlx5/core/main.c    |   1 +
 include/linux/mlx5/driver.h                   |   1 +
 include/net/netdev_queues.h                   |   1 +
 include/net/netmem.h                          |  24 ++
 net/core/devmem.c                             |  85 ++++++
 net/core/devmem.h                             |   6 +
 net/core/netdev_queues.c                      |  51 ++++
 18 files changed, 573 insertions(+), 36 deletions(-)


base-commit: fa5196392691051963726787944db872420dfa75
-- 
2.44.0

[PATCH net-next V2 01/13] net/mlx5: Log the data direct to PF device mapping

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

Emit an informational message when a data direct device is bound to a
PF, showing the data direct BDF alongside the PF BDF (the mlx5_core log
prefix), to make the mapping observable from dmesg.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/data_direct.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c b/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c
index 5afb11a2a9f0..7db94b8a5b79 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c
@@ -21,6 +21,7 @@ static LIST_HEAD(mlx5_data_direct_reg_list);
 static DEFINE_MUTEX(mlx5_data_direct_mutex);
 
 struct mlx5_data_direct_registration {
+	struct mlx5_core_dev *mdev;
 	char vuid[MLX5_ST_SZ_BYTES(array1024_auto) + 1];
 	struct list_head list;
 	struct blocking_notifier_head users;
@@ -189,6 +190,9 @@ static void mlx5_data_direct_bind(struct mlx5_data_direct_registration *reg,
 				  struct mlx5_data_direct_dev *dev)
 {
 	WRITE_ONCE(reg->dd.dev, dev);
+
+	mlx5_core_info(reg->mdev, "bound data direct device %s\n",
+		       pci_name(dev->pdev));
 }
 
 static void
@@ -212,6 +216,7 @@ int mlx5_data_direct_init(struct mlx5_core_dev *mdev)
 	if (!reg)
 		return -ENOMEM;
 
+	reg->mdev = mdev;
 	BLOCKING_INIT_NOTIFIER_HEAD(&reg->users);
 
 	err = mlx5_data_direct_query_vuid(mdev, reg->vuid);
-- 
2.44.0

[PATCH net-next V2 02/13] net/mlx5: Detach auxiliary devices on attach failure during load

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

The device attach phase fails in mlx5_load_one_devl_locked() when
the first device fails to attach. The error path doesn't detach
the remaining devices that successfully attached.

Fix the above condition by properly calling detach on the error
path.

This issue was found during code inspection and was not reproduced.
Hence it doesn't deserve a Fixes tag.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 1a45474ce03d..7e762f72b19e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1547,6 +1547,7 @@ int mlx5_load_one_devl_locked(struct mlx5_core_dev *dev, bool recovery)
 	return 0;
 
 err_attach:
+	mlx5_detach_device(dev, true);
 	clear_bit(MLX5_INTERFACE_STATE_UP, &dev->intf_state);
 	mlx5_unload(dev);
 err_load:
-- 
2.44.0

[PATCH net-next V2 03/13] net/mlx5e: Register supported netdevs as data direct users

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

Register the netdev as a data direct user so it is notified when the
affiliated data direct device is unbound.

Register during NIC enable and unregister during disable so suspend
and resume follow the data direct registration lifetime in mlx5_core.

The notifier callback only traces the unbind event for now; tearing down
the data direct datapath is wired up together with that datapath in
later patches.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  1 +
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 20 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index a3945195f454..78cf6aedd04d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -984,6 +984,7 @@ struct mlx5e_priv {
 	struct mlx5e_mqprio_rl    *mqprio_rl;
 	struct dentry             *dfs_root;
 	struct mlx5_devcom_comp_dev *devcom;
+	struct notifier_block      data_direct_nb;
 };
 
 static inline u16 mlx5e_stats_nch_read(const struct mlx5e_priv *priv)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index c3c2c5720c6f..1993da0fdb91 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -33,6 +33,7 @@
 #include <linux/dim.h>
 #include <net/tc_act/tc_gact.h>
 #include <linux/mlx5/fs.h>
+#include <linux/mlx5/data_direct.h>
 #include <net/vxlan.h>
 #include <net/geneve.h>
 #include <linux/bpf.h>
@@ -6022,6 +6023,18 @@ void mlx5e_destroy_q_counters(struct mlx5e_priv *priv)
 	}
 }
 
+static int mlx5e_data_direct_event(struct notifier_block *nb,
+				   unsigned long action, void *data)
+{
+	struct mlx5e_priv *priv = container_of(nb, struct mlx5e_priv,
+					       data_direct_nb);
+
+	if (action == MLX5_DATA_DIRECT_UNBIND)
+		mlx5_core_dbg(priv->mdev, "data direct device unbound\n");
+
+	return NOTIFY_OK;
+}
+
 static int mlx5e_nic_init(struct mlx5_core_dev *mdev,
 			  struct net_device *netdev)
 {
@@ -6224,6 +6237,12 @@ static int mlx5e_nic_enable(struct mlx5e_priv *priv)
 
 	mlx5e_pcie_cong_event_init(priv);
 	mlx5e_hv_vhca_stats_create(priv);
+
+	if (mlx5_data_direct_supported(mdev)) {
+		priv->data_direct_nb.notifier_call = mlx5e_data_direct_event;
+		mlx5_data_direct_register(mdev, &priv->data_direct_nb);
+	}
+
 	if (netdev->reg_state != NETREG_REGISTERED)
 		return 0;
 	mlx5e_dcbnl_init_app(priv);
@@ -6268,6 +6287,7 @@ static void mlx5e_nic_disable(struct mlx5e_priv *priv)
 
 	mlx5e_nic_set_rx_mode(priv);
 
+	mlx5_data_direct_unregister(mdev, &priv->data_direct_nb);
 	mlx5e_pcie_cong_event_cleanup(priv);
 	mlx5e_hv_vhca_stats_destroy(priv);
 	if (mlx5e_monitor_counter_supported(priv))
-- 
2.44.0

[PATCH net-next V2 04/13] net/mlx5e: Pre-calculate UMR padding and entry size

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

A future patch will add KSM UMRs alongside MTT UMRs for data direct. To
avoid additional ifs in the datapath, pre-calculate the UMR padding and
entry size based on the UMR mode of the RQ.

An additional side benefit is that the padding check from
mlx5e_alloc_rx_mpwqe() is no longer needed.

This patch doesn't introduce any functional changes. The mlx5e_rq struct
doesn't grow because the new members were added in a hole.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h     |  3 +++
 .../net/ethernet/mellanox/mlx5/core/en_main.c    | 11 +++++++++++
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c  | 16 ++++++----------
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 78cf6aedd04d..000493f42912 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -696,6 +696,9 @@ struct mlx5e_rq {
 			u8                     umr_wqebbs;
 			u8                     mtts_per_wqe;
 			u8                     umr_mode;
+			u8                     umr_entry_size;
+			u8                     umr_pad_bytes;
+			u16                    umr_pad_offset;
 			struct mlx5e_mpw_linear_info *linear_info;
 			struct mlx5e_shampo_hd *shampo;
 		} mpwqe;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 1993da0fdb91..96b521d8bed3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -901,6 +901,7 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
 	void *rqc_wq = MLX5_ADDR_OF(rqc, rq_param->rqc, wq);
 	struct mlx5_core_dev *mdev = rq->mdev;
 	u32 pool_order = 0;
+	u16 entries_bytes;
 	u32 pool_size;
 	int wq_sz;
 	int err;
@@ -947,6 +948,16 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
 			mlx5e_mpwrq_mtts_per_wqe(mdev, rq->mpwqe.page_shift,
 						 rq->mpwqe.umr_mode);
 
+		rq->mpwqe.umr_entry_size =
+			mlx5e_mpwrq_umr_entry_size(rq->mpwqe.umr_mode);
+		entries_bytes = rq->mpwqe.pages_per_wqe *
+				rq->mpwqe.umr_entry_size;
+		rq->mpwqe.umr_pad_offset = entries_bytes;
+		/* padding can be 0 when no padding is needed */
+		rq->mpwqe.umr_pad_bytes =
+			ALIGN(entries_bytes, MLX5_UMR_FLEX_ALIGNMENT) -
+			entries_bytes;
+
 		pool_size = rq->mpwqe.pages_per_wqe <<
 			mlx5e_mpwqe_get_log_rq_size(mdev, params, rqo);
 		pool_order = rq->mpwqe.page_shift - PAGE_SHIFT;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 22fea4739a2c..a9392ee5376e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -675,16 +675,11 @@ static int mlx5e_alloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
 		};
 	}
 
-	/* Pad if needed, in case the value set to ucseg->xlt_octowords
-	 * in mlx5e_build_umr_wqe() needed alignment.
+	/* Pad when the value set to ucseg->xlt_octowords in
+	 * mlx5e_build_umr_wqe() needed alignment.
 	 */
-	if (rq->mpwqe.pages_per_wqe & (MLX5_UMR_MTT_NUM_ENTRIES_ALIGNMENT - 1)) {
-		int pad = ALIGN(rq->mpwqe.pages_per_wqe, MLX5_UMR_MTT_NUM_ENTRIES_ALIGNMENT) -
-			rq->mpwqe.pages_per_wqe;
-
-		memset(&umr_wqe->inline_mtts[rq->mpwqe.pages_per_wqe], 0,
-		       sizeof(*umr_wqe->inline_mtts) * pad);
-	}
+	memset((u8 *)umr_wqe->inline_mtts + rq->mpwqe.umr_pad_offset, 0,
+	       rq->mpwqe.umr_pad_bytes);
 
 	bitmap_zero(wi->skip_release_bitmap, rq->mpwqe.pages_per_wqe);
 	wi->consumed_strides = 0;
@@ -693,7 +688,8 @@ static int mlx5e_alloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
 		cpu_to_be32((sq->pc << MLX5_WQE_CTRL_WQE_INDEX_SHIFT) |
 			    MLX5_OPCODE_UMR);
 
-	offset = (ix * rq->mpwqe.mtts_per_wqe) * sizeof(struct mlx5_mtt) / MLX5_OCTWORD;
+	offset = ix * rq->mpwqe.mtts_per_wqe *
+		 rq->mpwqe.umr_entry_size / MLX5_OCTWORD;
 	umr_wqe->hdr.uctrl.xlt_offset = cpu_to_be16(offset);
 
 	sq->db.wqe_info[pi] = (struct mlx5e_icosq_wqe_info) {
-- 
2.44.0

[PATCH net-next V2 05/13] net/mlx5e: Add data direct ethtool private flag

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

This patch adds the data direct private flag infrastructure.
The MLX5E_PFLAG_DATA_DIRECT flag is needed in the follow-up patches to
detect the configuration of data direct from the user side.

For now the flag can't be actually set. This will be implemented in a
follow-up patch once all pieces are put together.

This patch doesn't introduce any functional changes.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h         | 1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 000493f42912..c5bbe5adea45 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -264,6 +264,7 @@ enum mlx5e_priv_flag {
 	MLX5E_PFLAG_XDP_TX_MPWQE,
 	MLX5E_PFLAG_SKB_TX_MPWQE,
 	MLX5E_PFLAG_TX_PORT_TS,
+	MLX5E_PFLAG_DATA_DIRECT,
 	MLX5E_NUM_PFLAGS, /* Keep last */
 };
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 1f534051cdfe..26f76449af35 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -2405,6 +2405,11 @@ static int set_pflag_tx_port_ts(struct net_device *netdev, bool enable)
 	return err;
 }
 
+static int set_pflag_data_direct(struct net_device *netdev, bool enable)
+{
+	return -EOPNOTSUPP;
+}
+
 static const struct pflag_desc mlx5e_priv_flags[MLX5E_NUM_PFLAGS] = {
 	{ "rx_cqe_moder",        set_pflag_rx_cqe_based_moder },
 	{ "tx_cqe_moder",        set_pflag_tx_cqe_based_moder },
@@ -2414,6 +2419,7 @@ static const struct pflag_desc mlx5e_priv_flags[MLX5E_NUM_PFLAGS] = {
 	{ "xdp_tx_mpwqe",        set_pflag_xdp_tx_mpwqe },
 	{ "skb_tx_mpwqe",        set_pflag_skb_tx_mpwqe },
 	{ "tx_port_ts",          set_pflag_tx_port_ts },
+	{ "data_direct",         set_pflag_data_direct },
 };
 
 static int mlx5e_handle_pflag(struct net_device *netdev,
-- 
2.44.0

[PATCH net-next V2 06/13] net/mlx5e: Add data direct RX infrastructure

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

Data direct is a HW feature that allows a type of peer-to-peer DMA
between the NIC and another device (e.g. GPU) going through a specific
PCIe topology.

For data direct to work on the RX path, the RQ will use a KSM mkey whose
entries will point to the crossed mkey. This is necessary to enable DMA
from the NIC to the other device.

This commit adds the necessary infrastructure on RX side:
- New RQ state flag for the data direct state.
- Create crossing mkey.
- Create KSM mkey and in data direct PD and make it point to the
  crossed mkey.
- Size the ICOSQ for the UMR mode of the RQ: KSM UMR WQEs map half the
  pages of MTT ones, so a data direct RQ needs twice as many of them.

This commit has no functional changes. data direct mode will be
enabled in a subsequent commit and mlx5e_queue_get_dma_dev() will then
point to the right device.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |   6 +
 .../ethernet/mellanox/mlx5/core/en/params.c   |  18 ++-
 .../ethernet/mellanox/mlx5/core/en/params.h   |   2 +
 .../mellanox/mlx5/core/en/reporter_rx.c       |   1 +
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 104 ++++++++++++++++--
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  12 +-
 6 files changed, 130 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index c5bbe5adea45..d716e70c6d47 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -342,6 +342,7 @@ enum {
 	MLX5E_RQ_STATE_SHAMPO, /* set when SHAMPO cap is used */
 	MLX5E_RQ_STATE_MINI_CQE_ENHANCED,  /* set when enhanced mini_cqe_cap is used */
 	MLX5E_RQ_STATE_XSK, /* set to indicate an xsk rq */
+	MLX5E_RQ_STATE_DATA_DIRECT, /* set when the rq maps data through data direct */
 	MLX5E_NUM_RQ_STATES, /* Must be kept last */
 };
 
@@ -685,6 +686,8 @@ struct mlx5e_rq {
 			struct mlx5e_mpw_info *info;
 			mlx5e_fp_skb_from_cqe_mpwrq skb_from_cqe_mpwrq;
 			__be32                 umr_mkey_be;
+			__be32                 ksm_child_mkey_be;
+			u32                    crossing_mkey;
 			u16                    num_strides;
 			u16                    actual_wq_head;
 			u8                     log_stride_sz;
@@ -1202,6 +1205,9 @@ static inline bool mlx5_tx_swp_supported(struct mlx5_core_dev *mdev)
 extern const struct ethtool_ops mlx5e_ethtool_ops;
 
 int mlx5e_create_mkey(struct mlx5_core_dev *mdev, u32 pdn, u32 *mkey);
+int mlx5e_create_crossing_vhca_mkey(struct mlx5_core_dev *mdev,
+				    u32 crossed_mkey, u64 crossed_len,
+				    u32 *crossing_mkey);
 int mlx5e_create_mdev_resources(struct mlx5_core_dev *mdev, bool create_tises);
 void mlx5e_destroy_mdev_resources(struct mlx5_core_dev *mdev);
 int mlx5e_modify_tirs_lb(struct mlx5_core_dev *mdev, bool enable_uc_lb,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
index 5caf7820a136..61a77ee77244 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
@@ -62,6 +62,9 @@ mlx5e_mpwrq_umr_mode(struct mlx5_core_dev *mdev,
 	bool unaligned = xsk ? xsk->unaligned : false;
 	bool oversized = false;
 
+	if (rqo && rqo->data_direct)
+		return MLX5E_MPWRQ_UMR_MODE_UNALIGNED;
+
 	if (xsk) {
 		oversized = xsk->chunk_size < (1 << page_shift);
 		WARN_ON_ONCE(xsk->chunk_size > (1 << page_shift));
@@ -1213,8 +1216,14 @@ static u32 mlx5e_max_xsk_wqebbs(struct mlx5_core_dev *mdev,
 
 static u8 mlx5e_build_icosq_log_wq_sz(struct mlx5_core_dev *mdev,
 				      struct mlx5e_params *params,
-				      struct mlx5e_rq_param *rq_param)
+				      struct mlx5e_rq_opt_param *rqo)
 {
+	/* Only the UMR mode matters here. Leave qcfg out: sizing for
+	 * PAGE_SIZE is an upper bound for larger RX page sizes.
+	 */
+	struct mlx5e_rq_opt_param umr_rqo = {
+		.data_direct = rqo && rqo->data_direct,
+	};
 	u32 wqebbs, total_pages, useful_space;
 
 	/* MLX5_WQ_TYPE_CYCLIC */
@@ -1222,7 +1231,7 @@ static u8 mlx5e_build_icosq_log_wq_sz(struct mlx5_core_dev *mdev,
 		return MLX5E_PARAMS_MINIMUM_LOG_SQ_SIZE;
 
 	/* UMR WQEs for the regular RQ. */
-	wqebbs = mlx5e_mpwrq_total_umr_wqebbs(mdev, params, NULL);
+	wqebbs = mlx5e_mpwrq_total_umr_wqebbs(mdev, params, &umr_rqo);
 
 	wqebbs += mlx5e_max_xsk_wqebbs(mdev, params);
 
@@ -1296,18 +1305,21 @@ void mlx5e_build_xdpsq_param(struct mlx5_core_dev *mdev,
 int mlx5e_build_channel_param(struct mlx5_core_dev *mdev,
 			      struct mlx5e_params *params,
 			      struct netdev_queue_config *qcfg,
+			      bool data_direct,
 			      struct mlx5e_channel_param *cparam)
 {
 	u8 icosq_log_wq_sz, async_icosq_log_wq_sz;
 	int err;
 
 	cparam->rq_opt.qcfg = qcfg;
+	cparam->rq_opt.data_direct = data_direct;
 
 	err = mlx5e_build_rq_param(mdev, params, &cparam->rq_opt, &cparam->rq);
 	if (err)
 		return err;
 
-	icosq_log_wq_sz = mlx5e_build_icosq_log_wq_sz(mdev, params, &cparam->rq);
+	icosq_log_wq_sz = mlx5e_build_icosq_log_wq_sz(mdev, params,
+						      &cparam->rq_opt);
 	async_icosq_log_wq_sz = mlx5e_build_async_icosq_log_wq_sz(mdev);
 
 	mlx5e_build_sq_param(mdev, params, &cparam->txq_sq);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.h b/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
index 275f9be53a34..43332015f445 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
@@ -15,6 +15,7 @@ struct mlx5e_xsk_param {
 struct mlx5e_rq_opt_param {
 	struct mlx5e_xsk_param *xsk;
 	struct netdev_queue_config *qcfg;
+	bool data_direct;
 };
 
 struct mlx5e_cq_param {
@@ -146,6 +147,7 @@ void mlx5e_build_xdpsq_param(struct mlx5_core_dev *mdev,
 int mlx5e_build_channel_param(struct mlx5_core_dev *mdev,
 			      struct mlx5e_params *params,
 			      struct netdev_queue_config *qcfg,
+			      bool data_direct,
 			      struct mlx5e_channel_param *cparam);
 
 void mlx5e_build_xsk_channel_param(struct mlx5_core_dev *mdev,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
index 6efb626b5506..f9fce1d0e9e7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
@@ -21,6 +21,7 @@ static const char * const rq_sw_state_type_name[] = {
 	[MLX5E_RQ_STATE_SHAMPO] = "shampo",
 	[MLX5E_RQ_STATE_MINI_CQE_ENHANCED] = "mini_cqe_enhanced",
 	[MLX5E_RQ_STATE_XSK] = "xsk",
+	[MLX5E_RQ_STATE_DATA_DIRECT] = "data_direct",
 };
 
 static int mlx5e_query_rq_state(struct mlx5_core_dev *dev, u32 rqn, u8 *state)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 96b521d8bed3..cee73f3b7a99 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -425,7 +425,7 @@ static u8 mlx5e_mpwrq_access_mode(enum mlx5e_mpwrq_umr_mode umr_mode)
 static int mlx5e_create_umr_mkey(struct mlx5_core_dev *mdev,
 				 u32 npages, u8 page_shift, u32 *umr_mkey,
 				 dma_addr_t filler_addr,
-				 enum mlx5e_mpwrq_umr_mode umr_mode,
+				 enum mlx5e_mpwrq_umr_mode umr_mode, u32 pdn,
 				 u32 xsk_chunk_size)
 {
 	struct mlx5_mtt *mtt;
@@ -465,7 +465,7 @@ static int mlx5e_create_umr_mkey(struct mlx5_core_dev *mdev,
 	MLX5_SET(mkc, mkc, access_mode_1_0, mlx5e_mpwrq_access_mode(umr_mode));
 	mlx5e_mkey_set_relaxed_ordering(mdev, mkc);
 	MLX5_SET(mkc, mkc, qpn, 0xffffff);
-	MLX5_SET(mkc, mkc, pd, mdev->mlx5e_res.hw_objs.pdn);
+	MLX5_SET(mkc, mkc, pd, pdn);
 	MLX5_SET64(mkc, mkc, len, npages << page_shift);
 	MLX5_SET(mkc, mkc, translations_octword_size, octwords);
 	if (umr_mode == MLX5E_MPWRQ_UMR_MODE_TRIPLE)
@@ -531,9 +531,11 @@ static int mlx5e_create_umr_mkey(struct mlx5_core_dev *mdev,
 static int mlx5e_create_rq_umr_mkey(struct mlx5_core_dev *mdev, struct mlx5e_rq *rq)
 {
 	u32 xsk_chunk_size = rq->xsk_pool ? rq->xsk_pool->chunk_size : 0;
+	bool dd = test_bit(MLX5E_RQ_STATE_DATA_DIRECT, &rq->state);
 	u32 wq_size = mlx5_wq_ll_get_size(&rq->mpwqe.wq);
 	u32 num_entries, max_num_entries;
-	u32 umr_mkey;
+	u32 umr_mkey = 0;
+	u32 pdn;
 	int err;
 
 	max_num_entries = mlx5e_mpwrq_max_num_entries(mdev, rq->mpwqe.umr_mode);
@@ -546,13 +548,80 @@ static int mlx5e_create_rq_umr_mkey(struct mlx5_core_dev *mdev, struct mlx5e_rq
 			      __func__, wq_size, rq->mpwqe.mtts_per_wqe,
 			      max_num_entries);
 
+	pdn = dd ? mdev->data_direct->pdn : mdev->mlx5e_res.hw_objs.pdn;
 	err = mlx5e_create_umr_mkey(mdev, num_entries, rq->mpwqe.page_shift,
 				    &umr_mkey, rq->wqe_overflow.addr,
-				    rq->mpwqe.umr_mode, xsk_chunk_size);
+				    rq->mpwqe.umr_mode, pdn,
+				    xsk_chunk_size);
 	rq->mpwqe.umr_mkey_be = cpu_to_be32(umr_mkey);
 	return err;
 }
 
+int mlx5e_create_crossing_vhca_mkey(struct mlx5_core_dev *mdev,
+				    u32 crossed_mkey, u64 crossed_len,
+				    u32 *crossing_mkey)
+{
+	int inlen = MLX5_ST_SZ_BYTES(create_mkey_in);
+	int access_mode = MLX5_MKC_ACCESS_MODE_CROSSING;
+	void *mkc;
+	u32 *in;
+	int err;
+
+	if (!MLX5_CAP_GEN(mdev, crossing_vhca_mkey))
+		return -EOPNOTSUPP;
+
+	in = kvzalloc(inlen, GFP_KERNEL);
+	if (!in)
+		return -ENOMEM;
+
+	mkc = MLX5_ADDR_OF(create_mkey_in, in, memory_key_mkey_entry);
+	MLX5_SET(mkc, mkc, access_mode_1_0, access_mode & 0x3);
+	MLX5_SET(mkc, mkc, access_mode_4_2, (access_mode >> 2) & 0x7);
+	MLX5_SET(mkc, mkc, crossing_target_vhca_id,
+		 MLX5_CAP_GEN(mdev, vhca_id));
+	MLX5_SET(mkc, mkc, translations_octword_size, crossed_mkey);
+	MLX5_SET(mkc, mkc, pd, mdev->mlx5e_res.hw_objs.pdn);
+	MLX5_SET(mkc, mkc, qpn, 0xffffff);
+	MLX5_SET(mkc, mkc, lr, 1);
+	MLX5_SET(mkc, mkc, lw, 1);
+	MLX5_SET64(mkc, mkc, start_addr, 0);
+	MLX5_SET64(mkc, mkc, len, crossed_len);
+
+	err = mlx5_core_create_mkey(mdev, crossing_mkey, in, inlen);
+	kvfree(in);
+	return err;
+}
+
+static int mlx5e_rq_alloc_data_direct_mkey(struct mlx5e_rq *rq)
+{
+	struct mlx5_core_dev *mdev = rq->mdev;
+	u32 umr_mkey;
+	u64 umr_len;
+	int wq_sz;
+
+	if (!test_bit(MLX5E_RQ_STATE_DATA_DIRECT, &rq->state))
+		return 0;
+
+	wq_sz = mlx5_wq_ll_get_size(&rq->mpwqe.wq);
+	umr_mkey = be32_to_cpu(rq->mpwqe.umr_mkey_be);
+	umr_len = (u64)wq_sz * rq->mpwqe.mtts_per_wqe << rq->mpwqe.page_shift;
+
+	rq->mpwqe.ksm_child_mkey_be =
+		cpu_to_be32(mdev->data_direct->mkey_ro_valid ?
+			    mdev->data_direct->mkey_ro :
+			    mdev->data_direct->mkey);
+
+	return mlx5e_create_crossing_vhca_mkey(mdev, umr_mkey,
+					       umr_len,
+					       &rq->mpwqe.crossing_mkey);
+}
+
+static void mlx5e_rq_free_data_direct_mkey(struct mlx5e_rq *rq)
+{
+	if (test_bit(MLX5E_RQ_STATE_DATA_DIRECT, &rq->state))
+		mlx5_core_destroy_mkey(rq->mdev, rq->mpwqe.crossing_mkey);
+}
+
 static void mlx5e_init_frags_partition(struct mlx5e_rq *rq)
 {
 	struct mlx5e_wqe_frag_info next_frag = {};
@@ -900,6 +969,7 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
 {
 	void *rqc_wq = MLX5_ADDR_OF(rqc, rq_param->rqc, wq);
 	struct mlx5_core_dev *mdev = rq->mdev;
+	bool data_direct = false;
 	u32 pool_order = 0;
 	u16 entries_bytes;
 	u32 pool_size;
@@ -937,6 +1007,11 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
 		if (err)
 			goto err_rq_wq_destroy;
 
+		data_direct = rqo && rqo->data_direct;
+
+		if (data_direct)
+			set_bit(MLX5E_RQ_STATE_DATA_DIRECT, &rq->state);
+
 		rq->mpwqe.umr_mode = mlx5e_mpwrq_umr_mode(mdev, rqo);
 		rq->mpwqe.pages_per_wqe =
 			mlx5e_mpwrq_pages_per_wqe(mdev, rq->mpwqe.page_shift,
@@ -975,10 +1050,14 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
 		if (err)
 			goto err_rq_drop_page;
 
-		err = mlx5e_rq_alloc_mpwqe_info(rq, node);
+		err = mlx5e_rq_alloc_data_direct_mkey(rq);
 		if (err)
 			goto err_rq_mkey;
 
+		err = mlx5e_rq_alloc_mpwqe_info(rq, node);
+		if (err)
+			goto err_rq_free_dd_mkey;
+
 		err = mlx5e_rq_alloc_mpwqe_linear_info(rq, node, params, rqo);
 		if (err)
 			goto err_free_mpwqe_info;
@@ -1071,7 +1150,9 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
 
 			wqe->data[0].addr = cpu_to_be64(dma_offset + headroom);
 			wqe->data[0].byte_count = cpu_to_be32(byte_count);
-			wqe->data[0].lkey = rq->mpwqe.umr_mkey_be;
+			wqe->data[0].lkey = data_direct ?
+				cpu_to_be32(rq->mpwqe.crossing_mkey) :
+				rq->mpwqe.umr_mkey_be;
 		} else {
 			struct mlx5e_rx_wqe_cyc *wqe =
 				mlx5_wq_cyc_get_wqe(&rq->wqe.wq, i);
@@ -1105,6 +1186,8 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
 		kvfree(rq->mpwqe.linear_info);
 err_free_mpwqe_info:
 		kvfree(rq->mpwqe.info);
+err_rq_free_dd_mkey:
+		mlx5e_rq_free_data_direct_mkey(rq);
 err_rq_mkey:
 		mlx5_core_destroy_mkey(mdev, be32_to_cpu(rq->mpwqe.umr_mkey_be));
 err_rq_drop_page:
@@ -1132,6 +1215,7 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
 		mlx5e_rq_free_shampo(rq);
 		kvfree(rq->mpwqe.linear_info);
 		kvfree(rq->mpwqe.info);
+		mlx5e_rq_free_data_direct_mkey(rq);
 		mlx5_core_destroy_mkey(rq->mdev, be32_to_cpu(rq->mpwqe.umr_mkey_be));
 		mlx5e_free_mpwqe_rq_drop_page(rq);
 		break;
@@ -2849,6 +2933,7 @@ static int mlx5e_open_channel(struct mlx5e_priv *priv, int ix,
 	struct mlx5e_xsk_param xsk;
 	bool async_icosq_needed;
 	struct mlx5e_channel *c;
+	bool data_direct;
 	unsigned int irq;
 	int vec_ix;
 	int cpu;
@@ -2873,7 +2958,12 @@ static int mlx5e_open_channel(struct mlx5e_priv *priv, int ix,
 		goto err_free;
 	}
 
-	err = mlx5e_build_channel_param(mdev, params, qcfg, cparam);
+	data_direct = MLX5E_GET_PFLAG(params, MLX5E_PFLAG_DATA_DIRECT) &&
+		      params->packet_merge.type == MLX5E_PACKET_MERGE_SHAMPO &&
+		      netif_rxq_has_unreadable_mp(netdev, ix);
+
+	err = mlx5e_build_channel_param(mdev, params, qcfg, data_direct,
+					cparam);
 	if (err)
 		goto err_free;
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index a9392ee5376e..0f9a946aef41 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -670,9 +670,15 @@ static int mlx5e_alloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
 			goto err_unmap;
 
 		addr = page_pool_get_dma_addr_netmem(frag_page->netmem);
-		umr_wqe->inline_mtts[i] = (struct mlx5_mtt) {
-			.ptag = cpu_to_be64(addr | MLX5_EN_WR),
-		};
+		if (test_bit(MLX5E_RQ_STATE_DATA_DIRECT, &rq->state))
+			umr_wqe->inline_ksms[i] = (struct mlx5_ksm) {
+				.key = rq->mpwqe.ksm_child_mkey_be,
+				.va  = cpu_to_be64(addr),
+			};
+		else
+			umr_wqe->inline_mtts[i] = (struct mlx5_mtt) {
+				.ptag = cpu_to_be64(addr | MLX5_EN_WR),
+			};
 	}
 
 	/* Pad when the value set to ucseg->xlt_octowords in
-- 
2.44.0

[PATCH net-next V2 07/13] net/mlx5e: SHAMPO, Use page_pool DMA sync helper for SKB frags

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

Switch to the page_pool DMA API which takes care of not calling
dma_sync_* for unreadable netmem. Do it only for the place where it
makes sense: SHAMPO payloads, the only RX buffers that can be
unreadable.

Until now it was ok to call the dma_sync_* API directly. But with the
introduction of data direct it will no longer be correct: the payload
netmem of a data direct RQ is mapped against the data direct device,
not rq->pdev.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 0f9a946aef41..9db3db5246ab 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -560,12 +560,11 @@ mlx5e_add_skb_frag(struct mlx5e_rq *rq, struct sk_buff *skb,
 		   u32 frag_offset, u32 len,
 		   unsigned int truesize)
 {
-	dma_addr_t addr = page_pool_get_dma_addr_netmem(frag_page->netmem);
 	u8 next_frag = skb_shinfo(skb)->nr_frags;
 	netmem_ref netmem = frag_page->netmem;
 
-	dma_sync_single_for_cpu(rq->pdev, addr + frag_offset, len,
-				rq->buff.map_dir);
+	page_pool_dma_sync_netmem_for_cpu(rq->page_pool, netmem, frag_offset,
+					  len);
 
 	if (skb_can_coalesce_netmem(skb, next_frag, netmem, frag_offset)) {
 		skb_coalesce_rx_frag(skb, next_frag - 1, len, truesize);
-- 
2.44.0

[PATCH net-next V2 08/13] net/mlx5e: Add data direct TX infrastructure

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

Data direct is a HW feature that allows a type of peer-to-peer DMA
between the NIC and another device (e.g. GPU) going through a specific
PCIe topology.

For data direct to work on the TX path, a crossing mkey is necessary
which is created as a mlx5e resource. This mkey is used by all SQs.
For simplicity always create this resource and link it to the SQ
if data direct is supported.

An SQ state flag is added as well to signal to the TX data path that
data direct is enabled.

This commit has no functional changes. data direct mode will be
enabled in a subsequent commit and mlx5e_queue_get_dma_dev() will then
point to the right device.

The data_direct private flag is not propagated to PTP SQs because
the queue selector will direct devmem net_iovs to regular SQs.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  2 +
 .../mellanox/mlx5/core/en/reporter_tx.c       |  1 +
 .../ethernet/mellanox/mlx5/core/en_common.c   | 38 ++++++++++++++++++-
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  6 +++
 .../net/ethernet/mellanox/mlx5/core/en_tx.c   |  4 +-
 include/linux/mlx5/driver.h                   |  1 +
 6 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index d716e70c6d47..23e45e87cdee 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -397,6 +397,7 @@ enum {
 	MLX5E_SQ_STATE_PENDING_XSK_TX,
 	MLX5E_SQ_STATE_PENDING_TLS_RX_RESYNC,
 	MLX5E_SQ_STATE_LOCK_NEEDED,
+	MLX5E_SQ_STATE_DATA_DIRECT, /* set when the sq maps net_iov frags through data direct */
 	MLX5E_NUM_SQ_STATES, /* Must be kept last */
 };
 
@@ -453,6 +454,7 @@ struct mlx5e_txqsq {
 	u8                         min_inline_mode;
 	struct device             *pdev;
 	__be32                     mkey_be;
+	__be32                     crossing_mkey_be;
 	unsigned long              state;
 	unsigned int               hw_mtu;
 	struct mlx5_clock         *clock;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
index 8409ae73768f..b4a4ff877bbc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
@@ -18,6 +18,7 @@ static const char * const sq_sw_state_type_name[] = {
 	[MLX5E_SQ_STATE_PENDING_XSK_TX] = "pending_xsk_tx",
 	[MLX5E_SQ_STATE_PENDING_TLS_RX_RESYNC] = "pending_tls_rx_resync",
 	[MLX5E_SQ_STATE_LOCK_NEEDED] = "lock_needed",
+	[MLX5E_SQ_STATE_DATA_DIRECT] = "data_direct",
 };
 
 static int mlx5e_wait_for_sq_flush(struct mlx5e_txqsq *sq)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_common.c b/drivers/net/ethernet/mellanox/mlx5/core/en_common.c
index c2d4bae533de..c1d3b2ba1864 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_common.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_common.c
@@ -30,6 +30,8 @@
  * SOFTWARE.
  */
 
+#include <linux/mlx5/data_direct.h>
+
 #include "devlink.h"
 #include "en.h"
 #include "lib/crypto.h"
@@ -153,6 +155,30 @@ mlx5e_get_devlink_param_num_doorbells(struct mlx5_core_dev *dev)
 	return err ? MLX5_DEFAULT_NUM_DOORBELLS : val.vu32;
 }
 
+static int mlx5e_create_data_direct_crossing_mkey(struct mlx5_core_dev *mdev)
+{
+	struct mlx5e_hw_objs *res = &mdev->mlx5e_res.hw_objs;
+
+	if (!mdev->data_direct ||
+	    !MLX5_CAP_GEN(mdev, crossing_vhca_mkey))
+		return 0;
+
+	return mlx5e_create_crossing_vhca_mkey(mdev, mdev->data_direct->mkey,
+					       U64_MAX,
+					       &res->data_direct_crossing_mkey);
+}
+
+static void mlx5e_destroy_data_direct_crossing_mkey(struct mlx5_core_dev *mdev)
+{
+	struct mlx5e_hw_objs *res = &mdev->mlx5e_res.hw_objs;
+
+	if (!mdev->data_direct ||
+	    !MLX5_CAP_GEN(mdev, crossing_vhca_mkey))
+		return;
+
+	mlx5_core_destroy_mkey(mdev, res->data_direct_crossing_mkey);
+}
+
 int mlx5e_create_mdev_resources(struct mlx5_core_dev *mdev, bool create_tises)
 {
 	struct mlx5e_hw_objs *res = &mdev->mlx5e_res.hw_objs;
@@ -177,12 +203,19 @@ int mlx5e_create_mdev_resources(struct mlx5_core_dev *mdev, bool create_tises)
 		goto err_dealloc_transport_domain;
 	}
 
+	err = mlx5e_create_data_direct_crossing_mkey(mdev);
+	if (err) {
+		mlx5_core_err(mdev, "create data direct crossing mkey failed, %d\n",
+			      err);
+		goto err_destroy_mkey;
+	}
+
 	num_doorbells = min(mlx5e_get_devlink_param_num_doorbells(mdev),
 			    mlx5e_get_max_num_channels(mdev));
 	res->bfregs = kzalloc_objs(*res->bfregs, num_doorbells);
 	if (!res->bfregs) {
 		err = -ENOMEM;
-		goto err_destroy_mkey;
+		goto err_destroy_crossing_mkey;
 	}
 
 	for (i = 0; i < num_doorbells; i++) {
@@ -221,6 +254,8 @@ int mlx5e_create_mdev_resources(struct mlx5_core_dev *mdev, bool create_tises)
 	for (i = 0; i < res->num_bfregs; i++)
 		mlx5_free_bfreg(mdev, res->bfregs + i);
 	kfree(res->bfregs);
+err_destroy_crossing_mkey:
+	mlx5e_destroy_data_direct_crossing_mkey(mdev);
 err_destroy_mkey:
 	mlx5_core_destroy_mkey(mdev, res->mkey);
 err_dealloc_transport_domain:
@@ -241,6 +276,7 @@ void mlx5e_destroy_mdev_resources(struct mlx5_core_dev *mdev)
 	for (unsigned int i = 0; i < res->num_bfregs; i++)
 		mlx5_free_bfreg(mdev, res->bfregs + i);
 	kfree(res->bfregs);
+	mlx5e_destroy_data_direct_crossing_mkey(mdev);
 	mlx5_core_destroy_mkey(mdev, res->mkey);
 	mlx5_core_dealloc_transport_domain(mdev, res->td.tdn);
 	mlx5_core_dealloc_pd(mdev, res->pdn);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index cee73f3b7a99..5b99099e4ed0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -1825,6 +1825,12 @@ static int mlx5e_alloc_txqsq(struct mlx5e_channel *c,
 		set_bit(MLX5E_SQ_STATE_IPSEC, &sq->state);
 	if (param->is_mpw)
 		set_bit(MLX5E_SQ_STATE_MPWQE, &sq->state);
+	if (MLX5E_GET_PFLAG(params, MLX5E_PFLAG_DATA_DIRECT)) {
+		u32 mkey = mdev->mlx5e_res.hw_objs.data_direct_crossing_mkey;
+
+		sq->crossing_mkey_be = cpu_to_be32(mkey);
+		set_bit(MLX5E_SQ_STATE_DATA_DIRECT, &sq->state);
+	}
 	sq->stop_room = param->stop_room;
 	sq->ptp_cyc2time = mlx5_sq_ts_translator(mdev);
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index 14479da98f42..62ffb41a64df 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -182,6 +182,7 @@ mlx5e_txwqe_build_dsegs(struct mlx5e_txqsq *sq, struct sk_buff *skb,
 			unsigned char *skb_data, u16 headlen,
 			struct mlx5_wqe_data_seg *dseg)
 {
+	bool dd = test_bit(MLX5E_SQ_STATE_DATA_DIRECT, &sq->state);
 	dma_addr_t dma_addr = 0;
 	u8 num_dma          = 0;
 	int i;
@@ -211,7 +212,8 @@ mlx5e_txwqe_build_dsegs(struct mlx5e_txqsq *sq, struct sk_buff *skb,
 			goto dma_unmap_wqe_err;
 
 		dseg->addr       = cpu_to_be64(dma_addr);
-		dseg->lkey       = sq->mkey_be;
+		dseg->lkey       = dd && skb_frag_is_net_iov(frag) ?
+				   sq->crossing_mkey_be : sq->mkey_be;
 		dseg->byte_count = cpu_to_be32(fsz);
 
 		mlx5e_dma_push_netmem(sq, skb_frag_netmem(frag), dma_addr, fsz);
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index abb99371ff13..4c8b447337f1 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -678,6 +678,7 @@ struct mlx5e_resources {
 		u32                        pdn;
 		struct mlx5_td             td;
 		u32			   mkey;
+		u32			   data_direct_crossing_mkey;
 		struct mlx5_sq_bfreg      *bfregs;
 		unsigned int               num_bfregs;
 #define MLX5_MAX_NUM_TC 8
-- 
2.44.0

[PATCH net-next V2 09/13] net/mlx5e: Use the correct DMA dev when data_direct is enabled

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

When the data_direct private flag is enabled, return the data direct
device instead of the NIC PF PCI device in .ndo_queue_get_dma_dev.

The memory provider maps the payload buffers against that device, so
the page_pool of a data direct RQ has to use it as well. Add
mlx5e_get_dma_dev() which returns the DMA device of an mlx5_core_dev for
either mode and use it in both places.

Running .ndo_queue_get_dma_dev while the data direct device is going
away returns NULL because the device pointer is already cleared.
Opening a data direct RQ fails with -ENODEV for the same reason.

Leased queues are not supported in data-direct mode because there is
currently no mechanism to revoke leased queues from the leaser.

Rename the dev argument to netdev because there are too many confusing
"dev"-based names in this function.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 49 ++++++++++++++++---
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 5b99099e4ed0..ebd41f24330d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -962,6 +962,19 @@ static void mlx5e_rq_free_shampo(struct mlx5e_rq *rq)
 	kvfree(shampo);
 }
 
+static struct device *mlx5e_get_dma_dev(struct mlx5_core_dev *mdev,
+					bool data_direct)
+{
+	struct mlx5_data_direct_dev *dd_dev;
+
+	if (!data_direct)
+		return mlx5_core_dma_dev(mdev);
+
+	dd_dev = mlx5_data_direct_get_dev(mdev->data_direct);
+
+	return dd_dev ? dd_dev->device : NULL;
+}
+
 static int mlx5e_alloc_rq(struct mlx5e_params *params,
 			  struct mlx5e_rq_param *rq_param,
 			  struct mlx5e_rq_opt_param *rqo,
@@ -1105,7 +1118,11 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
 		pp_params.flags     = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;
 		pp_params.pool_size = pool_size;
 		pp_params.nid       = node;
-		pp_params.dev       = rq->pdev;
+		pp_params.dev       = mlx5e_get_dma_dev(mdev, data_direct);
+		if (!pp_params.dev) {
+			err = -ENODEV;
+			goto err_free_by_rq_type;
+		}
 		pp_params.napi      = rq->cq.napi;
 		pp_params.netdev    = rq->netdev;
 		pp_params.dma_dir   = rq->buff.map_dir;
@@ -5897,27 +5914,45 @@ static int mlx5e_queue_start(struct net_device *dev,
 	return 0;
 }
 
-static struct device *mlx5e_queue_get_dma_dev(struct net_device *dev,
+static bool mlx5e_has_leased_rxqs(struct net_device *netdev)
+{
+	for (u32 i = 0; i < netdev->real_num_rx_queues; i++)
+		if (READ_ONCE(__netif_get_rx_queue(netdev, i)->lease))
+			return true;
+
+	return false;
+}
+
+static struct device *mlx5e_queue_get_dma_dev(struct net_device *netdev,
 					      int queue_index)
 {
-	struct mlx5e_priv *priv = netdev_priv(dev);
+	struct mlx5e_priv *priv = netdev_priv(netdev);
 	struct mlx5e_channels *channels;
-	struct device *pdev = NULL;
+	struct device *dma_dev = NULL;
 	struct mlx5e_channel *ch;
+	bool dd;
 
 	channels = &priv->channels;
 
 	mutex_lock(&priv->state_lock);
 
-	if (queue_index >= channels->num)
+	/* Allow only opened channels. */
+	if (queue_index < 0 || queue_index >= channels->num)
+		goto out;
+
+	dd = MLX5E_GET_PFLAG(&channels->params, MLX5E_PFLAG_DATA_DIRECT);
+	if (dd && mlx5e_has_leased_rxqs(netdev)) {
+		mlx5_core_err_rl(priv->mdev,
+				 "Can't get queue DMA device: leased RX queues are not supported in data-direct mode\n");
 		goto out;
+	}
 
 	ch = channels->c[queue_index];
-	pdev = ch->pdev;
+	dma_dev = mlx5e_get_dma_dev(ch->mdev, dd);
 out:
 	mutex_unlock(&priv->state_lock);
 
-	return pdev;
+	return dma_dev;
 }
 
 static const struct netdev_queue_mgmt_ops mlx5e_queue_mgmt_ops = {
-- 
2.44.0

[PATCH net-next V2 10/13] net: devmem: add netdev_has_dmabuf_binding() helper

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

Currently there is no way to check whether a dmabuf is bound on a TX
queue. RX side has netif_rxq_has_unreadable_mp() which gives a hint
of it.

To help with that, add a helper to check whether a devmem dmabuf
binding may still be in use on a given netdev, optionally filtered by
the DMA device the dmabuf was mapped against.

A binding can outlive the user's unbind: it can still be referenced by
in-flight TX skbs, whose DMA mappings are only valid for the device the
dmabuf was attached to. Reporting such a binding as gone would let a
driver swap the DMA device out from under those skbs. Bindings stay in
the bindings xarray until they are freed, so walk it and report every
binding of the netdev that is still mapped. Bindings that were already
unmapped are not reported.

This API is necessary for the upcoming support of data direct in mlx5e.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Cc: Bobby Eshleman <bobbyeshleman@gmail.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 include/net/netmem.h | 24 ++++++++++++++++++++++++
 net/core/devmem.c    | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index fb693bc1702a..35bb6fa0f719 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -397,16 +397,40 @@ static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)
 	return netmem_to_nmdesc(netmem)->dma_addr;
 }
 
+struct net_device;
+
 #if defined(CONFIG_NET_DEVMEM)
 static inline bool net_is_devmem_iov(const struct net_iov *niov)
 {
 	return niov->type == NET_IOV_DMABUF;
 }
+
+/**
+ * netdev_has_dmabuf_binding - is there a dmabuf binding for a
+ * given @dev and @dma_dev
+ *
+ * @dev: netdev to check bindings
+ * @dma_dev: dma device to check, when NULL it will not be checked
+ *
+ * A binding is reported for as long as it is mapped, which can be long
+ * after the user unbound it: net_iovs of a TX binding can still be queued
+ * in flight, with DMA mappings that are only valid for the device the dmabuf
+ * was attached to.
+ *
+ * Return: true if a binding is found under @dev and possibly @dma_dev
+ */
+bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev);
 #else
 static inline bool net_is_devmem_iov(const struct net_iov *niov)
 {
 	return false;
 }
+
+static inline bool netdev_has_dmabuf_binding(struct net_device *dev,
+					     struct device *dma_dev)
+{
+	return false;
+}
 #endif
 
 void net_iov_inc_ref(struct net_iov *niov);
diff --git a/net/core/devmem.c b/net/core/devmem.c
index 5c7d9640e9d1..dd3987bbd9f6 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -513,6 +513,49 @@ struct net_devmem_dmabuf_binding *net_devmem_lookup_dmabuf(u32 id)
 	return binding;
 }
 
+/* The attachment is cleared once the binding has been unmapped. */
+static bool
+net_devmem_binding_mapped_to(struct net_devmem_dmabuf_binding *binding,
+			     struct device *dma_dev)
+{
+	lockdep_assert_held(&binding->lock);
+
+	return binding->attachment &&
+	       (!dma_dev || binding->attachment->dev == dma_dev);
+}
+
+bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev)
+{
+	struct net_devmem_dmabuf_binding *binding;
+	unsigned long xa_idx;
+	bool found = false;
+
+	netdev_assert_locked(dev);
+
+	/* A binding stays in the xarray until it is freed. binding->dev is
+	 * only cleared under the instance lock of that netdev, and the free
+	 * takes it before erasing the binding, so a binding found under @dev
+	 * stays alive while the xarray lock is dropped.
+	 */
+	xa_lock(&net_devmem_dmabuf_bindings);
+	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
+		if (READ_ONCE(binding->dev) != dev)
+			continue;
+
+		xa_unlock(&net_devmem_dmabuf_bindings);
+		mutex_lock(&binding->lock);
+		found = net_devmem_binding_mapped_to(binding, dma_dev);
+		mutex_unlock(&binding->lock);
+		xa_lock(&net_devmem_dmabuf_bindings);
+		if (found)
+			break;
+	}
+	xa_unlock(&net_devmem_dmabuf_bindings);
+
+	return found;
+}
+EXPORT_SYMBOL_GPL(netdev_has_dmabuf_binding);
+
 void net_devmem_net_iov_inc_ref(struct net_iov *niov)
 {
 	net_devmem_dmabuf_binding_get(net_devmem_iov_binding(niov));
-- 
2.44.0

[PATCH net-next V2 11/13] net: add netdev_unbind_dma_dev() helper

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

Currently the netdev code that binds dmabufs to a DMA device assumes
that the device is the netdev's own PCI device, and the unbind cleanup
is done under that assumption.

With the introduction of data direct in mlx5, this assumption no longer
holds: the DMA device is not the netdev's PCI device. When the data
direct DMA device is unbound, there is no way to clean up the bindings.

Add a helper that unbinds the dmabufs of a closed or detached netdev
from a DMA device, independent of the memory provider in use:

- RX: uninstall the memory provider of every RX queue whose DMA device
  is the departing one or is gone. The uninstall callback unmaps the
  buffers, for devmem and io_uring zcrx alike. The queues get regular
  page pools on the next open.

- devmem TX: detach and unmap the TX bindings of the netdev mapped
  against the device, as netdev unregister does. New sends fail and
  in-flight skbs are dropped at xmit.

The netdev must be closed or detached so that the hardware no longer
accesses the mappings and the qdiscs hold no validated skbs. A detached
netdev also works because the driver has already torn down the queues
and new bindings can't be added.

For now leased queues owned by virtual netdevs are not supported by this
API. mlx5 refuses zero-copy while RX queues are leased in data direct
mode.

The helper only unmaps. The providers and bindings stay allocated until
their users release them.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 include/net/netdev_queues.h |  1 +
 net/core/devmem.c           | 42 ++++++++++++++++++++++++++++++
 net/core/devmem.h           |  6 +++++
 net/core/netdev_queues.c    | 51 +++++++++++++++++++++++++++++++++++++
 4 files changed, 100 insertions(+)

diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
index c5335e935b7d..f3f174f342b9 100644
--- a/include/net/netdev_queues.h
+++ b/include/net/netdev_queues.h
@@ -235,6 +235,7 @@ void netdev_queue_config(struct net_device *dev, int rxq,
 			 struct netdev_queue_config *qcfg);
 
 bool netif_rxq_has_unreadable_mp(struct net_device *dev, unsigned int rxq_idx);
+int netdev_unbind_dma_dev(struct net_device *dev, struct device *dma_dev);
 
 /**
  * DOC: Lockless queue stopping / waking helpers.
diff --git a/net/core/devmem.c b/net/core/devmem.c
index dd3987bbd9f6..bc8ae12b2318 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -556,6 +556,48 @@ bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev)
 }
 EXPORT_SYMBOL_GPL(netdev_has_dmabuf_binding);
 
+/* Unmap the bindings of a closed @dev that use @dma_dev, the way netdev
+ * unregister does. Senders that raced the detach drop their skbs at xmit.
+ */
+void net_devmem_unbind_dma_dev(struct net_device *dev, struct device *dma_dev)
+{
+	struct net_devmem_dmabuf_binding *binding;
+	unsigned long xa_idx;
+	bool match, live;
+
+	netdev_assert_locked(dev);
+
+	xa_lock(&net_devmem_dmabuf_bindings);
+	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
+		if (READ_ONCE(binding->dev) != dev)
+			continue;
+
+		/* Hide the binding from TX lookups while it is checked. */
+		live = xa_get_mark(&net_devmem_dmabuf_bindings, xa_idx,
+				   NET_DEVMEM_BINDING_LIVE);
+		if (live)
+			__xa_clear_mark(&net_devmem_dmabuf_bindings, xa_idx,
+					NET_DEVMEM_BINDING_LIVE);
+		xa_unlock(&net_devmem_dmabuf_bindings);
+
+		mutex_lock(&binding->lock);
+		match = net_devmem_binding_mapped_to(binding, dma_dev);
+		mutex_unlock(&binding->lock);
+		if (match)
+			net_devmem_dmabuf_binding_unmap(binding);
+
+		xa_lock(&net_devmem_dmabuf_bindings);
+		if (match) {
+			WRITE_ONCE(binding->vdev, NULL);
+			WRITE_ONCE(binding->dev, NULL);
+		} else if (live) {
+			__xa_set_mark(&net_devmem_dmabuf_bindings, xa_idx,
+				      NET_DEVMEM_BINDING_LIVE);
+		}
+	}
+	xa_unlock(&net_devmem_dmabuf_bindings);
+}
+
 void net_devmem_net_iov_inc_ref(struct net_iov *niov)
 {
 	net_devmem_dmabuf_binding_get(net_devmem_iov_binding(niov));
diff --git a/net/core/devmem.h b/net/core/devmem.h
index c4f476947dfa..06a86efc807d 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -106,6 +106,7 @@ void net_devmem_unlock_dev(struct net_device *dev,
 			   netdevice_tracker *dev_tracker);
 void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding);
 void net_devmem_dev_unregister(struct net_device *dev);
+void net_devmem_unbind_dma_dev(struct net_device *dev, struct device *dma_dev);
 int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 				    struct net_devmem_dmabuf_binding *binding,
 				    struct netlink_ext_ack *extack);
@@ -209,6 +210,11 @@ static inline void net_devmem_dev_unregister(struct net_device *dev)
 {
 }
 
+static inline void net_devmem_unbind_dma_dev(struct net_device *dev,
+					     struct device *dma_dev)
+{
+}
+
 static inline int
 net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 				struct net_devmem_dmabuf_binding *binding,
diff --git a/net/core/netdev_queues.c b/net/core/netdev_queues.c
index 27cb6397bdd4..71cc7bfc087d 100644
--- a/net/core/netdev_queues.c
+++ b/net/core/netdev_queues.c
@@ -1,10 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
+#include <net/netdev_lock.h>
 #include <net/netdev_queues.h>
 #include <net/netdev_rx_queue.h>
 #include <net/xdp_sock_drv.h>
 
 #include "dev.h"
+#include "devmem.h"
 
 static struct device *
 __netdev_queue_get_dma_dev(struct net_device *dev, unsigned int idx)
@@ -58,6 +60,55 @@ struct device *netdev_queue_get_dma_dev(struct net_device *dev,
 	return dma_dev;
 }
 
+/**
+ * netdev_unbind_dma_dev() - drop the memory providers of a departing DMA device
+ * @dev:	net_device, closed or detached by the caller
+ * @dma_dev:	DMA device the driver is about to lose
+ *
+ * Uninstalls the memory provider of every RX queue of @dev that maps
+ * against @dma_dev. Unmaps the devmem bindings of @dev that use @dma_dev.
+ * The bindings stay allocated until their users release them.
+ *
+ * The provider of a leased RX queue is owned by the virtual netdev and
+ * can't be torn down here. Drivers must not report a DMA device they can
+ * lose for leased queues; such queues are skipped with a warning.
+ *
+ * Return: 0 on success, -EBUSY if @dev is running and present.
+ */
+int netdev_unbind_dma_dev(struct net_device *dev, struct device *dma_dev)
+{
+	unsigned int i;
+
+	netdev_assert_locked(dev);
+
+	if (netif_running(dev) && netif_device_present(dev))
+		return -EBUSY;
+
+	for (i = 0; i < dev->real_num_rx_queues; i++) {
+		struct netdev_rx_queue *rxq = __netif_get_rx_queue(dev, i);
+		struct pp_memory_provider_params p = rxq->mp_params;
+		struct device *queue_dev;
+
+		if (!p.mp_ops)
+			continue;
+
+		queue_dev = __netdev_queue_get_dma_dev(dev, i);
+		if (queue_dev && queue_dev != dma_dev)
+			continue;
+
+		if (WARN_ON_ONCE(rxq->lease))
+			continue;
+
+		memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
+		__netif_mp_uninstall_rxq(rxq, &p);
+	}
+
+	net_devmem_unbind_dma_dev(dev, dma_dev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(netdev_unbind_dma_dev);
+
 bool netdev_can_create_queue(const struct net_device *dev,
 			     struct netlink_ext_ack *extack)
 {
-- 
2.44.0

[PATCH net-next V2 12/13] net/mlx5e: Disable data direct on data direct device unbind

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

To prevent invalid accesses after the data direct device is unbound,
stop the netdev and drop the providers using the unbound DMA device. To
prevent further use disable the private flag.

Also release the providers during suspend because the notifier
is disabled at that point and a data direct device unbind would
go unnoticed. This would result in dangling references to the
unbound DMA device.

FW reset, devlink reload and health recovery are not affected: they
unregister the netdev, which releases the memory providers and dmabuf
bindings.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/data_direct.c |  4 +-
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 67 ++++++++++++++++++-
 2 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c b/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c
index 7db94b8a5b79..246a03f44afd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c
@@ -198,9 +198,11 @@ static void mlx5_data_direct_bind(struct mlx5_data_direct_registration *reg,
 static void
 mlx5_data_direct_do_unbind(struct mlx5_data_direct_registration *reg)
 {
+	struct mlx5_data_direct_dev *dev = reg->dd.dev;
+
 	WRITE_ONCE(reg->dd.dev, NULL);
 	blocking_notifier_call_chain(&reg->users, MLX5_DATA_DIRECT_UNBIND,
-				     NULL);
+				     dev);
 }
 
 int mlx5_data_direct_init(struct mlx5_core_dev *mdev)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index ebd41f24330d..63d91ef0dda8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -6170,9 +6170,53 @@ static int mlx5e_data_direct_event(struct notifier_block *nb,
 {
 	struct mlx5e_priv *priv = container_of(nb, struct mlx5e_priv,
 					       data_direct_nb);
+	struct mlx5_data_direct_dev *dd_dev = data;
+	struct net_device *netdev = priv->netdev;
+	struct mlx5e_params new_params;
+	bool enabled, reopen;
+	int err;
+
+	if (action != MLX5_DATA_DIRECT_UNBIND)
+		return NOTIFY_DONE;
+
+	rtnl_lock();
+	netdev_lock(netdev);
+
+	mutex_lock(&priv->state_lock);
+	enabled = MLX5E_GET_PFLAG(&priv->channels.params,
+				  MLX5E_PFLAG_DATA_DIRECT);
+	mutex_unlock(&priv->state_lock);
+	if (!enabled)
+		goto out;
+
+	reopen = netif_running(netdev);
+	netif_close(netdev);
+
+	err = netdev_unbind_dma_dev(netdev, dd_dev->device);
+	if (WARN_ON_ONCE(err)) {
+		netdev_err(netdev, "Failed to unbind dma device, won't reopen netdev, err=%d\n",
+			   err);
+		goto out;
+	}
+
+	mutex_lock(&priv->state_lock);
+	new_params = priv->channels.params;
+	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, false);
+	mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, false);
+	mutex_unlock(&priv->state_lock);
+
+	netdev_info(netdev, "data_direct device unbound, disabled data_direct\n");
 
-	if (action == MLX5_DATA_DIRECT_UNBIND)
-		mlx5_core_dbg(priv->mdev, "data direct device unbound\n");
+	if (reopen) {
+		err = netif_open(netdev, NULL);
+		if (err)
+			netdev_err(netdev, "Failed to reopen after data_direct unbind, err=%d\n",
+				   err);
+	}
+
+out:
+	netdev_unlock(netdev);
+	rtnl_unlock();
 
 	return NOTIFY_OK;
 }
@@ -6407,6 +6451,24 @@ static int mlx5e_nic_enable(struct mlx5e_priv *priv)
 	return err;
 }
 
+static void mlx5e_data_direct_unbind_dev(struct mlx5e_priv *priv)
+{
+	struct mlx5_data_direct_dev *dd_dev;
+	bool enabled;
+
+	dd_dev = mlx5_data_direct_get_dev(priv->mdev->data_direct);
+	if (!dd_dev)
+		return;
+
+	mutex_lock(&priv->state_lock);
+	enabled = MLX5E_GET_PFLAG(&priv->channels.params,
+				  MLX5E_PFLAG_DATA_DIRECT);
+	mutex_unlock(&priv->state_lock);
+	if (enabled)
+		WARN_ON_ONCE(netdev_unbind_dma_dev(priv->netdev,
+						   dd_dev->device));
+}
+
 static void mlx5e_nic_disable(struct mlx5e_priv *priv)
 {
 	struct mlx5_core_dev *mdev = priv->mdev;
@@ -6424,6 +6486,7 @@ static void mlx5e_nic_disable(struct mlx5e_priv *priv)
 		mlx5e_close_trap(priv->en_trap);
 		priv->en_trap = NULL;
 	}
+	mlx5e_data_direct_unbind_dev(priv);
 	netdev_unlock(priv->netdev);
 	rtnl_unlock();
 
-- 
2.44.0

[PATCH net-next V2 13/13] net/mlx5e: Enable the data direct netdev feature

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

Add the implementation of the data direct private flag.
This enables the feature altogether.

Do not allow state changes while a memory provider (devmem or io_uring
zcrx) is installed on an RX queue or while dmabufs are still bound to
the netdev for TX: their buffers are mapped against the DMA device of
the current mode. Same for leased rx queues.

Reject enabling the flag on uplink representors or in socket direct
mode.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  1 +
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  | 63 ++++++++++++++++++-
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  2 +-
 3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 23e45e87cdee..cc7cf23b5c56 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -1155,6 +1155,7 @@ int fn##_ctx(struct mlx5e_priv *priv, void *context) \
 	return fn(priv); \
 }
 int mlx5e_safe_reopen_channels(struct mlx5e_priv *priv);
+bool mlx5e_has_leased_rxqs(struct net_device *netdev);
 int mlx5e_safe_switch_params(struct mlx5e_priv *priv,
 			     struct mlx5e_params *new_params,
 			     mlx5e_fp_preactivate preactivate,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 26f76449af35..2204b370f439 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -32,6 +32,7 @@
 
 #include <linux/dim.h>
 #include <linux/ethtool_netlink.h>
+#include <linux/mlx5/data_direct.h>
 #include <net/netdev_queues.h>
 
 #include "en.h"
@@ -40,7 +41,9 @@
 #include "en/port.h"
 #include "en/params.h"
 #include "en/ptp.h"
+#include "en_rep.h"
 #include "lib/clock.h"
+#include "lib/mlx5.h"
 #include "en/fs_ethtool.h"
 
 #define LANES_UNKNOWN		 0
@@ -2407,7 +2410,65 @@ static int set_pflag_tx_port_ts(struct net_device *netdev, bool enable)
 
 static int set_pflag_data_direct(struct net_device *netdev, bool enable)
 {
-	return -EOPNOTSUPP;
+	struct mlx5e_priv *priv = netdev_priv(netdev);
+	struct mlx5_core_dev *mdev = priv->mdev;
+	struct mlx5e_params new_params;
+	unsigned int i;
+
+	if (enable) {
+		if (!mlx5_data_direct_supported(mdev) ||
+		    !MLX5_CAP_GEN(mdev, crossing_vhca_mkey)) {
+			netdev_err(netdev, "data_direct is not supported by the device\n");
+			return -EOPNOTSUPP;
+		}
+
+		if (!MLX5_CAP_GEN(mdev, shampo)) {
+			netdev_err(netdev, "data_direct requires SHAMPO (Header-Data Split) support\n");
+			return -EOPNOTSUPP;
+		}
+
+		if (mlx5_get_sd(mdev)) {
+			netdev_err(netdev, "data_direct is not supported with socket direct\n");
+			return -EOPNOTSUPP;
+		}
+
+		if (mlx5e_is_uplink_rep(priv)) {
+			netdev_err(netdev, "data_direct is not supported for uplink representors\n");
+			return -EOPNOTSUPP;
+		}
+
+		if (!mlx5_data_direct_get_dev(mdev->data_direct)) {
+			netdev_err(netdev, "data_direct device not bound\n");
+			return -ENODEV;
+		}
+
+		if (mlx5e_has_leased_rxqs(netdev)) {
+			netdev_err(netdev, "data_direct is not supported with leased RX queues\n");
+			return -EBUSY;
+		}
+	}
+
+	/* Changing the flag reopens the channels with a different DMA device,
+	 * which would break the memory providers installed on the RX queues
+	 * (devmem or io_uring zcrx) and the dmabufs bound for TX.
+	 */
+	for (i = 0; i < netdev->real_num_rx_queues; i++) {
+		if (netif_rxq_has_unreadable_mp(netdev, i)) {
+			netdev_err(netdev, "Can't change data_direct while rxq %u has a memory provider\n",
+				   i);
+			return -EBUSY;
+		}
+	}
+
+	if (netdev_has_dmabuf_binding(netdev, NULL)) {
+		netdev_err(netdev, "Can't change data_direct while dmabufs are bound\n");
+		return -EBUSY;
+	}
+
+	new_params = priv->channels.params;
+	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, enable);
+
+	return mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
 }
 
 static const struct pflag_desc mlx5e_priv_flags[MLX5E_NUM_PFLAGS] = {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 63d91ef0dda8..ba4b09ab2d70 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5914,7 +5914,7 @@ static int mlx5e_queue_start(struct net_device *dev,
 	return 0;
 }
 
-static bool mlx5e_has_leased_rxqs(struct net_device *netdev)
+bool mlx5e_has_leased_rxqs(struct net_device *netdev)
 {
 	for (u32 i = 0; i < netdev->real_num_rx_queues; i++)
 		if (READ_ONCE(__netif_get_rx_queue(netdev, i)->lease))
-- 
2.44.0

Re: [PATCH net-next V2 10/13] net: devmem: add netdev_has_dmabuf_binding() helper

[Stanislav Fomichev]

On 11/10, Tariq Toukan wrote:
> From: Dragos Tatulea <dtatulea@nvidia.com>
> 
> Currently there is no way to check whether a dmabuf is bound on a TX
> queue. RX side has netif_rxq_has_unreadable_mp() which gives a hint
> of it.
> 
> To help with that, add a helper to check whether a devmem dmabuf
> binding may still be in use on a given netdev, optionally filtered by
> the DMA device the dmabuf was mapped against.
> 
> A binding can outlive the user's unbind: it can still be referenced by
> in-flight TX skbs, whose DMA mappings are only valid for the device the
> dmabuf was attached to. Reporting such a binding as gone would let a
> driver swap the DMA device out from under those skbs. Bindings stay in
> the bindings xarray until they are freed, so walk it and report every
> binding of the netdev that is still mapped. Bindings that were already
> unmapped are not reported.
> 
> This API is necessary for the upcoming support of data direct in mlx5e.
> 
> Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
> Cc: Bobby Eshleman <bobbyeshleman@gmail.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
>  include/net/netmem.h | 24 ++++++++++++++++++++++++
>  net/core/devmem.c    | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
> 
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index fb693bc1702a..35bb6fa0f719 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -397,16 +397,40 @@ static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)
>  	return netmem_to_nmdesc(netmem)->dma_addr;
>  }
>  
> +struct net_device;
> +
>  #if defined(CONFIG_NET_DEVMEM)
>  static inline bool net_is_devmem_iov(const struct net_iov *niov)
>  {
>  	return niov->type == NET_IOV_DMABUF;
>  }
> +
> +/**
> + * netdev_has_dmabuf_binding - is there a dmabuf binding for a
> + * given @dev and @dma_dev
> + *
> + * @dev: netdev to check bindings
> + * @dma_dev: dma device to check, when NULL it will not be checked
> + *
> + * A binding is reported for as long as it is mapped, which can be long
> + * after the user unbound it: net_iovs of a TX binding can still be queued
> + * in flight, with DMA mappings that are only valid for the device the dmabuf
> + * was attached to.
> + *
> + * Return: true if a binding is found under @dev and possibly @dma_dev
> + */
> +bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev);
>  #else
>  static inline bool net_is_devmem_iov(const struct net_iov *niov)
>  {
>  	return false;
>  }
> +
> +static inline bool netdev_has_dmabuf_binding(struct net_device *dev,
> +					     struct device *dma_dev)
> +{
> +	return false;
> +}
>  #endif
>  
>  void net_iov_inc_ref(struct net_iov *niov);
> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 5c7d9640e9d1..dd3987bbd9f6 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -513,6 +513,49 @@ struct net_devmem_dmabuf_binding *net_devmem_lookup_dmabuf(u32 id)
>  	return binding;
>  }
>  
> +/* The attachment is cleared once the binding has been unmapped. */
> +static bool
> +net_devmem_binding_mapped_to(struct net_devmem_dmabuf_binding *binding,
> +			     struct device *dma_dev)
> +{
> +	lockdep_assert_held(&binding->lock);
> +
> +	return binding->attachment &&
> +	       (!dma_dev || binding->attachment->dev == dma_dev);
> +}
> +
> +bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev)
> +{
> +	struct net_devmem_dmabuf_binding *binding;
> +	unsigned long xa_idx;
> +	bool found = false;
> +
> +	netdev_assert_locked(dev);
> +
> +	/* A binding stays in the xarray until it is freed. binding->dev is
> +	 * only cleared under the instance lock of that netdev, and the free
> +	 * takes it before erasing the binding, so a binding found under @dev
> +	 * stays alive while the xarray lock is dropped.
> +	 */
> +	xa_lock(&net_devmem_dmabuf_bindings);
> +	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
> +		if (READ_ONCE(binding->dev) != dev)
> +			continue;
> +
> +		xa_unlock(&net_devmem_dmabuf_bindings);
> +		mutex_lock(&binding->lock);
> +		found = net_devmem_binding_mapped_to(binding, dma_dev);
> +		mutex_unlock(&binding->lock);
> +		xa_lock(&net_devmem_dmabuf_bindings);

Sorry, my cache is already empty after c4466d2c3e77.. I wonder if it would
be easier to change kfree(binding) to kfree_rcu(binding, ...) and then
we can rcu_lock/xa_for_each here without xa_lock/mutex? All we care is our
netdev bindings, and these are not being changed as long as we
hold the instance lock (but the other ones are now concurrently freed,
hence rcu so we can safely do READ_ONCE on the binding)..

And then the same applies to path 11?

(trying to keep all the complexity in net_devmem_dev_unregister)

Re: [PATCH net-next V2 02/13] net/mlx5: Detach auxiliary devices on attach failure during load

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message says the fix "doesn't deserve a Fixes tag"
  because the bug was found by code inspection and never reproduced.

--

net/mlx5: Detach auxiliary devices on attach failure during load

This patch adds a mlx5_detach_device() call to the err_attach path of
mlx5_load_one_devl_locked(). The goal is to detach the auxiliary devices
that attached successfully before mlx5_attach_device() hit a failure.

> This issue was found during code inspection and was not reproduced.
> Hence it doesn't deserve a Fixes tag.

[Severity: Low]
Is this a valid reason to leave out the Fixes tag? The Fixes convention
points at the commit that introduced the bug. It doesn't exempt bugs that
were found by inspection and never reproduced.

mlx5_attach_device() in dev.c stops at the first failing entry. It does
not unwind the entries it has already added or resumed:

drivers/net/ethernet/mellanox/mlx5/core/dev.c:mlx5_attach_device() {
    ...
		if (ret) {
			mlx5_core_warn(dev, "Device[%d] (%s) failed to load\n",
				       i, mlx5_adev_devices[i].suffix);

			break;
		}
    ...
}

Before this patch, err_attach in mlx5_load_one_devl_locked() went
directly to clear_bit() and mlx5_unload(). mlx5_unload() destroys the
EQ table, the IRQ table, the flow steering core, the clock and the
bfreg. mlx5_function_teardown() then tears down the HCA.

The aux drivers at lower indices were still bound at that point. One
example is a resumed mlx5e netdev whose CQs still reference the freed
EQs. Later, TX timeout recovery or the remove path
(mlx5_unregister_device() -> aux remove -> CQ destroy) could touch
those freed resources.

This can be triggered by any aux probe or resume failure, such as
-ENOMEM, during devlink reload, firmware reset, health recovery
(recovery=true) or PCI resume.

According to git blame, the err_attach label without a detach came from:

Fixes: 6dea2f7eff96 ("net/mlx5: Separate probe vs. reload flows")

Could the tag be added, and could the commit message say what happens
without the fix (resources torn down while aux drivers are still
attached)? Without the tag, stable maintainers have no signal to pick
this up.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 1a45474ce03db..7e762f72b19ef 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -1547,6 +1547,7 @@ int mlx5_load_one_devl_locked(struct mlx5_core_dev *dev, bool recovery)
>  	return 0;
>  
>  err_attach:
> +	mlx5_detach_device(dev, true);
>  	clear_bit(MLX5_INTERFACE_STATE_UP, &dev->intf_state);
>  	mlx5_unload(dev);
>  err_load:

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110183826.2927274-1-tariqt%40nvidia.com

Re: [PATCH net-next V2 06/13] net/mlx5e: Add data direct RX infrastructure

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 3 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] A data-direct RQ's KSM UMR mkey has child entries from two
  different PDs and DMA domains.
- [Medium] mlx5e_alloc_rq() calls mlx5e_rq_alloc_data_direct_mkey() right
  after mlx5e_create_rq_umr_mkey().
- [Medium] net/mlx5e: SHAMPO header buffer double-sized in
  mlx5e_shampo_hd_per_wqe()
- [Low] Data-direct RQs are forced into MLX5E_MPWRQ_UMR_MODE_UNALIGNED
  (KSM) by mlx5e_mpwrq_umr_mode().
- [Low] mlx5e_create_umr_mkey() rejects MLX5E_MPWRQ_UMR_MODE_UNALIGNED
  when MLX5_CAP_GEN(mdev, fixed_buffer_size) is missing and logs…

--

net/mlx5e: Add data direct RX infrastructure

This commit adds the RX side infrastructure for data direct in mlx5e. It
adds a new RQ state flag and a crossing mkey over the RQ's UMR mkey. It
also creates the KSM UMR mkey in the data direct PD and sizes the ICOSQ for
the KSM UMR mode. Data direct itself is enabled in a later commit.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
> index 5caf7820a136b..61a77ee77244a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
> @@ -62,6 +62,9 @@ mlx5e_mpwrq_umr_mode(struct mlx5_core_dev *mdev,
>  	bool unaligned = xsk ? xsk->unaligned : false;
>  	bool oversized = false;
>  
> +	if (rqo && rqo->data_direct)
> +		return MLX5E_MPWRQ_UMR_MODE_UNALIGNED;
> +

[Severity: Medium]
mlx5e_open_channel() only enables data direct together with SHAMPO,
so every data-direct RQ also allocates a SHAMPO header buffer.
Switching those RQs to UNALIGNED here changes the WQE geometry, but
the header buffer sizing doesn't seem to follow.

mlx5_rq_shampo_alloc() sizes the buffer through
mlx5e_shampo_hd_per_wq(), which calls mlx5e_shampo_hd_per_wqe(). That
helper still computes the stride geometry without rqo:

	u16 num_strides = BIT(mlx5e_mpwqe_get_log_num_strides(mdev, params, NULL));
	u8 log_stride_sz = mlx5e_mpwqe_get_log_stride_size(mdev, params, NULL);

With rqo == NULL this uses the ALIGNED (MTT) WQE size. With 4K pages
and 16-byte KSM entries, mlx5e_mpwrq_log_wqe_sz() is one smaller for
the data-direct RQ. Each WQE therefore holds half as many strides and
half as many header reservations.

The log_wq_sz in rq_param->rqc, however, was computed with rqo through
mlx5e_mpwqe_get_log_rq_size(), so the ring has twice as many WQEs.
mlx5e_shampo_hd_per_wq() then multiplies the unhalved hd_per_wqe by
the doubled wq_size, and hd_per_wq comes out twice as large as needed.

Apart from allocating twice the header pages and DMA mappings, this
looks like it can overflow the hardware field. Take an 8192 entry RX
ring with sw_mtu <= 1024. hd_per_wq would go from 32768 to 65536, and
mlx5e_create_rq() would write order_base_2(hd_per_wq) = 16 into the
4-bit log_headers_buffer_entry_num field, which truncates to 0. The
device would then be told the header buffer has one entry while the
driver posts headers across 65536 of them.

Should mlx5e_shampo_hd_per_wqe() take the RQ's rqo
(cparam->rq_opt), so it sees the same UMR mode as the RQ?
mlx5e_shampo_get_log_cq_size() already passes rqo to the same helpers.

[Severity: Low]

This puts data-direct RQs under the KSM limits. Is there anything that
checks the ring size against those limits, the way mlx5e_mpwrq_validate_xsk()
does for XSK?

mlx5e_ethtool_get_ringparam() still computes rx_max_pending from the
MLX5E_MPWRQ_UMR_MODE_ALIGNED geometry. Its comment says only XSK RQs may
clamp the queue length. mlx5e_mpwqe_get_log_rq_size() makes the same
assumption:

	if (WARN_ON_ONCE(params->log_rq_mtu_frames > log_pkts_per_wqe + max_log_rq_size))
		return max_log_rq_size;

KSM entries are twice the size of MTT entries, and max_num_entries is
bounded by 1 << log_max_klm_list_size. Take a device with
log_max_klm_list_size <= 15, an MTU around 9000 and an RX ring of 8192.
log_pkts_per_wqe would be 3 and max_log_rq_size would be 9, so 3 + 9 < 13.

Would that trigger the WARN_ON_ONCE() and quietly shrink the RX ring below
the size ethtool reports? Neither mlx5e_open_channel() nor the later
set_pflag_data_direct() appears to check for this. The comments in both
places would also no longer match the code.

[ ... ]

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 96b521d8bed35..cee73f3b7a997 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -425,7 +425,7 @@ static u8 mlx5e_mpwrq_access_mode(enum mlx5e_mpwrq_umr_mode umr_mode)
>  static int mlx5e_create_umr_mkey(struct mlx5_core_dev *mdev,
>  				 u32 npages, u8 page_shift, u32 *umr_mkey,
>  				 dma_addr_t filler_addr,
> -				 enum mlx5e_mpwrq_umr_mode umr_mode,
> +				 enum mlx5e_mpwrq_umr_mode umr_mode, u32 pdn,
>  				 u32 xsk_chunk_size)
>  {

[Severity: Low]

Data-direct RQs now use UNALIGNED mode, so they hit the existing
fixed_buffer_size check at the top of this function:

	if ((umr_mode == MLX5E_MPWRQ_UMR_MODE_UNALIGNED ||
	     umr_mode == MLX5E_MPWRQ_UMR_MODE_TRIPLE) &&
	    !MLX5_CAP_GEN(mdev, fixed_buffer_size)) {
		mlx5_core_warn(mdev, "Unaligned AF_XDP requires fixed_buffer_size capability\n");
		return -EINVAL;
	}

On a device without fixed_buffer_size, opening a data-direct channel would
fail with -EINVAL and a warning about AF_XDP, which the user never set up.

Should the message be updated? Alternatively, could fixed_buffer_size be
checked before data direct is enabled?

[ ... ]

> @@ -546,13 +548,80 @@ static int mlx5e_create_rq_umr_mkey(struct mlx5_core_dev *mdev, struct mlx5e_rq
>  			      __func__, wq_size, rq->mpwqe.mtts_per_wqe,
>  			      max_num_entries);
>  
> +	pdn = dd ? mdev->data_direct->pdn : mdev->mlx5e_res.hw_objs.pdn;
>  	err = mlx5e_create_umr_mkey(mdev, num_entries, rq->mpwqe.page_shift,
>  				    &umr_mkey, rq->wqe_overflow.addr,
> -				    rq->mpwqe.umr_mode, xsk_chunk_size);
> +				    rq->mpwqe.umr_mode, pdn,
> +				    xsk_chunk_size);

[Severity: Medium]

With dd set, the parent KSM mkey is now created in
mdev->data_direct->pdn. However, mlx5e_create_umr_mkey() still fills every
UNALIGNED entry with the regular NIC mkey and the filler page:

mlx5e_create_umr_mkey() {
    ...
	case MLX5E_MPWRQ_UMR_MODE_UNALIGNED:
		ksm = MLX5_ADDR_OF(create_mkey_in, in, klm_pas_mtt);
		for (i = 0; i < npages; i++)
			ksm[i] = (struct mlx5_ksm) {
				.key = cpu_to_be32(mdev->mlx5e_res.hw_objs.mkey),
				.va = cpu_to_be64(filler_addr),
			};
    ...
}

hw_objs.mkey belongs to hw_objs.pdn. filler_addr is rq->wqe_overflow.addr,
which is mapped with dma_map_page(rq->pdev, ...) against the NIC function.

At runtime, mlx5e_alloc_rx_mpwqe() only rewrites inline_ksms[0] through
inline_ksms[pages_per_wqe - 1] with ksm_child_mkey_be.
mlx5e_mpwrq_mtts_per_wqe() returns ALIGN(pages_per_wqe + 1, ...), so each
WQE has at least one overflow entry that is never rewritten. That entry
still points at hw_objs.mkey.

Does this leave the data-direct KSM mkey with child mkeys from two
different PDs? If the device checks that an indirect mkey and its child
mkeys share a PD, could a packet larger than MTU that writes into the
overflow slot cause a local protection error and put the RQ into error
recovery? The filler page is supposed to absorb such writes.

For comparison, mlx5_ib creates its data-direct KSM mkey in
data_direct->pdn. _mlx5r_umr_update_mr_pas() fills its entries only with
data_direct->mkey or mkey_ro, or with key=0/va=0 on ZAP.

This code looks unchanged at the end of the series.

[ ... ]

> @@ -975,10 +1050,14 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
>  		if (err)
>  			goto err_rq_drop_page;
>  
> -		err = mlx5e_rq_alloc_mpwqe_info(rq, node);
> +		err = mlx5e_rq_alloc_data_direct_mkey(rq);
>  		if (err)
>  			goto err_rq_mkey;

[Severity: Medium]

This creates the crossing mkey right after mlx5e_create_rq_umr_mkey().
At that point the crossed UMR mkey is still in the free state, because
mlx5e_create_umr_mkey() sets:

	MLX5_SET(mkc, mkc, free, 1);

The crossed mkey only becomes valid after the first UMR WQE completes on
the ICOSQ, once the channel is activated.

The other in-tree crossing user is reg_user_mr_dmabuf_by_data_direct() in
drivers/infiniband/hw/mlx5/mr.c, and it uses the opposite order. First,
reg_user_mr_dmabuf() enables the crossed KSM mkey through UMR. Only then is
reg_create_crossing_vhca_mr() called.

Will CREATE_MKEY with MLX5E_MKC_ACCESS_MODE_CROSSING always accept a
crossed mkey that is still free? If firmware checks the crossed mkey's
state at creation time, mlx5e_create_crossing_vhca_mkey() would fail here
for every data-direct RQ.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110183826.2927274-1-tariqt%40nvidia.com

Re: [PATCH net-next V2 07/13] net/mlx5e: SHAMPO, Use page_pool DMA sync helper for SKB frags

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message describes this patch as preparation for data
  direct only.

--

net/mlx5e: SHAMPO, Use page_pool DMA sync helper for SKB frags

This patch changes mlx5e_add_skb_frag() to call
page_pool_dma_sync_netmem_for_cpu() instead of calling
dma_sync_single_for_cpu() on rq->pdev directly. The stated reason is the
upcoming data direct support, where SHAMPO payload netmem is mapped
against the data direct device rather than rq->pdev.

> Switch to the page_pool DMA API which takes care of not calling
> dma_sync_* for unreadable netmem. Do it only for the place where it
> makes sense: SHAMPO payloads, the only RX buffers that can be
> unreadable.
>
> Until now it was ok to call the dma_sync_* API directly. But with the
> introduction of data direct it will no longer be correct: the payload
> netmem of a data direct RQ is mapped against the data direct device,
> not rq->pdev.

[Severity: Low]
Is "Until now it was ok to call the dma_sync_* API directly" accurate?

Since commit db3010bb5a01 ("net/mlx5e: Add support for UNREADABLE netmem
page pools"), mlx5e_alloc_rq() allows unreadable netmem for SHAMPO RQs:

drivers/net/ethernet/mellanox/mlx5/core/en_main.c:mlx5e_alloc_rq() {
    ...
		/* Shampo header data split allow for unreadable netmem */
		if (test_bit(MLX5E_RQ_STATE_SHAMPO, &rq->state))
			pp_params.flags |= PP_FLAG_ALLOW_UNREADABLE_NETMEM;
    ...
}

This means these RQs can be backed by the dma-buf devmem memory provider,
which does this:

net/core/devmem.c:mp_dmabuf_devmem_init() {
    ...
	/* dma-buf dma addresses do not need and should not be used with
	 * dma_sync_for_cpu/device. Force disable dma_sync.
	 */
	pool->dma_sync = false;
	pool->dma_sync_for_cpu = false;
    ...
}

That rule was added by commit 7dba339faae9 ("page_pool: disable sync for
cpu for dmabuf memory provider"), before mlx5 supported unreadable
netmem. Its commit message asks drivers to switch to this helper when
they add netmem support.

Before this patch, mlx5e_add_skb_frag() always called
dma_sync_single_for_cpu(rq->pdev, addr + frag_offset, ...) on these
dma-buf addresses.

At this commit, pp_params.dev is rq->pdev and pp_params.dma_dir is
rq->buff.map_dir. For regular page pools the new helper therefore does
the same thing as the old code. The only functional change is that the
sync is now skipped for dma-buf devmem pools, and that fixes the
existing behaviour.

Should the commit message say that this also fixes the devmem case? It
could also carry a Fixes: db3010bb5a01 tag, so backporters don't treat it
as prep work for data direct only.

A smaller point: page_pool_dma_sync_netmem_for_cpu() does not check for
unreadable netmem in general. It checks a per-pool flag:

	if (!pool->dma_sync_for_cpu)
		return;

Only the dma-buf provider clears that flag. io_uring zcrx pools are also
unreadable, but they leave it set and are still synced through
pool->p.dev.

Could the wording "takes care of not calling dma_sync_* for unreadable
netmem" be made more precise?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110183826.2927274-1-tariqt%40nvidia.com

Re: [PATCH net-next V2 09/13] net/mlx5e: Use the correct DMA dev when data_direct is enabled

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This patch makes the data-direct PCI device (dd_dev->device) the
  DMA device for the page_pool of data-direct RQs (mlx5e_alloc_rq,…
- [Low] mlx5e_get_dma_dev() (en_main.c:965-976) reads
  mdev->data_direct->dev with READ_ONCE() through…

--

net/mlx5e: Use the correct DMA dev when data_direct is enabled

This commit adds mlx5e_get_dma_dev(). When data_direct is enabled it
returns the data direct device instead of the PF PCI device. The helper
is used for the RQ page_pool DMA device and for .ndo_queue_get_dma_dev,
and the latter now refuses to return a device in data-direct mode when
RX queues are leased.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 5b99099e4ed0c..ebd41f24330d0 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -962,6 +962,19 @@ static void mlx5e_rq_free_shampo(struct mlx5e_rq *rq)
>  	kvfree(shampo);
>  }
>  
> +static struct device *mlx5e_get_dma_dev(struct mlx5_core_dev *mdev,
> +					bool data_direct)
> +{
> +	struct mlx5_data_direct_dev *dd_dev;
> +
> +	if (!data_direct)
> +		return mlx5_core_dma_dev(mdev);
> +
> +	dd_dev = mlx5_data_direct_get_dev(mdev->data_direct);
> +
> +	return dd_dev ? dd_dev->device : NULL;

[Severity: Low]
Is anything keeping dd_dev alive between the READ_ONCE() in
mlx5_data_direct_get_dev() and the dd_dev->device dereference here?

The unbind side clears the pointer, runs the notifier chain, and then
frees the device:

mlx5_data_direct_remove()
    mlx5_data_direct_dev_unreg(dev)
        ...
        mlx5_data_direct_do_unbind()
            WRITE_ONCE(reg->dd.dev, NULL);
            blocking_notifier_call_chain(&reg->users,
                                         MLX5_DATA_DIRECT_UNBIND, NULL);
    pci_disable_device(pdev);
    kfree(dev->vuid);
    kfree(dev);

At this commit, mlx5e_data_direct_event() only logs on
MLX5_DATA_DIRECT_UNBIND. It takes none of the locks the readers hold,
which is priv->state_lock in mlx5e_queue_get_dma_dev() and in the channel
open path that reaches mlx5e_alloc_rq().

If a reader has already loaded a non-NULL dd_dev, can it dereference it
after kfree(dev)? The returned struct device pointer is also used
without taking a reference.

This does not look reachable at this commit. set_pflag_data_direct()
still returns -EOPNOTSUPP, so data_direct is never true here.

Later in the series, "net/mlx5e: Disable data direct on data direct
device unbind" makes the notifier take rtnl_lock(), netdev_lock() and
priv->state_lock before it returns. That serializes it against these
readers before kfree(dev) runs.

> +}
> +
>  static int mlx5e_alloc_rq(struct mlx5e_params *params,
>  			  struct mlx5e_rq_param *rq_param,
>  			  struct mlx5e_rq_opt_param *rqo,
> @@ -1105,7 +1118,11 @@ static int mlx5e_alloc_rq(struct mlx5e_params *params,
>  		pp_params.flags     = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;
>  		pp_params.pool_size = pool_size;
>  		pp_params.nid       = node;
> -		pp_params.dev       = rq->pdev;
> +		pp_params.dev       = mlx5e_get_dma_dev(mdev, data_direct);

[Severity: Low]
With this change, the page_pool of a data direct RQ is created against
dd_dev->device. mlx5e_queue_get_dma_dev() also hands that device to
memory providers (devmem, io_uring zcrx), which map their buffers
against it.

What tears these down when the data direct device is unbound? At this
commit, this is the only mlx5e consumer of MLX5_DATA_DIRECT_UNBIND:

mlx5e_data_direct_event() {
    ...
	if (action == MLX5_DATA_DIRECT_UNBIND)
		mlx5_core_dbg(priv->mdev, "data direct device unbound\n");

	return NOTIFY_OK;
}

It doesn't close the data direct RQs, destroy their page pools, or
unbind the memory provider mappings. mlx5_data_direct_remove() then
runs pci_disable_device() and kfree(dev) right after the notifier
returns.

Could the page_pool and dmabuf mappings outlive the device they were
mapped against?

This also doesn't appear reachable at this commit, since
set_pflag_data_direct() in en_ethtool.c still returns -EOPNOTSUPP.

Later in the series, "net/mlx5e: Disable data direct on data direct
device unbind" rewrites mlx5e_data_direct_event(). The new version calls
netif_close() and netdev_unbind_dma_dev(), then clears
MLX5E_PFLAG_DATA_DIRECT. The flag only becomes settable later, in
"net/mlx5e: Enable the data direct netdev feature".

> +		if (!pp_params.dev) {
> +			err = -ENODEV;
> +			goto err_free_by_rq_type;
> +		}
>  		pp_params.napi      = rq->cq.napi;
>  		pp_params.netdev    = rq->netdev;
>  		pp_params.dma_dir   = rq->buff.map_dir;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110183826.2927274-1-tariqt%40nvidia.com

Re: [PATCH net-next V2 10/13] net: devmem: add netdev_has_dmabuf_binding() helper

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The kernel-doc and commit message promise more than
  netdev_has_dmabuf_binding() actually checks.

--

net: devmem: add netdev_has_dmabuf_binding() helper

This adds an exported helper, netdev_has_dmabuf_binding(). It walks the
global devmem bindings xarray and reports whether a binding of a given
netdev is still mapped, with an optional filter on the DMA device. The
goal is to let drivers (data direct support in mlx5e) avoid swapping
the DMA device while in-flight skbs may still use dmabuf mappings.

> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index fb693bc1702a4..35bb6fa0f7194 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h

[ ... ]

> +/**
> + * netdev_has_dmabuf_binding - is there a dmabuf binding for a
> + * given @dev and @dma_dev
> + *
> + * @dev: netdev to check bindings
> + * @dma_dev: dma device to check, when NULL it will not be checked
> + *
> + * A binding is reported for as long as it is mapped, which can be long
> + * after the user unbound it: net_iovs of a TX binding can still be queued
> + * in flight, with DMA mappings that are only valid for the device the dmabuf
> + * was attached to.
> + *
> + * Return: true if a binding is found under @dev and possibly @dma_dev
> + */
> +bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev);

[ ... ]

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 5c7d9640e9d14..dd3987bbd9f6c 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c

[ ... ]

> +bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev)
> +{
> +	struct net_devmem_dmabuf_binding *binding;
> +	unsigned long xa_idx;
> +	bool found = false;
> +
> +	netdev_assert_locked(dev);
> +
> +	/* A binding stays in the xarray until it is freed. binding->dev is
> +	 * only cleared under the instance lock of that netdev, and the free
> +	 * takes it before erasing the binding, so a binding found under @dev
> +	 * stays alive while the xarray lock is dropped.
> +	 */
> +	xa_lock(&net_devmem_dmabuf_bindings);
> +	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
> +		if (READ_ONCE(binding->dev) != dev)
> +			continue;

[Severity: Medium]
Does this check match what the kernel-doc promises, "A binding is
reported for as long as it is mapped"? The commit message also says the
helper will "report every binding of the netdev that is still mapped".

For leased RX queues, binding->dev may not be the device that holds the
mapping. netdev_nl_bind_rx_doit() passes the netdev the user named,
which can be a virtual leasee such as netkit:

net/core/netdev-genl.c:netdev_nl_bind_rx_doit() {
    ...
    binding = net_devmem_bind_dmabuf(netdev, NULL, dma_dev, DMA_FROM_DEVICE,
    ...
}

The dma_dev, though, comes from the physical queue the RX queue is
leased from:

net/core/netdev_queues.c:netdev_queue_get_dma_dev() {
    ...
	hw_rxq = __netif_get_rx_queue(dev, idx)->lease;

	netdev_lock(hw_rxq->dev);
	idx = get_netdev_rx_queue_index(hw_rxq);
	dma_dev = __netdev_queue_get_dma_dev(hw_rxq->dev, idx);
    ...
}

So binding->dev is the leasee, while binding->attachment->dev is the
DMA device of the physical NIC. That also seems to disagree with the
comment on struct net_devmem_dmabuf_binding::dev ("Physical NIC that
does the actual DMA for this binding"), which this helper relies on.

On top of that, both teardown paths skip the unmap in the lease case
but still clear binding->dev:

net/core/devmem.c:mp_dmabuf_devmem_uninstall() {
    ...
			if (!rxq->lease)
				net_devmem_dmabuf_binding_unmap(binding);
			ASSERT_EXCLUSIVE_WRITER(binding->dev);
			WRITE_ONCE(binding->dev, NULL);
    ...
}

net/core/devmem.c:net_devmem_dev_unregister() {
    ...
		if (!netif_is_queue_leasee(dev))
			net_devmem_dmabuf_binding_unmap(binding);
		xa_lock(&net_devmem_dmabuf_bindings);
		WRITE_ONCE(binding->dev, NULL);
    ...
}

After this, the binding stays in the xarray with attachment != NULL
until in-flight net_iovs release it. The check above can no longer
match it for any dev.

Could netdev_has_dmabuf_binding(phys_dev, phys_dma_dev) then return
false while a binding is still mapped against phys_dma_dev?

The only caller in this series, set_pflag_data_direct() in mlx5e,
already rejects installed RX memory providers and leased RX queues, so
it may not hit this today. Since the helper is exported, would it make
sense to either restrict the kernel-doc and commit message to TX
bindings (or bindings whose dev is still set), or to also match
bindings by attachment->dev against the DMA device(s) of @dev?

> +
> +		xa_unlock(&net_devmem_dmabuf_bindings);
> +		mutex_lock(&binding->lock);
> +		found = net_devmem_binding_mapped_to(binding, dma_dev);
> +		mutex_unlock(&binding->lock);
> +		xa_lock(&net_devmem_dmabuf_bindings);
> +		if (found)
> +			break;
> +	}
> +	xa_unlock(&net_devmem_dmabuf_bindings);
> +
> +	return found;
> +}
> +EXPORT_SYMBOL_GPL(netdev_has_dmabuf_binding);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110183826.2927274-1-tariqt%40nvidia.com

Re: [PATCH net-next V2 11/13] net: add netdev_unbind_dma_dev() helper

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 7 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 · Low: 4

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] netdev_unbind_dma_dev() (net/core/netdev_queues.c:84) refuses
  only when the netdev is both running and present (`netif_running(dev)…
- [Medium] The two halves of netdev_unbind_dma_dev() use different rules
  for which mappings belong to @dma_dev.
- [Medium] __netif_mp_close_rxq() (net/core/netdev_rx_queue.c:291)
  quietly accepts an already-uninstalled provider only when…
- [Low] The commit message says "A detached netdev also works because ...
  new bindings can't be added", but the core does not enforce this for…
- [Low] Several in-tree comments that list binding lifecycle rules were
  not updated for the new site that clears and unmaps bindings, and the…
- [Low] netdev_unbind_dma_dev() treats a NULL result from
  __netdev_queue_get_dma_dev() as "this queue maps against the departing…
- [Low] net_devmem_unbind_dma_dev() (devmem.c:575-596) clears
  NET_DEVMEM_BINDING_LIVE to "hide the binding from TX lookups while it…

--

net: add netdev_unbind_dma_dev() helper

This adds an exported helper for a closed or detached netdev. It
uninstalls the memory providers of RX queues mapped against a departing
DMA device and unmaps the devmem bindings that use that device. It is
meant for mlx5 data direct, where the DMA device is not the netdev's own
PCI device.

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index dd3987bbd9f6c..bc8ae12b2318a 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -556,6 +556,48 @@ bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev)
>  }
>  EXPORT_SYMBOL_GPL(netdev_has_dmabuf_binding);
>  
> +/* Unmap the bindings of a closed @dev that use @dma_dev, the way netdev
> + * unregister does. Senders that raced the detach drop their skbs at xmit.
> + */
> +void net_devmem_unbind_dma_dev(struct net_device *dev, struct device *dma_dev)
> +{
> +	struct net_devmem_dmabuf_binding *binding;
> +	unsigned long xa_idx;
> +	bool match, live;
> +
> +	netdev_assert_locked(dev);
> +
> +	xa_lock(&net_devmem_dmabuf_bindings);
> +	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
> +		if (READ_ONCE(binding->dev) != dev)
> +			continue;
> +
> +		/* Hide the binding from TX lookups while it is checked. */
> +		live = xa_get_mark(&net_devmem_dmabuf_bindings, xa_idx,
> +				   NET_DEVMEM_BINDING_LIVE);
> +		if (live)
> +			__xa_clear_mark(&net_devmem_dmabuf_bindings, xa_idx,
> +					NET_DEVMEM_BINDING_LIVE);
> +		xa_unlock(&net_devmem_dmabuf_bindings);

[Severity: Low]
Does clearing the mark here actually hide the binding from TX lookups?
net_devmem_lookup_dmabuf() runs under rcu_read_lock() and does:

    binding = xa_find(&net_devmem_dmabuf_bindings, &xa_idx, id,
                      NET_DEVMEM_BINDING_LIVE);

net_devmem_unbind_dmabuf() needs a grace period after clearing the mark
for exactly this reason:

    /* Ensure no tx net_devmem_lookup_dmabuf() are in flight after the
     * mark is cleared.
     */
    synchronize_net();

There is no synchronize_net() here. A lookup already in flight can still
find a matching binding and take a reference. The safety of matching
bindings then rests only on clearing binding->dev and the check at xmit
time.

There is also a side effect for bindings that don't match. A binding on
@dev that doesn't use @dma_dev is invisible while xa_lock is dropped. A
concurrent SO_DEVMEM_TX sendmsg on it could get a spurious -EINVAL.

The mark is cleared on entry and restored on exit to guard this region,
rather than to express state. Is the clear and restore needed at all?

> +
> +		mutex_lock(&binding->lock);
> +		match = net_devmem_binding_mapped_to(binding, dma_dev);
> +		mutex_unlock(&binding->lock);
> +		if (match)
> +			net_devmem_dmabuf_binding_unmap(binding);

[Severity: Medium]
The commit message calls this half "devmem TX", and the comment says it
works "the way netdev unregister does". However, nothing here filters on
binding->direction or LIVE. net_devmem_dev_unregister() skips LIVE RX
bindings:

    if (binding->direction == DMA_FROM_DEVICE) {
        if (READ_ONCE(binding->dev) != dev ||
            xa_get_mark(..., NET_DEVMEM_BINDING_LIVE))
            continue;

The two halves of netdev_unbind_dma_dev() also pick mappings by
different rules. The RX loop uses the driver's current per-queue report
and skips leased queues. Here, net_devmem_binding_mapped_to() treats a
NULL @dma_dev as a wildcard:

    return binding->attachment &&
           (!dma_dev || binding->attachment->dev == dma_dev);

Take an RX binding whose queue the RX loop skipped but which matches
here. That can happen with a NULL @dma_dev, with a driver that already
reports a different device, or on the leased-queue path. Such a binding
is unmapped and gets binding->dev cleared while rxq->mp_params and
binding->bound_rxqs still point to it. mp_dmabuf_devmem_uninstall()
avoids this by clearing dev only once bound_rxqs is empty.

A later netlink socket close then reaches net_devmem_unbind_dmabuf()
with a NULL binding->dev:

    if (netif_mp_close_rxq(binding->dev, rxq_idx, &mp_params))

Wouldn't netif_mp_close_rxq() then dereference dev->real_num_rx_queues
through NULL? If the netdev is reopened first, the page pool would be
built from the provider that is still installed, over an sgt that has
already been unmapped.

This doesn't look reachable from the in-tree mlx5 caller. But the helper
is exported, and netdev_has_dmabuf_binding() documents a NULL dma_dev as
"any". Should this half filter on DMA_TO_DEVICE, or should a NULL
@dma_dev be rejected?

> +
> +		xa_lock(&net_devmem_dmabuf_bindings);
> +		if (match) {
> +			WRITE_ONCE(binding->vdev, NULL);
> +			WRITE_ONCE(binding->dev, NULL);

[Severity: Low]
This isn't a bug, but some comments about the binding lifecycle don't
cover this new site.

The net_devmem_lock_dev() kernel-doc lists where binding->dev is
cleared:

 * NULL while holding netdev_lock(dev) (in mp_dmabuf_devmem_uninstall(),
 * net_devmem_unbind_dmabuf(), or net_devmem_dev_unregister() during

In struct net_devmem_dmabuf_binding, the vdev comment says "cleared with
WRITE_ONCE() on unregistration". The ref comment says "TX bindings are
also unmapped earlier if their device is unregistered". Both of these now
also happen when the DMA device goes away.

The new netdev_unbind_dma_dev() kernel-doc says it uninstalls queues
"that map against @dma_dev". It also uninstalls queues for which the
driver reports no DMA device at all.

Should these comments be updated?

> +		} else if (live) {
> +			__xa_set_mark(&net_devmem_dmabuf_bindings, xa_idx,
> +				      NET_DEVMEM_BINDING_LIVE);
> +		}
> +	}
> +	xa_unlock(&net_devmem_dmabuf_bindings);
> +}

[ ... ]

> diff --git a/net/core/netdev_queues.c b/net/core/netdev_queues.c
> index 27cb6397bdd4e..71cc7bfc087dd 100644
> --- a/net/core/netdev_queues.c
> +++ b/net/core/netdev_queues.c

[ ... ]

> @@ -58,6 +60,55 @@ struct device *netdev_queue_get_dma_dev(struct net_device *dev,
>  	return dma_dev;
>  }
>  
> +/**
> + * netdev_unbind_dma_dev() - drop the memory providers of a departing DMA device
> + * @dev:	net_device, closed or detached by the caller
> + * @dma_dev:	DMA device the driver is about to lose

[Severity: Low]
The commit message says:

  A detached netdev also works because the driver has already torn down
  the queues and new bindings can't be added.

Is that enforced for io_uring zcrx? The devmem netlink bind paths check
netif_device_present(). zcrx_register_netdev() only needs
netdev_queue_get_dma_dev() to return non-NULL before it calls
netif_mp_open_rxq(). Neither netif_mp_open_rxq() nor
__netif_mp_open_rxq() checks whether the device is present.

So once this returns and the instance lock is dropped, the only thing
stopping a new zcrx area from being mapped against the departing device
is the driver's ndo_queue_get_dma_dev() no longer reporting it. mlx5
does stop reporting it: channels->num is 0 when closed, and dd.dev is
cleared before the notifier runs.

The kernel-doc only says "net_device, closed or detached by the caller".
Should it also state that the driver must stop reporting @dma_dev?

> + *
> + * Uninstalls the memory provider of every RX queue of @dev that maps
> + * against @dma_dev. Unmaps the devmem bindings of @dev that use @dma_dev.
> + * The bindings stay allocated until their users release them.

[ ... ]

> + * Return: 0 on success, -EBUSY if @dev is running and present.
> + */
> +int netdev_unbind_dma_dev(struct net_device *dev, struct device *dma_dev)
> +{
> +	unsigned int i;
> +
> +	netdev_assert_locked(dev);
> +
> +	if (netif_running(dev) && netif_device_present(dev))
> +		return -EBUSY;

[Severity: High]
Can a netdev that is running but detached get past this check while
validated devmem TX skbs are still sitting in a qdisc?

The commit message says:

  The netdev must be closed or detached so that the hardware no longer
  accesses the mappings and the qdiscs hold no validated skbs.

netif_device_detach() only clears __LINK_STATE_PRESENT and stops the TX
queues. Only dev_close()->dev_deactivate()->qdisc_reset() empties the
qdiscs.

The mlx5 caller later in this series seems to reach exactly this state
in mlx5e_nic_disable():

    if (netif_running(priv->netdev))
        mlx5e_close(priv->netdev);
    netif_device_detach(priv->netdev);
    ...
    mlx5e_data_direct_unbind_dev(priv);

mlx5e_close() is the ndo_stop, so __LINK_STATE_START stays set. The
check above then passes because the device is not present.

Consider this sequence:

sch_direct_xmit()
  validate_xmit_skb_list()
    validate_xmit_unreadable_skb()   <- binding->dev still set, passes
                                     mlx5e_close() stops the txq
  dev_requeue_skb(skb, q)            <- skb parked in q->gso_skb

netdev_unbind_dma_dev()
  net_devmem_unbind_dma_dev()
    net_devmem_dmabuf_binding_unmap()
      dma_buf_unmap_attachment()
      dma_buf_unpin()

On resume, mlx5e_open() and netif_device_attach() wake the queue.
dequeue_skb() then returns the gso_skb with:

    /* skb in gso_skb were already validated */
    *validate = false;

The binding->dev check in validate_xmit_unreadable_skb() never runs
again for that skb. Would the NIC then be given net_iov DMA addresses
that are no longer mapped for the DMA device?

The comment above net_devmem_unbind_dma_dev() says "Senders that raced
the detach drop their skbs at xmit". That doesn't seem to hold for these
requeued skbs.

For RX, the check relies on an unenforced convention that a detached
driver has already torn down its RX rings. mlx5 meets it because it
closes before detaching. A caller that only detaches would have its
dmabuf and zcrx mappings revoked while RX descriptors still point at
them. Should the exported helper enforce or document this?

> +
> +	for (i = 0; i < dev->real_num_rx_queues; i++) {
> +		struct netdev_rx_queue *rxq = __netif_get_rx_queue(dev, i);
> +		struct pp_memory_provider_params p = rxq->mp_params;
> +		struct device *queue_dev;
> +
> +		if (!p.mp_ops)
> +			continue;
> +
> +		queue_dev = __netdev_queue_get_dma_dev(dev, i);
> +		if (queue_dev && queue_dev != dma_dev)
> +			continue;

[Severity: Low]
Here a NULL from __netdev_queue_get_dma_dev() counts as a match. Is that
intended for a closed netdev?

mlx5e_queue_get_dma_dev() returns NULL whenever:

    /* Allow only opened channels. */
    if (queue_index < 0 || queue_index >= channels->num)
        goto out;

mlx5e_close_channels() sets chs->num = 0. So in the closed state this
helper requires, the @dma_dev filter appears to do nothing for the only
in-tree caller. Every RX queue with a provider is uninstalled, whichever
device it is actually mapped to.

If a driver's queues can map against more than one device, wouldn't
providers on a device that isn't going away be torn down as well? A
leased queue with a provider would also get past this filter and hit
the WARN_ON_ONCE(rxq->lease). The TX half filters on
binding->attachment->dev instead.

mlx5 doesn't allow data direct to be toggled while providers exist, so
there's no in-tree effect today. The semantics still look fragile.

> +
> +		if (WARN_ON_ONCE(rxq->lease))
> +			continue;
> +
> +		memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
> +		__netif_mp_uninstall_rxq(rxq, &p);

[Severity: Medium]
Can this race with io_close_queue() in io_uring/zcrx.c and trigger the
WARN_ON_ONCE() in __netif_mp_close_rxq()?

__netif_mp_close_rxq() only accepts an already-uninstalled provider on
the unregister path:

    if (dev->reg_state > NETREG_REGISTERED &&
        !rxq->mp_params.mp_ops)
        return 0;

    if (WARN_ON_ONCE(rxq->mp_params.mp_ops != old_p->mp_ops ||

Here mp_params is cleared on a device that is still registered. One
possible sequence:

io_close_queue()
  copies ifq->netdev, sets ifq->netdev = NULL under pp_lock
  netdev_lock(netdev)   <- blocks, held by the unbind caller
                                  netdev_unbind_dma_dev()
                                    memset(&rxq->mp_params, 0, ...)
                                    __netif_mp_uninstall_rxq()
                                      io_pp_uninstall()
                                        io_zcrx_unmap_areas()
                                        io_zcrx_drop_netdev()
                                        (ifq->if_rxq left unchanged)
  if (ifq->if_rxq != -1)
      netif_mp_close_rxq(netdev, ifq->if_rxq, &p);
        __netif_mp_close_rxq()
          reg_state == NETREG_REGISTERED, mp_ops == NULL
          WARN_ON_ONCE(...) -> -EINVAL

The areas are already unmapped, so cleanup still completes. Would a
zcrx teardown that races with a data direct unbind or a suspend produce
a WARN splat (fatal with panic_on_warn) plus a spurious -EINVAL?

> +	}
> +
> +	net_devmem_unbind_dma_dev(dev, dma_dev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(netdev_unbind_dma_dev);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110183826.2927274-1-tariqt%40nvidia.com

Re: [PATCH net-next V2 12/13] net/mlx5e: Disable data direct on data direct device unbind

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The suspend teardown and the notifier teardown leave the
  data_direct private flag in different states.
- [Medium] mlx5e_data_direct_event() can run against a netdev that
  mlx5e_nic_disable() or mlx5e_nic_enable() is in the middle of…
- [Low] Socket-direct gap: both new paths only look at priv->mdev's data-
  direct device, while mlx5e_queue_get_dma_dev() picks the DMA device…
- [Low] Notifier payload contract: mlx5_data_direct_do_unbind() now
  passes reg->dd.dev as the UNBIND payload.

--

net/mlx5e: Disable data direct on data direct device unbind

When the data direct device is unbound, the notifier handler now stops the
netdev, releases the providers bound to that DMA device and clears the
data_direct private flag. The suspend path also releases those providers,
because the notifier is not registered while the device is suspended.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c b/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c
> index 7db94b8a5b79c..246a03f44afd3 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/data_direct.c
> @@ -198,9 +198,11 @@ static void mlx5_data_direct_bind(struct mlx5_data_direct_registration *reg,
>  static void
>  mlx5_data_direct_do_unbind(struct mlx5_data_direct_registration *reg)
>  {
> +	struct mlx5_data_direct_dev *dev = reg->dd.dev;
> +
>  	WRITE_ONCE(reg->dd.dev, NULL);
>  	blocking_notifier_call_chain(&reg->users, MLX5_DATA_DIRECT_UNBIND,
> -				     NULL);
> +				     dev);
>  }

[Severity: Low]
This isn't a bug, but the UNBIND payload is now an implicit contract. It is
not documented next to enum mlx5_data_direct_event in
include/linux/mlx5/data_direct.h.

reg->dd.dev can be NULL here when mlx5_data_direct_cleanup() calls this for
a registration that was never bound. mlx5e_data_direct_event() dereferences
dd_dev->device without a NULL check.

As far as I can tell, that path can't be hit today.
mlx5_data_direct_cleanup() is only called from mlx5_unload(). By then,
mlx5_detach_device() or mlx5_unregister_device() has already gone through
mlx5e_nic_disable()->mlx5_data_direct_unregister(). The dereference also
sits behind the pflag check, and the pflag can only be set while dd.dev is
non-NULL.

Would it make sense to document the payload type and the fact that it can
be NULL, or to add a NULL check in the handler?

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index ebd41f24330d0..63d91ef0dda80 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -6170,9 +6170,53 @@ static int mlx5e_data_direct_event(struct notifier_block *nb,

[ ... ]

> +	if (action != MLX5_DATA_DIRECT_UNBIND)
> +		return NOTIFY_DONE;
> +
> +	rtnl_lock();
> +	netdev_lock(netdev);
> +
> +	mutex_lock(&priv->state_lock);
> +	enabled = MLX5E_GET_PFLAG(&priv->channels.params,
> +				  MLX5E_PFLAG_DATA_DIRECT);
> +	mutex_unlock(&priv->state_lock);
> +	if (!enabled)
> +		goto out;
> +
> +	reopen = netif_running(netdev);
> +	netif_close(netdev);

[Severity: Medium]
Can this handler run against a netdev that mlx5e_nic_disable() has already
detached? Nothing checks netif_device_present() before netif_close().

On the disable side, mlx5_data_direct_do_unbind() sets dd.dev to NULL. The
handler then blocks in rtnl_lock() while mlx5e_nic_disable() holds rtnl and
runs:

	if (netif_running(priv->netdev))
		mlx5e_close(priv->netdev);
	netif_device_detach(priv->netdev);
	...
	mlx5e_data_direct_unbind_dev(priv);
	netdev_unlock(priv->netdev);
	rtnl_unlock();

	mlx5e_nic_set_rx_mode(priv);

	mlx5_data_direct_unregister(mdev, &priv->data_direct_nb);

mlx5e_data_direct_unbind_dev() finds mlx5_data_direct_get_dev() == NULL and
returns without doing anything. mlx5e_close() is the ndo-level call, so
IFF_UP and __LINK_STATE_START stay set.

Once rtnl is dropped, the handler continues with the pflag still set and
reopen = true. netif_close()->__dev_close_many() clears IFF_UP and
__LINK_STATE_START and sends NETDEV_GOING_DOWN/NETDEV_DOWN. Then
netif_open()->__dev_open() fails with -ENODEV because the device is
detached.

The profile can be enabled again later, on resume or for the uplink rep
profile after an eswitch mode change. At that point mlx5e_nic_enable()
checks:

	if (netif_running(netdev))
		mlx5e_open(netdev);

That check is now false. Does the interface then stay administratively
down?

The enable side has a similar window. mlx5e_nic_enable() registers the
notifier before it takes rtnl and before netif_device_attach(). An unbind
handled in that window would do the same close and failed open on a netdev
that is still detached.

Is a netif_device_present() check needed here, or some other handling for
the detached case?

> +
> +	err = netdev_unbind_dma_dev(netdev, dd_dev->device);
> +	if (WARN_ON_ONCE(err)) {
> +		netdev_err(netdev, "Failed to unbind dma device, won't reopen netdev, err=%d\n",
> +			   err);
> +		goto out;
> +	}
> +
> +	mutex_lock(&priv->state_lock);
> +	new_params = priv->channels.params;
> +	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, false);
> +	mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, false);
> +	mutex_unlock(&priv->state_lock);
> +
> +	netdev_info(netdev, "data_direct device unbound, disabled data_direct\n");
>  
> -	if (action == MLX5_DATA_DIRECT_UNBIND)
> -		mlx5_core_dbg(priv->mdev, "data direct device unbound\n");
> +	if (reopen) {
> +		err = netif_open(netdev, NULL);
> +		if (err)
> +			netdev_err(netdev, "Failed to reopen after data_direct unbind, err=%d\n",
> +				   err);
> +	}

[ ... ]

> @@ -6407,6 +6451,24 @@ static int mlx5e_nic_enable(struct mlx5e_priv *priv)
>  	return err;
>  }
>  
> +static void mlx5e_data_direct_unbind_dev(struct mlx5e_priv *priv)
> +{
> +	struct mlx5_data_direct_dev *dd_dev;
> +	bool enabled;
> +
> +	dd_dev = mlx5_data_direct_get_dev(priv->mdev->data_direct);

[Severity: Low]
This helper and the notifier only look at priv->mdev's data direct device.
mlx5e_queue_get_dma_dev() picks the DMA device per channel:

	dma_dev = mlx5e_get_dma_dev(ch->mdev, dd);

Here ch->mdev comes from mlx5_sd_ch_ix_get_dev(). With socket direct, would
providers and dmabuf bindings mapped to a secondary PF's data direct device
stay in place?

This appears to be resolved later in the series. The patch "net/mlx5e:
Enable the data direct netdev feature" makes set_pflag_data_direct() return
-EOPNOTSUPP when mlx5_get_sd(mdev) is non-NULL, and at this commit the flag
can't be enabled at all.

> +	if (!dd_dev)
> +		return;
> +
> +	mutex_lock(&priv->state_lock);
> +	enabled = MLX5E_GET_PFLAG(&priv->channels.params,
> +				  MLX5E_PFLAG_DATA_DIRECT);
> +	mutex_unlock(&priv->state_lock);
> +	if (enabled)
> +		WARN_ON_ONCE(netdev_unbind_dma_dev(priv->netdev,
> +						   dd_dev->device));
> +}

[Severity: Medium]
mlx5e_data_direct_event() clears the flag, but this helper only releases the
providers. MLX5E_PFLAG_DATA_DIRECT stays set in priv->channels.params. Can
that leave a stale flag after resume?

On PCI PM suspend the path is:

mlx5_suspend()
  mlx5_unload_one(dev, true)
    mlx5e_suspend()
      mlx5e_detach_netdev()
        mlx5e_nic_disable()
          mlx5e_data_direct_unbind_dev()

While the device is suspended, mlx5_unload()->mlx5_data_direct_cleanup()
frees the registration, and the mlx5e notifier is unregistered.

The data direct PCI function could go away in that window. The commit
message says such an unbind "would go unnoticed". In that case
mlx5_data_direct_init() finds no matching device on resume, and dd.dev
stays NULL.

mlx5e_attach_netdev() and mlx5e_nic_enable() keep the old params.
mlx5e_nic_enable() registers the notifier again without checking the flag
against mlx5_data_direct_get_dev().

That has three effects:

- ethtool reports data_direct as on with no device behind it.
- mlx5e_queue_get_dma_dev()->mlx5e_get_dma_dev() returns NULL for every
  queue. Devmem and io_uring zcrx binds then fail instead of falling back
  to the PF DMA device.
- mlx5e_alloc_txqsq() keeps creating SQs with MLX5E_SQ_STATE_DATA_DIRECT
  and the crossing mkey.

The commit message says "To prevent further use disable the private flag",
but the flag is not cleared on this path. Should the suspend path clear it
too, or should mlx5e_nic_enable() check it again?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261110183826.2927274-1-tariqt%40nvidia.com