| Message ID | 20191230160112.3838434-6-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] drm/i915/selftests: Flush the context worker | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Keep scrubbing the kernel_context image with poison before we reset it > in order to demonstrate that we will be resilient in the case where it > is accidentally overwritten on idle. > > Suggested-by: Imre Deak <imre.deak@intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_context_types.h | 2 ++ > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 +++++++++++++++++- > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 ++-- > 3 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h > index 9527a659546c..ca1420fb8b53 100644 > --- a/drivers/gpu/drm/i915/gt/intel_context_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h > @@ -17,6 +17,8 @@ > #include "intel_engine_types.h" > #include "intel_sseu.h" > > +#define CONTEXT_REDZONE POISON_INUSE > + > struct i915_gem_context; > struct i915_vma; > struct intel_context; > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > index 1b9f73948f22..ea90ab3e396e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > @@ -37,8 +37,24 @@ static int __engine_unpark(struct intel_wakeref *wf) > > /* Discard stale context state from across idling */ > ce = engine->kernel_context; > - if (ce) > + if (ce) { > + GEM_BUG_ON(test_bit(CONTEXT_VALID_BIT, &ce->flags)); > + > + /* First poison the image to verify we never fully trust it */ > + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) && ce->state) { > + struct drm_i915_gem_object *obj = ce->state->obj; > + int type = i915_coherent_map_type(engine->i915); > + > + map = i915_gem_object_pin_map(obj, type); > + if (!IS_ERR(map)) { > + memset(map, CONTEXT_REDZONE, obj->base.size); Just pondering if the dword granularity would suffice. Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > + i915_gem_object_flush_map(obj); > + i915_gem_object_unpin_map(obj); > + } > + } > + > ce->ops->reset(ce); > + } > > if (engine->unpark) > engine->unpark(engine); > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > index 6d26d84812b6..f81e70cfd194 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -2409,7 +2409,7 @@ set_redzone(void *vaddr, const struct intel_engine_cs *engine) > > vaddr += engine->context_size; > > - memset(vaddr, POISON_INUSE, I915_GTT_PAGE_SIZE); > + memset(vaddr, CONTEXT_REDZONE, I915_GTT_PAGE_SIZE); > } > > static void > @@ -2420,7 +2420,7 @@ check_redzone(const void *vaddr, const struct intel_engine_cs *engine) > > vaddr += engine->context_size; > > - if (memchr_inv(vaddr, POISON_INUSE, I915_GTT_PAGE_SIZE)) > + if (memchr_inv(vaddr, CONTEXT_REDZONE, I915_GTT_PAGE_SIZE)) > dev_err_once(engine->i915->drm.dev, > "%s context redzone overwritten!\n", > engine->name); > -- > 2.25.0.rc0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2019-12-30 16:23:20) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Keep scrubbing the kernel_context image with poison before we reset it > > in order to demonstrate that we will be resilient in the case where it > > is accidentally overwritten on idle. > > > > Suggested-by: Imre Deak <imre.deak@intel.com> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_context_types.h | 2 ++ > > drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 +++++++++++++++++- > > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 ++-- > > 3 files changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h > > index 9527a659546c..ca1420fb8b53 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_context_types.h > > +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h > > @@ -17,6 +17,8 @@ > > #include "intel_engine_types.h" > > #include "intel_sseu.h" > > > > +#define CONTEXT_REDZONE POISON_INUSE > > + > > struct i915_gem_context; > > struct i915_vma; > > struct intel_context; > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > index 1b9f73948f22..ea90ab3e396e 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c > > @@ -37,8 +37,24 @@ static int __engine_unpark(struct intel_wakeref *wf) > > > > /* Discard stale context state from across idling */ > > ce = engine->kernel_context; > > - if (ce) > > + if (ce) { > > + GEM_BUG_ON(test_bit(CONTEXT_VALID_BIT, &ce->flags)); > > + > > + /* First poison the image to verify we never fully trust it */ > > + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) && ce->state) { > > + struct drm_i915_gem_object *obj = ce->state->obj; > > + int type = i915_coherent_map_type(engine->i915); > > + > > + map = i915_gem_object_pin_map(obj, type); > > + if (!IS_ERR(map)) { > > + memset(map, CONTEXT_REDZONE, obj->base.size); > > Just pondering if the dword granularity would suffice. Pure debug, so convenience of memchr_inv() rules. -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h index 9527a659546c..ca1420fb8b53 100644 --- a/drivers/gpu/drm/i915/gt/intel_context_types.h +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h @@ -17,6 +17,8 @@ #include "intel_engine_types.h" #include "intel_sseu.h" +#define CONTEXT_REDZONE POISON_INUSE + struct i915_gem_context; struct i915_vma; struct intel_context; diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c index 1b9f73948f22..ea90ab3e396e 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c @@ -37,8 +37,24 @@ static int __engine_unpark(struct intel_wakeref *wf) /* Discard stale context state from across idling */ ce = engine->kernel_context; - if (ce) + if (ce) { + GEM_BUG_ON(test_bit(CONTEXT_VALID_BIT, &ce->flags)); + + /* First poison the image to verify we never fully trust it */ + if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) && ce->state) { + struct drm_i915_gem_object *obj = ce->state->obj; + int type = i915_coherent_map_type(engine->i915); + + map = i915_gem_object_pin_map(obj, type); + if (!IS_ERR(map)) { + memset(map, CONTEXT_REDZONE, obj->base.size); + i915_gem_object_flush_map(obj); + i915_gem_object_unpin_map(obj); + } + } + ce->ops->reset(ce); + } if (engine->unpark) engine->unpark(engine); diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 6d26d84812b6..f81e70cfd194 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -2409,7 +2409,7 @@ set_redzone(void *vaddr, const struct intel_engine_cs *engine) vaddr += engine->context_size; - memset(vaddr, POISON_INUSE, I915_GTT_PAGE_SIZE); + memset(vaddr, CONTEXT_REDZONE, I915_GTT_PAGE_SIZE); } static void @@ -2420,7 +2420,7 @@ check_redzone(const void *vaddr, const struct intel_engine_cs *engine) vaddr += engine->context_size; - if (memchr_inv(vaddr, POISON_INUSE, I915_GTT_PAGE_SIZE)) + if (memchr_inv(vaddr, CONTEXT_REDZONE, I915_GTT_PAGE_SIZE)) dev_err_once(engine->i915->drm.dev, "%s context redzone overwritten!\n", engine->name);
Keep scrubbing the kernel_context image with poison before we reset it in order to demonstrate that we will be resilient in the case where it is accidentally overwritten on idle. Suggested-by: Imre Deak <imre.deak@intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/gt/intel_context_types.h | 2 ++ drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 +++++++++++++++++- drivers/gpu/drm/i915/gt/intel_lrc.c | 4 ++-- 3 files changed, 21 insertions(+), 3 deletions(-)