[Intel-gfx] [PATCH] drm/i915: Do not enable movntdqa optimization in hypervisor guest

[Du, Changbin]

On Fri, Dec 22, 2017 at 11:11:15AM +0000, Chris Wilson wrote:
> Quoting changbin.du at intel.com (2017-12-22 06:31:49)
> > From: Changbin Du <changbin.du at intel.com>
> > 
> > Our QA reported a problem caused by movntdqa instructions. Currently,
> > the KVM hypervisor doesn't support VEX-prefix instructions emulation.
> > If users passthrough a GPU to guest with vfio option 'x-no-mmap=on',
> > then all access to the BARs will be trapped and emulated. The KVM
> > hypervisor would raise an inertal error to qemu which cause the guest
> > killed. (Since 'movntdqa' ins is not supported.)
> > 
> > This patch try not to enable movntdqa optimization if the driver is
> > running in hypervisor guest.
> > 
> > Signed-off-by: Changbin Du <changbin.du at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_memcpy.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_memcpy.c b/drivers/gpu/drm/i915/i915_memcpy.c
> > index 49a0794..8921f40 100644
> > --- a/drivers/gpu/drm/i915/i915_memcpy.c
> > +++ b/drivers/gpu/drm/i915/i915_memcpy.c
> > @@ -96,6 +96,11 @@ bool i915_memcpy_from_wc(void *dst, const void *src, unsigned long len)
> >  
> >  void i915_memcpy_init_early(struct drm_i915_private *dev_priv)
> >  {
> > -       if (static_cpu_has(X86_FEATURE_XMM4_1))
> > +       /**
> 
> This isn't a kerneldoc, so just /* for a regular comment.
>
Thanks, will update.
 
> > +        * Some hypervisors (e.g. KVM) don't support VEX-prefix instructions
> > +        * emulation. So don't enable movntdqa in hypervisor guest.
> > +        */
> > +       if (static_cpu_has(X86_FEATURE_XMM4_1) &&
> > +           !boot_cpu_has(X86_FEATURE_HYPERVISOR))
> >                 static_branch_enable(&has_movntdqa);
> 
> Code checks out, and I believe you that some hypervisor setups may trap
> those instructions, so
> 
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> 
> We could try and jigger the init order about, but we do need this setup
> quite early so that we use the knowledge in other setup. Considering
> that, an explicit check for a hypervisor seems sensible.
> -Chris
And this check is not only for GVTg, but also for physical GPU passthrough. So
it is beyong intel_vgpu_active().

-- 
Thanks,
Changbin Du