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

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

[Tina Zhang]

v13->v14:
1) add PROBE, DMABUF and REGION flags. (Alex)
2) return -ENXIO when gem proxy object is banned by ioctl.
   (Chris) (Daniel)
3) add some details about the float pixel format. (Daniel)
4) add F suffix to the defined name. (Daniel)
5) refine pixel format table. (Zhenyu)

v12->v13:
1) add comments to GEM proxy. (Chris)
2) don't ban GEM proxy in i915_gem_sw_finish_ioctl. (Chris)
3) check GEM proxy bar after finishing i915_gem_object_wait. (Chris)
4) remove GEM proxy bar in i915_gem_madvise_ioctl.

v11->v12:
1) add drm_format_mod back. (Gerd and Zhenyu)
2) add region_index. (Gerd)
3) refine the lifecycle of dmabuf.
4) send to dri-devel@lists.freedesktop.org. (Ville) 

v10->v11:
1) rename plane_type to drm_plane_type. (Gerd)
2) move fields of vfio_device_query_gfx_plane to
   vfio_device_gfx_plane_info. (Gerd)
3) remove drm_format_mod, start fields. (Daniel)
4) remove plane_id.

v9->v10:
1) remove dma-buf management
2) refine the ABI API VFIO_DEVICE_QUERY_GFX_PLANE
3) track the dma-buf create and release in kernel mode

v8->v9:
1) refine the dma-buf ioctl definition
2) add a lock to protect the dmabuf list
3) move drm format change to a separate patch
4) codes cleanup

v7->v8:
1) refine framebuffer decoder code
2) fix a bug in decoding primary plane

v6->v7:
1) release dma-buf related allocations in dma-buf's associated release
   function.
2) refine ioctl interface for querying plane info or create dma-buf
3) refine framebuffer decoder code
4) the patch series is based on 4.12.0-rc1

v5->v6:
1) align the dma-buf life cycle with the vfio device.
2) add the dma-buf related operations in a separate patch.
3) i915 releated changes.

v4->v5:
1) fix bug while checking whether the gem obj is gvt's dma-buf when user
   change caching mode or domains. Add a helper function to do it.
2) add definition for the query plane and create dma-buf.

v3->v4:
1) fix bug while checking whether the gem obj is gvt's dma-buf when set
   caching mode or doamins.

v2->v3:
1) add a field gvt_plane_info in the drm_i915_gem_obj structure to save
   the decoded plane information to avoid look up while need the plane info.
2) declare a new flag I915_GEM_OBJECT_IS_GVT_DMABUF in drm_i915_gem_object
   to represent the gem obj for gvt's dma-buf. The tiling mode, caching mode
   and domains can not be changed for this kind of gem object.
3) change dma-buf related information to be more generic. So other vendor
   can use the same interface.

v1->v2:
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
rendering or other operations.

This patch set can be tried with the following example:
	https://github.com/01org/Igvtg-qemu  branch: topic/dmabuf

Tina Zhang (7):
  drm/i915/gvt: Add framebuffer decoder support
  drm: Introduce RGB 64-bit 16:16:16:16 float format
  drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format
  drm/i915/gvt: Add opregion support
  vfio: ABI for mdev display dma-buf operation
  drm/i915: Introduce GEM proxy
  drm/i915/gvt: Dmabuf support for GVT-g

 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      | 387 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/dmabuf.h      |  58 ++++
 drivers/gpu/drm/i915/gvt/fb_decoder.c  | 521 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/fb_decoder.h  | 169 +++++++++++
 drivers/gpu/drm/i915/gvt/gvt.c         |   1 +
 drivers/gpu/drm/i915/gvt/gvt.h         |  10 +
 drivers/gpu/drm/i915/gvt/hypercall.h   |   3 +
 drivers/gpu/drm/i915/gvt/kvmgt.c       | 151 +++++++++-
 drivers/gpu/drm/i915/gvt/mpt.h         |  45 +++
 drivers/gpu/drm/i915/gvt/opregion.c    |  26 +-
 drivers/gpu/drm/i915/gvt/vgpu.c        |   6 +-
 drivers/gpu/drm/i915/i915_gem.c        |  24 +-
 drivers/gpu/drm/i915/i915_gem_object.h |   9 +
 drivers/gpu/drm/i915/i915_gem_tiling.c |   8 +
 include/uapi/drm/drm_fourcc.h          |   4 +
 include/uapi/linux/vfio.h              |  43 +++
 19 files changed, 1459 insertions(+), 13 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

-- 
2.7.4

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

[PATCH v14 1/7] drm/i915/gvt: Add framebuffer decoder support

[Tina Zhang]

This patch is to introduce the framebuffer decoder which can decode guest
OS's framebuffer information, including primary, cursor and sprite plane.

v14:
- refine pixel format table. (Zhenyu)

v9:
- move drm format change to a separate patch. (Xiaoguang)

v8:
- fix a bug in decoding primary plane. (Tina)

v7:
- refine framebuffer decoder code. (Zhenyu)

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>

diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
index f5486cb9..019d596 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -1,7 +1,8 @@
 GVT_DIR := gvt
 GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
 	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
-	execlist.o scheduler.o sched_policy.o render.o cmd_parser.o
+	execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \
+	fb_decoder.o
 
 ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
 i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
index 3c31843..fb7fdba 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -67,7 +67,7 @@ static int edp_pipe_is_enabled(struct intel_vgpu *vgpu)
 	return 1;
 }
 
