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

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> This patch introduces the very basic framework of GVT-g device model,
> includes basic prototypes, definitions, initialization.
> 
> v6:
> - Refine introduction in Kconfig. (Chris)
> - The exposed API functions will take struct intel_gvt * instead of
> void *. (Chris/Tvrtko)
> - Remove most memebers of strct intel_gvt_device_info. Will add them
> in the device model patches.(Chris)
> - Remove gvt_info() and gvt_err() in debug.h. (Chris)
> - Move GVT kernel parameter into i915_params. (Chris)
> - Remove include/drm/i915_gvt.h, as GVT-g will be built within i915.
> - Remove the redundant struct i915_gvt *, as the functions in i915
> will directly take struct intel_gvt *.
> - Add more comments for reviewer.
> 
> v5:
> Take Tvrtko's comments:
> - Fix the misspelled words in Kconfig
> - 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         |  19 ++++
>  drivers/gpu/drm/i915/Makefile        |   5 +
>  drivers/gpu/drm/i915/gvt/Makefile    |   5 +
>  drivers/gpu/drm/i915/gvt/debug.h     |  34 +++++++
>  drivers/gpu/drm/i915/gvt/gvt.c       | 181 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.h       |  75 +++++++++++++++
>  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      |  10 ++
>  drivers/gpu/drm/i915/i915_params.c   |   5 +
>  drivers/gpu/drm/i915/i915_params.h   |   1 +
>  drivers/gpu/drm/i915/intel_gvt.c     |  90 +++++++++++++++++
>  drivers/gpu/drm/i915/intel_gvt.h     |  46 +++++++++
>  14 files changed, 571 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
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 29a32b1..e4fd9da 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -57,6 +57,25 @@ config DRM_I915_USERPTR
>  
>  	  If in doubt, say "Y".
>  
> +config DRM_I915_GVT
> +        bool "Enable Intel GVT-g graphics virtualization host support"
> +        depends on DRM_I915
> +        default n
> +        help
> +	  Choose this option if you want to enable Intel GVT-g graphics
> +	  virtualization technology host support with integrated graphics.
> +	  With GVT-g, it's possible to have one integrated graphics
> +	  device shared by multiple VMs under different hypervisors.
> +
> +	  Note that at least one hypervisor like Xen or KVM is required for
> +	  this driver to work, and it only supports newer device from
> +	  Broadwell+. For further information and setup guide, you can
> +	  visit: http://01.org/zh/igvt-g.

Maybe link should be http://01.org/igvt-g (first result in Google)
which currently redirects to that link, /zh/ would indicate the page to
be in Chinese, so maybe the page ought to be fixed to change the page
language to English in settings.

