>-----Original Message----- >From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx] >Sent: Wednesday, June 14, 2017 5:25 AM >To: Chen, Xiaoguang <xiaoguang.chen@xxxxxxxxx> >Cc: kraxel@xxxxxxxxxx; chris@xxxxxxxxxxxxxxxxxx; intel- >gfx@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >zhenyuw@xxxxxxxxxxxxxxx; Lv, Zhiyuan <zhiyuan.lv@xxxxxxxxx>; intel-gvt- >dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>; Tian, Kevin ><kevin.tian@xxxxxxxxx> >Subject: Re: [PATCH v8 4/6] vfio: Define vfio based vgpu's dma-buf operations > >On Fri, 9 Jun 2017 14:50:40 +0800 >Xiaoguang Chen <xiaoguang.chen@xxxxxxxxx> wrote: > >> Here we defined a new ioctl to create a fd for a vfio device based on >> the input type. Now only one type is supported that is a dma-buf >> management fd. >> Two ioctls are defined for the dma-buf management fd: query the vfio >> vgpu's plane information and create a dma-buf for a plane. >> >> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@xxxxxxxxx> >> --- >> include/uapi/linux/vfio.h | 58 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 58 insertions(+) >> >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h >> index ae46105..24427b7 100644 >> --- a/include/uapi/linux/vfio.h >> +++ b/include/uapi/linux/vfio.h >> @@ -502,6 +502,64 @@ struct vfio_pci_hot_reset { >> >> #define VFIO_DEVICE_PCI_HOT_RESET _IO(VFIO_TYPE, VFIO_BASE + 13) >> >> +/** >> + * VFIO_DEVICE_GET_FD - _IO(VFIO_TYPE, VFIO_BASE + 14, __u32) >> + * >> + * Create a fd for a vfio device based on the input type >> + * Vendor driver should handle this ioctl to create a fd and manage >> +the >> + * life cycle of this fd. >> + * >> + * Return: a fd if vendor support that type, -errno if not supported >> +*/ >> + >> +#define VFIO_DEVICE_GET_FD _IO(VFIO_TYPE, VFIO_BASE + 14) >> + >> +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; >> +}; >> + >> +#define VFIO_DEVICE_DMABUF_MGR_FD 0 /* Supported fd types */ > >Move this #define up above vfio_vgpu_plane_info to associate it with the >VFIO_DEVICE_GET_FD ioctl. OK. > >> + >> +/* >> + * VFIO_DEVICE_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15, >> + * struct vfio_vgpu_query_plane) >> + * Query plane information >> + */ >> +struct vfio_vgpu_query_plane { >> + __u32 argsz; >> + __u32 flags; >> + struct vfio_vgpu_plane_info plane_info; >> + __u32 plane_id; >> + __u32 padding; > >This padding doesn't make sense. This padding is still needed if we do not move the plane_id into vfio_vgpu_plane_info. Right? > >> +}; >> + >> +#define VFIO_DEVICE_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15) >> + >> +/* >> + * VFIO_DEVICE_CREATE_DMABUF - _IO(VFIO, VFIO_BASE + 16, >> + * struct >vfio_vgpu_create_dmabuf) >> + * >> + * Create a dma-buf for a plane >> + */ >> +struct vfio_vgpu_create_dmabuf { >> + __u32 argsz; >> + __u32 flags; >> + struct vfio_vgpu_plane_info plane_info; >> + __s32 fd; >> + __u32 plane_id; >> +}; > >Both of these have a plane_id, should plane_id simply replace the padding in >plane_info? Precisely speaking plane_id does not belong to the plane info. All the other information are decoded from plane except plane id. >If not, let's at least put them in the same order so that plane_id is >after plane_info for both structs. Ok. > >> + >> +#define VFIO_DEVICE_CREATE_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 16) > >I don't think these should be named just VFIO_DEVICE_foo, that implies they're >ioctls on the vfio device fd, they're not. They need to be associated both in name >and more complete descriptions as ioctls to the fd returned from a request for a >VFIO_DEVICE_DMABUF_MGR_FD. Perhaps VFIO_DMABUF_MGR_QUERY_PLANE >and VFIO_DMABUF_MGR_CREATE_DMABUF. I'm also not sure why we're using >"vgpu" in the structure names here either, the ioctls aren't named after vgpus. >Aren't these rather generic to graphics dmabufs, not specifically vgpus? Make sense. I will change the names. Thanks, > >Alex > >> + >> /* -------- API for Type1 VFIO IOMMU -------- */ >> >> /** _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
