Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1644026 > unrolled thread

[PATCH v2 0/5] drm/i915/gvt: Dma-buf support for GVT-g

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/5] drm/i915/gvt: Dma-buf support for GVT-g Xiaoguang Chen <xiaoguang.chen@intel.com> - 2017-05-18 12:00 +0200
    [PATCH v2 1/5] drm/i915/gvt: Extend the GVT-g architecture to support vfio device region Xiaoguang Chen <xiaoguang.chen@intel.com> - 2017-05-18 12:00 +0200
    [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g Xiaoguang Chen <xiaoguang.chen@intel.com> - 2017-05-18 12:00 +0200
      RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g "He, Min" <min.he@intel.com> - 2017-05-18 17:50 +0200
        RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g "Chen, Xiaoguang" <xiaoguang.chen@intel.com> - 2017-05-19 04:30 +0200
          RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g "He, Min" <min.he@intel.com> - 2017-05-22 04:30 +0200
    [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf Xiaoguang Chen <xiaoguang.chen@intel.com> - 2017-05-18 12:00 +0200
      Re: [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can  get the dma-buf Alex Williamson <alex.williamson@redhat.com> - 2017-05-19 18:40 +0200
        RE: [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can  get the dma-buf "Chen, Xiaoguang" <xiaoguang.chen@intel.com> - 2017-05-22 04:10 +0200

#1644026 — [PATCH v2 0/5] drm/i915/gvt: Dma-buf support for GVT-g

v2->v1:
1) create a management fd for dma-buf operations.
2) alloc gem object's backing storage in gem obj's get_pages() callback.

This patch set adds the dma-buf support for intel GVT-g.
dma-buf is a uniform mechanism to share DMA buffers across different
devices and sub-systems.
dma-buf for intel GVT-g is mainly used to share the vgpu's framebuffer
to other users or sub-systems so they can use the dma-buf to show the
desktop of a vm which uses intel vgpu.

The main idea is we create a gem object and set vgpu's framebuffer as
the backing storage of this gem object. And associate this gem obj
to a dma-buf object then export this dma-buf at the meantime
generate a file descriptor for this dma-buf. Finally deliver this file
descriptor to user space. And user can use this dma-buf fd to do render
or other operations.
User need to create a fd(for intel GVT-g dma-buf support it is a:dma-buf
management fd) then user can use this fd to query the plane information
or create a dma-buf. The life cycle of this fd is managed by GVT-g user
do not need to care about that.

We have an example program on how to use the dma-buf. You can download
the program to have a try. Good luck :)
git repo: https://github.com/01org/igvtg-qemu branch:kvmgt_dmabuf_example.

Xiaoguang Chen (5):
  drm/i915/gvt: Extend the GVT-g architecture to support vfio device
    region
  drm/i915/gvt: OpRegion support for GVT-g
  drm/i915/gvt: Frame buffer decoder support for GVT-g
  drm/i915/gvt: Dmabuf support for GVT-g
  drm/i915/gvt: Adding interface so user space can get the dma-buf

 drivers/gpu/drm/i915/gvt/Makefile     |   3 +-
 drivers/gpu/drm/i915/gvt/display.c    |   2 +-
 drivers/gpu/drm/i915/gvt/display.h    |   2 +
 drivers/gpu/drm/i915/gvt/dmabuf.c     | 321 ++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/dmabuf.h     |  44 +++
 drivers/gpu/drm/i915/gvt/fb_decoder.c | 487 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/fb_decoder.h | 166 ++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.c        |   2 +
 drivers/gpu/drm/i915/gvt/gvt.h        |   7 +
 drivers/gpu/drm/i915/gvt/kvmgt.c      | 207 ++++++++++++++-
 drivers/gpu/drm/i915/gvt/opregion.c   |  12 +-
 include/uapi/drm/i915_drm.h           |  29 ++
 include/uapi/linux/vfio.h             |  12 +
 13 files changed, 1287 insertions(+), 7 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.c
 create mode 100644 drivers/gpu/drm/i915/gvt/dmabuf.h
 create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.c
 create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.h

-- 
1.9.1

[toc] | [next] | [standalone]

#1644027 — [PATCH v2 1/5] drm/i915/gvt: Extend the GVT-g architecture to support vfio device region

Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
---
 drivers/gpu/drm/i915/gvt/kvmgt.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 1ae0b40..3c6a02b 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -53,11 +53,21 @@
 #define VFIO_PCI_INDEX_TO_OFFSET(index) ((u64)(index) << VFIO_PCI_OFFSET_SHIFT)
 #define VFIO_PCI_OFFSET_MASK    (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1)
 
+struct vfio_region;
+struct intel_vgpu_regops {
+	size_t (*rw)(struct intel_vgpu *vgpu, char *buf,
+			size_t count, loff_t *ppos, bool iswrite);
+	void (*release)(struct intel_vgpu *vgpu,
+			struct vfio_region *region);
+};
+
 struct vfio_region {
 	u32				type;
 	u32				subtype;
 	size_t				size;
 	u32				flags;
+	const struct intel_vgpu_regops	*ops;
+	void				*data;
 };
 
 struct kvmgt_pgfn {
@@ -642,7 +652,7 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,
 	int ret = -EINVAL;
 
 
-	if (index >= VFIO_PCI_NUM_REGIONS) {
+	if (index >= VFIO_PCI_NUM_REGIONS + vgpu->vdev.num_regions) {
 		gvt_vgpu_err("invalid index: %u\n", index);
 		return -EINVAL;
 	}
@@ -676,8 +686,11 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,
 	case VFIO_PCI_BAR5_REGION_INDEX:
 	case VFIO_PCI_VGA_REGION_INDEX:
 	case VFIO_PCI_ROM_REGION_INDEX:
+		break;
 	default:
-		gvt_vgpu_err("unsupported region: %u\n", index);
+		index -= VFIO_PCI_NUM_REGIONS;
+		return vgpu->vdev.region[index].ops->rw(vgpu, buf, count,
+				ppos, is_write);
 	}
 
 	return ret == 0 ? count : ret;
@@ -940,7 +953,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 
 		info.flags = VFIO_DEVICE_FLAGS_PCI;
 		info.flags |= VFIO_DEVICE_FLAGS_RESET;
-		info.num_regions = VFIO_PCI_NUM_REGIONS;
+		info.num_regions = VFIO_PCI_NUM_REGIONS +
+				vgpu->vdev.num_regions;
 		info.num_irqs = VFIO_PCI_NUM_IRQS;
 
 		return copy_to_user((void __user *)arg, &info, minsz) ?
@@ -1061,6 +1075,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 		}
 
 		if (caps.size) {
+			info.flags |= VFIO_REGION_INFO_FLAG_CAPS;
 			if (info.argsz < sizeof(info) + caps.size) {
 				info.argsz = sizeof(info) + caps.size;
 				info.cap_offset = 0;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1644029 — [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g

OpRegion is needed to support display related operation for
intel vgpu.

A vfio device region is added to intel vgpu to deliver the
host OpRegion information to user space so user space can
construct the OpRegion for vgpu.

Signed-off-by: Bing Niu <bing.niu@intel.com>
Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
---
 drivers/gpu/drm/i915/gvt/kvmgt.c    | 97 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/opregion.c | 12 ++++-
 2 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 3c6a02b..389f072 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -53,6 +53,8 @@
 #define VFIO_PCI_INDEX_TO_OFFSET(index) ((u64)(index) << VFIO_PCI_OFFSET_SHIFT)
 #define VFIO_PCI_OFFSET_MASK    (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1)
 
+#define OPREGION_SIGNATURE "IntelGraphicsMem"
+
 struct vfio_region;
 struct intel_vgpu_regops {
 	size_t (*rw)(struct intel_vgpu *vgpu, char *buf,
@@ -436,6 +438,92 @@ static void kvmgt_protect_table_del(struct kvmgt_guest_info *info,
 	}
 }
 
+static size_t intel_vgpu_reg_rw_opregion(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;
+	void *base = vgpu->vdev.region[i].data;
+	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
+
+	if (pos >= vgpu->vdev.region[i].size || iswrite) {
+		gvt_vgpu_err("invalid op or offset for Intel vgpu OpRegion\n");
+		return -EINVAL;
+	}
+	count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
+	memcpy(buf, base + pos, count);
+
+	return count;
+}
+
+static void intel_vgpu_reg_release_opregion(struct intel_vgpu *vgpu,
+		struct vfio_region *region)
+{
+	memunmap(region->data);
+}
+
+static const struct intel_vgpu_regops intel_vgpu_regops_opregion = {
+	.rw = intel_vgpu_reg_rw_opregion,
+	.release = intel_vgpu_reg_release_opregion,
+};
+
+static int intel_vgpu_register_reg(struct intel_vgpu *vgpu,
+		unsigned int type, unsigned int subtype,
+		const struct intel_vgpu_regops *ops,
+		size_t size, u32 flags, void *data)
+{
+	struct vfio_region *region;
+
+	region = krealloc(vgpu->vdev.region,
+			(vgpu->vdev.num_regions + 1) * sizeof(*region),
+			GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+
+	vgpu->vdev.region = region;
+	vgpu->vdev.region[vgpu->vdev.num_regions].type = type;
+	vgpu->vdev.region[vgpu->vdev.num_regions].subtype = subtype;
+	vgpu->vdev.region[vgpu->vdev.num_regions].ops = ops;
+	vgpu->vdev.region[vgpu->vdev.num_regions].size = size;
+	vgpu->vdev.region[vgpu->vdev.num_regions].flags = flags;
+	vgpu->vdev.region[vgpu->vdev.num_regions].data = data;
+	vgpu->vdev.num_regions++;
+
+	return 0;
+}
+
+static int intel_vgpu_reg_init_opregion(struct intel_vgpu *vgpu)
+{
+	unsigned int addr;
+	void *base;
+	int ret;
+
+	addr = vgpu->gvt->opregion.opregion_pa;
+	if (!addr || !(~addr))
+		return -ENODEV;
+
+	base = memremap(addr, OPREGION_SIZE, MEMREMAP_WB);
+	if (!base)
+		return -ENOMEM;
+
+	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
+		memunmap(base);
+		return -EINVAL;
+	}
+
+	ret = intel_vgpu_register_reg(vgpu,
+			PCI_VENDOR_ID_INTEL | VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
+			VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION,
+			&intel_vgpu_regops_opregion, OPREGION_SIZE,
+			VFIO_REGION_INFO_FLAG_READ, base);
+	if (ret) {
+		memunmap(base);
+		return ret;
+	}
+
+	return ret;
+}
+
 static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 {
 	struct intel_vgpu *vgpu = NULL;
@@ -467,6 +555,15 @@ static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 	vgpu->vdev.mdev = mdev;
 	mdev_set_drvdata(mdev, vgpu);
 
+	ret = intel_vgpu_reg_init_opregion(vgpu);
+	if (ret) {
+		gvt_vgpu_err("create OpRegion failed\n");
+		goto out;
+	}
+
+	gvt_dbg_core("create OpRegion succeeded for mdev:%s\n",
+			dev_name(mdev_dev(mdev)));
+
 	gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n",
 		     dev_name(mdev_dev(mdev)));
 	ret = 0;
diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c
index 3117991..99591bc 100644
--- a/drivers/gpu/drm/i915/gvt/opregion.c
+++ b/drivers/gpu/drm/i915/gvt/opregion.c
@@ -283,14 +283,22 @@ int intel_vgpu_emulate_opregion_request(struct intel_vgpu *vgpu, u32 swsci)
 {
 	u32 *scic, *parm;
 	u32 func, subfunc;
+	u32 scic_val, parm_val;
+	u32 gpa;
 
-	scic = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_SCIC;
-	parm = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_PARM;
+	memcpy(&gpa, vgpu_cfg_space(vgpu) + INTEL_GVT_PCI_OPREGION,
+			sizeof(gpa));
+	intel_gvt_hypervisor_read_gpa(vgpu, gpa + INTEL_GVT_OPREGION_SCIC,
+			&scic_val, sizeof(scic_val));
+	intel_gvt_hypervisor_read_gpa(vgpu, gpa + INTEL_GVT_OPREGION_PARM,
+			&parm_val, sizeof(parm_val));
 
 	if (!(swsci & SWSCI_SCI_SELECT)) {
 		gvt_vgpu_err("requesting SMI service\n");
 		return 0;
 	}
+	scic = &scic_val;
+	parm = &parm_val;
 	/* ignore non 0->1 trasitions */
 	if ((vgpu_cfg_space(vgpu)[INTEL_GVT_PCI_SWSCI]
 				& SWSCI_SCI_TRIGGER) ||
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1644703 — RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g

Xiaoguang,
I have some comments inline. Thanks.

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Xiaoguang Chen
> Sent: Thursday, May 18, 2017 2:50 AM
> To: alex.williamson@redhat.com; kraxel@redhat.com; intel-
> gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; intel-gvt-
> dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, Kevin
> <kevin.tian@intel.com>
> Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
> <xiaoguang.chen@intel.com>
> Subject: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
> 
> OpRegion is needed to support display related operation for
> intel vgpu.
> 
> A vfio device region is added to intel vgpu to deliver the
> host OpRegion information to user space so user space can
> construct the OpRegion for vgpu.
> 
> Signed-off-by: Bing Niu <bing.niu@intel.com>
> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c    | 97
> +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/opregion.c | 12 ++++-
>  2 files changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 3c6a02b..389f072 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -53,6 +53,8 @@
>  #define VFIO_PCI_INDEX_TO_OFFSET(index) ((u64)(index) <<
> VFIO_PCI_OFFSET_SHIFT)
>  #define VFIO_PCI_OFFSET_MASK    (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1)
> 
> +#define OPREGION_SIGNATURE "IntelGraphicsMem"
> +
>  struct vfio_region;
>  struct intel_vgpu_regops {
>  	size_t (*rw)(struct intel_vgpu *vgpu, char *buf,
> @@ -436,6 +438,92 @@ static void kvmgt_protect_table_del(struct
> kvmgt_guest_info *info,
>  	}
>  }
> 
> +static size_t intel_vgpu_reg_rw_opregion(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;
> +	void *base = vgpu->vdev.region[i].data;
> +	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> +
> +	if (pos >= vgpu->vdev.region[i].size || iswrite) {
> +		gvt_vgpu_err("invalid op or offset for Intel vgpu OpRegion\n");
> +		return -EINVAL;
> +	}
> +	count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
> +	memcpy(buf, base + pos, count);
> +
> +	return count;
> +}
> +
> +static void intel_vgpu_reg_release_opregion(struct intel_vgpu *vgpu,
> +		struct vfio_region *region)
> +{
> +	memunmap(region->data);
> +}
> +
> +static const struct intel_vgpu_regops intel_vgpu_regops_opregion = {
> +	.rw = intel_vgpu_reg_rw_opregion,
> +	.release = intel_vgpu_reg_release_opregion,
> +};
> +
> +static int intel_vgpu_register_reg(struct intel_vgpu *vgpu,
> +		unsigned int type, unsigned int subtype,
> +		const struct intel_vgpu_regops *ops,
> +		size_t size, u32 flags, void *data)
> +{
> +	struct vfio_region *region;
> +
> +	region = krealloc(vgpu->vdev.region,
> +			(vgpu->vdev.num_regions + 1) * sizeof(*region),
> +			GFP_KERNEL);
> +	if (!region)
> +		return -ENOMEM;
> +
> +	vgpu->vdev.region = region;
> +	vgpu->vdev.region[vgpu->vdev.num_regions].type = type;
> +	vgpu->vdev.region[vgpu->vdev.num_regions].subtype = subtype;
> +	vgpu->vdev.region[vgpu->vdev.num_regions].ops = ops;
> +	vgpu->vdev.region[vgpu->vdev.num_regions].size = size;
> +	vgpu->vdev.region[vgpu->vdev.num_regions].flags = flags;
> +	vgpu->vdev.region[vgpu->vdev.num_regions].data = data;
> +	vgpu->vdev.num_regions++;
> +
> +	return 0;
> +}
> +
> +static int intel_vgpu_reg_init_opregion(struct intel_vgpu *vgpu)
> +{
> +	unsigned int addr;
> +	void *base;
> +	int ret;
> +
> +	addr = vgpu->gvt->opregion.opregion_pa;
> +	if (!addr || !(~addr))
> +		return -ENODEV;
> +
> +	base = memremap(addr, OPREGION_SIZE, MEMREMAP_WB);
> +	if (!base)
> +		return -ENOMEM;
> +
> +	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
> +		memunmap(base);
> +		return -EINVAL;
> +	}
> +
> +	ret = intel_vgpu_register_reg(vgpu,
> +			PCI_VENDOR_ID_INTEL |
> VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
> +			VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION,
> +			&intel_vgpu_regops_opregion, OPREGION_SIZE,
> +			VFIO_REGION_INFO_FLAG_READ, base);
In current GVT code, we already have a function init_vgpu_opregion,
which will copy the physical opregion into a per vgpu structure: 
vgpu_opregion(vgpu)->va, why don’t you reuse this function and set the
vgpu_opregion(vgpu)->va to the intel_vgpu_register_reg function?

> +	if (ret) {
> +		memunmap(base);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
>  static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
>  {
>  	struct intel_vgpu *vgpu = NULL;
> @@ -467,6 +555,15 @@ static int intel_vgpu_create(struct kobject *kobj, struct
> mdev_device *mdev)
>  	vgpu->vdev.mdev = mdev;
>  	mdev_set_drvdata(mdev, vgpu);
> 
> +	ret = intel_vgpu_reg_init_opregion(vgpu);
> +	if (ret) {
> +		gvt_vgpu_err("create OpRegion failed\n");
> +		goto out;
> +	}
> +
> +	gvt_dbg_core("create OpRegion succeeded for mdev:%s\n",
> +			dev_name(mdev_dev(mdev)));
> +
>  	gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n",
>  		     dev_name(mdev_dev(mdev)));
>  	ret = 0;
> diff --git a/drivers/gpu/drm/i915/gvt/opregion.c
> b/drivers/gpu/drm/i915/gvt/opregion.c
> index 3117991..99591bc 100644
> --- a/drivers/gpu/drm/i915/gvt/opregion.c
> +++ b/drivers/gpu/drm/i915/gvt/opregion.c
> @@ -283,14 +283,22 @@ int intel_vgpu_emulate_opregion_request(struct
> intel_vgpu *vgpu, u32 swsci)
>  {
>  	u32 *scic, *parm;
>  	u32 func, subfunc;
> +	u32 scic_val, parm_val;
> +	u32 gpa;
> 
> -	scic = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_SCIC;
> -	parm = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_PARM;
> +	memcpy(&gpa, vgpu_cfg_space(vgpu) + INTEL_GVT_PCI_OPREGION,
> +			sizeof(gpa));
> +	intel_gvt_hypervisor_read_gpa(vgpu, gpa +
> INTEL_GVT_OPREGION_SCIC,
> +			&scic_val, sizeof(scic_val));
> +	intel_gvt_hypervisor_read_gpa(vgpu, gpa +
> INTEL_GVT_OPREGION_PARM,
> +			&parm_val, sizeof(parm_val));
As I said, you can call init_vgpu_opregion(), then vgpu_opregion(vgpu)->va will
be valid and you don’t need to change here. 

> 
>  	if (!(swsci & SWSCI_SCI_SELECT)) {
>  		gvt_vgpu_err("requesting SMI service\n");
>  		return 0;
>  	}
> +	scic = &scic_val;
> +	parm = &parm_val;
>  	/* ignore non 0->1 trasitions */
>  	if ((vgpu_cfg_space(vgpu)[INTEL_GVT_PCI_SWSCI]
>  				& SWSCI_SCI_TRIGGER) ||
> --
> 1.9.1
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

[toc] | [prev] | [next] | [standalone]

#1645134 — RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g

Hi min,

>-----Original Message-----
>From: He, Min
>Sent: Thursday, May 18, 2017 11:44 PM
>To: Chen, Xiaoguang <xiaoguang.chen@intel.com>;
>alex.williamson@redhat.com; kraxel@redhat.com; intel-
>gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
>zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; intel-gvt-
>dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, Kevin
><kevin.tian@intel.com>
>Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
><xiaoguang.chen@intel.com>
>Subject: RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
>
>Xiaoguang,
>I have some comments inline. Thanks.
>
>> -----Original Message-----
>> From: intel-gvt-dev
>> [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
>> Xiaoguang Chen
>> Sent: Thursday, May 18, 2017 2:50 AM
>> To: alex.williamson@redhat.com; kraxel@redhat.com; intel-
>> gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
>> zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>;
>> intel-gvt- dev@lists.freedesktop.org; Wang, Zhi A
>> <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
>> Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
>> <xiaoguang.chen@intel.com>
>> Subject: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
>>
>> OpRegion is needed to support display related operation for intel
>> vgpu.
>>
>> A vfio device region is added to intel vgpu to deliver the host
>> OpRegion information to user space so user space can construct the
>> OpRegion for vgpu.
>>
>> Signed-off-by: Bing Niu <bing.niu@intel.com>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gvt/kvmgt.c    | 97
>> +++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/gvt/opregion.c | 12 ++++-
>>  2 files changed, 107 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 3c6a02b..389f072 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -53,6 +53,8 @@
>>  #define VFIO_PCI_INDEX_TO_OFFSET(index) ((u64)(index) <<
>> VFIO_PCI_OFFSET_SHIFT)
>>  #define VFIO_PCI_OFFSET_MASK    (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1)
>>
>> +#define OPREGION_SIGNATURE "IntelGraphicsMem"
>> +
>>  struct vfio_region;
>>  struct intel_vgpu_regops {
>>  	size_t (*rw)(struct intel_vgpu *vgpu, char *buf, @@ -436,6 +438,92
>> @@ static void kvmgt_protect_table_del(struct kvmgt_guest_info *info,
>>  	}
>>  }
>>
>> +static size_t intel_vgpu_reg_rw_opregion(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;
>> +	void *base = vgpu->vdev.region[i].data;
>> +	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
>> +
>> +	if (pos >= vgpu->vdev.region[i].size || iswrite) {
>> +		gvt_vgpu_err("invalid op or offset for Intel vgpu OpRegion\n");
>> +		return -EINVAL;
>> +	}
>> +	count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
>> +	memcpy(buf, base + pos, count);
>> +
>> +	return count;
>> +}
>> +
>> +static void intel_vgpu_reg_release_opregion(struct intel_vgpu *vgpu,
>> +		struct vfio_region *region)
>> +{
>> +	memunmap(region->data);
>> +}
>> +
>> +static const struct intel_vgpu_regops intel_vgpu_regops_opregion = {
>> +	.rw = intel_vgpu_reg_rw_opregion,
>> +	.release = intel_vgpu_reg_release_opregion, };
>> +
>> +static int intel_vgpu_register_reg(struct intel_vgpu *vgpu,
>> +		unsigned int type, unsigned int subtype,
>> +		const struct intel_vgpu_regops *ops,
>> +		size_t size, u32 flags, void *data) {
>> +	struct vfio_region *region;
>> +
>> +	region = krealloc(vgpu->vdev.region,
>> +			(vgpu->vdev.num_regions + 1) * sizeof(*region),
>> +			GFP_KERNEL);
>> +	if (!region)
>> +		return -ENOMEM;
>> +
>> +	vgpu->vdev.region = region;
>> +	vgpu->vdev.region[vgpu->vdev.num_regions].type = type;
>> +	vgpu->vdev.region[vgpu->vdev.num_regions].subtype = subtype;
>> +	vgpu->vdev.region[vgpu->vdev.num_regions].ops = ops;
>> +	vgpu->vdev.region[vgpu->vdev.num_regions].size = size;
>> +	vgpu->vdev.region[vgpu->vdev.num_regions].flags = flags;
>> +	vgpu->vdev.region[vgpu->vdev.num_regions].data = data;
>> +	vgpu->vdev.num_regions++;
>> +
>> +	return 0;
>> +}
>> +
>> +static int intel_vgpu_reg_init_opregion(struct intel_vgpu *vgpu) {
>> +	unsigned int addr;
>> +	void *base;
>> +	int ret;
>> +
>> +	addr = vgpu->gvt->opregion.opregion_pa;
>> +	if (!addr || !(~addr))
>> +		return -ENODEV;
>> +
>> +	base = memremap(addr, OPREGION_SIZE, MEMREMAP_WB);
>> +	if (!base)
>> +		return -ENOMEM;
>> +
>> +	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
>> +		memunmap(base);
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = intel_vgpu_register_reg(vgpu,
>> +			PCI_VENDOR_ID_INTEL |
>> VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
>> +			VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION,
>> +			&intel_vgpu_regops_opregion, OPREGION_SIZE,
>> +			VFIO_REGION_INFO_FLAG_READ, base);
>In current GVT code, we already have a function init_vgpu_opregion, which will
>copy the physical opregion into a per vgpu structure:
>vgpu_opregion(vgpu)->va, why don’t you reuse this function and set the
>vgpu_opregion(vgpu)->va to the intel_vgpu_register_reg function?
The reason is we do not the know the gpa of the OpRegion.
For KVMGT the gpa of the OpRegion is allocated by seabios.

>
>> +	if (ret) {
>> +		memunmap(base);
>> +		return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  static int intel_vgpu_create(struct kobject *kobj, struct mdev_device
>> *mdev)  {
>>  	struct intel_vgpu *vgpu = NULL;
>> @@ -467,6 +555,15 @@ static int intel_vgpu_create(struct kobject
>> *kobj, struct mdev_device *mdev)
>>  	vgpu->vdev.mdev = mdev;
>>  	mdev_set_drvdata(mdev, vgpu);
>>
>> +	ret = intel_vgpu_reg_init_opregion(vgpu);
>> +	if (ret) {
>> +		gvt_vgpu_err("create OpRegion failed\n");
>> +		goto out;
>> +	}
>> +
>> +	gvt_dbg_core("create OpRegion succeeded for mdev:%s\n",
>> +			dev_name(mdev_dev(mdev)));
>> +
>>  	gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n",
>>  		     dev_name(mdev_dev(mdev)));
>>  	ret = 0;
>> diff --git a/drivers/gpu/drm/i915/gvt/opregion.c
>> b/drivers/gpu/drm/i915/gvt/opregion.c
>> index 3117991..99591bc 100644
>> --- a/drivers/gpu/drm/i915/gvt/opregion.c
>> +++ b/drivers/gpu/drm/i915/gvt/opregion.c
>> @@ -283,14 +283,22 @@ int intel_vgpu_emulate_opregion_request(struct
>> intel_vgpu *vgpu, u32 swsci)
>>  {
>>  	u32 *scic, *parm;
>>  	u32 func, subfunc;
>> +	u32 scic_val, parm_val;
>> +	u32 gpa;
>>
>> -	scic = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_SCIC;
>> -	parm = vgpu_opregion(vgpu)->va + INTEL_GVT_OPREGION_PARM;
>> +	memcpy(&gpa, vgpu_cfg_space(vgpu) + INTEL_GVT_PCI_OPREGION,
>> +			sizeof(gpa));
>> +	intel_gvt_hypervisor_read_gpa(vgpu, gpa +
>> INTEL_GVT_OPREGION_SCIC,
>> +			&scic_val, sizeof(scic_val));
>> +	intel_gvt_hypervisor_read_gpa(vgpu, gpa +
>> INTEL_GVT_OPREGION_PARM,
>> +			&parm_val, sizeof(parm_val));
>As I said, you can call init_vgpu_opregion(), then vgpu_opregion(vgpu)->va will be
>valid and you don’t need to change here.
>
>>
>>  	if (!(swsci & SWSCI_SCI_SELECT)) {
>>  		gvt_vgpu_err("requesting SMI service\n");
>>  		return 0;
>>  	}
>> +	scic = &scic_val;
>> +	parm = &parm_val;
>>  	/* ignore non 0->1 trasitions */
>>  	if ((vgpu_cfg_space(vgpu)[INTEL_GVT_PCI_SWSCI]
>>  				& SWSCI_SCI_TRIGGER) ||
>> --
>> 1.9.1
>>
>> _______________________________________________
>> intel-gvt-dev mailing list
>> intel-gvt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

[toc] | [prev] | [next] | [standalone]

#1646489 — RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g

> -----Original Message-----
> From: Chen, Xiaoguang
> Sent: Thursday, May 18, 2017 7:20 PM
> To: He, Min <min.he@intel.com>; alex.williamson@redhat.com;
> kraxel@redhat.com; intel-gfx@lists.freedesktop.org; linux-
> kernel@vger.kernel.org; zhenyuw@linux.intel.com; Lv, Zhiyuan
> <zhiyuan.lv@intel.com>; intel-gvt-dev@lists.freedesktop.org; Wang, Zhi A
> <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
> Cc: Niu, Bing <bing.niu@intel.com>
> Subject: RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
> 
> Hi min,
> 
> >-----Original Message-----
> >From: He, Min
> >Sent: Thursday, May 18, 2017 11:44 PM
> >To: Chen, Xiaoguang <xiaoguang.chen@intel.com>;
> >alex.williamson@redhat.com; kraxel@redhat.com; intel-
> >gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> >zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; intel-gvt-
> >dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, Kevin
> ><kevin.tian@intel.com>
> >Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
> ><xiaoguang.chen@intel.com>
> >Subject: RE: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
> >
> >Xiaoguang,
> >I have some comments inline. Thanks.
> >
> >> -----Original Message-----
> >> From: intel-gvt-dev
> >> [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> >> Xiaoguang Chen
> >> Sent: Thursday, May 18, 2017 2:50 AM
> >> To: alex.williamson@redhat.com; kraxel@redhat.com; intel-
> >> gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org;
> >> zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>;
> >> intel-gvt- dev@lists.freedesktop.org; Wang, Zhi A
> >> <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
> >> Cc: Niu, Bing <bing.niu@intel.com>; Chen, Xiaoguang
> >> <xiaoguang.chen@intel.com>
> >> Subject: [PATCH v2 2/5] drm/i915/gvt: OpRegion support for GVT-g
> >>
> >> OpRegion is needed to support display related operation for intel
> >> vgpu.
> >>
> >> A vfio device region is added to intel vgpu to deliver the host
> >> OpRegion information to user space so user space can construct the
> >> OpRegion for vgpu.
> >>
> >> Signed-off-by: Bing Niu <bing.niu@intel.com>
> >> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/gvt/kvmgt.c    | 97
> >> +++++++++++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/i915/gvt/opregion.c | 12 ++++-
> >>  2 files changed, 107 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> >> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> >> index 3c6a02b..389f072 100644
> >> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> >> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> >> @@ -53,6 +53,8 @@
> >>  #define VFIO_PCI_INDEX_TO_OFFSET(index) ((u64)(index) <<
> >> VFIO_PCI_OFFSET_SHIFT)
> >>  #define VFIO_PCI_OFFSET_MASK    (((u64)(1) << VFIO_PCI_OFFSET_SHIFT) - 1)
> >>
> >> +#define OPREGION_SIGNATURE "IntelGraphicsMem"
> >> +
> >>  struct vfio_region;
> >>  struct intel_vgpu_regops {
> >>  	size_t (*rw)(struct intel_vgpu *vgpu, char *buf, @@ -436,6 +438,92
> >> @@ static void kvmgt_protect_table_del(struct kvmgt_guest_info *info,
> >>  	}
> >>  }
> >>
> >> +static size_t intel_vgpu_reg_rw_opregion(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;
> >> +	void *base = vgpu->vdev.region[i].data;
> >> +	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> >> +
> >> +	if (pos >= vgpu->vdev.region[i].size || iswrite) {
> >> +		gvt_vgpu_err("invalid op or offset for Intel vgpu OpRegion\n");
> >> +		return -EINVAL;
> >> +	}
> >> +	count = min(count, (size_t)(vgpu->vdev.region[i].size - pos));
> >> +	memcpy(buf, base + pos, count);
> >> +
> >> +	return count;
> >> +}
> >> +
> >> +static void intel_vgpu_reg_release_opregion(struct intel_vgpu *vgpu,
> >> +		struct vfio_region *region)
> >> +{
> >> +	memunmap(region->data);
> >> +}
> >> +
> >> +static const struct intel_vgpu_regops intel_vgpu_regops_opregion = {
> >> +	.rw = intel_vgpu_reg_rw_opregion,
> >> +	.release = intel_vgpu_reg_release_opregion, };
> >> +
> >> +static int intel_vgpu_register_reg(struct intel_vgpu *vgpu,
> >> +		unsigned int type, unsigned int subtype,
> >> +		const struct intel_vgpu_regops *ops,
> >> +		size_t size, u32 flags, void *data) {
> >> +	struct vfio_region *region;
> >> +
> >> +	region = krealloc(vgpu->vdev.region,
> >> +			(vgpu->vdev.num_regions + 1) * sizeof(*region),
> >> +			GFP_KERNEL);
> >> +	if (!region)
> >> +		return -ENOMEM;
> >> +
> >> +	vgpu->vdev.region = region;
> >> +	vgpu->vdev.region[vgpu->vdev.num_regions].type = type;
> >> +	vgpu->vdev.region[vgpu->vdev.num_regions].subtype = subtype;
> >> +	vgpu->vdev.region[vgpu->vdev.num_regions].ops = ops;
> >> +	vgpu->vdev.region[vgpu->vdev.num_regions].size = size;
> >> +	vgpu->vdev.region[vgpu->vdev.num_regions].flags = flags;
> >> +	vgpu->vdev.region[vgpu->vdev.num_regions].data = data;
> >> +	vgpu->vdev.num_regions++;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int intel_vgpu_reg_init_opregion(struct intel_vgpu *vgpu) {
> >> +	unsigned int addr;
> >> +	void *base;
> >> +	int ret;
> >> +
> >> +	addr = vgpu->gvt->opregion.opregion_pa;
> >> +	if (!addr || !(~addr))
> >> +		return -ENODEV;
> >> +
> >> +	base = memremap(addr, OPREGION_SIZE, MEMREMAP_WB);
> >> +	if (!base)
> >> +		return -ENOMEM;
> >> +
> >> +	if (memcmp(base, OPREGION_SIGNATURE, 16)) {
> >> +		memunmap(base);
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	ret = intel_vgpu_register_reg(vgpu,
> >> +			PCI_VENDOR_ID_INTEL |
> >> VFIO_REGION_TYPE_PCI_VENDOR_TYPE,
> >> +			VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION,
> >> +			&intel_vgpu_regops_opregion, OPREGION_SIZE,
> >> +			VFIO_REGION_INFO_FLAG_READ, base);
> >In current GVT code, we already have a function init_vgpu_opregion, which will
> >copy the physical opregion into a per vgpu structure:
> >vgpu_opregion(vgpu)->va, why don’t you reuse this function and set the
> >vgpu_opregion(vgpu)->va to the intel_vgpu_register_reg function?
> The reason is we do not the know the gpa of the OpRegion.
> For KVMGT the gpa of the OpRegion is allocated by seabios.

Maybe you can consolidate them into one function to remove duplicate code.

[toc] | [prev] | [next] | [standalone]

#1644032 — [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf

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.

Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
---
 drivers/gpu/drm/i915/gvt/gvt.c   |  2 +
 drivers/gpu/drm/i915/gvt/gvt.h   |  3 ++
 drivers/gpu/drm/i915/gvt/kvmgt.c | 89 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h      |  2 +
 include/uapi/linux/vfio.h        | 12 ++++++
 5 files changed, 108 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 2032917..48e04e6 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -54,6 +54,8 @@
 	.vgpu_reset = intel_gvt_reset_vgpu,
 	.vgpu_activate = intel_gvt_activate_vgpu,
 	.vgpu_deactivate = intel_gvt_deactivate_vgpu,
+	.vgpu_query_dmabuf = intel_vgpu_query_dmabuf,
+	.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 a553120..b7fdfd5 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -185,6 +185,7 @@ struct intel_vgpu {
 		struct kvm *kvm;
 		struct work_struct release_work;
 		atomic_t released;
+		struct vfio_device *vfio_device;
 	} vdev;
 #endif
 	struct intel_vgpu_plane_info *plane_info;
@@ -469,6 +470,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_dmabuf)(struct intel_vgpu *, void *);
+	int (*vgpu_create_dmabuf)(struct intel_vgpu *, void *);
 };
 
 
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 389f072..9a663df 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,66 @@ 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 (vgpu->vdev.vfio_device != NULL)
+		vfio_device_put(vgpu->vdev.vfio_device);
+	else
+		gvt_vgpu_err("intel vgpu dmabuf mgr fd is in a wrong state\n");
+
+	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;
+	struct intel_vgpu_dmabuf dmabuf;
+	int ret;
+	struct fd f;
+
+	minsz = offsetofend(struct intel_vgpu_dmabuf, tiled);
+	if (copy_from_user(&dmabuf, (void __user *)arg, minsz))
+		return -EFAULT;
+
+	f = fdget(dmabuf.fd);
+
+	if (ioctl == INTEL_VGPU_QUERY_DMABUF)
+		ret = intel_gvt_ops->vgpu_query_dmabuf(vgpu, &dmabuf);
+	else if (ioctl == INTEL_VGPU_GENERATE_DMABUF)
+		ret = intel_gvt_ops->vgpu_create_dmabuf(vgpu, &dmabuf);
+	else {
+		fdput(f);
+		gvt_vgpu_err("unsupported dmabuf operation\n");
+		return -EINVAL;
+	}
+
+	if (ret != 0) {
+		fdput(f);
+		gvt_vgpu_err("gvt-g get dmabuf failed:%d\n", ret);
+		return -EINVAL;
+	}
+	fdput(f);
+
+	return copy_to_user((void __user *)arg, &dmabuf, minsz) ? -EFAULT : 0;
+}
+
+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 +1320,34 @@ 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 != INTEL_VGPU_DMABUF_MGR_FD) {
+			gvt_vgpu_err("not supported fd type:%d\n", type);
+			return -EINVAL;
+		}
+
+		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;
+		}
+
+		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;
+
+		return fd;
 	}
 
 	return 0;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index cde4f8e..9d28433 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1466,6 +1466,8 @@ struct intel_vgpu_dmabuf {
 	__u32 tiled;
 };
 
+#define INTEL_VGPU_DMABUF_MGR_FD	0
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index ae46105..c81500b 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -502,6 +502,18 @@ 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)
+
 /* -------- API for Type1 VFIO IOMMU -------- */
 
 /**
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

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

On Thu, 18 May 2017 17:50:05 +0800
Xiaoguang Chen <xiaoguang.chen@intel.com> 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.
> 
> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/gvt.c   |  2 +
>  drivers/gpu/drm/i915/gvt/gvt.h   |  3 ++
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 89 ++++++++++++++++++++++++++++++++++++++++
>  include/uapi/drm/i915_drm.h      |  2 +
>  include/uapi/linux/vfio.h        | 12 ++++++
>  5 files changed, 108 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> index 2032917..48e04e6 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.c
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -54,6 +54,8 @@
>  	.vgpu_reset = intel_gvt_reset_vgpu,
>  	.vgpu_activate = intel_gvt_activate_vgpu,
>  	.vgpu_deactivate = intel_gvt_deactivate_vgpu,
> +	.vgpu_query_dmabuf = intel_vgpu_query_dmabuf,
> +	.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 a553120..b7fdfd5 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -185,6 +185,7 @@ struct intel_vgpu {
>  		struct kvm *kvm;
>  		struct work_struct release_work;
>  		atomic_t released;
> +		struct vfio_device *vfio_device;
>  	} vdev;
>  #endif
>  	struct intel_vgpu_plane_info *plane_info;
> @@ -469,6 +470,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_dmabuf)(struct intel_vgpu *, void *);
> +	int (*vgpu_create_dmabuf)(struct intel_vgpu *, void *);
>  };
>  
>  
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 389f072..9a663df 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,66 @@ 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 (vgpu->vdev.vfio_device != NULL)
> +		vfio_device_put(vgpu->vdev.vfio_device);
> +	else
> +		gvt_vgpu_err("intel vgpu dmabuf mgr fd is in a wrong state\n");

You could do:

if (WARN_ON(!vgpu->vdev.vfio_device))
	return -EINVAL;

> +
> +	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;
> +	struct intel_vgpu_dmabuf dmabuf;
> +	int ret;
> +	struct fd f;
> +
> +	minsz = offsetofend(struct intel_vgpu_dmabuf, tiled);
> +	if (copy_from_user(&dmabuf, (void __user *)arg, minsz))
> +		return -EFAULT;
> +
> +	f = fdget(dmabuf.fd);
> +
> +	if (ioctl == INTEL_VGPU_QUERY_DMABUF)
> +		ret = intel_gvt_ops->vgpu_query_dmabuf(vgpu, &dmabuf);
> +	else if (ioctl == INTEL_VGPU_GENERATE_DMABUF)
> +		ret = intel_gvt_ops->vgpu_create_dmabuf(vgpu, &dmabuf);

Why are these still Intel specific?

> +	else {
> +		fdput(f);
> +		gvt_vgpu_err("unsupported dmabuf operation\n");
> +		return -EINVAL;
> +	}
> +
> +	if (ret != 0) {
> +		fdput(f);
> +		gvt_vgpu_err("gvt-g get dmabuf failed:%d\n", ret);
> +		return -EINVAL;
> +	}
> +	fdput(f);
> +
> +	return copy_to_user((void __user *)arg, &dmabuf, minsz) ? -EFAULT : 0;
> +}
> +
> +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 +1320,34 @@ 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 != INTEL_VGPU_DMABUF_MGR_FD) {

Yet more unnecessary Intel specific code.  Userspace doesn't want to
handle a dmabuf differently between Intel, AMD, and NVIDIA, so if we
have no reason to need vendor specific ioctl and parameters, then don't.

> +			gvt_vgpu_err("not supported fd type:%d\n", type);

Just return error, users can abuse any logging they can get to.

> +			return -EINVAL;
> +		}
> +
> +		fd = anon_inode_getfd("intel-vgpu-dmabuf-mgr-fd",
> +			&intel_vgpu_dmabuf_mgr_fd_ops,
> +			vgpu, O_RDWR | O_CLOEXEC);

And this ordering is still wrong, do this last so you don't need to
worry about getting the vfio_device reference below failing and calling
release unnecessarily (though it seems this fd is leaked in that case
anyway).

> +		if (fd < 0) {
> +			gvt_vgpu_err("create dmabuf mgr fd failed\n");
> +			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;
> +
> +		return fd;
>  	}
>  
>  	return 0;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index cde4f8e..9d28433 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1466,6 +1466,8 @@ struct intel_vgpu_dmabuf {
>  	__u32 tiled;
>  };
>  
> +#define INTEL_VGPU_DMABUF_MGR_FD	0
> +
>  #if defined(__cplusplus)
>  }
>  #endif
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index ae46105..c81500b 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -502,6 +502,18 @@ 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)
> +

The known input types would need to be defined here too.  Definition of
the ioctls available on that type should also be documented here.

>  /* -------- API for Type1 VFIO IOMMU -------- */
>  
>  /**

[toc] | [prev] | [next] | [standalone]

#1646484 — RE: [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get the dma-buf

Hi Alex,

>-----Original Message-----
>From: Alex Williamson [mailto:alex.williamson@redhat.com]
>Sent: Saturday, May 20, 2017 12:34 AM
>To: Chen, Xiaoguang <xiaoguang.chen@intel.com>
>Cc: kraxel@redhat.com; intel-gfx@lists.freedesktop.org; linux-
>kernel@vger.kernel.org; zhenyuw@linux.intel.com; Lv, Zhiyuan
><zhiyuan.lv@intel.com>; intel-gvt-dev@lists.freedesktop.org; Wang, Zhi A
><zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
>Subject: Re: [PATCH v2 5/5] drm/i915/gvt: Adding interface so user space can get
>the dma-buf
>
>On Thu, 18 May 2017 17:50:05 +0800
>Xiaoguang Chen <xiaoguang.chen@intel.com> 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.
>>
>> Signed-off-by: Xiaoguang Chen <xiaoguang.chen@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gvt/gvt.c   |  2 +
>>  drivers/gpu/drm/i915/gvt/gvt.h   |  3 ++
>>  drivers/gpu/drm/i915/gvt/kvmgt.c | 89
>++++++++++++++++++++++++++++++++++++++++
>>  include/uapi/drm/i915_drm.h      |  2 +
>>  include/uapi/linux/vfio.h        | 12 ++++++
>>  5 files changed, 108 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c
>> b/drivers/gpu/drm/i915/gvt/gvt.c index 2032917..48e04e6 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.c
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
>> @@ -54,6 +54,8 @@
>>  	.vgpu_reset = intel_gvt_reset_vgpu,
>>  	.vgpu_activate = intel_gvt_activate_vgpu,
>>  	.vgpu_deactivate = intel_gvt_deactivate_vgpu,
>> +	.vgpu_query_dmabuf = intel_vgpu_query_dmabuf,
>> +	.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 a553120..b7fdfd5 100644
>> --- a/drivers/gpu/drm/i915/gvt/gvt.h
>> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>> @@ -185,6 +185,7 @@ struct intel_vgpu {
>>  		struct kvm *kvm;
>>  		struct work_struct release_work;
>>  		atomic_t released;
>> +		struct vfio_device *vfio_device;
>>  	} vdev;
>>  #endif
>>  	struct intel_vgpu_plane_info *plane_info; @@ -469,6 +470,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_dmabuf)(struct intel_vgpu *, void *);
>> +	int (*vgpu_create_dmabuf)(struct intel_vgpu *, void *);
>>  };
>>
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 389f072..9a663df 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,66 @@ 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 (vgpu->vdev.vfio_device != NULL)
>> +		vfio_device_put(vgpu->vdev.vfio_device);
>> +	else
>> +		gvt_vgpu_err("intel vgpu dmabuf mgr fd is in a wrong state\n");
>
>You could do:
>
>if (WARN_ON(!vgpu->vdev.vfio_device))
>	return -EINVAL;
OK. 

>
>> +
>> +	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;
>> +	struct intel_vgpu_dmabuf dmabuf;
>> +	int ret;
>> +	struct fd f;
>> +
>> +	minsz = offsetofend(struct intel_vgpu_dmabuf, tiled);
>> +	if (copy_from_user(&dmabuf, (void __user *)arg, minsz))
>> +		return -EFAULT;
>> +
>> +	f = fdget(dmabuf.fd);
>> +
>> +	if (ioctl == INTEL_VGPU_QUERY_DMABUF)
>> +		ret = intel_gvt_ops->vgpu_query_dmabuf(vgpu, &dmabuf);
>> +	else if (ioctl == INTEL_VGPU_GENERATE_DMABUF)
>> +		ret = intel_gvt_ops->vgpu_create_dmabuf(vgpu, &dmabuf);
>
>Why are these still Intel specific?
Ah, the v2 patch series were sent before our discussion in other thread. Will change it in v3.

>
>> +	else {
>> +		fdput(f);
>> +		gvt_vgpu_err("unsupported dmabuf operation\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (ret != 0) {
>> +		fdput(f);
>> +		gvt_vgpu_err("gvt-g get dmabuf failed:%d\n", ret);
>> +		return -EINVAL;
>> +	}
>> +	fdput(f);
>> +
>> +	return copy_to_user((void __user *)arg, &dmabuf, minsz) ? -EFAULT :
>> +0; }
>> +
>> +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 +1320,34 @@ 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 != INTEL_VGPU_DMABUF_MGR_FD) {
>
>Yet more unnecessary Intel specific code.  Userspace doesn't want to handle a
>dmabuf differently between Intel, AMD, and NVIDIA, so if we have no reason to
>need vendor specific ioctl and parameters, then don't.
Yes. I will change in v3.

>
>> +			gvt_vgpu_err("not supported fd type:%d\n", type);
>
>Just return error, users can abuse any logging they can get to.
OK.

>
>> +			return -EINVAL;
>> +		}
>> +
>> +		fd = anon_inode_getfd("intel-vgpu-dmabuf-mgr-fd",
>> +			&intel_vgpu_dmabuf_mgr_fd_ops,
>> +			vgpu, O_RDWR | O_CLOEXEC);
>
>And this ordering is still wrong, do this last so you don't need to worry about
>getting the vfio_device reference below failing and calling release unnecessarily
>(though it seems this fd is leaked in that case anyway).
Will change in v3.

>
>> +		if (fd < 0) {
>> +			gvt_vgpu_err("create dmabuf mgr fd failed\n");
>> +			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;
>> +
>> +		return fd;
>>  	}
>>
>>  	return 0;
>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> index cde4f8e..9d28433 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -1466,6 +1466,8 @@ struct intel_vgpu_dmabuf {
>>  	__u32 tiled;
>>  };
>>
>> +#define INTEL_VGPU_DMABUF_MGR_FD	0
>> +
>>  #if defined(__cplusplus)
>>  }
>>  #endif
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index ae46105..c81500b 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -502,6 +502,18 @@ 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)
>> +
>
>The known input types would need to be defined here too.  Definition of the
>ioctls available on that type should also be documented here.
Yes. Will do it in v3.
Thanks. 

>
>>  /* -------- API for Type1 VFIO IOMMU -------- */
>>
>>  /**

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web