Re: [RFC PATCH 4/4] drm/i915/gvt: move public gvt headers out into global include

From: Julian Stecklina <julian.stecklina@cyberus-technology.de>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: intel-gvt-dev@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	zhiyuan.lv@intel.com, hang.yuan@intel.com,
	Zhenyu Wang <zhenyuw@linux.intel.com>,
	Thomas Prescher <thomas.prescher@cyberus-technology.de>
Subject: Re: [RFC PATCH 4/4] drm/i915/gvt: move public gvt headers out into global include
Date: Thu, 16 Jan 2020 15:13:01 +0100	[thread overview]
Message-ID: <9b32e225ee680e61716e300eb1ed8387599cc0dd.camel@cyberus-technology.de> (raw)
In-Reply-To: <20200115152215.GA3830321@kroah.com>

Hi Greg, Christoph,

On Wed, 2020-01-15 at 16:22 +0100, Greg KH wrote:
> On Thu, Jan 09, 2020 at 07:13:57PM +0200, Julian Stecklina wrote:
> > Now that the GVT interface to hypervisors does not depend on i915/GVT
> > internals anymore, we can move the headers to the global include/.
> > 
> > This makes out-of-tree modules for hypervisor integration possible.
> 
> What kind of out-of-tree modules do you need/want for this?

The mediated virtualization support in the i915 driver needs a backend to the
hypervisor. There is currently one backend for KVM in the tree
(drivers/gpu/drm/i915/gvt/kvmgt.c) and at least 3 other hypervisor backends out
of tree in various states of development that I know of. We are currently
developing one of these.

> 
> Also, as Christoph said, adding exports for functions that are not used
> by anything within the kernel tree itself is not ok, that's not how we
> work.

The exports are used by the KVM hypervisor backend. The patchset I sent
basically decouples KVMGT from i915 driver internals. So personally I would
count this as a benefit in itself.

There is already an indirection in place that looks like it is intended to
decouple the hypervisor backends from the i915 driver core: intel_gvt_ops. This
is a struct of function pointers that the hypervisor backend uses to talk to the
GPU mediator code.

Unfortunately, this struct doesn't cover all usecases and the KVM hypervisor
backend directly touches the i915 devices' internal state in very few places. My
current solution was to wrap these accesses in accessor functions and
EXPORT_SYMBOL_GPL them.

If the more acceptable solution is to add more function pointers to
intel_gvt_ops instead of exporting symbols, I'm happy to go along this route.

> And why do they somehow have to be out of the tree?  We want them in the
> tree, and so should you, as it will save you time and money if they are.

I also want these hypervisor backends in the tree, but from a development
workflow having the ability to build them as a out-of-tree modules is very
convenient. I guess this is also true for the developers working on the other
hypervisor backends.

When I looked at the status quo in i915/gvt a couple of weeks ago, it seemed
like it would be a win for everyone. Let me just clearly say that we have no
intention of doing binary blob drivers. :)

Thanks,
Julian

[1] 
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/gvt/gvt.h#L555