[Intel-gfx] [PATCH 2/2] drm/i915/gen7: Clear all EU/L3 residual contexts

2020-02-20 Thread Akeem G Abodunrin
From: Prathap Kumar Valsan 

On gen7 and gen7.5 devices, there could be leftover data residuals in
EU/L3 from the retiring context. This patch introduces workaround to clear
that residual contexts, by submitting a batch buffer with dedicated HW
context to the GPU with ring allocation for each context switching.

This security mitigation changes does not triggers any performance
regression. Performance is on par with current drm-tips.

v2: Add igt generated header file for CB kernel assembled with Mesa tool
and addressed use of Kernel macro for ptr_align comment.

v3: Resolve Sparse warnings with newly generated, and imported CB
kernel.

v4: Include new igt generated CB kernel for gen7 and gen7.5. Also
add code formatting and compiler warnings changes (Chris Wilson)

Signed-off-by: Mika Kuoppala 
Signed-off-by: Prathap Kumar Valsan 
Signed-off-by: Akeem G Abodunrin 
Cc: Chris Wilson 
Cc: Balestrieri Francesco 
Cc: Bloomfield Jon 
Cc: Dutt Sudeep 
---
 drivers/gpu/drm/i915/Makefile |   1 +
 drivers/gpu/drm/i915/gt/gen7_renderclear.c| 402 ++
 drivers/gpu/drm/i915/gt/gen7_renderclear.h|  15 +
 drivers/gpu/drm/i915/gt/hsw_clear_kernel.c|  61 +++
 drivers/gpu/drm/i915/gt/intel_gpu_commands.h  |  17 +-
 .../gpu/drm/i915/gt/intel_ring_submission.c   |   3 +-
 drivers/gpu/drm/i915/gt/ivb_clear_kernel.c|  61 +++
 7 files changed, 556 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/gen7_renderclear.c
 create mode 100644 drivers/gpu/drm/i915/gt/gen7_renderclear.h
 create mode 100644 drivers/gpu/drm/i915/gt/hsw_clear_kernel.c
 create mode 100644 drivers/gpu/drm/i915/gt/ivb_clear_kernel.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index b314d44ded5e..ebe3a160f588 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -79,6 +79,7 @@ gt-y += \
gt/debugfs_gt.o \
gt/debugfs_gt_pm.o \
gt/gen6_ppgtt.o \
+   gt/gen7_renderclear.o \
gt/gen8_ppgtt.o \
gt/intel_breadcrumbs.o \
gt/intel_context.o \
diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c 
b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
new file mode 100644
index ..de595b66a746
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
@@ -0,0 +1,402 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "gen7_renderclear.h"
+#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))
+
+struct cb_kernel {
+   const void *data;
+   u32 size;
+};
+
+#define CB_KERNEL(name) { .data = (name), .size = sizeof(name) }
+
+#include "ivb_clear_kernel.c"
+static const struct cb_kernel cb_kernel_ivb = CB_KERNEL(ivb_clear_kernel);
+
+#include "hsw_clear_kernel.c"
+static const struct cb_kernel cb_kernel_hsw = CB_KERNEL(hsw_clear_kernel);
+
+struct batch_chunk {
+   struct i915_vma *vma;
+   u32 offset;
+   u32 *start;
+   u32 *end;
+   u32 max_items;
+};
+
+struct batch_vals {
+   u32 max_primitives;
+   u32 max_urb_entries;
+   u32 cmd_size;
+   u32 state_size;
+   u32 state_start;
+   u32 batch_size;
+   u32 surface_height;
+   u32 surface_width;
+   u32 scratch_size;
+   u32 max_size;
+};
+
+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;
+   bv->surface_height = 16 * 16;
+   bv->surface_width = 32 * 2 * 16;
+   } else {
+   bv->max_primitives = 128;
+   bv->max_urb_entries = MAX_URB_ENTRIES / 2;
+   bv->surface_height = 16 * 8;
+   bv->surface_width = 32 * 16;
+   }
+   bv->cmd_size = bv->max_primitives * 4096;
+   bv->state_size = STATE_SIZE;
+   bv->state_start = bv->cmd_size;
+   bv->batch_size = bv->cmd_size + bv->state_size;
+   bv->scratch_size = bv->surface_height * bv->surface_width;
+   bv->max_size = bv->batch_size + bv->scratch_size;
+}
+
+static void batch_init(struct batch_chunk *bc,
+  struct i915_vma *vma,
+  u32 *start, u32 offset, u32 max_bytes)
+{
+   bc->vma = vma;
+   bc->offset = offset;
+   bc->start = start + bc->offset / sizeof(*bc->start);
+   bc->end = bc->start;
+   bc->max_items = max_bytes / sizeof(*bc->start);
+}
+
+static u32 batch_offset(const struct batch_chunk *bc, u32 *cs)
+{
+   return (cs - bc->start) * sizeof(*bc->start) + bc->offset;
+}
+
+static u32 batch_addr(const struct batch_chunk *bc)
+{
+   return bc->vma->node.start;
+}
+
+static void batch_add(struct batch_chunk *bc, const u32 d)
+{
+   GEM_BUG_ON((bc->end - bc->start) >= 

Re: [Intel-gfx] [PATCH 2/2] drm/i915/gen7: Clear all EU/L3 residual contexts

2020-02-03 Thread Joonas Lahtinen
Quoting Bloomfield, Jon (2020-01-31 17:45:02)
> Reducing audience as this series is of high interest externally.
> 
> I fully agree with Joonas' suggestion here, and we have been looking at doing 
> just that. But can we iterate as a follow up patch series? Putting in the 
> infra to support igt assembly from source will take a little time (igt 
> assembler doesn't like the source right now, so it looks like it will need 
> updating), and we are under pressure to get this security fix out.

In order to merge upstream, we need to be able to show the
readable source code with appropriate license for the
execution kernel and provide compiling instructions.

Compiling with publicly available tools is enough and at
merge time it doesn't necessarily have to integrate with
IGT fully due to security aspect.

If we can't fulfill those requirements, then the right place
for the blob is in linux-firmware.

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915/gen7: Clear all EU/L3 residual contexts

2020-01-31 Thread Bloomfield, Jon
Reducing audience as this series is of high interest externally.

I fully agree with Joonas' suggestion here, and we have been looking at doing 
just that. But can we iterate as a follow up patch series? Putting in the infra 
to support igt assembly from source will take a little time (igt assembler 
doesn't like the source right now, so it looks like it will need updating), and 
we are under pressure to get this security fix out.

Jon

> -Original Message-
> From: Joonas Lahtinen 
> Sent: Friday, January 31, 2020 1:52 AM
> To: Abodunrin, Akeem G ; Wilson, Chris P
> ; Phillips, D Scott ;
> Vetter, Daniel ; Stewart, David C
> ; dri-de...@lists.freedesktop.org; Balestrieri,
> Francesco ; intel-gfx@lists.freedesktop.org;
> Nikula, Jani ; Bloomfield, Jon
> ; Kuoppala, Mika ;
> Aran, Omer ; Pathi, Pragyansri
> ; Kumar Valsan, Prathap
> ; Dutt, Sudeep ;
> Luck, Tony 
> Subject: Re: [PATCH 2/2] drm/i915/gen7: Clear all EU/L3 residual contexts
> 
> Quoting Akeem G Abodunrin (2020-01-30 18:57:21)
> > From: Prathap Kumar Valsan 
> >
> > On gen7 and gen7.5 devices, there could be leftover data residuals in
> > EU/L3 from the retiring context. This patch introduces workaround to clear
> > that residual contexts, by submitting a batch buffer with dedicated HW
> > context to the GPU with ring allocation for each context switching.
> >
> > This security mitigation change does not trigger any performance
> > regression. Performance is on par with current mainline/drm-tip.
> >
> > Signed-off-by: Mika Kuoppala 
> > Signed-off-by: Prathap Kumar Valsan 
> > Signed-off-by: Akeem G Abodunrin 
> > Cc: Chris Wilson 
> > Cc: Balestrieri Francesco 
> > Cc: Bloomfield Jon 
> > Cc: Dutt Sudeep 
> > ---
> >  drivers/gpu/drm/i915/Makefile |   1 +
> >  drivers/gpu/drm/i915/gt/gen7_renderclear.c| 535 ++
> >  drivers/gpu/drm/i915/gt/gen7_renderclear.h|  15 +
> >  drivers/gpu/drm/i915/gt/intel_gpu_commands.h  |  17 +-
> >  .../gpu/drm/i915/gt/intel_ring_submission.c   |   3 +-
> >  drivers/gpu/drm/i915/i915_utils.h |   5 +
> >  6 files changed, 572 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/gt/gen7_renderclear.c
> >  create mode 100644 drivers/gpu/drm/i915/gt/gen7_renderclear.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile
> b/drivers/gpu/drm/i915/Makefile
> > index 3c88d7d8c764..f96bae664a03 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -78,6 +78,7 @@ gt-y += \
> > gt/debugfs_gt.o \
> > gt/debugfs_gt_pm.o \
> > gt/gen6_ppgtt.o \
> > +   gt/gen7_renderclear.o \
> > gt/gen8_ppgtt.o \
> > gt/intel_breadcrumbs.o \
> > gt/intel_context.o \
> > diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> > new file mode 100644
> > index ..a6f5f1602e33
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> > @@ -0,0 +1,535 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright © 2019 Intel Corporation
> > + */
> > +
> > +#include "gen7_renderclear.h"
> > +#include "i915_drv.h"
> > +#include "i915_utils.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))
> > +
> > +/*
> > + * Media CB Kernel for gen7 devices
> > + * TODO: Add comments to kernel, indicating what each array of hex does
> or
> > + * include header file, which has assembly source and support in igt to be
> > + * able to generate kernel in this same format
> > + */
> 
> Having the original source code for the kernels in IGT is the
> best way to proceed. The kernels should also be split into
> separate files which can be generated from IGT and copied
> over as-is for easy verification.
> 
> Regards, Joonas
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915/gen7: Clear all EU/L3 residual contexts

2020-01-31 Thread Joonas Lahtinen
Quoting Akeem G Abodunrin (2020-01-30 18:57:21)
> From: Prathap Kumar Valsan 
> 
> On gen7 and gen7.5 devices, there could be leftover data residuals in
> EU/L3 from the retiring context. This patch introduces workaround to clear
> that residual contexts, by submitting a batch buffer with dedicated HW
> context to the GPU with ring allocation for each context switching.
> 
> This security mitigation change does not trigger any performance
> regression. Performance is on par with current mainline/drm-tip.
> 
> Signed-off-by: Mika Kuoppala 
> Signed-off-by: Prathap Kumar Valsan 
> Signed-off-by: Akeem G Abodunrin 
> Cc: Chris Wilson 
> Cc: Balestrieri Francesco 
> Cc: Bloomfield Jon 
> Cc: Dutt Sudeep 
> ---
>  drivers/gpu/drm/i915/Makefile |   1 +
>  drivers/gpu/drm/i915/gt/gen7_renderclear.c| 535 ++
>  drivers/gpu/drm/i915/gt/gen7_renderclear.h|  15 +
>  drivers/gpu/drm/i915/gt/intel_gpu_commands.h  |  17 +-
>  .../gpu/drm/i915/gt/intel_ring_submission.c   |   3 +-
>  drivers/gpu/drm/i915/i915_utils.h |   5 +
>  6 files changed, 572 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gt/gen7_renderclear.c
>  create mode 100644 drivers/gpu/drm/i915/gt/gen7_renderclear.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 3c88d7d8c764..f96bae664a03 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -78,6 +78,7 @@ gt-y += \
> gt/debugfs_gt.o \
> gt/debugfs_gt_pm.o \
> gt/gen6_ppgtt.o \
> +   gt/gen7_renderclear.o \
> gt/gen8_ppgtt.o \
> gt/intel_breadcrumbs.o \
> gt/intel_context.o \
> diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c 
> b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> new file mode 100644
> index ..a6f5f1602e33
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
> @@ -0,0 +1,535 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2019 Intel Corporation
> + */
> +
> +#include "gen7_renderclear.h"
> +#include "i915_drv.h"
> +#include "i915_utils.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))
> +
> +/*
> + * Media CB Kernel for gen7 devices
> + * TODO: Add comments to kernel, indicating what each array of hex does or
> + * include header file, which has assembly source and support in igt to be
> + * able to generate kernel in this same format
> + */

Having the original source code for the kernels in IGT is the
best way to proceed. The kernels should also be split into
separate files which can be generated from IGT and copied
over as-is for easy verification.

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915/gen7: Clear all EU/L3 residual contexts

2020-01-31 Thread Jani Nikula
On Thu, 30 Jan 2020, Akeem G Abodunrin  wrote:
> diff --git a/drivers/gpu/drm/i915/i915_utils.h 
> b/drivers/gpu/drm/i915/i915_utils.h
> index b0ade76bec90..7ac5b3565845 100644
> --- a/drivers/gpu/drm/i915/i915_utils.h
> +++ b/drivers/gpu/drm/i915/i915_utils.h
> @@ -172,6 +172,11 @@ __check_struct_size(size_t base, size_t arr, size_t 
> count, size_t *size)
>   (typeof(ptr))(__v + 1); \
>  })
>  
> +#define ptr_align(ptr, align) ({ \
> + unsigned long __v = (unsigned long)(ptr);   \
> + (typeof(ptr))round_up(__v, (align));\
> +})
> +

There's PTR_ALIGN() in include/kernel.h.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/2] drm/i915/gen7: Clear all EU/L3 residual contexts

2020-01-30 Thread Akeem G Abodunrin
From: Prathap Kumar Valsan 

On gen7 and gen7.5 devices, there could be leftover data residuals in
EU/L3 from the retiring context. This patch introduces workaround to clear
that residual contexts, by submitting a batch buffer with dedicated HW
context to the GPU with ring allocation for each context switching.

This security mitigation change does not trigger any performance
regression. Performance is on par with current mainline/drm-tip.

Signed-off-by: Mika Kuoppala 
Signed-off-by: Prathap Kumar Valsan 
Signed-off-by: Akeem G Abodunrin 
Cc: Chris Wilson 
Cc: Balestrieri Francesco 
Cc: Bloomfield Jon 
Cc: Dutt Sudeep 
---
 drivers/gpu/drm/i915/Makefile |   1 +
 drivers/gpu/drm/i915/gt/gen7_renderclear.c| 535 ++
 drivers/gpu/drm/i915/gt/gen7_renderclear.h|  15 +
 drivers/gpu/drm/i915/gt/intel_gpu_commands.h  |  17 +-
 .../gpu/drm/i915/gt/intel_ring_submission.c   |   3 +-
 drivers/gpu/drm/i915/i915_utils.h |   5 +
 6 files changed, 572 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/gen7_renderclear.c
 create mode 100644 drivers/gpu/drm/i915/gt/gen7_renderclear.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 3c88d7d8c764..f96bae664a03 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -78,6 +78,7 @@ gt-y += \
gt/debugfs_gt.o \
gt/debugfs_gt_pm.o \
gt/gen6_ppgtt.o \
+   gt/gen7_renderclear.o \
gt/gen8_ppgtt.o \
gt/intel_breadcrumbs.o \
gt/intel_context.o \
diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c 
b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
new file mode 100644
index ..a6f5f1602e33
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c
@@ -0,0 +1,535 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "gen7_renderclear.h"
+#include "i915_drv.h"
+#include "i915_utils.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))
+
+/*
+ * Media CB Kernel for gen7 devices
+ * TODO: Add comments to kernel, indicating what each array of hex does or
+ * include header file, which has assembly source and support in igt to be
+ * able to generate kernel in this same format
+ */
+static const u32 cb7_kernel[][4] = {
+   { 0x0001, 0x26020128, 0x0024, 0x },
+   { 0x0040, 0x20280c21, 0x0028, 0x0001 },
+   { 0x0110, 0x2c20, 0x002c, 0x },
+   { 0x00010220, 0x34001c00, 0x1400, 0x002c },
+   { 0x0061, 0x20600061, 0x, 0x },
+   { 0x0008, 0x20601c85, 0x0e00, 0x000c },
+   { 0x0005, 0x20601ca5, 0x0060, 0x0001 },
+   { 0x0008, 0x20641c85, 0x0e00, 0x000d },
+   { 0x0005, 0x20641ca5, 0x0064, 0x0003 },
+   { 0x0041, 0x207424a5, 0x0064, 0x0034 },
+   { 0x0040, 0x206014a5, 0x0060, 0x0074 },
+   { 0x0008, 0x20681c85, 0x0e00, 0x0008 },
+   { 0x0005, 0x20681ca5, 0x0068, 0x000f },
+   { 0x0041, 0x20701ca5, 0x0060, 0x0010 },
+   { 0x0040, 0x206814a5, 0x0068, 0x0070 },
+   { 0x0061, 0x20a00061, 0x, 0x },
+   { 0x0005, 0x206c1c85, 0x0e00, 0x0007 },
+   { 0x0041, 0x206c1ca5, 0x006c, 0x0004 },
+   { 0x0061, 0x20800021, 0x008d, 0x },
+   { 0x0001, 0x20800021, 0x006c, 0x },
+   { 0x0001, 0x20840021, 0x0068, 0x },
+   { 0x0001, 0x20880061, 0x, 0x0003 },
+   { 0x0005, 0x208c0d21, 0x0086, 0x },
+   { 0x05600032, 0x20a01fa1, 0x008d0080, 0x02190001 },
+   { 0x0040, 0x20a01ca5, 0x00a0, 0x0001 },
+   { 0x05600032, 0x20a01fa1, 0x008d0080, 0x040a8001 },
+   { 0x0240, 0x20281c21, 0x0028, 0x },
+   { 0x00010220, 0x34001c00, 0x1400, 0xfffc },
+   { 0x0001, 0x26020128, 0x0024, 0x },
+   { 0x0001, 0x22e4, 0x, 0x },
+   { 0x0001, 0x220801ec, 0x, 0x007f007f },
+   { 0x0061, 0x20400021, 0x008d, 0x },
+   { 0x0061, 0x2fe00021, 0x008d, 0x },
+   { 0x0021, 0x20400121, 0x00450020, 0x },
+   { 0x0001, 0x20480061, 0x, 0x000f000f },
+   { 0x0005, 0x204c0d21, 0x0046, 0xffef },
+   { 0x0081, 0x20600061, 0x, 0x },
+   { 0x0081, 0x20800061, 0x, 0x },
+   { 0x0081, 0x20a00061, 0x, 0x },
+   { 0x0081, 0x20c00061, 0x, 0x },
+   { 0x0081, 0x20e00061, 0x, 0x },
+   { 0x0081, 0x2161, 0x, 0x },
+   { 0x0081,