[Intel-gfx] [PATCH 2/7] drm/i915: Enable full ppgtt for vgpu

[Daniel Vetter]

On Thu, Aug 27, 2015 at 10:28:49AM +0800, Zhiyuan Lv wrote:
> Hi Danie,
> 
> On Wed, Aug 26, 2015 at 10:47:37AM +0200, Daniel Vetter wrote:
> > On Thu, Aug 20, 2015 at 01:57:13PM +0300, Joonas Lahtinen wrote:
> > > On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> > > > The full ppgtt is supported in Intel GVT-g device model. So the
> > > > restriction can be removed.
> > > > 
> > > > Signed-off-by: Zhiyuan Lv <zhiyuan.lv at intel.com>
> > > > Signed-off-by: Zhi Wang <zhi.a.wang at intel.com>
> > > 
> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > 
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ---
> > > >  1 file changed, 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> > > > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > > index ed10e77..823005c 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > > @@ -108,9 +108,6 @@ static int sanitize_enable_ppgtt(struct 
> > > > drm_device *dev, int enable_ppgtt)
> > > >  	has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
> > > >  	has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
> > > >  
> > > > -	if (intel_vgpu_active(dev))
> > > > -		has_full_ppgtt = false; /* emulation is too hard */
> > 
> > Don't we need a feature check for the virtual gpu here? Or at least a
> > platform check? Seems like the backwards/forwards compat story isn't too
> > thought out yet here. Note that the kernel of the host and the guest might
> > not be the same at all, much less the kvm part.
> 
> Yeah, backwards/forwards compatibility is not considered, since we are
> just to start the upstream of iGVT-g host changes. Right now if people
> uses new enough iGVT-g code (off-tree now), both HSW and BDW should
> work with PPGTT.
> 
> So new host with both old guest or new guest are working. The only
> thing impacted is old host with new guest kernel. In order to keep it
> work, I can change the code like:
> 
> +	if (intel_vgpu_active(dev))
> +		has_full_ppgtt = !IS_HASWELL(dev);
> 
> Any comments? Thanks for the review!

In the end that's your call (well until we have users screaming that a
kernel upgrade breaks their xengt setup, then you have to keep backwards
compat). But since it's a requirement from Linus Torvalds for upstream I
highly suggest you start thinking/testing for backwards compat (both on
host and guest side when the other part is older) right away. Otherwise
trying to shoehorn a good backwards compat scheme later on could be really
painful.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch