Re: [PATCH v2 0/8] Add enlightenments for vGPU

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Oct 21, 2014 at 06:16:26PM +0200, Daniel Vetter wrote:
> On Fri, Oct 17, 2014 at 01:37:11PM +0800, Yu Zhang wrote:
> > Intel GVT-g (previously known as XenGT), is a complete GPU
> > virtualization solution with mediated pass-through for 4th
> > generation Intel Core processors - Haswell platform. This
> > technology presents a virtual full-fledged GPU to each Virtual
> > Machine (VM). VMs can directly access performance-critical
> > resources, without intervention from the hypervisor in most
> > cases, while privileged operations from VMs are trap-and-emulated
> > at minimal cost. For detail, please refer to
> > https://01.org/xen/blogs/wangbo85/2014/intel-gvt-gxengt-pubic-release
> > 
> > This patch set includes necessary code changes when i915 driver
> > runs inside a VM. Though ideally we can run an unmodified i915
> > driver in VM, adding such enlightenments can greatly reduce the
> > virtualization complexity in orders of magnitude. Code changes
> > for the host side, which includes the actual Intel GVT-g
> > implementation, were sent out in another patchset.
> > 
> > The primary change introduced here is to implement so-called
> > "address space ballooning" technique. XenGT 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 and queuing commands. However w/ the
> > partitioning an unmodified i915 driver would assume a smaller
> > graphics memory starting from address ZERO, so requires XenGT
> > core module (vgt) 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, XenGT 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. Then vgt module only needs to scan and validate
> > graphics addresses w/o complexity of translation.
> > 
> > Note: The partitioning of global graphics memory may break some
> > applications, with large objects in the aperture, because current
> > userspace assumes half of the aperture usable. That would need
> > separate fix either in user space (e.g. remove assumption in mesa)
> > or in kernel (w/ some faulting mechanism).
> > 
> > The partitioning knowledge is conveyed through a reserved MMIO
> > range, called PVINFO, which will be architecturally reserved in
> > future hardware generations. Another information carried through
> > PVINFO is about the number of fence registers. As a global resource
> > XenGT also partitions them among VMs.
> > 
> > Other changes are trivial as optimizations, to either reduce the
> > trap overhead or disable power management features which don't
> > make sense in a virtualized environment.
> > 
> > Yu Zhang (8):
> >   drm/i915: Introduce a PV INFO page structure for Intel GVT-g.
> >   drm/i915: Adds graphic address space ballooning logic
> >   drm/i915: Partition the fence registers for vgpu in i915 driver
> >   drm/i915: Disable framebuffer compression for i915 driver in VM
> >   drm/i915: Add the display switch logic for vgpu in i915 driver
> >   drm/i915: Disable power management for i915 driver in VM
> >   drm/i915: Create vgpu specific write MMIO to reduce traps
> >   drm/i915: Support alias ppgtt in VM if ppgtt is enabled
> > 
> >  drivers/gpu/drm/i915/i915_dma.c     |  11 +++
> >  drivers/gpu/drm/i915/i915_drv.h     |  12 +++
> >  drivers/gpu/drm/i915/i915_gem.c     |   5 +
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 186 +++++++++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/i915/i915_vgt_if.h  |  93 ++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_pm.c     |   8 ++
> >  drivers/gpu/drm/i915/intel_uncore.c |  22 +++++
> >  7 files changed, 334 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/i915_vgt_if.h
> 
> I seem to have two v2 versions of this patch series. Anything changed or
> why the resend? I didn't see any comment on the older version ...

Well, looked through it anyway. On a high level this looks good for the
vgt integration for guests. I think we need some polish though still,
specifically for documentation.

- Please extract all the various intel_vgt_* functions spread all over the
  tree into a new i915_vgt_if.c (or intel_vgt.c, but then the header
  should be changed, too I think).

- Please add kerneldoc to all the functions which are non-static and so
  used by the driver outside of your kernel module.

- Please add a DOC: section detailing some of the high-level design
  considerations of vGT and also put that into the new file. I think in
  the future we should also keep the guest<->host abi revisions in there
  (i.e. the stuff in PV_INFO).

- Please pull all the new documentation together and integrate it into the
  i915 section of the drm docbook. A good place is probably a new
  subsection "Paravirtualized Guest Support (vGPU)" under the driver core
  section.

There's quite a few i915 driver subsystems which are already nicely
documented like this, but I also have a small howto:

http://blog.ffwll.ch/2014/06/documentation-for-drmi915.html

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch