Re: [PATCH v6 3/5] dma-buf: add optional get_tph() callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `get_tph` callback takes `struct dma_buf *` instead of `struct dma_buf_attachment *`, failing to account for per-attachment mappings and dynamic buffer migrations.
- [High] The new `get_tph` operation lacks a corresponding core API wrapper function in `dma-buf.c` and is omitted from the subsystem locking documentation.
--

commit 4644c9f73b091302efc6ec2a805dcc7affb8aaf6
Author: Zhiping Zhang <zhipingz@meta.com>

dma-buf: add optional get_tph() callback

This commit adds an optional get_tph callback to the dma-buf ops so an
exporter can return PCIe TPH (TLP Processing Hints) metadata to an importer.
The API provides a discoverable negotiation point between exporter and
importer.

> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index d1203da56fc5f..8437dbe4a83ec 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -113,6 +113,37 @@ struct dma_buf_ops {
>  	 */
>  	void (*unpin)(struct dma_buf_attachment *attach);
>  
> +	/**
> +	 * @get_tph:
> +	 * @dmabuf: DMA buffer for which to retrieve TPH metadata
> +	 * @extended: false to request the 8-bit ST namespace, true to request
> +	 *            the 16-bit Extended ST namespace
> +	 * @steering_tag: Returns the raw TPH steering tag for the requested
> +	 *                namespace
> +	 * @ph: Returns the TPH processing hint (2-bit value)
> +	 *
> +	 * Return the TPH (TLP Processing Hints) metadata associated with this
> +	 * DMA buffer for the requested steering-tag namespace. 8-bit ST and
> +	 * 16-bit Extended ST are distinct namespaces in the PCIe TPH ST table
> +	 * and may both be present with different values, so the exporter must
> +	 * select the value that matches @extended and must not substitute one
> +	 * for the other.
> +	 *
> +	 * The exporter owns the completing address space for @dmabuf and
> +	 * therefore decides whether it can derive meaningful TPH metadata for
> +	 * that completer. The dma-buf core treats the returned ST/PH tuple as
> +	 * opaque transport metadata; importers that support TPH place it on
> +	 * outbound TLPs, while exporters that cannot derive a useful tuple
> +	 * simply return -EOPNOTSUPP.
> +	 *
> +	 * Return 0 on success, or -EOPNOTSUPP if no metadata is available for
> +	 * the requested namespace.
> +	 *
> +	 * This callback is optional.
> +	 */
> +	int (*get_tph)(struct dma_buf *dmabuf, bool extended,
> +		       u16 *steering_tag, u8 *ph);

[Severity: High]
Does this callback need to take a struct dma_buf_attachment * instead of a
struct dma_buf *?

A dma-buf's physical backing storage is only stable in the context of a mapped
attachment. Exporters might provide device memory to a peer-to-peer capable
importer, but system memory to another importer. Furthermore, dynamic dma-bufs
can migrate their backing storage between VRAM and system memory, which changes
the completer and the correct Steering Tag.

By querying TPH metadata on the global dma_buf object without an attachment
context, the exporter cannot know which importer's mapping is being queried.
Can this prevent the exporter from returning the correct completing address
space's Steering Tag?

[Severity: High]
Should this new operation have a corresponding core API wrapper function exported
in drivers/dma-buf/dma-buf.c and documented in the locking convention block?

In the dma-buf architecture, ops are typically encapsulated behind core wrappers
to enforce cross-subsystem locking, validate inputs, and handle tracing. By
omitting the wrapper, importers are forced to bypass the core API and
dereference the ops structure directly:

drivers/infiniband/hw/mlx5/mr.c:get_tph_mr_dmabuf() {
    ...
    ret = dmabuf->ops->get_tph(dmabuf, extended, &steering_tag, &local_ph);
    ...
}

Does the lack of a wrapper function like dma_buf_get_tph() violate the expected
API encapsulation for dma-buf operations?

>  	/**
>  	 * @map_dma_buf:
>  	 *

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608185646.4085127-1-zhipingz@meta.com?part=3