Re: [PATCH v2] drm/i915: Sanitize engine context sizes

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ke, 2017-04-26 at 14:16 +0100, Tvrtko Ursulin wrote:
> On 26/04/2017 13:20, Joonas Lahtinen wrote:
> > 
> > @@ -443,21 +417,6 @@ int i915_gem_context_init(struct drm_i915_private *dev_priv)
> >  	BUILD_BUG_ON(MAX_CONTEXT_HW_ID > INT_MAX);
> >  	ida_init(&dev_priv->context_hw_ida);
> > 
> > -	if (i915.enable_execlists) {
> > -		/* NB: intentionally left blank. We will allocate our own
> > -		 * backing objects as we need them, thank you very much */
> > -		dev_priv->hw_context_size = 0;
> > -	} else if (HAS_HW_CONTEXTS(dev_priv)) {
> > -		dev_priv->hw_context_size =
> > -			round_up(get_context_size(dev_priv),
> > -				 I915_GTT_PAGE_SIZE);
> 
> Is this rounding up lost when used from __create_hw_context?

Added it back for Gen 7 and 6 where it could do something. Others have
been rounded up in the heads of engineers already.

> > +static u32
> > +__intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
> Very minor, but Chris has been trying to establish i915 instead of 
> dev_priv in new code.

It'd shadow the global i915 in this case, so leaving it as is :/

> > @@ -134,6 +208,10 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
> >  	engine->irq_shift = info->irq_shift;
> >  	engine->class = info->class;
> >  	engine->instance = info->instance;
> > +	engine->context_size = __intel_engine_context_size(dev_priv,
> > +							   engine->class);
> > +	if (WARN_ON(engine->context_size > BIT(20)))
> > +		engine->context_size = 0;
> 
> Don't know the history to tell whether upgrade of DRM_DEBUG_DRIVER to a 
> WARN_ON is ok.

Talked with Chris, if it triggers, it should definitely be WARN_ON :)
Never seen in the past.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx