Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Zhiping Zhang <zhipingz@meta.com>
Cc: fengchengwen <fengchengwen@huawei.com>,
	Alex Williamson <alex@shazbot.org>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	Bjorn Helgaas <helgaas@kernel.org>,
	linux-rdma@vger.kernel.org, linux-pci@vger.kernel.org,
	netdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Keith Busch <kbusch@kernel.org>, Yochai Cohen <yochai@nvidia.com>,
	Yishai Hadas <yishaih@nvidia.com>,
	kvm@vger.kernel.org
Subject: Re: [PATCH v2 1/2] vfio: add dma-buf get_tph callback and DMA_BUF_TPH feature
Date: Wed, 13 May 2026 09:31:41 +0300	[thread overview]
Message-ID: <20260513063141.GC15586@unreal> (raw)
In-Reply-To: <CAH3zFs2x2QebpqDC0qwQg_GP2FVs-qqNxPupck40cEHNrBMEsA@mail.gmail.com>

On Wed, May 06, 2026 at 11:23:19AM -0700, Zhiping Zhang wrote:
> On Tue, May 5, 2026 at 11:58 PM fengchengwen <fengchengwen@huawei.com> wrote:
> >
> > >
> > On 5/1/2026 4:06 AM, Zhiping Zhang wrote:
> > > Add a dma-buf callback that returns raw TPH metadata from the exporter
> > > so peer devices can reuse the steering tag and processing hint
> > > associated with a VFIO-exported buffer.
> > >
> > > Add a new VFIO_DEVICE_FEATURE_DMA_BUF_TPH ioctl that takes the fd from
> > > VFIO_DEVICE_FEATURE_DMA_BUF along with a steering tag and processing
> > > hint, validates the fd is a vfio-exported dma-buf belonging to this
> > > device, and stores the TPH values under memory_lock. This keeps the
> > > existing VFIO_DEVICE_FEATURE_DMA_BUF uAPI completely unchanged.
> > >
> > > The user sequences setting TPH on the dma-buf before the importer
> > > consumes it.
> > >
> > > Add an st_width parameter to get_tph() so the exporter can reject
> > > steering tags that exceed the consumer's supported width (8 vs 16 bit).
> > > When no TPH metadata was supplied, get_tph() returns -EOPNOTSUPP.
> > >
> > > Signed-off-by: Zhiping Zhang <zhipingz@meta.com>
> > >
> > > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> > > --- a/drivers/vfio/pci/vfio_pci_core.c
> > > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > > @@ -1534,6 +1534,9 @@ int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags,
> > >               return vfio_pci_core_feature_token(vdev, flags, arg, argsz);
> > >       case VFIO_DEVICE_FEATURE_DMA_BUF:
> > >               return vfio_pci_core_feature_dma_buf(vdev, flags, arg, argsz);
> > > +     case VFIO_DEVICE_FEATURE_DMA_BUF_TPH:
> > > +             return vfio_pci_core_feature_dma_buf_tph(vdev, flags, arg,
> > > +                                                      argsz);
> > >       default:
> > >               return -ENOTTY;
> > >       }
> > > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> > > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > > @@ -19,6 +19,9 @@ struct vfio_pci_dma_buf {
> > >       u32 nr_ranges;
> > >       struct kref kref;
> > >       struct completion comp;
> > > +     u16 steering_tag;
> > > +     u8 ph;
> > > +     u8 tph_present : 1;
> > >       u8 revoked : 1;
> > >  };
> > >
> > > @@ -69,6 +72,22 @@ vfio_pci_dma_buf_map(struct dma_buf_attachment *attachment,
> > >       return ret;
> > >  }
> > >
> > > +static int vfio_pci_dma_buf_get_tph(struct dma_buf *dmabuf, u16 *steering_tag,
> > > +                                 u8 *ph, u8 st_width)
> > > +{
> > > +     struct vfio_pci_dma_buf *priv = dmabuf->priv;
> > > +
> > > +     if (!priv->tph_present)
> > > +             return -EOPNOTSUPP;
> > > +
> > > +     if (st_width < 16 && priv->steering_tag > ((1U << st_width) - 1))
> > > +             return -EINVAL;
> >
> > The checker will failed in following cases:
> > 1. If the exporter passed 8bit st, and importer support 16bit st, then it will pass
> >    the checker.
> > 2. The exporter enabled 16bit st and its st is < 256 (note: the pcie protocol doesn't
> >    restrict 16bit-st must >=256), and importer only support 8bit st, then it will also
> >    pass the checker
> >
> > Suggest userspace passing both st(8bit) and extend-st(16bit), and importer chose the
> > right one.
> >
> 
>  Agreed — 8-bit ST and 16-bit Extended ST are distinct namespaces
> (firmware returns
> them as separate fields with separate validity bits), so a numeric
> range check is insufficient.
> For v3 I'll change the uAPI to carry both, gated by a flags field:
> 
>   #define VFIO_DMA_BUF_TPH_ST (1 << 0)  /* steering_tag valid */
>   #define VFIO_DMA_BUF_TPH_ST_EXT (1 << 1)  /* steering_tag_ext valid
> */
>   struct vfio_device_feature_dma_buf_tph {
>       __s32 dmabuf_fd;
>       __u32 flags;
>       __u16 steering_tag;       /* 8-bit ST */
>       __u16 steering_tag_ext;   /* 16-bit Extended ST */

I wonder whether `steering_tag` and `steering_tag_ext` can coexist  
and hold different values at the same time.

BTW, please send your patches with diffstat.

Thanks

>       __u8  ph;
>       __u8  reserved[3];
>   };
> 
> get_tph() then picks the field matching the importer's st_width and
> returns -EOPNOTSUPP
> if that one isn't valid.
> 
> Thanks,
> Zhiping

  reply	other threads:[~2026-05-13  6:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260430200704.352228-1-zhipingz@meta.com>
     [not found] ` <20260430200704.352228-2-zhipingz@meta.com>
     [not found]   ` <20260504154459.77b8153d@shazbot.org>
2026-05-05  6:54     ` [PATCH v2 1/2] vfio: add dma-buf get_tph callback and DMA_BUF_TPH feature Zhiping Zhang
     [not found]   ` <09995589-b81d-4cb7-a313-15a943d8b28d@huawei.com>
2026-05-06 18:23     ` Zhiping Zhang
2026-05-13  6:31       ` Leon Romanovsky [this message]
     [not found] ` <20260430200704.352228-3-zhipingz@meta.com>
     [not found]   ` <a63179d7-28b1-4269-9ef2-c20368d0b91c@huawei.com>
2026-05-06 18:13     ` [PATCH v2 2/2] RDMA/mlx5: get tph for p2p access when registering dma-buf mr Zhiping Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260513063141.GC15586@unreal \
    --to=leon@kernel.org \
    --cc=alex@shazbot.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=fengchengwen@huawei.com \
    --cc=helgaas@kernel.org \
    --cc=jgg@ziepe.ca \
    --cc=kbusch@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=yishaih@nvidia.com \
    --cc=yochai@nvidia.com \
    --cc=zhipingz@meta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox