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

[PATCH net-next 00/10] 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 selected through a new data direct priv flag.

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.

During data_direct device unbind, a re-creation of the channels is
triggered.

A note about the unbind: it does not does not tear down dmabuf bindings
attached to the data direct device; the recreated channels will still
pick up the binding via rxq->mp_params and post DMA addresses from the
data direct IOMMU domain to the PF, causing IOMMU faults. A devmem
revoke hook is needed to close those bindings before the switch. This is
out of scope for this series.

A note about io_uring support: currently io_uring rejects the
configuration where the dmabuf dev != page_pool dev.

Regards,
Tariq

[1] https://lore.kernel.org/all/cover.1722512548.git.leon@kernel.org/

Dragos Tatulea (10):
  net/mlx5: Log the data direct to PF device mapping
  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: Add data direct TX infrastructure
  net/mlx5e: Use the correct DMA dev when data_direct pdev enabled
  net/mlx5e: Recreate netdev channels on data direct device unbind
  net: devmem: add netdev_has_dmabuf_binding() helper
  net/mlx5e: Enable the data direct netdev feature

 .../ethernet/mellanox/mlx5/core/data_direct.c |   5 +
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  13 ++
 .../ethernet/mellanox/mlx5/core/en/params.c   |   5 +
 .../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  |  40 ++++
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 173 +++++++++++++++++-
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  28 +--
 .../net/ethernet/mellanox/mlx5/core/en_tx.c   |   4 +-
 include/linux/mlx5/driver.h                   |   1 +
 include/net/netmem.h                          |  23 +++
 net/core/devmem.c                             |  30 +++
 net/core/devmem.h                             |   2 +
 15 files changed, 342 insertions(+), 24 deletions(-)


base-commit: 8df0638138d3e0344fd1fb36cf2d1ca1cf5028f0
-- 
2.44.0

[PATCH net-next 01/10] 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>
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 02/10] 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>
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 | 21 +++++++++++++++++++
 2 files changed, 22 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 f90b5a34e097..a2b3ac3011c9 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>
@@ -6023,6 +6024,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)
 {
@@ -6225,6 +6238,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);
@@ -6251,6 +6270,8 @@ static void mlx5e_nic_disable(struct mlx5e_priv *priv)
 {
 	struct mlx5_core_dev *mdev = priv->mdev;
 
+	mlx5_data_direct_unregister(mdev, &priv->data_direct_nb);
+
 	if (priv->netdev->reg_state == NETREG_REGISTERED)
 		mlx5e_dcbnl_delete_app(priv);
 
-- 
2.44.0

[PATCH net-next 03/10] 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 having additional ifs in the datapath, pre-calculate the UMR
padding and entry size based on the type of mkey used for UMR type used.

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>
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 a2b3ac3011c9..b86c7b52774a 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 e3f915beebe1..9e1224930e06 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -674,16 +674,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;
@@ -692,7 +687,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 04/10] 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>
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 261c466a4d36..b3eced1dd43f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -2400,6 +2400,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 },
@@ -2409,6 +2414,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 05/10] 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.

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>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |   6 ++
 .../ethernet/mellanox/mlx5/core/en/params.c   |   5 +
 .../ethernet/mellanox/mlx5/core/en/params.h   |   2 +
 .../mellanox/mlx5/core/en/reporter_rx.c       |   1 +
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 101 ++++++++++++++++--
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  12 ++-
 6 files changed, 118 insertions(+), 9 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..88b4cd5497d8 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));
@@ -1296,12 +1299,14 @@ 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)
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 b86c7b52774a..e0d46e067c72 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 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,11 @@ 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) &&
+		      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 9e1224930e06..c992da08caae 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -669,9 +669,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 06/10] 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.

Signed-off-by: Dragos Tatulea <dtatulea@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 e0d46e067c72..d5282a301621 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 07/10] net/mlx5e: Use the correct DMA dev when data_direct pdev enabled

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

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

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

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

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index d5282a301621..6476baf71bde 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5897,10 +5897,10 @@ static int mlx5e_queue_start(struct net_device *dev,
 	return 0;
 }
 