-static int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe)
+int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe)
 {
 	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
 
diff --git a/drivers/gpu/drm/i915/gvt/display.h b/drivers/gpu/drm/i915/gvt/display.h
index d73de22..b46b868 100644
--- a/drivers/gpu/drm/i915/gvt/display.h
+++ b/drivers/gpu/drm/i915/gvt/display.h
@@ -179,4 +179,6 @@ int intel_vgpu_init_display(struct intel_vgpu *vgpu, u64 resolution);
 void intel_vgpu_reset_display(struct intel_vgpu *vgpu);
 void intel_vgpu_clean_display(struct intel_vgpu *vgpu);
 
+int pipe_is_enabled(struct intel_vgpu *vgpu, int pipe);
+
 #endif
diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c b/drivers/gpu/drm/i915/gvt/fb_decoder.c
new file mode 100644
index 0000000..d380ab8
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
@@ -0,0 +1,507 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *    Kevin Tian <kevin.tian@intel.com>
+ *
+ * Contributors:
+ *    Bing Niu <bing.niu@intel.com>
+ *    Xu Han <xu.han@intel.com>
+ *    Ping Gao <ping.a.gao@intel.com>
+ *    Xiaoguang Chen <xiaoguang.chen@intel.com>
+ *    Yang Liu <yang2.liu@intel.com>
+ *    Tina Zhang <tina.zhang@intel.com>
+ *
+ */
+
+#include <uapi/drm/drm_fourcc.h>
+#include "i915_drv.h"
+#include "gvt.h"
+
+#define PRIMARY_FORMAT_NUM	16
+struct pixel_format {
+	int	drm_format;	/* Pixel format in DRM definition */
+	int	bpp;		/* Bits per pixel, 0 indicates invalid */
+	char	*desc;		/* The description */
+};
+
+static struct pixel_format bdw_pixel_formats[] = {
+	{DRM_FORMAT_C8, 8, "8-bit Indexed"},
+	{DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"},
+	{DRM_FORMAT_XRGB8888, 32, "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"},
+	{DRM_FORMAT_XBGR2101010, 32, "32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"},
+
+	{DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
+	{DRM_FORMAT_XBGR8888, 32, "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
+
+	/* non-supported format has bpp default to 0 */
+	{0, 0, NULL},
+};
+
+static struct pixel_format skl_pixel_formats[] = {
+	{DRM_FORMAT_YUYV, 16, "16-bit packed YUYV (8:8:8:8 MSB-V:Y2:U:Y1)"},
+	{DRM_FORMAT_UYVY, 16, "16-bit packed UYVY (8:8:8:8 MSB-Y2:V:Y1:U)"},
+	{DRM_FORMAT_YVYU, 16, "16-bit packed YVYU (8:8:8:8 MSB-U:Y2:V:Y1)"},
+	{DRM_FORMAT_VYUY, 16, "16-bit packed VYUY (8:8:8:8 MSB-Y2:U:Y1:V)"},
+
+	{DRM_FORMAT_C8, 8, "8-bit Indexed"},
+	{DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"},
+	{DRM_FORMAT_ABGR8888, 32, "32-bit RGBA (8:8:8:8 MSB-A:B:G:R)"},
+	{DRM_FORMAT_XBGR8888, 32, "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
+
+	{DRM_FORMAT_ARGB8888, 32, "32-bit BGRA (8:8:8:8 MSB-A:R:G:B)"},
+	{DRM_FORMAT_XRGB8888, 32, "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"},
+	{DRM_FORMAT_XBGR2101010, 32, "32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"},
+	{DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
+
+	/* non-supported format has bpp default to 0 */
+	{0, 0, NULL},
+};
+
+static int bdw_format_to_drm(int format)
+{
+	int bdw_pixel_formats_index = 6;
+
+	switch (format) {
+	case DISPPLANE_8BPP:
+		bdw_pixel_formats_index = 0;
+		break;
+	case DISPPLANE_BGRX565:
+		bdw_pixel_formats_index = 1;
+		break;
+	case DISPPLANE_BGRX888:
+		bdw_pixel_formats_index = 2;
+		break;
+	case DISPPLANE_RGBX101010:
+		bdw_pixel_formats_index = 3;
+		break;
+	case DISPPLANE_BGRX101010:
+		bdw_pixel_formats_index = 4;
+		break;
+	case DISPPLANE_RGBX888:
+		bdw_pixel_formats_index = 5;
+		break;
+
+	default:
+		break;
+	}
+
+	return bdw_pixel_formats_index;
+}
+
+static int skl_format_to_drm(int format, bool rgb_order, bool alpha,
+	int yuv_order)
+{
+	int skl_pixel_formats_index = 12;
+
+	switch (format) {
+	case PLANE_CTL_FORMAT_INDEXED:
+		skl_pixel_formats_index = 4;
+		break;
+	case PLANE_CTL_FORMAT_RGB_565:
+		skl_pixel_formats_index = 5;
+		break;
+	case PLANE_CTL_FORMAT_XRGB_8888:
+		if (rgb_order)
+			skl_pixel_formats_index = alpha ? 6 : 7;
+		else
+			skl_pixel_formats_index = alpha ? 8 : 9;
+		break;
+	case PLANE_CTL_FORMAT_XRGB_2101010:
+		skl_pixel_formats_index = rgb_order ? 10 : 11;
+		break;
+	case PLANE_CTL_FORMAT_YUV422:
+		skl_pixel_formats_index = yuv_order >> 16;
+		if (skl_pixel_formats_index > 3)
+			return -EINVAL;
+		break;
+
+	default:
+		break;
+	}
+
+	return skl_pixel_formats_index;
+}
+
+static u32 intel_vgpu_get_stride(struct intel_vgpu *vgpu, int pipe,
+	u32 tiled, int stride_mask, int bpp)
+{
+	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+
+	u32 stride_reg = vgpu_vreg(vgpu, DSPSTRIDE(pipe)) & stride_mask;
+	u32 stride = stride_reg;
+
+	if (IS_SKYLAKE(dev_priv)) {
+		switch (tiled) {
+		case PLANE_CTL_TILED_LINEAR:
+			stride = stride_reg * 64;
+			break;
+		case PLANE_CTL_TILED_X:
+			stride = stride_reg * 512;
+			break;
+		case PLANE_CTL_TILED_Y:
+			stride = stride_reg * 128;
+			break;
+		case PLANE_CTL_TILED_YF:
+			if (bpp == 8)
+				stride = stride_reg * 64;
+			else if (bpp == 16 || bpp == 32 || bpp == 64)
+				stride = stride_reg * 128;
+			else
+				gvt_dbg_core("skl: unsupported bpp:%d\n", bpp);
+			break;
+		default:
+			gvt_dbg_core("skl: unsupported tile format:%x\n",
+				tiled);
+		}
+	}
+
+	return stride;
+}
+
+static int get_active_pipe(struct intel_vgpu *vgpu)
+{
+	int i;
+
+	for (i = 0; i < I915_MAX_PIPES; i++)
+		if (pipe_is_enabled(vgpu, i))
+			break;
+
+	return i;
+}
+
+/**
+ * intel_vgpu_decode_primary_plane - Decode primary plane
+ * @vgpu: input vgpu
+ * @plane: primary plane to save decoded info
+ * This function is called for decoding plane
+ *
+ * Returns:
+ * 0 on success, non-zero if failed.
+ */
+int intel_vgpu_decode_primary_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_primary_plane_format *plane)
+{
+	u32 val, fmt;
+	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+	int pipe;
+
+	pipe = get_active_pipe(vgpu);
+	if (pipe >= I915_MAX_PIPES)
+		return -ENODEV;
+
+	val = vgpu_vreg(vgpu, DSPCNTR(pipe));
+	plane->enabled = !!(val & DISPLAY_PLANE_ENABLE);
+	if (!plane->enabled)
+		return -ENODEV;
+
+	if (IS_SKYLAKE(dev_priv)) {
+		plane->tiled = (val & PLANE_CTL_TILED_MASK) >>
+		_PLANE_CTL_TILED_SHIFT;
+		fmt = skl_format_to_drm(
+			val & PLANE_CTL_FORMAT_MASK,
+			val & PLANE_CTL_ORDER_RGBX,
+			val & PLANE_CTL_ALPHA_MASK,
+			val & PLANE_CTL_YUV422_ORDER_MASK);
+		plane->bpp = skl_pixel_formats[fmt].bpp;
+		plane->drm_format = skl_pixel_formats[fmt].drm_format;
+	} else {
+		plane->tiled = !!(val & DISPPLANE_TILED);
+		fmt = bdw_format_to_drm(val & DISPPLANE_PIXFORMAT_MASK);
+		plane->bpp = bdw_pixel_formats[fmt].bpp;
+		plane->drm_format = bdw_pixel_formats[fmt].drm_format;
+	}
+
+	if (!plane->bpp) {
+		gvt_vgpu_err("Non-supported pixel format (0x%x)\n", fmt);
+		return -EINVAL;
+	}
+
+	plane->hw_format = fmt;
+
+	plane->base = vgpu_vreg(vgpu, DSPSURF(pipe)) & GTT_PAGE_MASK;
+	if (!intel_gvt_ggtt_validate_range(vgpu, plane->base, 0)) {
+		gvt_vgpu_err("invalid gma address: %lx\n",
+			     (unsigned long)plane->base);
+		return  -EINVAL;
+	}
+
+	plane->base_gpa = intel_vgpu_gma_to_gpa(vgpu->gtt.ggtt_mm, plane->base);
+	if (plane->base_gpa == INTEL_GVT_INVALID_ADDR) {
+		gvt_vgpu_err("invalid gma address: %lx\n",
+				(unsigned long)plane->base);
+		return  -EINVAL;
+	}
+
+	plane->stride = intel_vgpu_get_stride(vgpu, pipe, (plane->tiled << 10),
+		(IS_SKYLAKE(dev_priv)) ? (_PRI_PLANE_STRIDE_MASK >> 6) :
+		_PRI_PLANE_STRIDE_MASK, plane->bpp);
+
+	plane->width = (vgpu_vreg(vgpu, PIPESRC(pipe)) & _PIPE_H_SRCSZ_MASK) >>
+		_PIPE_H_SRCSZ_SHIFT;
+	plane->width += 1;
+	plane->height = (vgpu_vreg(vgpu, PIPESRC(pipe)) &
+			_PIPE_V_SRCSZ_MASK) >> _PIPE_V_SRCSZ_SHIFT;
+	plane->height += 1;	/* raw height is one minus the real value */
+
+	val = vgpu_vreg(vgpu, DSPTILEOFF(pipe));
+	plane->x_offset = (val & _PRI_PLANE_X_OFF_MASK) >>
+		_PRI_PLANE_X_OFF_SHIFT;
+	plane->y_offset = (val & _PRI_PLANE_Y_OFF_MASK) >>
+		_PRI_PLANE_Y_OFF_SHIFT;
+
+	return 0;
+}
+
+#define CURSOR_FORMAT_NUM	(1 << 6)
+struct cursor_mode_format {
+	int	drm_format;	/* Pixel format in DRM definition */
+	u8	bpp;		/* Bits per pixel; 0 indicates invalid */
+	u32	width;		/* In pixel */
+	u32	height;		/* In lines */
+	char	*desc;		/* The description */
+};
+
+static struct cursor_mode_format cursor_pixel_formats[] = {
+	{DRM_FORMAT_ARGB8888, 32, 128, 128, "128x128 32bpp ARGB"},
+	{DRM_FORMAT_ARGB8888, 32, 256, 256, "256x256 32bpp ARGB"},
+	{DRM_FORMAT_ARGB8888, 32, 64, 64, "64x64 32bpp ARGB"},
+	{DRM_FORMAT_ARGB8888, 32, 64, 64, "64x64 32bpp ARGB"},
+
+	/* non-supported format has bpp default to 0 */
+	{0, 0, 0, 0, NULL},
+};
+
+static int cursor_mode_to_drm(int mode)
+{
+	int cursor_pixel_formats_index = 4;
+
+	switch (mode) {
+	case CURSOR_MODE_128_ARGB_AX:
+		cursor_pixel_formats_index = 0;
+		break;
+	case CURSOR_MODE_256_ARGB_AX:
+		cursor_pixel_formats_index = 1;
+		break;
+	case CURSOR_MODE_64_ARGB_AX:
+		cursor_pixel_formats_index = 2;
+		break;
+	case CURSOR_MODE_64_32B_AX:
+		cursor_pixel_formats_index = 3;
+		break;
+
+	default:
+		break;
+	}
+
+	return cursor_pixel_formats_index;
+}
+
+/**
+ * intel_vgpu_decode_cursor_plane - Decode sprite plane
+ * @vgpu: input vgpu
+ * @plane: cursor plane to save decoded info
+ * This function is called for decoding plane
+ *
+ * Returns:
+ * 0 on success, non-zero if failed.
+ */
+int intel_vgpu_decode_cursor_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_cursor_plane_format *plane)
+{
+	u32 val, mode, index;
+	u32 alpha_plane, alpha_force;
+	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
+	int pipe;
+
+	pipe = get_active_pipe(vgpu);
+	if (pipe >= I915_MAX_PIPES)
+		return -ENODEV;
+
+	val = vgpu_vreg(vgpu, CURCNTR(pipe));
+	mode = val & CURSOR_MODE;
+	plane->enabled = (mode != CURSOR_MODE_DISABLE);
+	if (!plane->enabled)
+		return -ENODEV;
+
+	index = cursor_mode_to_drm(mode);
+
+	if (!cursor_pixel_formats[index].bpp) {
+		gvt_vgpu_err("Non-supported cursor mode (0x%x)\n", mode);
+		return -EINVAL;
+	}
+	plane->mode = mode;
+	plane->bpp = cursor_pixel_formats[index].bpp;
+	plane->drm_format = cursor_pixel_formats[index].drm_format;
+	plane->width = cursor_pixel_formats[index].width;
+	plane->height = cursor_pixel_formats[index].height;
+
+	alpha_plane = (val & _CURSOR_ALPHA_PLANE_MASK) >>
+				_CURSOR_ALPHA_PLANE_SHIFT;
+	alpha_force = (val & _CURSOR_ALPHA_FORCE_MASK) >>
+				_CURSOR_ALPHA_FORCE_SHIFT;
+	if (alpha_plane || alpha_force)
+		gvt_dbg_core("alpha_plane=0x%x, alpha_force=0x%x\n",
+			alpha_plane, alpha_force);
+
+	plane->base = vgpu_vreg(vgpu, CURBASE(pipe)) & GTT_PAGE_MASK;
+	if (!intel_gvt_ggtt_validate_range(vgpu, plane->base, 0)) {
+		gvt_vgpu_err("invalid gma address: %lx\n",
+			     (unsigned long)plane->base);
+		return  -EINVAL;
+	}
+
+	plane->base_gpa = intel_vgpu_gma_to_gpa(vgpu->gtt.ggtt_mm, plane->base);
+	if (plane->base_gpa == INTEL_GVT_INVALID_ADDR) {
+		gvt_vgpu_err("invalid gma address: %lx\n",
+				(unsigned long)plane->base);
+		return  -EINVAL;
+	}
+
+	val = vgpu_vreg(vgpu, CURPOS(pipe));
+	plane->x_pos = (val & _CURSOR_POS_X_MASK) >> _CURSOR_POS_X_SHIFT;
+	plane->x_sign = (val & _CURSOR_SIGN_X_MASK) >> _CURSOR_SIGN_X_SHIFT;
+	plane->y_pos = (val & _CURSOR_POS_Y_MASK) >> _CURSOR_POS_Y_SHIFT;
+	plane->y_sign = (val & _CURSOR_SIGN_Y_MASK) >> _CURSOR_SIGN_Y_SHIFT;
+
+	return 0;
+}
+
+#define SPRITE_FORMAT_NUM	(1 << 3)
+
+static struct pixel_format sprite_pixel_formats[SPRITE_FORMAT_NUM] = {
+	[0x0] = {DRM_FORMAT_YUV422, 16, "YUV 16-bit 4:2:2 packed"},
+	[0x1] = {DRM_FORMAT_XRGB2101010, 32, "RGB 32-bit 2:10:10:10"},
+	[0x2] = {DRM_FORMAT_XRGB8888, 32, "RGB 32-bit 8:8:8:8"},
+	[0x4] = {DRM_FORMAT_AYUV, 32,
+		"YUV 32-bit 4:4:4 packed (8:8:8:8 MSB-X:Y:U:V)"},
+};
+
+/**
+ * intel_vgpu_decode_sprite_plane - Decode sprite plane
+ * @vgpu: input vgpu
+ * @plane: sprite plane to save decoded info
+ * This function is called for decoding plane
+ *
+ * Returns:
+ * 0 on success, non-zero if failed.
+ */
+int intel_vgpu_decode_sprite_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_sprite_plane_format *plane)
+{
+	u32 val, fmt;
+	u32 color_order, yuv_order;
+	int drm_format;
+	int pipe;
+
+	pipe = get_active_pipe(vgpu);
+	if (pipe >= I915_MAX_PIPES)
+		return -ENODEV;
+
+	val = vgpu_vreg(vgpu, SPRCTL(pipe));
+	plane->enabled = !!(val & SPRITE_ENABLE);
+	if (!plane->enabled)
+		return -ENODEV;
+
+	plane->tiled = !!(val & SPRITE_TILED);
+	color_order = !!(val & SPRITE_RGB_ORDER_RGBX);
+	yuv_order = (val & SPRITE_YUV_BYTE_ORDER_MASK) >>
+				_SPRITE_YUV_ORDER_SHIFT;
+
+	fmt = (val & SPRITE_PIXFORMAT_MASK) >> _SPRITE_FMT_SHIFT;
+	if (!sprite_pixel_formats[fmt].bpp) {
+		gvt_vgpu_err("Non-supported pixel format (0x%x)\n", fmt);
+		return -EINVAL;
+	}
+	plane->hw_format = fmt;
+	plane->bpp = sprite_pixel_formats[fmt].bpp;
+	drm_format = sprite_pixel_formats[fmt].drm_format;
+
+	/* Order of RGB values in an RGBxxx buffer may be ordered RGB or
+	 * BGR depending on the state of the color_order field
+	 */
+	if (!color_order) {
+		if (drm_format == DRM_FORMAT_XRGB2101010)
+			drm_format = DRM_FORMAT_XBGR2101010;
+		else if (drm_format == DRM_FORMAT_XRGB8888)
+			drm_format = DRM_FORMAT_XBGR8888;
+	}
+
+	if (drm_format == DRM_FORMAT_YUV422) {
+		switch (yuv_order) {
+		case 0:
+			drm_format = DRM_FORMAT_YUYV;
+			break;
+		case 1:
+			drm_format = DRM_FORMAT_UYVY;
+			break;
+		case 2:
+			drm_format = DRM_FORMAT_YVYU;
+			break;
+		case 3:
+			drm_format = DRM_FORMAT_VYUY;
+			break;
+		default:
+			/* yuv_order has only 2 bits */
+			break;
+		}
+	}
+
+	plane->drm_format = drm_format;
+
+	plane->base = vgpu_vreg(vgpu, SPRSURF(pipe)) & GTT_PAGE_MASK;
+	if (!intel_gvt_ggtt_validate_range(vgpu, plane->base, 0)) {
+		gvt_vgpu_err("invalid gma address: %lx\n",
+			     (unsigned long)plane->base);
+		return  -EINVAL;
+	}
+
+	plane->base_gpa = intel_vgpu_gma_to_gpa(vgpu->gtt.ggtt_mm, plane->base);
+	if (plane->base_gpa == INTEL_GVT_INVALID_ADDR) {
+		gvt_vgpu_err("invalid gma address: %lx\n",
+				(unsigned long)plane->base);
+		return  -EINVAL;
+	}
+
+	plane->stride = vgpu_vreg(vgpu, SPRSTRIDE(pipe)) &
+				_SPRITE_STRIDE_MASK;
+
+	val = vgpu_vreg(vgpu, SPRSIZE(pipe));
+	plane->height = (val & _SPRITE_SIZE_HEIGHT_MASK) >>
+		_SPRITE_SIZE_HEIGHT_SHIFT;
+	plane->width = (val & _SPRITE_SIZE_WIDTH_MASK) >>
+		_SPRITE_SIZE_WIDTH_SHIFT;
+	plane->height += 1;	/* raw height is one minus the real value */
+	plane->width += 1;	/* raw width is one minus the real value */
+
+	val = vgpu_vreg(vgpu, SPRPOS(pipe));
+	plane->x_pos = (val & _SPRITE_POS_X_MASK) >> _SPRITE_POS_X_SHIFT;
+	plane->y_pos = (val & _SPRITE_POS_Y_MASK) >> _SPRITE_POS_Y_SHIFT;
+
+	val = vgpu_vreg(vgpu, SPROFFSET(pipe));
+	plane->x_offset = (val & _SPRITE_OFFSET_START_X_MASK) >>
+			   _SPRITE_OFFSET_START_X_SHIFT;
+	plane->y_offset = (val & _SPRITE_OFFSET_START_Y_MASK) >>
+			   _SPRITE_OFFSET_START_Y_SHIFT;
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.h b/drivers/gpu/drm/i915/gvt/fb_decoder.h
new file mode 100644
index 0000000..cb055f3
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.h
@@ -0,0 +1,169 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *    Kevin Tian <kevin.tian@intel.com>
+ *
+ * Contributors:
+ *    Bing Niu <bing.niu@intel.com>
+ *    Xu Han <xu.han@intel.com>
+ *    Ping Gao <ping.a.gao@intel.com>
+ *    Xiaoguang Chen <xiaoguang.chen@intel.com>
+ *    Yang Liu <yang2.liu@intel.com>
+ *    Tina Zhang <tina.zhang@intel.com>
+ *
+ */
+
+#ifndef _GVT_FB_DECODER_H_
+#define _GVT_FB_DECODER_H_
+
+#define _PLANE_CTL_FORMAT_SHIFT		24
+#define _PLANE_CTL_TILED_SHIFT		10
+#define _PIPE_V_SRCSZ_SHIFT		0
+#define _PIPE_V_SRCSZ_MASK		(0xfff << _PIPE_V_SRCSZ_SHIFT)
+#define _PIPE_H_SRCSZ_SHIFT		16
+#define _PIPE_H_SRCSZ_MASK		(0x1fff << _PIPE_H_SRCSZ_SHIFT)
+
+#define _PRI_PLANE_FMT_SHIFT		26
+#define _PRI_PLANE_STRIDE_MASK		(0x3ff << 6)
+#define _PRI_PLANE_X_OFF_SHIFT		0
+#define _PRI_PLANE_X_OFF_MASK		(0x1fff << _PRI_PLANE_X_OFF_SHIFT)
+#define _PRI_PLANE_Y_OFF_SHIFT		16
+#define _PRI_PLANE_Y_OFF_MASK		(0xfff << _PRI_PLANE_Y_OFF_SHIFT)
+
+#define _CURSOR_MODE			0x3f
+#define _CURSOR_ALPHA_FORCE_SHIFT	8
+#define _CURSOR_ALPHA_FORCE_MASK	(0x3 << _CURSOR_ALPHA_FORCE_SHIFT)
+#define _CURSOR_ALPHA_PLANE_SHIFT	10
+#define _CURSOR_ALPHA_PLANE_MASK	(0x3 << _CURSOR_ALPHA_PLANE_SHIFT)
+#define _CURSOR_POS_X_SHIFT		0
+#define _CURSOR_POS_X_MASK		(0x1fff << _CURSOR_POS_X_SHIFT)
+#define _CURSOR_SIGN_X_SHIFT		15
+#define _CURSOR_SIGN_X_MASK		(1 << _CURSOR_SIGN_X_SHIFT)
+#define _CURSOR_POS_Y_SHIFT		16
+#define _CURSOR_POS_Y_MASK		(0xfff << _CURSOR_POS_Y_SHIFT)
+#define _CURSOR_SIGN_Y_SHIFT		31
+#define _CURSOR_SIGN_Y_MASK		(1 << _CURSOR_SIGN_Y_SHIFT)
+
+#define _SPRITE_FMT_SHIFT		25
+#define _SPRITE_COLOR_ORDER_SHIFT	20
+#define _SPRITE_YUV_ORDER_SHIFT		16
+#define _SPRITE_STRIDE_SHIFT		6
+#define _SPRITE_STRIDE_MASK		(0x1ff << _SPRITE_STRIDE_SHIFT)
+#define _SPRITE_SIZE_WIDTH_SHIFT	0
+#define _SPRITE_SIZE_HEIGHT_SHIFT	16
+#define _SPRITE_SIZE_WIDTH_MASK		(0x1fff << _SPRITE_SIZE_WIDTH_SHIFT)
+#define _SPRITE_SIZE_HEIGHT_MASK	(0xfff << _SPRITE_SIZE_HEIGHT_SHIFT)
+#define _SPRITE_POS_X_SHIFT		0
+#define _SPRITE_POS_Y_SHIFT		16
+#define _SPRITE_POS_X_MASK		(0x1fff << _SPRITE_POS_X_SHIFT)
+#define _SPRITE_POS_Y_MASK		(0xfff << _SPRITE_POS_Y_SHIFT)
+#define _SPRITE_OFFSET_START_X_SHIFT	0
+#define _SPRITE_OFFSET_START_Y_SHIFT	16
+#define _SPRITE_OFFSET_START_X_MASK	(0x1fff << _SPRITE_OFFSET_START_X_SHIFT)
+#define _SPRITE_OFFSET_START_Y_MASK	(0xfff << _SPRITE_OFFSET_START_Y_SHIFT)
+
+enum GVT_FB_EVENT {
+	FB_MODE_SET_START = 1,
+	FB_MODE_SET_END,
+	FB_DISPLAY_FLIP,
+};
+
+enum DDI_PORT {
+	DDI_PORT_NONE	= 0,
+	DDI_PORT_B	= 1,
+	DDI_PORT_C	= 2,
+	DDI_PORT_D	= 3,
+	DDI_PORT_E	= 4
+};
+
+struct intel_gvt;
+
+/* color space conversion and gamma correction are not included */
+struct intel_vgpu_primary_plane_format {
+	u8	enabled;	/* plane is enabled */
+	u8	tiled;		/* X-tiled */
+	u8	bpp;		/* bits per pixel */
+	u32	hw_format;	/* format field in the PRI_CTL register */
+	u32	drm_format;	/* format in DRM definition */
+	u32	base;		/* framebuffer base in graphics memory */
+	u64     base_gpa;
+	u32	x_offset;	/* in pixels */
+	u32	y_offset;	/* in lines */
+	u32	width;		/* in pixels */
+	u32	height;		/* in lines */
+	u32	stride;		/* in bytes */
+};
+
+struct intel_vgpu_sprite_plane_format {
+	u8	enabled;	/* plane is enabled */
+	u8	tiled;		/* X-tiled */
+	u8	bpp;		/* bits per pixel */
+	u32	hw_format;	/* format field in the SPR_CTL register */
+	u32	drm_format;	/* format in DRM definition */
+	u32	base;		/* sprite base in graphics memory */
+	u64     base_gpa;
+	u32	x_pos;		/* in pixels */
+	u32	y_pos;		/* in lines */
+	u32	x_offset;	/* in pixels */
+	u32	y_offset;	/* in lines */
+	u32	width;		/* in pixels */
+	u32	height;		/* in lines */
+	u32	stride;		/* in bytes */
+};
+
+struct intel_vgpu_cursor_plane_format {
+	u8	enabled;
+	u8	mode;		/* cursor mode select */
+	u8	bpp;		/* bits per pixel */
+	u32	drm_format;	/* format in DRM definition */
+	u32	base;		/* cursor base in graphics memory */
+	u64     base_gpa;
+	u32	x_pos;		/* in pixels */
+	u32	y_pos;		/* in lines */
+	u8	x_sign;		/* X Position Sign */
+	u8	y_sign;		/* Y Position Sign */
+	u32	width;		/* in pixels */
+	u32	height;		/* in lines */
+	u32	x_hot;		/* in pixels */
+	u32	y_hot;		/* in pixels */
+};
+
+struct intel_vgpu_pipe_format {
+	struct intel_vgpu_primary_plane_format	primary;
+	struct intel_vgpu_sprite_plane_format	sprite;
+	struct intel_vgpu_cursor_plane_format	cursor;
+	enum DDI_PORT ddi_port;  /* the DDI port that pipe is connected to */
+};
+
+struct intel_vgpu_fb_format {
+	struct intel_vgpu_pipe_format	pipes[I915_MAX_PIPES];
+};
+
+int intel_vgpu_decode_primary_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_primary_plane_format *plane);
+int intel_vgpu_decode_cursor_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_cursor_plane_format *plane);
+int intel_vgpu_decode_sprite_plane(struct intel_vgpu *vgpu,
+	struct intel_vgpu_sprite_plane_format *plane);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 4c1a442..0f562ee 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -46,6 +46,7 @@
 #include "sched_policy.h"
 #include "render.h"
 #include "cmd_parser.h"
+#include "fb_decoder.h"
 
 #define GVT_MAX_VGPU 8
 
-- 
2.7.4

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

[PATCH v14 2/7] drm: Introduce RGB 64-bit 16:16:16:16 float format

[Tina Zhang]

The RGB 64-bit 16:16:16:16 float pixel format is needed by some Apps in
windows. The float format in each component is 1:5:10 MSb-sign:exponent:
fraction.

This patch is to introduce the format to drm, so that the windows guest's
framebuffer in this kind of format can be recognized and used by linux
host.

v14:
- add some details about the float pixel format. (Daniel)
- add F suffix to the defined name. (Daniel)

v12:
- send to dri-devel at lists.freedesktop.org. (Ville)

v9:
- separated from framebuffer decoder patch. (Zhenyu) (Xiaoguang)

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 76c9101..575014f 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -113,6 +113,10 @@ extern "C" {
 
 #define DRM_FORMAT_AYUV		fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
 
+/* 64 bpp RGB 16:16:16:16 Floating Point */
+#define DRM_FORMAT_XRGB161616F  fourcc_code('X', 'R', '3', 'F') /* [63:0] x:R:G:B 16:16:16:16 little endian */
+#define DRM_FORMAT_XBGR161616F  fourcc_code('X', 'B', '3', 'F') /* [63:0] x:B:G:R 16:16:16:16 little endian */
+
 /*
  * 2 plane RGB + A
  * index 0 = RGB plane, same format as the corresponding non _A8 format has
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v14 3/7] drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format

[Tina Zhang]

The RGB 64-bit 16:16:16:16 float pixel format is needed by windows 10
guest VM. This patch is to add this pixel format support to gvt device
model. Without this patch, some Apps, e.g. "DXGIGammaVM.exe", will crash
and make guest screen black.

Signed-off-by: Tina Zhang <tina.zhang@intel.com>

diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c b/drivers/gpu/drm/i915/gvt/fb_decoder.c
index d380ab8..49053f4 100644
--- a/drivers/gpu/drm/i915/gvt/fb_decoder.c
+++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
@@ -52,6 +52,8 @@ static struct pixel_format bdw_pixel_formats[] = {
 
 	{DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
 	{DRM_FORMAT_XBGR8888, 32, "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
+	{DRM_FORMAT_XRGB161616F, 64,
+		"64-bit XRGB Floating Point (16:16:16:16 MSB-X:R:G:B)"},
 
 	/* non-supported format has bpp default to 0 */
 	{0, 0, NULL},
@@ -73,13 +75,18 @@ static struct pixel_format skl_pixel_formats[] = {
 	{DRM_FORMAT_XBGR2101010, 32, "32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"},
 	{DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
 
+	{DRM_FORMAT_XRGB161616F, 64,
+		"64-bit XRGB Floating Point (16:16:16:16 MSB-X:R:G:B)"},
+	{DRM_FORMAT_XBGR161616F, 64,
+		"64-bit XBGR Floating Point (16:16:16:16 MSB-X:B:G:R)"},
+
 	/* non-supported format has bpp default to 0 */
 	{0, 0, NULL},
 };
 
 static int bdw_format_to_drm(int format)
 {
-	int bdw_pixel_formats_index = 6;
+	int bdw_pixel_formats_index = 7;
 
 	switch (format) {
 	case DISPPLANE_8BPP:
@@ -100,7 +107,9 @@ static int bdw_format_to_drm(int format)
 	case DISPPLANE_RGBX888:
 		bdw_pixel_formats_index = 5;
 		break;
-
+	case DISPPLANE_RGBX161616:
+		bdw_pixel_formats_index = 6;
+		break;
 	default:
 		break;
 	}
@@ -111,7 +120,7 @@ static int bdw_format_to_drm(int format)
 static int skl_format_to_drm(int format, bool rgb_order, bool alpha,
 	int yuv_order)
 {
-	int skl_pixel_formats_index = 12;
+	int skl_pixel_formats_index = 14;
 
 	switch (format) {
 	case PLANE_CTL_FORMAT_INDEXED:
@@ -129,6 +138,9 @@ static int skl_format_to_drm(int format, bool rgb_order, bool alpha,
 	case PLANE_CTL_FORMAT_XRGB_2101010:
 		skl_pixel_formats_index = rgb_order ? 10 : 11;
 		break;
+	case PLANE_CTL_FORMAT_XRGB_16161616F:
+		skl_pixel_formats_index = rgb_order ? 12 : 13;
+		break;
 	case PLANE_CTL_FORMAT_YUV422:
 		skl_pixel_formats_index = yuv_order >> 16;
 		if (skl_pixel_formats_index > 3)
@@ -389,11 +401,13 @@ int intel_vgpu_decode_cursor_plane(struct intel_vgpu *vgpu,
 #define SPRITE_FORMAT_NUM	(1 << 3)
 
 static struct pixel_format sprite_pixel_formats[SPRITE_FORMAT_NUM] = {
-	[0x0] = {DRM_FORMAT_YUV422, 16, "YUV 16-bit 4:2:2 packed"},
-	[0x1] = {DRM_FORMAT_XRGB2101010, 32, "RGB 32-bit 2:10:10:10"},
-	[0x2] = {DRM_FORMAT_XRGB8888, 32, "RGB 32-bit 8:8:8:8"},
+	[0x0] = {DRM_FORMAT_YUV422, 16, "16-bit YUV 4:2:2 packed"},
+	[0x1] = {DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
+	[0x2] = {DRM_FORMAT_XRGB8888, 32, "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
+	[0x3]  = {DRM_FORMAT_XRGB161616F, 64,
+		    "64-bit XRGB Floating Point (16:16:16:16 MSB-X:R:G:B)"},
 	[0x4] = {DRM_FORMAT_AYUV, 32,
-		"YUV 32-bit 4:4:4 packed (8:8:8:8 MSB-X:Y:U:V)"},
+		"32-bit YUV 4:4:4 packed (8:8:8:8 MSB-X:Y:U:V)"},
 };
 
 /**
-- 
2.7.4

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

[PATCH v14 4/7] drm/i915/gvt: Add opregion support

[Tina Zhang]

Windows guest driver needs vbt in opregion, to configure the setting
for display. Without opregion support, the display registers won't
be set and this blocks display model to get the correct information
of the guest display plane.

This patch is to provide a virtual opregion for guest. Current
implementation is to fill the virtual opregion with the content in
host's opregion. The original author of this patch is Xiaoguang Chen.

Signed-off-by: Bing Niu <bing.niu@intel.com>
Signed-off-by: Tina Zhang <tina.zhang@intel.com>

diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
index df7f33a..32c345c 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -55,6 +55,7 @@ struct intel_gvt_mpt {
 			      unsigned long mfn, unsigned int nr, bool map);
 	int (*set_trap_area)(unsigned long handle, u64 start, u64 end,
 			     bool map);
+	int (*set_opregion)(void *vgpu);
 };
 
 extern struct intel_gvt_mpt xengt_mpt;
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index fd0c85f..6b0a330 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -53,11 +53,23 @@ static const struct intel_gvt_ops *intel_gvt_ops;
 #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,
+			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 {
@@ -430,6 +442,91 @@ 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 kvmgt_set_opregion(void *p_vgpu)
+{
+	struct intel_vgpu *vgpu = (struct intel_vgpu *)p_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;
+}
+
 static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 {
 	struct intel_vgpu *vgpu = NULL;
@@ -646,7 +743,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;
 	}
@@ -680,8 +777,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;
@@ -944,7 +1044,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) ?
@@ -1065,6 +1166,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;
@@ -1513,6 +1615,7 @@ struct intel_gvt_mpt kvmgt_mpt = {
 	.read_gpa = kvmgt_read_gpa,
 	.write_gpa = kvmgt_write_gpa,
 	.gfn_to_mfn = kvmgt_gfn_to_pfn,
+	.set_opregion = kvmgt_set_opregion,
 };
 EXPORT_SYMBOL_GPL(kvmgt_mpt);
 
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index f0e5487..9e73f2e 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -292,4 +292,19 @@ static inline int intel_gvt_hypervisor_set_trap_area(
 	return intel_gvt_host.mpt->set_trap_area(vgpu->handle, start, end, map);
 }
 
+/**
+ * intel_gvt_hypervisor_set_opregion - Set opregion for guest
+ * @vgpu: a vGPU
+ *
+ * Returns:
+ * Zero on success, negative error code if failed.
+ */
+static inline int intel_gvt_hypervisor_set_opregion(struct intel_vgpu *vgpu)
+{
+	if (!intel_gvt_host.mpt->set_opregion)
+		return 0;
+
+	return intel_gvt_host.mpt->set_opregion(vgpu);
+}
+
 #endif /* _GVT_MPT_H_ */
diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c
index 3117991..04c0452 100644
--- a/drivers/gpu/drm/i915/gvt/opregion.c
+++ b/drivers/gpu/drm/i915/gvt/opregion.c
@@ -113,23 +113,37 @@ void intel_vgpu_clean_opregion(struct intel_vgpu *vgpu)
  */
 int intel_vgpu_init_opregion(struct intel_vgpu *vgpu, u32 gpa)
 {
-	int ret;
+	int ret = 0;
+	unsigned long pfn;
 
 	gvt_dbg_core("vgpu%d: init vgpu opregion\n", vgpu->id);
 
-	if (intel_gvt_host.hypervisor_type == INTEL_GVT_HYPERVISOR_XEN) {
+	switch (intel_gvt_host.hypervisor_type) {
+	case INTEL_GVT_HYPERVISOR_KVM:
+		pfn = intel_gvt_hypervisor_gfn_to_mfn(vgpu, gpa >> PAGE_SHIFT);
+		vgpu_opregion(vgpu)->va = memremap(pfn << PAGE_SHIFT,
+						INTEL_GVT_OPREGION_SIZE,
+						MEMREMAP_WB);
+		if (!vgpu_opregion(vgpu)->va) {
+			gvt_vgpu_err("failed to map guest opregion\n");
+			ret = -EFAULT;
+		}
+		break;
+	case INTEL_GVT_HYPERVISOR_XEN:
 		gvt_dbg_core("emulate opregion from kernel\n");
 
 		ret = init_vgpu_opregion(vgpu, gpa);
 		if (ret)
-			return ret;
+			break;
 
 		ret = map_vgpu_opregion(vgpu, true);
-		if (ret)
-			return ret;
+		break;
+	default:
+		ret = -EINVAL;
+		gvt_vgpu_err("not supported hypervisor\n");
 	}
 
-	return 0;
+	return ret;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 3deadcb..bcbf535 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -383,6 +383,10 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
 	if (ret)
 		goto out_clean_shadow_ctx;
 
+	ret = intel_gvt_hypervisor_set_opregion(vgpu);
+	if (ret)
+		goto out_clean_shadow_ctx;
+
 	mutex_unlock(&gvt->lock);
 
 	return vgpu;
-- 
2.7.4

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

[PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Tina Zhang]

Add VFIO_DEVICE_QUERY_GFX_PLANE ioctl command to let user mode query and
get the plan and its related information. This ioctl can be invoked with:
1) either flag DMABUF or REGION is set. Vendor driver returns success and
the plane_info only when the specific kind of buffer is supported.
2) flag PROBE is set with either DMABUF or REGION. Vendor driver returns
success only when the specific kind of buffer is supported.

The dma-buf's life cycle is handled by user mode and tracked by kernel.
The returned fd in struct vfio_device_query_gfx_plane can be a new
fd or an old fd of a re-exported dma-buf. Host user mode can check the
value of fd and to see if it needs to create new resource according to
the new fd or just use the existed resource related to the old fd.

v14:
- add PROBE, DMABUF and REGION flags. (Alex)

v12:
- add drm_format_mod back. (Gerd and Zhenyu)
- add region_index. (Gerd)

v11:
- rename plane_type to drm_plane_type. (Gerd)
- move fields of vfio_device_query_gfx_plane to vfio_device_gfx_plane_info.
  (Gerd)
- remove drm_format_mod, start fields. (Daniel)
- remove plane_id.

v10:
- refine the ABI API VFIO_DEVICE_QUERY_GFX_PLANE. (Alex) (Gerd)

v3:
- add a field gvt_plane_info in the drm_i915_gem_obj structure to save
  the decoded plane information to avoid look up while need the plane
  info. (Gerd)

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index ae46105..fccc87f 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -502,6 +502,49 @@ struct vfio_pci_hot_reset {
 
 #define VFIO_DEVICE_PCI_HOT_RESET	_IO(VFIO_TYPE, VFIO_BASE + 13)
 
+/**
+ * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE, VFIO_BASE + 14, struct vfio_device_query_gfx_plane)
+ *
+ * Set the drm_plane_type and flags, then retrieve information about the gfx plane.
+ *
+ * flags:
+ *	VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_DMABUF are set to ask if the mdev
+ * 	    supports dma-buf. 0 on support, -EINVAL on no support for dma-buf.
+ *      VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_REGION are set to ask if the mdev
+ *          supports region. 0 on support, -EINVAL on no support for region.
+ *      VFIO_GFX_PLANE_TYPE_DMABUF or VFIO_GFX_PLANE_TYPE_REGION is set with each call to
+ *          query the plane info.
+ *      Others are invalid and return -EINVAL.
+ *
+ * Return: 0 on success, -ENODEV with all out fields zero on mdev device initialization, -errno on
+ * other failure.
+ */
+struct vfio_device_gfx_plane_info {
+	__u32 argsz;
+	__u32 flags;
+#define VFIO_GFX_PLANE_TYPE_PROBE (1 << 0)
+#define VFIO_GFX_PLANE_TYPE_DMABUF (1 << 1)
+#define VFIO_GFX_PLANE_TYPE_REGION (1 << 2)
+	/* in */
+	__u32 drm_plane_type;	/* type of plane: DRM_PLANE_TYPE_* */
+	/* out */
+	__u32 drm_format;	/* drm format of plane */
+	__u64 drm_format_mod;   /* tiled mode */
+	__u32 width;	/* width of plane */
+	__u32 height;	/* height of plane */
+	__u32 stride;	/* stride of plane */
+	__u32 size;	/* size of plane in bytes, align on page*/
+	__u32 x_pos;	/* horizontal position of cursor plane, upper left corner in pixels */
+	__u32 y_pos;	/* vertical position of cursor plane, upper left corner in lines*/
+	union {
+		__u32 region_index;	/* region index */
+		__s32 fd;	/* dma-buf fd */
+	};
+};
+
+#define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14)
+
+
 /* -------- API for Type1 VFIO IOMMU -------- */
 
 /**
-- 
2.7.4

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

[PATCH v14 6/7] drm/i915: Introduce GEM proxy

[Tina Zhang]

GEM proxy is a kind of GEM, whose backing physical memory is pinned
and produced by guest VM and is used by host as read only. With GEM
proxy, host is able to access guest physical memory through GEM object
interface. As GEM proxy is such a special kind of GEM, a new flag
I915_GEM_OBJECT_IS_PROXY is introduced to ban host from changing the
backing storage of GEM proxy.

v14:
- return -ENXIO when gem proxy object is banned by ioctl.
  (Chris) (Daniel)

v13:
- add comments to GEM proxy. (Chris)
- don't ban GEM proxy in i915_gem_sw_finish_ioctl. (Chris)
- check GEM proxy bar after finishing i915_gem_object_wait. (Chris)
- remove GEM proxy bar in i915_gem_madvise_ioctl.

v6:
- add gem proxy barrier in the following ioctls. (Chris)
  i915_gem_set_caching_ioctl
  i915_gem_set_domain_ioctl
  i915_gem_sw_finish_ioctl
  i915_gem_set_tiling_ioctl
  i915_gem_madvise_ioctl

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 000a764..7f1e7ab 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1584,6 +1584,16 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	if (err)
 		goto out;
 
+	/* Proxy objects do not control access to the backing storage, ergo
+	 * they cannot be used as a means to manipulate the cache domain
+	 * tracking for that backing storage. The proxy object is always
+	 * considered to be outside of any cache domain.
+	 */
+	if (i915_gem_object_is_proxy(obj)) {
+		err = -ENXIO;
+		goto out;
+	}
+
 	/* Flush and acquire obj->pages so that we are coherent through
 	 * direct access in memory with previous cached writes through
 	 * shmemfs and that our cache domain tracking remains valid.
@@ -1640,6 +1650,10 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
+	/* Proxy objects are barred from CPU access, so there is no
+	 * need to ban sw_finish as it is a nop.
+	 */
+
 	/* Pinned buffers may be scanout, so flush the cache */
 	i915_gem_object_flush_if_display(obj);
 	i915_gem_object_put(obj);
@@ -1690,7 +1704,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 	 */
 	if (!obj->base.filp) {
 		i915_gem_object_put(obj);
-		return -EINVAL;
+		return -ENXIO;
 	}
 
 	addr = vm_mmap(obj->base.filp, 0, args->size,
@@ -3749,6 +3763,14 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
+	/* The caching mode of proxy object is handled by its generator, and not
+	 * expected to be changed by user mode.
+	 */
+	if (i915_gem_object_is_proxy(obj)) {
+		ret = -ENXIO;
+		goto out;
+	}
+
 	if (obj->cache_level == level)
 		goto out;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 5b19a49..f3b382a 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -39,6 +39,7 @@ struct drm_i915_gem_object_ops {
 	unsigned int flags;
 #define I915_GEM_OBJECT_HAS_STRUCT_PAGE BIT(0)
 #define I915_GEM_OBJECT_IS_SHRINKABLE   BIT(1)
+#define I915_GEM_OBJECT_IS_PROXY	BIT(2)
 
 	/* Interface between the GEM object and its backing storage.
 	 * get_pages() is called once prior to the use of the associated set
@@ -300,6 +301,12 @@ i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj)
 }
 
 static inline bool
+i915_gem_object_is_proxy(const struct drm_i915_gem_object *obj)
+{
+	return obj->ops->flags & I915_GEM_OBJECT_IS_PROXY;
+}
+
+static inline bool
 i915_gem_object_is_active(const struct drm_i915_gem_object *obj)
 {
 	return obj->active_count;
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index fb5231f..f617012 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -345,6 +345,14 @@ i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
 	if (!obj)
 		return -ENOENT;
 
+	/* The tiling mode of proxy objects is handled by its generator, and not
+	 * expected to be changed by user mode.
+	 */
+	if (i915_gem_object_is_proxy(obj)) {
+		err = -ENXIO;
+		goto err;
+	}
+
 	if (!i915_tiling_ok(obj, args->tiling_mode, args->stride)) {
 		err = -EINVAL;
 		goto err;
-- 
2.7.4

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

[PATCH v14 7/7] drm/i915/gvt: Dmabuf support for GVT-g

[Tina Zhang]

This patch introduces a guest's framebuffer sharing mechanism based on
dma-buf subsystem. With this sharing mechanism, guest's framebuffer can
be shared between guest VM and host.

v14:
- add PROBE, DMABUF and REGION flags. (Alex)

v12:
- refine the lifecycle of dmabuf.

v9:
- remove dma-buf management. (Alex)
- track the dma-buf create and release in kernel mode. (Gerd) (Daniel)

v8:
- refine the dma-buf ioctl definition.(Alex)
- add a lock to protect the dmabuf list. (Alex)

v7:
- release dma-buf related allocations in dma-buf's associated release
  function. (Alex)
- refine ioctl interface for querying plane info or create dma-buf.
  (Alex)

v6:
- align the dma-buf life cycle with the vfio device. (Alex)
- add the dma-buf related operations in a separate patch. (Gerd)
- i915 related changes. (Chris)

v5:
- fix bug while checking whether the gem obj is gvt's dma-buf when user
  change caching mode or domains. Add a helper function to do it.
  (Xiaoguang)
- add definition for the query plane and create dma-buf. (Xiaoguang)

v4:
- fix bug while checking whether the gem obj is gvt's dma-buf when set
  caching mode or doamins. (Xiaoguang)

v3:
- declare a new flag I915_GEM_OBJECT_IS_GVT_DMABUF in drm_i915_gem_object
  to represent the gem obj for gvt's dma-buf. The tiling mode, caching
  mode and domains can not be changed for this kind of gem object. (Alex)
- change dma-buf related information to be more generic. So other vendor
  can use the same interface. (Alex)

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

Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Gerd Hoffmann <kraxel@redhat.com>

diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
index 019d596..18f43cb 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -2,7 +2,7 @@ GVT_DIR := gvt
 GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
 	interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
 	execlist.o scheduler.o sched_policy.o render.o cmd_parser.o \
-	fb_decoder.o
+	fb_decoder.o dmabuf.o
 
 ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
 i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c
new file mode 100644
index 0000000..ca91321
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
@@ -0,0 +1,387 @@
+/*
+ * Copyright 2017 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Zhiyuan Lv <zhiyuan.lv@intel.com>
+ *
+ * Contributors:
+ *    Xiaoguang Chen
+ *    Tina Zhang <tina.zhang@intel.com>
+ */
+
+#include <linux/dma-buf.h>
+#include <drm/drmP.h>
+#include <linux/vfio.h>
+
+#include "i915_drv.h"
+#include "gvt.h"
+
+#define GEN8_DECODE_PTE(pte) (pte & GENMASK_ULL(63, 12))
+
+#define VBLNAK_TIMER_PERIOD 16000000
+
+static struct sg_table *intel_vgpu_gem_get_pages(
+		struct drm_i915_gem_object *obj)
+{
+	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+	struct sg_table *st;
+	struct scatterlist *sg;
+	int i, ret;
+	gen8_pte_t __iomem *gtt_entries;
+	struct intel_vgpu_fb_info *fb_info;
+
+	fb_info = (struct intel_vgpu_fb_info *)obj->gvt_info;
+	if (WARN_ON(!fb_info))
+		return ERR_PTR(-ENODEV);
+
+	st = kmalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return ERR_PTR(-ENOMEM);
+
+	ret = sg_alloc_table(st, fb_info->size, GFP_KERNEL);
+	if (ret) {
+		kfree(st);
+		return ERR_PTR(ret);
+	}
+	gtt_entries = (gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
+		(fb_info->start >> PAGE_SHIFT);
+	for_each_sg(st->sgl, sg, fb_info->size, i) {
+		sg->offset = 0;
+		sg->length = PAGE_SIZE;
+		sg_dma_address(sg) =
+			GEN8_DECODE_PTE(readq(&gtt_entries[i]));
+		sg_dma_len(sg) = PAGE_SIZE;
+	}
+
+	return st;
+}
+
+static void intel_vgpu_gem_put_pages(struct drm_i915_gem_object *obj,
+		struct sg_table *pages)
+{
+	sg_free_table(pages);
+	kfree(pages);
+}
+
+static void intel_vgpu_gem_release(struct drm_i915_gem_object *obj)
+{
+	struct intel_vgpu_dmabuf_obj *dmabuf_obj;
+	struct intel_vgpu_fb_info *fb_info;
+	struct intel_vgpu *vgpu;
+	struct list_head *pos;
+
+	fb_info = (struct intel_vgpu_fb_info *)obj->gvt_info;
+	if (WARN_ON(!fb_info || !fb_info->vgpu)) {
+		gvt_err("gvt info is invalid\n");
+		goto out;
+	}
+
+	vgpu = fb_info->vgpu;
+	mutex_lock(&vgpu->dmabuf_list_lock);
+	list_for_each(pos, &vgpu->dmabuf_obj_list_head) {
+		dmabuf_obj = container_of(pos, struct intel_vgpu_dmabuf_obj,
+						list);
+		if ((dmabuf_obj != NULL) && (dmabuf_obj->obj == obj)) {
+			kfree(dmabuf_obj);
+			list_del(pos);
+			break;
+		}
+	}
+	mutex_unlock(&vgpu->dmabuf_list_lock);
+	intel_gvt_hypervisor_put_vfio_device(vgpu);
+out:
+	kfree(obj->gvt_info);
+}
+
+static const struct drm_i915_gem_object_ops intel_vgpu_gem_ops = {
+	.flags = I915_GEM_OBJECT_IS_PROXY,
+	.get_pages = intel_vgpu_gem_get_pages,
+	.put_pages = intel_vgpu_gem_put_pages,
+	.release = intel_vgpu_gem_release,
+};
+
+static struct drm_i915_gem_object *intel_vgpu_create_gem(struct drm_device *dev,
+		struct intel_vgpu_fb_info *info)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_gem_object *obj;
+
+	obj = i915_gem_object_alloc(dev_priv);
+	if (obj == NULL)
+		return NULL;
+
+	drm_gem_private_object_init(dev, &obj->base,
+		info->size << PAGE_SHIFT);
+	i915_gem_object_init(obj, &intel_vgpu_gem_ops);
+
+	obj->base.read_domains = I915_GEM_DOMAIN_GTT;
+	obj->base.write_domain = 0;
+	if (IS_SKYLAKE(dev_priv)) {
+		unsigned int tiling_mode = 0;
+		unsigned int stride = 0;
+
+		switch (info->drm_format_mod << 10) {
+		case PLANE_CTL_TILED_LINEAR:
+			tiling_mode = I915_TILING_NONE;
+			break;
+		case PLANE_CTL_TILED_X:
+			tiling_mode = I915_TILING_X;
+			stride = info->stride;
+			break;
+		case PLANE_CTL_TILED_Y:
+			tiling_mode = I915_TILING_Y;
+			stride = info->stride;
+			break;
+		default:
+			gvt_dbg_core("not supported tiling mode\n");
+		}
+		obj->tiling_and_stride = tiling_mode | stride;
+	} else {
+		obj->tiling_and_stride = info->drm_format_mod ?
+					I915_TILING_X : 0;
+	}
+
+	return obj;
+}
+
+static int intel_vgpu_get_plane_info(struct drm_device *dev,
+		struct intel_vgpu *vgpu,
+		struct intel_vgpu_fb_info *info,
+		int plane_id)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct intel_vgpu_primary_plane_format p;
+	struct intel_vgpu_cursor_plane_format c;
+	int ret;
+
+	if (plane_id == DRM_PLANE_TYPE_PRIMARY) {
+		ret = intel_vgpu_decode_primary_plane(vgpu, &p);
+		if (ret)
+			return ret;
+		info->start = p.base;
+		info->start_gpa = p.base_gpa;
+		info->width = p.width;
+		info->height = p.height;
+		info->stride = p.stride;
+		info->drm_format = p.drm_format;
+		info->drm_format_mod = p.tiled;
+		info->size = (((p.stride * p.height * p.bpp) / 8) +
+				(PAGE_SIZE - 1)) >> PAGE_SHIFT;
+	} else if (plane_id == DRM_PLANE_TYPE_CURSOR) {
+		ret = intel_vgpu_decode_cursor_plane(vgpu, &c);
+		if (ret)
+			return ret;
+		info->start = c.base;
+		info->start_gpa = c.base_gpa;
+		info->width = c.width;
+		info->height = c.height;
+		info->stride = c.width * (c.bpp / 8);
+		info->drm_format = c.drm_format;
+		info->drm_format_mod = 0;
+		info->x_pos = c.x_pos;
+		info->y_pos = c.y_pos;
+		info->size = (((info->stride * c.height * c.bpp) / 8)
+				+ (PAGE_SIZE - 1)) >> PAGE_SHIFT;
+	} else {
+		gvt_vgpu_err("invalid plane id:%d\n", plane_id);
+		return -EINVAL;
+	}
+
+	if (info->size == 0) {
+		gvt_vgpu_err("fb size is zero\n");
+		return -EINVAL;
+	}
+
+	if (info->start & (PAGE_SIZE - 1)) {
+		gvt_vgpu_err("Not aligned fb address:0x%llx\n", info->start);
+		return -EFAULT;
+	}
+	if (((info->start >> PAGE_SHIFT) + info->size) >
+		ggtt_total_entries(&dev_priv->ggtt)) {
+		gvt_vgpu_err("Invalid GTT offset or size\n");
+		return -EFAULT;
+	}
+
+	if (!intel_gvt_ggtt_validate_range(vgpu, info->start, info->size)) {
+		gvt_vgpu_err("invalid gma addr\n");
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+static int intel_vgpu_pick_exposed_dmabuf(struct intel_vgpu *vgpu,
+		struct intel_vgpu_fb_info *latest_info)
+{
+	struct list_head *pos;
+	struct intel_vgpu_fb_info *fb_info;
+	struct intel_vgpu_dmabuf_obj *dmabuf_obj;
+	struct dma_buf *dmabuf;
+	int ret = -1;
+
+	mutex_lock(&vgpu->dmabuf_list_lock);
+	list_for_each(pos, &vgpu->dmabuf_obj_list_head) {
+		dmabuf_obj = container_of(pos, struct intel_vgpu_dmabuf_obj,
+						list);
+		if ((dmabuf_obj == NULL) ||
+		    (dmabuf_obj->obj == NULL) ||
+		    (dmabuf_obj->obj->gvt_info == NULL))
+			continue;
+
+		fb_info = dmabuf_obj->obj->gvt_info;
+
+		if ((fb_info->start == latest_info->start) &&
+		    (fb_info->start_gpa == latest_info->start_gpa) &&
+		    (fb_info->size == latest_info->size) &&
+		    (fb_info->drm_format_mod == latest_info->drm_format_mod) &&
+		    (fb_info->drm_format == latest_info->drm_format) &&
+		    (fb_info->width == latest_info->width) &&
+		    (fb_info->height == latest_info->height) &&
+		    (fb_info->stride == latest_info->stride)) {
+			dmabuf = dma_buf_get(dmabuf_obj->fd);
+			if (IS_ERR(dmabuf)) {
+				continue;
+			}
+			dma_buf_put(dmabuf);
+			ret = dmabuf_obj->fd;
+			break;
+		}
+	}
+	mutex_unlock(&vgpu->dmabuf_list_lock);
+
+	return ret;
+}
+
+static void update_fb_info(struct vfio_device_gfx_plane_info *gvt_dmabuf,
+		      struct intel_vgpu_fb_info *fb_info)
+{
+	gvt_dmabuf->drm_format = fb_info->drm_format;
+	gvt_dmabuf->width = fb_info->width;
+	gvt_dmabuf->height = fb_info->height;
+	gvt_dmabuf->stride = fb_info->stride;
+	gvt_dmabuf->size = fb_info->size;
+	gvt_dmabuf->x_pos = fb_info->x_pos;
+	gvt_dmabuf->y_pos = fb_info->y_pos;
+}
+
+int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args)
+{
+	struct drm_device *dev = &vgpu->gvt->dev_priv->drm;
+	struct vfio_device_gfx_plane_info *gvt_dmabuf = args;
+	struct intel_vgpu_dmabuf_obj *dmabuf_obj;
+	struct drm_i915_gem_object *obj;
+	struct intel_vgpu_fb_info fb_info;
+	struct dma_buf *dmabuf;
+	int ret = 0;
+
+	if (gvt_dmabuf->flags == (VFIO_GFX_PLANE_TYPE_DMABUF |
+				       VFIO_GFX_PLANE_TYPE_PROBE))
+		return ret;
+	else if ((gvt_dmabuf->flags & ~VFIO_GFX_PLANE_TYPE_DMABUF) ||
+			(!gvt_dmabuf->flags))
+		return -EINVAL;
+
+	ret = intel_vgpu_get_plane_info(dev, vgpu, &fb_info,
+					gvt_dmabuf->drm_plane_type);
+	if (ret != 0)
+		goto out;
+
+	/* If exists, pick up the exposed dmabuf fd */
+	ret = intel_vgpu_pick_exposed_dmabuf(vgpu, &fb_info);
+	if (ret > 0) {
+		update_fb_info(gvt_dmabuf, &fb_info);
+		gvt_dmabuf->fd = ret;
+		ret = 0;
+		goto out;
+	}
+
+	/* Need to expose a new one*/
+	dmabuf_obj = kmalloc(sizeof(struct intel_vgpu_dmabuf_obj), GFP_KERNEL);
+	if (!dmabuf_obj) {
+		gvt_vgpu_err("alloc dmabuf_obj failed\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	obj = intel_vgpu_create_gem(dev, &fb_info);
+	if (obj == NULL) {
+		gvt_vgpu_err("create gvt gem obj failed:%d\n", vgpu->id);
+		ret = -ENOMEM;
+		goto out_free_dmabuf;
+	}
+
+	dmabuf_obj->obj = obj;
+
+	obj->gvt_info = kmalloc(sizeof(struct intel_vgpu_fb_info), GFP_KERNEL);
+	if (!obj->gvt_info) {
+		gvt_vgpu_err("allocate intel vgpu fb info failed\n");
+		ret = -ENOMEM;
+		goto out_free_gem;
+	}
+
+	fb_info.vgpu = vgpu;
+	dmabuf_obj->vgpu = vgpu;
+	memcpy(obj->gvt_info, &fb_info, sizeof(struct intel_vgpu_fb_info));
+
+	dmabuf = i915_gem_prime_export(dev, &obj->base, DRM_CLOEXEC | DRM_RDWR);
+	if (IS_ERR(dmabuf)) {
+		gvt_vgpu_err("export dma-buf failed\n");
+		ret = PTR_ERR(dmabuf);
+		goto out_free_info;
+	}
+	i915_gem_object_put(obj);
+	obj->base.dma_buf = dmabuf;
+
+	ret = dma_buf_fd(dmabuf, DRM_CLOEXEC | DRM_RDWR);
+	if (ret < 0) {
+		gvt_vgpu_err("create dma-buf fd failed ret:%d\n", ret);
+		goto out_free;
+	}
+
+	if (intel_gvt_hypervisor_get_vfio_device(vgpu)) {
+		gvt_vgpu_err("get vfio device failed\n");
+		put_unused_fd(ret);
+		goto out_free;
+	}
+
+	update_fb_info(gvt_dmabuf, &fb_info);
+	gvt_dmabuf->fd = ret;
+	dmabuf_obj->fd = ret;
+
+	INIT_LIST_HEAD(&dmabuf_obj->list);
+	mutex_lock(&vgpu->dmabuf_list_lock);
+	list_add_tail(&dmabuf_obj->list, &vgpu->dmabuf_obj_list_head);
+	mutex_unlock(&vgpu->dmabuf_list_lock);
+
+	return 0;
+
+out_free:
+	dma_buf_put(dmabuf);
+out_free_info:
+	kfree(obj->gvt_info);
+out_free_gem:
+	i915_gem_object_put(obj);
+out_free_dmabuf:
+	kfree(dmabuf_obj);
+out:
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.h b/drivers/gpu/drm/i915/gvt/dmabuf.h
new file mode 100644
index 0000000..e8ab894
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright(c) 2017 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *    Zhiyuan Lv <zhiyuan.lv@intel.com>
+ *
+ * Contributors:
+ *    Xiaoguang Chen
+ *    Tina Zhang <tina.zhang@intel.com>
+ */
+
+#ifndef _GVT_DMABUF_H_
+#define _GVT_DMABUF_H_
+#include <linux/vfio.h>
+
+struct intel_vgpu_fb_info {
+	__u64 start;
+	__u64 start_gpa;
+	__u64 drm_format_mod;
+	__u32 drm_format;	/* drm format of plane */
+	__u32 width;	/* width of plane */
+	__u32 height;	/* height of plane */
+	__u32 stride;	/* stride of plane */
+	__u32 size;	/* size of plane in bytes, align on page*/
+	__u32 x_pos;	/* horizontal position of cursor plane, upper left corner in pixels */
+	__u32 y_pos;	/* vertical position of cursor plane, upper left corner in lines*/
+	struct intel_vgpu *vgpu;
+};
+
+struct intel_vgpu_dmabuf_obj {
+	struct intel_vgpu *vgpu;
+	struct drm_i915_gem_object *obj;
+	int fd;
+	struct list_head list;
+};
+
+int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index c27c683..f77770b 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -54,6 +54,7 @@ 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,
 };
 
 /**
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 0f562ee..0f313ee 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -47,6 +47,7 @@
 #include "render.h"
 #include "cmd_parser.h"
 #include "fb_decoder.h"
+#include "dmabuf.h"
 
 #define GVT_MAX_VGPU 8
 
@@ -183,8 +184,15 @@ struct intel_vgpu {
 		struct kvm *kvm;
 		struct work_struct release_work;
 		atomic_t released;
+		struct vfio_device *vfio_device;
 	} vdev;
 #endif
+
+	struct list_head dmabuf_obj_list_head;
+	struct mutex dmabuf_list_lock;
+
+	struct completion vblank_done;
+
 };
 
 struct intel_gvt_gm {
@@ -500,6 +508,7 @@ 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 *);
 };
 
 
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
index 32c345c..a1bd82f 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -56,6 +56,8 @@ struct intel_gvt_mpt {
 	int (*set_trap_area)(unsigned long handle, u64 start, u64 end,
 			     bool map);
 	int (*set_opregion)(void *vgpu);
+	int (*get_vfio_device)(void *vgpu);
+	void (*put_vfio_device)(void *vgpu);
 };
 
 extern struct intel_gvt_mpt xengt_mpt;
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 6b0a330..cac4aeb 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -492,10 +492,23 @@ static int intel_vgpu_register_reg(struct intel_vgpu *vgpu,
 	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 kvmgt_get_vfio_device(void *p_vgpu)
+{
+	struct intel_vgpu *vgpu = (struct intel_vgpu *)p_vgpu;
 
+	vgpu->vdev.vfio_device = vfio_device_get_from_dev(
+		mdev_dev(vgpu->vdev.mdev));
+	if (!vgpu->vdev.vfio_device) {
+		gvt_vgpu_err("failed to get vfio device\n");
+		return -ENODEV;
+	}
 	return 0;
 }
 
+
 static int kvmgt_set_opregion(void *p_vgpu)
 {
 	struct intel_vgpu *vgpu = (struct intel_vgpu *)p_vgpu;
@@ -527,6 +540,14 @@ static int kvmgt_set_opregion(void *p_vgpu)
 	return ret;
 }
 
+static void kvmgt_put_vfio_device(void *vgpu)
+{
+	if (WARN_ON(!((struct intel_vgpu *)vgpu)->vdev.vfio_device))
+		return;
+
+	vfio_device_put(((struct intel_vgpu *)vgpu)->vdev.vfio_device);
+}
+
 static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 {
 	struct intel_vgpu *vgpu = NULL;
@@ -1253,6 +1274,22 @@ 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_QUERY_GFX_PLANE) {
+		struct vfio_device_gfx_plane_info dmabuf;
+		int ret = 0;
+
+		minsz = offsetofend(struct vfio_device_gfx_plane_info, fd);
+		if (copy_from_user(&dmabuf, (void __user *)arg, minsz))
+			return -EFAULT;
+		if (dmabuf.argsz < minsz)
+			return -EINVAL;
+
+		ret = intel_gvt_ops->vgpu_query_plane(vgpu, &dmabuf);
+		if (ret != 0)
+			return ret;
+
+		return copy_to_user((void __user *)arg, &dmabuf, minsz) ?
+								-EFAULT : 0;
 	}
 
 	return 0;
@@ -1475,6 +1512,9 @@ static int kvmgt_guest_init(struct mdev_device *mdev)
 	kvmgt_protect_table_init(info);
 	gvt_cache_init(vgpu);
 
+	mutex_init(&vgpu->dmabuf_list_lock);
+	init_completion(&vgpu->vblank_done);
+
 	info->track_node.track_write = kvmgt_page_track_write;
 	info->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
 	kvm_page_track_register_notifier(kvm, &info->track_node);
@@ -1616,6 +1656,8 @@ struct intel_gvt_mpt kvmgt_mpt = {
 	.write_gpa = kvmgt_write_gpa,
 	.gfn_to_mfn = kvmgt_gfn_to_pfn,
 	.set_opregion = kvmgt_set_opregion,
+	.get_vfio_device = kvmgt_get_vfio_device,
+	.put_vfio_device = kvmgt_put_vfio_device,
 };
 EXPORT_SYMBOL_GPL(kvmgt_mpt);
 
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
index 9e73f2e..86443ff 100644
--- a/drivers/gpu/drm/i915/gvt/mpt.h
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -307,4 +307,34 @@ static inline int intel_gvt_hypervisor_set_opregion(struct intel_vgpu *vgpu)
 	return intel_gvt_host.mpt->set_opregion(vgpu);
 }
 
+/**
+ * intel_gvt_hypervisor_get_vfio_device - increase vfio device ref count
+ * @vgpu: a vGPU
+ *
+ * Returns:
+ * Zero on success, negative error code if failed.
+ */
+static inline int intel_gvt_hypervisor_get_vfio_device(struct intel_vgpu *vgpu)
+{
+	if (!intel_gvt_host.mpt->get_vfio_device)
+		return 0;
+
+	return intel_gvt_host.mpt->get_vfio_device(vgpu);
+}
+
+/**
+ * intel_gvt_hypervisor_put_vfio_device - decrease vfio device ref count
+ * @vgpu: a vGPU
+ *
+ * Returns:
+ * Zero on success, negative error code if failed.
+ */
+static inline void intel_gvt_hypervisor_put_vfio_device(struct intel_vgpu *vgpu)
+{
+	if (!intel_gvt_host.mpt->put_vfio_device)
+		return;
+
+	intel_gvt_host.mpt->put_vfio_device(vgpu);
+}
+
 #endif /* _GVT_MPT_H_ */
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index bcbf535..19cbbbb 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -346,7 +346,7 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
 	vgpu->gvt = gvt;
 	vgpu->sched_ctl.weight = param->weight;
 	bitmap_zero(vgpu->tlb_handle_pending, I915_NUM_ENGINES);
-
+	INIT_LIST_HEAD(&vgpu->dmabuf_obj_list_head);
 	intel_vgpu_init_cfg_space(vgpu, param->primary);
 
 	ret = intel_vgpu_init_mmio(vgpu);
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index f3b382a..c91e336 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -199,6 +199,8 @@ struct drm_i915_gem_object {
 		} userptr;
 
 		unsigned long scratch;
+
+		void *gvt_info;
 	};
 
 	/** for phys allocated objects */
-- 
2.7.4

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

Re: [PATCH v14 6/7] drm/i915: Introduce GEM proxy

[Joonas Lahtinen]

On Fri, 2017-08-18 at 18:21 +0800, Tina Zhang wrote:
> GEM proxy is a kind of GEM, whose backing physical memory is pinned
> and produced by guest VM and is used by host as read only. With GEM
> proxy, host is able to access guest physical memory through GEM object
> interface. As GEM proxy is such a special kind of GEM, a new flag
> I915_GEM_OBJECT_IS_PROXY is introduced to ban host from changing the
> backing storage of GEM proxy.
> 
> v14:
> - return -ENXIO when gem proxy object is banned by ioctl.
>   (Chris) (Daniel)
> 
> v13:
> - add comments to GEM proxy. (Chris)
> - don't ban GEM proxy in i915_gem_sw_finish_ioctl. (Chris)
> - check GEM proxy bar after finishing i915_gem_object_wait. (Chris)
> - remove GEM proxy bar in i915_gem_madvise_ioctl.
> 
> v6:
> - add gem proxy barrier in the following ioctls. (Chris)
>   i915_gem_set_caching_ioctl
>   i915_gem_set_domain_ioctl
>   i915_gem_sw_finish_ioctl
>   i915_gem_set_tiling_ioctl
>   i915_gem_madvise_ioctl
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

This is now as agreed.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

PS. I think you have --no-stat option turned on in git format-patch.
Please turn it back off, it'll make it far easier for anybody reading
the patches to get an overview of which files the patch touches.
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/gvt: Dma-buf support for GVT-g

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: Dma-buf support for GVT-g
URL   : https://patchwork.freedesktop.org/series/28980/
State : success

== Summary ==

Series 28980v1 drm/i915/gvt: Dma-buf support for GVT-g
https://patchwork.freedesktop.org/api/1.0/series/28980/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test gem_ringfill:
        Subgroup basic-default:
                pass       -> SKIP       (fi-bsw-n3050) fdo#101915

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:451s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:441s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:362s
fi-bsw-n3050     total:279  pass:242  dwarn:0   dfail:0   fail:0   skip:37  time:560s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:521s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:528s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:511s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:609s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:443s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:423s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:425s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:510s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:478s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:596s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:597s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:533s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:473s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:483s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:488s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:446s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:484s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:546s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:404s

8183095ca4abce6ec2ad43c1e36e877792c140f1 drm-tip: 2017y-08m-18d-09h-08m-28s UTC integration manifest
06591ef22442 drm/i915/gvt: Dmabuf support for GVT-g
69d9112b7ab5 drm/i915: Introduce GEM proxy
d905c36647f8 vfio: ABI for mdev display dma-buf operation
bd594b6c0125 drm/i915/gvt: Add opregion support
83284f2a7ef6 drm/i915/gvt: Add RGB 64-bit 16:16:16:16 float format
09d19af42ee3 drm: Introduce RGB 64-bit 16:16:16:16 float format
3fc810c1addb drm/i915/gvt: Add framebuffer decoder support

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5437/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Gerd Hoffmann]

On Fri, 2017-08-18 at 18:21 +0800, Tina Zhang wrote:
> +/**
> + * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE, VFIO_BASE + 14,
> struct vfio_device_query_gfx_plane)
> + *
> + * Set the drm_plane_type and flags, then retrieve information about
> the gfx plane.
> + *
> + * flags:
> + *     VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_DMABUF are
> set to ask if the mdev
> + *         supports dma-buf. 0 on support, -EINVAL on no support for
> dma-buf.
> + *      VFIO_GFX_PLANE_TYPE_PROBE and VFIO_GFX_PLANE_TYPE_REGION are
> set to ask if the mdev
> + *          supports region. 0 on support, -EINVAL on no support for
> region.
> + *      VFIO_GFX_PLANE_TYPE_DMABUF or VFIO_GFX_PLANE_TYPE_REGION is
> set with each call to
> + *          query the plane info.
> + *      Others are invalid and return -EINVAL.
> + *
> + * Return: 0 on success, -ENODEV with all out fields zero on mdev
> device initialization,

Do you mean PROBE calls here?

The behavior in case the guest driver hasn't yet initialized the plane 
should be documented here too.

cheers,
  Gerd

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

Re: [PATCH v14 1/7] drm/i915/gvt: Add framebuffer decoder support

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 2289 bytes --]

On 2017.08.18 18:21:30 +0800, Tina Zhang wrote:
> This patch is to introduce the framebuffer decoder which can decode guest
> OS's framebuffer information, including primary, cursor and sprite plane.
> 
> v14:
> - refine pixel format table. (Zhenyu)
> 
> v9:
> - move drm format change to a separate patch. (Xiaoguang)
> 
> v8:
> - fix a bug in decoding primary plane. (Tina)
> 
> v7:
> - refine framebuffer decoder code. (Zhenyu)
> 
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
>

> +static struct pixel_format bdw_pixel_formats[] = {
> +	{DRM_FORMAT_C8, 8, "8-bit Indexed"},
> +	{DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"},
> +	{DRM_FORMAT_XRGB8888, 32, "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"},
> +	{DRM_FORMAT_XBGR2101010, 32, "32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"},
> +
> +	{DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
> +	{DRM_FORMAT_XBGR8888, 32, "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
> +
> +	/* non-supported format has bpp default to 0 */
> +	{0, 0, NULL},
> +};
> +
> +static struct pixel_format skl_pixel_formats[] = {
> +	{DRM_FORMAT_YUYV, 16, "16-bit packed YUYV (8:8:8:8 MSB-V:Y2:U:Y1)"},
> +	{DRM_FORMAT_UYVY, 16, "16-bit packed UYVY (8:8:8:8 MSB-Y2:V:Y1:U)"},
> +	{DRM_FORMAT_YVYU, 16, "16-bit packed YVYU (8:8:8:8 MSB-U:Y2:V:Y1)"},
> +	{DRM_FORMAT_VYUY, 16, "16-bit packed VYUY (8:8:8:8 MSB-Y2:U:Y1:V)"},
> +
> +	{DRM_FORMAT_C8, 8, "8-bit Indexed"},
> +	{DRM_FORMAT_RGB565, 16, "16-bit BGRX (5:6:5 MSB-R:G:B)"},
> +	{DRM_FORMAT_ABGR8888, 32, "32-bit RGBA (8:8:8:8 MSB-A:B:G:R)"},
> +	{DRM_FORMAT_XBGR8888, 32, "32-bit RGBX (8:8:8:8 MSB-X:B:G:R)"},
> +
> +	{DRM_FORMAT_ARGB8888, 32, "32-bit BGRA (8:8:8:8 MSB-A:R:G:B)"},
> +	{DRM_FORMAT_XRGB8888, 32, "32-bit BGRX (8:8:8:8 MSB-X:R:G:B)"},
> +	{DRM_FORMAT_XBGR2101010, 32, "32-bit RGBX (2:10:10:10 MSB-X:B:G:R)"},
> +	{DRM_FORMAT_XRGB2101010, 32, "32-bit BGRX (2:10:10:10 MSB-X:R:G:B)"},
> +
> +	/* non-supported format has bpp default to 0 */
> +	{0, 0, NULL},
> +};

What about KBL support of this? If not fully supported now, need to set
proper state at "probe" ioctl time.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v14 0/7] drm/i915/gvt: Dma-buf support for GVT-g

[Gerd Hoffmann]

On Fri, 2017-08-18 at 18:21 +0800, Tina Zhang wrote:
> v13->v14:
> 1) add PROBE, DMABUF and REGION flags. (Alex)
> 2) return -ENXIO when gem proxy object is banned by ioctl.
>    (Chris) (Daniel)
> 3) add some details about the float pixel format. (Daniel)
> 4) add F suffix to the defined name. (Daniel)
> 5) refine pixel format table. (Zhenyu)

Ok, patch series applies cleanly to 4.13-rc6.  The new flags are
working fine.

However, I see VFIO_DEVICE_QUERY_GFX_PLANE failures which I think
should not be there.  When the guest didn't define a plane yet I get
"No such device" errors instead of a plane_info struct with fields
(drm_format, width, height, size) set to zero.  I also see "Bad
address" errors now and then with no obvious cause.

Cursor support isn't working too.

I'm testing with "-display egl-headless -spice gl=off,port=...".  In
that configuration qemu will import the dma-bufs as textures and reads
them using ReadPixels for display.

qemu branch: https://www.kraxel.org/cgit/qemu/log/?h=work/intel-vgpu

The qemu branch has support for both dmabufs and regions.  The region-
based display code is totaly untested though.

cheers,
  Gerd

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

Re: [PATCH v14 0/7] drm/i915/gvt: Dma-buf support for GVT-g

[Wang, Zhi A]

Hi Gerd:
    Thanks for the testing. We will find out the problem and refresh the whole patch series.

Thanks,
Zhi.

-----Original Message-----
From: Gerd Hoffmann [mailto:kraxel@redhat.com] 
Sent: Friday, August 25, 2017 2:47 PM
To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Lv, Zhiyuan <zhiyuan.lv@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>; alex.williamson@redhat.com; chris@chris-wilson.co.uk; daniel@ffwll.ch; joonas.lahtinen@linux.intel.com; kwankhede@nvidia.com
Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
Subject: Re: [PATCH v14 0/7] drm/i915/gvt: Dma-buf support for GVT-g

On Fri, 2017-08-18 at 18:21 +0800, Tina Zhang wrote:
> v13->v14:
> 1) add PROBE, DMABUF and REGION flags. (Alex)
> 2) return -ENXIO when gem proxy object is banned by ioctl.
>    (Chris) (Daniel)
> 3) add some details about the float pixel format. (Daniel)
> 4) add F suffix to the defined name. (Daniel)
> 5) refine pixel format table. (Zhenyu)

Ok, patch series applies cleanly to 4.13-rc6.  The new flags are working fine.

However, I see VFIO_DEVICE_QUERY_GFX_PLANE failures which I think should not be there.  When the guest didn't define a plane yet I get "No such device" errors instead of a plane_info struct with fields (drm_format, width, height, size) set to zero.  I also see "Bad address" errors now and then with no obvious cause.

Cursor support isn't working too.

I'm testing with "-display egl-headless -spice gl=off,port=...".  In that configuration qemu will import the dma-bufs as textures and reads them using ReadPixels for display.

qemu branch: https://www.kraxel.org/cgit/qemu/log/?h=work/intel-vgpu

The qemu branch has support for both dmabufs and regions.  The region- based display code is totaly untested though.

cheers,
  Gerd

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

Re: [PATCH v14 0/7] drm/i915/gvt: Dma-buf support for GVT-g

[Zhang, Tina]

Thanks Zhi and Gerd for testing v14 patch-set.

> -----Original Message-----
> From: Wang, Zhi A
> Sent: Friday, August 25, 2017 8:53 PM
> To: Gerd Hoffmann <kraxel@redhat.com>; Zhang, Tina
> <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Lv, Zhiyuan
> <zhiyuan.lv@intel.com>; Tian, Kevin <kevin.tian@intel.com>;
> alex.williamson@redhat.com; chris@chris-wilson.co.uk; daniel@ffwll.ch;
> joonas.lahtinen@linux.intel.com; kwankhede@nvidia.com
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: RE: [PATCH v14 0/7] drm/i915/gvt: Dma-buf support for GVT-g
> 
> Hi Gerd:
>     Thanks for the testing. We will find out the problem and refresh the whole
> patch series.
> 
> Thanks,
> Zhi.
> 
> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Friday, August 25, 2017 2:47 PM
> To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Lv, Zhiyuan
> <zhiyuan.lv@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, Kevin
> <kevin.tian@intel.com>; alex.williamson@redhat.com; chris@chris-
> wilson.co.uk; daniel@ffwll.ch; joonas.lahtinen@linux.intel.com;
> kwankhede@nvidia.com
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [PATCH v14 0/7] drm/i915/gvt: Dma-buf support for GVT-g
> 
> On Fri, 2017-08-18 at 18:21 +0800, Tina Zhang wrote:
> > v13->v14:
> > 1) add PROBE, DMABUF and REGION flags. (Alex)
> > 2) return -ENXIO when gem proxy object is banned by ioctl.
> >    (Chris) (Daniel)
> > 3) add some details about the float pixel format. (Daniel)
> > 4) add F suffix to the defined name. (Daniel)
> > 5) refine pixel format table. (Zhenyu)
> 
> Ok, patch series applies cleanly to 4.13-rc6.  The new flags are working fine.
> 
> However, I see VFIO_DEVICE_QUERY_GFX_PLANE failures which I think should
> not be there.  When the guest didn't define a plane yet I get "No such device"
> errors instead of a plane_info struct with fields (drm_format, width, height, size)
> set to zero.  I also see "Bad address" errors now and then with no obvious cause.
The idea is to return "No such device" error with fields set to zero.
There are two cases, in which the "No such device" error is returned: one is the guest IGD driver
has not finished the initialization and the other is the plane is disabled by guest IGD driver.
If we prefer to return success in these two situations with fields set to zero, I can add it in the
next version.

We didn't meet the "Bad address" errors before. I will try your qemu repo to see whether I can
meet the issue. Thanks.
 
> Cursor support isn't working too.
It seems there is some issue in i915 of version 4.13-rc6, which blocks the cursor on native platform.
I just tried the rc7 (the latest staging), there is on such issue.
Thanks.

Tina
> 
> I'm testing with "-display egl-headless -spice gl=off,port=...".  In that
> configuration qemu will import the dma-bufs as textures and reads them using
> ReadPixels for display.
> 
> qemu branch: https://www.kraxel.org/cgit/qemu/log/?h=work/intel-vgpu
> 
> The qemu branch has support for both dmabufs and regions.  The region- based
> display code is totaly untested though.
> 
> cheers,
>   Gerd

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

Re: [PATCH v14 0/7] drm/i915/gvt: Dma-buf support for GVT-g

[Gerd Hoffmann]

  Hi,

> > However, I see VFIO_DEVICE_QUERY_GFX_PLANE failures which I think
> > should
> > not be there.  When the guest didn't define a plane yet I get "No
> > such device"
> > errors instead of a plane_info struct with fields (drm_format,
> > width, height, size)
> > set to zero.  I also see "Bad address" errors now and then with no
> > obvious cause.
> 
> The idea is to return "No such device" error with fields set to zero.
> There are two cases, in which the "No such device" error is returned:
> one is the guest IGD driver
> has not finished the initialization and the other is the plane is
> disabled by guest IGD driver.
> If we prefer to return success in these two situations with fields
> set to zero, I can add it in the
> next version.

Yes, success should be returned in those cases.  Querying the plane
information worked and the struct is filled by the driver.  Userspace
can figure that there is no plane defined by the guest by looking at
the struct fields.

> > Cursor support isn't working too.
> 
> It seems there is some issue in i915 of version 4.13-rc6, which
> blocks the cursor on native platform.
> I just tried the rc7 (the latest staging), there is on such issue.
> Thanks.

OK, I'll go re-test with the just-released 4.13 kernel then.

thanks,
  Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Gerd Hoffmann]

  Hi,

[ bringing a private discussion back to the list ]

> The dma-buf's life cycle is handled by user mode and tracked by
> kernel.
> The returned fd in struct vfio_device_query_gfx_plane can be a new
> fd or an old fd of a re-exported dma-buf. Host user mode can check
> the
> value of fd and to see if it needs to create new resource according
> to
> the new fd or just use the existed resource related to the old fd.

Ok, this idea has a fundamental flaw:  The life cycle of the dma-buf
and the file handle are not identical.  The dma-buf can exist longer
than the file handle in case other references to the dma-buf exist.  So
when trying to use the file handle as identifier for the dma-buf you'll
end up with all sorts of strange effects.

So, I'd suggest to use a id instead, and add a ioctl to get a dmabuf
for a given id (incremental patch):

