On Thu, 1 Jun 2017 03:01:28 +0000 "Chen, Xiaoguang" <[email protected]> wrote:
> Hi Kirti, > > >-----Original Message----- > >From: Kirti Wankhede [mailto:[email protected]] > >Sent: Thursday, June 01, 2017 1:23 AM > >To: Chen, Xiaoguang <[email protected]>; Gerd Hoffmann > ><[email protected]>; [email protected]; [email protected]; > >[email protected]; [email protected]; > >[email protected]; Lv, Zhiyuan <[email protected]>; intel-gvt- > >[email protected]; Wang, Zhi A <[email protected]>; Tian, Kevin > ><[email protected]> > >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:[email protected]] > >>> Sent: Monday, May 29, 2017 3:20 PM > >>> To: Chen, Xiaoguang <[email protected]>; > >>> [email protected]; [email protected]; intel- > >>> [email protected]; [email protected]; > >>> [email protected]; Lv, Zhiyuan <[email protected]>; > >>> intel-gvt- [email protected]; Wang, Zhi A > >>> <[email protected]>; Tian, Kevin <[email protected]> > >>> 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
