| Message ID | 20210110150404.19535-3-chris@chris-wilson.co.uk |
|---|---|
| State | Accepted |
| Commit | 984cadea032b103c5824a5f29d0a36b3e9df6333 |
| Headers | show |
| Series | [01/11] drm/i915/gt: Limit VFE threads based on GT | expand |
> -----Original Message----- > From: Chris Wilson <chris@chris-wilson.co.uk> > Sent: Sunday, January 10, 2021 7:04 AM > To: intel-gfx@lists.freedesktop.org > Cc: Chris Wilson <chris@chris-wilson.co.uk>; Joonas Lahtinen > <joonas.lahtinen@linux.intel.com>; Bloomfield, Jon > <jon.bloomfield@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; > stable@vger.kernel.org > Subject: [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 <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Jon Bloomfield <jon.bloomfield@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: stable@vger.kernel.org # v5.7 > --- Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>?
On Sun, Jan 10, 2021 at 03:03:56PM +0000, Chris Wilson wrote: > 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 I'm afraid this will have the same faith as the rc6 and the validation impact :/ > Fixes: 47f8253d2b89 ("drm/i915/gen7: Clear all EU/L3 residual contexts") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Jon Bloomfield <jon.bloomfield@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: stable@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 000000000000..8d5637cfa734 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_mitigations.c > @@ -0,0 +1,148 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2021 Intel Corporation > + */ > + > +#include <linux/kernel.h> > +#include <linux/moduleparam.h> > +#include <linux/slab.h> > +#include <linux/string.h> > + > +#include "i915_drv.h" > +#include "i915_mitigations.h" > + > +static unsigned long mitigations = ~0UL; > + > +enum { > + CLEAR_RESIDUALS = 0, specially worse if this list grows... > +}; > + > +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 |= BIT(i); > + else > + new &= ~BIT(i); > + break; > + } > + } > + if (i == ARRAY_SIZE(names)) { > + pr_err("Bad %s.mitigations=%s, '%s' is unknown\n", > + DRIVER_NAME, val, tok); > + err = -EINVAL; > + break; > + } > + } > + kfree(str); > + if (err) > + return err; > + > + WRITE_ONCE(mitigations, new); > + return 0; > +} > + > +static int mitigations_get(char *buffer, const struct kernel_param *kp) > +{ > + unsigned long local = READ_ONCE(mitigations); > + int count, i; > + bool enable; > + > + if (!local) > + return scnprintf(buffer, PAGE_SIZE, "%s\n", "off"); > + > + if (local & BIT(BITS_PER_LONG - 1)) { > + count = scnprintf(buffer, PAGE_SIZE, "%s,", "auto"); > + enable = false; > + } else { > + enable = true; > + count = 0; > + } > + > + for (i = 0; i < ARRAY_SIZE(names); i++) { > + if ((local & BIT(i)) != enable) > + continue; > + > + count += scnprintf(buffer + count, PAGE_SIZE - count, > + "%s%s,", enable ? "" : "!", names[i]); > + } > + > + buffer[count - 1] = '\n'; > + return count; > +} > + > +static const struct kernel_param_ops ops = { > + .set = mitigations_set, > + .get = mitigations_get, > +}; > + > +module_param_cb_unsafe(mitigations, &ops, NULL, 0600); > +MODULE_PARM_DESC(mitigations, > +"Selectively enable security mitigations for all Intel® GPUs in the system.\n" > +"\n" > +" auto -- enables all mitigations required for the platform [default]\n" > +" off -- disables all mitigations\n" > +"\n" > +"Individual mitigations can be enabled by passing a comma-separated string,\n" > +"e.g. mitigations=residuals to enable only clearing residuals or\n" > +"mitigations=auto,noresiduals to disable only the clear residual mitigation.\n" > +"Either '!' or 'no' may be used to switch from enabling the mitigation to\n" > +"disabling it.\n" but I liked this structure to at least stop the growing of the params... Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > +"\n" > +"Active mitigations for Ivybridge, Baytrail, Haswell:\n" > +" residuals -- clear all thread-local registers between contexts" > +); > diff --git a/drivers/gpu/drm/i915/i915_mitigations.h b/drivers/gpu/drm/i915/i915_mitigations.h > new file mode 100644 > index 000000000000..1359d8135287 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_mitigations.h > @@ -0,0 +1,13 @@ > +/* SPDX-License-Identifier: MIT */ > +/* > + * Copyright © 2021 Intel Corporation > + */ > + > +#ifndef __I915_MITIGATIONS_H__ > +#define __I915_MITIGATIONS_H__ > + > +#include <linux/types.h> > + > +bool i915_mitigate_clear_residuals(void); > + > +#endif /* __I915_MITIGATIONS_H__ */ > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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 000000000000..8d5637cfa734 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_mitigations.c @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2021 Intel Corporation + */ + +#include <linux/kernel.h> +#include <linux/moduleparam.h> +#include <linux/slab.h> +#include <linux/string.h> + +#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 |= BIT(i); + else + new &= ~BIT(i); + break; + } + } + if (i == ARRAY_SIZE(names)) { + pr_err("Bad %s.mitigations=%s, '%s' is unknown\n", + DRIVER_NAME, val, tok); + err = -EINVAL; + break; + } + } + kfree(str); + if (err) + return err; + + WRITE_ONCE(mitigations, new); + return 0; +} + +static int mitigations_get(char *buffer, const struct kernel_param *kp) +{ + unsigned long local = READ_ONCE(mitigations); + int count, i; + bool enable; + + if (!local) + return scnprintf(buffer, PAGE_SIZE, "%s\n", "off"); + + if (local & BIT(BITS_PER_LONG - 1)) { + count = scnprintf(buffer, PAGE_SIZE, "%s,", "auto"); + enable = false; + } else { + enable = true; + count = 0; + } + + for (i = 0; i < ARRAY_SIZE(names); i++) { + if ((local & BIT(i)) != enable) + continue; + + count += scnprintf(buffer + count, PAGE_SIZE - count, + "%s%s,", enable ? "" : "!", names[i]); + } + + buffer[count - 1] = '\n'; + return count; +} + +static const struct kernel_param_ops ops = { + .set = mitigations_set, + .get = mitigations_get, +}; + +module_param_cb_unsafe(mitigations, &ops, NULL, 0600); +MODULE_PARM_DESC(mitigations, +"Selectively enable security mitigations for all Intel® GPUs in the system.\n" +"\n" +" auto -- enables all mitigations required for the platform [default]\n" +" off -- disables all mitigations\n" +"\n" +"Individual mitigations can be enabled by passing a comma-separated string,\n" +"e.g. mitigations=residuals to enable only clearing residuals or\n" +"mitigations=auto,noresiduals to disable only the clear residual mitigation.\n" +"Either '!' or 'no' may be used to switch from enabling the mitigation to\n" +"disabling it.\n" +"\n" +"Active mitigations for Ivybridge, Baytrail, Haswell:\n" +" residuals -- clear all thread-local registers between contexts" +); diff --git a/drivers/gpu/drm/i915/i915_mitigations.h b/drivers/gpu/drm/i915/i915_mitigations.h new file mode 100644 index 000000000000..1359d8135287 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_mitigations.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2021 Intel Corporation + */ + +#ifndef __I915_MITIGATIONS_H__ +#define __I915_MITIGATIONS_H__ + +#include <linux/types.h> + +bool i915_mitigate_clear_residuals(void); + +#endif /* __I915_MITIGATIONS_H__ */
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 <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Jon Bloomfield <jon.bloomfield@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: stable@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