--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -538,12 +538,22 @@ struct vfio_device_gfx_plane_info {
        __u32 y_pos;    /* vertical position of cursor plane, upper
left corner in lines*/
        union {
                __u32 region_index;     /* region index */
-               __s32 fd;       /* dma-buf fd */
+               __u32 dmabuf_id;        /* dma-buf id */
        };
 };
 
 #define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14)
 
+struct vfio_device_gfx_dmabuf_fd {
+       __u32 argsz;
+       __u32 flags;
+       /* in */
+       __u32 dmabuf_id;
+       /* out */
+       __s32 dmabuf_fd;
+};
+
+#define VFIO_DEVICE_GET_GFX_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 15)
 
 /* -------- API for Type1 VFIO IOMMU -------- */

[ no changes for a region-based display ]

git branch, kernel, with updated dmabuf patch:
  https://www.kraxel.org/cgit/linux/log/?h=gvt-dmabuf-v14

qemu branch:
  https://www.kraxel.org/cgit/qemu/log/?h=work/intel-vgpu

cheers,
  Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

It's really tricky to handle the cached dmabuf_obj's life-cycle without touching other kernel modules (e.g. i915 or dmabuf).The proposed two ioctls will be helpful.

So, there is a problem about the releasing cached dmabuf_obj. We cannot rely on the drm_i915_gem_object_ops.release() to release the cached dmabuf_obj,
as this release operation is running in another thread, which leads to a racing condition and tricky to be solved without touching other modules.

So, in order to solve that kind of problem, I’d like to add one more ioctl, which is used for user mode to close the dmabuf_obj. 

Including the proposed two ioctls,  the incremental patch is like this:

diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index bf40f7b..6aa6860 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -538,12 +538,33 @@ struct vfio_device_gfx_plane_info {
        __u32 y_pos;    /* vertical position of cursor plane, upper left corner in lines*/
        union {
                __u32 region_index;     /* region index */
-               __s32 fd;       /* dma-buf fd */
+               __s32 dmabuf_id;        /* dma-buf fd */
        };
 };

 #define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14)

+struct vfio_device_gfx_dmabuf_fd {
+         __u32 argsz;
+         __u32 flags;
+         /* in */
+        __u32 dmabuf_id;
+        /* out */
+        __s32 dmabuf_fd;
+};
+
+#define VFIO_DEVICE_GET_GFX_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 15)
+
+
+struct vfio_device_gfx_buffer {
+        __u32 argsz;
+        __u32 flags;
+        /* in */
+        __u32 id;
+};
+
+#define VFIO_DEVICE_CLOSE_BUF _IO(VFIO_TYPE, VFIO_BASE + 16)

And here are some details:

1. VFIO_DEVICE_QUERY_GFX_PLANE: is used for user mode to ask kernel part to create a buffer (in dmabuf case is dmabuf_obj in kernel) and return the buffer info.

2. VFIO_DEVICE_GET_GFX_DMABUF: is used for user mode to ask kernel part which interface it likes the buffer to be wrapped.
Actually, I think there could be a bunch of types, including DMBUF type.
So, maybe we can change the IOCTL's name to some generic name and use flags field to distinguish them.

In dmabuf case, a new dmabuf will be created each time with this ioctl invoked, and installed with a new fd.
The dmabuf is just a wrapper of kernel dmabuf_obj distinguished by dmabuf_id. So there could be more
than one dmabuf as the wrappers of the same dmabuf_obj, if this ioctl was invoked with the same dmabuf_id
many times. The dmabuf and its fd is fully controlled by user mode. And the VFIO_DEVICE_CLOSE_BUF can
guarantee the referenced dmabuf_obj will be released at last, after all the dmabufs are released.

3. VFIO_DEVICE_CLOSE_BUF: is used for user mode to tell kernel part to release that buffer.
In dmabuf case, this ioctl won't release the dmabuf_obj at once. It just decreases the reference count of the dmabuf_obj
and make sure this dmabuf_obj won't be reused through VFIO_DEVICE_QUERY_GFX_PLANE again. At last, after all the
referencing dmabuf are released by user mode, the dmabuf_obj will be released by kernel.

Thanks,

BR,
Tina

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Tuesday, September 26, 2017 3:12 PM
> To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org; Alex
> Williamson <alex.williamson@redhat.com>; Daniel Vetter
> <daniel.vetter@ffwll.ch>
> Subject: Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation
> 
>   Hi,
> 
> [ bringing a private discussion back to the list ]
> 
> > The dma-buf's life cycle is handled by user mode and tracked by
> > kernel.
> > The returned fd in struct vfio_device_query_gfx_plane can be a new fd
> > or an old fd of a re-exported dma-buf. Host user mode can check the
> > value of fd and to see if it needs to create new resource according to
> > the new fd or just use the existed resource related to the old fd.
> 
> Ok, this idea has a fundamental flaw:  The life cycle of the dma-buf and the file
> handle are not identical.  The dma-buf can exist longer than the file handle in
> case other references to the dma-buf exist.  So when trying to use the file handle
> as identifier for the dma-buf you'll end up with all sorts of strange effects.
> 
> So, I'd suggest to use a id instead, and add a ioctl to get a dmabuf for a given id
> (incremental patch):
> 
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -538,12 +538,22 @@ struct vfio_device_gfx_plane_info {
>         __u32 y_pos;    /* vertical position of cursor plane, upper left corner in
> lines*/
>         union {
>                 __u32 region_index;     /* region index */
> -               __s32 fd;       /* dma-buf fd */
> +               __u32 dmabuf_id;        /* dma-buf id */
>         };
>  };
> 
>  #define VFIO_DEVICE_QUERY_GFX_PLANE _IO(VFIO_TYPE, VFIO_BASE + 14)
> 
> +struct vfio_device_gfx_dmabuf_fd {
> +       __u32 argsz;
> +       __u32 flags;
> +       /* in */
> +       __u32 dmabuf_id;
> +       /* out */
> +       __s32 dmabuf_fd;
> +};
> +
> +#define VFIO_DEVICE_GET_GFX_DMABUF _IO(VFIO_TYPE, VFIO_BASE + 15)
> 
>  /* -------- API for Type1 VFIO IOMMU -------- */
> 
> [ no changes for a region-based display ]
> 
> git branch, kernel, with updated dmabuf patch:
>   https://www.kraxel.org/cgit/linux/log/?h=gvt-dmabuf-v14
> 
> qemu branch:
>   https://www.kraxel.org/cgit/qemu/log/?h=work/intel-vgpu
> 
> cheers,
>   Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Gerd Hoffmann]

  Hi,

> So, there is a problem about the releasing cached dmabuf_obj. We
> cannot rely on the drm_i915_gem_object_ops.release() to release the
> cached dmabuf_obj,
> as this release operation is running in another thread, which leads
> to a racing condition and tricky to be solved without touching other
> modules.

PLANE_INFO just creates a intel_vgpu_dmabuf_obj.

GET_DMABUF creates a fresh proxy gem object and dmabuf.

proxy gem object references intel_vgpu_dmabuf_obj but not the other way
around.  Then you can simply refcount intel_vgpu_dmabuf_obj and be done
with it.

https://www.kraxel.org/cgit/linux/commit/?h=gvt-dmabuf-v14&id=350a0e834
971e6f53d7235d8b6167bed4dccf074

Note: Patch renamed intel_vgpu_dmabuf_obj to intel_vgpu_fb_obj, because
it doesn't refer to dmabufs any more.  It basically carries the guest
plane/framebuffer information and the ID associated with it.

> So, in order to solve that kind of problem, I’d like to add one more
> ioctl, which is used for user mode to close the dmabuf_obj. 

Depending on userspace notifying the kernel for that kind of cleanups
is a bad idea.  What happens in case userspace crashes?  Do you leak
dmabufs then?

cheers,
  Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

Thanks for the patch. Actually, I did the same thing in my local repo and also, I have a patch for the local Qemu repo to test it. I will send them out later.

The reason why I want to propose the close IOCTL is because that the current lock (fb_obj_list_lock), cannot sync the intel_vgpu_fb_info releasing and reusing.
You see, the intel_vgpu_fb_info reusing and releasing are in different threads. There is a case that intel_vgpu_find_dmabuf can return a intel_vgpu_fb_obj, while the intel_vgpu_fb_obj
is on the way to be released. That's the problem.

The invoker of the close IOCTL is only Qemu. So, if the Qemu crashes, the whole vGPU's resource is going to be released. We can handle our dmabuf_obj to be released there.

Thanks.

BR,
Tina

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Gerd Hoffmann
> Sent: Wednesday, September 27, 2017 6:11 PM
> To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Wang, Zhi
> A <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org;
> intel-gvt-dev@lists.freedesktop.org; Alex Williamson
> <alex.williamson@redhat.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
> Subject: Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation
> 
>   Hi,
> 
> > So, there is a problem about the releasing cached dmabuf_obj. We
> > cannot rely on the drm_i915_gem_object_ops.release() to release the
> > cached dmabuf_obj, as this release operation is running in another
> > thread, which leads to a racing condition and tricky to be solved
> > without touching other modules.
> 
> PLANE_INFO just creates a intel_vgpu_dmabuf_obj.
> 
> GET_DMABUF creates a fresh proxy gem object and dmabuf.
> 
> proxy gem object references intel_vgpu_dmabuf_obj but not the other way
> around.  Then you can simply refcount intel_vgpu_dmabuf_obj and be done
> with it.
> 
> https://www.kraxel.org/cgit/linux/commit/?h=gvt-dmabuf-v14&id=350a0e834
> 971e6f53d7235d8b6167bed4dccf074
> 
> Note: Patch renamed intel_vgpu_dmabuf_obj to intel_vgpu_fb_obj, because it
> doesn't refer to dmabufs any more.  It basically carries the guest
> plane/framebuffer information and the ID associated with it.
> 
> > So, in order to solve that kind of problem, I’d like to add one more
> > ioctl, which is used for user mode to close the dmabuf_obj.
> 
> Depending on userspace notifying the kernel for that kind of cleanups is a bad
> idea.  What happens in case userspace crashes?  Do you leak dmabufs then?
> 
> cheers,
>   Gerd
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

So, there won't be dmabuf leaking problem, as we release all the dmabuf_obj in the release ops when user space crashing.

Can we just stop considering the way to fix the dmabuf life-cycle issue and try to just consider the generic way to handle buffer exposing?
Does the generic way need the close ioctl?
In my opinion, it's like to build up a producer-consumer way to expose the buffer:

	                        Create buffer and return its info
Mdev devices  -----------------------------------------------------> User space
                           <----------------------------------------------------- 
(producer)                              Close it                                                 (consumer)

Alex and Gerd, can you share your thoughts?
Thanks.


BR,
Tina

> -----Original Message-----
> From: Zhang, Tina
> Sent: Friday, September 29, 2017 7:43 AM
> To: 'Gerd Hoffmann' <kraxel@redhat.com>; zhenyuw@linux.intel.com; Wang,
> Zhi A <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org;
> intel-gvt-dev@lists.freedesktop.org; Alex Williamson
> <alex.williamson@redhat.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
> Subject: RE: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation
> 
> Thanks for the patch. Actually, I did the same thing in my local repo and also, I
> have a patch for the local Qemu repo to test it. I will send them out later.
> 
> The reason why I want to propose the close IOCTL is because that the current
> lock (fb_obj_list_lock), cannot sync the intel_vgpu_fb_info releasing and
> reusing.
> You see, the intel_vgpu_fb_info reusing and releasing are in different threads.
> There is a case that intel_vgpu_find_dmabuf can return a intel_vgpu_fb_obj,
> while the intel_vgpu_fb_obj is on the way to be released. That's the problem.
> 
> The invoker of the close IOCTL is only Qemu. So, if the Qemu crashes, the whole
> vGPU's resource is going to be released. We can handle our dmabuf_obj to be
> released there.
> 
> Thanks.
> 
> BR,
> Tina
> 
> > -----Original Message-----
> > From: intel-gvt-dev
> > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of Gerd
> > Hoffmann
> > Sent: Wednesday, September 27, 2017 6:11 PM
> > To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Wang,
> > Zhi A <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>;
> > intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org;
> > Alex Williamson <alex.williamson@redhat.com>; Lv, Zhiyuan
> > <zhiyuan.lv@intel.com>
> > Subject: Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf
> > operation
> >
> >   Hi,
> >
> > > So, there is a problem about the releasing cached dmabuf_obj. We
> > > cannot rely on the drm_i915_gem_object_ops.release() to release the
> > > cached dmabuf_obj, as this release operation is running in another
> > > thread, which leads to a racing condition and tricky to be solved
> > > without touching other modules.
> >
> > PLANE_INFO just creates a intel_vgpu_dmabuf_obj.
> >
> > GET_DMABUF creates a fresh proxy gem object and dmabuf.
> >
> > proxy gem object references intel_vgpu_dmabuf_obj but not the other
> > way around.  Then you can simply refcount intel_vgpu_dmabuf_obj and be
> > done with it.
> >
> > https://www.kraxel.org/cgit/linux/commit/?h=gvt-dmabuf-
> v14&id=350a0e83
> > 4
> > 971e6f53d7235d8b6167bed4dccf074
> >
> > Note: Patch renamed intel_vgpu_dmabuf_obj to intel_vgpu_fb_obj,
> > because it doesn't refer to dmabufs any more.  It basically carries
> > the guest plane/framebuffer information and the ID associated with it.
> >
> > > So, in order to solve that kind of problem, I’d like to add one more
> > > ioctl, which is used for user mode to close the dmabuf_obj.
> >
> > Depending on userspace notifying the kernel for that kind of cleanups
> > is a bad idea.  What happens in case userspace crashes?  Do you leak dmabufs
> then?
> >
> > cheers,
> >   Gerd
> >
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

  Hi,