-static struct device *mlx5e_queue_get_dma_dev(struct net_device *dev,
+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 mlx5e_channel *ch;
@@ -5913,7 +5913,16 @@ static struct device *mlx5e_queue_get_dma_dev(struct net_device *dev,
 		goto out;
 
 	ch = channels->c[queue_index];
-	pdev = ch->pdev;
+	if (MLX5E_GET_PFLAG(&channels->params, MLX5E_PFLAG_DATA_DIRECT)) {
+		struct mlx5_core_dev *mdev = ch->mdev;
+		struct mlx5_data_direct_dev *dd_dev;
+
+		dd_dev = mlx5_data_direct_get_dev(mdev->data_direct);
+		if (dd_dev)
+			pdev = &dd_dev->pdev->dev;
+	} else {
+		pdev = ch->pdev;
+	}
 out:
 	mutex_unlock(&priv->state_lock);
 
-- 
2.44.0

[PATCH net-next 08/10] net/mlx5e: Recreate netdev channels on data direct device unbind

[Tariq Toukan]

From: Dragos Tatulea <dtatulea@nvidia.com>

To prevent queues from using data direct resources, disable the data
direct private flag and trigger channel re-creation for each netdev that
is registered to a data direct device in the unbind callback.

Channels will be re-created without the data direct flag.

Note that this does not tear down dmabuf bindings attached to the data
direct device; the recreated channels will still pick up the binding via
rxq->mp_params and post DMA addresses from the data direct IOMMU domain
to the PF, causing IOMMU faults. A devmem revoke hook is needed to close
those bindings before the switch.

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

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 6476baf71bde..fb3e0bd00f04 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -6144,9 +6144,28 @@ static int mlx5e_data_direct_event(struct notifier_block *nb,
 {
 	struct mlx5e_priv *priv = container_of(nb, struct mlx5e_priv,
 					       data_direct_nb);
+	struct mlx5e_params new_params;
+
+	if (action != MLX5_DATA_DIRECT_UNBIND)
+		return NOTIFY_DONE;
+
+	rtnl_lock();
+	netdev_lock(priv->netdev);
+	mutex_lock(&priv->state_lock);
 
-	if (action == MLX5_DATA_DIRECT_UNBIND)
-		mlx5_core_dbg(priv->mdev, "data direct device unbound\n");
+	if (!MLX5E_GET_PFLAG(&priv->channels.params, MLX5E_PFLAG_DATA_DIRECT))
+		goto out;
+
+	new_params = priv->channels.params;
+	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, false);
+	mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
+	netdev_info(priv->netdev,
+		    "data_direct device unbound, disabled data_direct\n");
+
+out:
+	mutex_unlock(&priv->state_lock);
+	netdev_unlock(priv->netdev);
+	rtnl_unlock();
 
 	return NOTIFY_OK;
 }
-- 
2.44.0

[PATCH net-next 09/10] 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 its xarray slot: 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. For this reason,
track bindings for their whole lifetime on a new list.

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 | 23 +++++++++++++++++++++++
 net/core/devmem.c    | 30 ++++++++++++++++++++++++++++++
 net/core/devmem.h    |  2 ++
 3 files changed, 55 insertions(+)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index 0cc8572f62ca..8bcf44f43d17 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -387,16 +387,39 @@ 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 until its last reference is dropped, 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 @dma_dev.
+ *
+ * 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 __get_netmem(netmem_ref netmem);
diff --git a/net/core/devmem.c b/net/core/devmem.c
index 07c62e2e3088..173d66d1cb6e 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -25,6 +25,9 @@
 
 static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1);
 
+static DEFINE_SPINLOCK(net_devmem_live_bindings_lock);
+static LIST_HEAD(net_devmem_live_bindings);
+
 static const struct memory_provider_ops dmabuf_devmem_ops;
 
 static struct net_iov *net_iov_free_pop(struct net_iov_area *area,
@@ -63,6 +66,10 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 {
 	struct net_devmem_dmabuf_binding *binding = container_of(wq, typeof(*binding), unbind_w);
 
+	spin_lock(&net_devmem_live_bindings_lock);
+	list_del(&binding->live_list);
+	spin_unlock(&net_devmem_live_bindings_lock);
+
 	if (binding->freelist)
 		WARN(binding->free_count != binding->area.num_niovs,
 		     "destroying dmabuf binding with outstanding net_iovs: total=%zu, free=%u",
@@ -321,6 +328,10 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 
 	list_add(&binding->list, &priv->bindings);
 
+	spin_lock(&net_devmem_live_bindings_lock);
+	list_add(&binding->live_list, &net_devmem_live_bindings);
+	spin_unlock(&net_devmem_live_bindings_lock);
+
 	return binding;
 
 err_free_niovs:
@@ -357,6 +368,25 @@ struct net_devmem_dmabuf_binding *net_devmem_lookup_dmabuf(u32 id)
 	return binding;
 }
 
+bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev)
+{
+	struct net_devmem_dmabuf_binding *binding;
+	bool found = false;
+
+	spin_lock(&net_devmem_live_bindings_lock);
+	list_for_each_entry(binding, &net_devmem_live_bindings, live_list) {
+		if (dev == READ_ONCE(binding->dev) &&
+		    (!dma_dev || binding->attachment->dev == dma_dev)) {
+			found = true;
+			break;
+		}
+	}
+	spin_unlock(&net_devmem_live_bindings_lock);
+
+	return found;
+}
+EXPORT_SYMBOL_GPL(netdev_has_dmabuf_binding);
+
 void net_devmem_get_net_iov(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 1cec10f0257c..eadf6df38687 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -54,6 +54,8 @@ struct net_devmem_dmabuf_binding {
 	 */
 	struct list_head list;
 
+	struct list_head live_list;
+
 	/* rxq's this binding is active on. */
 	struct xarray bound_rxqs;
 
-- 
2.44.0

[PATCH net-next 10/10] 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 dmabufs are still bound
to the netdev.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  | 36 ++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index b3eced1dd43f..8ab1150ce3f2 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"
@@ -2402,7 +2403,40 @@ 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;
+
+	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_data_direct_get_dev(mdev->data_direct)) {
+			netdev_err(netdev, "data_direct device not bound\n");
+			return -ENODEV;
+		}
+	}
+
+	/* Changing the flag reopens the channels with a different DMA device,
+	 * which would break any dmabuf currently bound to this netdev.
+	 */
+	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] = {
-- 
2.44.0

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

[Mina Almasry]

On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
>
> 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 selected through a new data direct priv flag.
>
> 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.
>
> During data_direct device unbind, a re-creation of the channels is
> triggered.
>
> A note about the unbind: it does not does not tear down dmabuf bindings
> attached to the data direct device; the recreated channels will still
> pick up the binding via rxq->mp_params and post DMA addresses from the
> data direct IOMMU domain to the PF, causing IOMMU faults. A devmem
> revoke hook is needed to close those bindings before the switch. This is
> out of scope for this series.
>

Elaborate on this please. Are you referring to the fact that on
page_pool_scrub we unmap the netmems dma-mapped by the page-pool, but
we don't unmap the netmems dma-mapped by the memory provider?

What's the impact of leaving this unfixed? Is the machine going to
crash if the device goes away but we don't unmap the dmabuf?
-- 
Thanks,
Mina

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

[Mina Almasry]

On Fri, Oct 9, 2026 at 9:42 AM Mina Almasry <almasrymina@google.com> wrote:
>
> On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
> >
> > 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 selected through a new data direct priv flag.
> >
> > 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.
> >
> > During data_direct device unbind, a re-creation of the channels is
> > triggered.
> >
> > A note about the unbind: it does not does not tear down dmabuf bindings
> > attached to the data direct device; the recreated channels will still
> > pick up the binding via rxq->mp_params and post DMA addresses from the
> > data direct IOMMU domain to the PF, causing IOMMU faults. A devmem
> > revoke hook is needed to close those bindings before the switch. This is
> > out of scope for this series.
> >
>
> Elaborate on this please. Are you referring to the fact that on
> page_pool_scrub we unmap the netmems dma-mapped by the page-pool, but
> we don't unmap the netmems dma-mapped by the memory provider?
>
> What's the impact of leaving this unfixed? Is the machine going to
> crash if the device goes away but we don't unmap the dmabuf?

Oh, you're referring to an even different bug than the one I had in
mind. (I'll see if I can submit a fix for the bug I had in mind).

But in general I think not addressing the issue you're referring to is
way too messy. I think we do indeed need to unbind the dma-buf if the
dma-dev is going away. The invariant should be that the
binding->attachment->dev should not change at all during the entire
lifetime of the binding. We already 'revoke' the dma-buf binding if
the netdev is going away, sorta (there is a bug there I need to fix.
The uninstall function in the provider doesn't actually unmap the
dma-buf :sad face:).

I've worked with my LLM to suggest some changes that (we think) make
this work, but you may disagree. I'll post the suggestions. But please
I think this should be handled one way or another.

-- 
Thanks,
Mina

Re: [PATCH net-next 08/10] net/mlx5e: Recreate netdev channels on data direct device unbind

[Mina Almasry]

On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
> From: Dragos Tatulea <dtatulea@nvidia.com>
>
> Note that this does not tear down dmabuf bindings attached to the data
> direct device; the recreated channels will still pick up the binding via
> rxq->mp_params and post DMA addresses from the data direct IOMMU domain
> to the PF, causing IOMMU faults. A devmem revoke hook is needed to close
> those bindings before the switch.

(Note: LLM-assisted review comment below.)

Rather than shipping this known IOMMU fault window on MLX5_DATA_DIRECT_UNBIND,
could we add the core detach helper in patch 09/10 and invoke it here under
netdev_lock(priv->netdev) before mlx5e_safe_switch_params()?

Specifically, reopening channels on the PF without detaching active bindings
leaves three issues:
1. RX: reopened queues pick up stale rxq->mp_params and post data_direct IOVAs
   onto ch->pdev.
2. TX: active TX bindings (net_devmem_dmabuf_bindings) and in-flight or
   retransmitted skbs still hold data_direct IOVAs while mlx5e_xmit() switches
   to sq->mkey_be (ch->pdev).
3. DMA unmap ordering: binding->sgt / binding->attachment stay mapped to
   &dd_dev->pdev->dev until userspace closes all sockets, after
   mlx5_data_direct_remove() has already returned.

If mlx5e_data_direct_event() calls a core helper such as
netdev_unbind_dmabuf_dma_dev(priv->netdev, &dd_dev->pdev->dev) before
mlx5e_safe_switch_params():
- netif_mp_close_rxq() clears rxq->mp_params so reopened channels use host
  page_pool pages on ch->pdev.
- Clearing WRITE_ONCE(binding->dev, NULL) + synchronize_net() causes existing
  validate_xmit_unreadable_skb() (READ_ONCE(binding->dev) != dev) to drop any
  in-flight/retransmitted TX skbs automatically with zero driver fast-path
  changes.
- dma_buf_unmap_attachment_unlocked() + dma_buf_detach() complete synchronously
  before mlx5_data_direct_remove() finishes.

-- 
Thanks,
Mina

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

[Mina Almasry]

On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
> From: Dragos Tatulea <dtatulea@nvidia.com>
>
> A binding can outlive its xarray slot: 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. For this reason,
> track bindings for their whole lifetime on a new list.

(Note: LLM-assisted review comment below.)

Two suggestions on the core design here:

1. Add a synchronous DMA detach helper (e.g. netdev_unbind_dmabuf_dma_dev(dev,
   dma_dev)) alongside netdev_has_dmabuf_binding() so patch 08/10 can cleanly
   tear down active bindings on MLX5_DATA_DIRECT_UNBIND:
   - Separate DMA mapping lifetime (ends synchronously when queues stop and
     dev/vdev/dma_dev goes away) from CPU net_iov lifetime (ends in
     __net_devmem_dmabuf_binding_free() when binding->ref hits 0).
   - For bindings matching (dev, binding->attachment->dev == dma_dev), close
     bound RX queues (netif_mp_close_rxq()), erase binding->id from
     net_devmem_dmabuf_bindings, clear WRITE_ONCE(binding->dev, NULL) /
     WRITE_ONCE(binding->vdev, NULL), call synchronize_net(), and unmap/detach
     binding->sgt and binding->attachment synchronously (setting them to NULL
     so __net_devmem_dmabuf_binding_free() skips them later).
   - Note: core devmem also has a pre-existing bug on physical/virtual
     NETDEV_UNREGISTER where mp_dmabuf_devmem_uninstall() and TX bindings leave
     dma_buf mapped past pci_driver->remove() and TX bindings leave
     binding->dev / binding->vdev dangling; sharing a single core detach
     helper solves both cleanly with zero new fields in
     struct net_devmem_dmabuf_binding.

2. Once detach (and net_devmem_unbind_dmabuf() on TX unbind) clears
   WRITE_ONCE(binding->dev, NULL), runs synchronize_net(), and unmaps
   binding->sgt synchronously, any binding already erased from
   net_devmem_dmabuf_bindings is already DMA-unmapped and rejected by
   validate_xmit_unreadable_skb() (READ_ONCE(binding->dev) != dev). That
   eliminates the need for net_devmem_live_bindings_lock / live_list —
   iterating net_devmem_dmabuf_bindings under netdev_lock(dev) suffices for
   both netdev_has_dmabuf_binding() and netdev_unbind_dmabuf_dma_dev().

-- 
Thanks,
Mina

Re: [PATCH net-next 10/10] net/mlx5e: Enable the data direct netdev feature

[Mina Almasry]

On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
> From: Dragos Tatulea <dtatulea@nvidia.com>
>
> +	/* Changing the flag reopens the channels with a different DMA device,
> +	 * which would break any dmabuf currently bound to this netdev.
> +	 */
> +	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);

(Note: LLM-assisted review comment below.)

set_pflag_data_direct() is called under rtnl_lock() only, whereas
netdev_nl_bind_rx_doit() and netdev_nl_bind_tx_doit() serialize bindings under
netdev_lock(netdev) without holding rtnl_lock().

Should set_pflag_data_direct() hold netdev_lock(netdev) across both
netdev_has_dmabuf_binding() and mlx5e_safe_switch_params() (just as
mlx5e_data_direct_event() does in patch 08/10) so a concurrent bind-rx/bind-tx
cannot race past the -EBUSY check while channels are switching DMA devices?

-- 
Thanks,
Mina

Re: [PATCH net-next 08/10] net/mlx5e: Recreate netdev channels on data direct device unbind

[Dragos Tatulea]

On 10.10.26 01:56, Mina Almasry wrote:
> On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
>> From: Dragos Tatulea <dtatulea@nvidia.com>
>>
>> Note that this does not tear down dmabuf bindings attached to the data
>> direct device; the recreated channels will still pick up the binding via
>> rxq->mp_params and post DMA addresses from the data direct IOMMU domain
>> to the PF, causing IOMMU faults. A devmem revoke hook is needed to close
>> those bindings before the switch.
> 
> (Note: LLM-assisted review comment below.)
> 
> Rather than shipping this known IOMMU fault window on MLX5_DATA_DIRECT_UNBIND,
> could we add the core detach helper in patch 09/10 and invoke it here under
> netdev_lock(priv->netdev) before mlx5e_safe_switch_params()?
> 
> Specifically, reopening channels on the PF without detaching active bindings
> leaves three issues:
> 1. RX: reopened queues pick up stale rxq->mp_params and post data_direct IOVAs
>    onto ch->pdev.
> 2. TX: active TX bindings (net_devmem_dmabuf_bindings) and in-flight or
>    retransmitted skbs still hold data_direct IOVAs while mlx5e_xmit() switches
>    to sq->mkey_be (ch->pdev).
> 3. DMA unmap ordering: binding->sgt / binding->attachment stay mapped to
>    &dd_dev->pdev->dev until userspace closes all sockets, after
>    mlx5_data_direct_remove() has already returned.
> 
> If mlx5e_data_direct_event() calls a core helper such as
> netdev_unbind_dmabuf_dma_dev(priv->netdev, &dd_dev->pdev->dev) before
> mlx5e_safe_switch_params():
> - netif_mp_close_rxq() clears rxq->mp_params so reopened channels use host
>   page_pool pages on ch->pdev.
> - Clearing WRITE_ONCE(binding->dev, NULL) + synchronize_net() causes existing
>   validate_xmit_unreadable_skb() (READ_ONCE(binding->dev) != dev) to drop any
>   in-flight/retransmitted TX skbs automatically with zero driver fast-path
>   changes.
> - dma_buf_unmap_attachment_unlocked() + dma_buf_detach() complete synchronously
>   before mlx5_data_direct_remove() finishes.
> 
My plan was to address this after this series is accepted.

Thanks,
Dragos

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

[Dragos Tatulea]

On 10.10.26 01:54, Mina Almasry wrote:
> On Fri, Oct 9, 2026 at 9:42 AM Mina Almasry <almasrymina@google.com> wrote:
>>
>> On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
>>>
>>> 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 selected through a new data direct priv flag.
>>>
>>> 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.
>>>
>>> During data_direct device unbind, a re-creation of the channels is
>>> triggered.
>>>
>>> A note about the unbind: it does not does not tear down dmabuf bindings
>>> attached to the data direct device; the recreated channels will still
>>> pick up the binding via rxq->mp_params and post DMA addresses from the
>>> data direct IOMMU domain to the PF, causing IOMMU faults. A devmem
>>> revoke hook is needed to close those bindings before the switch. This is
>>> out of scope for this series.
>>>
>>
>> Elaborate on this please. Are you referring to the fact that on
>> page_pool_scrub we unmap the netmems dma-mapped by the page-pool, but
>> we don't unmap the netmems dma-mapped by the memory provider?
>>
>> What's the impact of leaving this unfixed? Is the machine going to
>> crash if the device goes away but we don't unmap the dmabuf?
> 
IOMMU faults because the dma device is invalid. Either because the
DD dev is still in use and was unbound. Or because the non-DD
one is used and is invalid (due to channel reopen).

> Oh, you're referring to an even different bug than the one I had in
> mind. (I'll see if I can submit a fix for the bug I had in mind).
> 
> But in general I think not addressing the issue you're referring to is
> way too messy. I think we do indeed need to unbind the dma-buf if the
> dma-dev is going away. The invariant should be that the
> binding->attachment->dev should not change at all during the entire
> lifetime of the binding. We already 'revoke' the dma-buf binding if
> the netdev is going away, sorta (there is a bug there I need to fix.
> The uninstall function in the provider doesn't actually unmap the
> dma-buf :sad face:).
> 
> I've worked with my LLM to suggest some changes that (we think) make
> this work, but you may disagree. I'll post the suggestions. But please
> I think this should be handled one way or another.
> 
My idea was to handle the unbind part in a subsequent series because
it probably needs some more back and forth to get this cleanup path
right.

If not acceptable I'll work on adding it to this series.

Thanks,
Dragos

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

[Tariq Toukan]

On 10/10/2026 2:54, Mina Almasry wrote:
> On Fri, Oct 9, 2026 at 9:42 AM Mina Almasry <almasrymina@google.com> wrote:
>>
>> On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
>>>
>>> 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 selected through a new data direct priv flag.
>>>
>>> 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.
>>>
>>> During data_direct device unbind, a re-creation of the channels is
>>> triggered.
>>>
>>> A note about the unbind: it does not does not tear down dmabuf bindings
>>> attached to the data direct device; the recreated channels will still
>>> pick up the binding via rxq->mp_params and post DMA addresses from the
>>> data direct IOMMU domain to the PF, causing IOMMU faults. A devmem
>>> revoke hook is needed to close those bindings before the switch. This is
>>> out of scope for this series.
>>>
>>
>> Elaborate on this please. Are you referring to the fact that on
>> page_pool_scrub we unmap the netmems dma-mapped by the page-pool, but
>> we don't unmap the netmems dma-mapped by the memory provider?
>>
>> What's the impact of leaving this unfixed? Is the machine going to
>> crash if the device goes away but we don't unmap the dmabuf?
> 
> Oh, you're referring to an even different bug than the one I had in
> mind. (I'll see if I can submit a fix for the bug I had in mind).
> 
> But in general I think not addressing the issue you're referring to is
> way too messy. I think we do indeed need to unbind the dma-buf if the
> dma-dev is going away. The invariant should be that the
> binding->attachment->dev should not change at all during the entire
> lifetime of the binding. We already 'revoke' the dma-buf binding if
> the netdev is going away, sorta (there is a bug there I need to fix.
> The uninstall function in the provider doesn't actually unmap the
> dma-buf :sad face:).
> 
> I've worked with my LLM to suggest some changes that (we think) make
> this work, but you may disagree. I'll post the suggestions. But please
> I think this should be handled one way or another.
> 

Mina, thanks for your review.

pw-bot: cr

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

[Mina Almasry]

On Sat, Oct 10, 2026 at 7:16 AM Dragos Tatulea <dtatulea@nvidia.com> wrote:
>
>
>
> On 10.10.26 01:54, Mina Almasry wrote:
> > On Fri, Oct 9, 2026 at 9:42 AM Mina Almasry <almasrymina@google.com> wrote:
> >>
> >> On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
> >>>
> >>> 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 selected through a new data direct priv flag.
> >>>
> >>> 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.
> >>>
> >>> During data_direct device unbind, a re-creation of the channels is
> >>> triggered.
> >>>
> >>> A note about the unbind: it does not does not tear down dmabuf bindings
> >>> attached to the data direct device; the recreated channels will still
> >>> pick up the binding via rxq->mp_params and post DMA addresses from the
> >>> data direct IOMMU domain to the PF, causing IOMMU faults. A devmem
> >>> revoke hook is needed to close those bindings before the switch. This is
> >>> out of scope for this series.
> >>>
> >>
> >> Elaborate on this please. Are you referring to the fact that on
> >> page_pool_scrub we unmap the netmems dma-mapped by the page-pool, but
> >> we don't unmap the netmems dma-mapped by the memory provider?
> >>
> >> What's the impact of leaving this unfixed? Is the machine going to
> >> crash if the device goes away but we don't unmap the dmabuf?
> >
> IOMMU faults because the dma device is invalid. Either because the
> DD dev is still in use and was unbound. Or because the non-DD
> one is used and is invalid (due to channel reopen).
>
> > Oh, you're referring to an even different bug than the one I had in
> > mind. (I'll see if I can submit a fix for the bug I had in mind).
> >
> > But in general I think not addressing the issue you're referring to is
> > way too messy. I think we do indeed need to unbind the dma-buf if the
> > dma-dev is going away. The invariant should be that the
> > binding->attachment->dev should not change at all during the entire
> > lifetime of the binding. We already 'revoke' the dma-buf binding if
> > the netdev is going away, sorta (there is a bug there I need to fix.
> > The uninstall function in the provider doesn't actually unmap the
> > dma-buf :sad face:).
> >
> > I've worked with my LLM to suggest some changes that (we think) make
> > this work, but you may disagree. I'll post the suggestions. But please
> > I think this should be handled one way or another.
> >
> My idea was to handle the unbind part in a subsequent series because
> it probably needs some more back and forth to get this cleanup path
> right.
>
> If not acceptable I'll work on adding it to this series.
>

Since these are mostly driver-local changes I don't want to be so
forceful to block it by saying it's unacceptable, but if at all
possible lets try not to exacerbate the current mess. It looks like we
already a bunch of lifetime bugs with regards to the device going
away. I've posted this series, which - I think - gives you the devmem
'revoke' mechanism that you want:

https://lore.kernel.org/netdev/20261010025532.839559-1-almasrymina@google.com/

Also working on that, I realized we don't use binding->vdev and
binding->dev correctly on binding rx. binding->vdev is meant to be the
device we do the bind to, and binding->dev is meant to be the
underlying dma_device (if they're different). That fits into your
model where you have the netdev you want to bind to, but have an
underlying device that you want to dma-map to.

I've gotten some feedback on my series and I'm working on v2. If it
makes sense for you to wait on my devmem core changes so you can
integrate into them better and avoid these lifetime bugs, lets do
that.

-- 
Thanks,
Mina

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

[Dragos Tatulea]

On 11.10.26 18:25, Mina Almasry wrote:
> On Sat, Oct 10, 2026 at 7:16 AM Dragos Tatulea <dtatulea@nvidia.com> wrote:
>>
>>
>>
>> On 10.10.26 01:54, Mina Almasry wrote:
>>> On Fri, Oct 9, 2026 at 9:42 AM Mina Almasry <almasrymina@google.com> wrote:
>>>>
>>>> On Thu, Oct 8, 2026 at 6:28 AM Tariq Toukan <tariqt@nvidia.com> wrote:
>>>>>
>>>>> 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 selected through a new data direct priv flag.
>>>>>
>>>>> 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.
>>>>>
>>>>> During data_direct device unbind, a re-creation of the channels is
>>>>> triggered.
>>>>>
>>>>> A note about the unbind: it does not does not tear down dmabuf bindings
>>>>> attached to the data direct device; the recreated channels will still
>>>>> pick up the binding via rxq->mp_params and post DMA addresses from the
>>>>> data direct IOMMU domain to the PF, causing IOMMU faults. A devmem
>>>>> revoke hook is needed to close those bindings before the switch. This is
>>>>> out of scope for this series.
>>>>>
>>>>
>>>> Elaborate on this please. Are you referring to the fact that on
>>>> page_pool_scrub we unmap the netmems dma-mapped by the page-pool, but
>>>> we don't unmap the netmems dma-mapped by the memory provider?
>>>>
>>>> What's the impact of leaving this unfixed? Is the machine going to
>>>> crash if the device goes away but we don't unmap the dmabuf?
>>>
>> IOMMU faults because the dma device is invalid. Either because the
>> DD dev is still in use and was unbound. Or because the non-DD
>> one is used and is invalid (due to channel reopen).
>>
>>> Oh, you're referring to an even different bug than the one I had in
>>> mind. (I'll see if I can submit a fix for the bug I had in mind).
>>>
>>> But in general I think not addressing the issue you're referring to is
>>> way too messy. I think we do indeed need to unbind the dma-buf if the
>>> dma-dev is going away. The invariant should be that the
>>> binding->attachment->dev should not change at all during the entire
>>> lifetime of the binding. We already 'revoke' the dma-buf binding if
>>> the netdev is going away, sorta (there is a bug there I need to fix.
>>> The uninstall function in the provider doesn't actually unmap the
>>> dma-buf :sad face:).
>>>
>>> I've worked with my LLM to suggest some changes that (we think) make
>>> this work, but you may disagree. I'll post the suggestions. But please
>>> I think this should be handled one way or another.
>>>
>> My idea was to handle the unbind part in a subsequent series because
>> it probably needs some more back and forth to get this cleanup path
>> right.
>>
>> If not acceptable I'll work on adding it to this series.
>>
> 
> Since these are mostly driver-local changes I don't want to be so
> forceful to block it by saying it's unacceptable, but if at all
> possible lets try not to exacerbate the current mess. It looks like we
> already a bunch of lifetime bugs with regards to the device going
> away. I've posted this series, which - I think - gives you the devmem
> 'revoke' mechanism that you want:
> 
> https://lore.kernel.org/netdev/20261010025532.839559-1-almasrymina@google.com/
> 
> Also working on that, I realized we don't use binding->vdev and
> binding->dev correctly on binding rx. binding->vdev is meant to be the
> device we do the bind to, and binding->dev is meant to be the
> underlying dma_device (if they're different). That fits into your
> model where you have the netdev you want to bind to, but have an
> underlying device that you want to dma-map to.
> 
> I've gotten some feedback on my series and I'm working on v2. If it
> makes sense for you to wait on my devmem core changes so you can
> integrate into them better and avoid these lifetime bugs, lets do
> that.
> 

Thanks for your patches Mina! This made it easier to implement the
unbind. I will add a dependency in the series to your patches. That's
if they don't get merged before.

Thanks,
Dragos

Re: [PATCH net-next 05/10] 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: 1 · 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).

- [High] In data-direct mode the ICOSQ is too small for the UMR WQEs
  posted to it, so the ring overruns.
- [Medium] In data-direct mode, mlx5e_create_rq_umr_mkey() now creates
  the KSM UMR mkey in mdev->data_direct->pdn.
- [Medium] The data-direct RX path dereferences the per-channel
  mdev->data_direct without a NULL check: ->pdn in…
- [Low] The commit message describes the mkey chain incorrectly.
- [Low] Data-direct RQs now always use the UNALIGNED (KSM) UMR mode, but
  nothing checks that mode's requirements before the channel is opened.

--

net/mlx5e: Add data direct RX infrastructure

This commit adds the RX-side infrastructure for data direct mode. It adds
an RQ state flag and creates a crossing mkey that the RQ WQEs use as their
lkey. It also switches data direct RQs to a KSM UMR mkey in the data
direct PD, whose entries point to the data direct mkey.

> 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.

[ ... ]

> - Create KSM mkey and in data direct PD and make it point to the
>   crossed mkey.

[Severity: Low]
This isn't a bug, but does this description match the code?

In mlx5e_rq_alloc_data_direct_mkey(), the KSM UMR mkey itself is passed
as the crossed mkey:

	umr_mkey = be32_to_cpu(rq->mpwqe.umr_mkey_be);
	...
	return mlx5e_create_crossing_vhca_mkey(mdev, umr_mkey,

The KSM entries written in mlx5e_alloc_rx_mpwqe() point to
mdev->data_direct->mkey or ->mkey_ro (ksm_child_mkey_be), not to the
crossed mkey.

So the chain in the code looks like:

  RQ lkey -> crossing mkey (hw_objs.pdn)
          -> crossed KSM UMR mkey (data_direct->pdn)
          -> data direct mkey

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
> index 5caf7820a136b..88b4cd5497d88 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: Low]
Data direct RQs now always use the UNALIGNED (KSM) mode. Is this mode
validated anywhere before the channel is opened?

mlx5e_mpwrq_validate_regular() still checks only the ALIGNED mode:

	enum mlx5e_mpwrq_umr_mode umr_mode = mlx5e_mpwrq_umr_mode(mdev, NULL);

set_pflag_data_direct(), added later in the series, doesn't check
fixed_buffer_size either. The first check happens during
mlx5e_alloc_rq(), in mlx5e_create_umr_mkey():

	mlx5_core_warn(mdev, "Unaligned AF_XDP requires fixed_buffer_size capability\n");
	return -EINVAL;

On a device without fixed_buffer_size, a data direct RQ with no AF_XDP in
use would fail channel open with this AF_XDP-specific message.

The KSM limit derived from log_max_klm_list_size isn't checked up front
either. mlx5e_create_rq_umr_mkey() only warns when num_entries is larger
than max_num_entries, and then continues.

[ ... ]

> @@ -1296,12 +1299,14 @@ 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)

[Severity: High]
The RQ parameters are now built with rq_opt.data_direct set, so the RQ
geometry is computed for KSM. Is the ICOSQ still sized for the MTT layout?

The next call, mlx5e_build_icosq_log_wq_sz(), does:

	/* UMR WQEs for the regular RQ. */
	wqebbs = mlx5e_mpwrq_total_umr_wqebbs(mdev, params, NULL);

With rqo == NULL, mlx5e_mpwrq_umr_mode() returns the ALIGNED mode, so the
budget assumes 8-byte MTT entries. KSM entries are 16 bytes. That halves
the maximum pages per WQE in mlx5e_mpwrq_log_wqe_sz(), which doubles the
RQ WQE count from mlx5e_mpwqe_get_log_rq_size(). Where the log WQE size
is capped at 17 (ECPF), the per-UMR WQEBB count grows instead.

With defaults (MTU 1500, log_rq_mtu_frames=10, 64-byte cachelines), the
MTT budget is 8 WQEs * 10 WQEBBs = 80. That rounds up to a 128-WQEBB
ICOSQ. The KSM RQ has 16 WQEs, so the first fill does:

mlx5e_post_rx_mpwqes()
  mlx5e_alloc_rx_mpwqe()   /* 15 times, 10 WQEBBs each */

That posts about 150 WQEBBs plus NOP padding in one batch before the
doorbell. MTU 9000 has the same problem: about 1270 WQEBBs are needed
for a 1024-WQEBB ICOSQ.

Neither mlx5e_post_rx_mpwqes() nor mlx5e_icosq_get_next_pi() checks for
free space. Can the producer then wrap onto UMR WQEs and sq->db.wqe_info[]
entries that the hardware hasn't completed yet?

This still seems to be the case at the end of the series. There,
mlx5e_build_icosq_log_wq_sz() still passes NULL, and the commit
"net/mlx5e: Enable the data direct netdev feature" makes this path
reachable.

[ ... ]

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index b86c7b52774a0..e0d46e067c720 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c

[ ... ]

> @@ -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]
The KSM UMR mkey is now created in mdev->data_direct->pdn. However,
mlx5e_create_umr_mkey() still fills every UNALIGNED entry with a child
key from the regular PD:

	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),
			};

mlx5e_alloc_rx_mpwqe() rewrites only the first pages_per_wqe entries of
each WQE with ksm_child_mkey_be.

mlx5e_mpwrq_mtts_per_wqe() reserves extra entries per WQE
(ALIGN(pages_per_wqe + 1, 4)) to absorb hardware write overflow. These
are never rewritten, so they keep hw_objs.mkey.

That seems to be the reason for moving the UMR mkey into the data direct
PD in the first place. If the hardware requires KSM child mkeys to be in
the same PD as the parent, would an oversized frame that reaches these
overflow entries cause a protection error? That would mean an RQ error
CQE instead of a harmless write to the filler page.

[ ... ]

> @@ -2873,7 +2958,11 @@ 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) &&
> +		      netif_rxq_has_unreadable_mp(netdev, ix);
> +
> +	err = mlx5e_build_channel_param(mdev, params, qcfg, data_direct,
> +					cparam);

[Severity: Medium]
data_direct is decided here from the netdev-wide pflag and
netif_rxq_has_unreadable_mp(). However, mdev comes from
mlx5_sd_ch_ix_get_dev(priv->mdev, ix), which can be a Socket-Direct
secondary PF. Can mdev->data_direct be NULL for this channel?

The RQ allocation path dereferences it without a check:

mlx5e_alloc_rq()
  mlx5e_create_rq_umr_mkey()
    pdn = dd ? mdev->data_direct->pdn : ...
  mlx5e_rq_alloc_data_direct_mkey()
    mdev->data_direct->mkey_ro_valid ? ...

mlx5_data_direct_init() leaves mdev->data_direct NULL when
mlx5_data_direct_supported() is false. set_pflag_data_direct(), added
later in the series, only checks priv->mdev.

mlx5e_queue_get_dma_dev() already handles a NULL ch->mdev->data_direct
through mlx5_data_direct_get_dev(), so that case seems to be expected.

Suppose the primary PF supports data direct and a secondary PF does not.
Would binding devmem to a queue owned by the secondary oops in
mlx5e_create_rq_umr_mkey()?

This can't be reached in this commit. It becomes reachable with the
commit "net/mlx5e: Enable the data direct netdev feature".

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

Re: [PATCH net-next 06/10] net/mlx5e: Add data direct TX infrastructure

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · 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).

- [High] net/mlx5e: GPU unbind causes DMA corruption for active devmem TX

--

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
> @@ -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);

[Severity: High]
When MLX5E_SQ_STATE_DATA_DIRECT is clear, this silently falls back to
sq->mkey_be for net_iov frags too. That only works if a net_iov can
never reach an SQ whose data direct state differs from the device the
net_iov was DMA mapped against. Nothing on the TX side enforces that.

The follow-up "net/mlx5e: Recreate netdev channels on data direct
device unbind" makes this reachable. On unbind,
mlx5e_data_direct_event() clears MLX5E_PFLAG_DATA_DIRECT and recreates
the channels. It does not do the netdev_has_dmabuf_binding() check
that the ethtool path does, so a devmem TX binding made while data
direct was on stays alive.

That binding was mapped against the device returned by
netdev_queue_get_dma_dev(), so its net_iov DMA addresses are IOVAs in
that device's domain. After the channels are recreated, dd is false in
mlx5e_txwqe_build_dsegs(). The same IOVAs then get posted with the
local mkey, and the NIC reads whatever they happen to resolve to in
its own domain. The result is silent corruption of transmitted data or
IOMMU faults, rather than a clean error to the sender.

How is a still-bound TX dmabuf supposed to survive the unbind? Should
the binding be invalidated before the queues are restarted? Or should
net_iov frags be dropped here when dd is false, instead of being sent
with sq->mkey_be?

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

Re: [PATCH net-next 07/10] net/mlx5e: Use the correct DMA dev when data_direct pdev enabled

[netdev-bot+sashiko]

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

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

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] This patch makes mlx5e_queue_get_dma_dev() return
  &dd_dev->pdev->dev, the device of a separate, independently removable…
- [Medium] With MLX5E_PFLAG_DATA_DIRECT set, mlx5e_queue_get_dma_dev()
  (drivers/net/ethernet/mellanox/mlx5/core/en_main.c:5916-5922) returns…
- [Medium] At this commit, mlx5e_queue_get_dma_dev() loads dd_dev =
  READ_ONCE(mdev->data_direct->dev) under priv->state_lock and…
- [Low] The commit message says 'Rename netdev argument to netdev', but
  the diff renames the parameter of mlx5e_queue_get_dma_dev() from 'dev'…

--

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

When MLX5E_PFLAG_DATA_DIRECT is set, this patch makes
mlx5e_queue_get_dma_dev() return the data direct PCI device instead of
the NIC PF DMA device. It also renames the net_device parameter of the
function.

> When the private flag is set, always return the data direct
> PCI device instead of the NIC PF PCI device in .ndo_queue_get_dma_dev.
>
> Rename netdev argument to netdev because there are too many confusing
> "dev"-based names in this function.

[Severity: Low]
This isn't a bug, but should this read "Rename dev argument to netdev"?
The diff renames the parameter from dev to netdev.

Is "always" accurate? If mlx5_data_direct_get_dev() returns NULL (no
bound dd device, or mdev->data_direct == NULL), the function returns NULL.
It does not fall back to the PF device.

At this commit, set_pflag_data_direct() in en_ethtool.c still returns
-EOPNOTSUPP, so the flag can't be set yet. Earlier patches in the series
say when a change has no runtime effect. Could this message say so too?

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index d5282a3016218..6476baf71bdee 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c

[ ... ]

> @@ -5913,7 +5913,16 @@ static struct device *mlx5e_queue_get_dma_dev(struct net_device *dev,
>  		goto out;
>  
>  	ch = channels->c[queue_index];
> -	pdev = ch->pdev;
> +	if (MLX5E_GET_PFLAG(&channels->params, MLX5E_PFLAG_DATA_DIRECT)) {
> +		struct mlx5_core_dev *mdev = ch->mdev;
> +		struct mlx5_data_direct_dev *dd_dev;
> +
> +		dd_dev = mlx5_data_direct_get_dev(mdev->data_direct);

[Severity: Medium]
What keeps dd_dev alive between this load and the dd_dev->pdev
dereference below? The reader holds priv->state_lock. At this commit the
unbind path does not take that lock:

mlx5_data_direct_remove()
  mlx5_data_direct_dev_unreg()
    mlx5_data_direct_do_unbind()
      clears the dd pointer, calls the UNBIND notifier
        mlx5e_data_direct_event()   <- only logs
  kfree(dev)

The pflag can't be set at this commit, so this path isn't reachable yet.
The next patch in the series, "net/mlx5e: Recreate netdev channels on data
direct device unbind", makes mlx5e_data_direct_event() take rtnl_lock,
netdev_lock and priv->state_lock before kfree(dev) runs. That closes the
race for priv->mdev.

A gap still seems to remain. mlx5e_nic_enable() registers the notifier
only on priv->mdev:

	mlx5_data_direct_register(mdev, &priv->data_direct_nb);

This reader uses ch->mdev instead. On a Socket-Direct netdev, can an
unbind of a secondary PF's dd device race with this lookup? That case
only arises if SD and data direct are used together.

> +		if (dd_dev)
> +			pdev = &dd_dev->pdev->dev;

[Severity: Medium]
Does the returned device match the device the RQ page_pool is created
with? mlx5e_alloc_rq() still uses:

	pp_params.dev       = rq->pdev;

rq->pdev is set in mlx5e_open_channel():

	c->pdev     = mlx5_core_dma_dev(mdev);

That is the PF device, even when the RQ is opened with data_direct set.

io_uring zcrx stores the ndo result and maps the area against it. It then
checks that device against the page_pool device:

io_register_zcrx_ifq()
	ifq->dev = netdev_queue_get_dma_dev(ifq->netdev, if_rxq, NETDEV_QUEUE_TYPE_RX);

io_pp_zc_init()
	if (WARN_ON_ONCE(ifq->dev != pp->p.dev))
		return -EINVAL;

With the pflag set, would registering a zcrx ifq hit this WARN, so that
page_pool_create() and the queue restart both fail?

Even without that check, zcrx_sync_for_device() syncs through pp->p.dev
(the PF). The area, however, was mapped on the data direct device.

There is also a related case. The final patch in the series guards the
pflag change only with netdev_has_dmabuf_binding(). That covers devmem
but not zcrx.

Suppose a zcrx memory provider is installed before the flag is flipped.
mlx5e_open_channel() then computes data_direct=true, because
netif_rxq_has_unreadable_mp() only checks mp_ops. The area is still mapped
on the PF, so the device check passes, but the RQ uses the data direct
PD/KSM mkey. Is that combination expected to work?

[Severity: High]
Can this device go away while a devmem binding still uses it? It belongs
to a separate PCI function (mlx5_dd) that can be removed on its own, and
no reference is taken on it here.

devmem keeps the pointer for as long as the binding exists:

net_devmem_bind_dmabuf()
  dma_buf_attach(binding->dmabuf, dma_dev)
    dma_buf_dynamic_attach()
      attach->dev = dev;    <- no get_device()

The mapping is only torn down later:

__net_devmem_dmabuf_binding_free()
  dma_buf_unmap_attachment_unlocked(binding->attachment, ...)
  dma_buf_detach(binding->dmabuf, binding->attachment)

mlx5_data_direct_remove() does not revoke these bindings. At this commit
the UNBIND notifier only logs.

Later in the series, "net/mlx5e: Recreate netdev channels on data direct
device unbind" clears the pflag and recreates the channels. Its commit
message says the recreated channels "will still pick up the binding via
rxq->mp_params and post DMA addresses from the data direct IOMMU domain
to the PF, causing IOMMU faults".

After a dd unbind or hot-remove with a binding still active, could the
NIC post dd-domain IOVAs through the PF? That would mean IOMMU faults, or
DMA into the wrong memory if the domain is reassigned, for example to
vfio.

On hot removal, could the struct pci_dev be freed while attach->dev still
points to it? The deferred __net_devmem_dmabuf_binding_free() would then
unmap and detach against freed memory.

netdev_has_dmabuf_binding() in the final patch blocks pflag changes from
ethtool. It doesn't seem to cover the dd device being unbound.

[ ... ]

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

Re: [PATCH net-next 08/10] net/mlx5e: Recreate netdev channels on data direct device unbind

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · 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).

- [High] On DD unbind, mlx5e_data_direct_event() clears
  MLX5E_PFLAG_DATA_DIRECT and rebuilds the channels, but it never…
- [Medium] mlx5e_data_direct_event()
  (drivers/net/ethernet/mellanox/mlx5/core/en_main.c) ignores the return…

--

net/mlx5e: Recreate netdev channels on data direct device unbind

On a data direct unbind event, this commit makes the mlx5e notifier clear
MLX5E_PFLAG_DATA_DIRECT and recreate the channels with
mlx5e_safe_switch_params(). It runs under rtnl, the netdev lock and
state_lock. The commit message notes that dmabuf bindings attached to the
data direct device are not torn down.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 6476baf71bdee..fb3e0bd00f04e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -6144,9 +6144,28 @@ static int mlx5e_data_direct_event(struct notifier_block *nb,
>  {
>  	struct mlx5e_priv *priv = container_of(nb, struct mlx5e_priv,
>  					       data_direct_nb);
> +	struct mlx5e_params new_params;
> +
> +	if (action != MLX5_DATA_DIRECT_UNBIND)
> +		return NOTIFY_DONE;
> +
> +	rtnl_lock();
> +	netdev_lock(priv->netdev);
> +	mutex_lock(&priv->state_lock);
>  
> -	if (action == MLX5_DATA_DIRECT_UNBIND)
> -		mlx5_core_dbg(priv->mdev, "data direct device unbound\n");
> +	if (!MLX5E_GET_PFLAG(&priv->channels.params, MLX5E_PFLAG_DATA_DIRECT))
> +		goto out;
> +
> +	new_params = priv->channels.params;
> +	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, false);
> +	mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);

[Severity: High]
What happens to devmem bindings that were mapped against the data direct
device when this switch runs?

While the pflag is set, mlx5e_queue_get_dma_dev() returns the data direct
PCI device. net_devmem_bind_dmabuf() therefore maps the dmabuf in the data
direct function's IOMMU domain, for the RX queues and for TX queue 0.

After the switch, mlx5e_open_channel() computes data_direct as false for
the recreated channels:

    data_direct = MLX5E_GET_PFLAG(params, MLX5E_PFLAG_DATA_DIRECT) &&
                  netif_rxq_has_unreadable_mp(netdev, ix);

and mlx5e_alloc_rq() then uses the ordinary PF UMR mkey:

    wqe->data[0].lkey = data_direct ?
                        cpu_to_be32(rq->mpwqe.crossing_mkey) :
                        rq->mpwqe.umr_mkey_be;

page_pool_init() picks up the same binding again from rxq->mp_params.
mp_dmabuf_devmem_init() does not check or remap the DMA device.

Won't the NIC then write received payload to data direct domain IOVAs as
the PF requester?

On TX, the new SQs no longer have MLX5E_SQ_STATE_DATA_DIRECT, so
mlx5e_txwqe_build_dsegs() uses sq->mkey_be for net_iov frags:

    dseg->lkey = dd && skb_frag_is_net_iov(frag) ?
                 sq->crossing_mkey_be : sq->mkey_be;

So the NIC would read PF domain memory at data direct domain IOVAs.

The commit message mentions IOMMU faults on RX. If an IOVA is also mapped
in the PF domain, could the result be silent memory corruption on RX,
with network-controlled payload, or an information leak on TX?

The later commit "net/mlx5e: Enable the data direct netdev feature" makes
the pflag settable. It also adds a netdev_has_dmabuf_binding() -EBUSY
check, but only in set_pflag_data_direct(). This unbind path still
switches unconditionally, and the series never adds a revoke hook.

At the end of the series, don't the queues still use data direct resources
after unbind, which is what this commit says it prevents?

Do the bindings need to be revoked or quiesced before this switch, or
should the switch be skipped while bindings exist?

> +	netdev_info(priv->netdev,
> +		    "data_direct device unbound, disabled data_direct\n");

[Severity: Medium]
Should the return value of mlx5e_safe_switch_params() be checked here?

When the interface is open, mlx5e_safe_switch_params() can fail if
kzalloc_obj() fails. It can also fail if mlx5e_open_channels() hits an
allocation or firmware command error:

    err = mlx5e_open_channels(priv, new_chs);
    if (err)
        goto err_cancel_selq;

Those error paths free only the new allocations and leave priv->channels
as it was. The old channels stay active:

- The SQs keep MLX5E_SQ_STATE_DATA_DIRECT and the crossing_mkey_be set in
  mlx5e_alloc_txqsq().
- The RQs keep the data direct mkeys.
- priv->channels.params still has MLX5E_PFLAG_DATA_DIRECT set.

Even so, the netdev_info() above reports that data_direct was disabled,
and NOTIFY_OK is returned.

mlx5_data_direct_do_unbind() sends this notification once, after it
clears reg->dd.dev, and never retries. mlx5_data_direct_remove() then
disables and frees the device without checking the result:

    mlx5_data_direct_dev_unreg(dev);
    pci_disable_device(pdev);
    kfree(dev->vuid);
    kfree(dev);

Could devmem-backed queues then keep sending DMA through a PCI function
that has been disabled, or rebound to another driver?

There is also a side effect: with the pflag still set and dd.dev NULL,
mlx5e_queue_get_dma_dev() still takes the data direct branch. But
mlx5_data_direct_get_dev() now returns NULL there, so later devmem or zcrx
binds would fail with no clear reason. Ethtool would also still show the
flag as enabled.

The later commit "net/mlx5e: Enable the data direct netdev feature" makes
this path reachable, and it does not change this callback.

> +
> +out:

[ ... ]

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

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

[netdev-bot+sashiko]

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

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

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_has_dmabuf_binding() matches only on `dev ==
  READ_ONCE(binding->dev)`.
- [Medium] netdev_has_dmabuf_binding() compares the caller's net_device
  pointer with binding->dev for every binding on…
- [Low] The kernel-doc for netdev_has_dmabuf_binding() in
  include/net/netmem.h says without qualification that 'A binding is…

--

net: devmem: add netdev_has_dmabuf_binding() helper

This adds netdev_has_dmabuf_binding(). It reports whether a devmem dmabuf
binding exists for a given netdev, and can also filter on the DMA device
the dmabuf was attached to. Bindings can outlive their xarray slot, so each
one now stays on a new global live list until it is freed.

> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 0cc8572f62caf..8bcf44f43d175 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -387,16 +387,39 @@ static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)

