Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> Or more simply just pass the plane id, because even the plane description did 
> not match the current one we will eventually create a dmabuf based on current 
> plane.

That is the current behavior.

Works as long as we return the plane description too, so userspace knows
what it actually got.

cheers,
  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> Or more simply just pass the plane id, because even the plane description did 
> not match the current one we will eventually create a dmabuf based on current 
> plane.

That is the current behavior.

Works as long as we return the plane description too, so userspace knows
what it actually got.

cheers,
  Gerd

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Gerd,

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Gerd Hoffmann
>Sent: Friday, May 19, 2017 4:57 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; linux-kernel@vger.kernel.org;
>zhen...@linux.intel.com; Alex Williamson <alex.william...@redhat.com>; Lv,
>Zhiyuan <zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi
>A <zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>  Hi,
>
>> >We could also do it the other way around:  Instead of having the
>> >kernel returning
>>
>> >the plane description userspace could pass it in, and the kernel
>> >throws -EINVAL in
>>
>> >case it doesn't match due to things having changed meanwhile.
>>
>> Or just return a dmabuf  based on the current plane ?
>
>If gvt is able to hand out dma-bufs for inactive planes, then yes, we can do 
>this.
>Have one ioctl where we pass in the plane id, get back a plane description 
>struct,
>and another where we pass in the plane description struct and get back a dma-
>buf fd.
Or more simply just pass the plane id, because even the plane description did 
not match the current one we will eventually create a dmabuf based on current 
plane.

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

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Gerd,

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Gerd Hoffmann
>Sent: Friday, May 19, 2017 4:57 PM
>To: Chen, Xiaoguang 
>Cc: Tian, Kevin ; linux-kernel@vger.kernel.org;
>zhen...@linux.intel.com; Alex Williamson ; Lv,
>Zhiyuan ; intel-gvt-...@lists.freedesktop.org; Wang, Zhi
>A 
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>  Hi,
>
>> >We could also do it the other way around:  Instead of having the
>> >kernel returning
>>
>> >the plane description userspace could pass it in, and the kernel
>> >throws -EINVAL in
>>
>> >case it doesn't match due to things having changed meanwhile.
>>
>> Or just return a dmabuf  based on the current plane ?
>
>If gvt is able to hand out dma-bufs for inactive planes, then yes, we can do 
>this.
>Have one ioctl where we pass in the plane id, get back a plane description 
>struct,
>and another where we pass in the plane description struct and get back a dma-
>buf fd.
Or more simply just pass the plane id, because even the plane description did 
not match the current one we will eventually create a dmabuf based on current 
plane.

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

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> >We could also do it the other way around:  Instead of having the kernel 
> >returning
> 
> >the plane description userspace could pass it in, and the kernel throws 
> >-EINVAL in
> 
> >case it doesn't match due to things having changed meanwhile.
> 
> Or just return a dmabuf  based on the current plane ?

If gvt is able to hand out dma-bufs for inactive planes, then yes, we
can do this.  Have one ioctl where we pass in the plane id, get back a
plane description struct, and another where we pass in the plane
description struct and get back a dma-buf fd.

cheers,
  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> >We could also do it the other way around:  Instead of having the kernel 
> >returning
> 
> >the plane description userspace could pass it in, and the kernel throws 
> >-EINVAL in
> 
> >case it doesn't match due to things having changed meanwhile.
> 
> Or just return a dmabuf  based on the current plane ?

If gvt is able to hand out dma-bufs for inactive planes, then yes, we
can do this.  Have one ioctl where we pass in the plane id, get back a
plane description struct, and another where we pass in the plane
description struct and get back a dma-buf fd.

cheers,
  Gerd

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Gerd and Alex,

>-Original Message-
>From: Gerd Hoffmann [mailto:kra...@redhat.com]
>Sent: Friday, May 19, 2017 4:05 PM
>To: Alex Williamson <alex.william...@redhat.com>
>Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>; linux-kernel@vger.kernel.org; zhen...@linux.intel.com;
>Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang,
>Zhi A <zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>  Hi,
>
>> > User space need to check whether there's a dmabuf for the plane(user space
>usually cached two or three dmabuf to handle double buffer or triple buffer
>situation) only there's no dmabuf for the plane we will create a dmabuf for
>it(another ioctl).
>>
>> If our ioctls are "Query current plane" and "Give me a dmabuf for
>> current plane", isn't that racey?  The current plane could have
>> changed between those two calls so the user doesn't absolutely know
>> which plane the dmabuf retrieved is for.  The "Give me a dmabuf"
>> therefore needs to take some sort of plane index so the user can
>> request a specific plane.
>
>The "give me a dmabuf" ioctl returns the plane description too, so userspace 
>can
>at least figure it did hit the race window.
>
>We could also do it the other way around:  Instead of having the kernel 
>returning
>the plane description userspace could pass it in, and the kernel throws 
>-EINVAL in
>case it doesn't match due to things having changed meanwhile.
Or just return a dmabuf  based on the current plane ?
Because if user got -EINVAL while ioctl "Give me a dmabuf" user should do the 
ioctl again.

>
>cheers,
>  Gerd

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Gerd and Alex,

>-Original Message-
>From: Gerd Hoffmann [mailto:kra...@redhat.com]
>Sent: Friday, May 19, 2017 4:05 PM
>To: Alex Williamson 
>Cc: Chen, Xiaoguang ; Tian, Kevin
>; linux-kernel@vger.kernel.org; zhen...@linux.intel.com;
>Lv, Zhiyuan ; intel-gvt-...@lists.freedesktop.org; Wang,
>Zhi A 
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>  Hi,
>
>> > User space need to check whether there's a dmabuf for the plane(user space
>usually cached two or three dmabuf to handle double buffer or triple buffer
>situation) only there's no dmabuf for the plane we will create a dmabuf for
>it(another ioctl).
>>
>> If our ioctls are "Query current plane" and "Give me a dmabuf for
>> current plane", isn't that racey?  The current plane could have
>> changed between those two calls so the user doesn't absolutely know
>> which plane the dmabuf retrieved is for.  The "Give me a dmabuf"
>> therefore needs to take some sort of plane index so the user can
>> request a specific plane.
>
>The "give me a dmabuf" ioctl returns the plane description too, so userspace 
>can
>at least figure it did hit the race window.
>
>We could also do it the other way around:  Instead of having the kernel 
>returning
>the plane description userspace could pass it in, and the kernel throws 
>-EINVAL in
>case it doesn't match due to things having changed meanwhile.
Or just return a dmabuf  based on the current plane ?
Because if user got -EINVAL while ioctl "Give me a dmabuf" user should do the 
ioctl again.

>
>cheers,
>  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> > User space need to check whether there's a dmabuf for the plane(user space 
> > usually cached two or three dmabuf to handle double buffer or triple buffer 
> > situation) only there's no dmabuf for the plane we will create a dmabuf for 
> > it(another ioctl).
> 
> If our ioctls are "Query current plane" and "Give me a dmabuf for
> current plane", isn't that racey?  The current plane could have changed
> between those two calls so the user doesn't absolutely know which plane
> the dmabuf retrieved is for.  The "Give me a dmabuf" therefore needs to
> take some sort of plane index so the user can request a specific
> plane.

The "give me a dmabuf" ioctl returns the plane description too, so
userspace can at least figure it did hit the race window.

We could also do it the other way around:  Instead of having the kernel
returning the plane description userspace could pass it in, and the
kernel throws -EINVAL in case it doesn't match due to things having
changed meanwhile.

cheers,
  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> > User space need to check whether there's a dmabuf for the plane(user space 
> > usually cached two or three dmabuf to handle double buffer or triple buffer 
> > situation) only there's no dmabuf for the plane we will create a dmabuf for 
> > it(another ioctl).
> 
> If our ioctls are "Query current plane" and "Give me a dmabuf for
> current plane", isn't that racey?  The current plane could have changed
> between those two calls so the user doesn't absolutely know which plane
> the dmabuf retrieved is for.  The "Give me a dmabuf" therefore needs to
> take some sort of plane index so the user can request a specific
> plane.

The "give me a dmabuf" ioctl returns the plane description too, so
userspace can at least figure it did hit the race window.

We could also do it the other way around:  Instead of having the kernel
returning the plane description userspace could pass it in, and the
kernel throws -EINVAL in case it doesn't match due to things having
changed meanwhile.

cheers,
  Gerd

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Thursday, May 18, 2017 10:56 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin <kevin.t...@intel.com>;
>linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
><zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Thu, 18 May 2017 01:51:38 +
>"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>
>> Hi Alex,
>>
>> >-Original Message-
>> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >Sent: Thursday, May 18, 2017 5:44 AM
>> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>> >Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin
>> ><kevin.t...@intel.com>; linux-kernel@vger.kernel.org;
>> >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>;
>> >intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
>> ><zhi.a.w...@intel.com>
>> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>> >dmabuf
>> >
>> >On Tue, 16 May 2017 10:16:28 +
>> >"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>> >
>> >> Hi Alex,
>> >>
>> >> >-Original Message-
>> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >> >Sent: Tuesday, May 16, 2017 1:44 AM
>> >> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>> >> >Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin
>> >> ><kevin.t...@intel.com>; intel-...@lists.freedesktop.org;
>> >> >linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> >> ><zhiyuan...@intel.com>; intel-gvt- d...@lists.freedesktop.org;
>> >> >Wang, Zhi A <zhi.a.w...@intel.com>
>> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting
>> >> >the dmabuf
>> >> >
>> >> >On Mon, 15 May 2017 03:36:50 + "Chen, Xiaoguang"
>> >> ><xiaoguang.c...@intel.com> wrote:
>> >> >
>> >> >> Hi Alex and Gerd,
>> >> >>
>> >> >> >-Original Message-
>> >> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >> >> >Sent: Saturday, May 13, 2017 12:38 AM
>> >> >> >To: Gerd Hoffmann <kra...@redhat.com>
>> >> >> >Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Tian, Kevin
>> >> >> ><kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
>> >> >> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> >> >> ><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org;
>> >> >> >Wang, Zhi A <zhi.a.w...@intel.com>
>> >> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting
>> >> >> >the dmabuf
>> >> >> >
>> >> >> >On Fri, 12 May 2017 11:12:05 +0200 Gerd Hoffmann
>> >> >> ><kra...@redhat.com> wrote:
>> >> >> >
>> >> >> >>   Hi,
>> >> >> >>
>> >> >> >> > If the contents of the framebuffer change or if the
>> >> >> >> > parameters of the framebuffer change?  I can't image that
>> >> >> >> > creating a new dmabuf fd for every visual change within the
>> >> >> >> > framebuffer would be efficient, but I don't have any
>> >> >> >> > concept of what a dmabuf actually
>> >does.
>> >> >> >>
>> >> >> >> Ok, some background:
>> >> >> >>
>> >> >> >> The drm subsystem has the concept of planes.  The most
>> >> >> >> important plane is the primary framebuffer (i.e. what gets
>> >> >> >> scanned out to the physical display).  The cursor is a plane
>> >> >> >> too, and there can be additional overlay planes for stuff like video
>playback.
>> >> >> >>
>> >> >> >> Typically there are multiple planes in a system and only one
>> >> >> >>

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Thursday, May 18, 2017 10:56 PM
>To: Chen, Xiaoguang 
>Cc: Gerd Hoffmann ; Tian, Kevin ;
>linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Thu, 18 May 2017 01:51:38 +
>"Chen, Xiaoguang"  wrote:
>
>> Hi Alex,
>>
>> >-Original Message-
>> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >Sent: Thursday, May 18, 2017 5:44 AM
>> >To: Chen, Xiaoguang 
>> >Cc: Gerd Hoffmann ; Tian, Kevin
>> >; linux-kernel@vger.kernel.org;
>> >zhen...@linux.intel.com; Lv, Zhiyuan ;
>> >intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
>> >
>> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>> >dmabuf
>> >
>> >On Tue, 16 May 2017 10:16:28 +
>> >"Chen, Xiaoguang"  wrote:
>> >
>> >> Hi Alex,
>> >>
>> >> >-Original Message-
>> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >> >Sent: Tuesday, May 16, 2017 1:44 AM
>> >> >To: Chen, Xiaoguang 
>> >> >Cc: Gerd Hoffmann ; Tian, Kevin
>> >> >; intel-...@lists.freedesktop.org;
>> >> >linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> >> >; intel-gvt- d...@lists.freedesktop.org;
>> >> >Wang, Zhi A 
>> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting
>> >> >the dmabuf
>> >> >
>> >> >On Mon, 15 May 2017 03:36:50 + "Chen, Xiaoguang"
>> >> > wrote:
>> >> >
>> >> >> Hi Alex and Gerd,
>> >> >>
>> >> >> >-Original Message-
>> >> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >> >> >Sent: Saturday, May 13, 2017 12:38 AM
>> >> >> >To: Gerd Hoffmann 
>> >> >> >Cc: Chen, Xiaoguang ; Tian, Kevin
>> >> >> >; intel-...@lists.freedesktop.org; linux-
>> >> >> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> >> >> >; intel-gvt-...@lists.freedesktop.org;
>> >> >> >Wang, Zhi A 
>> >> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting
>> >> >> >the dmabuf
>> >> >> >
>> >> >> >On Fri, 12 May 2017 11:12:05 +0200 Gerd Hoffmann
>> >> >> > wrote:
>> >> >> >
>> >> >> >>   Hi,
>> >> >> >>
>> >> >> >> > If the contents of the framebuffer change or if the
>> >> >> >> > parameters of the framebuffer change?  I can't image that
>> >> >> >> > creating a new dmabuf fd for every visual change within the
>> >> >> >> > framebuffer would be efficient, but I don't have any
>> >> >> >> > concept of what a dmabuf actually
>> >does.
>> >> >> >>
>> >> >> >> Ok, some background:
>> >> >> >>
>> >> >> >> The drm subsystem has the concept of planes.  The most
>> >> >> >> important plane is the primary framebuffer (i.e. what gets
>> >> >> >> scanned out to the physical display).  The cursor is a plane
>> >> >> >> too, and there can be additional overlay planes for stuff like video
>playback.
>> >> >> >>
>> >> >> >> Typically there are multiple planes in a system and only one
>> >> >> >> of them gets scanned out to the crtc, i.e. the fbdev
>> >> >> >> emulation creates one plane for the framebuffer console.  The
>> >> >> >> X-Server creates a plane too, and when you switch between
>> >> >> >> X-Server and framebuffer console via ctrl-alt-fn the intel
>> >> >> >> driver just reprograms the encoder to scan out the one or the other
>plane to the crtc.
>> >> >> >>
>> >> >> >> The dma-buf handed out by gvt is a reference to a plane.  I
>> >> >> >> think on the host side gvt can see only the active plane
>> >> >>

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Thu, 18 May 2017 01:51:38 +
"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:

> Hi Alex,
> 
> >-Original Message-
> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >Sent: Thursday, May 18, 2017 5:44 AM
> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
> >Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin <kevin.t...@intel.com>;
> >linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> ><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
> ><zhi.a.w...@intel.com>
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >On Tue, 16 May 2017 10:16:28 +
> >"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
> >  
> >> Hi Alex,
> >>  
> >> >-Original Message-
> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >> >Sent: Tuesday, May 16, 2017 1:44 AM
> >> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
> >> >Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin
> >> ><kevin.t...@intel.com>; intel-...@lists.freedesktop.org;
> >> >linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> >> ><zhiyuan...@intel.com>; intel-gvt- d...@lists.freedesktop.org; Wang,
> >> >Zhi A <zhi.a.w...@intel.com>
> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
> >> >dmabuf
> >> >
> >> >On Mon, 15 May 2017 03:36:50 +
> >> >"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
> >> >  
> >> >> Hi Alex and Gerd,
> >> >>  
> >> >> >-Original Message-
> >> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >> >> >Sent: Saturday, May 13, 2017 12:38 AM
> >> >> >To: Gerd Hoffmann <kra...@redhat.com>
> >> >> >Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Tian, Kevin
> >> >> ><kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
> >> >> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> >> >> ><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang,
> >> >> >Zhi A <zhi.a.w...@intel.com>
> >> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting
> >> >> >the dmabuf
> >> >> >
> >> >> >On Fri, 12 May 2017 11:12:05 +0200 Gerd Hoffmann
> >> >> ><kra...@redhat.com> wrote:
> >> >> >  
> >> >> >>   Hi,
> >> >> >>  
> >> >> >> > If the contents of the framebuffer change or if the parameters
> >> >> >> > of the framebuffer change?  I can't image that creating a new
> >> >> >> > dmabuf fd for every visual change within the framebuffer would
> >> >> >> > be efficient, but I don't have any concept of what a dmabuf 
> >> >> >> > actually  
> >does.  
> >> >> >>
> >> >> >> Ok, some background:
> >> >> >>
> >> >> >> The drm subsystem has the concept of planes.  The most important
> >> >> >> plane is the primary framebuffer (i.e. what gets scanned out to
> >> >> >> the physical display).  The cursor is a plane too, and there can
> >> >> >> be additional overlay planes for stuff like video playback.
> >> >> >>
> >> >> >> Typically there are multiple planes in a system and only one of
> >> >> >> them gets scanned out to the crtc, i.e. the fbdev emulation
> >> >> >> creates one plane for the framebuffer console.  The X-Server
> >> >> >> creates a plane too, and when you switch between X-Server and
> >> >> >> framebuffer console via ctrl-alt-fn the intel driver just
> >> >> >> reprograms the encoder to scan out the one or the other plane to the 
> >> >> >> crtc.
> >> >> >>
> >> >> >> The dma-buf handed out by gvt is a reference to a plane.  I
> >> >> >> think on the host side gvt can see only the active plane (from
> >> >> >> encoder/crtc register
> >> >> >> programming) not the inactive ones.
> >> >> >>
> >> >> >

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Thu, 18 May 2017 01:51:38 +
"Chen, Xiaoguang"  wrote:

