On Thu, 1 Jun 2017 03:01:28 +0000 "Chen, Xiaoguang" <xiaoguang.c...@intel.com> wrote:
> Hi Kirti, > > >-----Original Message----- > >From: Kirti Wankhede [mailto:kwankh...@nvidia.com] > >Sent: Thursday, June 01, 2017 1:23 AM > >To: Chen, Xiaoguang <xiaoguang.c...@intel.com>; Gerd Hoffmann > ><kra...@redhat.com>; alex.william...@redhat.com; ch...@chris-wilson.co.uk; > >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>; Tian, Kevin > ><kevin.t...@intel.com> > >Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf operations > > > > > > > >On 5/31/2017 11:48 AM, Chen, Xiaoguang wrote: > >> Hi, > >> > >>> -----Original Message----- > >>> From: Gerd Hoffmann [mailto:kra...@redhat.com] > >>> Sent: Monday, May 29, 2017 3:20 PM > >>> To: Chen, Xiaoguang <xiaoguang.c...@intel.com>; > >>> alex.william...@redhat.com; ch...@chris-wilson.co.uk; intel- > >>> g...@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>; Tian, Kevin <kevin.t...@intel.com> > >>> Subject: Re: [PATCH v6 4/6] vfio: Define vfio based vgpu's dma-buf > >>> operations > >>> > >>>> +struct vfio_vgpu_dmabuf_info { > >>>> + __u32 argsz; > >>>> + __u32 flags; > >>>> + struct vfio_vgpu_plane_info plane_info; > >>>> + __s32 fd; > >>>> + __u32 pad; > >>>> +}; > >>> > >>> Hmm, now you have argsz and flags twice in vfio_vgpu_dmabuf_info ... > >>> > >>> I think we should have something like this: > >>> > >>> struct vfio_vgpu_plane_info { > >>> __u64 start; > >>> __u64 drm_format_mod; > >>> __u32 drm_format; > >>> __u32 width; > >>> __u32 height; > >>> __u32 stride; > >>> __u32 size; > >>> __u32 x_pos; > >>> __u32 y_pos; > >>> __u32 padding; > >>> }; > >>> > >>> struct vfio_vgpu_query_plane { > >>> __u32 argsz; > >>> __u32 flags; > >>> struct vfio_vgpu_plane_info plane_info; > >>> __u32 plane_id; > >>> __u32 padding; > >>> }; > >>> > >>> struct vfio_vgpu_create_dmabuf { > >>> __u32 argsz; > >>> __u32 flags; > >>> struct vfio_vgpu_plane_info plane_info; > >>> __u32 plane_id; > >>> __s32 fd; > >>> }; > >> Good suggestion will apply in the next version. > >> Thanks for review :) > >> > > > >Can you define what are the expected values of 'flags' would be? > Flags is not used in this case. It is defined to follow the rules of vfio > ioctls. An important note about flags, the vendor driver must validate it. If they don't and the user passes an arbitrary value there, then we have a backwards compatibility issue with ever attempting to use the flags field. The user passing in a flag unknown to the vendor driver should return an -EINVAL response. In this case, we haven't defined any flags, so the vendor driver needs to force the user to pass zero. Thanks, Alex
