[Intel-gfx] [RFC 01/29] drm/i915/gvt: Introduce the basic architecture of GVT-g
Joonas Lahtinen
joonas.lahtinen at linux.intel.com
Thu Feb 4 11:25:26 UTC 2016
Hi,
On ke, 2016-02-03 at 14:01 +0800, Zhi Wang wrote:
> Hi Joonas:
> Thanks you very much! We're very excited for receiving your advice
> and continue to be open to any comments. :)
>
> I'm supposed that we should make the agreements on i915 host change at
> the very beginning, as I'm concerned during the discussion of i915 host
> change, you know, maybe some code of GVT-g needs to be refined or
> re-designed. So we put the i915 host changes to the beginning of the
> patch set for the convenience of discussion.
>
> I summarize your comments as below, very applicate. :)
>
> - Replace boolean return value with int as much as possible.
> - Replace customized debug ASSERT() function & macros with DRM_DEBUG_*
> - Document all non-static functions like i915
> - Fix all whitespace via scripts/cleanpatch.pl
> - Commit function structure refinement.
> - Change the register access behavior just like what i915 does.
>
> For other comments, see my comments below. :)
>
> On 01/29/16 21:57, Joonas Lahtinen wrote:
> > Hi,
> >
> > TL;DR Overall, we have same problem as with the scheduler series, there
> > is too much placeholder stuff for easy review. Just squash enough code
> > into one commit so it actually does something logical that can be
> > reviewed and then extend it later. Then it can be reviewed and pushed.
> > Just splitting the code down to achieve smaller patches is not the
> > right thing to do.
> >
> > Comments on the overall code: You need to document all header file
> > functions (in the source files), and it is good to document the static
> > functions within a file too, to make future maintenance easier.
> >
> > It is not about splitting the code down to small chunks, but splitting
> > it down to small *logical* chunks. It doesn't make sense to commit
> > dozens of empty bodied functions for review, and then later add their
> > code.
> >
> > If you add functions, only add them at a patch that takes them into use
> > too, unless we're talking about general purpose shared code. And also
> > remember to add the function body and documentation header. If you
> > simply add a "return 0;" or similar function body, do add a comment to
> > why the function does not exist and when it will.
> >
> > Then, there is a trend of having a boolean return values in the code.
> > When possible, it should rather be int and the cause for failure should
> > be propagated from the last level all the way to up (-ENOMEN etc.).
> > This way debugging becomes easier and if new error conditions appear,
> > there is less of a maintenance burden to add the propagation later.
> >
> > Finally, make sure to look at the existing driver parts and
> > https://www.kernel.org/doc/Documentation/CodingStyle
> > for proper coding style. There are lots of whitespace fixes needed in
> > this series, like array initializations.
> >
> > I hope to see this first patch rerolled so that you squash some of the
> > later commits into it so that all functions have a body and you add
> > documentation for the functions so I can both see what it should do and
> > what it actually does. Only reroll the first patch, to keep the
> > iterative step smaller. Lets only then continue with the rest of the
> > series once we've reached a consensus on the formatting and style
> > basics.
> >
> > See more comments below.
> >
> > On to, 2016-01-28 at 18:21 +0800, Zhi Wang wrote:
> > > This patch introduces the very basic framework of GVT-g device model,
> > > includes basic prototypes, definitions, initialization.
> > > ---
> > > arch/x86/include/asm/xen/interface.h | 3 +
> > > drivers/gpu/drm/i915/Kconfig | 16 ++
> > > drivers/gpu/drm/i915/Makefile | 2 +
> > > drivers/gpu/drm/i915/gvt/Makefile | 5 +
> > > drivers/gpu/drm/i915/gvt/debug.h | 72 +++++++
> > > drivers/gpu/drm/i915/gvt/fb_decoder.c | 34 ++++
> > > drivers/gpu/drm/i915/gvt/fb_decoder.h | 110 ++++++++++
> > > drivers/gpu/drm/i915/gvt/gvt.c | 366 ++++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/gvt/gvt.h | 130 ++++++++++++
> > > drivers/gpu/drm/i915/gvt/hypercall.h | 30 +++
> > > drivers/gpu/drm/i915/gvt/mpt.h | 97 +++++++++
> > > drivers/gpu/drm/i915/gvt/params.c | 29 +++
> > > drivers/gpu/drm/i915/gvt/params.h | 34 ++++
> > > drivers/gpu/drm/i915/gvt/reg.h | 34 ++++
> > > drivers/gpu/drm/i915/i915_dma.c | 19 ++
> > > drivers/gpu/drm/i915/i915_drv.h | 6 +
> > > drivers/gpu/drm/i915/i915_vgpu.h | 3 +
> > > include/xen/interface/xen.h | 1 +
> > > 18 files changed, 991 insertions(+)
> > > 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/fb_decoder.c
> > > create mode 100644 drivers/gpu/drm/i915/gvt/fb_decoder.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/gvt/params.c
> > > create mode 100644 drivers/gpu/drm/i915/gvt/params.h
> > > create mode 100644 drivers/gpu/drm/i915/gvt/reg.h
> > >
> > > diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h
> > > index 62ca03e..6ff4986 100644
> > > --- a/arch/x86/include/asm/xen/interface.h
> > > +++ b/arch/x86/include/asm/xen/interface.h
> > > @@ -73,6 +73,9 @@
> > > #endif
> > >
> > > #ifndef __ASSEMBLY__
> > > +
> > > +#include
> > > +
> >
> > I don't follow why this would need to be added if the file is not
> > modified otherwise. Each header should only include what they use.
> >
> > If this is an existing bug (that xen/interface.h can not be included
> > without including linux/types.h), it should be split to a separate
> > patch and sent to Xen team. Same for include/xen/interface/xen.h.
> >
> > > /* Explicitly size integers that represent pfns in the public interface
> > > * with Xen so that on ARM we can have one ABI that works for 32 and 64
> > > * bit guests. */
> > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> > > index 051eab3..89ff723 100644
> > > --- a/drivers/gpu/drm/i915/Kconfig
> > > +++ b/drivers/gpu/drm/i915/Kconfig
> > > @@ -47,3 +47,19 @@ config DRM_I915_PRELIMINARY_HW_SUPPORT
> > > option changes the default for that module option.
> > >
> > > If in doubt, say "N".
> > > +
> > > +config I915_GVT
> > > + tristate "GVT-g host driver"
> > > + depends on DRM_I915
> > > + select IRQ_WORK
> > > + default y
> >
> > Defaulting to "n" would make sense initially.
> >
> [Zhi] Sure, will do.
> > > + help
> > > + Enabling GVT-g mediated graphics passthrough 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
> > > + opterations such as apperture accesses and ring buffer operations
> > > + are pass-throughed to VM, with a minimal set of conflicting resources
> > > + (e.g. display settings) mediated by vGT driver. The benefit of vGT
> > > + 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.
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index 0851de07..d4df410 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -91,6 +91,8 @@ i915-y += dvo_ch7017.o \
> > > intel_sdvo.o \
> > > intel_tv.o
> > >
> > > +obj-$(CONFIG_I915_GVT) += gvt/
> > > +
> > > # virtual gpu code
> > > i915-y += i915_vgpu.o
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
> > > new file mode 100644
> > > index 0000000..6935b78
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/Makefile
> > > @@ -0,0 +1,5 @@
> > > +GVT_SOURCE := gvt.o params.o fb_decoder.o
> > > +
> > > +ccflags-y += -I$(src) -I$(src)/.. -Wall -Werror -Wno-unused-function
> > > +i915_gvt-y := $(GVT_SOURCE)
> > > +obj-$(CONFIG_I915_GVT) += i915_gvt.o
> > > diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
> > > new file mode 100644
> > > index 0000000..18e1467
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/debug.h
> > > @@ -0,0 +1,72 @@
> > > +/*
> > > + * 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 ASSERT(x) \
> > > + do { \
> > > + if (!(x)) { \
> > > + printk("Assert at %s line %d\n", \
> > > + __FILE__, __LINE__); \
> > > + } \
> > > + } while (0);
> > > +
> > > +#define ASSERT_NUM(x, y) \
> > > + do { \
> > > + if (!(x)) { \
> > > + printk("Assert at %s line %d para 0x%llx\n", \
> > > + __FILE__, __LINE__, (u64)y); \
> > > + } \
> > > + } while (0);
> > > +
> >
> > There already is WARN_ON (and i915_drv.h modifies it a little). Do not
> > introduce custom functions like this, if the existing ones need
> > improvement, improve them.
> >
> [Zhi] OK. Will do as what i915 does
> > > +#define gvt_info(fmt, args...) \
> > > + printk(KERN_INFO"[GVT-g] "fmt"\n", ##args)
> > > +
> > > +#define gvt_err(fmt, args...) \
> > > + printk(KERN_ERR"%s() - %d: "fmt"\n", __func__, __LINE__, ##args)
> > > +
> > > +#define gvt_warn(fmt, args...) \
> > > + printk(KERN_WARNING"%s() - %d: "fmt"\n", __func__, __LINE__, ##args)
> > > +
> > > +#define gvt_dbg(level, fmt, args...) do { \
> > > + if (gvt.debug & level) \
> > > + printk(KERN_DEBUG"%s() - %d: "fmt"\n", __func__, __LINE__, ##args); \
> > > + }while(0)
> > > +
> > > +enum {
> > > + GVT_DBG_CORE = (1 << 0),
> > > + GVT_DBG_MM = (1 << 1),
> > > + GVT_DBG_IRQ = (1 << 2),
> > > +};
> > > +
> > > +#define gvt_dbg_core(fmt, args...) \
> > > + gvt_dbg(GVT_DBG_CORE, fmt, ##args)
> > > +
> > > +#define gvt_dbg_mm(fmt, args...) \
> > > + gvt_dbg(GVT_DBG_MM, fmt, ##args)
> > > +
> > > +#define gvt_dbg_irq(fmt, args...) \
> > > + gvt_dbg(GVT_DBG_IRQ, fmt, ##args)
> > > +
> > > +#endif
> >
> > This should be integrated better to DRM debugging options, custom
> > debugging code only for i915 was rejected a while ago.
> >
> [ZHI] Thanks for sharing the history. :) Will change that.
> > > diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.c b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > new file mode 100644
> > > index 0000000..a219819
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.c
> > > @@ -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.
> > > + */
> > > +
> > > +#include "gvt.h"
> > > +
> > > +int gvt_decode_fb_format(struct pgt_device *pdev, int vmid, struct gvt_fb_format *fb)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +int gvt_fb_notifier_call_chain(unsigned long val, void *data)
> > > +{
> > > + return 0;
> > > +}
> >
> > Kerneldoc missing for these functions. It is all the same to squash
> > later patches to introduce the code to these functions already,
> > reviewing utility functions with no kerneldoc and no body makes it
> > somewhat difficult to see the big picture.
> >
>
> [Zhi] One question. I saw i915 put some function instruction in *.c. Is
> it also OK for kernel doc generating the proper doc files?
Yes, (again referring to history) there was a suggestion to move all
the documentation to header files and it was decided on that the
documentation should RATHER be in the implementation than header files.
Then it's closer to the implementation when on is changing the function
and one has a higher chance of updating it, rather than switching
between header and implementation.
>
> > > diff --git a/drivers/gpu/drm/i915/gvt/fb_decoder.h b/drivers/gpu/drm/i915/gvt/fb_decoder.h
> > > new file mode 100644
> > > index 0000000..2c29ed4
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/fb_decoder.h
> > > @@ -0,0 +1,110 @@
> > > +/*
> > > + * 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_FB_DECODER_H_
> > > +#define _GVT_FB_DECODER_H_
> > > +
> > > +typedef enum {
> > > + FB_MODE_SET_START = 1,
> > > + FB_MODE_SET_END,
> > > + FB_DISPLAY_FLIP,
> > > +}gvt_fb_event_t;
> > > +
> > > +typedef enum {
> > > + DDI_PORT_NONE = 0,
> > > + DDI_PORT_B = 1,
> > > + DDI_PORT_C = 2,
> > > + DDI_PORT_D = 3,
> > > + DDI_PORT_E = 4
> > > +} ddi_port_t;
> > > +
> > > +struct pgt_device;
> > > +
> > > +struct gvt_fb_notify_msg {
> > > + unsigned vm_id;
> > > + unsigned pipe_id; /* id starting from 0 */
> > > + unsigned plane_id; /* primary, cursor, or sprite */
> > > +};
> > > +
> > > +/* color space conversion and gamma correction are not included */
> > > +struct gvt_primary_plane_format {
> > > + u8 enabled; /* plane is enabled */
> > > + u8 tiled; /* X-tiled */
> > > + u8 bpp; /* bits per pixel */
> > > + u32 hw_format; /* format field in the PRI_CTL register */
> > > + u32 drm_format; /* format in DRM definition */
> > > + u32 base; /* framebuffer base in graphics memory */
> > > + u32 x_offset; /* in pixels */
> > > + u32 y_offset; /* in lines */
> > > + u32 width; /* in pixels */
> > > + u32 height; /* in lines */
> > > + u32 stride; /* in bytes */
> > > +};
> > > +
> > > +struct gvt_sprite_plane_format {
> > > + u8 enabled; /* plane is enabled */
> > > + u8 tiled; /* X-tiled */
> > > + u8 bpp; /* bits per pixel */
> > > + u32 hw_format; /* format field in the SPR_CTL register */
> > > + u32 drm_format; /* format in DRM definition */
> > > + u32 base; /* sprite base in graphics memory */
> > > + u32 x_pos; /* in pixels */
> > > + u32 y_pos; /* in lines */
> > > + u32 x_offset; /* in pixels */
> > > + u32 y_offset; /* in lines */
> > > + u32 width; /* in pixels */
> > > + u32 height; /* in lines */
> > > +};
> > > +
> > > +struct gvt_cursor_plane_format {
> > > + u8 enabled;
> > > + u8 mode; /* cursor mode select */
> > > + u8 bpp; /* bits per pixel */
> > > + u32 drm_format; /* format in DRM definition */
> > > + u32 base; /* cursor base in graphics memory */
> > > + u32 x_pos; /* in pixels */
> > > + u32 y_pos; /* in lines */
> > > + u8 x_sign; /* X Position Sign */
> > > + u8 y_sign; /* Y Position Sign */
> > > + u32 width; /* in pixels */
> > > + u32 height; /* in lines */
> > > + u32 x_hot; /* in pixels */
> > > + u32 y_hot; /* in pixels */
> > > +};
> > > +
> >
> > The above structs have a lot in common, would it make sense to have the
> > common members + plane type and then union for the plane type specific
> > data. I suspect having it all split this way will lead to more utility
> > functions somewhere.
> >
> [Zhi] Thanks. Will do that.
>
> > > +struct gvt_pipe_format {
> > > + struct gvt_primary_plane_format primary;
> > > + struct gvt_sprite_plane_format sprite;
> > > + struct gvt_cursor_plane_format cursor;
> > > + ddi_port_t ddi_port; /* the DDI port that the pipe is connected to */
> > > +};
> > > +
> > > +struct gvt_fb_format{
> > > + struct gvt_pipe_format pipes[I915_MAX_PIPES];
> > > +};
> > > +
> > > +extern int gvt_fb_notifier_call_chain(unsigned long val, void *data);
> > > +extern int gvt_decode_fb_format(struct pgt_device *pdev, int vmid,
> > > + struct gvt_fb_format *fb);
> > > +
> > > +#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..041d10f
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/gvt.c
> > > @@ -0,0 +1,366 @@
> > > +/*
> > > + * 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 "gvt.h"
> > > +
> > > +struct gvt_host gvt_host;
> > > +
> > > +extern struct gvt_kernel_dm xengt_kdm;
> > > +extern struct gvt_kernel_dm kvmgt_kdm;
> > > +
> > > +static const char *supported_hypervisors[] = {
> >
> > should be "static const char * const".
> >
> > > + [GVT_HYPERVISOR_TYPE_XEN] = "Xen Hypervisor",
> > > + [GVT_HYPERVISOR_TYPE_KVM] = "KVM",
> > > +};
> > > +
> > > +static bool gvt_init_host(void)
> > > +{
> > > + struct gvt_host *host = &gvt_host;
> > > +
> > > + if (!gvt.enable) {
> > > + gvt_dbg_core("GVT-g has been disabled by kernel parameter");
> > > + return false;
> > > + }
> > > +
> > > + if (host->initialized) {
> > > + gvt_err("GVT-g has already been initialized!");
> > > + return false;
> > > + }
> > > +
> > > + if (xen_initial_domain()) {
> >
> > Shouldn't Xen code be CONFIG_XEN_DOM0 and CONFIG_XEN #ifdef protected?
> >
> [Zhi] The following code piece shows xen_initial_domain()/xen_domain()
> could also be used when CONFIG_XEN_DOM0 is not set.
>
> #ifdef CONFIG_XEN
> extern enum xen_domain_type xen_domain_type;
> #else
> #define xen_domain_type XEN_NATIVE
> #endif
>
Ok, then it's cool. I missed this construct :)
> #define xen_domain() (xen_domain_type != XEN_NATIVE)
> #define xen_pv_domain() (xen_domain() && \
> xen_domain_type == XEN_PV_DOMAIN)
> #define xen_hvm_domain() (xen_domain() && \
> xen_domain_type == XEN_HVM_DOMAIN)
>
> #ifdef CONFIG_XEN_DOM0
> #include
> #include
>
> #define xen_initial_domain() (xen_domain() && \
> xen_start_info &&
> xen_start_info->flags & SIF_INITDOMAIN)
> #else /* !CONFIG_XEN_DOM0 */
> #define xen_initial_domain() (0)
> #endif /* CONFIG_XEN_DOM0 */
>
> > > + /* Xen Dom0 */
> > > + host->kdm = try_then_request_module(symbol_get(xengt_kdm), "xengt");
> > > + host->hypervisor_type = GVT_HYPERVISOR_TYPE_XEN;
> > > + } else if(xen_domain()) {
> > > + /* Xen DomU */
> > > + return false;
> > > + } else {
> > > + /* not in Xen. Try KVMGT */
> > > + host->kdm = try_then_request_module(symbol_get(kvmgt_kdm), "kvm");
> > > + host->hypervisor_type = GVT_HYPERVISOR_TYPE_KVM;
> > > + }
> > > +
> >
> > Why do we need to keep track of hypervisor type, are there plenty of
> > hypervisor specific behaviour differences?
>
> [Zhi] As GVT-g needs some hypervisor services to work, we write a
> abstraction layer to connect GVT-g to different hypervisor. But still
> there are some emulation logic couldn't be fitted into that abstraction
> layer, like OpRegion emulation. In Xen, we emulates it in kernel space,
> while we emulates it in Qemu under KVM. I also agreed that we should
> find some approach to improve it just like what you said.
>
Yep, if correcting it is not currently possible, I can live with it.
But host->hypervisor_type dependent code should be kept to absolute
minimum.
Regards, Joonas
> > If it is just for printing
> > the below debug, I think it's better to move the debug into above code
> > that detects the hypervisor, wrap them with appropriate CONFIG #ifdefs.
> >
> > > + if (!host->kdm)
> > > + return false;
> > > +
> > > + if (!hypervisor_detect_host())
> > > + return false;
> > > +
> > > + gvt_info("Running with hypervisor %s in host mode",
> > > + supported_hypervisors[host->hypervisor_type]);
> > > +
> > > + idr_init(&host->device_idr);
> > > + mutex_init(&host->device_idr_lock);
> > > +
> > > + host->initialized = true;
> > > + return true;
> > > +}
> > > +
> > > +static bool init_device_info(struct pgt_device *pdev)
> > > +{
> > > + struct gvt_device_info *info = &pdev->device_info;
> > > +
> > > + if (!IS_BROADWELL(pdev->dev_priv)) {
> > > + gvt_err("Unsupported GEN device");
> > > + return false;
> > > + }
> > > +
> > > + if (IS_BROADWELL(pdev->dev_priv)) {
> > > + info->max_gtt_gm_sz = (1UL << 32);
> > > + /*
> > > + * 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.
> > > + */
> > > + info->gtt_start_offset = (1UL << 23);
> > > + info->max_gtt_size = (1UL << 23);
> > > + info->gtt_entry_size = 8;
> > > + info->gtt_entry_size_shift = 3;
> > > + info->gmadr_bytes_in_cmd = 8;
> >
> > Would this information be useful in a header as #defines too?
> >
> [Zhi] Probably. Consider that one GVT-g featured kernel could be able to
> run on different platforms, the informaion data structure could be
> different. But I agree to define the magic numbers in a header files. :)
>
> > > + }
> > > +
> > > + gvt_info("Device info:");
> > > + printk(" max_gtt_gm_sz: %llx\n", info->max_gtt_gm_sz);
> > > + printk(" max_gtt_size: %x\n", info->max_gtt_size);
> > > + printk(" gtt_size_entry: %x\n", info->gtt_entry_size);
> > > + printk(" gtt_entry_size_shift: %x\n", info->gtt_entry_size_shift);
> > > + printk(" gtt_start_offset: %x\n", info->gtt_start_offset);
> > > + printk(" gtt_end_offset: %x\n", info->gtt_end_offset);
> > > +
> >
> > Just put this to kind of stuff to i915_debugfs.c.
> >
> [Zhi] Thanks.
>
> > > + return true;
> > > +}
> > > +
> > > +static void init_initial_cfg_space_state(struct pgt_device *pdev)
> > > +{
> > > + struct pci_dev *pci_dev = pdev->dev_priv->dev->pdev;
> > > + int i;
> > > +
> > > + gvt_dbg_core("init initial cfg space, id %d", pdev->id);
> > > +
> > > + for (i = 0; i < GVT_CFG_SPACE_SZ; i += 4)
> >
> > += sizeof(...)
> >
> > > + pci_read_config_dword(pci_dev, i,
> > > + (u32 *)&pdev->initial_cfg_space[i]);
> > > +
> > > + for (i = 0; i < 3; i++) {
> >
> > No magic numbers make a #define for 3 and give it a descriptive name.
> >
> > > + pdev->bar_size[i] = pci_resource_len(pci_dev, i * 2);
> > > + gvt_info("bar %d size: %llx", i, pdev->bar_size[i]);
> > > + }
> > > +}
> > > +
> > > +static void clean_initial_mmio_state(struct pgt_device *pdev)
> > > +{
> > > + if (pdev->gttmmio_va) {
> > > + iounmap(pdev->gttmmio_va);
> > > + pdev->gttmmio_va = NULL;
> > > + }
> > > +
> > > + if (pdev->gmadr_va) {
> > > + iounmap(pdev->gmadr_va);
> > > + pdev->gmadr_va = NULL;
> > > + }
> > > +}
> > > +
> > > +static bool init_initial_mmio_state(struct pgt_device *pdev)
> > > +{
> > > + u64 bar0, bar1;
> > > +
> > > + gvt_dbg_core("init initial mmio state, id %d", pdev->id);
> > > +
> > > + bar0 = *(u64 *)&pdev->initial_cfg_space[GVT_REG_CFG_SPACE_BAR0];
> > > + bar1 = *(u64 *)&pdev->initial_cfg_space[GVT_REG_CFG_SPACE_BAR1];
> > > +
> > > + pdev->gttmmio_base = bar0 & ~0xf;
> > > + pdev->mmio_size = 2 * 1024 * 1024;
> > > + pdev->reg_num = pdev->mmio_size / 4;
> > > + pdev->gmadr_base = bar1 & ~0xf;
> > > +
> >
> > Many magic numbers.
> >
> > > + pdev->gttmmio_va = ioremap(pdev->gttmmio_base, pdev->bar_size[0]);
> > > + if (!pdev->gttmmio_va) {
> > > + gvt_err("fail to map GTTMMIO BAR.");
> >
> > These should be if(WARN_ON(...))
> >
> > > + return false;
> > > + }
> > > +
> > > + pdev->gmadr_va = ioremap(pdev->gmadr_base, pdev->bar_size[2]);
> > > + if (!pdev->gmadr_va) {
> > > + gvt_err("fail to map GMADR BAR.");
> > > + goto err;
> > > + }
> > > +
> > > + gvt_info("bar0: 0x%llx, bar1: 0x%llx", bar0, bar1);
> > > + gvt_info("mmio size: %x", pdev->mmio_size);
> > > + gvt_info("gttmmio: 0x%llx, gmadr: 0x%llx", pdev->gttmmio_base, pdev->gmadr_base);
> > > + gvt_info("gttmmio_va: %p", pdev->gttmmio_va);
> > > + gvt_info("gmadr_va: %p", pdev->gmadr_va);
> > > +
> > > + return true;
> > > +err:
> > > + clean_initial_mmio_state(pdev);
> > > + return false;
> > > +}
> > > +
> > > +static int gvt_service_thread(void *data)
> > > +{
> > > + struct pgt_device *pdev = (struct pgt_device *)data;
> > > + int r;
> > > +
> > > + gvt_dbg_core("service thread start, pgt %d", pdev->id);
> > > +
> > > + while(!kthread_should_stop()) {
> > > + r = wait_event_interruptible(pdev->service_thread_wq,
> > > + kthread_should_stop() || pdev->service_request);
> > > +
> > > + if (kthread_should_stop())
> > > + break;
> > > +
> > > + if (r) {
> > > + gvt_warn("service thread is waken up by unexpected signal.");
> >
> > Should be WARN_ONCE, to avoid future disasters with CI.
> >
> [Zhi] Thanks.
> > > + continue;
> > > + }
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +static void clean_service_thread(struct pgt_device *pdev)
> > > +{
> > > + if (pdev->service_thread) {
> > > + kthread_stop(pdev->service_thread);
> > > + pdev->service_thread = NULL;
> > > + }
> > > +}
> > > +
> > > +static bool init_service_thread(struct pgt_device *pdev)
> > > +{
> > > + init_waitqueue_head(&pdev->service_thread_wq);
> > > +
> > > + pdev->service_thread = kthread_run(gvt_service_thread,
> > > + pdev, "gvt_service_thread%d", pdev->id);
> > > +
> > > + if (!pdev->service_thread) {
> > > + gvt_err("fail to start service thread.");
> > > + return false;
> > > + }
> > > +
> > > + return true;
> > > +}
> > > +
> > > +static void clean_pgt_device(struct pgt_device *pdev)
> > > +{
> > > + clean_service_thread(pdev);
> > > + clean_initial_mmio_state(pdev);
> > > +}
> > > +
> > > +static bool init_pgt_device(struct pgt_device *pdev, struct drm_i915_private *dev_priv)
> > > +{
> > > + if (!init_device_info(pdev))
> > > + return false;
> > > +
> > > + init_initial_cfg_space_state(pdev);
> > > +
> > > + if (!init_initial_mmio_state(pdev))
> > > + goto err;
> > > +
> > > + if (!init_service_thread(pdev))
> > > + goto err;
> > > +
> > > + return true;
> > > +err:
> > > + clean_pgt_device(pdev);
> > > + return false;
> > > +}
> > > +
> > > +static bool post_init_pgt_device(struct pgt_device *pdev)
> > > +{
> > > + return true;
> > > +}
> > > +
> > > +static void free_pgt_device(struct pgt_device *pdev)
> > > +{
> > > + struct gvt_host *host = &gvt_host;
> > > +
> > > + mutex_lock(&host->device_idr_lock);
> > > + idr_remove(&host->device_idr, pdev->id);
> > > + mutex_unlock(&host->device_idr_lock);
> > > +
> > > + vfree(pdev);
> > > +}
> > > +
> > > +static struct pgt_device *alloc_pgt_device(struct drm_i915_private *dev_priv)
> > > +{
> > > + struct gvt_host *host = &gvt_host;
> > > + struct pgt_device *pdev = NULL;
> > > +
> > > + pdev = vzalloc(sizeof(*pdev));
> > > + if (!pdev) {
> > > + gvt_err("fail to allocate memory for pgt device.");
> > > + return NULL;
> > > + }
> > > +
> > > + mutex_lock(&host->device_idr_lock);
> > > + pdev->id = idr_alloc(&host->device_idr, pdev, 0, 0, GFP_KERNEL);
> > > + mutex_unlock(&host->device_idr_lock);
> > > +
> > > + if (pdev->id < 0) {
> > > + gvt_err("fail to allocate pgt device id.");
> > > + goto err;
> > > + }
> > > +
> > > + mutex_init(&pdev->lock);
> > > + pdev->dev_priv = dev_priv;
> > > + idr_init(&pdev->instance_idr);
> > > +
> > > + return pdev;
> > > +err:
> > > + free_pgt_device(pdev);
> > > + return NULL;
> > > +}
> > > +
> > > +void gvt_destroy_pgt_device(void *private_data)
> > > +{
> > > + struct pgt_device *pdev = (struct pgt_device *)private_data;
> > > +
> > > + clean_pgt_device(pdev);
> > > + free_pgt_device(pdev);
> > > +}
> > > +
> > > +void *gvt_create_pgt_device(struct drm_i915_private *dev_priv)
> > > +{
> > > + struct pgt_device *pdev = NULL;
> > > + struct gvt_host *host = &gvt_host;
> > > +
> > > + if (!host->initialized && !gvt_init_host()) {
> > > + gvt_err("gvt_init_host fail");
> > > + return NULL;
> > > + }
> > > +
> > > + gvt_dbg_core("create new pgt device, i915 dev_priv: %p", dev_priv);
> > > +
> > > + pdev = alloc_pgt_device(dev_priv);
> > > + if (!pdev) {
> > > + gvt_err("fail to allocate memory for pgt device.");
> > > + goto err;
> > > + }
> > > +
> > > + gvt_dbg_core("init pgt device, id %d", pdev->id);
> > > +
> > > + if (!init_pgt_device(pdev, dev_priv)) {
> > > + gvt_err("fail to init physical device state.");
> > > + goto err;
> > > + }
> > > +
> > > + gvt_dbg_core("pgt device creation done, id %d", pdev->id);
> > > +
> > > + return pdev;
> > > +err:
> > > + if (pdev) {
> > > + gvt_destroy_pgt_device(pdev);
> > > + pdev = NULL;
> > > + }
> > > + return NULL;
> > > +}
> > > +
> > > +bool gvt_post_init_pgt_device(void *private_data)
> > > +{
> > > + struct pgt_device *pdev = (struct pgt_device *)private_data;
> > > + struct gvt_host *host = &gvt_host;
> > > +
> > > + if (!host->initialized) {
> > > + gvt_err("gvt_host haven't been initialized.");
> > > + return false;
> > > + }
> > > +
> > > + gvt_dbg_core("post init pgt device %d", pdev->id);
> > > +
> > > + if (!post_init_pgt_device(pdev)) {
> > > + gvt_err("fail to post init physical device state.");
> > > + return false;
> > > + }
> > > +
> > > + return true;
> > > +}
> > > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > > new file mode 100644
> > > index 0000000..6c85bba
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > > @@ -0,0 +1,130 @@
> > > +/*
> > > + * 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 "params.h"
> > > +#include "reg.h"
> > > +#include "hypercall.h"
> > > +#include "mpt.h"
> > > +#include "fb_decoder.h"
> > > +
> > > +#define GVT_MAX_VGPU 8
> > > +
> > > +enum {
> > > + GVT_HYPERVISOR_TYPE_XEN = 0,
> > > + GVT_HYPERVISOR_TYPE_KVM,
> > > +};
> > > +
> > > +struct gvt_host {
> > > + bool initialized;
> > > + int hypervisor_type;
> > > + struct mutex device_idr_lock;
> > > + struct idr device_idr;
> > > + struct gvt_kernel_dm *kdm;
> > > +};
> > > +
> > > +extern struct gvt_host gvt_host;
> > > +
> > > +/* Describe the limitation of HW.*/
> > > +struct 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;
> > > +};
> > > +
> > > +struct vgt_device {
> > > + int id;
> > > + int vm_id;
> > > + struct pgt_device *pdev;
> > > + bool warn_untrack;
> > > +};
> > > +
> > > +struct pgt_device {
> > > + struct mutex lock;
> > > + int id;
> > > +
> > > + struct drm_i915_private *dev_priv;
> > > + struct idr instance_idr;
> > > +
> > > + struct gvt_device_info device_info;
> > > +
> > > + u8 initial_cfg_space[GVT_CFG_SPACE_SZ];
> > > + u64 bar_size[GVT_BAR_NUM];
> > > +
> > > + u64 gttmmio_base;
> > > + void *gttmmio_va;
> > > +
> > > + u64 gmadr_base;
> > > + void *gmadr_va;
> > > +
> > > + u32 mmio_size;
> > > + u32 reg_num;
> > > +
> > > + wait_queue_head_t service_thread_wq;
> > > + struct task_struct *service_thread;
> > > + unsigned long service_request;
> > > +};
> > > +
> >
> > Here-->
> >
> > > +static inline u32 gvt_mmio_read(struct pgt_device *pdev,
> > > + u32 reg)
> > > +{
> > > + struct drm_i915_private *dev_priv = pdev->dev_priv;
> > > + i915_reg_t tmp = {.reg = reg};
> > > + return I915_READ(tmp);
> > > +}
> > > +
> > > +static inline void gvt_mmio_write(struct pgt_device *pdev,
> > > + u32 reg, u32 val)
> > > +{
> > > + struct drm_i915_private *dev_priv = pdev->dev_priv;
> > > + i915_reg_t tmp = {.reg = reg};
> > > + I915_WRITE(tmp, val);
> > > +}
> > > +
> > > +static inline u64 gvt_mmio_read64(struct pgt_device *pdev,
> > > + u32 reg)
> > > +{
> > > + struct drm_i915_private *dev_priv = pdev->dev_priv;
> > > + i915_reg_t tmp = {.reg = reg};
> > > + return I915_READ64(tmp);
> > > +}
> > > +
> > > +static inline void gvt_mmio_write64(struct pgt_device *pdev,
> > > + u32 reg, u64 val)
> > > +{
> > > + struct drm_i915_private *dev_priv = pdev->dev_priv;
> > > + i915_reg_t tmp = {.reg = reg};
> > > + I915_WRITE64(tmp, val);
> > > +}
> > > +
> >
> > <-- Why? The i915_reg_t type was added to avoid problems, this code is
> > not used anywhere, and it has no documentation, so I can not review it.
> >
> > I wrote comments at the top of the post.
> >
> [Zhi] Thanks, I will check that.
>
> > Regards, Joonas
> >
> > > +#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..0a41874
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/hypercall.h
> > > @@ -0,0 +1,30 @@
> > > +/*
> > > + * 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_
> > > +
> > > +struct gvt_kernel_dm {
> > > +};
> > > +
> > > +#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..bbe4465
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/mpt.h
> > > @@ -0,0 +1,97 @@
> > > +/*
> > > + * 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_
> > > +
> > > +struct vgt_device;
> > > +
> > > +static inline unsigned long hypervisor_g2m_pfn(struct vgt_device *vgt,
> > > + unsigned long g_pfn)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline int hypervisor_pause_domain(struct vgt_device *vgt)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline int hypervisor_shutdown_domain(struct vgt_device *vgt)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline int hypervisor_set_trap_area(struct vgt_device *vgt,
> > > + uint64_t start, uint64_t end, bool map)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline bool hypervisor_detect_host(void)
> > > +{
> > > + return false;
> > > +}
> > > +
> > > +static inline int hypervisor_virt_to_mfn(void *addr)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline void *hypervisor_mfn_to_virt(int mfn)
> > > +{
> > > + return NULL;
> > > +}
> > > +
> > > +static inline void hypervisor_inject_msi(struct vgt_device *vgt)
> > > +{
> > > + return;
> > > +}
> > > +
> > > +static inline int hypervisor_hvm_init(struct vgt_device *vgt)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline void hypervisor_hvm_exit(struct vgt_device *vgt)
> > > +{
> > > +}
> > > +
> > > +static inline void *hypervisor_gpa_to_va(struct vgt_device *vgt, unsigned long gpa)
> > > +{
> > > + return NULL;
> > > +}
> > > +
> > > +static inline bool hypervisor_read_va(struct vgt_device *vgt, void *va,
> > > + void *val, int len, int atomic)
> > > +{
> > > + return false;
> > > +}
> > > +
> > > +static inline bool hypervisor_write_va(struct vgt_device *vgt, void *va,
> > > + void *val, int len, int atomic)
> > > +{
> > > + return false;
> > > +}
> > > +
> > > +#endif /* _GVT_MPT_H_ */
> > > diff --git a/drivers/gpu/drm/i915/gvt/params.c b/drivers/gpu/drm/i915/gvt/params.c
> > > new file mode 100644
> > > index 0000000..dfc33c3
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/params.c
> > > @@ -0,0 +1,29 @@
> > > +/*
> > > + * 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 "gvt.h"
> > > +
> > > +struct gvt_kernel_params gvt = {
> > > + .enable = true,
> > > + .debug = 0,
> > > +};
> > > diff --git a/drivers/gpu/drm/i915/gvt/params.h b/drivers/gpu/drm/i915/gvt/params.h
> > > new file mode 100644
> > > index 0000000..d2955b9
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/params.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_PARAMS_H_
> > > +#define _GVT_PARAMS_H_
> > > +
> > > +struct gvt_kernel_params {
> > > + bool enable;
> > > + int debug;
> > > +};
> > > +
> > > +extern struct gvt_kernel_params gvt;
> > > +
> > > +#endif
> > > diff --git a/drivers/gpu/drm/i915/gvt/reg.h b/drivers/gpu/drm/i915/gvt/reg.h
> > > new file mode 100644
> > > index 0000000..d363b74
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/i915/gvt/reg.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_REG_H
> > > +#define _GVT_REG_H
> > > +
> > > +#define GVT_CFG_SPACE_SZ 256
> > > +#define GVT_BAR_NUM 4
> > > +
> > > +#define GVT_REG_CFG_SPACE_BAR0 0x10
> > > +#define GVT_REG_CFG_SPACE_BAR1 0x18
> > > +#define GVT_REG_CFG_SPACE_BAR2 0x20
> > > +
> > > +#endif
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > > index 4725e8d..eca8e50 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -943,6 +943,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> > >
> > > intel_uncore_init(dev);
> > >
> > > + dev_priv->vgpu.host_private_data = gvt_create_pgt_device(dev_priv);
> > > + if(intel_gvt_host_active(dev))
> > > + DRM_INFO("GVT-g is running in host mode\n");
> > > +
> > > ret = i915_gem_gtt_init(dev);
> > > if (ret)
> > > goto out_freecsr;
> > > @@ -1067,6 +1071,13 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> > > goto out_power_well;
> > > }
> > >
> > > + if (intel_gvt_host_active(dev)) {
> > > + if (!gvt_post_init_pgt_device(dev_priv->vgpu.host_private_data)) {
> > > + DRM_ERROR("failed to post init pgt device\n");
> > > + goto out_power_well;
> > > + }
> > > + }
> > > +
> > > /*
> > > * Notify a valid surface after modesetting,
> > > * when running inside a VM.
> > > @@ -1117,6 +1128,10 @@ out_gtt:
> > > i915_global_gtt_cleanup(dev);
> > > out_freecsr:
> > > intel_csr_ucode_fini(dev_priv);
> > > + if (intel_gvt_host_active(dev)) {
> > > + gvt_destroy_pgt_device(dev_priv->vgpu.host_private_data);
> > > + dev_priv->vgpu.host_private_data = NULL;
> > > + }
> > > intel_uncore_fini(dev);
> > > pci_iounmap(dev->pdev, dev_priv->regs);
> > > put_bridge:
> > > @@ -1165,6 +1180,10 @@ int i915_driver_unload(struct drm_device *dev)
> > >
> > > intel_modeset_cleanup(dev);
> > >
> > > + if (intel_gvt_host_active(dev)) {
> > > + gvt_destroy_pgt_device(dev_priv->vgpu.host_private_data);
> > > + dev_priv->vgpu.host_private_data = NULL;
> > > + }
> > > /*
> > > * free the memory space allocated for the child device
> > > * config parsed from VBT
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 01cc982..db3c79b 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1673,6 +1673,7 @@ struct i915_workarounds {
> > >
> > > struct i915_virtual_gpu {
> > > bool active;
> > > + void *host_private_data;
> > > };
> > >
> > > struct i915_execbuffer_params {
> > > @@ -2747,6 +2748,11 @@ static inline bool intel_vgpu_active(struct drm_device *dev)
> > > return to_i915(dev)->vgpu.active;
> > > }
> > >
> > > +static inline bool intel_gvt_host_active(struct drm_device *dev)
> > > +{
> > > + return to_i915(dev)->vgpu.host_private_data ? true : false;
> > > +}
> > > +
> > > void
> > > i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe,
> > > u32 status_mask);
> > > diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
> > > index 3c83b47..942490a 100644
> > > --- a/drivers/gpu/drm/i915/i915_vgpu.h
> > > +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> > > @@ -113,5 +113,8 @@ struct vgt_if {
> > > extern void i915_check_vgpu(struct drm_device *dev);
> > > extern int intel_vgt_balloon(struct drm_device *dev);
> > > extern void intel_vgt_deballoon(void);
> > > +extern void *gvt_create_pgt_device(struct drm_i915_private *dev_priv);
> > > +extern bool gvt_post_init_pgt_device(void *private_data);
> > > +extern void gvt_destroy_pgt_device(void *private_data);
> > >
> > > #endif /* _I915_VGPU_H_ */
> > > diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> > > index d133112..78a38f1 100644
> > > --- a/include/xen/interface/xen.h
> > > +++ b/include/xen/interface/xen.h
> > > @@ -28,6 +28,7 @@
> > > #define __XEN_PUBLIC_XEN_H__
> > >
> > > #include
> > > +#include
> > >
> > > /*
> > > * XEN "SYSTEM CALLS" (a.k.a. HYPERCALLS).
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
More information about the Intel-gfx
mailing list