> Hi Alex,
> 
> >-Original Message-
> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >Sent: Thursday, May 18, 2017 5:44 AM
> >To: Chen, Xiaoguang 
> >Cc: Gerd Hoffmann ; Tian, Kevin ;
> >linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> >; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
> >
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >On Tue, 16 May 2017 10:16:28 +
> >"Chen, Xiaoguang"  wrote:
> >  
> >> Hi Alex,
> >>  
> >> >-Original Message-
> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >> >Sent: Tuesday, May 16, 2017 1:44 AM
> >> >To: Chen, Xiaoguang 
> >> >Cc: Gerd Hoffmann ; Tian, Kevin
> >> >; intel-...@lists.freedesktop.org;
> >> >linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> >> >; intel-gvt- d...@lists.freedesktop.org; Wang,
> >> >Zhi A 
> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
> >> >dmabuf
> >> >
> >> >On Mon, 15 May 2017 03:36:50 +
> >> >"Chen, Xiaoguang"  wrote:
> >> >  
> >> >> Hi Alex and Gerd,
> >> >>  
> >> >> >-Original Message-
> >> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >> >> >Sent: Saturday, May 13, 2017 12:38 AM
> >> >> >To: Gerd Hoffmann 
> >> >> >Cc: Chen, Xiaoguang ; Tian, Kevin
> >> >> >; intel-...@lists.freedesktop.org; linux-
> >> >> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> >> >> >; intel-gvt-...@lists.freedesktop.org; Wang,
> >> >> >Zhi A 
> >> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting
> >> >> >the dmabuf
> >> >> >
> >> >> >On Fri, 12 May 2017 11:12:05 +0200 Gerd Hoffmann
> >> >> > wrote:
> >> >> >  
> >> >> >>   Hi,
> >> >> >>  
> >> >> >> > If the contents of the framebuffer change or if the parameters
> >> >> >> > of the framebuffer change?  I can't image that creating a new
> >> >> >> > dmabuf fd for every visual change within the framebuffer would
> >> >> >> > be efficient, but I don't have any concept of what a dmabuf 
> >> >> >> > actually  
> >does.  
> >> >> >>
> >> >> >> Ok, some background:
> >> >> >>
> >> >> >> The drm subsystem has the concept of planes.  The most important
> >> >> >> plane is the primary framebuffer (i.e. what gets scanned out to
> >> >> >> the physical display).  The cursor is a plane too, and there can
> >> >> >> be additional overlay planes for stuff like video playback.
> >> >> >>
> >> >> >> Typically there are multiple planes in a system and only one of
> >> >> >> them gets scanned out to the crtc, i.e. the fbdev emulation
> >> >> >> creates one plane for the framebuffer console.  The X-Server
> >> >> >> creates a plane too, and when you switch between X-Server and
> >> >> >> framebuffer console via ctrl-alt-fn the intel driver just
> >> >> >> reprograms the encoder to scan out the one or the other plane to the 
> >> >> >> crtc.
> >> >> >>
> >> >> >> The dma-buf handed out by gvt is a reference to a plane.  I
> >> >> >> think on the host side gvt can see only the active plane (from
> >> >> >> encoder/crtc register
> >> >> >> programming) not the inactive ones.
> >> >> >>
> >> >> >> The dma-buf can be imported as opengl texture and then be used
> >> >> >> to render the guest display to a host window.  I think it is
> >> >> >> even possible to use the dma-buf as plane in the host drm driver
> >> >> >> and scan it out directly to a physical display.  The actual
> >> >> >> framebuffer content stays in gpu memory all the time, the cpu never 
> >> >> >> has  
> >to touch it.  
> >> >> >&g

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> > +static long intel_vgpu_dmabuf_mgr_fd_ioctl(struct file *filp,
> > +   unsigned int ioctl, unsigned long arg)
> > +{
> > +   struct intel_vgpu *vgpu = filp->private_data;
> > +   int minsz;
> > +   struct intel_vgpu_dmabuf dmabuf;
> > +   int ret;
> > +   struct fd f;
> > +   f = fdget(dmabuf.fd);
> > +   minsz = offsetofend(struct intel_vgpu_dmabuf, tiled);
> > +   if (copy_from_user(, (void __user *)arg, minsz))
> > +   return -EFAULT;
> > +   if (ioctl == INTEL_VGPU_QUERY_DMABUF)
> > +   ret = intel_gvt_ops->vgpu_query_dmabuf(vgpu, );
> > +   else if (ioctl == INTEL_VGPU_GENERATE_DMABUF)
> > +   ret = intel_gvt_ops->vgpu_generate_dmabuf(vgpu, );
> 
> Why do we need vendor specific ioctls here?  Aren't querying the
> current plane and getting an fd for that plane very generic concepts? 
> Is the resulting dmabuf Intel specific?

The dmabuf certainly isn't, and I think the ioctl interface can easily
be made pretty generic too.  The struct intel_vgpu_dmabuf hasn't much
intel-specific bits in there.  The only thing is the tiled bool, and I
think that can be replaced with a modifier code field (see
fourcc_mod_code() in drm/drm_fourcc.h).

Apparently the amd guys are working on vcpu support too, but using sriov
instead of mdev (saw this in the news only, have no more details).  They
can possibly support such an interface too.

cheers,
  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> > +static long intel_vgpu_dmabuf_mgr_fd_ioctl(struct file *filp,
> > +   unsigned int ioctl, unsigned long arg)
> > +{
> > +   struct intel_vgpu *vgpu = filp->private_data;
> > +   int minsz;
> > +   struct intel_vgpu_dmabuf dmabuf;
> > +   int ret;
> > +   struct fd f;
> > +   f = fdget(dmabuf.fd);
> > +   minsz = offsetofend(struct intel_vgpu_dmabuf, tiled);
> > +   if (copy_from_user(, (void __user *)arg, minsz))
> > +   return -EFAULT;
> > +   if (ioctl == INTEL_VGPU_QUERY_DMABUF)
> > +   ret = intel_gvt_ops->vgpu_query_dmabuf(vgpu, );
> > +   else if (ioctl == INTEL_VGPU_GENERATE_DMABUF)
> > +   ret = intel_gvt_ops->vgpu_generate_dmabuf(vgpu, );
> 
> Why do we need vendor specific ioctls here?  Aren't querying the
> current plane and getting an fd for that plane very generic concepts? 
> Is the resulting dmabuf Intel specific?

The dmabuf certainly isn't, and I think the ioctl interface can easily
be made pretty generic too.  The struct intel_vgpu_dmabuf hasn't much
intel-specific bits in there.  The only thing is the tiled bool, and I
think that can be replaced with a modifier code field (see
fourcc_mod_code() in drm/drm_fourcc.h).

Apparently the amd guys are working on vcpu support too, but using sriov
instead of mdev (saw this in the news only, have no more details).  They
can possibly support such an interface too.

cheers,
  Gerd

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex,

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Thursday, May 18, 2017 5:44 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin <kevin.t...@intel.com>;
>linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
><zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Tue, 16 May 2017 10:16:28 +
>"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>
>> Hi Alex,
>>
>> >-Original Message-
>> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >Sent: Tuesday, May 16, 2017 1:44 AM
>> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>> >Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin
>> ><kevin.t...@intel.com>; intel-...@lists.freedesktop.org;
>> >linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> ><zhiyuan...@intel.com>; intel-gvt- d...@lists.freedesktop.org; Wang,
>> >Zhi A <zhi.a.w...@intel.com>
>> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>> >dmabuf
>> >
>> >On Mon, 15 May 2017 03:36:50 +
>> >"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>> >
>> >> Hi Alex and Gerd,
>> >>
>> >> >-Original Message-
>> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >> >Sent: Saturday, May 13, 2017 12:38 AM
>> >> >To: Gerd Hoffmann <kra...@redhat.com>
>> >> >Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Tian, Kevin
>> >> ><kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
>> >> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> >> ><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang,
>> >> >Zhi A <zhi.a.w...@intel.com>
>> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting
>> >> >the dmabuf
>> >> >
>> >> >On Fri, 12 May 2017 11:12:05 +0200 Gerd Hoffmann
>> >> ><kra...@redhat.com> wrote:
>> >> >
>> >> >>   Hi,
>> >> >>
>> >> >> > If the contents of the framebuffer change or if the parameters
>> >> >> > of the framebuffer change?  I can't image that creating a new
>> >> >> > dmabuf fd for every visual change within the framebuffer would
>> >> >> > be efficient, but I don't have any concept of what a dmabuf actually
>does.
>> >> >>
>> >> >> Ok, some background:
>> >> >>
>> >> >> The drm subsystem has the concept of planes.  The most important
>> >> >> plane is the primary framebuffer (i.e. what gets scanned out to
>> >> >> the physical display).  The cursor is a plane too, and there can
>> >> >> be additional overlay planes for stuff like video playback.
>> >> >>
>> >> >> Typically there are multiple planes in a system and only one of
>> >> >> them gets scanned out to the crtc, i.e. the fbdev emulation
>> >> >> creates one plane for the framebuffer console.  The X-Server
>> >> >> creates a plane too, and when you switch between X-Server and
>> >> >> framebuffer console via ctrl-alt-fn the intel driver just
>> >> >> reprograms the encoder to scan out the one or the other plane to the 
>> >> >> crtc.
>> >> >>
>> >> >> The dma-buf handed out by gvt is a reference to a plane.  I
>> >> >> think on the host side gvt can see only the active plane (from
>> >> >> encoder/crtc register
>> >> >> programming) not the inactive ones.
>> >> >>
>> >> >> The dma-buf can be imported as opengl texture and then be used
>> >> >> to render the guest display to a host window.  I think it is
>> >> >> even possible to use the dma-buf as plane in the host drm driver
>> >> >> and scan it out directly to a physical display.  The actual
>> >> >> framebuffer content stays in gpu memory all the time, the cpu never has
>to touch it.
>> >> >>
>> >> >> It is possible to cache the dma-buf handles, 

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex,

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Thursday, May 18, 2017 5:44 AM
>To: Chen, Xiaoguang 
>Cc: Gerd Hoffmann ; Tian, Kevin ;
>linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Tue, 16 May 2017 10:16:28 +
>"Chen, Xiaoguang"  wrote:
>
>> Hi Alex,
>>
>> >-Original Message-
>> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >Sent: Tuesday, May 16, 2017 1:44 AM
>> >To: Chen, Xiaoguang 
>> >Cc: Gerd Hoffmann ; Tian, Kevin
>> >; intel-...@lists.freedesktop.org;
>> >linux-kernel@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> >; intel-gvt- d...@lists.freedesktop.org; Wang,
>> >Zhi A 
>> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>> >dmabuf
>> >
>> >On Mon, 15 May 2017 03:36:50 +
>> >"Chen, Xiaoguang"  wrote:
>> >
>> >> Hi Alex and Gerd,
>> >>
>> >> >-Original Message-
>> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >> >Sent: Saturday, May 13, 2017 12:38 AM
>> >> >To: Gerd Hoffmann 
>> >> >Cc: Chen, Xiaoguang ; Tian, Kevin
>> >> >; intel-...@lists.freedesktop.org; linux-
>> >> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> >> >; intel-gvt-...@lists.freedesktop.org; Wang,
>> >> >Zhi A 
>> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting
>> >> >the dmabuf
>> >> >
>> >> >On Fri, 12 May 2017 11:12:05 +0200 Gerd Hoffmann
>> >> > wrote:
>> >> >
>> >> >>   Hi,
>> >> >>
>> >> >> > If the contents of the framebuffer change or if the parameters
>> >> >> > of the framebuffer change?  I can't image that creating a new
>> >> >> > dmabuf fd for every visual change within the framebuffer would
>> >> >> > be efficient, but I don't have any concept of what a dmabuf actually
>does.
>> >> >>
>> >> >> Ok, some background:
>> >> >>
>> >> >> The drm subsystem has the concept of planes.  The most important
>> >> >> plane is the primary framebuffer (i.e. what gets scanned out to
>> >> >> the physical display).  The cursor is a plane too, and there can
>> >> >> be additional overlay planes for stuff like video playback.
>> >> >>
>> >> >> Typically there are multiple planes in a system and only one of
>> >> >> them gets scanned out to the crtc, i.e. the fbdev emulation
>> >> >> creates one plane for the framebuffer console.  The X-Server
>> >> >> creates a plane too, and when you switch between X-Server and
>> >> >> framebuffer console via ctrl-alt-fn the intel driver just
>> >> >> reprograms the encoder to scan out the one or the other plane to the 
>> >> >> crtc.
>> >> >>
>> >> >> The dma-buf handed out by gvt is a reference to a plane.  I
>> >> >> think on the host side gvt can see only the active plane (from
>> >> >> encoder/crtc register
>> >> >> programming) not the inactive ones.
>> >> >>
>> >> >> The dma-buf can be imported as opengl texture and then be used
>> >> >> to render the guest display to a host window.  I think it is
>> >> >> even possible to use the dma-buf as plane in the host drm driver
>> >> >> and scan it out directly to a physical display.  The actual
>> >> >> framebuffer content stays in gpu memory all the time, the cpu never has
>to touch it.
>> >> >>
>> >> >> It is possible to cache the dma-buf handles, i.e. when the guest
>> >> >> boots you'll get the first for the fbcon plane, when the
>> >> >> x-server starts the second for the x-server framebuffer, and
>> >> >> when the user switches to the text console via ctrl-alt-fn you
>> >> >> can re-use the fbcon dma-buf you already have.
>> >> >>
>> >> >> The caching becomes more important for good performance when the
>> >> >> guest uses

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Tue, 16 May 2017 10:16:28 +
"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:

> Hi Alex,
> 
> >-Original Message-
> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >Sent: Tuesday, May 16, 2017 1:44 AM
> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
> >Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin <kevin.t...@intel.com>;
> >intel-...@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
> >d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >On Mon, 15 May 2017 03:36:50 +
> >"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
> >  
> >> Hi Alex and Gerd,
> >>  
> >> >-Original Message-
> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >> >Sent: Saturday, May 13, 2017 12:38 AM
> >> >To: Gerd Hoffmann <kra...@redhat.com>
> >> >Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Tian, Kevin
> >> ><kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
> >> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> >> ><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang,
> >> >Zhi A <zhi.a.w...@intel.com>
> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
> >> >dmabuf
> >> >
> >> >On Fri, 12 May 2017 11:12:05 +0200
> >> >Gerd Hoffmann <kra...@redhat.com> wrote:
> >> >  
> >> >>   Hi,
> >> >>  
> >> >> > If the contents of the framebuffer change or if the parameters of
> >> >> > the framebuffer change?  I can't image that creating a new dmabuf
> >> >> > fd for every visual change within the framebuffer would be
> >> >> > efficient, but I don't have any concept of what a dmabuf actually 
> >> >> > does.  
> >> >>
> >> >> Ok, some background:
> >> >>
> >> >> The drm subsystem has the concept of planes.  The most important
> >> >> plane is the primary framebuffer (i.e. what gets scanned out to the
> >> >> physical display).  The cursor is a plane too, and there can be
> >> >> additional overlay planes for stuff like video playback.
> >> >>
> >> >> Typically there are multiple planes in a system and only one of
> >> >> them gets scanned out to the crtc, i.e. the fbdev emulation creates
> >> >> one plane for the framebuffer console.  The X-Server creates a
> >> >> plane too, and when you switch between X-Server and framebuffer
> >> >> console via ctrl-alt-fn the intel driver just reprograms the
> >> >> encoder to scan out the one or the other plane to the crtc.
> >> >>
> >> >> The dma-buf handed out by gvt is a reference to a plane.  I think
> >> >> on the host side gvt can see only the active plane (from
> >> >> encoder/crtc register
> >> >> programming) not the inactive ones.
> >> >>
> >> >> The dma-buf can be imported as opengl texture and then be used to
> >> >> render the guest display to a host window.  I think it is even
> >> >> possible to use the dma-buf as plane in the host drm driver and
> >> >> scan it out directly to a physical display.  The actual framebuffer
> >> >> content stays in gpu memory all the time, the cpu never has to touch it.
> >> >>
> >> >> It is possible to cache the dma-buf handles, i.e. when the guest
> >> >> boots you'll get the first for the fbcon plane, when the x-server
> >> >> starts the second for the x-server framebuffer, and when the user
> >> >> switches to the text console via ctrl-alt-fn you can re-use the
> >> >> fbcon dma-buf you already have.
> >> >>
> >> >> The caching becomes more important for good performance when the
> >> >> guest uses pageflipping (wayland does): define two planes, render
> >> >> into one while displaying the other, then flip the two for a atomic
> >> >> display update.
> >> >>
> >> >> The caching also makes it a bit difficult to create a good interface.
> >> >> So, the current patch set creates:
> >>

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Tue, 16 May 2017 10:16:28 +
"Chen, Xiaoguang"  wrote:

> Hi Alex,
> 
> >-Original Message-
> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >Sent: Tuesday, May 16, 2017 1:44 AM
> >To: Chen, Xiaoguang 
> >Cc: Gerd Hoffmann ; Tian, Kevin ;
> >intel-...@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> >zhen...@linux.intel.com; Lv, Zhiyuan ; intel-gvt-
> >d...@lists.freedesktop.org; Wang, Zhi A 
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >On Mon, 15 May 2017 03:36:50 +
> >"Chen, Xiaoguang"  wrote:
> >  
> >> Hi Alex and Gerd,
> >>  
> >> >-Original Message-
> >> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >> >Sent: Saturday, May 13, 2017 12:38 AM
> >> >To: Gerd Hoffmann 
> >> >Cc: Chen, Xiaoguang ; Tian, Kevin
> >> >; intel-...@lists.freedesktop.org; linux-
> >> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> >> >; intel-gvt-...@lists.freedesktop.org; Wang,
> >> >Zhi A 
> >> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
> >> >dmabuf
> >> >
> >> >On Fri, 12 May 2017 11:12:05 +0200
> >> >Gerd Hoffmann  wrote:
> >> >  
> >> >>   Hi,
> >> >>  
> >> >> > If the contents of the framebuffer change or if the parameters of
> >> >> > the framebuffer change?  I can't image that creating a new dmabuf
> >> >> > fd for every visual change within the framebuffer would be
> >> >> > efficient, but I don't have any concept of what a dmabuf actually 
> >> >> > does.  
> >> >>
> >> >> Ok, some background:
> >> >>
> >> >> The drm subsystem has the concept of planes.  The most important
> >> >> plane is the primary framebuffer (i.e. what gets scanned out to the
> >> >> physical display).  The cursor is a plane too, and there can be
> >> >> additional overlay planes for stuff like video playback.
> >> >>
> >> >> Typically there are multiple planes in a system and only one of
> >> >> them gets scanned out to the crtc, i.e. the fbdev emulation creates
> >> >> one plane for the framebuffer console.  The X-Server creates a
> >> >> plane too, and when you switch between X-Server and framebuffer
> >> >> console via ctrl-alt-fn the intel driver just reprograms the
> >> >> encoder to scan out the one or the other plane to the crtc.
> >> >>
> >> >> The dma-buf handed out by gvt is a reference to a plane.  I think
> >> >> on the host side gvt can see only the active plane (from
> >> >> encoder/crtc register
> >> >> programming) not the inactive ones.
> >> >>
> >> >> The dma-buf can be imported as opengl texture and then be used to
> >> >> render the guest display to a host window.  I think it is even
> >> >> possible to use the dma-buf as plane in the host drm driver and
> >> >> scan it out directly to a physical display.  The actual framebuffer
> >> >> content stays in gpu memory all the time, the cpu never has to touch it.
> >> >>
> >> >> It is possible to cache the dma-buf handles, i.e. when the guest
> >> >> boots you'll get the first for the fbcon plane, when the x-server
> >> >> starts the second for the x-server framebuffer, and when the user
> >> >> switches to the text console via ctrl-alt-fn you can re-use the
> >> >> fbcon dma-buf you already have.
> >> >>
> >> >> The caching becomes more important for good performance when the
> >> >> guest uses pageflipping (wayland does): define two planes, render
> >> >> into one while displaying the other, then flip the two for a atomic
> >> >> display update.
> >> >>
> >> >> The caching also makes it a bit difficult to create a good interface.
> >> >> So, the current patch set creates:
> >> >>
> >> >>   (a) A way to query the active planes (ioctl
> >> >>   INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
> >> >>   (b) A way to create a dma-buf for the active plane (ioctl
> >> >>   INTEL_VGPU_GENERATE_DMABUF).
> >> >>
> >> >> Typical us

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex,

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Tuesday, May 16, 2017 1:44 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin <kevin.t...@intel.com>;
>intel-...@lists.freedesktop.org; linux-kernel@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Mon, 15 May 2017 03:36:50 +
>"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>
>> Hi Alex and Gerd,
>>
>> >-Original Message-
>> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >Sent: Saturday, May 13, 2017 12:38 AM
>> >To: Gerd Hoffmann <kra...@redhat.com>
>> >Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Tian, Kevin
>> ><kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
>> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> ><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang,
>> >Zhi A <zhi.a.w...@intel.com>
>> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>> >dmabuf
>> >
>> >On Fri, 12 May 2017 11:12:05 +0200
>> >Gerd Hoffmann <kra...@redhat.com> wrote:
>> >
>> >>   Hi,
>> >>
>> >> > If the contents of the framebuffer change or if the parameters of
>> >> > the framebuffer change?  I can't image that creating a new dmabuf
>> >> > fd for every visual change within the framebuffer would be
>> >> > efficient, but I don't have any concept of what a dmabuf actually does.
>> >>
>> >> Ok, some background:
>> >>
>> >> The drm subsystem has the concept of planes.  The most important
>> >> plane is the primary framebuffer (i.e. what gets scanned out to the
>> >> physical display).  The cursor is a plane too, and there can be
>> >> additional overlay planes for stuff like video playback.
>> >>
>> >> Typically there are multiple planes in a system and only one of
>> >> them gets scanned out to the crtc, i.e. the fbdev emulation creates
>> >> one plane for the framebuffer console.  The X-Server creates a
>> >> plane too, and when you switch between X-Server and framebuffer
>> >> console via ctrl-alt-fn the intel driver just reprograms the
>> >> encoder to scan out the one or the other plane to the crtc.
>> >>
>> >> The dma-buf handed out by gvt is a reference to a plane.  I think
>> >> on the host side gvt can see only the active plane (from
>> >> encoder/crtc register
>> >> programming) not the inactive ones.
>> >>
>> >> The dma-buf can be imported as opengl texture and then be used to
>> >> render the guest display to a host window.  I think it is even
>> >> possible to use the dma-buf as plane in the host drm driver and
>> >> scan it out directly to a physical display.  The actual framebuffer
>> >> content stays in gpu memory all the time, the cpu never has to touch it.
>> >>
>> >> It is possible to cache the dma-buf handles, i.e. when the guest
>> >> boots you'll get the first for the fbcon plane, when the x-server
>> >> starts the second for the x-server framebuffer, and when the user
>> >> switches to the text console via ctrl-alt-fn you can re-use the
>> >> fbcon dma-buf you already have.
>> >>
>> >> The caching becomes more important for good performance when the
>> >> guest uses pageflipping (wayland does): define two planes, render
>> >> into one while displaying the other, then flip the two for a atomic
>> >> display update.
>> >>
>> >> The caching also makes it a bit difficult to create a good interface.
>> >> So, the current patch set creates:
>> >>
>> >>   (a) A way to query the active planes (ioctl
>> >>   INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
>> >>   (b) A way to create a dma-buf for the active plane (ioctl
>> >>   INTEL_VGPU_GENERATE_DMABUF).
>> >>
>> >> Typical userspace workflow is to first query the plane, then check
>> >> if it already has a dma-buf for it, and if not create one.
>> >
>> >Thank you!  This is immensely he

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex,

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Tuesday, May 16, 2017 1:44 AM
>To: Chen, Xiaoguang 
>Cc: Gerd Hoffmann ; Tian, Kevin ;
>intel-...@lists.freedesktop.org; linux-kernel@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan ; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A 
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Mon, 15 May 2017 03:36:50 +
>"Chen, Xiaoguang"  wrote:
>
>> Hi Alex and Gerd,
>>
>> >-Original Message-
>> >From: Alex Williamson [mailto:alex.william...@redhat.com]
>> >Sent: Saturday, May 13, 2017 12:38 AM
>> >To: Gerd Hoffmann 
>> >Cc: Chen, Xiaoguang ; Tian, Kevin
>> >; intel-...@lists.freedesktop.org; linux-
>> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>> >; intel-gvt-...@lists.freedesktop.org; Wang,
>> >Zhi A 
>> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>> >dmabuf
>> >
>> >On Fri, 12 May 2017 11:12:05 +0200
>> >Gerd Hoffmann  wrote:
>> >
>> >>   Hi,
>> >>
>> >> > If the contents of the framebuffer change or if the parameters of
>> >> > the framebuffer change?  I can't image that creating a new dmabuf
>> >> > fd for every visual change within the framebuffer would be
>> >> > efficient, but I don't have any concept of what a dmabuf actually does.
>> >>
>> >> Ok, some background:
>> >>
>> >> The drm subsystem has the concept of planes.  The most important
>> >> plane is the primary framebuffer (i.e. what gets scanned out to the
>> >> physical display).  The cursor is a plane too, and there can be
>> >> additional overlay planes for stuff like video playback.
>> >>
>> >> Typically there are multiple planes in a system and only one of
>> >> them gets scanned out to the crtc, i.e. the fbdev emulation creates
>> >> one plane for the framebuffer console.  The X-Server creates a
>> >> plane too, and when you switch between X-Server and framebuffer
>> >> console via ctrl-alt-fn the intel driver just reprograms the
>> >> encoder to scan out the one or the other plane to the crtc.
>> >>
>> >> The dma-buf handed out by gvt is a reference to a plane.  I think
>> >> on the host side gvt can see only the active plane (from
>> >> encoder/crtc register
>> >> programming) not the inactive ones.
>> >>
>> >> The dma-buf can be imported as opengl texture and then be used to
>> >> render the guest display to a host window.  I think it is even
>> >> possible to use the dma-buf as plane in the host drm driver and
>> >> scan it out directly to a physical display.  The actual framebuffer
>> >> content stays in gpu memory all the time, the cpu never has to touch it.
>> >>
>> >> It is possible to cache the dma-buf handles, i.e. when the guest
>> >> boots you'll get the first for the fbcon plane, when the x-server
>> >> starts the second for the x-server framebuffer, and when the user
>> >> switches to the text console via ctrl-alt-fn you can re-use the
>> >> fbcon dma-buf you already have.
>> >>
>> >> The caching becomes more important for good performance when the
>> >> guest uses pageflipping (wayland does): define two planes, render
>> >> into one while displaying the other, then flip the two for a atomic
>> >> display update.
>> >>
>> >> The caching also makes it a bit difficult to create a good interface.
>> >> So, the current patch set creates:
>> >>
>> >>   (a) A way to query the active planes (ioctl
>> >>   INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
>> >>   (b) A way to create a dma-buf for the active plane (ioctl
>> >>   INTEL_VGPU_GENERATE_DMABUF).
>> >>
>> >> Typical userspace workflow is to first query the plane, then check
>> >> if it already has a dma-buf for it, and if not create one.
>> >
>> >Thank you!  This is immensely helpful!
>> >
>> >> > What changes to the framebuffer require a new dmabuf fd?
>> >> > Shouldn't the user query the parameters of the framebuffer
>> >> > through a dmabuf fd and shouldn't the dmabuf fd have some
>> >> > signaling mechanism to the user (eventfd perhaps) to not

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Mon, 15 May 2017 03:36:50 +
"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:

> Hi Alex and Gerd,
> 
> >-Original Message-
> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >Sent: Saturday, May 13, 2017 12:38 AM
> >To: Gerd Hoffmann <kra...@redhat.com>
> >Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Tian, Kevin
> ><kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> ><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
> ><zhi.a.w...@intel.com>
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >On Fri, 12 May 2017 11:12:05 +0200
> >Gerd Hoffmann <kra...@redhat.com> wrote:
> >  
> >>   Hi,
> >>  
> >> > If the contents of the framebuffer change or if the parameters of
> >> > the framebuffer change?  I can't image that creating a new dmabuf fd
> >> > for every visual change within the framebuffer would be efficient,
> >> > but I don't have any concept of what a dmabuf actually does.  
> >>
> >> Ok, some background:
> >>
> >> The drm subsystem has the concept of planes.  The most important plane
> >> is the primary framebuffer (i.e. what gets scanned out to the physical
> >> display).  The cursor is a plane too, and there can be additional
> >> overlay planes for stuff like video playback.
> >>
> >> Typically there are multiple planes in a system and only one of them
> >> gets scanned out to the crtc, i.e. the fbdev emulation creates one
> >> plane for the framebuffer console.  The X-Server creates a plane too,
> >> and when you switch between X-Server and framebuffer console via
> >> ctrl-alt-fn the intel driver just reprograms the encoder to scan out
> >> the one or the other plane to the crtc.
> >>
> >> The dma-buf handed out by gvt is a reference to a plane.  I think on
> >> the host side gvt can see only the active plane (from encoder/crtc
> >> register
> >> programming) not the inactive ones.
> >>
> >> The dma-buf can be imported as opengl texture and then be used to
> >> render the guest display to a host window.  I think it is even
> >> possible to use the dma-buf as plane in the host drm driver and scan
> >> it out directly to a physical display.  The actual framebuffer content
> >> stays in gpu memory all the time, the cpu never has to touch it.
> >>
> >> It is possible to cache the dma-buf handles, i.e. when the guest boots
> >> you'll get the first for the fbcon plane, when the x-server starts the
> >> second for the x-server framebuffer, and when the user switches to the
> >> text console via ctrl-alt-fn you can re-use the fbcon dma-buf you
> >> already have.
> >>
> >> The caching becomes more important for good performance when the guest
> >> uses pageflipping (wayland does): define two planes, render into one
> >> while displaying the other, then flip the two for a atomic display
> >> update.
> >>
> >> The caching also makes it a bit difficult to create a good interface.
> >> So, the current patch set creates:
> >>
> >>   (a) A way to query the active planes (ioctl
> >>   INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
> >>   (b) A way to create a dma-buf for the active plane (ioctl
> >>   INTEL_VGPU_GENERATE_DMABUF).
> >>
> >> Typical userspace workflow is to first query the plane, then check if
> >> it already has a dma-buf for it, and if not create one.  
> >
> >Thank you!  This is immensely helpful!
> >  
> >> > What changes to the framebuffer require a new dmabuf fd?  Shouldn't
> >> > the user query the parameters of the framebuffer through a dmabuf fd
> >> > and shouldn't the dmabuf fd have some signaling mechanism to the
> >> > user (eventfd perhaps) to notify the user to re-evaluate the parameters? 
> >> >  
> >>
> >> dma-bufs don't support that, they are really just a handle to a piece
> >> of memory, all metadata (format, size) most be communicated by other means.
> >>  
> >> > Otherwise are you imagining that the user polls the vfio region?  
> >>
> >> Hmm, notification support would probably a good reason to have a
> >> separate file handle to manage the dma-bufs (instead of using
> >> driver-specific ioctls on the vfio

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Mon, 15 May 2017 03:36:50 +
"Chen, Xiaoguang"  wrote:

> Hi Alex and Gerd,
> 
> >-Original Message-
> >From: Alex Williamson [mailto:alex.william...@redhat.com]
> >Sent: Saturday, May 13, 2017 12:38 AM
> >To: Gerd Hoffmann 
> >Cc: Chen, Xiaoguang ; Tian, Kevin
> >; intel-...@lists.freedesktop.org; linux-
> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> >; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
> >
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >On Fri, 12 May 2017 11:12:05 +0200
> >Gerd Hoffmann  wrote:
> >  
> >>   Hi,
> >>  
> >> > If the contents of the framebuffer change or if the parameters of
> >> > the framebuffer change?  I can't image that creating a new dmabuf fd
> >> > for every visual change within the framebuffer would be efficient,
> >> > but I don't have any concept of what a dmabuf actually does.  
> >>
> >> Ok, some background:
> >>
> >> The drm subsystem has the concept of planes.  The most important plane
> >> is the primary framebuffer (i.e. what gets scanned out to the physical
> >> display).  The cursor is a plane too, and there can be additional
> >> overlay planes for stuff like video playback.
> >>
> >> Typically there are multiple planes in a system and only one of them
> >> gets scanned out to the crtc, i.e. the fbdev emulation creates one
> >> plane for the framebuffer console.  The X-Server creates a plane too,
> >> and when you switch between X-Server and framebuffer console via
> >> ctrl-alt-fn the intel driver just reprograms the encoder to scan out
> >> the one or the other plane to the crtc.
> >>
> >> The dma-buf handed out by gvt is a reference to a plane.  I think on
> >> the host side gvt can see only the active plane (from encoder/crtc
> >> register
> >> programming) not the inactive ones.
> >>
> >> The dma-buf can be imported as opengl texture and then be used to
> >> render the guest display to a host window.  I think it is even
> >> possible to use the dma-buf as plane in the host drm driver and scan
> >> it out directly to a physical display.  The actual framebuffer content
> >> stays in gpu memory all the time, the cpu never has to touch it.
> >>
> >> It is possible to cache the dma-buf handles, i.e. when the guest boots
> >> you'll get the first for the fbcon plane, when the x-server starts the
> >> second for the x-server framebuffer, and when the user switches to the
> >> text console via ctrl-alt-fn you can re-use the fbcon dma-buf you
> >> already have.
> >>
> >> The caching becomes more important for good performance when the guest
> >> uses pageflipping (wayland does): define two planes, render into one
> >> while displaying the other, then flip the two for a atomic display
> >> update.
> >>
> >> The caching also makes it a bit difficult to create a good interface.
> >> So, the current patch set creates:
> >>
> >>   (a) A way to query the active planes (ioctl
> >>   INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
> >>   (b) A way to create a dma-buf for the active plane (ioctl
> >>   INTEL_VGPU_GENERATE_DMABUF).
> >>
> >> Typical userspace workflow is to first query the plane, then check if
> >> it already has a dma-buf for it, and if not create one.  
> >
> >Thank you!  This is immensely helpful!
> >  
> >> > What changes to the framebuffer require a new dmabuf fd?  Shouldn't
> >> > the user query the parameters of the framebuffer through a dmabuf fd
> >> > and shouldn't the dmabuf fd have some signaling mechanism to the
> >> > user (eventfd perhaps) to notify the user to re-evaluate the parameters? 
> >> >  
> >>
> >> dma-bufs don't support that, they are really just a handle to a piece
> >> of memory, all metadata (format, size) most be communicated by other means.
> >>  
> >> > Otherwise are you imagining that the user polls the vfio region?  
> >>
> >> Hmm, notification support would probably a good reason to have a
> >> separate file handle to manage the dma-bufs (instead of using
> >> driver-specific ioctls on the vfio fd), because the driver could also
> >> use the management fd for notifications then.  
> >
> >I like this idea of a separate control fd for dmabufs, it provides not on

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex and Gerd,

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Saturday, May 13, 2017 12:38 AM
>To: Gerd Hoffmann <kra...@redhat.com>
>Cc: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Tian, Kevin
><kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
><zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
><zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fri, 12 May 2017 11:12:05 +0200
>Gerd Hoffmann <kra...@redhat.com> wrote:
>
>>   Hi,
>>
>> > If the contents of the framebuffer change or if the parameters of
>> > the framebuffer change?  I can't image that creating a new dmabuf fd
>> > for every visual change within the framebuffer would be efficient,
>> > but I don't have any concept of what a dmabuf actually does.
>>
>> Ok, some background:
>>
>> The drm subsystem has the concept of planes.  The most important plane
>> is the primary framebuffer (i.e. what gets scanned out to the physical
>> display).  The cursor is a plane too, and there can be additional
>> overlay planes for stuff like video playback.
>>
>> Typically there are multiple planes in a system and only one of them
>> gets scanned out to the crtc, i.e. the fbdev emulation creates one
>> plane for the framebuffer console.  The X-Server creates a plane too,
>> and when you switch between X-Server and framebuffer console via
>> ctrl-alt-fn the intel driver just reprograms the encoder to scan out
>> the one or the other plane to the crtc.
>>
>> The dma-buf handed out by gvt is a reference to a plane.  I think on
>> the host side gvt can see only the active plane (from encoder/crtc
>> register
>> programming) not the inactive ones.
>>
>> The dma-buf can be imported as opengl texture and then be used to
>> render the guest display to a host window.  I think it is even
>> possible to use the dma-buf as plane in the host drm driver and scan
>> it out directly to a physical display.  The actual framebuffer content
>> stays in gpu memory all the time, the cpu never has to touch it.
>>
>> It is possible to cache the dma-buf handles, i.e. when the guest boots
>> you'll get the first for the fbcon plane, when the x-server starts the
>> second for the x-server framebuffer, and when the user switches to the
>> text console via ctrl-alt-fn you can re-use the fbcon dma-buf you
>> already have.
>>
>> The caching becomes more important for good performance when the guest
>> uses pageflipping (wayland does): define two planes, render into one
>> while displaying the other, then flip the two for a atomic display
>> update.
>>
>> The caching also makes it a bit difficult to create a good interface.
>> So, the current patch set creates:
>>
>>   (a) A way to query the active planes (ioctl
>>   INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
>>   (b) A way to create a dma-buf for the active plane (ioctl
>>   INTEL_VGPU_GENERATE_DMABUF).
>>
>> Typical userspace workflow is to first query the plane, then check if
>> it already has a dma-buf for it, and if not create one.
>
>Thank you!  This is immensely helpful!
>
>> > What changes to the framebuffer require a new dmabuf fd?  Shouldn't
>> > the user query the parameters of the framebuffer through a dmabuf fd
>> > and shouldn't the dmabuf fd have some signaling mechanism to the
>> > user (eventfd perhaps) to notify the user to re-evaluate the parameters?
>>
>> dma-bufs don't support that, they are really just a handle to a piece
>> of memory, all metadata (format, size) most be communicated by other means.
>>
>> > Otherwise are you imagining that the user polls the vfio region?
>>
>> Hmm, notification support would probably a good reason to have a
>> separate file handle to manage the dma-bufs (instead of using
>> driver-specific ioctls on the vfio fd), because the driver could also
>> use the management fd for notifications then.
>
>I like this idea of a separate control fd for dmabufs, it provides not only a 
>central
>management point, but also a nice abstraction for the vfio device specific
>interface.  We potentially only need a single
>VFIO_DEVICE_GET_DMABUF_MGR_FD() ioctl to get a dmabuf management fd
>(perhaps with a type parameter, ex. GFX) where maybe we could have vfio-core
>incorporate this reference into the group lifecycle, so the vendor driver only
>needs t

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex and Gerd,

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Saturday, May 13, 2017 12:38 AM
>To: Gerd Hoffmann 
>Cc: Chen, Xiaoguang ; Tian, Kevin
>; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>; intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fri, 12 May 2017 11:12:05 +0200
>Gerd Hoffmann  wrote:
>
>>   Hi,
>>
>> > If the contents of the framebuffer change or if the parameters of
>> > the framebuffer change?  I can't image that creating a new dmabuf fd
>> > for every visual change within the framebuffer would be efficient,
>> > but I don't have any concept of what a dmabuf actually does.
>>
>> Ok, some background:
>>
>> The drm subsystem has the concept of planes.  The most important plane
>> is the primary framebuffer (i.e. what gets scanned out to the physical
>> display).  The cursor is a plane too, and there can be additional
>> overlay planes for stuff like video playback.
>>
>> Typically there are multiple planes in a system and only one of them
>> gets scanned out to the crtc, i.e. the fbdev emulation creates one
>> plane for the framebuffer console.  The X-Server creates a plane too,
>> and when you switch between X-Server and framebuffer console via
>> ctrl-alt-fn the intel driver just reprograms the encoder to scan out
>> the one or the other plane to the crtc.
>>
>> The dma-buf handed out by gvt is a reference to a plane.  I think on
>> the host side gvt can see only the active plane (from encoder/crtc
>> register
>> programming) not the inactive ones.
>>
>> The dma-buf can be imported as opengl texture and then be used to
>> render the guest display to a host window.  I think it is even
>> possible to use the dma-buf as plane in the host drm driver and scan
>> it out directly to a physical display.  The actual framebuffer content
>> stays in gpu memory all the time, the cpu never has to touch it.
>>
>> It is possible to cache the dma-buf handles, i.e. when the guest boots
>> you'll get the first for the fbcon plane, when the x-server starts the
>> second for the x-server framebuffer, and when the user switches to the
>> text console via ctrl-alt-fn you can re-use the fbcon dma-buf you
>> already have.
>>
>> The caching becomes more important for good performance when the guest
>> uses pageflipping (wayland does): define two planes, render into one
>> while displaying the other, then flip the two for a atomic display
>> update.
>>
>> The caching also makes it a bit difficult to create a good interface.
>> So, the current patch set creates:
>>
>>   (a) A way to query the active planes (ioctl
>>   INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
>>   (b) A way to create a dma-buf for the active plane (ioctl
>>   INTEL_VGPU_GENERATE_DMABUF).
>>
>> Typical userspace workflow is to first query the plane, then check if
>> it already has a dma-buf for it, and if not create one.
>
>Thank you!  This is immensely helpful!
>
>> > What changes to the framebuffer require a new dmabuf fd?  Shouldn't
>> > the user query the parameters of the framebuffer through a dmabuf fd
>> > and shouldn't the dmabuf fd have some signaling mechanism to the
>> > user (eventfd perhaps) to notify the user to re-evaluate the parameters?
>>
>> dma-bufs don't support that, they are really just a handle to a piece
>> of memory, all metadata (format, size) most be communicated by other means.
>>
>> > Otherwise are you imagining that the user polls the vfio region?
>>
>> Hmm, notification support would probably a good reason to have a
>> separate file handle to manage the dma-bufs (instead of using
>> driver-specific ioctls on the vfio fd), because the driver could also
>> use the management fd for notifications then.
>
>I like this idea of a separate control fd for dmabufs, it provides not only a 
>central
>management point, but also a nice abstraction for the vfio device specific
>interface.  We potentially only need a single
>VFIO_DEVICE_GET_DMABUF_MGR_FD() ioctl to get a dmabuf management fd
>(perhaps with a type parameter, ex. GFX) where maybe we could have vfio-core
>incorporate this reference into the group lifecycle, so the vendor driver only
>needs to fdget/put this manager fd for the various plane dmabuf fds spawned in
>order to get core-level reference counting.
Following is my understanding of the managem

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Fri, 12 May 2017 06:56:03 +
"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:

> Hi Gerd,
> 
> >-Original Message-
> >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
> >Behalf Of Gerd Hoffmann
> >Sent: Thursday, May 11, 2017 9:28 PM
> >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
> >Cc: Tian, Kevin <kevin.t...@intel.com>; intel-...@lists.freedesktop.org; 
> >linux-
> >ker...@vger.kernel.org; zhen...@linux.intel.com; Alex Williamson
> ><alex.william...@redhat.com>; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
> >d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >  Hi,
> >  
> >> While read the framebuffer region we have to tell the vendor driver which  
> >framebuffer we want to read? There are two framebuffers now in KVMGT that is
> >primary and cursor.  
> >> There are two methods to implement this:
> >> 1) write the plane id first and then read the framebuffer.
> >> 2) create 2 vfio regions one for primary and one for cursor.  
> >
> >(3) Place information for both planes into one vfio region.
> >Which allows to fetch both with a single read() syscall.  
> That works too. Then using the ioctl to get the dmabuf fd if needed. And 
> plane id can be ioctl's parameter.
> 
> How about method 2 primary plane and cursor plane are different and should 
> generate different dmabuf for each of them.
> 
> >
> >The question is how you'll get the file descriptor then.  If the ioctl 
> >returns the
> >dma-buf fd only you have a racy interface:  Things can change between 
> >read(vfio-
> >region) and ioctl(need-dmabuf-fd).  
> You are right. So when creating the dmabuf we may have to decode the 
> framebuffer and create the dmabuf using the latest framebuffer information 
> and we must return the framebuffer information together with the dmabuf fd.
> 
> In the current implementation I saved the framebuffer information while user 
> querying the framebuffer and generate the dmabuf using the saved information 
> no error found yet but in theory there are sync problems.
> 
> >
> >ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix the 
> >race,
> >but then it is easier to go with ioctl only interface (simliar to the 
> >orginal one from
> >dec last year) I think.  
> Yes. ioctl works for it.
> But based on the mail last week. If I understand correctly Alex hope to query 
> the framebuffer information by reading the vfio device region and then get 
> the dmabuf fd using ioctl.

No, I was explaining that I had questioned whether we could use a vfio
region in place of a separate dmabuf fd.  We can't.  I have no
particular desire to use a vfio region just for querying framebuffer
info.  I prefer the dmabuf manager fd idea that Gerd suggested.  Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Fri, 12 May 2017 06:56:03 +
"Chen, Xiaoguang"  wrote:

> Hi Gerd,
> 
> >-Original Message-
> >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
> >Behalf Of Gerd Hoffmann
> >Sent: Thursday, May 11, 2017 9:28 PM
> >To: Chen, Xiaoguang 
> >Cc: Tian, Kevin ; intel-...@lists.freedesktop.org; 
> >linux-
> >ker...@vger.kernel.org; zhen...@linux.intel.com; Alex Williamson
> >; Lv, Zhiyuan ; intel-gvt-
> >d...@lists.freedesktop.org; Wang, Zhi A 
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >  Hi,
> >  
> >> While read the framebuffer region we have to tell the vendor driver which  
> >framebuffer we want to read? There are two framebuffers now in KVMGT that is
> >primary and cursor.  
> >> There are two methods to implement this:
> >> 1) write the plane id first and then read the framebuffer.
> >> 2) create 2 vfio regions one for primary and one for cursor.  
> >
> >(3) Place information for both planes into one vfio region.
> >Which allows to fetch both with a single read() syscall.  
> That works too. Then using the ioctl to get the dmabuf fd if needed. And 
> plane id can be ioctl's parameter.
> 
> How about method 2 primary plane and cursor plane are different and should 
> generate different dmabuf for each of them.
> 
> >
> >The question is how you'll get the file descriptor then.  If the ioctl 
> >returns the
> >dma-buf fd only you have a racy interface:  Things can change between 
> >read(vfio-
> >region) and ioctl(need-dmabuf-fd).  
> You are right. So when creating the dmabuf we may have to decode the 
> framebuffer and create the dmabuf using the latest framebuffer information 
> and we must return the framebuffer information together with the dmabuf fd.
> 
> In the current implementation I saved the framebuffer information while user 
> querying the framebuffer and generate the dmabuf using the saved information 
> no error found yet but in theory there are sync problems.
> 
> >
> >ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix the 
> >race,
> >but then it is easier to go with ioctl only interface (simliar to the 
> >orginal one from
> >dec last year) I think.  
> Yes. ioctl works for it.
> But based on the mail last week. If I understand correctly Alex hope to query 
> the framebuffer information by reading the vfio device region and then get 
> the dmabuf fd using ioctl.

No, I was explaining that I had questioned whether we could use a vfio
region in place of a separate dmabuf fd.  We can't.  I have no
particular desire to use a vfio region just for querying framebuffer
info.  I prefer the dmabuf manager fd idea that Gerd suggested.  Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Fri, 12 May 2017 11:12:05 +0200
Gerd Hoffmann  wrote:

>   Hi,
> 
> > If the contents of the framebuffer change or if the parameters of the
> > framebuffer change?  I can't image that creating a new dmabuf fd for
> > every visual change within the framebuffer would be efficient, but I
> > don't have any concept of what a dmabuf actually does.  
> 
> Ok, some background:
> 
> The drm subsystem has the concept of planes.  The most important plane
> is the primary framebuffer (i.e. what gets scanned out to the physical
> display).  The cursor is a plane too, and there can be additional
> overlay planes for stuff like video playback.
> 
> Typically there are multiple planes in a system and only one of them
> gets scanned out to the crtc, i.e. the fbdev emulation creates one plane
> for the framebuffer console.  The X-Server creates a plane too, and when
> you switch between X-Server and framebuffer console via ctrl-alt-fn the
> intel driver just reprograms the encoder to scan out the one or the
> other plane to the crtc.
> 
> The dma-buf handed out by gvt is a reference to a plane.  I think on the
> host side gvt can see only the active plane (from encoder/crtc register
> programming) not the inactive ones.
> 
> The dma-buf can be imported as opengl texture and then be used to render
> the guest display to a host window.  I think it is even possible to use
> the dma-buf as plane in the host drm driver and scan it out directly to
> a physical display.  The actual framebuffer content stays in gpu memory
> all the time, the cpu never has to touch it.
> 
> It is possible to cache the dma-buf handles, i.e. when the guest boots
> you'll get the first for the fbcon plane, when the x-server starts the
> second for the x-server framebuffer, and when the user switches to the
> text console via ctrl-alt-fn you can re-use the fbcon dma-buf you
> already have.
> 
> The caching becomes more important for good performance when the guest
> uses pageflipping (wayland does): define two planes, render into one
> while displaying the other, then flip the two for a atomic display
> update.
> 
> The caching also makes it a bit difficult to create a good interface.
> So, the current patch set creates:
> 
>   (a) A way to query the active planes (ioctl
>   INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
>   (b) A way to create a dma-buf for the active plane (ioctl
>   INTEL_VGPU_GENERATE_DMABUF).
> 
> Typical userspace workflow is to first query the plane, then check if it
> already has a dma-buf for it, and if not create one.

Thank you!  This is immensely helpful!
 
> > What changes to the framebuffer require a new dmabuf fd?  Shouldn't the
> > user query the parameters of the framebuffer through a dmabuf fd and
> > shouldn't the dmabuf fd have some signaling mechanism to the user
> > (eventfd perhaps) to notify the user to re-evaluate the parameters?  
> 
> dma-bufs don't support that, they are really just a handle to a piece of
> memory, all metadata (format, size) most be communicated by other means.
> 
> > Otherwise are you imagining that the user polls the vfio region?  
> 
> Hmm, notification support would probably a good reason to have a
> separate file handle to manage the dma-bufs (instead of using
> driver-specific ioctls on the vfio fd), because the driver could also
> use the management fd for notifications then.

I like this idea of a separate control fd for dmabufs, it provides not
only a central management point, but also a nice abstraction for the
vfio device specific interface.  We potentially only need a single
VFIO_DEVICE_GET_DMABUF_MGR_FD() ioctl to get a dmabuf management fd
(perhaps with a type parameter, ex. GFX) where maybe we could have
vfio-core incorporate this reference into the group lifecycle, so the
vendor driver only needs to fdget/put this manager fd for the various
plane dmabuf fds spawned in order to get core-level reference counting.

The dmabuf manager fd can be separately versioned from vfio or make use
of some vendor magic if there are portions that are vendor specific
(the above examples for query/get ioctls really don't seem vendor
specific).
 
> I'm not sure how useful notification support actually is though.
> Notifications when another plane gets mapped to the crtc should be easy.
> But I'm not sure it is possible to get notifications when the plane
> content changes, especially in case the guest does software rendering so
> the display is updated without gvt seeing guest activity on the
> rendering pipeline.  Without the later qemu needs a timer for display
> updates _anyway_ ...

Seems like it has benefits even if we don't have an initial use for
notification mechanisms.  Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Fri, 12 May 2017 11:12:05 +0200
Gerd Hoffmann  wrote:

>   Hi,
> 
> > If the contents of the framebuffer change or if the parameters of the
> > framebuffer change?  I can't image that creating a new dmabuf fd for
> > every visual change within the framebuffer would be efficient, but I
> > don't have any concept of what a dmabuf actually does.  
> 
> Ok, some background:
> 
> The drm subsystem has the concept of planes.  The most important plane
> is the primary framebuffer (i.e. what gets scanned out to the physical
> display).  The cursor is a plane too, and there can be additional
> overlay planes for stuff like video playback.
> 
> Typically there are multiple planes in a system and only one of them
> gets scanned out to the crtc, i.e. the fbdev emulation creates one plane
> for the framebuffer console.  The X-Server creates a plane too, and when
> you switch between X-Server and framebuffer console via ctrl-alt-fn the
> intel driver just reprograms the encoder to scan out the one or the
> other plane to the crtc.
> 
> The dma-buf handed out by gvt is a reference to a plane.  I think on the
> host side gvt can see only the active plane (from encoder/crtc register
> programming) not the inactive ones.
> 
> The dma-buf can be imported as opengl texture and then be used to render
> the guest display to a host window.  I think it is even possible to use
> the dma-buf as plane in the host drm driver and scan it out directly to
> a physical display.  The actual framebuffer content stays in gpu memory
> all the time, the cpu never has to touch it.
> 
> It is possible to cache the dma-buf handles, i.e. when the guest boots
> you'll get the first for the fbcon plane, when the x-server starts the
> second for the x-server framebuffer, and when the user switches to the
> text console via ctrl-alt-fn you can re-use the fbcon dma-buf you
> already have.
> 
> The caching becomes more important for good performance when the guest
> uses pageflipping (wayland does): define two planes, render into one
> while displaying the other, then flip the two for a atomic display
> update.
> 
> The caching also makes it a bit difficult to create a good interface.
> So, the current patch set creates:
> 
>   (a) A way to query the active planes (ioctl
>   INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
>   (b) A way to create a dma-buf for the active plane (ioctl
>   INTEL_VGPU_GENERATE_DMABUF).
> 
> Typical userspace workflow is to first query the plane, then check if it
> already has a dma-buf for it, and if not create one.

Thank you!  This is immensely helpful!
 
> > What changes to the framebuffer require a new dmabuf fd?  Shouldn't the
> > user query the parameters of the framebuffer through a dmabuf fd and
> > shouldn't the dmabuf fd have some signaling mechanism to the user
> > (eventfd perhaps) to notify the user to re-evaluate the parameters?  
> 
> dma-bufs don't support that, they are really just a handle to a piece of
> memory, all metadata (format, size) most be communicated by other means.
> 
> > Otherwise are you imagining that the user polls the vfio region?  
> 
> Hmm, notification support would probably a good reason to have a
> separate file handle to manage the dma-bufs (instead of using
> driver-specific ioctls on the vfio fd), because the driver could also
> use the management fd for notifications then.

I like this idea of a separate control fd for dmabufs, it provides not
only a central management point, but also a nice abstraction for the
vfio device specific interface.  We potentially only need a single
VFIO_DEVICE_GET_DMABUF_MGR_FD() ioctl to get a dmabuf management fd
(perhaps with a type parameter, ex. GFX) where maybe we could have
vfio-core incorporate this reference into the group lifecycle, so the
vendor driver only needs to fdget/put this manager fd for the various
plane dmabuf fds spawned in order to get core-level reference counting.

The dmabuf manager fd can be separately versioned from vfio or make use
of some vendor magic if there are portions that are vendor specific
(the above examples for query/get ioctls really don't seem vendor
specific).
 
> I'm not sure how useful notification support actually is though.
> Notifications when another plane gets mapped to the crtc should be easy.
> But I'm not sure it is possible to get notifications when the plane
> content changes, especially in case the guest does software rendering so
> the display is updated without gvt seeing guest activity on the
> rendering pipeline.  Without the later qemu needs a timer for display
> updates _anyway_ ...

Seems like it has benefits even if we don't have an initial use for
notification mechanisms.  Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> If the contents of the framebuffer change or if the parameters of the
> framebuffer change?  I can't image that creating a new dmabuf fd for
> every visual change within the framebuffer would be efficient, but I
> don't have any concept of what a dmabuf actually does.

Ok, some background:

The drm subsystem has the concept of planes.  The most important plane
is the primary framebuffer (i.e. what gets scanned out to the physical
display).  The cursor is a plane too, and there can be additional
overlay planes for stuff like video playback.

Typically there are multiple planes in a system and only one of them
gets scanned out to the crtc, i.e. the fbdev emulation creates one plane
for the framebuffer console.  The X-Server creates a plane too, and when
you switch between X-Server and framebuffer console via ctrl-alt-fn the
intel driver just reprograms the encoder to scan out the one or the
other plane to the crtc.

The dma-buf handed out by gvt is a reference to a plane.  I think on the
host side gvt can see only the active plane (from encoder/crtc register
programming) not the inactive ones.

The dma-buf can be imported as opengl texture and then be used to render
the guest display to a host window.  I think it is even possible to use
the dma-buf as plane in the host drm driver and scan it out directly to
a physical display.  The actual framebuffer content stays in gpu memory
all the time, the cpu never has to touch it.

It is possible to cache the dma-buf handles, i.e. when the guest boots
you'll get the first for the fbcon plane, when the x-server starts the
second for the x-server framebuffer, and when the user switches to the
text console via ctrl-alt-fn you can re-use the fbcon dma-buf you
already have.

The caching becomes more important for good performance when the guest
uses pageflipping (wayland does): define two planes, render into one
while displaying the other, then flip the two for a atomic display
update.

The caching also makes it a bit difficult to create a good interface.
So, the current patch set creates:

  (a) A way to query the active planes (ioctl
  INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
  (b) A way to create a dma-buf for the active plane (ioctl
  INTEL_VGPU_GENERATE_DMABUF).

Typical userspace workflow is to first query the plane, then check if it
already has a dma-buf for it, and if not create one.

> What changes to the framebuffer require a new dmabuf fd?  Shouldn't the
> user query the parameters of the framebuffer through a dmabuf fd and
> shouldn't the dmabuf fd have some signaling mechanism to the user
> (eventfd perhaps) to notify the user to re-evaluate the parameters?

dma-bufs don't support that, they are really just a handle to a piece of
memory, all metadata (format, size) most be communicated by other means.

> Otherwise are you imagining that the user polls the vfio region?

Hmm, notification support would probably a good reason to have a
separate file handle to manage the dma-bufs (instead of using
driver-specific ioctls on the vfio fd), because the driver could also
use the management fd for notifications then.

I'm not sure how useful notification support actually is though.
Notifications when another plane gets mapped to the crtc should be easy.
But I'm not sure it is possible to get notifications when the plane
content changes, especially in case the guest does software rendering so
the display is updated without gvt seeing guest activity on the
rendering pipeline.  Without the later qemu needs a timer for display
updates _anyway_ ...

cheers,
  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> If the contents of the framebuffer change or if the parameters of the
> framebuffer change?  I can't image that creating a new dmabuf fd for
> every visual change within the framebuffer would be efficient, but I
> don't have any concept of what a dmabuf actually does.

Ok, some background:

The drm subsystem has the concept of planes.  The most important plane
is the primary framebuffer (i.e. what gets scanned out to the physical
display).  The cursor is a plane too, and there can be additional
overlay planes for stuff like video playback.

Typically there are multiple planes in a system and only one of them
gets scanned out to the crtc, i.e. the fbdev emulation creates one plane
for the framebuffer console.  The X-Server creates a plane too, and when
you switch between X-Server and framebuffer console via ctrl-alt-fn the
intel driver just reprograms the encoder to scan out the one or the
other plane to the crtc.

The dma-buf handed out by gvt is a reference to a plane.  I think on the
host side gvt can see only the active plane (from encoder/crtc register
programming) not the inactive ones.

The dma-buf can be imported as opengl texture and then be used to render
the guest display to a host window.  I think it is even possible to use
the dma-buf as plane in the host drm driver and scan it out directly to
a physical display.  The actual framebuffer content stays in gpu memory
all the time, the cpu never has to touch it.

It is possible to cache the dma-buf handles, i.e. when the guest boots
you'll get the first for the fbcon plane, when the x-server starts the
second for the x-server framebuffer, and when the user switches to the
text console via ctrl-alt-fn you can re-use the fbcon dma-buf you
already have.

The caching becomes more important for good performance when the guest
uses pageflipping (wayland does): define two planes, render into one
while displaying the other, then flip the two for a atomic display
update.

The caching also makes it a bit difficult to create a good interface.
So, the current patch set creates:

  (a) A way to query the active planes (ioctl
  INTEL_VGPU_QUERY_DMABUF added by patch 5/6 of this series).
  (b) A way to create a dma-buf for the active plane (ioctl
  INTEL_VGPU_GENERATE_DMABUF).

Typical userspace workflow is to first query the plane, then check if it
already has a dma-buf for it, and if not create one.

> What changes to the framebuffer require a new dmabuf fd?  Shouldn't the
> user query the parameters of the framebuffer through a dmabuf fd and
> shouldn't the dmabuf fd have some signaling mechanism to the user
> (eventfd perhaps) to notify the user to re-evaluate the parameters?

dma-bufs don't support that, they are really just a handle to a piece of
memory, all metadata (format, size) most be communicated by other means.

> Otherwise are you imagining that the user polls the vfio region?

Hmm, notification support would probably a good reason to have a
separate file handle to manage the dma-bufs (instead of using
driver-specific ioctls on the vfio fd), because the driver could also
use the management fd for notifications then.

I'm not sure how useful notification support actually is though.
Notifications when another plane gets mapped to the crtc should be easy.
But I'm not sure it is possible to get notifications when the plane
content changes, especially in case the guest does software rendering so
the display is updated without gvt seeing guest activity on the
rendering pipeline.  Without the later qemu needs a timer for display
updates _anyway_ ...

cheers,
  Gerd

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Gerd,

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Gerd Hoffmann
>Sent: Thursday, May 11, 2017 9:28 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; Alex Williamson
><alex.william...@redhat.com>; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>  Hi,
>
>> While read the framebuffer region we have to tell the vendor driver which
>framebuffer we want to read? There are two framebuffers now in KVMGT that is
>primary and cursor.
>> There are two methods to implement this:
>> 1) write the plane id first and then read the framebuffer.
>> 2) create 2 vfio regions one for primary and one for cursor.
>
>(3) Place information for both planes into one vfio region.
>Which allows to fetch both with a single read() syscall.
That works too. Then using the ioctl to get the dmabuf fd if needed. And plane 
id can be ioctl's parameter.

How about method 2 primary plane and cursor plane are different and should 
generate different dmabuf for each of them.

>
>The question is how you'll get the file descriptor then.  If the ioctl returns 
>the
>dma-buf fd only you have a racy interface:  Things can change between 
>read(vfio-
>region) and ioctl(need-dmabuf-fd).
You are right. So when creating the dmabuf we may have to decode the 
framebuffer and create the dmabuf using the latest framebuffer information and 
we must return the framebuffer information together with the dmabuf fd.

In the current implementation I saved the framebuffer information while user 
querying the framebuffer and generate the dmabuf using the saved information no 
error found yet but in theory there are sync problems.

>
>ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix the race,
>but then it is easier to go with ioctl only interface (simliar to the orginal 
>one from
>dec last year) I think.
Yes. ioctl works for it.
But based on the mail last week. If I understand correctly Alex hope to query 
the framebuffer information by reading the vfio device region and then get the 
dmabuf fd using ioctl.

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

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Gerd,

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Gerd Hoffmann
>Sent: Thursday, May 11, 2017 9:28 PM
>To: Chen, Xiaoguang 
>Cc: Tian, Kevin ; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; Alex Williamson
>; Lv, Zhiyuan ; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A 
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>  Hi,
>
>> While read the framebuffer region we have to tell the vendor driver which
>framebuffer we want to read? There are two framebuffers now in KVMGT that is
>primary and cursor.
>> There are two methods to implement this:
>> 1) write the plane id first and then read the framebuffer.
>> 2) create 2 vfio regions one for primary and one for cursor.
>
>(3) Place information for both planes into one vfio region.
>Which allows to fetch both with a single read() syscall.
That works too. Then using the ioctl to get the dmabuf fd if needed. And plane 
id can be ioctl's parameter.

How about method 2 primary plane and cursor plane are different and should 
generate different dmabuf for each of them.

>
>The question is how you'll get the file descriptor then.  If the ioctl returns 
>the
>dma-buf fd only you have a racy interface:  Things can change between 
>read(vfio-
>region) and ioctl(need-dmabuf-fd).
You are right. So when creating the dmabuf we may have to decode the 
framebuffer and create the dmabuf using the latest framebuffer information and 
we must return the framebuffer information together with the dmabuf fd.

In the current implementation I saved the framebuffer information while user 
querying the framebuffer and generate the dmabuf using the saved information no 
error found yet but in theory there are sync problems.

>
>ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix the race,
>but then it is easier to go with ioctl only interface (simliar to the orginal 
>one from
>dec last year) I think.
Yes. ioctl works for it.
But based on the mail last week. If I understand correctly Alex hope to query 
the framebuffer information by reading the vfio device region and then get the 
dmabuf fd using ioctl.

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

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Friday, May 12, 2017 10:58 AM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: Gerd Hoffmann <kra...@redhat.com>; Tian, Kevin <kevin.t...@intel.com>;
>intel-...@lists.freedesktop.org; linux-kernel@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fri, 12 May 2017 02:12:10 +
>"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
>
>> Hi Alex and Gerd,
>>
>> >-Original Message-
>> >From: intel-gvt-dev
>> >[mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf Of
>> >Alex Williamson
>> >Sent: Thursday, May 11, 2017 11:45 PM
>> >To: Gerd Hoffmann <kra...@redhat.com>
>> >Cc: Tian, Kevin <kevin.t...@intel.com>;
>> >intel-...@lists.freedesktop.org; linux- ker...@vger.kernel.org;
>> >zhen...@linux.intel.com; Lv, Zhiyuan <zhiyuan...@intel.com>; Chen,
>> >Xiaoguang <xiaoguang.c...@intel.com>; intel-
>> >gvt-...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>
>> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>> >dmabuf
>> >
>> >On Thu, 11 May 2017 15:27:53 +0200
>> >Gerd Hoffmann <kra...@redhat.com> wrote:
>> >
>> >>   Hi,
>> >>
>> >> > While read the framebuffer region we have to tell the vendor
>> >> > driver which
>> >framebuffer we want to read? There are two framebuffers now in KVMGT
>> >that is primary and cursor.
>> >> > There are two methods to implement this:
>> >> > 1) write the plane id first and then read the framebuffer.
>> >> > 2) create 2 vfio regions one for primary and one for cursor.
>> >>
>> >> (3) Place information for both planes into one vfio region.
>> >> Which allows to fetch both with a single read() syscall.
>> >>
>> >> The question is how you'll get the file descriptor then.  If the
>> >> ioctl returns the dma-buf fd only you have a racy interface:
>> >> Things can change between read(vfio-region) and ioctl(need-dmabuf-fd).
>> >>
>> >> ioctl(need-dma-buf) could return both dmabuf fd and plane info to
>> >> fix the race, but then it is easier to go with ioctl only interface
>> >> (simliar to the orginal one from dec last year) I think.
>> >
>> >If the dmabuf fd is provided by a separate mdev vendor driver
>> >specific ioctl, I don't see how vfio regions should be involved.  Selecting 
>> >which
>framebuffer
>> >should be an ioctl parameter.
>> Based on your last mail. I think the implementation looks like this:
>> 1) user query the framebuffer information by reading the vfio region.
>> 2) if the framebuffer changed(such as framebuffer's graphics address changed,
>size changed etc) we will need to create a new dmabuf fd.
>> 3) create a new dmabuf fd using vfio device specific ioctl.
>>
>> >What sort of information needs to be conveyed
>> >about each plane?
>> Only plane id is needed.
>>
>> >Is it static information or something that needs to be read
>> >repeatedly?
>> It is static information. For our case plane id 1 represent primary plane 
>> and 3 for
>cursor plane. 2 means sprite plane which will not be used in our case.
>>
>> >Do we need it before we get the dmabuf fd or can it be an ioctl on
>> >the dmabuf fd?
>> We need it while query the framebuffer. In kernel we need the plane id to
>decide which plane we should decode.
>> Below is my current implementation:
>> 1) user first query the framebuffer(primary or cursor) and kernel decode the
>framebuffer and return the framebuffer information to user and also save a copy
>in kernel.
>> 2) user compared the framebuffer and if the framebuffer changed  creating a
>new dmabuf fd.
>
>If the contents of the framebuffer change or if the parameters of the 
>framebuffer
>change?  
If the parameters of the framebuffer change we need to create new dmabuf.


