Re: [Intel-gfx] [PATCH 6/6] drm/i915/gt: Always poison the kernel_context image before unparking

2019-12-30 Thread Chris Wilson 
Quoting Mika Kuoppala (2019-12-30 16:23:20)
> Chris Wilson  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 
> > Signed-off-by: Chris Wilson 
> > Cc: Imre Deak 
> > ---
> >  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, >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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 6/6] drm/i915/gt: Always poison the kernel_context image before unparking

2019-12-30 Thread Mika Kuoppala 
Chris Wilson  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 
> Signed-off-by: Chris Wilson 
> Cc: Imre Deak 
> ---
>  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, >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 

> + 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 6/6] drm/i915/gt: Always poison the kernel_context image before unparking

2019-12-30 Thread Chris Wilson 
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 
Signed-off-by: Chris Wilson 
Cc: Imre Deak 
---
 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, >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);
-- 
2.25.0.rc0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx