On Tue, 16 Feb 2016, Zhi Wang <zhi.a.wang@xxxxxxxxx> wrote: > 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 :(. Rather than introducing new debug macros of your own, I think I'd rather see any combination of the following: * DRM_DEBUG_*, DRM_ERROR, and friends. * dev_dbg, dev_info, dev_err, and friends. See the dynamic debug facilities for fine grained control of what is printed. * pr_debug, pr_info, pr_err, and friends. See pr_fmt for defining custom prefixes for what is printed. BR, Jani. > > 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). > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
