drm/i915: Support ro ppgtt mapped cmdparser shadow buffers

commit 4f7af1948abcb18b4772fe1bcd84d7d27d96258c upstream.

For Gen7, the original cmdparser motive was to permit limited
use of register read/write instructions in unprivileged BB's.
This worked by copying the user supplied bb to a kmd owned
bb, and running it in secure mode, from the ggtt, only if
the scanner finds no unsafe commands or registers.

For Gen8+ we can't use this same technique because running bb's
from the ggtt also disables access to ppgtt space. But we also
do not actually require 'secure' execution since we are only
trying to reduce the available command/register set. Instead we
will copy the user buffer to a kmd owned read-only bb in ppgtt,
and run in the usual non-secure mode.

Note that ro pages are only supported by ppgtt (not ggtt), but
luckily that's exactly what we need.

Add the required paths to map the shadow buffer to ppgtt ro for Gen8+

v2: IS_GEN7/IS_GEN (Mika)
v3: rebase
v4: rebase
v5: rebase

Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris.p.wilson@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 73b88da6412757b37859b3b3af18bb91fd78018c..88f4d270d6561a67ab15c289d7317e3bf19ebcdb 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2645,6 +2645,12 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
 #define IS_GEN8(dev)   (!!(INTEL_INFO(dev)->gen_mask & BIT(7)))
 #define IS_GEN9(dev)   (!!(INTEL_INFO(dev)->gen_mask & BIT(8)))
 
+/*
+ * The Gen7 cmdparser copies the scanned buffer to the ggtt for execution
+ * All later gens can run the final buffer from the ppgtt
+ */
+#define CMDPARSER_USES_GGTT(dev_priv) IS_GEN7(dev_priv)
+
 #define ENGINE_MASK(id)        BIT(id)
 #define RENDER_RING    ENGINE_MASK(RCS)
 #define BSD_RING       ENGINE_MASK(VCS)
@@ -3016,6 +3022,14 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
                         u64 alignment,
                         u64 flags);
 
+struct i915_vma * __must_check
+i915_gem_object_pin(struct drm_i915_gem_object *obj,
+                   struct i915_address_space *vm,
+                   const struct i915_ggtt_view *view,
+                   u64 size,
+                   u64 alignment,
+                   u64 flags);
+
 int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
                  u32 flags);
 void __i915_vma_set_map_and_fenceable(struct i915_vma *vma);

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f1c4e95baa28da6c848cb3293d6652c369f2ee1c..dff3eabd674f1c56b04880e395f0389f8238b441 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3826,6 +3826,19 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
                         u64 flags)
 {
        struct i915_address_space *vm = &to_i915(obj->base.dev)->ggtt.base;
+
+       return i915_gem_object_pin(obj, vm, view, size, alignment,
+                                  flags | PIN_GLOBAL);
+}
+
+struct i915_vma *
+i915_gem_object_pin(struct drm_i915_gem_object *obj,
+                   struct i915_address_space *vm,
+                   const struct i915_ggtt_view *view,
+                   u64 size,
+                   u64 alignment,
+                   u64 flags)
+{
        struct i915_vma *vma;
        int ret;
 
@@ -3850,7 +3863,7 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
                        return ERR_PTR(ret);
        }
 
-       ret = i915_vma_pin(vma, size, alignment, flags | PIN_GLOBAL);
+       ret = i915_vma_pin(vma, size, alignment, flags);
        if (ret)
                return ERR_PTR(ret);
 

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 069f26a3f3ba9b8564a03307506b6293149052fd..dbfab0b66c631871b7a2f2be15b11079d1c5a67c 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1401,11 +1401,36 @@ i915_reset_gen7_sol_offsets(struct drm_i915_gem_request *req)
        return 0;
 }
 
+static struct i915_vma*
+shadow_batch_pin(struct drm_i915_gem_object *obj, struct i915_address_space *vm)
+{
+       struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+       u64 flags;
+
+       /*
+        * PPGTT backed shadow buffers must be mapped RO, to prevent
+        * post-scan tampering
+        */
+       if (CMDPARSER_USES_GGTT(dev_priv)) {
+               flags = PIN_GLOBAL;
+               vm = &dev_priv->ggtt.base;
+       } else if (vm->has_read_only) {
+               flags = PIN_USER;
+               i915_gem_object_set_readonly(obj);
+       } else {
+               DRM_DEBUG("Cannot prevent post-scan tampering without RO capable vm\n");
+               return ERR_PTR(-EINVAL);
+       }
+
+       return i915_gem_object_pin(obj, vm, NULL, 0, 0, flags);
+}
+
 static struct i915_vma *
 i915_gem_execbuffer_parse(struct intel_engine_cs *engine,
                          struct drm_i915_gem_exec_object2 *shadow_exec_entry,
                          struct drm_i915_gem_object *batch_obj,
                          struct eb_vmas *eb,
+                         struct i915_address_space *vm,
                          u32 batch_start_offset,
                          u32 batch_len)
 {
@@ -1424,14 +1449,21 @@ i915_gem_execbuffer_parse(struct intel_engine_cs *engine,
                                      batch_start_offset,
                                      batch_len);
        if (ret) {
-               if (ret == -EACCES) /* unhandled chained batch */
+               /*
+                * Unsafe GGTT-backed buffers can still be submitted safely
+                * as non-secure.
+                * For PPGTT backing however, we have no choice but to forcibly
+                * reject unsafe buffers
+                */
+               if (CMDPARSER_USES_GGTT(eb->i915) && (ret == -EACCES))
+                       /* Execute original buffer non-secure */
                        vma = NULL;
                else
                        vma = ERR_PTR(ret);
                goto out;
        }
 
-       vma = i915_gem_object_ggtt_pin(shadow_batch_obj, NULL, 0, 0, 0);
+       vma = shadow_batch_pin(shadow_batch_obj, vm);
        if (IS_ERR(vma))
                goto out;
 
@@ -1722,7 +1754,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
                vma = i915_gem_execbuffer_parse(engine, &shadow_exec_entry,
                                                params->batch->obj,
-                                               eb,
+                                               eb, vm,
                                                args->batch_start_offset,
                                                args->batch_len);
                if (IS_ERR(vma)) {
@@ -1731,16 +1763,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
                }
 
                if (vma) {
-                       /*
-                        * Batch parsed and accepted:
-                        *
-                        * Set the DISPATCH_SECURE bit to remove the NON_SECURE
-                        * bit from MI_BATCH_BUFFER_START commands issued in
-                        * the dispatch_execbuffer implementations. We
-                        * specifically don't want that set on batches the
-                        * command parser has accepted.
-                        */
-                       dispatch_flags |= I915_DISPATCH_SECURE;
+                       if (CMDPARSER_USES_GGTT(dev_priv))
+                               dispatch_flags |= I915_DISPATCH_SECURE;
                        params->args_batch_start_offset = 0;
                        params->batch = vma;
                }