>I can't image that creating a new dmabuf fd for every visual change
>within the framebuffer would be efficient, but I don't have any concept of 
>what a
>dmabuf actually does.
>
>> 3) kernel create a new dmabuf fd based on saved framebuffer information.
>>
>> So we need plane id in step 1.
&g

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

>-Original Message-
>From: Alex Williamson [mailto:alex.william...@redhat.com]
>Sent: Friday, May 12, 2017 10:58 AM
>To: Chen, Xiaoguang 
>Cc: Gerd Hoffmann ; Tian, Kevin ;
>intel-...@lists.freedesktop.org; linux-kernel@vger.kernel.org;
>zhen...@linux.intel.com; Lv, Zhiyuan ; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A 
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fri, 12 May 2017 02:12:10 +
>"Chen, Xiaoguang"  wrote:
>
>> Hi Alex and Gerd,
>>
>> >-Original Message-
>> >From: intel-gvt-dev
>> >[mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On Behalf Of
>> >Alex Williamson
>> >Sent: Thursday, May 11, 2017 11:45 PM
>> >To: Gerd Hoffmann 
>> >Cc: Tian, Kevin ;
>> >intel-...@lists.freedesktop.org; linux- ker...@vger.kernel.org;
>> >zhen...@linux.intel.com; Lv, Zhiyuan ; Chen,
>> >Xiaoguang ; intel-
>> >gvt-...@lists.freedesktop.org; Wang, Zhi A 
>> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>> >dmabuf
>> >
>> >On Thu, 11 May 2017 15:27:53 +0200
>> >Gerd Hoffmann  wrote:
>> >
>> >>   Hi,
>> >>
>> >> > While read the framebuffer region we have to tell the vendor
>> >> > driver which
>> >framebuffer we want to read? There are two framebuffers now in KVMGT
>> >that is primary and cursor.
>> >> > There are two methods to implement this:
>> >> > 1) write the plane id first and then read the framebuffer.
>> >> > 2) create 2 vfio regions one for primary and one for cursor.
>> >>
>> >> (3) Place information for both planes into one vfio region.
>> >> Which allows to fetch both with a single read() syscall.
>> >>
>> >> The question is how you'll get the file descriptor then.  If the
>> >> ioctl returns the dma-buf fd only you have a racy interface:
>> >> Things can change between read(vfio-region) and ioctl(need-dmabuf-fd).
>> >>
>> >> ioctl(need-dma-buf) could return both dmabuf fd and plane info to
>> >> fix the race, but then it is easier to go with ioctl only interface
>> >> (simliar to the orginal one from dec last year) I think.
>> >
>> >If the dmabuf fd is provided by a separate mdev vendor driver
>> >specific ioctl, I don't see how vfio regions should be involved.  Selecting 
>> >which
>framebuffer
>> >should be an ioctl parameter.
>> Based on your last mail. I think the implementation looks like this:
>> 1) user query the framebuffer information by reading the vfio region.
>> 2) if the framebuffer changed(such as framebuffer's graphics address changed,
>size changed etc) we will need to create a new dmabuf fd.
>> 3) create a new dmabuf fd using vfio device specific ioctl.
>>
>> >What sort of information needs to be conveyed
>> >about each plane?
>> Only plane id is needed.
>>
>> >Is it static information or something that needs to be read
>> >repeatedly?
>> It is static information. For our case plane id 1 represent primary plane 
>> and 3 for
>cursor plane. 2 means sprite plane which will not be used in our case.
>>
>> >Do we need it before we get the dmabuf fd or can it be an ioctl on
>> >the dmabuf fd?
>> We need it while query the framebuffer. In kernel we need the plane id to
>decide which plane we should decode.
>> Below is my current implementation:
>> 1) user first query the framebuffer(primary or cursor) and kernel decode the
>framebuffer and return the framebuffer information to user and also save a copy
>in kernel.
>> 2) user compared the framebuffer and if the framebuffer changed  creating a
>new dmabuf fd.
>
>If the contents of the framebuffer change or if the parameters of the 
>framebuffer
>change?  
If the parameters of the framebuffer change we need to create new dmabuf.


