Re: [RFC v2 1/2] vfio: add callback to get tph info for dmabuf

Leon Romanovsky
2026-04-13T19:23:07Z

On Mon, Apr 13, 2026 at 11:32:48AM -0700, Zhiping Zhang wrote:
> On Thu, Apr 9, 2026 at 5:04 AM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > >
> > On Tue, Mar 31, 2026 at 01:44:02PM -0600, Keith Busch wrote:
> > > On Tue, Mar 31, 2026 at 10:02:20PM +0300, Leon Romanovsky wrote:
> > > >
> > > > Right, what about adding TPH fields to struct vfio_region_dma_range
> > > > instead of struct vfio_device_feature_dma_buf?
> > >
> > > You might have to show me with code what you're talking about because I
> > > can't see any way we can add fields to any struct here without breaking
> > > backward compatibility.
> > >
> > > If we can't claim bits out of the unused "flags" field for this feature,
> > > then my initial reply is the only sane approach: we can introduce a new
> > > feature and struct for it that closely mirrors the existing one, but
> > > with the extra hint fields.
> >
> > Something like that, on top of this proposal:
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > index 3961afa640391..70d5ee1e3ef7b 100644
> > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > @@ -241,9 +241,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
> > return -EFAULT;
> >
> > if (!get_dma_buf.nr_ranges ||
> > - (get_dma_buf.flags & ~(VFIO_DMABUF_FL_TPH |
> > - VFIO_DMABUF_TPH_PH_MASK |
> > - VFIO_DMABUF_TPH_ST_MASK)))
> > + (get_dma_buf.flags & ~VFIO_DMABUF_FLAG_TPH))
> > return -EINVAL;
> >
> > /*
> > @@ -300,13 +298,10 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
> > ret = PTR_ERR(priv->dmabuf);
> > goto err_dev_put;
> > }
> > - if (get_dma_buf.flags & VFIO_DMABUF_FL_TPH) {
> > - priv->steering_tag = (get_dma_buf.flags &
> > - VFIO_DMABUF_TPH_ST_MASK) >>
> > - VFIO_DMABUF_TPH_ST_SHIFT;
> > - priv->ph = (get_dma_buf.flags &
> > - VFIO_DMABUF_TPH_PH_MASK) >>
> > - VFIO_DMABUF_TPH_PH_SHIFT;
> > + if (get_dma_buf.flags & VFIO_DMABUF_FLAG_TPH) {
> > + priv->steering_tag =
> > + dma_ranges[get_dma_buf.nr_ranges + 1].tph.tag;
> > + priv->ph = dma_ranges[get_dma_buf.nr_ranges + 1].tph.ph;
> > }
> > /* dma_buf_put() now frees priv */
> > INIT_LIST_HEAD(&priv->dmabufs_elm);
> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > index e2a8962641d2c..a8b8d8b1a3278 100644
> > --- a/include/uapi/linux/vfio.h
> > +++ b/include/uapi/linux/vfio.h
> > @@ -1497,20 +1497,30 @@ struct vfio_device_feature_bus_master {
> > */
> > #define VFIO_DEVICE_FEATURE_DMA_BUF 11
> >
> > +struct vfio_region_dma_tph {
> > + u16 tag;
> > + u8 ph;
> > +};
> > +
> > struct vfio_region_dma_range {
> > - __u64 offset;
> > - __u64 length;
> > + union {
> > + __u64 offset;
> > + struct vfio_region_dma_tph tph;
> > + };
> > + union {
> > + __u64 length;
> > + __u64 reserved;
> > + };
> > +};
> > +
> > +enum {
> > + VFIO_DMABUF_FLAG_TPH = 1 << 0,
> > };
> >
> > struct vfio_device_feature_dma_buf {
> > __u32 region_index;
> > __u32 open_flags;
> > __u32 flags;
> > -#define VFIO_DMABUF_FL_TPH (1U << 0) /* TPH info is present */
> > -#define VFIO_DMABUF_TPH_PH_SHIFT 1 /* bits 1-2: PH (2-bit) */
> > -#define VFIO_DMABUF_TPH_PH_MASK 0x6U
> > -#define VFIO_DMABUF_TPH_ST_SHIFT 16 /* bits 16-31: steering tag */
> > -#define VFIO_DMABUF_TPH_ST_MASK 0xffff0000U
> > __u32 nr_ranges;
> > struct vfio_region_dma_range dma_ranges[] __counted_by(nr_ranges);
> > };
>
> Sounds good, thanks! We will follow up and move this RFC to a formal patch.

Great. Also, please rename "struct vfio_region_dma_range dma_ranges" to
something that makes it clear this is a storage object, not something
limited to a DMA range.

Thanks

>
> Zhiping
>