diff mbox

[3/9] drm/i915: gvt: Introduce the basic architecture of GVT-g

Message ID 1463473149-5876-4-git-send-email-zhi.a.wang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wang, Zhi A May 17, 2016, 8:19 a.m. UTC
This patch introduces the very basic framework of GVT-g device model,
includes basic prototypes, definitions, initialization.

v5:
Take Tvrtko's comments:
- Fix the misspelled words in Kconfg
- Let functions take drm_i915_private * instead of struct drm_device *
- Remove redundant prints/local varible initialization

v3:
Take Joonas' comments:
- Change file name i915_gvt.* to intel_gvt.*
- Move GVT kernel parameter into intel_gvt.c
- Remove redundant debug macros
- Change error handling style
- Add introductions for some stub functions
- Introduce drm/i915_gvt.h.

Take Kevin's comments:
- Move GVT-g host/guest check into intel_vgt_balloon in i915_gem_gtt.c

v2:
- Introduce i915_gvt.c.
It's necessary to introduce the stubs between i915 driver and GVT-g host,
as GVT-g components is configurable in kernel config. When disabled, the
stubs here do nothing.

Take Joonas' comments:
- Replace boolean return value with int.
- Replace customized info/warn/debug macros with DRM macros.
- Document all non-static functions like i915.
- Remove empty and unused functions.
- Replace magic number with marcos.
- Set GVT-g in kernel config to "n" by default.

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/Kconfig         |  15 +++
 drivers/gpu/drm/i915/Makefile        |   5 +
 drivers/gpu/drm/i915/gvt/Makefile    |   5 +
 drivers/gpu/drm/i915/gvt/debug.h     |  36 ++++++
 drivers/gpu/drm/i915/gvt/gvt.c       | 205 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h       |  84 ++++++++++++++
 drivers/gpu/drm/i915/gvt/hypercall.h |  38 +++++++
 drivers/gpu/drm/i915/gvt/mpt.h       |  49 +++++++++
 drivers/gpu/drm/i915/i915_dma.c      |  17 ++-
 drivers/gpu/drm/i915/i915_drv.h      |  12 ++
 drivers/gpu/drm/i915/intel_gvt.c     |  98 +++++++++++++++++
 drivers/gpu/drm/i915/intel_gvt.h     |  49 +++++++++
 include/drm/i915_gvt.h               |  31 ++++++
 13 files changed, 640 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
 create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
 create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
 create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.c
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.h
 create mode 100644 include/drm/i915_gvt.h

Comments

Tvrtko Ursulin May 18, 2016, 11:22 a.m. UTC | #1
On 17/05/16 09:19, Zhi Wang wrote:
> This patch introduces the very basic framework of GVT-g device model,
> includes basic prototypes, definitions, initialization.
>
> v5:
> Take Tvrtko's comments:
> - Fix the misspelled words in Kconfg
> - Let functions take drm_i915_private * instead of struct drm_device *
> - Remove redundant prints/local varible initialization
>
> v3:
> Take Joonas' comments:
> - Change file name i915_gvt.* to intel_gvt.*
> - Move GVT kernel parameter into intel_gvt.c
> - Remove redundant debug macros
> - Change error handling style
> - Add introductions for some stub functions
> - Introduce drm/i915_gvt.h.
>
> Take Kevin's comments:
> - Move GVT-g host/guest check into intel_vgt_balloon in i915_gem_gtt.c
>
> v2:
> - Introduce i915_gvt.c.
> It's necessary to introduce the stubs between i915 driver and GVT-g host,
> as GVT-g components is configurable in kernel config. When disabled, the
> stubs here do nothing.
>
> Take Joonas' comments:
> - Replace boolean return value with int.
> - Replace customized info/warn/debug macros with DRM macros.
> - Document all non-static functions like i915.
> - Remove empty and unused functions.
> - Replace magic number with marcos.
> - Set GVT-g in kernel config to "n" by default.
>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>   drivers/gpu/drm/i915/Kconfig         |  15 +++
>   drivers/gpu/drm/i915/Makefile        |   5 +
>   drivers/gpu/drm/i915/gvt/Makefile    |   5 +
>   drivers/gpu/drm/i915/gvt/debug.h     |  36 ++++++
>   drivers/gpu/drm/i915/gvt/gvt.c       | 205 +++++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/i915/gvt/gvt.h       |  84 ++++++++++++++
>   drivers/gpu/drm/i915/gvt/hypercall.h |  38 +++++++
>   drivers/gpu/drm/i915/gvt/mpt.h       |  49 +++++++++
>   drivers/gpu/drm/i915/i915_dma.c      |  17 ++-
>   drivers/gpu/drm/i915/i915_drv.h      |  12 ++
>   drivers/gpu/drm/i915/intel_gvt.c     |  98 +++++++++++++++++
>   drivers/gpu/drm/i915/intel_gvt.h     |  49 +++++++++
>   include/drm/i915_gvt.h               |  31 ++++++
>   13 files changed, 640 insertions(+), 4 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
>   create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
>   create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
>   create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
>   create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
>   create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
>   create mode 100644 drivers/gpu/drm/i915/intel_gvt.c
>   create mode 100644 drivers/gpu/drm/i915/intel_gvt.h
>   create mode 100644 include/drm/i915_gvt.h
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 29a32b1..feb56ee 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -57,6 +57,21 @@ config DRM_I915_USERPTR
>
>   	  If in doubt, say "Y".
>
> +config DRM_I915_GVT
> +        bool "Intel GVT-g host driver"
> +        depends on DRM_I915
> +        default n
> +        help
> +          Enabling GVT-g mediated graphics pass-through technique for Intel i915
> +          based integrated graphics card. With GVT-g, it's possible to have one
> +          integrated i915 device shared by multiple VMs. Performance critical
> +          operations such as aperture accesses and ring buffer operations
> +          are passed-through to VM, with a minimal set of conflicting resources

Maybe I am confused but to me this is not clear. If we have multiple VMs 
sharing the GPU where performance critical operations are passed-through 
to the *VM* ? Aren't they passed-through to the GPU/host/hypervisor or 
something rather than the VM?

> +          (e.g. display settings) mediated by GVT host driver. The benefit of GVT
> +          is on both the performance, given that each VM could directly operate
> +          its aperture space and submit commands like running on native, and
> +          the feature completeness, given that a true GEN hardware is exposed.
> +
>   menu "drm/i915 Debugging"
>   depends on DRM_I915
>   depends on EXPERT
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 63c4d2b..e48145b 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -103,6 +103,11 @@ i915-y += i915_vgpu.o
>   # legacy horrors
>   i915-y += i915_dma.o
>
> +ifeq ($(CONFIG_DRM_I915_GVT),y)
> +i915-y += intel_gvt.o
> +include $(src)/gvt/Makefile
> +endif
> +
>   obj-$(CONFIG_DRM_I915)  += i915.o
>
>   CFLAGS_i915_trace_points.o := -I$(src)
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> new file mode 100644
> index 0000000..d0f21a6
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -0,0 +1,5 @@
> +GVT_DIR := gvt
> +GVT_SOURCE := gvt.o
> +
> +ccflags-y                      += -I$(src) -I$(src)/$(GVT_DIR) -Wall
> +i915-y			       += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
> diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
> new file mode 100644
> index 0000000..5b067d2
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/debug.h
> @@ -0,0 +1,36 @@
> +/*
> + * 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.
> + */
> +
> +#ifndef __GVT_DEBUG_H__
> +#define __GVT_DEBUG_H__
> +
> +#define gvt_info(fmt, args...) \
> +	DRM_INFO("gvt: "fmt, ##args)
> +
> +#define gvt_err(fmt, args...) \
> +	DRM_ERROR("gvt: "fmt, ##args)
> +
> +#define gvt_dbg_core(fmt, args...) \
> +	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> new file mode 100644
> index 0000000..aa40357
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -0,0 +1,205 @@
> +/*
> + * 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.
> + */
> +
> +#include <linux/types.h>
> +#include <xen/xen.h>
> +#include <linux/kthread.h>
> +
> +#include "gvt.h"
> +
> +struct intel_gvt_host intel_gvt_host;
> +
> +static const char * const supported_hypervisors[] = {
> +	[INTEL_GVT_HYPERVISOR_XEN] = "XEN",
> +	[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
> +};
> +
> +#define MB(x) (x * 1024ULL * 1024ULL)
> +#define GB(x) (x * MB(1024))
> +
> +/*
> + * The layout of BAR0 in BDW:
> + * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->|
> + *
> + * GTT offset in BAR0 starts from 8MB to 16MB, and
> + * Whatever GTT size is configured in BIOS,
> + * the size of BAR0 is always 16MB. The actual configured
> + * GTT size can be found in GMCH_CTRL.
> + */
> +static struct intel_gvt_device_info broadwell_device_info = {
> +	.max_gtt_gm_sz = GB(4), /* 4GB */
> +	.gtt_start_offset = MB(8), /* 8MB */
> +	.max_gtt_size = MB(8), /* 8MB */
> +	.gtt_entry_size = 8,
> +	.gtt_entry_size_shift = 3,
> +	.gmadr_bytes_in_cmd = 8,
> +	.mmio_size = MB(2), /* 2MB */
> +	.mmio_bar = 0, /* BAR 0 */
> +	.max_support_vgpu = 8,
> +};
> +
> +static int init_gvt_host(void)
> +{
> +	if (WARN(intel_gvt_host.initialized,
> +			"Intel GVT host has already been initialized\n"))
> +		return -EINVAL;
> +
> +	/* Xen DOM U */
> +	if (xen_domain() && !xen_initial_domain())
> +		return -ENODEV;
> +
> +	if (xen_initial_domain()) {
> +		/* Xen Dom0 */
> +		intel_gvt_host.mpt = try_then_request_module(
> +				symbol_get(xengt_mpt), "xengt");
> +		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_XEN;
> +	} else {
> +		/* not in Xen. Try KVMGT */
> +		intel_gvt_host.mpt = try_then_request_module(
> +				symbol_get(kvmgt_mpt), "kvm");
> +		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_KVM;
> +	}
> +
> +	if (!intel_gvt_host.mpt) {
> +		gvt_err("Fail to load any MPT modules.\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!intel_gvt_hypervisor_detect_host())
> +		return -ENODEV;
> +
> +	gvt_info("Running with hypervisor %s in host mode\n",
> +			supported_hypervisors[intel_gvt_host.hypervisor_type]);
> +
> +	idr_init(&intel_gvt_host.gvt_idr);
> +	mutex_init(&intel_gvt_host.gvt_idr_lock);
> +
> +	intel_gvt_host.initialized = true;
> +	return 0;
> +}
> +
> +static int init_device_info(struct intel_gvt *gvt)
> +{
> +	if (IS_BROADWELL(gvt->dev_priv))
> +		gvt->device_info = &broadwell_device_info;
> +	else
> +		return -ENODEV;
> +

My previous comment was that intel_gvt_create_device (which calls this) 
can only get called if is_supported_device succeeded. And the latter has 
the same IS_BROADWELL check.

So to me error handling in init_device_info looks redundant.

I would probably just remove this function and initialize device info 
directly in intel_gvt_create_device which would simplify error handling 
there and you probably would end up with one debug message in there as 
well then.

> +	return 0;
> +}
> +
> +static void free_gvt_device(struct intel_gvt *gvt)
> +{
> +	mutex_lock(&intel_gvt_host.gvt_idr_lock);
> +	idr_remove(&intel_gvt_host.gvt_idr, gvt->id);
> +	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
> +
> +	vfree(gvt);
> +}
> +
> +static struct intel_gvt *alloc_gvt_device(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_gvt *gvt;
> +	int ret;
> +
> +	gvt = vzalloc(sizeof(*gvt));
> +	if (!gvt)
> +		return ERR_PTR(-ENOMEM);
> +
> +	mutex_lock(&intel_gvt_host.gvt_idr_lock);
> +	ret = idr_alloc(&intel_gvt_host.gvt_idr, gvt, 0, 0, GFP_KERNEL);
> +	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
> +
> +	if (ret < 0)
> +		goto err;
> +
> +	gvt->id = ret;
> +	mutex_init(&gvt->lock);
> +	gvt->dev_priv = dev_priv;
> +	idr_init(&gvt->vgpu_idr);
> +
> +	return gvt;
> +err:
> +	free_gvt_device(gvt);
> +	return ERR_PTR(ret);
> +}
> +
> +/**
> + * intel_gvt_destroy_device - destroy a GVT device
> + * @gvt_device: gvt device
> + *
> + * This function is called at the driver unloading stage, to destroy a
> + * GVT device and free the related resources.
> + *
> + */
> +void intel_gvt_destroy_device(void *device)

You said this is a void * because you do not want to include gvt/gvt.h - 
but this is gvt.c so why does not it not make sense to include it?

Or you mean you don't want to include it from the callers of 
intel_gvt_destroy_device?

I think it is really not ideal to have void * in the API here and also 
in the data structures elsewhere. Are you sure headers cannot be 
re-organized so that the correct types are used? Maybe a few forward 
declarations at the right places?

> +{
> +	struct intel_gvt *gvt = (struct intel_gvt *)device;
> +
> +	free_gvt_device(gvt);
> +}
> +
> +/**
> + * intel_gvt_create_device - create a GVT device
> + * @dev_priv: drm i915 private data
> + *
> + * This function is called at the initialization stage, to create a
> + * GVT device and initialize necessary GVT components for it.
> + *
> + * Returns:
> + * pointer to the intel gvt device structure, error pointer if failed.
> + */
> +void *intel_gvt_create_device(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_gvt *gvt;
> +	int ret;
> +
> +	if (!intel_gvt_host.initialized) {
> +		ret = init_gvt_host();
> +		if (ret)
> +			return ERR_PTR(ret);
> +	}
> +
> +	gvt_dbg_core("create new gvt device\n");
> +
> +	gvt = alloc_gvt_device(dev_priv);
> +	if (IS_ERR(gvt)) {
> +		ret = PTR_ERR(gvt);
> +		goto out_err;
> +	}
> +
> +	gvt_dbg_core("init gvt device, id %d\n", gvt->id);
> +
> +	ret = init_device_info(gvt);
> +	if (ret)
> +		goto out_free_gvt_device;
> +
> +	gvt_dbg_core("gvt device creation done, id %d\n", gvt->id);
> +
> +	return gvt;
> +
> +out_free_gvt_device:
> +	free_gvt_device(gvt);
> +out_err:
> +	return ERR_PTR(ret);
> +}
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> new file mode 100644
> index 0000000..9e6e22b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -0,0 +1,84 @@
> +/*
> + * 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.
> + */
> +
> +#ifndef _GVT_H_
> +#define _GVT_H_
> +
> +#include "i915_drv.h"
> +#include "i915_vgpu.h"
> +
> +#include "debug.h"
> +#include "hypercall.h"
> +
> +#define GVT_MAX_VGPU 8
> +#define GVT_ALIGN(addr, size) ((addr) & (~((typeof(addr))(size) - 1)))
> +
> +enum {
> +	INTEL_GVT_HYPERVISOR_XEN = 0,
> +	INTEL_GVT_HYPERVISOR_KVM,
> +};
> +
> +struct intel_gvt_host {
> +	bool initialized;
> +	int hypervisor_type;
> +	struct mutex gvt_idr_lock;
> +	struct idr gvt_idr;
> +	struct intel_gvt_mpt *mpt;
> +};
> +
> +extern struct intel_gvt_host intel_gvt_host;
> +
> +/* Describe the limitation of HW.*/
> +struct intel_gvt_device_info {
> +	u64 max_gtt_gm_sz;
> +	u32 gtt_start_offset;
> +	u32 gtt_end_offset;
> +	u32 max_gtt_size;
> +	u32 gtt_entry_size;
> +	u32 gtt_entry_size_shift;
> +	u32 gmadr_bytes_in_cmd;
> +	u32 mmio_size;
> +	u32 mmio_bar;
> +	u32 max_support_vgpu;
> +};
> +
> +struct intel_vgpu {
> +	struct intel_gvt *gvt;
> +	int id;
> +	int vm_id;
> +	bool warn_untrack;
> +};
> +
> +struct intel_gvt {
> +	struct mutex lock;
> +	int id;
> +
> +	struct drm_i915_private *dev_priv;
> +	struct idr vgpu_idr;
> +
> +	struct intel_gvt_device_info *device_info;
> +};
> +
> +#include "mpt.h"
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
> new file mode 100644
> index 0000000..254df8b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> @@ -0,0 +1,38 @@
> +/*
> + * 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.
> + */
> +
> +#ifndef _GVT_HYPERCALL_H_
> +#define _GVT_HYPERCALL_H_
> +
> +/*
> + * Specific GVT-g MPT modules function collections. Currently GVT-g supports
> + * both Xen and KVM by providing dedicated hypervisor-related MPT modules.
> + */
> +struct intel_gvt_mpt {
> +	int (*detect_host)(void);
> +};
> +
> +extern struct intel_gvt_mpt xengt_mpt;
> +extern struct intel_gvt_mpt kvmgt_mpt;
> +
> +#endif /* _GVT_HYPERCALL_H_ */
> diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
> new file mode 100644
> index 0000000..783f4f8
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> @@ -0,0 +1,49 @@
> +/*
> + * 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.
> + */
> +
> +#ifndef _GVT_MPT_H_
> +#define _GVT_MPT_H_
> +
> +/**
> + * DOC: Hypervisor Service APIs for GVT-g Core Logic
> + *
> + * This is the glue layer between specific hypervisor MPT modules and GVT-g core
> + * logic. Each kind of hypervisor MPT module provides a collection of function
> + * callbacks via gvt_kernel_dm and will be attached to GVT host when driver
> + * loading. GVT-g core logic will call these APIs to request specific services
> + * from hypervisor.
> + */
> +
> +/**
> + * intel_gvt_hypervisor_detect_host - check if GVT-g is running within
> + * hypervisor host/privilged domain
> + *
> + * Returns:
> + * Zero on success, -ENODEV if current kernel is running inside a VM
> + */
> +static inline int intel_gvt_hypervisor_detect_host(void)
> +{
> +	return intel_gvt_host.mpt->detect_host();
> +}
> +
> +#endif /* _GVT_MPT_H_ */
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 547100f..502d7cd 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -35,6 +35,7 @@
>   #include "intel_drv.h"
>   #include <drm/i915_drm.h>
>   #include "i915_drv.h"
> +#include "intel_gvt.h"
>   #include "i915_vgpu.h"
>   #include "i915_trace.h"
>   #include <linux/pci.h>
> @@ -1245,18 +1246,22 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>   		goto out_ggtt;
>   	}
>
> +	ret = intel_gvt_init(dev_priv);
> +	if (ret)
> +		goto out_ggtt;
> +
>   	/* WARNING: Apparently we must kick fbdev drivers before vgacon,
>   	 * otherwise the vga fbdev driver falls over. */
>   	ret = i915_kick_out_firmware_fb(dev_priv);
>   	if (ret) {
>   		DRM_ERROR("failed to remove conflicting framebuffer drivers\n");
> -		goto out_ggtt;
> +		goto out_gvt;
>   	}
>
>   	ret = i915_kick_out_vgacon(dev_priv);
>   	if (ret) {
>   		DRM_ERROR("failed to remove conflicting VGA console\n");
> -		goto out_ggtt;
> +		goto out_gvt;
>   	}
>
>   	pci_set_master(dev->pdev);
> @@ -1267,7 +1272,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>   		if (ret) {
>   			DRM_ERROR("failed to set DMA mask\n");
>
> -			goto out_ggtt;
> +			goto out_gvt;
>   		}
>   	}
>
> @@ -1297,7 +1302,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>   				     aperture_size);
>   	if (!ggtt->mappable) {
>   		ret = -EIO;
> -		goto out_ggtt;
> +		goto out_gvt;
>   	}
>
>   	ggtt->mtrr = arch_phys_wc_add(ggtt->mappable_base,
> @@ -1330,6 +1335,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>
>   	return 0;
>
> +out_gvt:
> +	intel_gvt_cleanup(dev_priv);
>   out_ggtt:
>   	i915_ggtt_cleanup_hw(dev);
>
> @@ -1488,6 +1495,8 @@ int i915_driver_unload(struct drm_device *dev)
>
>   	intel_fbdev_fini(dev);
>
> +	intel_gvt_cleanup(dev_priv);
> +
>   	ret = i915_gem_suspend(dev);
>   	if (ret) {
>   		DRM_ERROR("failed to idle hardware: %d\n", ret);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 72f0b02..7d0b8d3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1703,6 +1703,10 @@ struct i915_workarounds {
>   	u32 hw_whitelist_count[I915_NUM_ENGINES];
>   };
>
> +struct i915_gvt {
> +	void *gvt;
> +};
> +
>   struct i915_virtual_gpu {
>   	bool active;
>   };
> @@ -1742,6 +1746,8 @@ struct drm_i915_private {
>
>   	struct i915_virtual_gpu vgpu;
>
> +	struct i915_gvt gvt;
> +
>   	struct intel_guc guc;
>
>   	struct intel_csr csr;
> @@ -2868,6 +2874,12 @@ void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
>   u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
>
>   void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
> +
> +static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
> +{
> +	return dev_priv->gvt.gvt ? true : false;
> +}
> +
>   static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
>   {
>   	return dev_priv->vgpu.active;
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
> new file mode 100644
> index 0000000..815cc9f
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -0,0 +1,98 @@
> +/*
> + * 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.
> + */
> +
> +#include "i915_drv.h"
> +#include "intel_gvt.h"
> +
> +/**
> + * DOC: Intel GVT-g host support
> + *
> + * Intel GVT-g is a graphics virtualization technology which shares the
> + * GPU among multiple virtual machines on a time-sharing basis. Each
> + * virtual machine is presented a virtual GPU (vGPU), which has equivalent
> + * features as the underlying physical GPU (pGPU), so i915 driver can run
> + * seamlessly in a virtual machine. This file provides the englightments
> + * of GVT and the necessary components used by GVT in i915 driver.
> + */
> +
> +struct gvt_kernel_params gvt_kparams = {
> +	.enable = false,
> +};
> +
> +/* i915.gvt_enable */
> +module_param_named(gvt_enable, gvt_kparams.enable, bool, 0600);
> +MODULE_PARM_DESC(gvt_enable, "Enable Intel GVT-g host support");
> +
> +static bool is_supported_device(struct drm_i915_private *dev_priv)
> +{
> +	if (IS_BROADWELL(dev_priv))
> +		return true;
> +	return false;
> +}
> +
> +/**
> + * intel_gvt_init - initialize GVT components
> + * @dev_priv: drm i915 private data
> + *
> + * This function is called at the initialization stage to initialize the
> + * GVT components.
> + */
> +int intel_gvt_init(struct drm_i915_private *dev_priv)
> +{
> +	void *device;
> +
> +	if (!gvt_kparams.enable) {
> +		DRM_DEBUG_DRIVER("GVT-g is disabled by kernel params\n");
> +		return 0;
> +	}
> +
> +	if (!is_supported_device(dev_priv)) {
> +		DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
> +		return 0;
> +	}
> +
> +	device = intel_gvt_create_device(dev_priv);
> +	if (IS_ERR(device)) {
> +		DRM_DEBUG_DRIVER("GVT-g is disabled\n");
> +		return 0;
> +	}
> +
> +	dev_priv->gvt.gvt = device;
> +	return 0;
> +}
> +
> +/**
> + * intel_gvt_cleanup - cleanup GVT components when i915 driver is unloading
> + * @dev_priv: drm i915 private *
> + *
> + * This function is called at the i915 driver unloading stage, to shutdown
> + * GVT components and release the related resources.
> + */
> +void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
> +{
> +	if (!intel_gvt_active(dev_priv))
> +		return;
> +
> +	intel_gvt_destroy_device(dev_priv->gvt.gvt);
> +	dev_priv->gvt.gvt = NULL;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
> new file mode 100644
> index 0000000..8079dfd
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.h
> @@ -0,0 +1,49 @@
> +/*
> + * 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.
> + */
> +
> +#ifndef _INTEL_GVT_H_
> +#define _INTEL_GVT_H_
> +
> +#ifdef CONFIG_DRM_I915_GVT
> +
> +#include <drm/i915_gvt.h>
> +
> +struct gvt_kernel_params {
> +	bool enable;
> +};
> +
> +extern struct gvt_kernel_params gvt_kparams;
> +
> +extern int intel_gvt_init(struct drm_i915_private *dev_priv);
> +extern void intel_gvt_cleanup(struct drm_i915_private *dev_priv);
> +#else
> +static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
> +{
> +	return 0;
> +}
> +static inline void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
> +{
> +}
> +#endif
> +
> +#endif /* _INTEL_GVT_H_ */
> diff --git a/include/drm/i915_gvt.h b/include/drm/i915_gvt.h
> new file mode 100644
> index 0000000..4ed8b88
> --- /dev/null
> +++ b/include/drm/i915_gvt.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + * 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, sub license, 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.
> + */
> +#ifndef _I915_GVT_H
> +#define _I915_GVT_H
> +
> +extern void *intel_gvt_create_device(void *dev_priv);

void * dev_priv, vey bad :) And the below signature I've commented already.

> +extern void intel_gvt_destroy_device(void *gvt);
> +
> +#endif /* _I915_GVT_H */
>


Regards,

Tvrtko
Wang, Zhi A May 20, 2016, 10:57 a.m. UTC | #2
Thanks Tvrtko.:) See my replies below.

> -----Original Message-----
> From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
> Sent: Wednesday, May 18, 2016 2:23 PM
> To: Wang, Zhi A <zhi.a.wang@intel.com>; intel-gfx@lists.freedesktop.org;
> joonas.lahtinen@linux.intel.com; chris@chris-wilson.co.uk; Tian, Kevin
> <kevin.tian@intel.com>; Lv, Zhiyuan <zhiyuan.lv@intel.com>
> Subject: Re: [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of
> GVT-g
> 
> 
> On 17/05/16 09:19, Zhi Wang wrote:
> > This patch introduces the very basic framework of GVT-g device model,
> > includes basic prototypes, definitions, initialization.
> >
> > v5:
> > Take Tvrtko's comments:
> > - Fix the misspelled words in Kconfg
> > - Let functions take drm_i915_private * instead of struct drm_device *
> > - Remove redundant prints/local varible initialization
> >
> > v3:
> > Take Joonas' comments:
> > - Change file name i915_gvt.* to intel_gvt.*
> > - Move GVT kernel parameter into intel_gvt.c
> > - Remove redundant debug macros
> > - Change error handling style
> > - Add introductions for some stub functions
> > - Introduce drm/i915_gvt.h.
> >
> > Take Kevin's comments:
> > - Move GVT-g host/guest check into intel_vgt_balloon in i915_gem_gtt.c
> >
> > v2:
> > - Introduce i915_gvt.c.
> > It's necessary to introduce the stubs between i915 driver and GVT-g
> > host, as GVT-g components is configurable in kernel config. When
> > disabled, the stubs here do nothing.
> >
> > Take Joonas' comments:
> > - Replace boolean return value with int.
> > - Replace customized info/warn/debug macros with DRM macros.
> > - Document all non-static functions like i915.
> > - Remove empty and unused functions.
> > - Replace magic number with marcos.
> > - Set GVT-g in kernel config to "n" by default.
> >
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >   drivers/gpu/drm/i915/Kconfig         |  15 +++
> >   drivers/gpu/drm/i915/Makefile        |   5 +
> >   drivers/gpu/drm/i915/gvt/Makefile    |   5 +
> >   drivers/gpu/drm/i915/gvt/debug.h     |  36 ++++++
> >   drivers/gpu/drm/i915/gvt/gvt.c       | 205
> +++++++++++++++++++++++++++++++++++
> >   drivers/gpu/drm/i915/gvt/gvt.h       |  84 ++++++++++++++
> >   drivers/gpu/drm/i915/gvt/hypercall.h |  38 +++++++
> >   drivers/gpu/drm/i915/gvt/mpt.h       |  49 +++++++++
> >   drivers/gpu/drm/i915/i915_dma.c      |  17 ++-
> >   drivers/gpu/drm/i915/i915_drv.h      |  12 ++
> >   drivers/gpu/drm/i915/intel_gvt.c     |  98 +++++++++++++++++
> >   drivers/gpu/drm/i915/intel_gvt.h     |  49 +++++++++
> >   include/drm/i915_gvt.h               |  31 ++++++
> >   13 files changed, 640 insertions(+), 4 deletions(-)
> >   create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
> >   create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
> >   create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
> >   create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
> >   create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
> >   create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
> >   create mode 100644 drivers/gpu/drm/i915/intel_gvt.c
> >   create mode 100644 drivers/gpu/drm/i915/intel_gvt.h
> >   create mode 100644 include/drm/i915_gvt.h
> >
> > diff --git a/drivers/gpu/drm/i915/Kconfig
> > b/drivers/gpu/drm/i915/Kconfig index 29a32b1..feb56ee 100644
> > --- a/drivers/gpu/drm/i915/Kconfig
> > +++ b/drivers/gpu/drm/i915/Kconfig
> > @@ -57,6 +57,21 @@ config DRM_I915_USERPTR
> >
> >   	  If in doubt, say "Y".
> >
> > +config DRM_I915_GVT
> > +        bool "Intel GVT-g host driver"
> > +        depends on DRM_I915
> > +        default n
> > +        help
> > +          Enabling GVT-g mediated graphics pass-through technique for
> Intel i915
> > +          based integrated graphics card. With GVT-g, it's possible to
> have one
> > +          integrated i915 device shared by multiple VMs. Performance
> critical
> > +          operations such as aperture accesses and ring buffer
> operations
> > +          are passed-through to VM, with a minimal set of conflicting
> > +resources
> 
> Maybe I am confused but to me this is not clear. If we have multiple VMs
> sharing the GPU where performance critical operations are passed-through to
> the *VM* ? Aren't they passed-through to the GPU/host/hypervisor or
> something rather than the VM?
> 
This section is talking about aperture partition.

For aperture partition, first hypervisor will allocate a aperture region and notify VM the begin/end of this region, then VM would know in the whole "guest" aperture bar, only a portion could be used. Then hypervisor directly map the guest aperture region to the allocated host aperture portion through the EPT. When guest accesses this aperture, it won't be trapped, just directly reached the related host aperture region.

> > +          (e.g. display settings) mediated by GVT host driver. The benefit
> of GVT
> > +          is on both the performance, given that each VM could directly
> operate
> > +          its aperture space and submit commands like running on native,
> and
> > +          the feature completeness, given that a true GEN hardware is
> exposed.
> > +
> >   menu "drm/i915 Debugging"
> >   depends on DRM_I915
> >   depends on EXPERT
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile index 63c4d2b..e48145b 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -103,6 +103,11 @@ i915-y += i915_vgpu.o
> >   # legacy horrors
> >   i915-y += i915_dma.o
> >
> > +ifeq ($(CONFIG_DRM_I915_GVT),y)
> > +i915-y += intel_gvt.o
> > +include $(src)/gvt/Makefile
> > +endif
> > +
> >   obj-$(CONFIG_DRM_I915)  += i915.o
> >
> >   CFLAGS_i915_trace_points.o := -I$(src) diff --git
> > a/drivers/gpu/drm/i915/gvt/Makefile
> > b/drivers/gpu/drm/i915/gvt/Makefile
> > new file mode 100644
> > index 0000000..d0f21a6
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/Makefile
> > @@ -0,0 +1,5 @@
> > +GVT_DIR := gvt
> > +GVT_SOURCE := gvt.o
> > +
> > +ccflags-y                      += -I$(src) -I$(src)/$(GVT_DIR) -Wall
> > +i915-y			       += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
> > diff --git a/drivers/gpu/drm/i915/gvt/debug.h
> > b/drivers/gpu/drm/i915/gvt/debug.h
> > new file mode 100644
> > index 0000000..5b067d2
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/debug.h
> > @@ -0,0 +1,36 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +#ifndef __GVT_DEBUG_H__
> > +#define __GVT_DEBUG_H__
> > +
> > +#define gvt_info(fmt, args...) \
> > +	DRM_INFO("gvt: "fmt, ##args)
> > +
> > +#define gvt_err(fmt, args...) \
> > +	DRM_ERROR("gvt: "fmt, ##args)
> > +
> > +#define gvt_dbg_core(fmt, args...) \
> > +	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
> > +
> > +#endif
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c
> > b/drivers/gpu/drm/i915/gvt/gvt.c new file mode 100644 index
> > 0000000..aa40357
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > @@ -0,0 +1,205 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +#include <linux/types.h>
> > +#include <xen/xen.h>
> > +#include <linux/kthread.h>
> > +
> > +#include "gvt.h"
> > +
> > +struct intel_gvt_host intel_gvt_host;
> > +
> > +static const char * const supported_hypervisors[] = {
> > +	[INTEL_GVT_HYPERVISOR_XEN] = "XEN",
> > +	[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
> > +};
> > +
> > +#define MB(x) (x * 1024ULL * 1024ULL) #define GB(x) (x * MB(1024))
> > +
> > +/*
> > + * The layout of BAR0 in BDW:
> > + * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->|
> > + *
> > + * GTT offset in BAR0 starts from 8MB to 16MB, and
> > + * Whatever GTT size is configured in BIOS,
> > + * the size of BAR0 is always 16MB. The actual configured
> > + * GTT size can be found in GMCH_CTRL.
> > + */
> > +static struct intel_gvt_device_info broadwell_device_info = {
> > +	.max_gtt_gm_sz = GB(4), /* 4GB */
> > +	.gtt_start_offset = MB(8), /* 8MB */
> > +	.max_gtt_size = MB(8), /* 8MB */
> > +	.gtt_entry_size = 8,
> > +	.gtt_entry_size_shift = 3,
> > +	.gmadr_bytes_in_cmd = 8,
> > +	.mmio_size = MB(2), /* 2MB */
> > +	.mmio_bar = 0, /* BAR 0 */
> > +	.max_support_vgpu = 8,
> > +};
> > +
> > +static int init_gvt_host(void)
> > +{
> > +	if (WARN(intel_gvt_host.initialized,
> > +			"Intel GVT host has already been initialized\n"))
> > +		return -EINVAL;
> > +
> > +	/* Xen DOM U */
> > +	if (xen_domain() && !xen_initial_domain())
> > +		return -ENODEV;
> > +
> > +	if (xen_initial_domain()) {
> > +		/* Xen Dom0 */
> > +		intel_gvt_host.mpt = try_then_request_module(
> > +				symbol_get(xengt_mpt), "xengt");
> > +		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_XEN;
> > +	} else {
> > +		/* not in Xen. Try KVMGT */
> > +		intel_gvt_host.mpt = try_then_request_module(
> > +				symbol_get(kvmgt_mpt), "kvm");
> > +		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_KVM;
> > +	}
> > +
> > +	if (!intel_gvt_host.mpt) {
> > +		gvt_err("Fail to load any MPT modules.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!intel_gvt_hypervisor_detect_host())
> > +		return -ENODEV;
> > +
> > +	gvt_info("Running with hypervisor %s in host mode\n",
> > +			supported_hypervisors[intel_gvt_host.hypervisor_type]);
> > +
> > +	idr_init(&intel_gvt_host.gvt_idr);
> > +	mutex_init(&intel_gvt_host.gvt_idr_lock);
> > +
> > +	intel_gvt_host.initialized = true;
> > +	return 0;
> > +}
> > +
> > +static int init_device_info(struct intel_gvt *gvt) {
> > +	if (IS_BROADWELL(gvt->dev_priv))
> > +		gvt->device_info = &broadwell_device_info;
> > +	else
> > +		return -ENODEV;
> > +
> 
> My previous comment was that intel_gvt_create_device (which calls this) can
> only get called if is_supported_device succeeded. And the latter has the same
> IS_BROADWELL check.
> 
> So to me error handling in init_device_info looks redundant.
> 
> I would probably just remove this function and initialize device info directly in
> intel_gvt_create_device which would simplify error handling there and you
> probably would end up with one debug message in there as well then.
> 
Good point. Got it now. :)
> > +	return 0;
> > +}
> > +
> > +static void free_gvt_device(struct intel_gvt *gvt) {
> > +	mutex_lock(&intel_gvt_host.gvt_idr_lock);
> > +	idr_remove(&intel_gvt_host.gvt_idr, gvt->id);
> > +	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
> > +
> > +	vfree(gvt);
> > +}
> > +
> > +static struct intel_gvt *alloc_gvt_device(struct drm_i915_private
> > +*dev_priv) {
> > +	struct intel_gvt *gvt;
> > +	int ret;
> > +
> > +	gvt = vzalloc(sizeof(*gvt));
> > +	if (!gvt)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	mutex_lock(&intel_gvt_host.gvt_idr_lock);
> > +	ret = idr_alloc(&intel_gvt_host.gvt_idr, gvt, 0, 0, GFP_KERNEL);
> > +	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
> > +
> > +	if (ret < 0)
> > +		goto err;
> > +
> > +	gvt->id = ret;
> > +	mutex_init(&gvt->lock);
> > +	gvt->dev_priv = dev_priv;
> > +	idr_init(&gvt->vgpu_idr);
> > +
> > +	return gvt;
> > +err:
> > +	free_gvt_device(gvt);
> > +	return ERR_PTR(ret);
> > +}
> > +
> > +/**
> > + * intel_gvt_destroy_device - destroy a GVT device
> > + * @gvt_device: gvt device
> > + *
> > + * This function is called at the driver unloading stage, to destroy
> > +a
> > + * GVT device and free the related resources.
> > + *
> > + */
> > +void intel_gvt_destroy_device(void *device)
> 
> You said this is a void * because you do not want to include gvt/gvt.h - but this
> is gvt.c so why does not it not make sense to include it?
> 
> Or you mean you don't want to include it from the callers of
> intel_gvt_destroy_device?
> 
> I think it is really not ideal to have void * in the API here and also in the data
> structures elsewhere. Are you sure headers cannot be re-organized so that the
> correct types are used? Maybe a few forward declarations at the right places?
> 
Yes. I don't want the caller of the intel_gvt_{creat/destroy}_device to include "gvt/gvt.h". As "struct intel_gvt" will be a big data structure in furture, if I get intel_gvt_*_device take it, I have to introduce a lot of sub-data structures.
Probably I can let them take "struct i915_gvt" directly. So we don't need to pass void * in API.

> > +{
> > +	struct intel_gvt *gvt = (struct intel_gvt *)device;
> > +
> > +	free_gvt_device(gvt);
> > +}
> > +
> > +/**
> > + * intel_gvt_create_device - create a GVT device
> > + * @dev_priv: drm i915 private data
> > + *
> > + * This function is called at the initialization stage, to create a
> > + * GVT device and initialize necessary GVT components for it.
> > + *
> > + * Returns:
> > + * pointer to the intel gvt device structure, error pointer if failed.
> > + */
> > +void *intel_gvt_create_device(struct drm_i915_private *dev_priv) {
> > +	struct intel_gvt *gvt;
> > +	int ret;
> > +
> > +	if (!intel_gvt_host.initialized) {
> > +		ret = init_gvt_host();
> > +		if (ret)
> > +			return ERR_PTR(ret);
> > +	}
> > +
> > +	gvt_dbg_core("create new gvt device\n");
> > +
> > +	gvt = alloc_gvt_device(dev_priv);
> > +	if (IS_ERR(gvt)) {
> > +		ret = PTR_ERR(gvt);
> > +		goto out_err;
> > +	}
> > +
> > +	gvt_dbg_core("init gvt device, id %d\n", gvt->id);
> > +
> > +	ret = init_device_info(gvt);
> > +	if (ret)
> > +		goto out_free_gvt_device;
> > +
> > +	gvt_dbg_core("gvt device creation done, id %d\n", gvt->id);
> > +
> > +	return gvt;
> > +
> > +out_free_gvt_device:
> > +	free_gvt_device(gvt);
> > +out_err:
> > +	return ERR_PTR(ret);
> > +}
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h
> > b/drivers/gpu/drm/i915/gvt/gvt.h new file mode 100644 index
> > 0000000..9e6e22b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > @@ -0,0 +1,84 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +#ifndef _GVT_H_
> > +#define _GVT_H_
> > +
> > +#include "i915_drv.h"
> > +#include "i915_vgpu.h"
> > +
> > +#include "debug.h"
> > +#include "hypercall.h"
> > +
> > +#define GVT_MAX_VGPU 8
> > +#define GVT_ALIGN(addr, size) ((addr) & (~((typeof(addr))(size) -
> > +1)))
> > +
> > +enum {
> > +	INTEL_GVT_HYPERVISOR_XEN = 0,
> > +	INTEL_GVT_HYPERVISOR_KVM,
> > +};
> > +
> > +struct intel_gvt_host {
> > +	bool initialized;
> > +	int hypervisor_type;
> > +	struct mutex gvt_idr_lock;
> > +	struct idr gvt_idr;
> > +	struct intel_gvt_mpt *mpt;
> > +};
> > +
> > +extern struct intel_gvt_host intel_gvt_host;
> > +
> > +/* Describe the limitation of HW.*/
> > +struct intel_gvt_device_info {
> > +	u64 max_gtt_gm_sz;
> > +	u32 gtt_start_offset;
> > +	u32 gtt_end_offset;
> > +	u32 max_gtt_size;
> > +	u32 gtt_entry_size;
> > +	u32 gtt_entry_size_shift;
> > +	u32 gmadr_bytes_in_cmd;
> > +	u32 mmio_size;
> > +	u32 mmio_bar;
> > +	u32 max_support_vgpu;
> > +};
> > +
> > +struct intel_vgpu {
> > +	struct intel_gvt *gvt;
> > +	int id;
> > +	int vm_id;
> > +	bool warn_untrack;
> > +};
> > +
> > +struct intel_gvt {
> > +	struct mutex lock;
> > +	int id;
> > +
> > +	struct drm_i915_private *dev_priv;
> > +	struct idr vgpu_idr;
> > +
> > +	struct intel_gvt_device_info *device_info; };
> > +
> > +#include "mpt.h"
> > +
> > +#endif
> > diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h
> > b/drivers/gpu/drm/i915/gvt/hypercall.h
> > new file mode 100644
> > index 0000000..254df8b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> > @@ -0,0 +1,38 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +#ifndef _GVT_HYPERCALL_H_
> > +#define _GVT_HYPERCALL_H_
> > +
> > +/*
> > + * Specific GVT-g MPT modules function collections. Currently GVT-g
> > +supports
> > + * both Xen and KVM by providing dedicated hypervisor-related MPT
> modules.
> > + */
> > +struct intel_gvt_mpt {
> > +	int (*detect_host)(void);
> > +};
> > +
> > +extern struct intel_gvt_mpt xengt_mpt; extern struct intel_gvt_mpt
> > +kvmgt_mpt;
> > +
> > +#endif /* _GVT_HYPERCALL_H_ */
> > diff --git a/drivers/gpu/drm/i915/gvt/mpt.h
> > b/drivers/gpu/drm/i915/gvt/mpt.h new file mode 100644 index
> > 0000000..783f4f8
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> > @@ -0,0 +1,49 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +#ifndef _GVT_MPT_H_
> > +#define _GVT_MPT_H_
> > +
> > +/**
> > + * DOC: Hypervisor Service APIs for GVT-g Core Logic
> > + *
> > + * This is the glue layer between specific hypervisor MPT modules and
> > +GVT-g core
> > + * logic. Each kind of hypervisor MPT module provides a collection of
> > +function
> > + * callbacks via gvt_kernel_dm and will be attached to GVT host when
> > +driver
> > + * loading. GVT-g core logic will call these APIs to request specific
> > +services
> > + * from hypervisor.
> > + */
> > +
> > +/**
> > + * intel_gvt_hypervisor_detect_host - check if GVT-g is running
> > +within
> > + * hypervisor host/privilged domain
> > + *
> > + * Returns:
> > + * Zero on success, -ENODEV if current kernel is running inside a VM
> > +*/ static inline int intel_gvt_hypervisor_detect_host(void)
> > +{
> > +	return intel_gvt_host.mpt->detect_host();
> > +}
> > +
> > +#endif /* _GVT_MPT_H_ */
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c index 547100f..502d7cd 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -35,6 +35,7 @@
> >   #include "intel_drv.h"
> >   #include <drm/i915_drm.h>
> >   #include "i915_drv.h"
> > +#include "intel_gvt.h"
> >   #include "i915_vgpu.h"
> >   #include "i915_trace.h"
> >   #include <linux/pci.h>
> > @@ -1245,18 +1246,22 @@ static int i915_driver_init_hw(struct
> drm_i915_private *dev_priv)
> >   		goto out_ggtt;
> >   	}
> >
> > +	ret = intel_gvt_init(dev_priv);
> > +	if (ret)
> > +		goto out_ggtt;
> > +
> >   	/* WARNING: Apparently we must kick fbdev drivers before vgacon,
> >   	 * otherwise the vga fbdev driver falls over. */
> >   	ret = i915_kick_out_firmware_fb(dev_priv);
> >   	if (ret) {
> >   		DRM_ERROR("failed to remove conflicting framebuffer
> drivers\n");
> > -		goto out_ggtt;
> > +		goto out_gvt;
> >   	}
> >
> >   	ret = i915_kick_out_vgacon(dev_priv);
> >   	if (ret) {
> >   		DRM_ERROR("failed to remove conflicting VGA console\n");
> > -		goto out_ggtt;
> > +		goto out_gvt;
> >   	}
> >
> >   	pci_set_master(dev->pdev);
> > @@ -1267,7 +1272,7 @@ static int i915_driver_init_hw(struct
> drm_i915_private *dev_priv)
> >   		if (ret) {
> >   			DRM_ERROR("failed to set DMA mask\n");
> >
> > -			goto out_ggtt;
> > +			goto out_gvt;
> >   		}
> >   	}
> >
> > @@ -1297,7 +1302,7 @@ static int i915_driver_init_hw(struct
> drm_i915_private *dev_priv)
> >   				     aperture_size);
> >   	if (!ggtt->mappable) {
> >   		ret = -EIO;
> > -		goto out_ggtt;
> > +		goto out_gvt;
> >   	}
> >
> >   	ggtt->mtrr = arch_phys_wc_add(ggtt->mappable_base,
> > @@ -1330,6 +1335,8 @@ static int i915_driver_init_hw(struct
> > drm_i915_private *dev_priv)
> >
> >   	return 0;
> >
> > +out_gvt:
> > +	intel_gvt_cleanup(dev_priv);
> >   out_ggtt:
> >   	i915_ggtt_cleanup_hw(dev);
> >
> > @@ -1488,6 +1495,8 @@ int i915_driver_unload(struct drm_device *dev)
> >
> >   	intel_fbdev_fini(dev);
> >
> > +	intel_gvt_cleanup(dev_priv);
> > +
> >   	ret = i915_gem_suspend(dev);
> >   	if (ret) {
> >   		DRM_ERROR("failed to idle hardware: %d\n", ret); diff --git
> > a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 72f0b02..7d0b8d3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1703,6 +1703,10 @@ struct i915_workarounds {
> >   	u32 hw_whitelist_count[I915_NUM_ENGINES];
> >   };
> >
> > +struct i915_gvt {
> > +	void *gvt;
> > +};
> > +
> >   struct i915_virtual_gpu {
> >   	bool active;
> >   };
> > @@ -1742,6 +1746,8 @@ struct drm_i915_private {
> >
> >   	struct i915_virtual_gpu vgpu;
> >
> > +	struct i915_gvt gvt;
> > +
> >   	struct intel_guc guc;
> >
> >   	struct intel_csr csr;
> > @@ -2868,6 +2874,12 @@ void intel_uncore_forcewake_put__locked(struct
> drm_i915_private *dev_priv,
> >   u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
> >
> >   void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
> > +
> > +static inline bool intel_gvt_active(struct drm_i915_private
> > +*dev_priv) {
> > +	return dev_priv->gvt.gvt ? true : false; }
> > +
> >   static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
> >   {
> >   	return dev_priv->vgpu.active;
> > diff --git a/drivers/gpu/drm/i915/intel_gvt.c
> > b/drivers/gpu/drm/i915/intel_gvt.c
> > new file mode 100644
> > index 0000000..815cc9f
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > @@ -0,0 +1,98 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +#include "i915_drv.h"
> > +#include "intel_gvt.h"
> > +
> > +/**
> > + * DOC: Intel GVT-g host support
> > + *
> > + * Intel GVT-g is a graphics virtualization technology which shares
> > +the
> > + * GPU among multiple virtual machines on a time-sharing basis. Each
> > + * virtual machine is presented a virtual GPU (vGPU), which has
> > +equivalent
> > + * features as the underlying physical GPU (pGPU), so i915 driver can
> > +run
> > + * seamlessly in a virtual machine. This file provides the
> > +englightments
> > + * of GVT and the necessary components used by GVT in i915 driver.
> > + */
> > +
> > +struct gvt_kernel_params gvt_kparams = {
> > +	.enable = false,
> > +};
> > +
> > +/* i915.gvt_enable */
> > +module_param_named(gvt_enable, gvt_kparams.enable, bool, 0600);
> > +MODULE_PARM_DESC(gvt_enable, "Enable Intel GVT-g host support");
> > +
> > +static bool is_supported_device(struct drm_i915_private *dev_priv) {
> > +	if (IS_BROADWELL(dev_priv))
> > +		return true;
> > +	return false;
> > +}
> > +
> > +/**
> > + * intel_gvt_init - initialize GVT components
> > + * @dev_priv: drm i915 private data
> > + *
> > + * This function is called at the initialization stage to initialize
> > +the
> > + * GVT components.
> > + */
> > +int intel_gvt_init(struct drm_i915_private *dev_priv) {
> > +	void *device;
> > +
> > +	if (!gvt_kparams.enable) {
> > +		DRM_DEBUG_DRIVER("GVT-g is disabled by kernel params\n");
> > +		return 0;
> > +	}
> > +
> > +	if (!is_supported_device(dev_priv)) {
> > +		DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
> > +		return 0;
> > +	}
> > +
> > +	device = intel_gvt_create_device(dev_priv);
> > +	if (IS_ERR(device)) {
> > +		DRM_DEBUG_DRIVER("GVT-g is disabled\n");
> > +		return 0;
> > +	}
> > +
> > +	dev_priv->gvt.gvt = device;
> > +	return 0;
> > +}
> > +
> > +/**
> > + * intel_gvt_cleanup - cleanup GVT components when i915 driver is
> > +unloading
> > + * @dev_priv: drm i915 private *
> > + *
> > + * This function is called at the i915 driver unloading stage, to
> > +shutdown
> > + * GVT components and release the related resources.
> > + */
> > +void intel_gvt_cleanup(struct drm_i915_private *dev_priv) {
> > +	if (!intel_gvt_active(dev_priv))
> > +		return;
> > +
> > +	intel_gvt_destroy_device(dev_priv->gvt.gvt);
> > +	dev_priv->gvt.gvt = NULL;
> > +}
> > diff --git a/drivers/gpu/drm/i915/intel_gvt.h
> > b/drivers/gpu/drm/i915/intel_gvt.h
> > new file mode 100644
> > index 0000000..8079dfd
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/intel_gvt.h
> > @@ -0,0 +1,49 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +#ifndef _INTEL_GVT_H_
> > +#define _INTEL_GVT_H_
> > +
> > +#ifdef CONFIG_DRM_I915_GVT
> > +
> > +#include <drm/i915_gvt.h>
> > +
> > +struct gvt_kernel_params {
> > +	bool enable;
> > +};
> > +
> > +extern struct gvt_kernel_params gvt_kparams;
> > +
> > +extern int intel_gvt_init(struct drm_i915_private *dev_priv); extern
> > +void intel_gvt_cleanup(struct drm_i915_private *dev_priv); #else
> > +static inline int intel_gvt_init(struct drm_i915_private *dev_priv) {
> > +	return 0;
> > +}
> > +static inline void intel_gvt_cleanup(struct drm_i915_private
> > +*dev_priv) { } #endif
> > +
> > +#endif /* _INTEL_GVT_H_ */
> > diff --git a/include/drm/i915_gvt.h b/include/drm/i915_gvt.h new file
> > mode 100644 index 0000000..4ed8b88
> > --- /dev/null
> > +++ b/include/drm/i915_gvt.h
> > @@ -0,0 +1,31 @@
> > +/*
> > + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> > + * 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, sub license, 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.
> > + */
> > +#ifndef _I915_GVT_H
> > +#define _I915_GVT_H
> > +
> > +extern void *intel_gvt_create_device(void *dev_priv);
> 
> void * dev_priv, vey bad :) And the below signature I've commented already.
> 
Sure. Will improve it in the next version. :)
> > +extern void intel_gvt_destroy_device(void *gvt);
> > +
> > +#endif /* _I915_GVT_H */
> >
> 
> 
> Regards,
> 
> Tvrtko
Chris Wilson May 20, 2016, 11:49 a.m. UTC | #3
On Tue, May 17, 2016 at 04:19:03AM -0400, Zhi Wang wrote:
> +config DRM_I915_GVT
> +        bool "Intel GVT-g host driver"
> +        depends on DRM_I915
> +        default n
> +        help
> +          Enabling GVT-g mediated graphics pass-through technique for Intel i915
> +          based integrated graphics card. With GVT-g, it's possible to have one
> +          integrated i915 device shared by multiple VMs. Performance critical
> +          operations such as aperture accesses and ring buffer operations
> +          are passed-through to VM, with a minimal set of conflicting resources
> +          (e.g. display settings) mediated by GVT host driver. The benefit of GVT
> +          is on both the performance, given that each VM could directly operate
> +          its aperture space and submit commands like running on native, and
> +          the feature completeness, given that a true GEN hardware is exposed.

Just like that? No userspace support required? i.e. can it work with
kvm or xen? Wants a comment on any possible danger (or why is it 'n' by
default)?

> +#ifndef __GVT_DEBUG_H__
> +#define __GVT_DEBUG_H__
> +
> +#define gvt_info(fmt, args...) \
> +	DRM_INFO("gvt: "fmt, ##args)
> +
> +#define gvt_err(fmt, args...) \
> +	DRM_ERROR("gvt: "fmt, ##args)

I think it is fair to say that neither of these will look nice in
user-facing messages.

> +
> +#define gvt_dbg_core(fmt, args...) \
> +	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
> new file mode 100644
> index 0000000..aa40357
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -0,0 +1,205 @@
> +/*
> + * 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.
> + */
> +
> +#include <linux/types.h>
> +#include <xen/xen.h>
> +#include <linux/kthread.h>
> +
> +#include "gvt.h"
> +
> +struct intel_gvt_host intel_gvt_host;
> +
> +static const char * const supported_hypervisors[] = {
> +	[INTEL_GVT_HYPERVISOR_XEN] = "XEN",
> +	[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
> +};
> +
> +#define MB(x) (x * 1024ULL * 1024ULL)
> +#define GB(x) (x * MB(1024))
> +
> +/*
> + * The layout of BAR0 in BDW:
> + * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->|
> + *
> + * GTT offset in BAR0 starts from 8MB to 16MB, and
> + * Whatever GTT size is configured in BIOS,
> + * the size of BAR0 is always 16MB. The actual configured
> + * GTT size can be found in GMCH_CTRL.
> + */
> +static struct intel_gvt_device_info broadwell_device_info = {
> +	.max_gtt_gm_sz = GB(4), /* 4GB */
> +	.gtt_start_offset = MB(8), /* 8MB */
> +	.max_gtt_size = MB(8), /* 8MB */
> +	.gtt_entry_size = 8,
> +	.gtt_entry_size_shift = 3,
> +	.gmadr_bytes_in_cmd = 8,
> +	.mmio_size = MB(2), /* 2MB */
> +	.mmio_bar = 0, /* BAR 0 */
> +	.max_support_vgpu = 8,

Too much superfluous detail upfront. It is hard to critique when you have
no idea what it is for. At the moment, it looks like you are duplicating
information we have elsewhere, and I'd rather not. (For starters, it
makes GVT a bigger maintainance burden.)

> +/**
> + * intel_gvt_destroy_device - destroy a GVT device
> + * @gvt_device: gvt device
> + *
> + * This function is called at the driver unloading stage, to destroy a
> + * GVT device and free the related resources.
> + *
> + */
> +void intel_gvt_destroy_device(void *device)

This should not be void *. You can forward declare struct intel_gvt such
that the core has an opaque pointer.

> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 72f0b02..7d0b8d3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1703,6 +1703,10 @@ struct i915_workarounds {
>  	u32 hw_whitelist_count[I915_NUM_ENGINES];
>  };
>  
> +struct i915_gvt {
> +	void *gvt;

As before. No void * here.

> +};

> +struct gvt_kernel_params {
> +	bool enable;
> +};
> +
> +extern struct gvt_kernel_params gvt_kparams;

No. Just make room for them inside i915_params. Module parameters
deserve extra scrutiny as they are (soft) ABI (even if we think users
shouldn't be using them).
-Chris
Wang, Zhi A May 20, 2016, 12:18 p.m. UTC | #4
Thanks! See my replies below.

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Friday, May 20, 2016 2:49 PM
> To: Wang, Zhi A <zhi.a.wang@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; tvrtko.ursulin@linux.intel.com;
> joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv,
> Zhiyuan <zhiyuan.lv@intel.com>
> Subject: Re: [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of
> GVT-g
> 
> On Tue, May 17, 2016 at 04:19:03AM -0400, Zhi Wang wrote:
> > +config DRM_I915_GVT
> > +        bool "Intel GVT-g host driver"
> > +        depends on DRM_I915
> > +        default n
> > +        help
> > +          Enabling GVT-g mediated graphics pass-through technique for
> Intel i915
> > +          based integrated graphics card. With GVT-g, it's possible to
> have one
> > +          integrated i915 device shared by multiple VMs. Performance
> critical
> > +          operations such as aperture accesses and ring buffer
> operations
> > +          are passed-through to VM, with a minimal set of conflicting
> resources
> > +          (e.g. display settings) mediated by GVT host driver. The benefit
> of GVT
> > +          is on both the performance, given that each VM could directly
> operate
> > +          its aperture space and submit commands like running on native,
> and
> > +          the feature completeness, given that a true GEN hardware is
> exposed.
> 
> Just like that? No userspace support required? i.e. can it work with kvm or xen?
> Wants a comment on any possible danger (or why is it 'n' by default)?
> 
Basically that text is only technical introduction. Or maybe we could discuss what we want here:
a. Basic technical introduction.
b. Need Xen and KVM
c. Possible danger as it's only preliminary for now.

i915 Kconfig introduction consists 3 parts:
a. Introduction
b. Userspace requirement
c. Limitiation

Can I follow that? :)

> > +#ifndef __GVT_DEBUG_H__
> > +#define __GVT_DEBUG_H__
> > +
> > +#define gvt_info(fmt, args...) \
> > +	DRM_INFO("gvt: "fmt, ##args)
> > +
> > +#define gvt_err(fmt, args...) \
> > +	DRM_ERROR("gvt: "fmt, ##args)
> 
> I think it is fair to say that neither of these will look nice in user-facing
> messages.
> 

Then how about [drm][gvt]? :)

> > +
> > +#define gvt_dbg_core(fmt, args...) \
> > +	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
> > +
> > +#endif
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.c
> > b/drivers/gpu/drm/i915/gvt/gvt.c new file mode 100644 index
> > 0000000..aa40357
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > @@ -0,0 +1,205 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +#include <linux/types.h>
> > +#include <xen/xen.h>
> > +#include <linux/kthread.h>
> > +
> > +#include "gvt.h"
> > +
> > +struct intel_gvt_host intel_gvt_host;
> > +
> > +static const char * const supported_hypervisors[] = {
> > +	[INTEL_GVT_HYPERVISOR_XEN] = "XEN",
> > +	[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
> > +};
> > +
> > +#define MB(x) (x * 1024ULL * 1024ULL) #define GB(x) (x * MB(1024))
> > +
> > +/*
> > + * The layout of BAR0 in BDW:
> > + * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->|
> > + *
> > + * GTT offset in BAR0 starts from 8MB to 16MB, and
> > + * Whatever GTT size is configured in BIOS,
> > + * the size of BAR0 is always 16MB. The actual configured
> > + * GTT size can be found in GMCH_CTRL.
> > + */
> > +static struct intel_gvt_device_info broadwell_device_info = {
> > +	.max_gtt_gm_sz = GB(4), /* 4GB */
> > +	.gtt_start_offset = MB(8), /* 8MB */
> > +	.max_gtt_size = MB(8), /* 8MB */
> > +	.gtt_entry_size = 8,
> > +	.gtt_entry_size_shift = 3,
> > +	.gmadr_bytes_in_cmd = 8,
> > +	.mmio_size = MB(2), /* 2MB */
> > +	.mmio_bar = 0, /* BAR 0 */
> > +	.max_support_vgpu = 8,
> 
> Too much superfluous detail upfront. It is hard to critique when you have no
> idea what it is for. At the moment, it looks like you are duplicating information
> we have elsewhere, and I'd rather not. (For starters, it makes GVT a bigger
> maintainance burden.)
> 
OK. Then I think I should remove most them in the first path and add them in the later patch. And do you want me to merge these HW descriptions into i915_device_info? Some of them might not be used by i915 driver itself. That's why I keep a dedicated data structure here.

> > +/**
> > + * intel_gvt_destroy_device - destroy a GVT device
> > + * @gvt_device: gvt device
> > + *
> > + * This function is called at the driver unloading stage, to destroy
> > +a
> > + * GVT device and free the related resources.
> > + *
> > + */
> > +void intel_gvt_destroy_device(void *device)
> 
> This should not be void *. You can forward declare struct intel_gvt such that the
> core has an opaque pointer.
> 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index 72f0b02..7d0b8d3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1703,6 +1703,10 @@ struct i915_workarounds {
> >  	u32 hw_whitelist_count[I915_NUM_ENGINES];
> >  };
> >
> > +struct i915_gvt {
> > +	void *gvt;
> 
> As before. No void * here.
> 
Let me think an approach to remove the "void *" here. How about we do like this:
struct a{
	Struct b;
};

Let i915 take only struct b, then in gvt we do container_of() so that we don't need to expose the declaration of struct a to i915? :)

> > +};
> 
> > +struct gvt_kernel_params {
> > +	bool enable;
> > +};
> > +
> > +extern struct gvt_kernel_params gvt_kparams;
> 
> No. Just make room for them inside i915_params. Module parameters deserve
> extra scrutiny as they are (soft) ABI (even if we think users shouldn't be using
> them).
OK. Will do.

> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre
Chris Wilson May 20, 2016, 12:37 p.m. UTC | #5
On Fri, May 20, 2016 at 12:18:10PM +0000, Wang, Zhi A wrote:
> Thanks! See my replies below.
> 
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > Sent: Friday, May 20, 2016 2:49 PM
> > To: Wang, Zhi A <zhi.a.wang@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; tvrtko.ursulin@linux.intel.com;
> > joonas.lahtinen@linux.intel.com; Tian, Kevin <kevin.tian@intel.com>; Lv,
> > Zhiyuan <zhiyuan.lv@intel.com>
> > Subject: Re: [PATCH 3/9] drm/i915: gvt: Introduce the basic architecture of
> > GVT-g
> > 
> > On Tue, May 17, 2016 at 04:19:03AM -0400, Zhi Wang wrote:
> > > +config DRM_I915_GVT
> > > +        bool "Intel GVT-g host driver"
> > > +        depends on DRM_I915
> > > +        default n
> > > +        help
> > > +          Enabling GVT-g mediated graphics pass-through technique for
> > Intel i915
> > > +          based integrated graphics card. With GVT-g, it's possible to
> > have one
> > > +          integrated i915 device shared by multiple VMs. Performance
> > critical
> > > +          operations such as aperture accesses and ring buffer
> > operations
> > > +          are passed-through to VM, with a minimal set of conflicting
> > resources
> > > +          (e.g. display settings) mediated by GVT host driver. The benefit
> > of GVT
> > > +          is on both the performance, given that each VM could directly
> > operate
> > > +          its aperture space and submit commands like running on native,
> > and
> > > +          the feature completeness, given that a true GEN hardware is
> > exposed.
> > 
> > Just like that? No userspace support required? i.e. can it work with kvm or xen?
> > Wants a comment on any possible danger (or why is it 'n' by default)?
> > 
> Basically that text is only technical introduction. Or maybe we could discuss what we want here:
> a. Basic technical introduction.
> b. Need Xen and KVM
> c. Possible danger as it's only preliminary for now.
> 
> i915 Kconfig introduction consists 3 parts:
> a. Introduction
> b. Userspace requirement
> c. Limitiation
> 
> Can I follow that? :)

I'm never going to argue against giving good advice to the reader. Even
if it is "for further details please see 01.org/blah"

> > > +#ifndef __GVT_DEBUG_H__
> > > +#define __GVT_DEBUG_H__
> > > +
> > > +#define gvt_info(fmt, args...) \
> > > +	DRM_INFO("gvt: "fmt, ##args)
> > > +
> > > +#define gvt_err(fmt, args...) \
> > > +	DRM_ERROR("gvt: "fmt, ##args)
> > 
> > I think it is fair to say that neither of these will look nice in user-facing
> > messages.
> > 
> 
> Then how about [drm][gvt]? :)

Just a comment to say to hesitate before using anything above DRM_DEBUG
and ask yourself "is this the right way to provide this information to
the user, is it as clear/useful as possible?"
> > > +static struct intel_gvt_device_info broadwell_device_info = {
> > > +	.max_gtt_gm_sz = GB(4), /* 4GB */
> > > +	.gtt_start_offset = MB(8), /* 8MB */
> > > +	.max_gtt_size = MB(8), /* 8MB */
> > > +	.gtt_entry_size = 8,
> > > +	.gtt_entry_size_shift = 3,
> > > +	.gmadr_bytes_in_cmd = 8,
> > > +	.mmio_size = MB(2), /* 2MB */
> > > +	.mmio_bar = 0, /* BAR 0 */
> > > +	.max_support_vgpu = 8,
> > 
> > Too much superfluous detail upfront. It is hard to critique when you have no
> > idea what it is for. At the moment, it looks like you are duplicating information
> > we have elsewhere, and I'd rather not. (For starters, it makes GVT a bigger
> > maintainance burden.)
> > 
> OK. Then I think I should remove most them in the first path and add them in the later patch. And do you want me to merge these HW descriptions into i915_device_info? Some of them might not be used by i915 driver itself. That's why I keep a dedicated data structure here.

It's going to be on a case by case basis, but I would rather have hw
descriptions in core.

> > > +/**
> > > + * intel_gvt_destroy_device - destroy a GVT device
> > > + * @gvt_device: gvt device
> > > + *
> > > + * This function is called at the driver unloading stage, to destroy
> > > +a
> > > + * GVT device and free the related resources.
> > > + *
> > > + */
> > > +void intel_gvt_destroy_device(void *device)
> > 
> > This should not be void *. You can forward declare struct intel_gvt such that the
> > core has an opaque pointer.
> > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h index 72f0b02..7d0b8d3 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1703,6 +1703,10 @@ struct i915_workarounds {
> > >  	u32 hw_whitelist_count[I915_NUM_ENGINES];
> > >  };
> > >
> > > +struct i915_gvt {
> > > +	void *gvt;
> > 
> > As before. No void * here.
> > 
> Let me think an approach to remove the "void *" here. How about we do like this:
> struct a{
> 	Struct b;
> };
> 
> Let i915 take only struct b, then in gvt we do container_of() so that we don't need to expose the declaration of struct a to i915? :)

That's one approach. For starters you can just

struct intel_gvt;
struct i915_gvt {
	struct intel_gvt *gvt;
};

But I wouldn't worry about encapsulation too much. You are only ever an
include away from us getting the innermost details of your structs.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 29a32b1..feb56ee 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -57,6 +57,21 @@  config DRM_I915_USERPTR
 
 	  If in doubt, say "Y".
 
+config DRM_I915_GVT
+        bool "Intel GVT-g host driver"
+        depends on DRM_I915
+        default n
+        help
+          Enabling GVT-g mediated graphics pass-through technique for Intel i915
+          based integrated graphics card. With GVT-g, it's possible to have one
+          integrated i915 device shared by multiple VMs. Performance critical
+          operations such as aperture accesses and ring buffer operations
+          are passed-through to VM, with a minimal set of conflicting resources
+          (e.g. display settings) mediated by GVT host driver. The benefit of GVT
+          is on both the performance, given that each VM could directly operate
+          its aperture space and submit commands like running on native, and
+          the feature completeness, given that a true GEN hardware is exposed.
+
 menu "drm/i915 Debugging"
 depends on DRM_I915
 depends on EXPERT
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 63c4d2b..e48145b 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -103,6 +103,11 @@  i915-y += i915_vgpu.o
 # legacy horrors
 i915-y += i915_dma.o
 
+ifeq ($(CONFIG_DRM_I915_GVT),y)
+i915-y += intel_gvt.o
+include $(src)/gvt/Makefile
+endif
+
 obj-$(CONFIG_DRM_I915)  += i915.o
 
 CFLAGS_i915_trace_points.o := -I$(src)
diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
new file mode 100644
index 0000000..d0f21a6
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -0,0 +1,5 @@ 
+GVT_DIR := gvt
+GVT_SOURCE := gvt.o
+
+ccflags-y                      += -I$(src) -I$(src)/$(GVT_DIR) -Wall
+i915-y			       += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
new file mode 100644
index 0000000..5b067d2
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/debug.h
@@ -0,0 +1,36 @@ 
+/*
+ * 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.
+ */
+
+#ifndef __GVT_DEBUG_H__
+#define __GVT_DEBUG_H__
+
+#define gvt_info(fmt, args...) \
+	DRM_INFO("gvt: "fmt, ##args)
+
+#define gvt_err(fmt, args...) \
+	DRM_ERROR("gvt: "fmt, ##args)
+
+#define gvt_dbg_core(fmt, args...) \
+	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
new file mode 100644
index 0000000..aa40357
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -0,0 +1,205 @@ 
+/*
+ * 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.
+ */
+
+#include <linux/types.h>
+#include <xen/xen.h>
+#include <linux/kthread.h>
+
+#include "gvt.h"
+
+struct intel_gvt_host intel_gvt_host;
+
+static const char * const supported_hypervisors[] = {
+	[INTEL_GVT_HYPERVISOR_XEN] = "XEN",
+	[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
+};
+
+#define MB(x) (x * 1024ULL * 1024ULL)
+#define GB(x) (x * MB(1024))
+
+/*
+ * The layout of BAR0 in BDW:
+ * |< - MMIO 2MB ->|<- Reserved 6MB ->|<- MAX GTT 8MB->|
+ *
+ * GTT offset in BAR0 starts from 8MB to 16MB, and
+ * Whatever GTT size is configured in BIOS,
+ * the size of BAR0 is always 16MB. The actual configured
+ * GTT size can be found in GMCH_CTRL.
+ */
+static struct intel_gvt_device_info broadwell_device_info = {
+	.max_gtt_gm_sz = GB(4), /* 4GB */
+	.gtt_start_offset = MB(8), /* 8MB */
+	.max_gtt_size = MB(8), /* 8MB */
+	.gtt_entry_size = 8,
+	.gtt_entry_size_shift = 3,
+	.gmadr_bytes_in_cmd = 8,
+	.mmio_size = MB(2), /* 2MB */
+	.mmio_bar = 0, /* BAR 0 */
+	.max_support_vgpu = 8,
+};
+
+static int init_gvt_host(void)
+{
+	if (WARN(intel_gvt_host.initialized,
+			"Intel GVT host has already been initialized\n"))
+		return -EINVAL;
+
+	/* Xen DOM U */
+	if (xen_domain() && !xen_initial_domain())
+		return -ENODEV;
+
+	if (xen_initial_domain()) {
+		/* Xen Dom0 */
+		intel_gvt_host.mpt = try_then_request_module(
+				symbol_get(xengt_mpt), "xengt");
+		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_XEN;
+	} else {
+		/* not in Xen. Try KVMGT */
+		intel_gvt_host.mpt = try_then_request_module(
+				symbol_get(kvmgt_mpt), "kvm");
+		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_KVM;
+	}
+
+	if (!intel_gvt_host.mpt) {
+		gvt_err("Fail to load any MPT modules.\n");
+		return -EINVAL;
+	}
+
+	if (!intel_gvt_hypervisor_detect_host())
+		return -ENODEV;
+
+	gvt_info("Running with hypervisor %s in host mode\n",
+			supported_hypervisors[intel_gvt_host.hypervisor_type]);
+
+	idr_init(&intel_gvt_host.gvt_idr);
+	mutex_init(&intel_gvt_host.gvt_idr_lock);
+
+	intel_gvt_host.initialized = true;
+	return 0;
+}
+
+static int init_device_info(struct intel_gvt *gvt)
+{
+	if (IS_BROADWELL(gvt->dev_priv))
+		gvt->device_info = &broadwell_device_info;
+	else
+		return -ENODEV;
+
+	return 0;
+}
+
+static void free_gvt_device(struct intel_gvt *gvt)
+{
+	mutex_lock(&intel_gvt_host.gvt_idr_lock);
+	idr_remove(&intel_gvt_host.gvt_idr, gvt->id);
+	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
+
+	vfree(gvt);
+}
+
+static struct intel_gvt *alloc_gvt_device(struct drm_i915_private *dev_priv)
+{
+	struct intel_gvt *gvt;
+	int ret;
+
+	gvt = vzalloc(sizeof(*gvt));
+	if (!gvt)
+		return ERR_PTR(-ENOMEM);
+
+	mutex_lock(&intel_gvt_host.gvt_idr_lock);
+	ret = idr_alloc(&intel_gvt_host.gvt_idr, gvt, 0, 0, GFP_KERNEL);
+	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
+
+	if (ret < 0)
+		goto err;
+
+	gvt->id = ret;
+	mutex_init(&gvt->lock);
+	gvt->dev_priv = dev_priv;
+	idr_init(&gvt->vgpu_idr);
+
+	return gvt;
+err:
+	free_gvt_device(gvt);
+	return ERR_PTR(ret);
+}
+
+/**
+ * intel_gvt_destroy_device - destroy a GVT device
+ * @gvt_device: gvt device
+ *
+ * This function is called at the driver unloading stage, to destroy a
+ * GVT device and free the related resources.
+ *
+ */
+void intel_gvt_destroy_device(void *device)
+{
+	struct intel_gvt *gvt = (struct intel_gvt *)device;
+
+	free_gvt_device(gvt);
+}
+
+/**
+ * intel_gvt_create_device - create a GVT device
+ * @dev_priv: drm i915 private data
+ *
+ * This function is called at the initialization stage, to create a
+ * GVT device and initialize necessary GVT components for it.
+ *
+ * Returns:
+ * pointer to the intel gvt device structure, error pointer if failed.
+ */
+void *intel_gvt_create_device(struct drm_i915_private *dev_priv)
+{
+	struct intel_gvt *gvt;
+	int ret;
+
+	if (!intel_gvt_host.initialized) {
+		ret = init_gvt_host();
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
+	gvt_dbg_core("create new gvt device\n");
+
+	gvt = alloc_gvt_device(dev_priv);
+	if (IS_ERR(gvt)) {
+		ret = PTR_ERR(gvt);
+		goto out_err;
+	}
+
+	gvt_dbg_core("init gvt device, id %d\n", gvt->id);
+
+	ret = init_device_info(gvt);
+	if (ret)
+		goto out_free_gvt_device;
+
+	gvt_dbg_core("gvt device creation done, id %d\n", gvt->id);
+
+	return gvt;
+
+out_free_gvt_device:
+	free_gvt_device(gvt);
+out_err:
+	return ERR_PTR(ret);
+}
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
new file mode 100644
index 0000000..9e6e22b
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -0,0 +1,84 @@ 
+/*
+ * 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.
+ */
+
+#ifndef _GVT_H_
+#define _GVT_H_
+
+#include "i915_drv.h"
+#include "i915_vgpu.h"
+
+#include "debug.h"
+#include "hypercall.h"
+
+#define GVT_MAX_VGPU 8
+#define GVT_ALIGN(addr, size) ((addr) & (~((typeof(addr))(size) - 1)))
+
+enum {
+	INTEL_GVT_HYPERVISOR_XEN = 0,
+	INTEL_GVT_HYPERVISOR_KVM,
+};
+
+struct intel_gvt_host {
+	bool initialized;
+	int hypervisor_type;
+	struct mutex gvt_idr_lock;
+	struct idr gvt_idr;
+	struct intel_gvt_mpt *mpt;
+};
+
+extern struct intel_gvt_host intel_gvt_host;
+
+/* Describe the limitation of HW.*/
+struct intel_gvt_device_info {
+	u64 max_gtt_gm_sz;
+	u32 gtt_start_offset;
+	u32 gtt_end_offset;
+	u32 max_gtt_size;
+	u32 gtt_entry_size;
+	u32 gtt_entry_size_shift;
+	u32 gmadr_bytes_in_cmd;
+	u32 mmio_size;
+	u32 mmio_bar;
+	u32 max_support_vgpu;
+};
+
+struct intel_vgpu {
+	struct intel_gvt *gvt;
+	int id;
+	int vm_id;
+	bool warn_untrack;
+};
+
+struct intel_gvt {
+	struct mutex lock;
+	int id;
+
+	struct drm_i915_private *dev_priv;
+	struct idr vgpu_idr;
+
+	struct intel_gvt_device_info *device_info;
+};
+
+#include "mpt.h"
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
new file mode 100644
index 0000000..254df8b
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -0,0 +1,38 @@ 
+/*
+ * 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.
+ */
+
+#ifndef _GVT_HYPERCALL_H_
+#define _GVT_HYPERCALL_H_
+
+/*
+ * Specific GVT-g MPT modules function collections. Currently GVT-g supports
+ * both Xen and KVM by providing dedicated hypervisor-related MPT modules.
+ */
+struct intel_gvt_mpt {
+	int (*detect_host)(void);
+};
+
+extern struct intel_gvt_mpt xengt_mpt;
+extern struct intel_gvt_mpt kvmgt_mpt;
+
+#endif /* _GVT_HYPERCALL_H_ */
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
new file mode 100644
index 0000000..783f4f8
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -0,0 +1,49 @@ 
+/*
+ * 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.
+ */
+
+#ifndef _GVT_MPT_H_
+#define _GVT_MPT_H_
+
+/**
+ * DOC: Hypervisor Service APIs for GVT-g Core Logic
+ *
+ * This is the glue layer between specific hypervisor MPT modules and GVT-g core
+ * logic. Each kind of hypervisor MPT module provides a collection of function
+ * callbacks via gvt_kernel_dm and will be attached to GVT host when driver
+ * loading. GVT-g core logic will call these APIs to request specific services
+ * from hypervisor.
+ */
+
+/**
+ * intel_gvt_hypervisor_detect_host - check if GVT-g is running within
+ * hypervisor host/privilged domain
+ *
+ * Returns:
+ * Zero on success, -ENODEV if current kernel is running inside a VM
+ */
+static inline int intel_gvt_hypervisor_detect_host(void)
+{
+	return intel_gvt_host.mpt->detect_host();
+}
+
+#endif /* _GVT_MPT_H_ */
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 547100f..502d7cd 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -35,6 +35,7 @@ 
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include "intel_gvt.h"
 #include "i915_vgpu.h"
 #include "i915_trace.h"
 #include <linux/pci.h>
@@ -1245,18 +1246,22 @@  static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 		goto out_ggtt;
 	}
 
+	ret = intel_gvt_init(dev_priv);
+	if (ret)
+		goto out_ggtt;
+
 	/* WARNING: Apparently we must kick fbdev drivers before vgacon,
 	 * otherwise the vga fbdev driver falls over. */
 	ret = i915_kick_out_firmware_fb(dev_priv);
 	if (ret) {
 		DRM_ERROR("failed to remove conflicting framebuffer drivers\n");
-		goto out_ggtt;
+		goto out_gvt;
 	}
 
 	ret = i915_kick_out_vgacon(dev_priv);
 	if (ret) {
 		DRM_ERROR("failed to remove conflicting VGA console\n");
-		goto out_ggtt;
+		goto out_gvt;
 	}
 
 	pci_set_master(dev->pdev);
@@ -1267,7 +1272,7 @@  static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 		if (ret) {
 			DRM_ERROR("failed to set DMA mask\n");
 
-			goto out_ggtt;
+			goto out_gvt;
 		}
 	}
 
@@ -1297,7 +1302,7 @@  static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 				     aperture_size);
 	if (!ggtt->mappable) {
 		ret = -EIO;
-		goto out_ggtt;
+		goto out_gvt;
 	}
 
 	ggtt->mtrr = arch_phys_wc_add(ggtt->mappable_base,
@@ -1330,6 +1335,8 @@  static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 
 	return 0;
 
+out_gvt:
+	intel_gvt_cleanup(dev_priv);
 out_ggtt:
 	i915_ggtt_cleanup_hw(dev);
 
@@ -1488,6 +1495,8 @@  int i915_driver_unload(struct drm_device *dev)
 
 	intel_fbdev_fini(dev);
 
+	intel_gvt_cleanup(dev_priv);
+
 	ret = i915_gem_suspend(dev);
 	if (ret) {
 		DRM_ERROR("failed to idle hardware: %d\n", ret);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 72f0b02..7d0b8d3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1703,6 +1703,10 @@  struct i915_workarounds {
 	u32 hw_whitelist_count[I915_NUM_ENGINES];
 };
 
+struct i915_gvt {
+	void *gvt;
+};
+
 struct i915_virtual_gpu {
 	bool active;
 };
@@ -1742,6 +1746,8 @@  struct drm_i915_private {
 
 	struct i915_virtual_gpu vgpu;
 
+	struct i915_gvt gvt;
+
 	struct intel_guc guc;
 
 	struct intel_csr csr;
@@ -2868,6 +2874,12 @@  void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
 u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
 
 void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
+
+static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
+{
+	return dev_priv->gvt.gvt ? true : false;
+}
+
 static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
 {
 	return dev_priv->vgpu.active;
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
new file mode 100644
index 0000000..815cc9f
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -0,0 +1,98 @@ 
+/*
+ * 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.
+ */
+
+#include "i915_drv.h"
+#include "intel_gvt.h"
+
+/**
+ * DOC: Intel GVT-g host support
+ *
+ * Intel GVT-g is a graphics virtualization technology which shares the
+ * GPU among multiple virtual machines on a time-sharing basis. Each
+ * virtual machine is presented a virtual GPU (vGPU), which has equivalent
+ * features as the underlying physical GPU (pGPU), so i915 driver can run
+ * seamlessly in a virtual machine. This file provides the englightments
+ * of GVT and the necessary components used by GVT in i915 driver.
+ */
+
+struct gvt_kernel_params gvt_kparams = {
+	.enable = false,
+};
+
+/* i915.gvt_enable */
+module_param_named(gvt_enable, gvt_kparams.enable, bool, 0600);
+MODULE_PARM_DESC(gvt_enable, "Enable Intel GVT-g host support");
+
+static bool is_supported_device(struct drm_i915_private *dev_priv)
+{
+	if (IS_BROADWELL(dev_priv))
+		return true;
+	return false;
+}
+
+/**
+ * intel_gvt_init - initialize GVT components
+ * @dev_priv: drm i915 private data
+ *
+ * This function is called at the initialization stage to initialize the
+ * GVT components.
+ */
+int intel_gvt_init(struct drm_i915_private *dev_priv)
+{
+	void *device;
+
+	if (!gvt_kparams.enable) {
+		DRM_DEBUG_DRIVER("GVT-g is disabled by kernel params\n");
+		return 0;
+	}
+
+	if (!is_supported_device(dev_priv)) {
+		DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
+		return 0;
+	}
+
+	device = intel_gvt_create_device(dev_priv);
+	if (IS_ERR(device)) {
+		DRM_DEBUG_DRIVER("GVT-g is disabled\n");
+		return 0;
+	}
+
+	dev_priv->gvt.gvt = device;
+	return 0;
+}
+
+/**
+ * intel_gvt_cleanup - cleanup GVT components when i915 driver is unloading
+ * @dev_priv: drm i915 private *
+ *
+ * This function is called at the i915 driver unloading stage, to shutdown
+ * GVT components and release the related resources.
+ */
+void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
+{
+	if (!intel_gvt_active(dev_priv))
+		return;
+
+	intel_gvt_destroy_device(dev_priv->gvt.gvt);
+	dev_priv->gvt.gvt = NULL;
+}
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
new file mode 100644
index 0000000..8079dfd
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -0,0 +1,49 @@ 
+/*
+ * 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.
+ */
+
+#ifndef _INTEL_GVT_H_
+#define _INTEL_GVT_H_
+
+#ifdef CONFIG_DRM_I915_GVT
+
+#include <drm/i915_gvt.h>
+
+struct gvt_kernel_params {
+	bool enable;
+};
+
+extern struct gvt_kernel_params gvt_kparams;
+
+extern int intel_gvt_init(struct drm_i915_private *dev_priv);
+extern void intel_gvt_cleanup(struct drm_i915_private *dev_priv);
+#else
+static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+static inline void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
+{
+}
+#endif
+
+#endif /* _INTEL_GVT_H_ */
diff --git a/include/drm/i915_gvt.h b/include/drm/i915_gvt.h
new file mode 100644
index 0000000..4ed8b88
--- /dev/null
+++ b/include/drm/i915_gvt.h
@@ -0,0 +1,31 @@ 
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ * 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, sub license, 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.
+ */
+#ifndef _I915_GVT_H
+#define _I915_GVT_H
+
+extern void *intel_gvt_create_device(void *dev_priv);
+extern void intel_gvt_destroy_device(void *gvt);
+
+#endif /* _I915_GVT_H */