> +
> +	  The first version is still preliminary. Use it under you own risk.
> +
> +	  If in doubt, say "N".
> +
>  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 7e29444..276abf1 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -104,6 +104,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..7ef412b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/debug.h
> @@ -0,0 +1,34 @@
> +/*
> + * 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_dbg_core(fmt, args...) \
> +	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
> +
> +/*
> + * Other GVT debug stuff will be introduced in the GVT device model patches.
> + */
> +
> +#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..9f1d3e7
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> @@ -0,0 +1,181 @@
> +/*
> + * 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 
> +#include 
> +#include 
> +
> +#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))
> +
> +/* Load MPT modules and detect if we're running in host */
> +static int init_gvt_host(void)
> +{
> +	if (intel_gvt_host.initialized)
> +		return 0;
> +
> +	/* Xen DOM U */
> +	if (xen_domain() && !xen_initial_domain())
> +		return -ENODEV;
> +
> +	/* Try to load MPT modules for hypervisors */
> +	if (xen_initial_domain()) {
> +		/* In 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;
> +	}
> +
> +	/* Fail to load MPT modules - bail out */
> +	if (!intel_gvt_host.mpt)
> +		return -EINVAL;
> +
> +	/* Try to detect if we're running in host instead of VM. */
> +	if (!intel_gvt_hypervisor_detect_host())
> +		return -ENODEV;
> +
> +	gvt_dbg_core("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 void init_device_info(struct intel_gvt *gvt)
> +{
> +	if (IS_BROADWELL(gvt->dev_priv))
> +		gvt->device_info.max_support_vgpus = 8;
> +	/* This function will grow large in GVT device model patches. */
> +}
> +
> +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;
> +
> +	/*
> +	 * This data structure will grow large in future, use vzalloc() at
> +	 * the beginning.
> +	 */
> +	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:

Nack. Free/fini/etc. functions only to be called after successful init.

For example in this case if the ID allocation fails (lets assume we run
out of IDs) gvt->id is not assigned to so 0 and the free_gvt_device
ends up removing ID 0 which could be rather bad?

This is the _exact_ reason why one needs to implement a proper teardown
path.

> +	free_gvt_device(gvt);
> +	return ERR_PTR(ret);
> +}
> +
> +/**
> + * intel_gvt_destroy_device - destroy a GVT device
> + * @gvt: intel 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(struct intel_gvt *gvt)
> +{
> +	/* Another de-initialization of GVT components will be introduced. */
> +	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.
> + *
> + */
> +struct intel_gvt *intel_gvt_create_device(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_gvt *gvt;
> +	int ret;
> +
> +	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);
> +
> +	init_device_info(gvt);
> +	/*
> +	 * Other initialization of GVT components will be called here.
> +	 */
> +	gvt_dbg_core("gvt device creation is done, id %d\n", gvt->id);
> +
> +	return 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..f021487
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -0,0 +1,75 @@
> +/*
> + * 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_pvinfo.h"
> +
> +#include "debug.h"
> +#include "hypercall.h"
> +
> +#define GVT_MAX_VGPU 8
> +#define GVT_ALIGN(addr, size) ((addr) & (~((typeof(addr))(size) - 1)))

There's round_down and round_up and whatnot in linux/kernel.h, please
use those or add a comment what super special this achieves.

> +
> +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 per-platform limitations. */
> +struct intel_gvt_device_info {
> +	u32 max_support_vgpus;
> +	/* This data structure will grow bigger in GVT device model patches */
> +};
> +
> +struct intel_vgpu {
> +	struct intel_gvt *gvt;
> +	int id;
> +	unsigned long handle; /* vGPU handle used by hypervisor MPT modules */
> +};
> +
> +struct intel_gvt {
> +	struct mutex lock;
> +	int id;
> +
> +	struct drm_i915_private *dev_priv;
> +	struct idr vgpu_idr;	/* vGPU IDR pool */
> +
> +	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 07edaed..0baf64a 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 
>  #include "i915_drv.h"
> +#include "intel_gvt.h"
>  #include "i915_vgpu.h"
>  #include "i915_trace.h"
>  #include 
> @@ -1244,18 +1245,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);
> @@ -1266,7 +1271,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;
>  		}
>  	}
>  
> @@ -1296,7 +1301,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,
> @@ -1329,6 +1334,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);
>  
> @@ -1487,6 +1494,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 e4c8e34..bdf499f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -61,6 +61,8 @@
>  #include "i915_gem_gtt.h"
>  #include "i915_gem_render_state.h"
>  
> +#include "intel_gvt.h"
> +
>  /* General customization:
>   */
>  
> @@ -1737,6 +1739,8 @@ struct drm_i915_private {
>  
>  	struct i915_virtual_gpu vgpu;
>  
> +	struct intel_gvt *gvt;

Same criticism as with GPU scheduler, why not data member like all
around?

> +
>  	struct intel_guc guc;
>  
>  	struct intel_csr csr;
> @@ -2938,6 +2942,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 ? 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/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 5e18cf9..c9615ce 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -60,6 +60,7 @@ struct i915_params i915 __read_mostly = {
>  	.enable_dp_mst = true,
>  	.inject_load_failure = 0,
>  	.enable_dpcd_backlight = false,
> +	.enable_gvt = false,
>  };
>  
>  module_param_named(modeset, i915.modeset, int, 0400);
> @@ -222,3 +223,7 @@ MODULE_PARM_DESC(inject_load_failure,
>  module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600);
>  MODULE_PARM_DESC(enable_dpcd_backlight,
>  	"Enable support for DPCD backlight control (default:false)");
> +
> +module_param_named(enable_gvt, i915.enable_gvt, bool, 0600);
> +MODULE_PARM_DESC(enable_gvt,
> +	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 1323261..0ad020b 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -63,6 +63,7 @@ struct i915_params {
>  	bool nuclear_pageflip;
>  	bool enable_dp_mst;
>  	bool enable_dpcd_backlight;
> +	bool enable_gvt;
>  };
>  
>  extern struct i915_params i915 __read_mostly;
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
> new file mode 100644
> index 0000000..8556f58
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -0,0 +1,90 @@
> +/*
> + * 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.
> + */
> +
> +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 create a GVT device.
> + *
> + * Returns:
> + * Zero on success, negative error code if failed.
> + *
> + */
> +int intel_gvt_init(struct drm_i915_private *dev_priv)
> +{
> +	if (!i915.enable_gvt) {
> +		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;
> +	}
> +
> +	dev_priv->gvt = intel_gvt_create_device(dev_priv);
> +	if (IS_ERR(dev_priv->gvt)) {
> +		DRM_DEBUG_DRIVER("GVT-g is disabled\n");
> +		dev_priv->gvt = NULL;
> +		return 0;
> +	}
> +	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);
> +	dev_priv->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..b9b361b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_gvt.h
> @@ -0,0 +1,46 @@
> +/*
> + * 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 "gvt/gvt.h"
> +extern int intel_gvt_init(struct drm_i915_private *dev_priv);
> +extern void intel_gvt_cleanup(struct drm_i915_private *dev_priv);
> +extern struct intel_gvt *intel_gvt_create_device(
> +		struct drm_i915_private *dev_priv);
> +extern void intel_gvt_destroy_device(struct intel_gvt *gvt);
> +#else
> +struct intel_gvt {
> +};

I think this might not be the best idea. I'd just have the structure
declared always, in this header (because we're in intel_gvt.h after
all, what a better place to look?) and have it as a data (not-pointer)
member in the drm_i915_private struct.

Regards, Joonas

> +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_ */
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx