[Intel-gfx] [PATCH v3] drm/i915: Enable guest i915 full ppgtt functionality

[Zhang, Tina]

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen at linux.intel.com]
> Sent: Tuesday, May 30, 2017 4:38 PM
> To: Zhang, Tina <tina.zhang at intel.com>; intel-gfx at lists.freedesktop.org
> Cc: zhenyuw at linux.intel.com; intel-gvt-dev at lists.freedesktop.org
> Subject: Re: [PATCH v3] drm/i915: Enable guest i915 full ppgtt functionality
> 
> On ma, 2017-05-22 at 16:19 +0800, Tina Zhang wrote:
> > Enable the guest i915 full ppgtt functionality when host can provide
> > this capability. vgt_caps is introduced to guest i915 driver to get
> > the vgpu capabilities from the device model. VGT_CPAS_FULL_PPGTT is
> > one of the capabilities type to let guest i915 dirver know that the
> > guest i915 full ppgtt is supported by device model.
> >
> > Changes since v1:
> > - Use u32 instead of uint32_t (Joonas)
> > - Move VGT_CAPS_FULL_PPGTT introduction to this patch and use #define
> >   instead of enum (Joonas)
> > - Rewrite the vgpu full ppgtt capability checking logic. (Joonas)
> > - Some coding style refine. (Joonas)
> >
> > Changes since v2:
> > - Divide the whole patch set into two separate patch series, with one
> >   patch in i915 side to check guest i915 full ppgtt capability and
> > enable
> >   it when this capability is supported by the device model, and the
> > other
> >   one in gvt side which fixs the blocking issue and enables the device
> >   model to provide the capability to guest. And this patch focuses on
> > guest
> >   i915 side. (Joonas)
> > - Change the title from "introduce vgt_caps to pvinfo" to
> >   "Enable guest i915 full ppgtt functionality". (Tina)
> >
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Signed-off-by: Tina Zhang <tina.zhang at intel.com>
> 
> I just noticed there is INTEL_VGT_IF_VERSION when I was looking to make sure
> that vgt_if is zeroed. Neither the version is incremented nor do I
> see VGT_PVINFO_PAGE getting zeroed.
Vgt_if gets its initial values populated by device model through reading hardware registers, named "SW Virtualization Reserved MMIO rang". These registers are always zeros when reading by device model. So, we use this way to initialize the vgt_if with zeros.
About the INTEL_VGT_IF_VERSION, agree that we need to doc a Spec to describe it in detail, I guess. But for this vgt_cap register definition, I'm afraid we don't need to bump the version. Because, actually, this register is already defined in the version 1.0, and we just didn't use it before.

> 
> What measures are in place to make sure running a new i915 under older
> DOM0 won't result in corruption?
This won't be a problem, I guess, as we have already considered this situation. (Thanks for Zhenyu's comments about this problem, before)
I915 with this new patch, will check whether the cap is enabled by device model. As the device model is old, this cap should not be enabled and the i915 in the guest will get value zero, which means device model cannot support the full ppgtt feature. So, in this way, the guest i915 can only use the aliasing ppgtt which is also the only choice for guest i915 before this patch.

> 
> The dependencies between i915 and gvt are rather tricky, so we'd need
> INTEL_VGT_IF_VERSION minor increment and also a one line change (zeroing of
> the new caps register) from gvt code to the same patch, otherwise bisecting will
> break.
> 
> Regards, Joonas
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation