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 <[email protected]> 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/[email protected]?part=3
