On 12/16/2014 9:19 PM, Tvrtko Ursulin wrote:
How about we remove "to support existing production" in this sentence? The "currently supported hardware" seems a bit redundant. :)On 12/16/2014 12:51 PM, Yu, Zhang wrote:Thank you very much for your thorough review, Tvrtko. :) On 12/12/2014 1:33 AM, Tvrtko Ursulin wrote:On 11/13/2014 12:02 PM, Yu Zhang wrote:Introduce a PV INFO structure, to facilitate the Intel GVT-g technology, which is a GPU virtualization solution with mediated pass-through. This page contains the shared information between i915 driver and the host emulator. For now, this structure utilizes an area of 4K bypes on HSW GPU's unused MMIO space to supportbytesGot it. Thanks.existing production. Future hardware will have the reserved windowIt is slightly unclear to me what "existing production" means?Our current gpu virtualization implementation for HSW. It's compared with future implematations for BDW etc.So maybe instead of "existing production" say "currently supported hardware" or similar?
architecturally defined, and layout of the page will be added in future BSpec. The i915 driver load routine detects if it is running in a VM by reading the contents of this PV INFO page. Thereafter a flag, vgpu.active is set, and intel_vgpu_active() is used by checking this flag to conclude if GPU is virtualized with Intel GVT-g. By now, intel_vgpu_active() will return true, only when the driver is running as a guest in the Intel GVT-g enhanced environment on HSW platform. v2: take Chris' comments: - call the i915_check_vgpu() in intel_uncore_init() - sanitize i915_check_vgpu() by adding BUILD_BUG_ON() and debug info take Daniel's comments: - put the definition of PV INFO into a new header - i915_vgt_if.h other changes: - access mmio regs by readq/readw in i915_check_vgpu() v3: take Daniel's comments: - move the i915/vgt interfaces into a new i915_vgpu.c - update makefile - add kerneldoc to functions which are non-static - add a DOC: section describing some of the high-level design - update drm docbook other changes: - rename i915_vgt_if.h to i915_vgpu.h Signed-off-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx> Signed-off-by: Jike Song <jike.song@xxxxxxxxx> Signed-off-by: Eddie Dong <eddie.dong@xxxxxxxxx> --- Documentation/DocBook/drm.tmpl | 5 +++ drivers/gpu/drm/i915/Makefile | 3 ++ drivers/gpu/drm/i915/i915_drv.h | 11 +++++ drivers/gpu/drm/i915/i915_vgpu.c | 85 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_vgpu.h | 85 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_uncore.c | 3 ++ 6 files changed, 192 insertions(+) create mode 100644 drivers/gpu/drm/i915/i915_vgpu.c create mode 100644 drivers/gpu/drm/i915/i915_vgpu.h diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 64d9c1e..e4db4cf 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -3832,6 +3832,11 @@ int num_ioctls;</synopsis> !Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_disable_interrupts !Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_enable_interrupts </sect2> + <sect2> + <title>Intel GVT-g Guest Support(vGPU)</title> +!Pdrivers/gpu/drm/i915/i915_vgpu.c Intel GVT-g guest support +!Idrivers/gpu/drm/i915/i915_vgpu.c + </sect2> </sect1> <sect1> <title>Display Hardware Handling</title> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 891e584..d9aa5ca 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -79,6 +79,9 @@ i915-y += dvo_ch7017.o \ intel_sdvo.o \ intel_tv.o +# virtual gpu code +i915-y += i915_vgpu.o + # legacy horrors i915-y += i915_dma.o \ i915_ums.o diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 54691bc..e1e221e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1470,6 +1470,10 @@ struct i915_workarounds { u32 count; }; +struct i915_virtual_gpu { + bool active; +}; + struct drm_i915_private { struct drm_device *dev; struct kmem_cache *slab; @@ -1482,6 +1486,8 @@ struct drm_i915_private { struct intel_uncore uncore; + struct i915_virtual_gpu vgpu; + struct intel_gmbus gmbus[GMBUS_NUM_PORTS]; @@ -2311,6 +2317,11 @@ extern void intel_uncore_check_errors(struct drm_device *dev); extern void intel_uncore_fini(struct drm_device *dev); extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore); +static inline bool intel_vgpu_active(struct drm_device *dev) +{ + return to_i915(dev)->vgpu.active; +} + 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.c b/drivers/gpu/drm/i915/i915_vgpu.c new file mode 100644 index 0000000..3f6b797 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_vgpu.c @@ -0,0 +1,85 @@ +/* + * Copyright(c) 2011-2014 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 "intel_drv.h" +#include "i915_vgpu.h" + + +/** + * DOC: Intel GVT-g guest support + * + * Intel GVT-g is a graphics virtualization technology which shares the + * GPU among multiple virtual machines on a time-sharing basis. Each + * virtual machine is presented a virtual GPU (vGPU), which has equivalent + * features as the underlying physical GPU (pGPU), so i915 driver can run + * seamlessly in a virtual machine. This file provides vGPU specific + * optimizations when running in a virtual machine, to reduce the complexity + * of vGPU emulation and to improve the overall performance. + * + * A primary function introduced here is so-called "address space ballooning" + * technique. Intel GVT-g partitions global graphics memory among multiple VMs, + * so each VM can directly access a portion of the memory w/o hypervisor's + * intervention, e.g. filling textures or queuing commands. However w/ thePersonal preference, but It may be nicer to avoid "w/o" and similar shorthand in documentation.OK. Will fix it. :)+ * partitioning an unmodified i915 driver would assume a smaller graphics + * memory starting from address ZERO, then requires vGPU emulation module to + * translate the graphics address between 'guest view' and 'host view', for + * all registers and command opcodes which contain a graphics memory address. + * To reduce the complexity, Intel GVT-g introduces "address space ballooning", + * by telling the exact partitioning knowledge to each guest i915 driver, which + * then reserves and prevents non-allocated portions from allocation. Thus vGPU + * emulation module module only needs to scan and validate graphics addresses + * w/o complexity of address translation. + * + */ + +/** + * i915_check_vgpu - detect virtual GPU + * @dev: drm device * + * + * This function is called at the initialization stage, to detect whether + * running on a vGPU. + */ +void i915_check_vgpu(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + uint64_t magic; + uint32_t version; + + BUILD_BUG_ON(sizeof(struct vgt_if) != VGT_PVINFO_SIZE); + + if (!IS_HASWELL(dev)) + return; + + magic = readq(dev_priv->regs + vgtif_reg(magic)); + if (magic != VGT_MAGIC) + return; + + version = INTEL_VGT_IF_VERSION_ENCODE( + readw(dev_priv->regs + vgtif_reg(version_major)), + readw(dev_priv->regs + vgtif_reg(version_minor))); + if (version != INTEL_VGT_IF_VERSION) + return;Would it be useful to something on version mismatch?Well. Thanks for your notification. :) In fact, this version judgement is just for future usages. By now, we have only one vgt interface version, 0x10000. But you are right, unlike the judgement for 'magic', which specifies the Intel GVT-g feature, and can be returned directly when false, failure for the 'version' may should trigger something like unload the driver. However, if so, this means we may need to: 1> return something in i915_check_vgpu(); 2> change the prototype of i915_check_vgpu() and its caller intel_uncore_init(); 3> check the return value of intel_uncore_init() in i915_driver_load() Is this approach acceptable?I would leave this for the future version. I was simply suggesting logging something on version mismatch. Just because it is unexpected so there is a trace in the logs why feature is not available. It's up to you.+ dev_priv->vgpu.active = true; + DRM_INFO("Virtual GPU for Intel GVT-g detected.\n"); +} diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h new file mode 100644 index 0000000..5f41d01c --- /dev/null +++ b/drivers/gpu/drm/i915/i915_vgpu.h @@ -0,0 +1,85 @@ +/* + * Copyright(c) 2011-2014 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 _I915_VGPU_H_ +#define _I915_VGPU_H_ + +/* The MMIO offset of the shared info between guest and host emulator */ +#define VGT_PVINFO_PAGE 0x78000 +#define VGT_PVINFO_SIZE 0x1000 + +/* + * The following structure pages are defined in GEN MMIO space + * for virtualization. (One page for now) + */ +#define VGT_MAGIC 0x4776544776544776 /* 'vGTvGTvG' */ +#define VGT_VERSION_MAJOR 1 +#define VGT_VERSION_MINOR 0 + +#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor)) +#define INTEL_VGT_IF_VERSION \ + INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR) + +struct vgt_if { + uint64_t magic; /* VGT_MAGIC */ + uint16_t version_major; + uint16_t version_minor; + uint32_t vgt_id; /* ID of vGT instance */ + uint32_t rsv1[12]; /* pad to offset 0x40 */In general I thought stdint types should be avoided in the kernel in favour of u32 and company. But I see i915 is full of them already so maybe my information is outdated.Let's keep the _t types, like explanations in Daniel's mail. Is this OK?Yes.+ /* + * Data structure to describe the balooning info of resources. + * Each VM can only have one portion of continuous area for now. + * (May support scattered resource in future) + * (starting from offset 0x40) + */ + struct { + /* Aperture register balooning */ + struct { + uint32_t my_base; + uint32_t my_size; + } low_gmadr; /* aperture */ + /* GMADR register balooning */ + struct { + uint32_t my_base; + uint32_t my_size; + } high_gmadr; /* non aperture */ + /* allowed fence registers */ + uint32_t fence_num; + uint32_t rsv2[3]; + } avail_rs; /* available/assigned resource */I didn't figure out yet why the concept is to pass in two ranges to mark as reserved, instead of one usable/allowed range and then implicitly all the rest as reserved?Like you said in 2/3 comments, this 2 part is for mappable and non-mapplable respectively. :)Are high and low memory as terms known in the i915 space so far as synonyms for mappable and non-mappable?
Well, I'm not sure.Daniel, do you think the "low gmadr/high gmadr" will cause any misunderstanding?
+ uint32_t rsv3[0x200 - 24]; /* pad to half page */ + /* + * The bottom half page is for response from Gfx driver to hypervisor. + * Set to reserved fields temporarily by now. + */ + uint32_t rsv4; + uint32_t display_ready; /* ready for display owner switch */ + uint32_t rsv5[0x200 - 2]; /* pad to one page */ +} __packed;You probably don't need __packed since you have BUILD_BUG_ON on size mismatch and pad to full size with reserved fields. I don't think it matters though.Well, IIRC, this __packed is added to follow Chris's comments. How about we keep it? Or will it hurt any place? :)The only purpose for packed here I can see is a double guard along the BUILD_BUG_ON. As I said I don't think it makes any difference so you can leave it in.
OK. Thanks.
Regards, Tvrtko
B.R. Yu _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