[ ... ]

> +/**
> + * 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 until its last reference is dropped, 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 @dma_dev.

[Severity: Low]
Is this also true for RX bindings?

netdev_has_dmabuf_binding() matches on binding->dev. For RX bindings,
mp_dmabuf_devmem_uninstall() clears that field when the last bound queue
goes away:

net/core/devmem.c:mp_dmabuf_devmem_uninstall() {
    ...
			if (xa_empty(&binding->bound_rxqs)) {
				mutex_lock(&binding->lock);
				ASSERT_EXCLUSIVE_WRITER(binding->dev);
				WRITE_ONCE(binding->dev, NULL);
				mutex_unlock(&binding->lock);
			}
    ...
}

This can happen on netdev unregister through dev_memory_provider_uninstall().
It can also happen on lease teardown through netif_rxq_cleanup_unlease().

After that, the netlink socket, page pools or user-held devmem tokens can
still hold references to the binding. Its sgt is also still mapped against
attachment->dev. Even so, the helper no longer reports the binding for any
netdev.

The helper also walks RX bindings without checking binding->direction.

Should the kernel-doc, and the commit message's "track bindings for their
whole lifetime", say that this only applies to TX bindings?

> + *
> + * 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 07c62e2e3088c..173d66d1cb6e2 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c

[ ... ]

> @@ -357,6 +368,25 @@ struct net_devmem_dmabuf_binding *net_devmem_lookup_dmabuf(u32 id)
>  	return binding;
>  }
>  
> +bool netdev_has_dmabuf_binding(struct net_device *dev, struct device *dma_dev)
> +{
> +	struct net_devmem_dmabuf_binding *binding;
> +	bool found = false;
> +
> +	spin_lock(&net_devmem_live_bindings_lock);
> +	list_for_each_entry(binding, &net_devmem_live_bindings, live_list) {
> +		if (dev == READ_ONCE(binding->dev) &&
> +		    (!dma_dev || binding->attachment->dev == dma_dev)) {

[Severity: High]
Can this miss RX bindings made on a leased queue?

netdev_nl_bind_rx_doit() stores the netdev the user named by ifindex as
binding->dev:

net/core/netdev-genl.c:netdev_nl_bind_rx_doit() {
    ...
	binding = net_devmem_bind_dmabuf(netdev, NULL, dma_dev, DMA_FROM_DEVICE,
					 dmabuf_fd, niov_shift, priv,
					 info->extack);
    ...
}

When that netdev is a virtual leasee such as netkit, the rest of the setup
still points at the physical NIC:

  - netdev_queue_get_dma_dev() follows the lease and returns the physical
    queue's DMA device. The dmabuf is attached and mapped against it.

  - netif_mp_open_rxq() installs the memory provider on the physical RX
    queue through __netif_get_rx_queue_lease(..., NETIF_VIRT_TO_PHYS).

The physical driver then fills its RX rings with net_iov DMA addresses
that are only valid for that dma_dev. But
netdev_has_dmabuf_binding(phys_netdev, ...) returns false, because
binding->dev is the netkit device.

TX doesn't have this problem, because netdev_nl_bind_tx_doit() stores the
physical bind_dev in binding->dev and the virtual device in vdev.

The only caller in the series is set_pflag_data_direct() in
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c:

    if (netdev_has_dmabuf_binding(netdev, NULL)) {

For a leased RX binding this check doesn't fire, so
mlx5e_safe_switch_params() goes ahead and reopens the channels. The queue
still has its mp installed, so this code in mlx5e_open_channel():

	data_direct = MLX5E_GET_PFLAG(params, MLX5E_PFLAG_DATA_DIRECT) &&
		      netif_rxq_has_unreadable_mp(netdev, ix);

would use the other DMA device with IOVAs that were mapped for the old one.

Could that cause IOMMU faults? Or could RX DMA land wherever those IOVAs
point in the other domain?

For RX bindings, should the lookup also match the physical device behind
a lease?

[Severity: Medium]
For TX bindings, could this pointer comparison match a stale binding->dev?

net_devmem_bind_dmabuf() sets binding->dev = dev without taking a netdev
reference, and netdev_nl_bind_tx_doit() doesn't take one either. The only
later write to the field is WRITE_ONCE(binding->dev, NULL) in
mp_dmabuf_devmem_uninstall(). That only runs for RX queues with an
installed memory provider.

With this patch, TX bindings stay on net_devmem_live_bindings until their
last percpu ref is dropped. That only happens once the owning netlink
socket is closed and all in-flight skbs are freed.

If the netdev is unregistered and freed before then, the list entry keeps
a dangling net_device pointer. An mlx5 driver rebind or a devlink reload
could do that.

If a new net_device is later allocated at the same address,
netdev_has_dmabuf_binding(new_netdev, NULL) would return true for a
binding that belongs to the old device. set_pflag_data_direct() would then
keep returning -EBUSY for as long as the old netlink socket stays open.

The stale TX binding->dev was already there before this patch. What this
patch adds is using it as a lookup key for the whole binding lifetime.

> +			found = true;
> +			break;
> +		}
> +	}
> +	spin_unlock(&net_devmem_live_bindings_lock);
> +
> +	return found;
> +}
> +EXPORT_SYMBOL_GPL(netdev_has_dmabuf_binding);

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

Re: [PATCH net-next 10/10] net/mlx5e: Enable the data direct netdev feature

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 · 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).

- [High] set_pflag_data_direct()
  (drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c:2431) is meant…
- [High] Before this commit, set_pflag_data_direct() always returned
  -EOPNOTSUPP, so the body of mlx5e_data_direct_event() never ran.
- [Medium] This commit makes mlx5e_data_direct_event() reachable, and
  that handler throws away the return value of…

--

net/mlx5e: Enable the data direct netdev feature

This commit adds the setter for the mlx5e data_direct private flag, which
turns the data direct feature on. It also refuses to change the flag while
dmabufs are bound to the netdev.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index b3eced1dd43fd..8ab1150ce3f20 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c

[ ... ]

> @@ -2402,7 +2403,40 @@ 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;
> +

[ ... ]

> +	/* Changing the flag reopens the channels with a different DMA device,
> +	 * which would break any dmabuf currently bound to this netdev.
> +	 */
> +	if (netdev_has_dmabuf_binding(netdev, NULL)) {
> +		netdev_err(netdev, "Can't change data_direct while dmabufs are bound\n");
> +		return -EBUSY;
> +	}