>I can't image that creating a new dmabuf fd for every visual change
>within the framebuffer would be efficient, but I don't have any concept of 
>what a
>dmabuf actually does.
>
>> 3) kernel create a new dmabuf fd based on saved framebuffer information.
>>
>> So we need plane id in step 1.
>> In step 3 we create a dmabuf fd only using saved framebuffer information(no
>other information is needed).
>
>What changes to the framebuffer require a new dmabuf fd?  Shouldn't the user
>query the parameters of the framebuffer through a dmabuf fd and shouldn't the
>dmabuf fd have some signaling mechanis

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Fri, 12 May 2017 02:12:10 +
"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:

> Hi Alex and Gerd,
> 
> >-Original Message-
> >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
> >Behalf Of Alex Williamson
> >Sent: Thursday, May 11, 2017 11:45 PM
> >To: Gerd Hoffmann <kra...@redhat.com>
> >Cc: Tian, Kevin <kevin.t...@intel.com>; intel-...@lists.freedesktop.org; 
> >linux-
> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> ><zhiyuan...@intel.com>; Chen, Xiaoguang <xiaoguang.c...@intel.com>; intel-
> >gvt-...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >On Thu, 11 May 2017 15:27:53 +0200
> >Gerd Hoffmann <kra...@redhat.com> wrote:
> >  
> >>   Hi,
> >>  
> >> > While read the framebuffer region we have to tell the vendor driver 
> >> > which  
> >framebuffer we want to read? There are two framebuffers now in KVMGT that is
> >primary and cursor.  
> >> > There are two methods to implement this:
> >> > 1) write the plane id first and then read the framebuffer.
> >> > 2) create 2 vfio regions one for primary and one for cursor.  
> >>
> >> (3) Place information for both planes into one vfio region.
> >> Which allows to fetch both with a single read() syscall.
> >>
> >> The question is how you'll get the file descriptor then.  If the ioctl
> >> returns the dma-buf fd only you have a racy interface:  Things can
> >> change between read(vfio-region) and ioctl(need-dmabuf-fd).
> >>
> >> ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix
> >> the race, but then it is easier to go with ioctl only interface
> >> (simliar to the orginal one from dec last year) I think.  
> >
> >If the dmabuf fd is provided by a separate mdev vendor driver specific 
> >ioctl, I
> >don't see how vfio regions should be involved.  Selecting which framebuffer
> >should be an ioctl parameter.
> Based on your last mail. I think the implementation looks like this:
> 1) user query the framebuffer information by reading the vfio region.
> 2) if the framebuffer changed(such as framebuffer's graphics address changed, 
> size changed etc) we will need to create a new dmabuf fd.
> 3) create a new dmabuf fd using vfio device specific ioctl.
> 
> >What sort of information needs to be conveyed
> >about each plane?
> Only plane id is needed.
> 
> >Is it static information or something that needs to be read
> >repeatedly?   
> It is static information. For our case plane id 1 represent primary plane and 
> 3 for cursor plane. 2 means sprite plane which will not be used in our case.
> 
> >Do we need it before we get the dmabuf fd or can it be an ioctl on
> >the dmabuf fd?  
> We need it while query the framebuffer. In kernel we need the plane id to 
> decide which plane we should decode.
> Below is my current implementation:
> 1) user first query the framebuffer(primary or cursor) and kernel decode the 
> framebuffer and return the framebuffer information to user and also save a 
> copy in kernel.
> 2) user compared the framebuffer and if the framebuffer changed  creating a 
> new dmabuf fd.

