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 support
bytes
Got it. Thanks.
existing production. Future hardware will have the reserved window
It 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/ the
Personal 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?
+ 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.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
