Re: [Intel-gfx] [PATCH v4] drm/doc: add rfc section for small BAR uapi

2022-05-17 Thread Abodunrin, Akeem G


> -Original Message-
> From: Tvrtko Ursulin 
> Sent: Tuesday, May 17, 2022 6:49 AM
> To: Auld, Matthew ; intel-gfx@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org; Thomas Hellström
> ; Landwerlin, Lionel G
> ; Bloomfield, Jon ;
> Daniel Vetter ; Justen, Jordan L
> ; Kenneth Graunke ;
> Abodunrin, Akeem G ; mesa-
> d...@lists.freedesktop.org
> Subject: Re: [PATCH v4] drm/doc: add rfc section for small BAR uapi
> 
> 
> On 17/05/2022 11:52, Matthew Auld wrote:
> > Add an entry for the new uapi needed for small BAR on DG2+.
> >
> > v2:
> >- Some spelling fixes and other small tweaks. (Akeem & Thomas)
> >- Rework error capture interactions, including no longer needing
> >  NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
> >- Add probed_cpu_visible_size. (Lionel)
> > v3:
> >- Drop the vma query for now.
> >- Add unallocated_cpu_visible_size as part of the region query.
> >- Improve the docs some more, including documenting the expected
> >  behaviour on older kernels, since this came up in some offline
> >  discussion.
> > v4:
> >- Various improvements all over. (Tvrtko)
> 
> You can ignore my previous reply, the clarifications from v4 read good to me.
> 
> Acked-by: Tvrtko Ursulin 

The patch looks good to me as well...

Acked-by: Akeem G Abodunrin 

> 
> Regards,
> 
> Tvrtko
> 
> >
> > Signed-off-by: Matthew Auld 
> > Cc: Thomas Hellström 
> > Cc: Lionel Landwerlin 
> > Cc: Tvrtko Ursulin 
> > Cc: Jon Bloomfield 
> > Cc: Daniel Vetter 
> > Cc: Jon Bloomfield 
> > Cc: Jordan Justen 
> > Cc: Kenneth Graunke 
> > Cc: Akeem G Abodunrin 
> > Cc: mesa-...@lists.freedesktop.org
> > ---
> >   Documentation/gpu/rfc/i915_small_bar.h   | 189
> +++
> >   Documentation/gpu/rfc/i915_small_bar.rst |  47 ++
> >   Documentation/gpu/rfc/index.rst  |   4 +
> >   3 files changed, 240 insertions(+)
> >   create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
> >   create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
> >
> > diff --git a/Documentation/gpu/rfc/i915_small_bar.h
> > b/Documentation/gpu/rfc/i915_small_bar.h
> > new file mode 100644
> > index ..c676640b23ef
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_small_bar.h
> > @@ -0,0 +1,189 @@
> > +/**
> > + * struct __drm_i915_memory_region_info - Describes one region as
> > +known to the
> > + * driver.
> > + *
> > + * Note this is using both struct drm_i915_query_item and struct
> drm_i915_query.
> > + * For this new query we are adding the new query id
> > +DRM_I915_QUERY_MEMORY_REGIONS
> > + * at &drm_i915_query_item.query_id.
> > + */
> > +struct __drm_i915_memory_region_info {
> > +   /** @region: The class:instance pair encoding */
> > +   struct drm_i915_gem_memory_class_instance region;
> > +
> > +   /** @rsvd0: MBZ */
> > +   __u32 rsvd0;
> > +
> > +   /**
> > +* @probed_size: Memory probed by the driver (-1 = unknown)
> > +*
> > +* Note that it should not be possible to ever encounter a zero value
> > +* here, also note that no current region type will ever return -1 here.
> > +* Although for future region types, this might be a possibility. The
> > +* same applies to the other size fields.
> > +*/
> > +   __u64 probed_size;
> > +
> > +   /**
> > +* @unallocated_size: Estimate of memory remaining (-1 = unknown)
> > +*
> > +* Requires CAP_PERFMON or CAP_SYS_ADMIN to get reliable
> accounting.
> > +* Without this (or if this is an older kernel) the value here will
> > +* always equal the @probed_size. Note this is only currently tracked
> > +* for I915_MEMORY_CLASS_DEVICE regions (for other types the value
> here
> > +* will always equal the @probed_size).
> > +*/
> > +   __u64 unallocated_size;
> > +
> > +   union {
> > +   /** @rsvd1: MBZ */
> > +   __u64 rsvd1[8];
> > +   struct {
> > +   /**
> > +* @probed_cpu_visible_size: Memory probed by the
> driver
> > +* that is CPU accessible. (-1 = unknown).
> > +*
> > +* This will be always be <= @probed_size, and the
> > +* remainder (if there is any) will not be CPU
> > +* accessible.
> > + 

Re: [Intel-gfx] [PATCH v2] drm/doc: add rfc section for small BAR uapi

2022-05-02 Thread Abodunrin, Akeem G


> -Original Message-
> From: Landwerlin, Lionel G 
> Sent: Monday, May 2, 2022 12:55 AM
> To: Auld, Matthew ; intel-gfx@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org; Thomas Hellström
> ; Bloomfield, Jon
> ; Daniel Vetter ; Justen,
> Jordan L ; Kenneth Graunke
> ; Abodunrin, Akeem G
> ; mesa-...@lists.freedesktop.org
> Subject: Re: [PATCH v2] drm/doc: add rfc section for small BAR uapi
> 
> On 20/04/2022 20:13, Matthew Auld wrote:
> > Add an entry for the new uapi needed for small BAR on DG2+.
> >
> > v2:
> >- Some spelling fixes and other small tweaks. (Akeem & Thomas)
> >- Rework error capture interactions, including no longer needing
> >  NEEDS_CPU_ACCESS for objects marked for capture. (Thomas)
> >- Add probed_cpu_visible_size. (Lionel)
> >
> > Signed-off-by: Matthew Auld 
> > Cc: Thomas Hellström 
> > Cc: Lionel Landwerlin 
> > Cc: Jon Bloomfield 
> > Cc: Daniel Vetter 
> > Cc: Jordan Justen 
> > Cc: Kenneth Graunke 
> > Cc: Akeem G Abodunrin 
> > Cc: mesa-...@lists.freedesktop.org
> > ---
> >   Documentation/gpu/rfc/i915_small_bar.h   | 190
> +++
> >   Documentation/gpu/rfc/i915_small_bar.rst |  58 +++
> >   Documentation/gpu/rfc/index.rst  |   4 +
> >   3 files changed, 252 insertions(+)
> >   create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
> >   create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
> >
> > diff --git a/Documentation/gpu/rfc/i915_small_bar.h
> > b/Documentation/gpu/rfc/i915_small_bar.h
> > new file mode 100644
> > index ..7bfd0cf44d35
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_small_bar.h
> > @@ -0,0 +1,190 @@
> > +/**
> > + * struct __drm_i915_memory_region_info - Describes one region as
> > +known to the
> > + * driver.
> > + *
> > + * Note this is using both struct drm_i915_query_item and struct
> drm_i915_query.
> > + * For this new query we are adding the new query id
> > +DRM_I915_QUERY_MEMORY_REGIONS
> > + * at &drm_i915_query_item.query_id.
> > + */
> > +struct __drm_i915_memory_region_info {
> > +   /** @region: The class:instance pair encoding */
> > +   struct drm_i915_gem_memory_class_instance region;
> > +
> > +   /** @rsvd0: MBZ */
> > +   __u32 rsvd0;
> > +
> > +   /** @probed_size: Memory probed by the driver (-1 = unknown) */
> > +   __u64 probed_size;
> > +
> > +   /** @unallocated_size: Estimate of memory remaining (-1 = unknown)
> */
> > +   __u64 unallocated_size;
> > +
> > +   union {
> > +   /** @rsvd1: MBZ */
> > +   __u64 rsvd1[8];
> > +   struct {
> > +   /**
> > +* @probed_cpu_visible_size: Memory probed by the
> driver
> > +* that is CPU accessible. (-1 = unknown).
> > +*
> > +* This will be always be <= @probed_size, and the
> > +* remainder(if there is any) will not be CPU
> > +* accessible.
> > +*/
> > +   __u64 probed_cpu_visible_size;
> > +   };
> 
> 
> Trying to implement userspace support in Vulkan for this, I have an additional
> question about the value of probed_cpu_visible_size.
> 
> When is it set to -1?
I believe it is set to -1 if it is unknown, and/or not cpu accessible... 

Cheers!
~Akeem
> 
> I'm guessing before there is support for this value it'll be 0 (MBZ).
> 
> After after it should either be the entire lmem or something smaller.
> 
> 
> -Lionel
> 
> 
> > +   };
> > +};
> > +
> > +/**
> > + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour,
> > +with added
> > + * extension support using struct i915_user_extension.
> > + *
> > + * Note that new buffer flags should be added here, at least for the
> > +stuff that
> > + * is immutable. Previously we would have two ioctls, one to create
> > +the object
> > + * with gem_create, and another to apply various parameters, however
> > +this
> > + * creates some ambiguity for the params which are considered
> > +immutable. Also in
> > + * general we're phasing out the various SET/GET ioctls.
> > + */
> > +struct __drm_i915_gem_create_ext {
> > +   /**
> > +* @size: Requested size for the object.
> > +*
> > +* The (page-aligned) allocated size for the object w

Re: [Intel-gfx] [PATCH 2/2] drm/doc: add rfc section for small BAR uapi

2022-02-22 Thread Abodunrin, Akeem G


> -Original Message-
> From: dri-devel  On Behalf Of
> Thomas Hellström
> Sent: Tuesday, February 22, 2022 2:36 AM
> To: Auld, Matthew ; intel-gfx@lists.freedesktop.org
> Cc: Daniel Vetter ; dri-de...@lists.freedesktop.org;
> Kenneth Graunke ; Bloomfield, Jon
> ; Justen, Jordan L ;
> mesa-...@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
> 
> 
> On 2/18/22 12:22, Matthew Auld wrote:
> > Add an entry for the new uapi needed for small BAR on DG2+.
> >
> > Signed-off-by: Matthew Auld 
> > Cc: Thomas Hellström 
> > Cc: Jon Bloomfield 
> > Cc: Daniel Vetter 
> > Cc: Jordan Justen 
> > Cc: Kenneth Graunke 
> > Cc: mesa-...@lists.freedesktop.org
> > ---
> >   Documentation/gpu/rfc/i915_small_bar.h   | 153
> +++
> >   Documentation/gpu/rfc/i915_small_bar.rst |  40 ++
> >   Documentation/gpu/rfc/index.rst  |   4 +
> >   3 files changed, 197 insertions(+)
> >   create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
> >   create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
> >
> > diff --git a/Documentation/gpu/rfc/i915_small_bar.h
> > b/Documentation/gpu/rfc/i915_small_bar.h
> > new file mode 100644
> > index ..fa65835fd608
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_small_bar.h
> > @@ -0,0 +1,153 @@
> > +/**
> > + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour,
> > +with added
> > + * extension support using struct i915_user_extension.
> > + *
> > + * Note that in the future we want to have our buffer flags here,
> 
> Does this sentence need updating, with the flags member?
> 
> 
> >   at least for
> > + * the stuff that is immutable. Previously we would have two ioctls,
> > +one to
> > + * create the object with gem_create, and another to apply various
> > +parameters,
> > + * however this creates some ambiguity for the params which are
> > +considered
> > + * immutable. Also in general we're phasing out the various SET/GET ioctls.
> > + */
> > +struct __drm_i915_gem_create_ext {
> > +   /**
> > +* @size: Requested size for the object.
> > +*
> > +* The (page-aligned) allocated size for the object will be returned.
> > +*
> > +* Note that for some devices we have might have further minimum
> > +* page-size restrictions(larger than 4K), like for device local-memory.
> > +* However in general the final size here should always reflect any
> > +* rounding up, if for example using the
> I915_GEM_CREATE_EXT_MEMORY_REGIONS
> > +* extension to place the object in device local-memory.
> > +*/
> > +   __u64 size;
> > +   /**
> > +* @handle: Returned handle for the object.
> > +*
> > +* Object handles are nonzero.
> > +*/
> > +   __u32 handle;
> > +   /**
> > +* @flags: Optional flags.
> > +*
> > +* Supported values:
> > +*
> > +* I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the
> kernel that
> > +* the object will need to be accessed via the CPU.
> > +*
> > +* Only valid when placing objects in I915_MEMORY_CLASS_DEVICE,
> and
> > +* only strictly required on platforms where only some of the device
> > +* memory is directly visible or mappable through the CPU, like on DG2+.
> > +*
> > +* One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM,
> to
> > +* ensure we can always spill the allocation to system memory, if we
> > +* can't place the object in the mappable part of
> > +* I915_MEMORY_CLASS_DEVICE.
> > +*
> > +* Note that buffers that need to be captured with
> EXEC_OBJECT_CAPTURE,
> > +* will need to enable this hint, if the object can also be placed in
> > +* I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call
> will
> > +* throw an error otherwise. This also means that such objects will need
> > +* I915_MEMORY_CLASS_SYSTEM set as a possible placement.
> > +*
> > +* Without this hint, the kernel will assume that non-mappable
> > +* I915_MEMORY_CLASS_DEVICE is preferred for this object. Note that
> the
> > +* kernel can still migrate the object to the mappable part, as a last
> > +* resort, if userspace ever CPU faults this object, but this might be
> > +* expensive, and so ideally should be avoided.
> > +*/
> > +#define I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS (1 << 0)
> > +   __u32 flags;
> > +   /**
> > +* @extensions: The chain of extensions to apply to this object.
> > +*
> > +* This will be useful in the future when we need to support several
> > +* different extensions, and we need to apply more than one when
> > +* creating the object. See struct i915_user_extension.
> > +*
> > +* If we don't supply any extensions then we get the same old
> gem_create
> > +* behaviour.
> > +*
> > +* For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
> > +* struct drm_i915_gem_create_ext_memory_regions.
> > +*
> > +* For I915_GEM_CREATE_EXT_PROTECT

Re: [Intel-gfx] [PATCH 1/5] drm/i915/gt: One more flush for Baytrail clear residuals

2021-01-19 Thread Abodunrin, Akeem G



> -Original Message-
> From: Chris Wilson 
> Sent: Tuesday, January 19, 2021 1:41 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: mika.kuopp...@linux.intel.com; Chris Wilson ;
> Abodunrin, Akeem G 
> Subject: [PATCH 1/5] drm/i915/gt: One more flush for Baytrail clear residuals
> 
> CI reports that Baytail requires one more invalidate after CACHE_MODE for it
> to be happy.
> 
> Fixes: ace44e13e577 ("drm/i915/gt: Clear CACHE_MODE prior to clearing
> residuals")
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Akeem G Abodunrin 
> ---
>  drivers/gpu/drm/i915/gt/gen7_renderclear.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> index 39478712769f..8551e6de50e8 100644
> --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> @@ -353,19 +353,21 @@ static void gen7_emit_pipeline_flush(struct
> batch_chunk *batch)
> 
>  static void gen7_emit_pipeline_invalidate(struct batch_chunk *batch)  {
> - u32 *cs = batch_alloc_items(batch, 0, 8);
> + u32 *cs = batch_alloc_items(batch, 0, 10);
> 
>   /* ivb: Stall before STATE_CACHE_INVALIDATE */
> - *cs++ = GFX_OP_PIPE_CONTROL(4);
> + *cs++ = GFX_OP_PIPE_CONTROL(5);
>   *cs++ = PIPE_CONTROL_STALL_AT_SCOREBOARD |
>   PIPE_CONTROL_CS_STALL;
>   *cs++ = 0;
>   *cs++ = 0;
> + *cs++ = 0;
> 
> - *cs++ = GFX_OP_PIPE_CONTROL(4);
> + *cs++ = GFX_OP_PIPE_CONTROL(5);
>   *cs++ = PIPE_CONTROL_STATE_CACHE_INVALIDATE;
>   *cs++ = 0;
>   *cs++ = 0;
> + *cs++ = 0;
> 
>   batch_advance(batch, cs);
>  }
> @@ -397,6 +399,7 @@ static void emit_batch(struct i915_vma * const vma,
>   batch_add(&cmds, 0x);
>   batch_add(&cmds, i915_mmio_reg_offset(CACHE_MODE_1));
>   batch_add(&cmds, 0x |
> PIXEL_SUBSPAN_COLLECT_OPT_DISABLE);
> + gen7_emit_pipeline_invalidate(&cmds);
>   gen7_emit_pipeline_flush(&cmds);
> 
>   /* Switch to the media pipeline and our base address */
> --
> 2.20.1

Reviewed-by: Akeem G Abodunrin 

Thank you!
~Akeem

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


Re: [Intel-gfx] [PATCH] drm/i915/gt: Clear CACHE_MODE prior to clearing residuals

2021-01-17 Thread Abodunrin, Akeem G



> -Original Message-
> From: Chris Wilson 
> Sent: Sunday, January 17, 2021 1:30 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson ; Mika Kuoppala
> ; Abodunrin, Akeem G
> 
> Subject: [PATCH] drm/i915/gt: Clear CACHE_MODE prior to clearing residuals
> 
> Since we do a bare context switch with no restore, the clear residual kernel
> runs on dirty state, and we must be careful to avoid executing bad state from
> context registers inherited from a malicious client.
> 
> Fixes: 008ead6ef8f5 ("drm/i915/gt: Restore clear-residual mitigations for
> Ivybridge, Baytrail")
> Fixes: 09aa9e45863e ("drm/i915/gt: Restore clear-residual mitigations for
> Ivybridge, Baytrail")
> Testcase: igt/gem_ctx_isolation # ivb,vlv
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Akeem G Abodunrin 
> ---
>  drivers/gpu/drm/i915/gt/gen7_renderclear.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> index 56bdcdaa9a88..0490c29788d7 100644
> --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> @@ -390,6 +390,14 @@ static void emit_batch(struct i915_vma * const
> vma,
>&cb_kernel_ivb,
>desc_count);
> 
> + gen7_emit_pipeline_invalidate(&cmds);
> + batch_add(&cmds, MI_LOAD_REGISTER_IMM(2));
> + batch_add(&cmds, i915_mmio_reg_offset(CACHE_MODE_0_GEN7));
> + batch_add(&cmds, 0x);
> + batch_add(&cmds, i915_mmio_reg_offset(CACHE_MODE_1));
> + batch_add(&cmds, 0x |
> PIXEL_SUBSPAN_COLLECT_OPT_DISABLE);
> + gen7_emit_pipeline_flush(&cmds);
> +
>   gen7_emit_pipeline_invalidate(&cmds);
>   batch_add(&cmds, PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
>   batch_add(&cmds, MI_NOOP);
> --
> 2.20.1

Reviewed-by: Akeem G Abodunrin 

Thanks,
~Akeem

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


Re: [Intel-gfx] [PATCH 1/3] drm/i915/gt: Limit VFE threads based on GT

2021-01-11 Thread Abodunrin, Akeem G



> -Original Message-
> From: Chris Wilson 
> Sent: Monday, January 11, 2021 1:03 PM
> To: Abodunrin, Akeem G ; intel-
> g...@lists.freedesktop.org
> Cc: stable@ ; Randy Wright
> 
> Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915/gt: Limit VFE threads based on
> GT
> 
> Quoting Abodunrin, Akeem G (2021-01-11 20:25:34)
> > >  static void
> > >  batch_get_defaults(struct drm_i915_private *i915, struct batch_vals *bv)
> {
> > >   if (IS_HASWELL(i915)) {
> > > - bv->max_primitives = 280;
> > > - bv->max_urb_entries = MAX_URB_ENTRIES;
> > > + switch (INTEL_INFO(i915)->gt) {
> > > + default:
> > > + case 1:
> > > + bv->max_threads = 70;
> > > + break;
> > > + case 2:
> > > + bv->max_threads = 140;
> > > + break;
> > > + case 3:
> > > + bv->max_threads = 280;
> > > + break;
> > > + }
> > >   bv->surface_height = 16 * 16;
> > >   bv->surface_width = 32 * 2 * 16;
> > >   } else {
> > > - bv->max_primitives = 128;
> > > - bv->max_urb_entries = MAX_URB_ENTRIES / 2;
> > > + switch (INTEL_INFO(i915)->gt) {
> > > + default:
> > > + case 1: /* including vlv */
> > > + bv->max_threads = 36;
> > > + break;
> > > + case 2:
> > > + bv->max_threads = 128;
> > > + break;
> > > + }
> > Do we really need to hardcode max number of threads per gt/platform?
> Why not calculating the number of active threads from the no_of_slices *
> 1024? - Also, is "64" not the minimum number of threads supported?
> 
> ivb,byt,hsw each has different numbers of threads per subslice, and each gt
> has a different number of subslice/slice (and not a simple doubling of
> subslice/slice from 1 -> 2 -> 3, although the total is!).
> 
> It's a choice between encoding a tuple for (num_threads, num_subslices,
> num_slices) or the combined value.
> 
> The goal is to run a shader in each HW thread to clear the thread-local
> registers, and only one shader in each.
> -Chris

Okay, let's go with simplified solution to achieve our goal here, instead of 
complex calculation...

Reviewed-by: Akeem G Abodunrin 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 03/11] drm/i915: Allow the sysadmin to override security mitigations

2021-01-11 Thread Abodunrin, Akeem G


> -Original Message-
> From: Intel-gfx  On Behalf Of Chris
> Wilson
> Sent: Sunday, January 10, 2021 7:04 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: sta...@vger.kernel.org; Chris Wilson 
> Subject: [Intel-gfx] [PATCH 03/11] drm/i915: Allow the sysadmin to override
> security mitigations
> 
> The clear-residuals mitigation is a relatively heavy hammer and under some
> circumstances the user may wish to forgo the context isolation in order to
> meet some performance requirement. Introduce a generic module parameter
> to allow selectively enabling/disabling different mitigations.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1858
> Fixes: 47f8253d2b89 ("drm/i915/gen7: Clear all EU/L3 residual contexts")
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Jon Bloomfield 
> Cc: Rodrigo Vivi 
> Cc: sta...@vger.kernel.org # v5.7
> ---
>  drivers/gpu/drm/i915/Makefile |   1 +
>  .../gpu/drm/i915/gt/intel_ring_submission.c   |   4 +-
>  drivers/gpu/drm/i915/i915_mitigations.c   | 148 ++
>  drivers/gpu/drm/i915/i915_mitigations.h   |  13 ++
>  4 files changed, 165 insertions(+), 1 deletion(-)  create mode 100644
> drivers/gpu/drm/i915/i915_mitigations.c
>  create mode 100644 drivers/gpu/drm/i915/i915_mitigations.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 4074d8cb0d6e..48f82c354611 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -38,6 +38,7 @@ i915-y += i915_drv.o \
> i915_config.o \
> i915_irq.o \
> i915_getparam.o \
> +   i915_mitigations.o \
> i915_params.o \
> i915_pci.o \
> i915_scatterlist.o \
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> index 724d56c9583d..657afd8ebc14 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> @@ -32,6 +32,7 @@
>  #include "gen6_ppgtt.h"
>  #include "gen7_renderclear.h"
>  #include "i915_drv.h"
> +#include "i915_mitigations.h"
>  #include "intel_breadcrumbs.h"
>  #include "intel_context.h"
>  #include "intel_gt.h"
> @@ -918,7 +919,8 @@ static int switch_context(struct i915_request *rq)
>   GEM_BUG_ON(HAS_EXECLISTS(engine->i915));
> 
>   if (engine->wa_ctx.vma && ce != engine->kernel_context) {
> - if (engine->wa_ctx.vma->private != ce) {
> + if (engine->wa_ctx.vma->private != ce &&
> + i915_mitigate_clear_residuals()) {
>   ret = clear_residuals(rq);
>   if (ret)
>   return ret;
> diff --git a/drivers/gpu/drm/i915/i915_mitigations.c
> b/drivers/gpu/drm/i915/i915_mitigations.c
> new file mode 100644
> index ..8d5637cfa734
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_mitigations.c
> @@ -0,0 +1,148 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "i915_drv.h"
> +#include "i915_mitigations.h"
> +
> +static unsigned long mitigations = ~0UL;
> +
> +enum {
> + CLEAR_RESIDUALS = 0,
> +};
> +
> +static const char * const names[] = {
> + [CLEAR_RESIDUALS] = "residuals",
> +};
> +
> +bool i915_mitigate_clear_residuals(void)
> +{
> + return READ_ONCE(mitigations) & BIT(CLEAR_RESIDUALS); }
> +
> +static int mitigations_set(const char *val, const struct kernel_param
> +*kp) {
> + unsigned long new = ~0UL;
> + char *str, *sep, *tok;
> + bool first = true;
> + int err = 0;
> +
> + BUILD_BUG_ON(ARRAY_SIZE(names) >=
> BITS_PER_TYPE(mitigations));
> +
> + str = kstrdup(val, GFP_KERNEL);
> + if (!str)
> + return -ENOMEM;
> +
> + for (sep = str; (tok = strsep(&sep, ","));) {
> + bool enable = true;
> + int i;
> +
> + /* Be tolerant of leading/trailing whitespace */
> + tok = strim(tok);
> +
> + if (first) {
> + first = false;
> +
> + if (!strcmp(tok, "auto")) {
> + new = ~0UL;
> + continue;
> + }
> +
> + new = 0;
> + if (!strcmp(tok, "off"))
> + continue;
> + }
> +
> + if (*tok == '!') {
> + enable = !enable;
> + tok++;
> + }
> +
> + if (!strncmp(tok, "no", 2)) {
> + enable = !enable;
> + tok += 2;
> + }
> +
> + if (*tok == '\0')
> + continue;
> +
> + for (i = 0; i < ARRAY_SIZE(names); i++) {
> + if (!strcmp(tok, names[i])) {
> + if (enable)
> + new |=

Re: [Intel-gfx] [PATCH 2/3] drm/i915/gt: Restore clear-residual mitigations for Ivybridge, Baytrail

2021-01-11 Thread Abodunrin, Akeem G



> -Original Message-
> From: Chris Wilson 
> Sent: Saturday, January 09, 2021 7:50 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson ; Mika Kuoppala
> ; Kumar Valsan, Prathap
> ; Abodunrin, Akeem G
> ; Bloomfield, Jon
> 
> Subject: [PATCH 2/3] drm/i915/gt: Restore clear-residual mitigations for
> Ivybridge, Baytrail
> 
> The mitigation is required for all gen7 platforms, now that it does not cause
> GPU hangs, restore it for Ivybridge and Baytrail.
> 
> Fixes: 47f8253d2b89 ("drm/i915/gen7: Clear all EU/L3 residual contexts")
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Prathap Kumar Valsan 
> Cc: Akeem G Abodunrin 
> Cc: Bloomfield Jon 
> ---
>  drivers/gpu/drm/i915/gt/intel_ring_submission.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> index 4ea741f488a8..72d4722441bf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c
> @@ -1326,7 +1326,7 @@ int intel_ring_submission_setup(struct
> intel_engine_cs *engine)
> 
>   GEM_BUG_ON(timeline->hwsp_ggtt != engine->status_page.vma);
> 
> - if (IS_HASWELL(engine->i915) && engine->class == RENDER_CLASS) {
> + if (IS_GEN(engine->i915, 7) && engine->class == RENDER_CLASS) {
>   err = gen7_ctx_switch_bb_init(engine);
>   if (err)
>   goto err_ring_unpin;
> --
> 2.20.1

I tried hard to remember why checking for HSW platform specifically in this 
case, since mitigation is applicable to all gen 7 (including 7.5) platforms - 
but couldn't recall why, and see no reason in the code doing it that way - so 
the changes make sense...

Reviewed-by: Akeem G Abodunrin 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/3] drm/i915/gt: Limit VFE threads based on GT

2021-01-11 Thread Abodunrin, Akeem G



> -Original Message-
> From: Chris Wilson 
> Sent: Saturday, January 09, 2021 7:49 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson ; Mika Kuoppala
> ; Kumar Valsan, Prathap
> ; Abodunrin, Akeem G
> ; Bloomfield, Jon
> ; Vivi, Rodrigo ; Randy
> Wright ; sta...@vger.kernel.org
> Subject: [PATCH 1/3] drm/i915/gt: Limit VFE threads based on GT
> 
> MEDIA_STATE_VFE only accepts the 'maximum number of threads' in the
> range [0, n-1] where n is #EU * (#threads/EU) with the number of threads
> based on plaform and the number of EU based on the number of slices and
> subslices. This is a fixed number per platform/gt, so appropriately limit the
> number of threads we spawn to match the device.
> 
> v2: Oversaturate the system with tasks to force execution on every HW
> thread; if the thread idles it is returned to the pool and may be reused again
> before an unused thread.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2024
> Fixes: 47f8253d2b89 ("drm/i915/gen7: Clear all EU/L3 residual contexts")
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> Cc: Prathap Kumar Valsan 
> Cc: Akeem G Abodunrin 
> Cc: Jon Bloomfield 
> Cc: Rodrigo Vivi 
> Cc: Randy Wright 
> Cc: sta...@vger.kernel.org # v5.7+
> ---
>  drivers/gpu/drm/i915/gt/gen7_renderclear.c | 91 --
>  1 file changed, 49 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> index d93d85cd3027..3ea7c9cc0f3d 100644
> --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> @@ -7,8 +7,6 @@
>  #include "i915_drv.h"
>  #include "intel_gpu_commands.h"
> 
> -#define MAX_URB_ENTRIES 64
> -#define STATE_SIZE (4 * 1024)
>  #define GT3_INLINE_DATA_DELAYS 0x1E00
>  #define batch_advance(Y, CS) GEM_BUG_ON((Y)->end != (CS))
> 
> @@ -34,38 +32,57 @@ struct batch_chunk {  };
> 
>  struct batch_vals {
> - u32 max_primitives;
> - u32 max_urb_entries;
> - u32 cmd_size;
> - u32 state_size;
> + u32 max_threads;
>   u32 state_start;
> - u32 batch_size;
> + u32 surface_start;
>   u32 surface_height;
>   u32 surface_width;
> - u32 scratch_size;
> - u32 max_size;
> + u32 size;
>  };
> 
> +static inline int num_primitives(const struct batch_vals *bv) {
> + /*
> +  * We need to oversaturate the GPU with work in order to dispatch
> +  * a shader on every HW thread.
> +  */
> + return bv->max_threads + 2;
> +}
> +
>  static void
>  batch_get_defaults(struct drm_i915_private *i915, struct batch_vals *bv)  {
>   if (IS_HASWELL(i915)) {
> - bv->max_primitives = 280;
> - bv->max_urb_entries = MAX_URB_ENTRIES;
> + switch (INTEL_INFO(i915)->gt) {
> + default:
> + case 1:
> + bv->max_threads = 70;
> + break;
> + case 2:
> + bv->max_threads = 140;
> + break;
> + case 3:
> + bv->max_threads = 280;
> + break;
> + }
>   bv->surface_height = 16 * 16;
>   bv->surface_width = 32 * 2 * 16;
>   } else {
> - bv->max_primitives = 128;
> - bv->max_urb_entries = MAX_URB_ENTRIES / 2;
> + switch (INTEL_INFO(i915)->gt) {
> + default:
> + case 1: /* including vlv */
> + bv->max_threads = 36;
> + break;
> + case 2:
> + bv->max_threads = 128;
> + break;
> + }
Do we really need to hardcode max number of threads per gt/platform? Why not 
calculating the number of active threads from the no_of_slices * 1024? - Also, 
is "64" not the minimum number of threads supported?

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


Re: [Intel-gfx] [PATCH] drm/i915/gt: Decouple completed requests on unwind

2020-06-16 Thread Abodunrin, Akeem G



> -Original Message-
> From: Intel-gfx  On Behalf Of Chris
> Wilson
> Sent: Tuesday, June 16, 2020 12:02 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson 
> Subject: [Intel-gfx] [PATCH] drm/i915/gt: Decouple completed requests on
> unwind
> 
> Since the introduction of preempt-to-busy, requests can complete in the
> background, even while they are not on the engine->active.requests list.
> As such, the engine->active.request list itself is not in strict retirement 
> order,
> and we have to scan the entire list while unwinding to not miss any.
> However, if the request is completed we currently leave it on the list [until
> retirement], but we could just as simply remove it and stop treating it as
> active. We would only have to then traverse it once while unwinding in quick
> succession.
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index e866b8d721ed..4eb397b0e14d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1114,8 +1114,10 @@ __unwind_incomplete_requests(struct
> intel_engine_cs *engine)
>   list_for_each_entry_safe_reverse(rq, rn,
>&engine->active.requests,
>sched.link) {
> - if (i915_request_completed(rq))
> - continue; /* XXX */
> + if (i915_request_completed(rq)) {
> + list_del_init(&rq->sched.link);

Albeit this seems like a valid approach to resolve inconsistence in the list of 
requests that are active or retired, but we can't just delete completed 
requests from the list until full retirement is done - otherwise we stand the 
risk of out-of-the-order list, and could lead to inconsistence (which is the 
original problem you intend to resolve). Have you thought about locking 
mechanism?

Regards,
~Akeem
> + continue;
> + }
> 
>   __i915_request_unsubmit(rq);
> 
> --
> 2.20.1
> 
> ___
> 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


Re: [Intel-gfx] [PATCH] drm/i915: Drop i915_request.i915 backpointer

2020-06-02 Thread Abodunrin, Akeem G



> -Original Message-
> From: Intel-gfx  On Behalf Of Chris
> Wilson
> Sent: Tuesday, June 02, 2020 3:10 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson 
> Subject: [Intel-gfx] [PATCH] drm/i915: Drop i915_request.i915 backpointer
> 
> We infrequently use the direct i915 backpointer from the i915_request, so do
> we really need to waste the space in the struct for it? 8 bytes from the most
> frequently allocated struct vs an 3 bytes and pointer chasing in using rq-
> >engine->i915?
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c  |  4 ++--
>  drivers/gpu/drm/i915/gt/gen2_engine_cs.c|  2 +-
>  drivers/gpu/drm/i915/gt/intel_context_sseu.c|  2 +-
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c   |  6 ++
>  drivers/gpu/drm/i915/gt/intel_lrc.c |  6 +++---
>  drivers/gpu/drm/i915/gt/intel_ring_submission.c |  6 +++---
>  drivers/gpu/drm/i915/gt/intel_workarounds.c |  4 ++--
>  drivers/gpu/drm/i915/gt/selftest_engine_cs.c|  2 +-
>  drivers/gpu/drm/i915/gt/selftest_mocs.c |  2 +-
>  drivers/gpu/drm/i915/gt/selftest_rc6.c  |  9 -
>  drivers/gpu/drm/i915/gt/selftest_timeline.c |  4 ++--
>  drivers/gpu/drm/i915/gvt/scheduler.c|  4 ++--
>  drivers/gpu/drm/i915/i915_request.c | 12 ++--
>  drivers/gpu/drm/i915/i915_request.h |  3 ---
>  drivers/gpu/drm/i915/i915_trace.h   | 10 +-
>  drivers/gpu/drm/i915/selftests/i915_perf.c  |  2 +-
>  drivers/gpu/drm/i915/selftests/igt_spinner.c| 14 +++---
>  17 files changed, 43 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 219a36995b96..02a5c0ce39ca 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1910,8 +1910,8 @@ static int i915_reset_gen7_sol_offsets(struct
> i915_request *rq)
>   u32 *cs;
>   int i;
> 
> - if (!IS_GEN(rq->i915, 7) || rq->engine->id != RCS0) {
> - drm_dbg(&rq->i915->drm, "sol reset is gen7/rcs only\n");
> + if (!IS_GEN(rq->engine->i915, 7) || rq->engine->id != RCS0) {
> + drm_dbg(&rq->engine->i915->drm, "sol reset is gen7/rcs
> only\n");
>   return -EINVAL;
>   }
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen2_engine_cs.c
> b/drivers/gpu/drm/i915/gt/gen2_engine_cs.c
> index 8d2e85081247..3fb0dc1fb910 100644
> --- a/drivers/gpu/drm/i915/gt/gen2_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen2_engine_cs.c
> @@ -77,7 +77,7 @@ int gen4_emit_flush_rcs(struct i915_request *rq, u32
> mode)
>   cmd = MI_FLUSH;
>   if (mode & EMIT_INVALIDATE) {
>   cmd |= MI_EXE_FLUSH;
> - if (IS_G4X(rq->i915) || IS_GEN(rq->i915, 5))
> + if (IS_G4X(rq->engine->i915) || IS_GEN(rq->engine->i915, 5))
>   cmd |= MI_INVALIDATE_ISP;
>   }
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_context_sseu.c
> b/drivers/gpu/drm/i915/gt/intel_context_sseu.c
> index 487299cb91f2..27ae48049239 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context_sseu.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context_sseu.c
> @@ -30,7 +30,7 @@ static int gen8_emit_rpcs_config(struct i915_request
> *rq,
>   *cs++ = MI_STORE_DWORD_IMM_GEN4 | MI_USE_GGTT;
>   *cs++ = lower_32_bits(offset);
>   *cs++ = upper_32_bits(offset);
> - *cs++ = intel_sseu_make_rpcs(rq->i915, &sseu);
> + *cs++ = intel_sseu_make_rpcs(rq->engine->i915, &sseu);
> 
>   intel_ring_advance(rq, cs);
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index c8c14981eb5d..e37490d459c2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -661,7 +661,6 @@ static int measure_breadcrumb_dw(struct
> intel_context *ce)
>   if (!frame)
>   return -ENOMEM;
> 
> - frame->rq.i915 = engine->i915;
>   frame->rq.engine = engine;
>   frame->rq.context = ce;
>   rcu_assign_pointer(frame->rq.timeline, ce->timeline); @@ -1192,8
> +1191,7 @@ bool intel_engine_can_store_dword(struct intel_engine_cs
> *engine)
>   }
>  }
> 
> -static int print_sched_attr(struct drm_i915_private *i915,
> - const struct i915_sched_attr *attr,
> +static int print_sched_attr(const struct i915_sched_attr *attr,
>   char *buf, int x, int len)
>  {
>   if (attr->priority == I915_PRIORITY_INVALID) @@ -1213,7 +1211,7
> @@ static void print_request(struct drm_printer *m,
>   char buf[80] = "";
>   int x = 0;
> 
> - x = print_sched_attr(rq->i915, &rq->sched.attr, buf, x, sizeof(buf));
> + x = print_sched_attr(&rq->sched.attr, buf, x, sizeof(buf));
> 
>   drm_printf(m, "%s %llx:%llx%s%s %s @ %dms: %s\n",
>  prefix,
> diff --git a/drivers/gpu/drm/i91

Re: [Intel-gfx] [PATCH] drm/i915: Avoid dereferencing a dead context

2020-04-28 Thread Abodunrin, Akeem G



> -Original Message-
> From: Intel-gfx  On Behalf Of Chris
> Wilson
> Sent: Tuesday, April 28, 2020 2:03 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson 
> Subject: [Intel-gfx] [PATCH] drm/i915: Avoid dereferencing a dead context
> 
> Once the intel_context is closed, the GEM context may be freed and so the
> link from intel_context.gem_context is invalid.
> 
> <3>[  219.782944] BUG: KASAN: use-after-free in
> intel_engine_coredump_alloc+0x1bc3/0x2250 [i915] <3>[  219.782996] Read
> of size 8 at addr 8881d7dff0b8 by task kworker/0:1/12
> 
> <4>[  219.783052] CPU: 0 PID: 12 Comm: kworker/0:1 Tainted: G U
> 5.7.0-rc2-g1f3ffd7683d54-kasan_118+ #1
> <4>[  219.783055] Hardware name: System manufacturer System Product
> Name/Z170 PRO GAMING, BIOS 3402 04/26/2017 <4>[  219.783105]
> Workqueue: events heartbeat [i915] <4>[  219.783109] Call Trace:
> <4>[  219.783113]  
> <4>[  219.783119]  dump_stack+0x96/0xdb
> <4>[  219.783177]  ? intel_engine_coredump_alloc+0x1bc3/0x2250 [i915]
> <4>[  219.783182]  print_address_description.constprop.6+0x16/0x310
> <4>[  219.783239]  ? intel_engine_coredump_alloc+0x1bc3/0x2250 [i915]
> <4>[  219.783295]  ? intel_engine_coredump_alloc+0x1bc3/0x2250 [i915]
> <4>[  219.783300]  __kasan_report+0x137/0x190 <4>[  219.783359]  ?
> intel_engine_coredump_alloc+0x1bc3/0x2250 [i915] <4>[  219.783366]
> kasan_report+0x32/0x50 <4>[  219.783426]
> intel_engine_coredump_alloc+0x1bc3/0x2250 [i915] <4>[  219.783481]
> execlists_reset+0x39c/0x13d0 [i915] <4>[  219.783494]  ?
> mark_held_locks+0x9e/0xe0 <4>[  219.783546]  ? execlists_hold+0xfc0/0xfc0
> [i915] <4>[  219.783551]  ? lockdep_hardirqs_on+0x348/0x5f0 <4>[
> 219.783557]  ? _raw_spin_unlock_irqrestore+0x34/0x60
> <4>[  219.783606]  ? execlists_submission_tasklet+0x118/0x3a0 [i915] <4>[
> 219.783615]  tasklet_action_common.isra.14+0x13b/0x410
> <4>[  219.783623]  ? __do_softirq+0x1e4/0x9a7 <4>[  219.783630]
> __do_softirq+0x226/0x9a7 <4>[  219.783643]
> do_softirq_own_stack+0x2a/0x40 <4>[  219.783647]   <4>[
> 219.783692]  ? heartbeat+0x3e2/0x10f0 [i915] <4>[  219.783696]
> do_softirq.part.13+0x49/0x50 <4>[  219.783700]
> __local_bh_enable_ip+0x1a2/0x1e0 <4>[  219.783748]
> heartbeat+0x409/0x10f0 [i915] <4>[  219.783801]  ?
> __live_idle_pulse+0x9f0/0x9f0 [i915] <4>[  219.783806]  ?
> lock_acquire+0x1ac/0x8a0 <4>[  219.783811]  ?
> process_one_work+0x811/0x1870 <4>[  219.783827]  ?
> rcu_read_lock_sched_held+0x9c/0xd0
> <4>[  219.783832]  ? rcu_read_lock_bh_held+0xb0/0xb0 <4>[  219.783836]  ?
> _raw_spin_unlock_irq+0x1f/0x40 <4>[  219.783845]
> process_one_work+0x8ca/0x1870 <4>[  219.783848]  ?
> lock_acquire+0x1ac/0x8a0 <4>[  219.783852]  ? worker_thread+0x1d0/0xb80
> <4>[  219.783864]  ? pwq_dec_nr_in_flight+0x2c0/0x2c0 <4>[  219.783870]  ?
> do_raw_spin_lock+0x129/0x290 <4>[  219.783886]
> worker_thread+0x82/0xb80 <4>[  219.783895]  ?
> __kthread_parkme+0xaf/0x1b0 <4>[  219.783902]  ?
> process_one_work+0x1870/0x1870 <4>[  219.783906]  kthread+0x34e/0x420
> <4>[  219.783911]  ? kthread_create_on_node+0xc0/0xc0 <4>[  219.783918]
> ret_from_fork+0x3a/0x50
> 
> <3>[  219.783950] Allocated by task 1264:
> <4>[  219.783975]  save_stack+0x19/0x40
> <4>[  219.783978]  __kasan_kmalloc.constprop.3+0xa0/0xd0
> <4>[  219.784029]  i915_gem_create_context+0xa2/0xab8 [i915] <4>[
> 219.784081]  i915_gem_context_create_ioctl+0x1fa/0x450 [i915] <4>[
> 219.784085]  drm_ioctl_kernel+0x1d8/0x270 <4>[  219.784088]
> drm_ioctl+0x676/0x930 <4>[  219.784092]  ksys_ioctl+0xb7/0xe0 <4>[
> 219.784096]  __x64_sys_ioctl+0x6a/0xb0 <4>[  219.784100]
> do_syscall_64+0x94/0x530 <4>[  219.784103]
> entry_SYSCALL_64_after_hwframe+0x49/0xb3
> 
> <3>[  219.784120] Freed by task 12:
> <4>[  219.784141]  save_stack+0x19/0x40
> <4>[  219.784145]  __kasan_slab_free+0x130/0x180 <4>[  219.784148]
> kmem_cache_free_bulk+0x1bd/0x500 <4>[  219.784152]
> kfree_rcu_work+0x1d8/0x890 <4>[  219.784155]
> process_one_work+0x8ca/0x1870 <4>[  219.784158]
> worker_thread+0x82/0xb80 <4>[  219.784162]  kthread+0x34e/0x420 <4>[
> 219.784165]  ret_from_fork+0x3a/0x50
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 4d54dba35302..a976cd67b3b3 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1207,8 +1207,6 @@ static void engine_record_registers(struct
> intel_engine_coredump *ee)  static void record_request(const struct
> i915_request *request,
>  struct i915_request_coredump *erq)  {
> - const struct i915_gem_context *ctx;
> -
>   erq->flags = request->fence.flags;
>   erq->context = request->fence.context;
>   erq->seqno = request->fence.seqno;
> @@ -1218,9 +1216,13 @@ static void record_request(const struct
> i915_request *request,
> 
>   erq->pid = 0;
>   

Re: [Intel-gfx] [CI 1/2] drm/i915/selftests: Verify context isolation

2020-04-25 Thread Abodunrin, Akeem G



> -Original Message-
> From: Intel-gfx  On Behalf Of Chris
> Wilson
> Sent: Friday, April 24, 2020 1:33 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [CI 1/2] drm/i915/selftests: Verify context isolation
> 
> No unprivileged context should ever be allowed to modify logical state that is
> visible to another; there should be no backchannels available or control
> leakage for malicious actors.
> 
> This test tries to write to a set of random registers using non-privileged
> instructions (ala userspace). It should only be allowed to write into its
> context state, and all writes should not be visible to a second context. To
> verify this, we store the value of the register before writing to it in 
> context A
> (as this should be the default value inherited from the golden context state)
> and do a read back from context B of the same register. The reads from both
> contexts should be identical, the default value, except for a few free running
> counters (either global or local).
> 
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 
> ---
>  .../drm/i915/gem/selftests/i915_gem_context.c | 441 ++
>  1 file changed, 441 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> index f4f933240b39..c5c3433174dc 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> @@ -1865,6 +1865,446 @@ static int igt_vm_isolation(void *arg)
>   return err;
>  }
> 
> +static struct i915_vma *create_vma(struct i915_address_space *vm,
> +size_t sz) {
> + struct drm_i915_gem_object *obj;
> + struct i915_vma *vma;
> +
> + obj = i915_gem_object_create_internal(vm->i915, sz);
> + if (IS_ERR(obj))
> + return ERR_CAST(obj);
> +
> + vma = i915_vma_instance(obj, vm, NULL);
> + if (IS_ERR(vma))
> + i915_gem_object_put(obj);
> +
> + return vma;
> +}
> +
> +struct iso_details {
> + unsigned long count;
> +};
> +
> +enum {
> + ISO_REG = 0,
> + ISO_POISON,
> + ISO_BEFORE,
> + ISO_AFTER,
> + __ISO__
> +};
> +
> +static int iso_write(struct i915_gem_context *ctx,
> +  struct intel_engine_cs *engine,
> +  struct drm_i915_gem_object *obj,
> +  const struct iso_details *iso,
> +  u32 *ctl)
> +{
> + struct i915_vma *batch, *vma;
> + struct intel_context *ce;
> + struct i915_request *rq;
> + u32 *cs;
> + int err;
> + int i;
> +
> + ce = i915_gem_context_get_engine(ctx, engine->legacy_idx);
> + if (IS_ERR(ce))
> + return PTR_ERR(ce);
> +
> + batch = create_vma(ce->vm, PAGE_ALIGN(16 * iso->count + 4));
> + if (IS_ERR(batch)) {
> + err = PTR_ERR(batch);
> + goto err_ce;
> + }
> +
> + vma = i915_vma_instance(obj, ce->vm, NULL);
> + if (IS_ERR(vma)) {
> + err = PTR_ERR(vma);
> + goto err_batch;
> + }
> +
> + err = i915_vma_pin(batch, 0, 0, PIN_USER);
> + if (err)
> + goto err_batch;
> +
> + err = i915_vma_pin(vma, 0, 0, PIN_USER);
> + if (err)
> + goto err_unpin_batch;
> +
> + cs = i915_gem_object_pin_map(batch->obj, I915_MAP_WC);
> + if (IS_ERR(cs)) {
> + err = PTR_ERR(cs);
> + goto err_vma;
> + }
> +
> + for (i = 0; i < iso->count; i++) {
> + *cs++ = MI_LOAD_REGISTER_IMM(1);
> + *cs++ = ctl[i * __ISO__ + ISO_REG];
> + *cs++ = ctl[i * __ISO__ + ISO_POISON];
> + }
> + *cs++ = MI_BATCH_BUFFER_END;
> +
> + i915_gem_object_flush_map(batch->obj);
> + i915_gem_object_unpin_map(batch->obj);
> +
> + rq = intel_context_create_request(ce);
> + if (IS_ERR(rq)) {
> + err = PTR_ERR(rq);
> + goto err_vma;
> + }
> +
> + i915_vma_lock(vma);
> + err = i915_request_await_object(rq, vma->obj, true);
> + if (err == 0)
> + err = i915_vma_move_to_active(vma, rq,
> EXEC_OBJECT_WRITE);
> + i915_vma_unlock(vma);
> + if (err)
> + goto err_rq;
> +
> + i915_vma_lock(batch);
> + err = i915_request_await_object(rq, batch->obj, false);
> + if (err == 0)
> + err = i915_vma_move_to_active(batch, rq, 0);
> + i915_vma_unlock(batch);
> + if (err)
> + goto err_rq;
> +
> + err = engine->emit_bb_start(rq, batch->node.start, batch->node.size,
> +0);
> +
> +err_rq:
> + i915_request_add(rq);
> +err_vma:
> + i915_vma_unpin(vma);
> +err_unpin_batch:
> + i915_vma_unpin(batch);
> +err_batch:
> + i915_vma_put(batch);
> +err_ce:
> + intel_context_put(ce);
> + return err;
> +}
> +
> +static int iso_read(struct i915_gem_context *ctx,
> + struct intel_engine_cs *engine,
> + struct drm_i915_gem_object *obj,
> + cons