If the contents of the framebuffer change or if the parameters of the
framebuffer change?  I can't image that creating a new dmabuf fd for
every visual change within the framebuffer would be efficient, but I
don't have any concept of what a dmabuf actually does.

> 3) kernel create a new dmabuf fd based on saved framebuffer information.
> 
> So we need plane id in step 1.
> In step 3 we create a dmabuf fd only using saved framebuffer information(no 
> other information is needed).

What changes to the framebuffer require a new dmabuf fd?  Shouldn't the
user query the parameters of the framebuffer through a dmabuf fd and
shouldn't the dmabuf fd have some signaling mechanism to the user
(eventfd perhaps) to notify the user to re-evaluate the parameters?
Otherwise are you imagining that the user polls the vfio region?  Why
can a dmabuf fd not persist across changes to the framebuffer?  Can
someone explain what a dmabuf is and how it works in terms that a
non-graphics person can understand?  Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Fri, 12 May 2017 02:12:10 +
"Chen, Xiaoguang"  wrote:

> Hi Alex and Gerd,
> 
> >-Original Message-
> >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
> >Behalf Of Alex Williamson
> >Sent: Thursday, May 11, 2017 11:45 PM
> >To: Gerd Hoffmann 
> >Cc: Tian, Kevin ; intel-...@lists.freedesktop.org; 
> >linux-
> >ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
> >; Chen, Xiaoguang ; intel-
> >gvt-...@lists.freedesktop.org; Wang, Zhi A 
> >Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >On Thu, 11 May 2017 15:27:53 +0200
> >Gerd Hoffmann  wrote:
> >  
> >>   Hi,
> >>  
> >> > While read the framebuffer region we have to tell the vendor driver 
> >> > which  
> >framebuffer we want to read? There are two framebuffers now in KVMGT that is
> >primary and cursor.  
> >> > There are two methods to implement this:
> >> > 1) write the plane id first and then read the framebuffer.
> >> > 2) create 2 vfio regions one for primary and one for cursor.  
> >>
> >> (3) Place information for both planes into one vfio region.
> >> Which allows to fetch both with a single read() syscall.
> >>
> >> The question is how you'll get the file descriptor then.  If the ioctl
> >> returns the dma-buf fd only you have a racy interface:  Things can
> >> change between read(vfio-region) and ioctl(need-dmabuf-fd).
> >>
> >> ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix
> >> the race, but then it is easier to go with ioctl only interface
> >> (simliar to the orginal one from dec last year) I think.  
> >
> >If the dmabuf fd is provided by a separate mdev vendor driver specific 
> >ioctl, I
> >don't see how vfio regions should be involved.  Selecting which framebuffer
> >should be an ioctl parameter.
> Based on your last mail. I think the implementation looks like this:
> 1) user query the framebuffer information by reading the vfio region.
> 2) if the framebuffer changed(such as framebuffer's graphics address changed, 
> size changed etc) we will need to create a new dmabuf fd.
> 3) create a new dmabuf fd using vfio device specific ioctl.
> 
> >What sort of information needs to be conveyed
> >about each plane?
> Only plane id is needed.
> 
> >Is it static information or something that needs to be read
> >repeatedly?   
> It is static information. For our case plane id 1 represent primary plane and 
> 3 for cursor plane. 2 means sprite plane which will not be used in our case.
> 
> >Do we need it before we get the dmabuf fd or can it be an ioctl on
> >the dmabuf fd?  
> We need it while query the framebuffer. In kernel we need the plane id to 
> decide which plane we should decode.
> Below is my current implementation:
> 1) user first query the framebuffer(primary or cursor) and kernel decode the 
> framebuffer and return the framebuffer information to user and also save a 
> copy in kernel.
> 2) user compared the framebuffer and if the framebuffer changed  creating a 
> new dmabuf fd.

If the contents of the framebuffer change or if the parameters of the
framebuffer change?  I can't image that creating a new dmabuf fd for
every visual change within the framebuffer would be efficient, but I
don't have any concept of what a dmabuf actually does.

> 3) kernel create a new dmabuf fd based on saved framebuffer information.
> 
> So we need plane id in step 1.
> In step 3 we create a dmabuf fd only using saved framebuffer information(no 
> other information is needed).

What changes to the framebuffer require a new dmabuf fd?  Shouldn't the
user query the parameters of the framebuffer through a dmabuf fd and
shouldn't the dmabuf fd have some signaling mechanism to the user
(eventfd perhaps) to notify the user to re-evaluate the parameters?
Otherwise are you imagining that the user polls the vfio region?  Why
can a dmabuf fd not persist across changes to the framebuffer?  Can
someone explain what a dmabuf is and how it works in terms that a
non-graphics person can understand?  Thanks,

Alex

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex and Gerd,

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Alex Williamson
>Sent: Thursday, May 11, 2017 11:45 PM
>To: Gerd Hoffmann <kra...@redhat.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
><zhiyuan...@intel.com>; Chen, Xiaoguang <xiaoguang.c...@intel.com>; intel-
>gvt-...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Thu, 11 May 2017 15:27:53 +0200
>Gerd Hoffmann <kra...@redhat.com> wrote:
>
>>   Hi,
>>
>> > While read the framebuffer region we have to tell the vendor driver which
>framebuffer we want to read? There are two framebuffers now in KVMGT that is
>primary and cursor.
>> > There are two methods to implement this:
>> > 1) write the plane id first and then read the framebuffer.
>> > 2) create 2 vfio regions one for primary and one for cursor.
>>
>> (3) Place information for both planes into one vfio region.
>> Which allows to fetch both with a single read() syscall.
>>
>> The question is how you'll get the file descriptor then.  If the ioctl
>> returns the dma-buf fd only you have a racy interface:  Things can
>> change between read(vfio-region) and ioctl(need-dmabuf-fd).
>>
>> ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix
>> the race, but then it is easier to go with ioctl only interface
>> (simliar to the orginal one from dec last year) I think.
>
>If the dmabuf fd is provided by a separate mdev vendor driver specific ioctl, I
>don't see how vfio regions should be involved.  Selecting which framebuffer
>should be an ioctl parameter.  
Based on your last mail. I think the implementation looks like this:
1) user query the framebuffer information by reading the vfio region.
2) if the framebuffer changed(such as framebuffer's graphics address changed, 
size changed etc) we will need to create a new dmabuf fd.
3) create a new dmabuf fd using vfio device specific ioctl.

>What sort of information needs to be conveyed
>about each plane?  
Only plane id is needed.

>Is it static information or something that needs to be read
>repeatedly? 
It is static information. For our case plane id 1 represent primary plane and 3 
for cursor plane. 2 means sprite plane which will not be used in our case.