[Severity: High]
Does this check catch every memory provider that can be installed on an
mlx5 RX queue?

netdev_has_dmabuf_binding() only walks net_devmem_live_bindings and
matches on binding->dev:

net/core/devmem.c:netdev_has_dmabuf_binding() {
    ...
	list_for_each_entry(binding, &net_devmem_live_bindings, live_list) {
		if (dev == READ_ONCE(binding->dev) &&
		    (!dma_dev || binding->attachment->dev == dma_dev)) {
    ...
}

This seems to miss two cases.

The first is a devmem RX binding made through a leased queue. In
netdev_nl_bind_rx_doit(), net_devmem_bind_dmabuf() is called with the
leasee netdev, so binding->dev is the virtual device. The dma_dev still
comes from mlx5e_queue_get_dma_dev() through the lease, and
netif_mp_open_rxq() installs the provider on the physical mlx5 rxq.

The second is io_uring zcrx. zcrx_register_netdev() does:

	ifq->dev = netdev_queue_get_dma_dev(ifq->netdev, if_rxq, NETDEV_QUEUE_TYPE_RX);

It then maps its area for that device and installs io_uring_pp_zc_ops
with netif_mp_open_rxq(). It never adds anything to
net_devmem_live_bindings.

In both cases the flag change is accepted and mlx5e_safe_switch_params()
reopens the channels. mlx5e_open_channel() then does:

	data_direct = MLX5E_GET_PFLAG(params, MLX5E_PFLAG_DATA_DIRECT) &&
		      netif_rxq_has_unreadable_mp(netdev, ix);

That is true for any unreadable provider, so the RQ switches between the
PF MTT UMR mkey and the data direct KSM crossing mkey. The page pool
netmems still hold IOVAs mapped for the old DMA device, and
mlx5e_alloc_rx_mpwqe() writes them into the KSM/MTT entries:

	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),

Could the NIC then DMA to IOVAs resolved through the other function's
IOMMU domain? That would mean IOMMU faults, or writes into unrelated pages
if the same IOVA happens to be mapped there.

The commit message says "Do not allow state changes while dmabufs are
still bound to the netdev". Should the setter also refuse the change when
any RX queue has a memory provider installed? One way would be to check
netif_rxq_has_unreadable_mp() for each queue below real_num_rx_queues.

> +
> +	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);

