Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1670431 > unrolled thread

RE: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf         operations "Zhang, Tina" <tina.zhang@intel.com> - 2017-06-20 10:50 +0200
    Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-20 13:00 +0200
      Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-20 17:10 +0200
        Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Kirti Wankhede <kwankhede@nvidia.com> - 2017-06-20 19:10 +0200
        RE: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations "Zhang, Tina" <tina.zhang@intel.com> - 2017-06-21 01:10 +0200
          Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-21 01:30 +0200
            RE: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations "Zhang, Tina" <tina.zhang@intel.com> - 2017-06-21 11:30 +0200
              Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-21 13:10 +0200
                Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-21 21:00 +0200
                  Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-22 10:40 +0200
                    Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-22 21:00 +0200
                      Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-23 09:30 +0200
                        Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf operations Zhi Wang <zhi.a.wang@intel.com> - 2017-06-23 10:10 +0200
                          Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-23 10:40 +0200
                            Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-23 18:50 +0200
                        Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-23 19:20 +0200
                          Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-26 08:20 +0200
                RE: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations "Zhang, Tina" <tina.zhang@intel.com> - 2017-06-22 02:30 +0200
        Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-21 09:40 +0200
      RE: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations "Zhang, Tina" <tina.zhang@intel.com> - 2017-06-24 00:00 +0200
        Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-26 08:40 +0200
          Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Alex Williamson <alex.williamson@redhat.com> - 2017-06-26 19:30 +0200
            Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-27 08:20 +0200
              RE: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations "Zhang, Tina" <tina.zhang@intel.com> - 2017-06-28 14:50 +0200
                Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Gerd Hoffmann <kraxel@redhat.com> - 2017-06-29 08:50 +0200
                  Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf  operations Daniel Vetter <daniel@ffwll.ch> - 2017-06-29 10:40 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1674464 — Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

  Hi,

> > With the generation we can also do something different:  Pass in
> > plane_type and
> > generation, and have VFIO_DEVICE_GET_DMABUF_FD return an error in
> > case
> > the generation doesn't match.  In that case it doesn't make much
> > sense any
> > more to have a separate plane_info struct, which was added so we
> > don't have
> > to duplicate things in query-plane and get- dmabuf ioctl structs.
> 
> Comparing with the current patch, this would make user space a little
> bit harder to
> get the dmabuf by calling VFIO_DEVICE_GET_DMABUF ioctl. Is it
> efficient for
> user mode usage?

user space has to call QUERY-PLANE first, then looks if it has a dma-
buf for that, if not call GET-DMABUF.

Problem is the guest could have changed the plane between the QUERY-
PLANE and GET-DMABUF ioctls.

Current patches (v8 series) just returns plane-info on GET-DMABUF too,
so userspace can at least detect something changed.

It would be easier for userspace if GET-DMABUF throws an error in case
the plane changed since the last QUERY-PLANE ioctl.  The generation id
would be one way to handle it, but possibly it is easier if the kernel
driver just keeps track internally.  So GET-DMABUF would be defined to
return a dmabuf for the plane returned by the previous QUERY-PLANE
ioctl (on the same file handle), or return an error in case the plane
has changed meanwhile.

cheers,
  Gerd

[toc] | [prev] | [next] | [standalone]

#1674961 — Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On Mon, 26 Jun 2017 08:39:17 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
> 
> > > With the generation we can also do something different:  Pass in
> > > plane_type and
> > > generation, and have VFIO_DEVICE_GET_DMABUF_FD return an error in
> > > case
> > > the generation doesn't match.  In that case it doesn't make much
> > > sense any
> > > more to have a separate plane_info struct, which was added so we
> > > don't have
> > > to duplicate things in query-plane and get- dmabuf ioctl structs.  
> > 
> > Comparing with the current patch, this would make user space a little
> > bit harder to
> > get the dmabuf by calling VFIO_DEVICE_GET_DMABUF ioctl. Is it
> > efficient for
> > user mode usage?  
> 
> user space has to call QUERY-PLANE first, then looks if it has a dma-
> buf for that, if not call GET-DMABUF.
> 
> Problem is the guest could have changed the plane between the QUERY-
> PLANE and GET-DMABUF ioctls.
> 
> Current patches (v8 series) just returns plane-info on GET-DMABUF too,
> so userspace can at least detect something changed.
> 
> It would be easier for userspace if GET-DMABUF throws an error in case
> the plane changed since the last QUERY-PLANE ioctl.  The generation id
> would be one way to handle it, but possibly it is easier if the kernel
> driver just keeps track internally.  So GET-DMABUF would be defined to
> return a dmabuf for the plane returned by the previous QUERY-PLANE
> ioctl (on the same file handle), or return an error in case the plane
> has changed meanwhile.

Hmm, I don't like that interface.  Can you cite examples of other
ioctls that behave this way?  It doesn't feel like an elegant user
interface; the user can get the dmabuf, but only after they query the
dmabuf, even though the get-dmabuf ioctl returns the same data as the
query-plane ioctl, but they can't get the dmabuf if the plane has
changed in the interim, which is not something the user can know.  Are
we causing our own problems with this model of cycling through dmabuf
fds?  We talked previously about an enum of plane types, primary and
cursor.  What if the user was simply able to get a dmabuf fd for each of
those and they queried the current plane information via those fds?
IOW, the fd is persistent and specific to a given plane type, but the
format within it is dynamic.  For instance, I don't have a separate
monitor on my desktop for each resolution I want to run, the monitor
adapts to the signal it gets.  I don't grasp the technical reasons why
the user can't stop using the dmabuf fd with the previous format
parameters and start using it with the new parameters.  Maybe the user
even has multiple dmabuf fds open, but they switch to only actively
using then one(s) that match the current format.  I don't know if
that's viable, but there seems to be a fundamental synchronization
issue if a given dmabuf fd only represents a transient state.  Thanks,

Alex

[toc] | [prev] | [next] | [standalone]

#1675321 — Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

  Hi,

> Hmm, I don't like that interface.  Can you cite examples of other
> ioctls that behave this way?  It doesn't feel like an elegant user
> interface; the user can get the dmabuf, but only after they query the
> dmabuf, even though the get-dmabuf ioctl returns the same data as the
> query-plane ioctl, but they can't get the dmabuf if the plane has
> changed in the interim, which is not something the user can
> know.  Are
> we causing our own problems with this model of cycling through dmabuf
> fds?  We talked previously about an enum of plane types, primary and
> cursor.  What if the user was simply able to get a dmabuf fd for each
> of
> those and they queried the current plane information via those fds?
> IOW, the fd is persistent and specific to a given plane type, but the
> format within it is dynamic.

Will not work due to how dma-bufs are designed.

But, yes, the QUERY then GET split is ugly for a number of reasons.

Does gvt track the live cycle of all dma-bufs it has handed out?
If so, then maybe we can let the kernel check whenever a dma-buf for
the current plane exists?  And if that isn't the case hand out a dma-
buf right away, without expecting userspace explicitly asking for it?

That will simplify the interface and remove the race condition at the
expense of some additional bookkeeping in the kernel.

cheers,
  Gerd

[toc] | [prev] | [next] | [standalone]

#1676629 — RE: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Gerd Hoffmann
> Sent: Tuesday, June 27, 2017 2:13 PM
> To: Alex Williamson <alex.williamson@redhat.com>
> Cc: Wang, Zhenyu Z <zhenyu.z.wang@intel.com>; intel-
> gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org; Chen, Xiaoguang
> <xiaoguang.chen@intel.com>; Zhang, Tina <tina.zhang@intel.com>; Kirti
> Wankhede <kwankhede@nvidia.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>;
> intel-gvt-dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com>
> Subject: Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf
> operations
> 
>   Hi,
> 
> > Hmm, I don't like that interface.  Can you cite examples of other
> > ioctls that behave this way?  It doesn't feel like an elegant user
> > interface; the user can get the dmabuf, but only after they query the
> > dmabuf, even though the get-dmabuf ioctl returns the same data as the
> > query-plane ioctl, but they can't get the dmabuf if the plane has
> > changed in the interim, which is not something the user can know.  Are
> > we causing our own problems with this model of cycling through dmabuf
> > fds?  We talked previously about an enum of plane types, primary and
> > cursor.  What if the user was simply able to get a dmabuf fd for each
> > of those and they queried the current plane information via those fds?
> > IOW, the fd is persistent and specific to a given plane type, but the
> > format within it is dynamic.
> 
> Will not work due to how dma-bufs are designed.
> 
> But, yes, the QUERY then GET split is ugly for a number of reasons.
> 
> Does gvt track the live cycle of all dma-bufs it has handed out?
The V9 implementation does track the dma-bufs' live cycle. The original idea was that leaving the dma-bufs' live cycle management to user mode.

> If so, then maybe we can let the kernel check whenever a dma-buf for the
> current plane exists?  And if that isn't the case hand out a dma- buf right away,
> without expecting userspace explicitly asking for it?
I think this is a good advice. We are going to try this idea and add some tracking logic to kernel mode.

> 
> That will simplify the interface and remove the race condition at the expense of
> some additional bookkeeping in the kernel.
In this case, maybe one ioctl like QUERY_PLAN is enough. We can block it this ioctl and return it when the fd and info are ready.

Thanks.
Tina


> 
> cheers,
>   Gerd
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

[toc] | [prev] | [next] | [standalone]

#1677441 — Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

  Hi,

> > Does gvt track the live cycle of all dma-bufs it has handed out?
> 
> The V9 implementation does track the dma-bufs' live cycle. The
> original idea was that leaving the dma-bufs' live cycle management to
> user mode.

That is still the case, user space decides which dma-bufs it'll go keep
cached.  But kernel space can see what user space is doing, so there is
no need to explicitly tell the kernel whenever a cached dma-buf exists
or not.

cheers,
  Gerd

[toc] | [prev] | [next] | [standalone]

#1677522 — Re: [Intel-gfx] [PATCH v9 5/7] vfio: Define vfio based dma-buf operations

On Thu, Jun 29, 2017 at 08:41:53AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > Does gvt track the live cycle of all dma-bufs it has handed out?
> > 
> > The V9 implementation does track the dma-bufs' live cycle. The
> > original idea was that leaving the dma-bufs' live cycle management to
> > user mode.
> 
> That is still the case, user space decides which dma-bufs it'll go keep
> cached.  But kernel space can see what user space is doing, so there is
> no need to explicitly tell the kernel whenever a cached dma-buf exists
> or not.

We do the same trick in drm_prime.c, keeping a cache of exported dma-buf
around for re-exporting. Since for prime sharing the use-case is almost
always re-importing as a drm gem buffer again we can then on re-import
also tell userspace whether it already has that buffer in it's userspace
buffer manager, but that's an additional optimization. With plain dma-buf
we could achieve the same by wiring up a real stat() implementation with
unique inode numbers (atm they all share the anon_inode singleton). But
thus far no one asked for that.

btw I'm lost a bit in the discussion (was on vacation), but I think all
the concerns I've noticed with the initial rfc have been raised already,
so things look good. I'll check the next rfc once that shows up.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web