Claude review: RDMA/mlx5: get tph for p2p access when registering dma-buf mr

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Critical: Steering tag index resource leak**

```c
+	ret = mlx5_st_alloc_index_by_tag(dev->mdev, steering_tag, st_index);
+	if (ret) {
+		*ph = MLX5_IB_NO_PH;
+		mlx5_ib_dbg(dev, "st_alloc_index_by_tag failed (%d)\n", ret);
+	}
```

`mlx5_st_alloc_index_by_tag()` allocates a refcounted entry in `st->idx_xa` (in non-direct mode). The resulting `st_index` is passed into `alloc_cacheable_mr()` and written into the hardware MKC, but it is **never stored** in a way that allows `mlx5_st_dealloc_index()` to be called when the MR is destroyed. Looking at `__mlx5_ib_dereg_mr()`, there is no call to `mlx5_st_dealloc_index()` — that only happens through the DMAH path (`mlx5_ib_dealloc_dmah()`).

This means every MR registered via the new dma-buf TPH fallback path leaks an xarray entry. Over time, the steering tag table fills up and `xa_alloc()` will eventually fail.

The MR needs to track whether it owns an st_index allocation and call `mlx5_st_dealloc_index()` during `__mlx5_ib_dereg_mr()`.

**Redundant `dma_buf_get()`/`dma_buf_put()`**

```c
+static void get_tph_mr_dmabuf(struct mlx5_ib_dev *dev, int fd, u16 *st_index,
+			      u8 *ph)
+{
+	...
+	dmabuf = dma_buf_get(fd);
+	if (IS_ERR(dmabuf))
+		return;
+	...
+	dma_buf_put(dmabuf);
+}
```

This function does `dma_buf_get(fd)` / `dma_buf_put(dmabuf)` solely to query TPH. The caller `reg_user_mr_dmabuf()` will immediately afterward call `ib_umem_dmabuf_get()`, which does another `dma_buf_get()` on the same fd. This is a wasted get/put cycle. Consider restructuring so the TPH query happens after the dmabuf is already acquired (e.g., query from `umem_dmabuf->dmabuf`), avoiding the redundant fd lookup.

**Direct ops call bypasses dma-buf framework**

```c
+	if (!dmabuf->ops->get_tph)
+		goto end_dbuf_put;
+
+	ret = dmabuf->ops->get_tph(dmabuf, &steering_tag, ph, st_width);
```

This calls through the ops table directly rather than through a framework wrapper function. Every other dma-buf operation (map, unmap, pin, unpin, attach, etc.) goes through a wrapper in `drivers/dma-buf/dma-buf.c`. The NULL check should be inside the wrapper, not at the call site. This also means the call bypasses any future framework-level tracing, locking, or validation.

**`ret = 0` initialization in `mlx5_st_create()`**

```c
-	int ret;
+	int ret = 0;
```

This fix addresses a real compiler warning, but it would be better in a separate patch since it's an independent bugfix unrelated to the TPH-from-dma-buf feature. It could also be submitted as a standalone fix that can be backported independently.

**`MODULE_IMPORT_NS("DMA_BUF")`**

```c
+MODULE_IMPORT_NS("DMA_BUF");
```

This is needed because `dma_buf_get()` and `dma_buf_put()` are exported in the `DMA_BUF` namespace. Correct.

**`st_width` derivation**

```c
+	st_width = (pdev->tph_req_type == PCI_TPH_REQ_EXT_TPH) ? 16 : 8;
```

This checks `tph_req_type` but doesn't first verify that TPH is actually enabled/supported on this device. If `pdev->tph_req_type` is `PCI_TPH_REQ_DISABLE` or 0 (no TPH), the function will derive `st_width = 8` and proceed to allocate a steering tag that the device can't use. The function should early-return if the device doesn't support TPH (or `dev->st` is NULL), before even querying the dma-buf.

**Error handling: `ph` set to `MLX5_IB_NO_PH` on alloc failure but steering_tag left as-is**

```c
+	ret = mlx5_st_alloc_index_by_tag(dev->mdev, steering_tag, st_index);
+	if (ret) {
+		*ph = MLX5_IB_NO_PH;
```

When `mlx5_st_alloc_index_by_tag()` fails, `*ph` is reset to `MLX5_IB_NO_PH` but `*st_index` is left in an indeterminate state (it was passed as a pointer and `mlx5_st_alloc_index_by_tag()` may or may not have written to it). The caller initialized `st_index = MLX5_MKC_PCIE_TPH_NO_STEERING_TAG_INDEX`, which should be fine if `mlx5_st_alloc_index_by_tag()` doesn't modify `*st_index` on error, but this isn't guaranteed — the xarray path could partially succeed. Safer to explicitly reset `*st_index = MLX5_MKC_PCIE_TPH_NO_STEERING_TAG_INDEX` in the error path.

**Refactoring of `mlx5_st_alloc_index` is clean**

The split into `mlx5_st_alloc_index_by_tag()` (takes raw tag) and `mlx5_st_alloc_index()` (resolves CPU → tag, then calls `_by_tag`) is a clean refactor. The `#ifndef CONFIG_PCIE_TPH` static inline stub returning `-EOPNOTSUPP` is correct.

---
Generated by Claude Code Patch Reviewer