[Intel-gfx] [PATCH 03/11] drm/i915: Allow the sysadmin to override security mitigations
Abodunrin, Akeem G
akeem.g.abodunrin at intel.com
Mon Jan 11 20:58:42 UTC 2021
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Chris
> Wilson
> Sent: Sunday, January 10, 2021 7:04 AM
> To: intel-gfx at lists.freedesktop.org
> Cc: stable at vger.kernel.org; Chris Wilson <chris at chris-wilson.co.uk>
> 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 <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Jon Bloomfield <jon.bloomfield at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: stable at 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,
> +};
> +
> +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__ */
Although this seems like ideal solution - giving users option to choose *potential* performance over security or vice-versa - However, I would have expected that this patch adds a DRM warning to inform users of the consequences of their action, whenever module parameter is used to disable any kind of mitigations. Well, that is my own perspective, not as a legal expert.
Thanks,
~Akeem
More information about the Intel-gfx
mailing list