> The reason why I want to propose the close IOCTL is because that the
> current lock (fb_obj_list_lock), cannot sync the intel_vgpu_fb_info
> releasing and reusing.
> You see, the intel_vgpu_fb_info reusing and releasing are in
> different threads. There is a case that intel_vgpu_find_dmabuf can
> return a intel_vgpu_fb_obj, while the intel_vgpu_fb_obj
> is on the way to be released. That's the problem.

Oh, right.  But that race is fixable.  We need to move the locks one
level up, so we don't only cover list operations (add/lookup/delete)
but also  the kref_{get,put} operations for the list elements.

Patch against my tree, only build-tested so far.

cheers,
  Gerd

[-- Attachment #2: Type: text/x-patch, Size: 3707 bytes --]

>From 3e8c30a857d98d36357e8d9bb04b7ccb72264543 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Fri, 29 Sep 2017 08:59:34 +0200
Subject: [PATCH] fix locking

---
 drivers/gpu/drm/i915/gvt/dmabuf.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/dmabuf.c b/drivers/gpu/drm/i915/gvt/dmabuf.c
index 2fb3247eff..06ff7bb04e 100644
--- a/drivers/gpu/drm/i915/gvt/dmabuf.c
+++ b/drivers/gpu/drm/i915/gvt/dmabuf.c
@@ -84,24 +84,25 @@ static void intel_vgpu_fb_obj_release(struct kref *kref)
 {
 	struct intel_vgpu_fb_obj *fb_obj =
 		container_of(kref, struct intel_vgpu_fb_obj, kref);
-	struct intel_vgpu *vgpu;
 
-	vgpu = fb_obj->vgpu;
-	mutex_lock(&vgpu->fb_obj_list_lock);
 	list_del(&fb_obj->list);
-	mutex_unlock(&vgpu->fb_obj_list_lock);
 	kfree(fb_obj);
 }
 
 static void intel_vgpu_gem_release(struct drm_i915_gem_object *obj)
 {
+	struct intel_vgpu *vgpu;
+
 	if (WARN_ON(!obj->gvt || !obj->gvt->vgpu)) {
 		gvt_err("gvt info is invalid\n");
 		return;
 	}
 
-	intel_gvt_hypervisor_put_vfio_device(obj->gvt->vgpu);
+	vgpu = obj->gvt->vgpu;
+	intel_gvt_hypervisor_put_vfio_device(vgpu);
+	mutex_lock(&vgpu->fb_obj_list_lock);
 	kref_put(&obj->gvt->kref, intel_vgpu_fb_obj_release);
+	mutex_unlock(&vgpu->fb_obj_list_lock);
 	obj->gvt = NULL;
 }
 
@@ -239,7 +240,6 @@ intel_vgpu_pick_exposed_dmabuf(struct intel_vgpu *vgpu,
 	struct list_head *pos;
 	struct intel_vgpu_fb_obj *fb_obj;
 
-	mutex_lock(&vgpu->fb_obj_list_lock);
 	list_for_each(pos, &vgpu->fb_obj_list_head) {
 		fb_obj = container_of(pos, struct intel_vgpu_fb_obj,
 					  list);
@@ -251,11 +251,9 @@ intel_vgpu_pick_exposed_dmabuf(struct intel_vgpu *vgpu,
 		    (fb_obj->fb.width == latest_info->width) &&
 		    (fb_obj->fb.height == latest_info->height) &&
 		    (fb_obj->fb.stride == latest_info->stride)) {
-			mutex_unlock(&vgpu->fb_obj_list_lock);
 			return fb_obj;
 		}
 	}
-	mutex_unlock(&vgpu->fb_obj_list_lock);
 	return NULL;
 }
 
@@ -265,16 +263,13 @@ intel_vgpu_find_dmabuf(struct intel_vgpu *vgpu, u32 dmabuf_id)
 	struct list_head *pos;
 	struct intel_vgpu_fb_obj *fb_obj;
 
-	mutex_lock(&vgpu->fb_obj_list_lock);
 	list_for_each(pos, &vgpu->fb_obj_list_head) {
 		fb_obj = container_of(pos, struct intel_vgpu_fb_obj,
 					  list);
 		if (fb_obj->dmabuf_id == dmabuf_id) {
-			mutex_unlock(&vgpu->fb_obj_list_lock);
 			return fb_obj;
 		}
 	}
-	mutex_unlock(&vgpu->fb_obj_list_lock);
 	return NULL;
 }
 
@@ -327,8 +322,10 @@ int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args)
 		return ret;
 
 	/* If exists, pick up the exposed dmabuf fd */
+	mutex_lock(&vgpu->fb_obj_list_lock);
 	fb_obj = intel_vgpu_pick_exposed_dmabuf(vgpu, &fb_info);
 	if (fb_obj != NULL) {
+		mutex_unlock(&vgpu->fb_obj_list_lock);
 		update_fb_info(gvt_dmabuf, fb_obj);
 		return 0;
 	}
@@ -345,7 +342,6 @@ int intel_vgpu_query_plane(struct intel_vgpu *vgpu, void *args)
 	fb_obj->fb = fb_info;
 	fb_obj->dmabuf_id = id++;
 
-	mutex_lock(&vgpu->fb_obj_list_lock);
 	list_add_tail(&fb_obj->list, &vgpu->fb_obj_list_head);
 	mutex_unlock(&vgpu->fb_obj_list_lock);
 	update_fb_info(gvt_dmabuf, fb_obj);
@@ -362,11 +358,15 @@ int intel_vgpu_get_dmabuf(struct intel_vgpu *vgpu, void *args)
 	struct dma_buf *dmabuf;
 	int ret;
 
+	mutex_lock(&vgpu->fb_obj_list_lock);
 	fb_obj = intel_vgpu_find_dmabuf(vgpu, gvt_dmabuf->dmabuf_id);
-	if (NULL == fb_obj)
+	if (NULL == fb_obj) {
+		mutex_unlock(&vgpu->fb_obj_list_lock);
 		return -EINVAL;
+	}
 
 	obj = intel_vgpu_create_gem(dev, fb_obj);
+	mutex_unlock(&vgpu->fb_obj_list_lock);
 	if (obj == NULL) {
 		gvt_vgpu_err("create gvt gem obj failed:%d\n", vgpu->id);
 		return -ENOMEM;
-- 
2.9.3


[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Gerd Hoffmann]

On Fri, 2017-09-29 at 07:04 +0000, Zhang, Tina wrote:
> So, there won't be dmabuf leaking problem, as we release all the
> dmabuf_obj in the release ops when user space crashing.
> 
> Can we just stop considering the way to fix the dmabuf life-cycle
> issue and try to just consider the generic way to handle buffer
> exposing?

Can you describe in more detail what you have in mind?

> Does the generic way need the close ioctl?

I think we don't need a close ioctl anyway.

cheers,
  Gerd


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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Gerd Hoffmann]

  Hi,

> +	__u32 x_pos;	/* horizontal position of cursor plane,
> upper left corner in pixels */
> +	__u32 y_pos;	/* vertical position of cursor plane,
> upper left corner in lines*/

Completely separate question:  Does the host know the cursor hotspot?

cheers,
  Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Friday, September 29, 2017 3:29 PM
> To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Wang, Zhi
> A <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>; Alex
> Williamson <alex.williamson@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org;
> intel-gvt-dev@lists.freedesktop.org; Lv, Zhiyuan <zhiyuan.lv@intel.com>
> Subject: Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation
> 
> On Fri, 2017-09-29 at 07:04 +0000, Zhang, Tina wrote:
> > So, there won't be dmabuf leaking problem, as we release all the
> > dmabuf_obj in the release ops when user space crashing.
> >
> > Can we just stop considering the way to fix the dmabuf life-cycle
> > issue and try to just consider the generic way to handle buffer
> > exposing?
> 
> Can you describe in more detail what you have in mind?
> 
> > Does the generic way need the close ioctl?
> 
> I think we don't need a close ioctl anyway.
Can you share your thoughts?
Do you think the fd interface is enough for all kinds of buffer exposed by Mdev?
Thanks.

BR,
Tina

> 
> cheers,
>   Gerd
> 

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Gerd Hoffmann]

  Hi,

> > > Does the generic way need the close ioctl?
> > 
> > I think we don't need a close ioctl anyway.
> 
> Can you share your thoughts?

See other mail.  I think the race can be fixed by changing the locking,
so a explicit close ioctl isn't needed.

> Do you think the fd interface is enough for all kinds of buffer
> exposed by Mdev?

What kind of buffers do you have in mind which might not be covered?

cheers,
  Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Friday, September 29, 2017 4:03 PM
> To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Wang, Zhi
> A <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>; Alex
> Williamson <alex.williamson@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org;
> intel-gvt-dev@lists.freedesktop.org; Lv, Zhiyuan <zhiyuan.lv@intel.com>
> Subject: Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation
> 
>   Hi,
> 
> > > > Does the generic way need the close ioctl?
> > >
> > > I think we don't need a close ioctl anyway.
> >
> > Can you share your thoughts?
> 
> See other mail.  I think the race can be fixed by changing the locking, so a explicit
> close ioctl isn't needed.
Yeah, I understand your idea. But unfortunately, it cannot solve the current issue.
There will still be a racing condition between releasing dmabuf_obj and reusing it.
For example, if the old reused dmabuf_obj is released just after query ioctl return it,  the next get_fd ioctl would
return error as the dmabuf_obj has already been closed.

> 
> > Do you think the fd interface is enough for all kinds of buffer
> > exposed by Mdev?
> 
> What kind of buffers do you have in mind which might not be covered?
I thinking about the case that would like to postpone the buffers releasing operation, after user space has closed all the fd.
Later these buffers may be used to expose to other kinds of fd to user space.
Thanks.

BR,
Tina
> 
> cheers,
>   Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Gerd Hoffmann]

  Hi,

> For example, if the old reused dmabuf_obj is released just after
> query ioctl return it,  the next get_fd ioctl would
> return error as the dmabuf_obj has already been closed.

My branch already grabs an extra reference when creating a new
dmabuf_obj, which will be dropped on GET_DMABUF ioctl, exactly to avoid
the dmabuf_obj disappear between QUERY_PLANE and GET_DMABUF ioctls.

Can easily be extended to handle the reuse case too.

https://www.kraxel.org/cgit/linux/commit/?h=gvt-dmabuf-v14&id=9959109ae
52cf15e119715a6b7de080fb849e3d2

While being at it also cleanup properly on close (so we don't leak
structs in case userspace never calls GET_DMABUF for a plane).

https://www.kraxel.org/cgit/linux/commit/?h=gvt-dmabuf-v14&id=c0b0c407e
33904e749dec1ef44ec01099c16d39f

> > > Do you think the fd interface is enough for all kinds of buffer
> > > exposed by Mdev?
> > 
> > What kind of buffers do you have in mind which might not be
> > covered?
> 
> I thinking about the case that would like to postpone the buffers
> releasing operation, after user space has closed all the fd.

Work fine.  qemu can import the dma-buf as opengl texture, which
creates a extra reference.  Then close the fd.  dma-buf continues to
exist as long as the texture referencing it exists.

> Later these buffers may be used to expose to other kinds of fd to
> user space.

Sorry, I don't understand that sentence.

cheers,
  Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Friday, September 29, 2017 6:21 PM
> To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Wang, Zhi
> A <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>; Alex
> Williamson <alex.williamson@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org;
> intel-gvt-dev@lists.freedesktop.org; Lv, Zhiyuan <zhiyuan.lv@intel.com>
> Subject: Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation
> 
>   Hi,
> 
> > For example, if the old reused dmabuf_obj is released just after query
> > ioctl return it,  the next get_fd ioctl would return error as the
> > dmabuf_obj has already been closed.
> 
> My branch already grabs an extra reference when creating a new dmabuf_obj,
> which will be dropped on GET_DMABUF ioctl, exactly to avoid the dmabuf_obj
> disappear between QUERY_PLANE and GET_DMABUF ioctls.
Yeah, that could solve the problem. But I'm not sure if it could be acceptable.
Zhenyu, can you share your comments?
Thanks.

BR,
Tina

> 
> Can easily be extended to handle the reuse case too.
> 
> https://www.kraxel.org/cgit/linux/commit/?h=gvt-dmabuf-v14&id=9959109ae
> 52cf15e119715a6b7de080fb849e3d2
> 
> While being at it also cleanup properly on close (so we don't leak structs in case
> userspace never calls GET_DMABUF for a plane).
> 
> https://www.kraxel.org/cgit/linux/commit/?h=gvt-dmabuf-v14&id=c0b0c407e
> 33904e749dec1ef44ec01099c16d39f
> 
> > > > Do you think the fd interface is enough for all kinds of buffer
> > > > exposed by Mdev?
> > >
> > > What kind of buffers do you have in mind which might not be covered?
> >
> > I thinking about the case that would like to postpone the buffers
> > releasing operation, after user space has closed all the fd.
> 
> Work fine.  qemu can import the dma-buf as opengl texture, which creates a
> extra reference.  Then close the fd.  dma-buf continues to exist as long as the
> texture referencing it exists.
> 
> > Later these buffers may be used to expose to other kinds of fd to user
> > space.
> 
> Sorry, I don't understand that sentence.
> 
> cheers,
>   Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Gerd Hoffmann]

  Hi,

> Yeah, that could solve the problem. But I'm not sure if it could be
> acceptable.
> Zhenyu, can you share your comments?

Ping?  Any progress here?  We are at 4.14-rc3 already.  v15 is needed
really soon now otherwise the 4.15 merge window will be missed.

cheers,
  Gerd

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

Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

> -----Original Message-----
> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Friday, October 6, 2017 8:13 PM
> To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Wang, Zhi
> A <zhi.a.wang@intel.com>; Tian, Kevin <kevin.tian@intel.com>; Alex
> Williamson <alex.williamson@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org;
> intel-gvt-dev@lists.freedesktop.org; Lv, Zhiyuan <zhiyuan.lv@intel.com>
> Subject: Re: [PATCH v14 5/7] vfio: ABI for mdev display dma-buf operation
> 
>   Hi,
> 
> > Yeah, that could solve the problem. But I'm not sure if it could be
> > acceptable.
> > Zhenyu, can you share your comments?
> 
> Ping?  Any progress here?  We are at 4.14-rc3 already.  v15 is needed really
> soon now otherwise the 4.15 merge window will be missed.
V15 will be submitted next week, after discussion with maintainers.
Thanks.

BR,
Tina
> 
> cheers,
>   Gerd

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