[Intel-gfx] [RFC 01/29] drm/i915/gvt: Introduce the basic architecture of GVT-g
Zhi Wang
zhi.a.wang at intel.com
Tue Feb 16 09:54:12 UTC 2016
Hi Joonas:
For the debug function/macros, could we use DRM_DEBUG_DRIVER as
output, but keep our internal debug switch & marcos, like:
#define gvt_dbg_xxx(xxx, xxx) \
if (gvt_debug & xxx) \
DRM_DEBUG_DRIVER(xxxxxx)
Is it OK for the maintainers? Or we have to use DRM_DEBUG_DRIVER
directly :(.
Thanks,
Zhi.
于 02/03/16 14:01, 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?
>
>>> 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
>
> #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 <xen/interface/xen.h>
> #include <asm/xen/hypervisor.h>
>
> #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.
>
> 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).
More information about the Intel-gfx
mailing list