Re: [Intel-gfx] [PATCH v4] drm/doc: add rfc section for small BAR uapi
> -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
> -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
> -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
> -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
> -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
> -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
> -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
> -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
> -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
> -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
> -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
> -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
> -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