>Do we need it before we get the dmabuf fd or can it be an ioctl on
>the dmabuf fd?
We need it while query the framebuffer. In kernel we need the plane id to 
decide which plane we should decode.
Below is my current implementation:
1) user first query the framebuffer(primary or cursor) and kernel decode the 
framebuffer and return the framebuffer information to user and also save a copy 
in kernel.
2) user compared the framebuffer and if the framebuffer changed  creating a new 
dmabuf fd.
3) kernel create a new dmabuf fd based on saved framebuffer information.

So we need plane id in step 1.
In step 3 we create a dmabuf fd only using saved framebuffer information(no 
other information is needed).

Chenxg.
>Thanks,
>
>Alex
>___
>intel-gvt-dev mailing list
>intel-gvt-...@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex and Gerd,

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Alex Williamson
>Sent: Thursday, May 11, 2017 11:45 PM
>To: Gerd Hoffmann 
>Cc: Tian, Kevin ; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>; Chen, Xiaoguang ; intel-
>gvt-...@lists.freedesktop.org; Wang, Zhi A 
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Thu, 11 May 2017 15:27:53 +0200
>Gerd Hoffmann  wrote:
>
>>   Hi,
>>
>> > While read the framebuffer region we have to tell the vendor driver which
>framebuffer we want to read? There are two framebuffers now in KVMGT that is
>primary and cursor.
>> > There are two methods to implement this:
>> > 1) write the plane id first and then read the framebuffer.
>> > 2) create 2 vfio regions one for primary and one for cursor.
>>
>> (3) Place information for both planes into one vfio region.
>> Which allows to fetch both with a single read() syscall.
>>
>> The question is how you'll get the file descriptor then.  If the ioctl
>> returns the dma-buf fd only you have a racy interface:  Things can
>> change between read(vfio-region) and ioctl(need-dmabuf-fd).
>>
>> ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix
>> the race, but then it is easier to go with ioctl only interface
>> (simliar to the orginal one from dec last year) I think.
>
>If the dmabuf fd is provided by a separate mdev vendor driver specific ioctl, I
>don't see how vfio regions should be involved.  Selecting which framebuffer
>should be an ioctl parameter.  
Based on your last mail. I think the implementation looks like this:
1) user query the framebuffer information by reading the vfio region.
2) if the framebuffer changed(such as framebuffer's graphics address changed, 
size changed etc) we will need to create a new dmabuf fd.
3) create a new dmabuf fd using vfio device specific ioctl.

>What sort of information needs to be conveyed
>about each plane?  
Only plane id is needed.

>Is it static information or something that needs to be read
>repeatedly? 
It is static information. For our case plane id 1 represent primary plane and 3 
for cursor plane. 2 means sprite plane which will not be used in our case.

>Do we need it before we get the dmabuf fd or can it be an ioctl on
>the dmabuf fd?
We need it while query the framebuffer. In kernel we need the plane id to 
decide which plane we should decode.
Below is my current implementation:
1) user first query the framebuffer(primary or cursor) and kernel decode the 
framebuffer and return the framebuffer information to user and also save a copy 
in kernel.
2) user compared the framebuffer and if the framebuffer changed  creating a new 
dmabuf fd.
3) kernel create a new dmabuf fd based on saved framebuffer information.

So we need plane id in step 1.
In step 3 we create a dmabuf fd only using saved framebuffer information(no 
other information is needed).

Chenxg.
>Thanks,
>
>Alex
>___
>intel-gvt-dev mailing list
>intel-gvt-...@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Thu, 11 May 2017 15:27:53 +0200
Gerd Hoffmann  wrote:

>   Hi,
> 
> > While read the framebuffer region we have to tell the vendor driver which 
> > framebuffer we want to read? There are two framebuffers now in KVMGT that 
> > is primary and cursor.
> > There are two methods to implement this:
> > 1) write the plane id first and then read the framebuffer.
> > 2) create 2 vfio regions one for primary and one for cursor.  
> 
> (3) Place information for both planes into one vfio region.
> Which allows to fetch both with a single read() syscall.
> 
> The question is how you'll get the file descriptor then.  If the ioctl
> returns the dma-buf fd only you have a racy interface:  Things can
> change between read(vfio-region) and ioctl(need-dmabuf-fd).
> 
> ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix
> the race, but then it is easier to go with ioctl only interface (simliar
> to the orginal one from dec last year) I think.

If the dmabuf fd is provided by a separate mdev vendor driver specific
ioctl, I don't see how vfio regions should be involved.  Selecting
which framebuffer should be an ioctl parameter.  What sort of
information needs to be conveyed about each plane?  Is it static
information or something that needs to be read repeatedly?  Do we need
it before we get the dmabuf fd or can it be an ioctl on the dmabuf fd?
Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Thu, 11 May 2017 15:27:53 +0200
Gerd Hoffmann  wrote:

>   Hi,
> 
> > While read the framebuffer region we have to tell the vendor driver which 
> > framebuffer we want to read? There are two framebuffers now in KVMGT that 
> > is primary and cursor.
> > There are two methods to implement this:
> > 1) write the plane id first and then read the framebuffer.
> > 2) create 2 vfio regions one for primary and one for cursor.  
> 
> (3) Place information for both planes into one vfio region.
> Which allows to fetch both with a single read() syscall.
> 
> The question is how you'll get the file descriptor then.  If the ioctl
> returns the dma-buf fd only you have a racy interface:  Things can
> change between read(vfio-region) and ioctl(need-dmabuf-fd).
> 
> ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix
> the race, but then it is easier to go with ioctl only interface (simliar
> to the orginal one from dec last year) I think.

If the dmabuf fd is provided by a separate mdev vendor driver specific
ioctl, I don't see how vfio regions should be involved.  Selecting
which framebuffer should be an ioctl parameter.  What sort of
information needs to be conveyed about each plane?  Is it static
information or something that needs to be read repeatedly?  Do we need
it before we get the dmabuf fd or can it be an ioctl on the dmabuf fd?
Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> While read the framebuffer region we have to tell the vendor driver which 
> framebuffer we want to read? There are two framebuffers now in KVMGT that is 
> primary and cursor.
> There are two methods to implement this:
> 1) write the plane id first and then read the framebuffer.
> 2) create 2 vfio regions one for primary and one for cursor.

(3) Place information for both planes into one vfio region.
Which allows to fetch both with a single read() syscall.

The question is how you'll get the file descriptor then.  If the ioctl
returns the dma-buf fd only you have a racy interface:  Things can
change between read(vfio-region) and ioctl(need-dmabuf-fd).

ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix
the race, but then it is easier to go with ioctl only interface (simliar
to the orginal one from dec last year) I think.

cheers,
  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> While read the framebuffer region we have to tell the vendor driver which 
> framebuffer we want to read? There are two framebuffers now in KVMGT that is 
> primary and cursor.
> There are two methods to implement this:
> 1) write the plane id first and then read the framebuffer.
> 2) create 2 vfio regions one for primary and one for cursor.

(3) Place information for both planes into one vfio region.
Which allows to fetch both with a single read() syscall.

The question is how you'll get the file descriptor then.  If the ioctl
returns the dma-buf fd only you have a racy interface:  Things can
change between read(vfio-region) and ioctl(need-dmabuf-fd).

ioctl(need-dma-buf) could return both dmabuf fd and plane info to fix
the race, but then it is easier to go with ioctl only interface (simliar
to the orginal one from dec last year) I think.

cheers,
  Gerd

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex,

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Alex Williamson
>Sent: Friday, May 05, 2017 11:11 PM
>To: Gerd Hoffmann <kra...@redhat.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
><zhiyuan...@intel.com>; Chen, Xiaoguang <xiaoguang.c...@intel.com>; intel-
>gvt-...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fri, 05 May 2017 08:55:31 +0200
>Gerd Hoffmann <kra...@redhat.com> wrote:
>
>>   Hi,
>>
>> > > >>Hmm, that looks like a rather strange way to return a file descriptor.
>> > > >>
>> > > >>What is the reason to not use ioctls on the vfio file handle, like
>> > > >>older version of these patches did?
>> > > >If I understood correctly that Alex prefer not to change the
>> > > >ioctls on the vfio file handle like the old version.
>> > > >So I used this way the smallest change to general vfio framework
>> > > >only adding a subregion definition.
>> >
>> > I think I was hoping we could avoid a separate file descriptor
>> > altogether and use a vfio region instead.
>>
>> What exactly did you have in mind?  Put the framebuffer information
>> (struct intel_vgpu_dmabuf) into the vfio region, then access it using
>> read/write/mmap?
>
>Yeah, that was my hope.  Adding a new file descriptor means we have one more
>reference floating around complicating the life cycle of the device, group, and
>container.  Furthermore this one is really only visible to the mdev vendor 
>driver,
>so we can't rely on vfio-core, the vendor driver will need to consider the
>reference when releasing the device.
While read the framebuffer region we have to tell the vendor driver which 
framebuffer we want to read? There are two framebuffers now in KVMGT that is 
primary and cursor.
There are two methods to implement this:
1) write the plane id first and then read the framebuffer.
2) create 2 vfio regions one for primary and one for cursor.

Which method do you prefer? Or do you have other idea to handle this problem?

chenxg

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex,

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Alex Williamson
>Sent: Friday, May 05, 2017 11:11 PM
>To: Gerd Hoffmann 
>Cc: Tian, Kevin ; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; Lv, Zhiyuan
>; Chen, Xiaoguang ; intel-
>gvt-...@lists.freedesktop.org; Wang, Zhi A 
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fri, 05 May 2017 08:55:31 +0200
>Gerd Hoffmann  wrote:
>
>>   Hi,
>>
>> > > >>Hmm, that looks like a rather strange way to return a file descriptor.
>> > > >>
>> > > >>What is the reason to not use ioctls on the vfio file handle, like
>> > > >>older version of these patches did?
>> > > >If I understood correctly that Alex prefer not to change the
>> > > >ioctls on the vfio file handle like the old version.
>> > > >So I used this way the smallest change to general vfio framework
>> > > >only adding a subregion definition.
>> >
>> > I think I was hoping we could avoid a separate file descriptor
>> > altogether and use a vfio region instead.
>>
>> What exactly did you have in mind?  Put the framebuffer information
>> (struct intel_vgpu_dmabuf) into the vfio region, then access it using
>> read/write/mmap?
>
>Yeah, that was my hope.  Adding a new file descriptor means we have one more
>reference floating around complicating the life cycle of the device, group, and
>container.  Furthermore this one is really only visible to the mdev vendor 
>driver,
>so we can't rely on vfio-core, the vendor driver will need to consider the
>reference when releasing the device.
While read the framebuffer region we have to tell the vendor driver which 
framebuffer we want to read? There are two framebuffers now in KVMGT that is 
primary and cursor.
There are two methods to implement this:
1) write the plane id first and then read the framebuffer.
2) create 2 vfio regions one for primary and one for cursor.

Which method do you prefer? Or do you have other idea to handle this problem?

chenxg

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Fri, 05 May 2017 08:55:31 +0200
Gerd Hoffmann  wrote:

>   Hi,
> 
> > > >>Hmm, that looks like a rather strange way to return a file descriptor.
> > > >>
> > > >>What is the reason to not use ioctls on the vfio file handle, like
> > > >>older version of these patches did?
> > > >If I understood correctly that Alex prefer not to change the ioctls on 
> > > >the vfio file
> > > >handle like the old version.
> > > >So I used this way the smallest change to general vfio framework only 
> > > >adding a
> > > >subregion definition.  
> > 
> > I think I was hoping we could avoid a separate file descriptor
> > altogether and use a vfio region instead.  
> 
> What exactly did you have in mind?  Put the framebuffer information
> (struct intel_vgpu_dmabuf) into the vfio region, then access it using
> read/write/mmap?

Yeah, that was my hope.  Adding a new file descriptor means we have one
more reference floating around complicating the life cycle of the
device, group, and container.  Furthermore this one is really only
visible to the mdev vendor driver, so we can't rely on vfio-core, the
vendor driver will need to consider the reference when releasing the
device.

> > However, it was explained
> > previously why this really needs to be a separate fd and I agree that
> > using a region to expose an fd is really awkward.  
> 
> Now with this patchset we have *two* kinds of separate file handles.
> First the anon-fd created by reading from the region.  This is then used
> to run the intel ioctls on, which in turn create the other kind of file
> handle (dma-buf-fd).
> 
> The dma-buf-fd really needs to be a separate fd, because it gets passed
> around as handle and because this is the way dma-bufs work (guess this
> is the discussion you are referring to).

Yep, we're going to need to trust the vendor driver to manage it, we
have lots of places where we need to trust the vendor driver for an
mdev device, unfortunately.
 
> I can't see a compelling reason for the anon-fd though.  I suspect this
> was done due to a misunderstanding ...

Yeah, vfio-core passes device ioctls to the vendor driver, so the
vendor driver should be able to implement a
VFIO_DEVICE_GVT_GET_DMABUF_FD ioctl direclty.  Ideally maybe this
isn't even GVT specific, and we'd s/GVT_//.  Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Fri, 05 May 2017 08:55:31 +0200
Gerd Hoffmann  wrote:

>   Hi,
> 
> > > >>Hmm, that looks like a rather strange way to return a file descriptor.
> > > >>
> > > >>What is the reason to not use ioctls on the vfio file handle, like
> > > >>older version of these patches did?
> > > >If I understood correctly that Alex prefer not to change the ioctls on 
> > > >the vfio file
> > > >handle like the old version.
> > > >So I used this way the smallest change to general vfio framework only 
> > > >adding a
> > > >subregion definition.  
> > 
> > I think I was hoping we could avoid a separate file descriptor
> > altogether and use a vfio region instead.  
> 
> What exactly did you have in mind?  Put the framebuffer information
> (struct intel_vgpu_dmabuf) into the vfio region, then access it using
> read/write/mmap?

Yeah, that was my hope.  Adding a new file descriptor means we have one
more reference floating around complicating the life cycle of the
device, group, and container.  Furthermore this one is really only
visible to the mdev vendor driver, so we can't rely on vfio-core, the
vendor driver will need to consider the reference when releasing the
device.

> > However, it was explained
> > previously why this really needs to be a separate fd and I agree that
> > using a region to expose an fd is really awkward.  
> 
> Now with this patchset we have *two* kinds of separate file handles.
> First the anon-fd created by reading from the region.  This is then used
> to run the intel ioctls on, which in turn create the other kind of file
> handle (dma-buf-fd).
> 
> The dma-buf-fd really needs to be a separate fd, because it gets passed
> around as handle and because this is the way dma-bufs work (guess this
> is the discussion you are referring to).

Yep, we're going to need to trust the vendor driver to manage it, we
have lots of places where we need to trust the vendor driver for an
mdev device, unfortunately.
 
> I can't see a compelling reason for the anon-fd though.  I suspect this
> was done due to a misunderstanding ...

Yeah, vfio-core passes device ioctls to the vendor driver, so the
vendor driver should be able to implement a
VFIO_DEVICE_GVT_GET_DMABUF_FD ioctl direclty.  Ideally maybe this
isn't even GVT specific, and we'd s/GVT_//.  Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> > >>Hmm, that looks like a rather strange way to return a file descriptor.
> > >>
> > >>What is the reason to not use ioctls on the vfio file handle, like
> > >>older version of these patches did?  
> > >If I understood correctly that Alex prefer not to change the ioctls on the 
> > >vfio file
> > >handle like the old version.
> > >So I used this way the smallest change to general vfio framework only 
> > >adding a
> > >subregion definition.
> 
> I think I was hoping we could avoid a separate file descriptor
> altogether and use a vfio region instead.

What exactly did you have in mind?  Put the framebuffer information
(struct intel_vgpu_dmabuf) into the vfio region, then access it using
read/write/mmap?

> However, it was explained
> previously why this really needs to be a separate fd and I agree that
> using a region to expose an fd is really awkward.

Now with this patchset we have *two* kinds of separate file handles.
First the anon-fd created by reading from the region.  This is then used
to run the intel ioctls on, which in turn create the other kind of file
handle (dma-buf-fd).

The dma-buf-fd really needs to be a separate fd, because it gets passed
around as handle and because this is the way dma-bufs work (guess this
is the discussion you are referring to).

I can't see a compelling reason for the anon-fd though.  I suspect this
was done due to a misunderstanding ...

cheers,
  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

  Hi,

> > >>Hmm, that looks like a rather strange way to return a file descriptor.
> > >>
> > >>What is the reason to not use ioctls on the vfio file handle, like
> > >>older version of these patches did?  
> > >If I understood correctly that Alex prefer not to change the ioctls on the 
> > >vfio file
> > >handle like the old version.
> > >So I used this way the smallest change to general vfio framework only 
> > >adding a
> > >subregion definition.
> 
> I think I was hoping we could avoid a separate file descriptor
> altogether and use a vfio region instead.

What exactly did you have in mind?  Put the framebuffer information
(struct intel_vgpu_dmabuf) into the vfio region, then access it using
read/write/mmap?

> However, it was explained
> previously why this really needs to be a separate fd and I agree that
> using a region to expose an fd is really awkward.