[Severity: High]
Before this patch, set_pflag_data_direct() always returned -EOPNOTSUPP,
so the body of mlx5e_data_direct_event() never ran. Now that the flag can
be set, what keeps the dmabuf invariant when the data direct PCI function
unbinds?

While the flag is set, mlx5e_queue_get_dma_dev() returns the data direct
device:

	dd_dev = mlx5_data_direct_get_dev(mdev->data_direct);
	if (dd_dev)
		pdev = &dd_dev->pdev->dev;

So devmem bindings and zcrx areas created during that time are attached
to, and mapped for, the data direct function.

On sysfs unbind or hot-unplug the call chain is:

mlx5_data_direct_remove()
  mlx5_data_direct_dev_unreg()
    mlx5_data_direct_do_unbind()
      mlx5e_data_direct_event()
        mlx5e_safe_switch_params()

mlx5e_data_direct_event() clears the flag and reopens the channels. It
does not call netdev_has_dmabuf_binding() or
netif_rxq_has_unreadable_mp(), and it does not revoke anything:

	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, false);
	mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);

rxq->mp_params is left in place. Would the recreated PF RQs keep posting
net_iovs whose DMA addresses come from the data direct function's IOMMU
domain?

After the notifier returns, mlx5_data_direct_remove() continues with:

	mlx5_data_direct_dev_unreg(dev);
	pci_disable_device(pdev);
	kfree(dev->vuid);
	kfree(dev);

It does not wait for the outstanding attachments. Later, the deferred
__net_devmem_dmabuf_binding_free() calls
dma_buf_unmap_attachment_unlocked() on an attachment whose device is the
removed function.

An earlier commit in this series, "net/mlx5e: Recreate netdev channels on
data direct device unbind", acknowledges the IOMMU faults and says a
devmem revoke hook is needed. No later commit in the series adds one.
Should the flag stay disabled until that revoke path exists?

[Severity: Medium]
This patch also makes the error handling in mlx5e_data_direct_event()
reachable. The handler ignores the return value of
mlx5e_safe_switch_params() and returns NOTIFY_OK:

	mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
	netdev_info(priv->netdev,
		    "data_direct device unbound, disabled data_direct\n");

Suppose the interface is open and mlx5e_safe_switch_params() fails in
kzalloc_obj() or mlx5e_open_channels(). priv->channels is then left
unchanged. MLX5E_PFLAG_DATA_DIRECT stays set, and the old RQs/SQs keep
their data direct state, their KSM crossing mkeys and their page pools
mapped for the data direct device.

mlx5_data_direct_remove() then goes on to pci_disable_device() and
kfree(dev). Would the live queues still be routing DMA through a disabled
PCI function at that point?

mlx5_data_direct_do_unbind() has also already done:

	WRITE_ONCE(reg->dd.dev, NULL);

Because the flag is stale, mlx5e_queue_get_dma_dev() still takes the data
direct branch. It returns NULL instead of falling back to ch->pdev, so new
zero-copy bindings would fail until someone clears the flag by hand.

On this forced path, should a failure take the affected channels down, or
otherwise make sure the data direct path is gone, before PCI teardown
continues?

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