Re: [PATCH v5 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 23 May 2017 18:32:01 +0800
Xiaoguang Chen <xiaoguang.chen@xxxxxxxxx> wrote:

> User space will try to create a management fd for the dma-buf operation.
> Using this management fd user can query the plane information and create
> a dma-buf fd if necessary.
> GVT-g will handle the life cycle of the management fd and will align the
> life cycle of the fd with the vfio device.
> User space should handle the life cycle of the created dma-buf fd close
> the dma-buf fd timely when finishing use.

The user cannot be expected to do this, each dmabuf fd also needs to
take a reference to the vfio device.  You'll need to find a way to hook
into the dmabuf release function.
 
> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/gvt/dmabuf.c     |  25 ++++----
>  drivers/gpu/drm/i915/gvt/dmabuf.h     |  21 -------
>  drivers/gpu/drm/i915/gvt/fb_decoder.h |   2 -
>  drivers/gpu/drm/i915/gvt/gvt.c        |   2 +
>  drivers/gpu/drm/i915/gvt/gvt.h        |   4 ++
>  drivers/gpu/drm/i915/gvt/kvmgt.c      | 107 ++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vfio.h             |  50 +++++++++++++++-
>  7 files changed, 175 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c
> index 415453b..a72b86efb 100644
> --- a/drivers/gpu/drm/i915/gvt/dmabuf.c
> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
> @@ -29,6 +29,7 @@
>  
>  #include <linux/dma-buf.h>
>  #include <drm/drmP.h>
> +#include <linux/vfio.h>
>  
>  #include "i915_drv.h"
>  #include "gvt.h"
> @@ -45,9 +46,9 @@ static struct sg_table *intel_vgpu_gem_get_pages(
>  	int i, ret;
>  	gen8_pte_t __iomem *gtt_entries;
>  	unsigned int fb_gma = 0, fb_size = 0;
> -	struct intel_vgpu_plane_info *plane_info;
> +	struct plane_info *plane_info;
>  
> -	plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info;
> +	plane_info = (struct plane_info *)obj->gvt_plane_info;
>  	if (WARN_ON(!plane_info))
>  		return ERR_PTR(-EINVAL);
>  
> @@ -81,9 +82,9 @@ static struct sg_table *intel_vgpu_gem_get_pages(
>  static void intel_vgpu_gem_put_pages(struct drm_i915_gem_object *obj,
>  		struct sg_table *pages)
>  {
> -	struct intel_vgpu_plane_info *plane_info;
> +	struct plane_info *plane_info;
>  
> -	plane_info = (struct intel_vgpu_plane_info *)obj->gvt_plane_info;
> +	plane_info = (struct plane_info *)obj->gvt_plane_info;
>  	if (WARN_ON(!plane_info))
>  		return;
>  
> @@ -98,7 +99,7 @@ static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = {
>  };
>  
>  static struct drm_i915_gem_object *intel_vgpu_create_gem(struct drm_device *dev,
> -		struct intel_vgpu_plane_info *info)
> +		struct plane_info *info)
>  {
>  	struct drm_i915_private *pri = dev->dev_private;
>  	struct drm_i915_gem_object *obj;
> @@ -141,14 +142,14 @@ static struct drm_i915_gem_object *intel_vgpu_create_gem(struct drm_device *dev,
>  	return obj;
>  }
>  
> -static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(
> +static struct plane_info *intel_vgpu_get_plane_info(
>  		struct drm_device *dev,
>  		struct intel_vgpu *vgpu, uint32_t plane_id)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_vgpu_primary_plane_format *p;
>  	struct intel_vgpu_cursor_plane_format *c;
> -	struct intel_vgpu_plane_info *info;
> +	struct plane_info *info;
>  	struct intel_vgpu_pipe_format *pipe;
>  
>  	info = kmalloc(sizeof(*info), GFP_KERNEL);
> @@ -159,7 +160,7 @@ static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(
>  	if (pipe == NULL)
>  		return NULL;
>  
> -	if (plane_id == INTEL_GVT_PLANE_PRIMARY) {
> +	if (plane_id == VFIO_PRIMARY_PLANE) {
>  		p = &pipe->primary;
>  		if (p != NULL) {
>  			info->start = p->base;
> @@ -175,7 +176,7 @@ static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(
>  			gvt_vgpu_err("invalid primary plane\n");
>  			return NULL;
>  		}
> -	} else if (plane_id == INTEL_GVT_PLANE_CURSOR) {
> +	} else if (plane_id == VFIO_CURSOR_PLANE) {
>  		c = &pipe->cursor;
>  		if (c != NULL) {
>  			info->start = c->base;
> @@ -228,7 +229,7 @@ static struct intel_vgpu_plane_info *intel_vgpu_get_plane_info(
>  int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args)
>  {
>  	struct drm_device *dev = &vgpu->gvt->dev_priv->drm;
> -	struct intel_vgpu_plane_info *info = args;
> +	struct plane_info *info = args;
>  
>  	info = intel_vgpu_get_plane_info(dev, vgpu, info->plane_id);
>  	if (info == NULL)
> @@ -242,8 +243,8 @@ int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args)
>  	struct dma_buf *dmabuf;
>  	struct drm_i915_gem_object *obj;
>  	struct drm_device *dev = &vgpu->gvt->dev_priv->drm;
> -	struct intel_vgpu_dmabuf *gvt_dmabuf = args;
> -	struct intel_vgpu_plane_info *info;
> +	struct dmabuf_info *gvt_dmabuf = args;
> +	struct plane_info *info;
>  	int ret;
>  
>  	info = intel_vgpu_get_plane_info(dev, vgpu,
> diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.h b/drivers/gpu/drm/i915/gvt/dmabuf.h
> index 43562af..e49bd4f 100644
> --- a/drivers/gpu/drm/i915/gvt/dmabuf.h
> +++ b/drivers/gpu/drm/i915/gvt/dmabuf.h
> @@ -26,27 +26,6 @@
>  #ifndef _GVT_DMABUF_H_
>  #define _GVT_DMABUF_H_
>  
> -struct intel_vgpu_plane_info {
> -	uint32_t plane_id;
> -	uint32_t drm_format;
> -	uint32_t width;
> -	uint32_t height;
> -	uint32_t stride;
> -	uint32_t start;
> -	uint32_t x_pos;
> -	uint32_t y_pos;
> -	uint32_t size;
> -	uint64_t drm_format_mod;
> -};
> -
> -#define INTEL_VGPU_QUERY_PLANE		0
> -#define INTEL_VGPU_GENERATE_DMABUF	1
> -
> -struct intel_vgpu_dmabuf {
> -	uint32_t fd;
> -	struct intel_vgpu_plane_info plane_info;
> -};
> -
>  int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args);
>  int intel_vgpu_create_dmabuf(struct intel_vgpu *vgpu, void *args);
>  
> diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.h b/drivers/gpu/drm/i915/gvt/fb_decoder.h
> index 1fd597f..96248ed 100644
> --- a/drivers/gpu/drm/i915/gvt/fb_decoder.h
> +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.h
> @@ -81,8 +81,6 @@
>  #define _SPRITE_OFFSET_START_X_MASK	(0x1fff << _SPRITE_OFFSET_START_X_SHIFT)
>  #define _SPRITE_OFFSET_START_Y_MASK	(0xfff << _SPRITE_OFFSET_START_Y_SHIFT)
>  
> -#define INTEL_GVT_PLANE_PRIMARY		1
> -#define INTEL_GVT_PLANE_CURSOR		2
>  #define INTEL_GVT_PLANE_SPRITE		3
>  
>  enum GVT_FB_EVENT {
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 2032917..dbc3f86 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -54,6 +54,8 @@ static const struct intel_gvt_ops intel_gvt_ops = {
>  	.vgpu_reset = intel_gvt_reset_vgpu,
>  	.vgpu_activate = intel_gvt_activate_vgpu,
>  	.vgpu_deactivate = intel_gvt_deactivate_vgpu,
> +	.vgpu_query_plane = intel_vgpu_query_plane,
> +	.vgpu_create_dmabuf = intel_vgpu_create_dmabuf,
>  };
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 763a8c5..0be422d 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -185,8 +185,10 @@ struct intel_vgpu {
>  		struct kvm *kvm;
>  		struct work_struct release_work;
>  		atomic_t released;
> +		struct vfio_device *vfio_device;
>  	} vdev;
>  #endif
> +	int dmabuf_mgr_fd;
>  };
>  
>  struct intel_gvt_gm {
> @@ -467,6 +469,8 @@ struct intel_gvt_ops {
>  	void (*vgpu_reset)(struct intel_vgpu *);
>  	void (*vgpu_activate)(struct intel_vgpu *);
>  	void (*vgpu_deactivate)(struct intel_vgpu *);
> +	int (*vgpu_query_plane)(struct intel_vgpu *vgpu, void *);
> +	int (*vgpu_create_dmabuf)(struct intel_vgpu *vgpu, void *);
>  };
>  
>  
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 389f072..765d742 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -41,6 +41,7 @@
>  #include <linux/kvm_host.h>
>  #include <linux/vfio.h>
>  #include <linux/mdev.h>
> +#include <linux/anon_inodes.h>
>  
>  #include "i915_drv.h"
>  #include "gvt.h"
> @@ -524,6 +525,85 @@ static int intel_vgpu_reg_init_opregion(struct intel_vgpu *vgpu)
>  	return ret;
>  }
>  
> +static int intel_vgpu_dmabuf_mgr_fd_mmap(struct file *file,
> +		struct vm_area_struct *vma)
> +{
> +	return -EPERM;
> +}
> +
> +static int intel_vgpu_dmabuf_mgr_fd_release(struct inode *inode,
> +		struct file *filp)
> +{
> +	struct intel_vgpu *vgpu = filp->private_data;
> +
> +	if (WARN_ON(!vgpu->vdev.vfio_device))
> +		return -EINVAL;
> +
> +	vfio_device_put(vgpu->vdev.vfio_device);
> +
> +	return 0;
> +}
> +
> +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;
> +	int ret;
> +	struct fd f;
> +
> +	f = fdget(vgpu->dmabuf_mgr_fd);
> +	if (!f.file)
> +		return -EBADF;
> +
> +	if (ioctl == VFIO_DEVICE_QUERY_PLANE) {
> +		struct plane_info info;
> +
> +		minsz = offsetofend(struct plane_info, drm_format_mod);
> +		if (copy_from_user(&info, (void __user *)arg, minsz)) {
> +			fdput(f);
> +			return -EFAULT;
> +		}
> +		ret = intel_gvt_ops->vgpu_query_plane(vgpu, &info);
> +		if (ret != 0) {
> +			fdput(f);
> +			gvt_vgpu_err("query plane failed:%d\n", ret);
> +			return -EINVAL;
> +		}
> +		fdput(f);
> +		return copy_to_user((void __user *)arg, &info, minsz) ?
> +								-EFAULT : 0;
> +	} else if (ioctl == VFIO_DEVICE_CREATE_DMABUF) {
> +		struct dmabuf_info dmabuf;
> +
> +		minsz = offsetofend(struct dmabuf_info, plane_info);
> +		if (copy_from_user(&dmabuf, (void __user *)arg, minsz)) {
> +			fdput(f);
> +			return -EFAULT;
> +		}
> +		ret = intel_gvt_ops->vgpu_create_dmabuf(vgpu, &dmabuf);
> +		if (ret != 0) {
> +			fdput(f);
> +			gvt_vgpu_err("create dmabuf failed:%d\n", ret);
> +			return -EINVAL;
> +		}
> +		fdput(f);
> +		return copy_to_user((void __user *)arg, &dmabuf, minsz) ?
> +								-EFAULT : 0;
> +	}
> +
> +	fdput(f);
> +	gvt_vgpu_err("unsupported dmabuf operation\n");
> +
> +	return -EINVAL;
> +}
> +
> +static const struct file_operations intel_vgpu_dmabuf_mgr_fd_ops = {
> +	.release        = intel_vgpu_dmabuf_mgr_fd_release,
> +	.unlocked_ioctl = intel_vgpu_dmabuf_mgr_fd_ioctl,
> +	.mmap           = intel_vgpu_dmabuf_mgr_fd_mmap,
> +	.llseek         = noop_llseek,
> +};
>  static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
>  {
>  	struct intel_vgpu *vgpu = NULL;
> @@ -1259,6 +1339,33 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
>  	} else if (cmd == VFIO_DEVICE_RESET) {
>  		intel_gvt_ops->vgpu_reset(vgpu);
>  		return 0;
> +	} else if (cmd == VFIO_DEVICE_GET_FD) {
> +		int fd;
> +		u32 type;
> +		struct vfio_device *device;
> +
> +		if (copy_from_user(&type, (void __user *)arg, sizeof(type)))
> +			return -EINVAL;
> +		if (type != VFIO_DEVICE_DMABUF_MGR_FD)
> +			return -EINVAL;
> +
> +		device = vfio_device_get_from_dev(mdev_dev(mdev));
> +		if (device == NULL) {
> +			gvt_vgpu_err("kvmgt: vfio device is null\n");
> +			return -EINVAL;
> +		}
> +		vgpu->vdev.vfio_device = device;
> +
> +		fd = anon_inode_getfd("intel-vgpu-dmabuf-mgr-fd",
> +			&intel_vgpu_dmabuf_mgr_fd_ops,
> +			vgpu, O_RDWR | O_CLOEXEC);
> +		if (fd < 0) {
> +			gvt_vgpu_err("create dmabuf mgr fd failed\n");
> +			return -EINVAL;
> +		}
> +		vgpu->dmabuf_mgr_fd = fd;
> +
> +		return fd;
>  	}
>  
>  	return 0;
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index ae46105..285dc16 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -502,10 +502,58 @@ 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)
> +
> +#define VFIO_DEVICE_DMABUF_MGR_FD	0 /* Supported fd types */
> +
> +/*
> + * VFIO_DEVICE_QUERY_PLANE - _IO(VFIO_TYPE, VFIO_BASE + 15, struct plane_info)
> + * Query plane information for a plane
> + */
> +struct plane_info {
> +	__u32 plane_id;
> +	__u32 drm_format;
> +	__u32 width;
> +	__u32 height;
> +	__u32 stride;
> +	__u32 start;
> +	__u32 x_pos;
> +	__u32 y_pos;
> +	__u32 size;
> +	__u64 drm_format_mod;
> +};

vfio ioctls should have flags and argsz or else they shouldn't be
defined as _IO type.

> +
> +#define VFIO_PRIMARY_PLANE		1
> +#define VFIO_CURSOR_PLANE		2
> +
> +#define VFIO_DEVICE_QUERY_PLANE _IO(VFIO_TYPE, VFIO_BASE + 15)
> +
> +/*
> + * VFIO_DEVICE_CREATE_DMABUF - _IO(VFIO, VFIO_BASE + 16, struct dmabuf_info)
> + *
> + * Create a dma-buf for a plane
> + */
> +struct dmabuf_info {
> +	__u32 fd;
> +	struct plane_info plane_info;
> +};
> +#define VFIO_DEVICE_CREATE_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 16)
> +

Same here.  Shouldn't fd also be an __s32?

>  /* -------- API for Type1 VFIO IOMMU -------- */
>  
>  /**
> - * VFIO_IOMMU_GET_INFO - _IOR(VFIO_TYPE, VFIO_BASE + 12, struct vfio_iommu_info)
> + * VFIO_IOMMU_GET_INFO - _IOR(VFIO_TYPE, VFIO_BASE + 12,
> + *							struct vfio_iommu_info)

Unrelated?

>   *
>   * Retrieve information about the IOMMU object. Fills in provided
>   * struct vfio_iommu_info. Caller sets argsz.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux