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

[Leon Romanovsky <leon@kernel.org>]

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
>