Now with this patchset we have *two* kinds of separate file handles.
First the anon-fd created by reading from the region.  This is then used
to run the intel ioctls on, which in turn create the other kind of file
handle (dma-buf-fd).

The dma-buf-fd really needs to be a separate fd, because it gets passed
around as handle and because this is the way dma-bufs work (guess this
is the discussion you are referring to).

I can't see a compelling reason for the anon-fd though.  I suspect this
was done due to a misunderstanding ...

cheers,
  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Thu, 4 May 2017 03:09:40 +
"Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:

> Hi Alex, do you have any comments for this interface?
> 
> >-Original Message-
> >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
> >Behalf Of Chen, Xiaoguang
> >Sent: Wednesday, May 03, 2017 9:39 AM
> >To: Gerd Hoffmann <kra...@redhat.com>
> >Cc: Tian, Kevin <kevin.t...@intel.com>; intel-...@lists.freedesktop.org; 
> >linux-
> >ker...@vger.kernel.org; zhen...@linux.intel.com; alex.william...@redhat.com;
> >Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; 
> >Wang,
> >Zhi A <zhi.a.w...@intel.com>
> >Subject: RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >
> >  
> >>-Original Message-
> >>From: Gerd Hoffmann [mailto:kra...@redhat.com]
> >>Sent: Tuesday, May 02, 2017 5:51 PM
> >>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
> >>Cc: alex.william...@redhat.com; intel-...@lists.freedesktop.org;
> >>intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
> >><zhi.a.w...@intel.com>; zhen...@linux.intel.com;
> >>linux-kernel@vger.kernel.org; Lv, Zhiyuan <zhiyuan...@intel.com>; Tian,
> >>Kevin <kevin.t...@intel.com>
> >>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
> >>dmabuf
> >>
> >>On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:  
> >>> +static size_t intel_vgpu_reg_rw_gvtg(struct intel_vgpu *vgpu, char
> >>> *buf,
> >>> +   size_t count, loff_t *ppos, bool iswrite) {
> >>> +   unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
> >>> +   VFIO_PCI_NUM_REGIONS;
> >>> +   loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> >>> +   int fd;
> >>> +
> >>> +   if (pos >= vgpu->vdev.region[i].size || iswrite) {
> >>> +   gvt_vgpu_err("invalid op or offset for Intel vgpu fd
> >>> region\n");
> >>> +   return -EINVAL;
> >>> +   }
> >>> +
> >>> +   fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
> >>> +   O_RDWR | O_CLOEXEC);
> >>> +   if (fd < 0) {
> >>> +   gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
> >>> +   return -EINVAL;
> >>> +   }
> >>> +
> >>> +   count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
> >>> +   memcpy(buf, , count);
> >>> +
> >>> +   return count;
> >>> +}  
> >>
> >>Hmm, that looks like a rather strange way to return a file descriptor.
> >>
> >>What is the reason to not use ioctls on the vfio file handle, like
> >>older version of these patches did?  
> >If I understood correctly that Alex prefer not to change the ioctls on the 
> >vfio file
> >handle like the old version.
> >So I used this way the smallest change to general vfio framework only adding 
> >a
> >subregion definition.

I think I was hoping we could avoid a separate file descriptor
altogether and use a vfio region instead.  However, it was explained
previously why this really needs to be a separate fd and I agree that
using a region to expose an fd is really awkward.  If we're going to
have a separate fd, let's use a device specific ioctl to get it.
Thanks,

Alex

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Alex Williamson]

On Thu, 4 May 2017 03:09:40 +
"Chen, Xiaoguang"  wrote:

> Hi Alex, do you have any comments for this interface?
> 
> >-Original Message-
> >From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
> >Behalf Of Chen, Xiaoguang
> >Sent: Wednesday, May 03, 2017 9:39 AM
> >To: Gerd Hoffmann 
> >Cc: Tian, Kevin ; intel-...@lists.freedesktop.org; 
> >linux-
> >ker...@vger.kernel.org; zhen...@linux.intel.com; alex.william...@redhat.com;
> >Lv, Zhiyuan ; intel-gvt-...@lists.freedesktop.org; 
> >Wang,
> >Zhi A 
> >Subject: RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
> >
> >
> >  
> >>-Original Message-
> >>From: Gerd Hoffmann [mailto:kra...@redhat.com]
> >>Sent: Tuesday, May 02, 2017 5:51 PM
> >>To: Chen, Xiaoguang 
> >>Cc: alex.william...@redhat.com; intel-...@lists.freedesktop.org;
> >>intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
> >>; zhen...@linux.intel.com;
> >>linux-kernel@vger.kernel.org; Lv, Zhiyuan ; Tian,
> >>Kevin 
> >>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
> >>dmabuf
> >>
> >>On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:  
> >>> +static size_t intel_vgpu_reg_rw_gvtg(struct intel_vgpu *vgpu, char
> >>> *buf,
> >>> +   size_t count, loff_t *ppos, bool iswrite) {
> >>> +   unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
> >>> +   VFIO_PCI_NUM_REGIONS;
> >>> +   loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> >>> +   int fd;
> >>> +
> >>> +   if (pos >= vgpu->vdev.region[i].size || iswrite) {
> >>> +   gvt_vgpu_err("invalid op or offset for Intel vgpu fd
> >>> region\n");
> >>> +   return -EINVAL;
> >>> +   }
> >>> +
> >>> +   fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
> >>> +   O_RDWR | O_CLOEXEC);
> >>> +   if (fd < 0) {
> >>> +   gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
> >>> +   return -EINVAL;
> >>> +   }
> >>> +
> >>> +   count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
> >>> +   memcpy(buf, , count);
> >>> +
> >>> +   return count;
> >>> +}  
> >>
> >>Hmm, that looks like a rather strange way to return a file descriptor.
> >>
> >>What is the reason to not use ioctls on the vfio file handle, like
> >>older version of these patches did?  
> >If I understood correctly that Alex prefer not to change the ioctls on the 
> >vfio file
> >handle like the old version.
> >So I used this way the smallest change to general vfio framework only adding 
> >a
> >subregion definition.

I think I was hoping we could avoid a separate file descriptor
altogether and use a vfio region instead.  However, it was explained
previously why this really needs to be a separate fd and I agree that
using a region to expose an fd is really awkward.  If we're going to
have a separate fd, let's use a device specific ioctl to get it.
Thanks,

Alex

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex, do you have any comments for this interface?

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Chen, Xiaoguang
>Sent: Wednesday, May 03, 2017 9:39 AM
>To: Gerd Hoffmann <kra...@redhat.com>
>Cc: Tian, Kevin <kevin.t...@intel.com>; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; alex.william...@redhat.com;
>Lv, Zhiyuan <zhiyuan...@intel.com>; intel-gvt-...@lists.freedesktop.org; Wang,
>Zhi A <zhi.a.w...@intel.com>
>Subject: RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>
>
>>-Original Message-
>>From: Gerd Hoffmann [mailto:kra...@redhat.com]
>>Sent: Tuesday, May 02, 2017 5:51 PM
>>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>>Cc: alex.william...@redhat.com; intel-...@lists.freedesktop.org;
>>intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
>><zhi.a.w...@intel.com>; zhen...@linux.intel.com;
>>linux-kernel@vger.kernel.org; Lv, Zhiyuan <zhiyuan...@intel.com>; Tian,
>>Kevin <kevin.t...@intel.com>
>>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>>dmabuf
>>
>>On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:
>>> +static size_t intel_vgpu_reg_rw_gvtg(struct intel_vgpu *vgpu, char
>>> *buf,
>>> +   size_t count, loff_t *ppos, bool iswrite) {
>>> +   unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
>>> +   VFIO_PCI_NUM_REGIONS;
>>> +   loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>>> +   int fd;
>>> +
>>> +   if (pos >= vgpu->vdev.region[i].size || iswrite) {
>>> +   gvt_vgpu_err("invalid op or offset for Intel vgpu fd
>>> region\n");
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
>>> +   O_RDWR | O_CLOEXEC);
>>> +   if (fd < 0) {
>>> +   gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
>>> +   memcpy(buf, , count);
>>> +
>>> +   return count;
>>> +}
>>
>>Hmm, that looks like a rather strange way to return a file descriptor.
>>
>>What is the reason to not use ioctls on the vfio file handle, like
>>older version of these patches did?
>If I understood correctly that Alex prefer not to change the ioctls on the 
>vfio file
>handle like the old version.
>So I used this way the smallest change to general vfio framework only adding a
>subregion definition.
>
>>
>>cheers,
>>  Gerd
>
>___
>intel-gvt-dev mailing list
>intel-gvt-...@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

Hi Alex, do you have any comments for this interface?

>-Original Message-
>From: intel-gvt-dev [mailto:intel-gvt-dev-boun...@lists.freedesktop.org] On
>Behalf Of Chen, Xiaoguang
>Sent: Wednesday, May 03, 2017 9:39 AM
>To: Gerd Hoffmann 
>Cc: Tian, Kevin ; intel-...@lists.freedesktop.org; linux-
>ker...@vger.kernel.org; zhen...@linux.intel.com; alex.william...@redhat.com;
>Lv, Zhiyuan ; intel-gvt-...@lists.freedesktop.org; Wang,
>Zhi A 
>Subject: RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>
>
>>-Original Message-
>>From: Gerd Hoffmann [mailto:kra...@redhat.com]
>>Sent: Tuesday, May 02, 2017 5:51 PM
>>To: Chen, Xiaoguang 
>>Cc: alex.william...@redhat.com; intel-...@lists.freedesktop.org;
>>intel-gvt- d...@lists.freedesktop.org; Wang, Zhi A
>>; zhen...@linux.intel.com;
>>linux-kernel@vger.kernel.org; Lv, Zhiyuan ; Tian,
>>Kevin 
>>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the
>>dmabuf
>>
>>On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:
>>> +static size_t intel_vgpu_reg_rw_gvtg(struct intel_vgpu *vgpu, char
>>> *buf,
>>> +   size_t count, loff_t *ppos, bool iswrite) {
>>> +   unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
>>> +   VFIO_PCI_NUM_REGIONS;
>>> +   loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>>> +   int fd;
>>> +
>>> +   if (pos >= vgpu->vdev.region[i].size || iswrite) {
>>> +   gvt_vgpu_err("invalid op or offset for Intel vgpu fd
>>> region\n");
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
>>> +   O_RDWR | O_CLOEXEC);
>>> +   if (fd < 0) {
>>> +   gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
>>> +   memcpy(buf, , count);
>>> +
>>> +   return count;
>>> +}
>>
>>Hmm, that looks like a rather strange way to return a file descriptor.
>>
>>What is the reason to not use ioctls on the vfio file handle, like
>>older version of these patches did?
>If I understood correctly that Alex prefer not to change the ioctls on the 
>vfio file
>handle like the old version.
>So I used this way the smallest change to general vfio framework only adding a
>subregion definition.
>
>>
>>cheers,
>>  Gerd
>
>___
>intel-gvt-dev mailing list
>intel-gvt-...@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

>-Original Message-
>From: Gerd Hoffmann [mailto:kra...@redhat.com]
>Sent: Tuesday, May 02, 2017 5:51 PM
>To: Chen, Xiaoguang <xiaoguang.c...@intel.com>
>Cc: alex.william...@redhat.com; intel-...@lists.freedesktop.org; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A <zhi.a.w...@intel.com>;
>zhen...@linux.intel.com; linux-kernel@vger.kernel.org; Lv, Zhiyuan
><zhiyuan...@intel.com>; Tian, Kevin <kevin.t...@intel.com>
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:
>> +static size_t intel_vgpu_reg_rw_gvtg(struct intel_vgpu *vgpu, char
>> *buf,
>> +   size_t count, loff_t *ppos, bool iswrite) {
>> +   unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
>> +   VFIO_PCI_NUM_REGIONS;
>> +   loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>> +   int fd;
>> +
>> +   if (pos >= vgpu->vdev.region[i].size || iswrite) {
>> +   gvt_vgpu_err("invalid op or offset for Intel vgpu fd
>> region\n");
>> +   return -EINVAL;
>> +   }
>> +
>> +   fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
>> +   O_RDWR | O_CLOEXEC);
>> +   if (fd < 0) {
>> +   gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
>> +   return -EINVAL;
>> +   }
>> +
>> +   count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
>> +   memcpy(buf, , count);
>> +
>> +   return count;
>> +}
>
>Hmm, that looks like a rather strange way to return a file descriptor.
>
>What is the reason to not use ioctls on the vfio file handle, like older 
>version of
>these patches did?
If I understood correctly that Alex prefer not to change the ioctls on the vfio 
file handle like the old version.
So I used this way the smallest change to general vfio framework only adding a 
subregion definition.

>
>cheers,
>  Gerd

RE: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Chen, Xiaoguang]

>-Original Message-
>From: Gerd Hoffmann [mailto:kra...@redhat.com]
>Sent: Tuesday, May 02, 2017 5:51 PM
>To: Chen, Xiaoguang 
>Cc: alex.william...@redhat.com; intel-...@lists.freedesktop.org; intel-gvt-
>d...@lists.freedesktop.org; Wang, Zhi A ;
>zhen...@linux.intel.com; linux-kernel@vger.kernel.org; Lv, Zhiyuan
>; Tian, Kevin 
>Subject: Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf
>
>On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:
>> +static size_t intel_vgpu_reg_rw_gvtg(struct intel_vgpu *vgpu, char
>> *buf,
>> +   size_t count, loff_t *ppos, bool iswrite) {
>> +   unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
>> +   VFIO_PCI_NUM_REGIONS;
>> +   loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>> +   int fd;
>> +
>> +   if (pos >= vgpu->vdev.region[i].size || iswrite) {
>> +   gvt_vgpu_err("invalid op or offset for Intel vgpu fd
>> region\n");
>> +   return -EINVAL;
>> +   }
>> +
>> +   fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
>> +   O_RDWR | O_CLOEXEC);
>> +   if (fd < 0) {
>> +   gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
>> +   return -EINVAL;
>> +   }
>> +
>> +   count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
>> +   memcpy(buf, , count);
>> +
>> +   return count;
>> +}
>
>Hmm, that looks like a rather strange way to return a file descriptor.
>
>What is the reason to not use ioctls on the vfio file handle, like older 
>version of
>these patches did?
If I understood correctly that Alex prefer not to change the ioctls on the vfio 
file handle like the old version.
So I used this way the smallest change to general vfio framework only adding a 
subregion definition.

>
>cheers,
>  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:
> +static size_t intel_vgpu_reg_rw_gvtg(struct intel_vgpu *vgpu, char
> *buf,
> +   size_t count, loff_t *ppos, bool iswrite)
> +{
> +   unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
> +   VFIO_PCI_NUM_REGIONS;
> +   loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> +   int fd;
> +
> +   if (pos >= vgpu->vdev.region[i].size || iswrite) {
> +   gvt_vgpu_err("invalid op or offset for Intel vgpu fd
> region\n");
> +   return -EINVAL;
> +   }
> +
> +   fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
> +   O_RDWR | O_CLOEXEC);
> +   if (fd < 0) {
> +   gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
> +   return -EINVAL;
> +   }
> +
> +   count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
> +   memcpy(buf, , count);
> +
> +   return count;
> +}

Hmm, that looks like a rather strange way to return a file descriptor.

What is the reason to not use ioctls on the vfio file handle, like older
version of these patches did?

cheers,
  Gerd

Re: [RFC PATCH 6/6] drm/i915/gvt: support QEMU getting the dmabuf

[Gerd Hoffmann]

On Fr, 2017-04-28 at 17:35 +0800, Xiaoguang Chen wrote:
> +static size_t intel_vgpu_reg_rw_gvtg(struct intel_vgpu *vgpu, char
> *buf,
> +   size_t count, loff_t *ppos, bool iswrite)
> +{
> +   unsigned int i = VFIO_PCI_OFFSET_TO_INDEX(*ppos) -
> +   VFIO_PCI_NUM_REGIONS;
> +   loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> +   int fd;
> +
> +   if (pos >= vgpu->vdev.region[i].size || iswrite) {
> +   gvt_vgpu_err("invalid op or offset for Intel vgpu fd
> region\n");
> +   return -EINVAL;
> +   }
> +
> +   fd = anon_inode_getfd("gvtg", _vgpu_gvtg_ops, vgpu,
> +   O_RDWR | O_CLOEXEC);
> +   if (fd < 0) {
> +   gvt_vgpu_err("create intel vgpu fd failed:%d\n", fd);
> +   return -EINVAL;
> +   }
> +
> +   count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
> +   memcpy(buf, , count);
> +
> +   return count;
> +}

Hmm, that looks like a rather strange way to return a file descriptor.

What is the reason to not use ioctls on the vfio file handle, like older
version of these patches did?

cheers,
  Gerd