Skip to content

Commit

Permalink
Merge Intel Gen8/Gen9 graphics fixes from Jon Bloomfield.
Browse files Browse the repository at this point in the history
This fixes two different classes of bugs in the Intel graphics hardware:

MMIO register read hang:
 "On Intels Gen8 and Gen9 Graphics hardware, a read of specific graphics
  MMIO registers when the product is in certain low power states causes
  a system hang.

  There are two potential triggers for DoS:
    a) H/W corruption of the RC6 save/restore vector
    b) Hard hang within the MIPI hardware

  This prevents the DoS in two areas of the hardware:
    1) Detect corruption of RC6 address on exit from low-power state,
       and if we find it corrupted, disable RC6 and RPM
    2) Permanently lower the MIPI MMIO timeout"

Blitter command streamer unrestricted memory accesses:
 "On Intels Gen9 Graphics hardware the Blitter Command Streamer (BCS)
  allows writing to Memory Mapped Input Output (MMIO) that should be
  blocked. With modifications of page tables, this can lead to privilege
  escalation. This exposure is limited to the Guest Physical Address
  space and does not allow for access outside of the graphics virtual
  machine.

  This series establishes a software parser into the Blitter command
  stream to scan for, and prevent, reads or writes to MMIO's that should
  not be accessible to non-privileged contexts.

  Much of the command parser infrastructure has existed for some time,
  and is used on Ivybridge/Haswell/Valleyview derived products to allow
  the use of features normally blocked by hardware. In this legacy
  context, the command parser is employed to allow normally unprivileged
  submissions to be run with elevated privileges in order to grant
  access to a limited set of extra capabilities. In this mode the parser
  is optional; In the event that the parser finds any construct that it
  cannot properly validate (e.g. nested command buffers), it simply
  aborts the scan and submits the buffer in non-privileged mode.

  For Gen9 Graphics, this series makes the parser mandatory for all
  Blitter submissions. The incoming user buffer is first copied to a
  kernel owned buffer, and parsed. If all checks are successful the
  kernel owned buffer is mapped READ-ONLY and submitted on behalf of the
  user. If any checks fail, or the parser is unable to complete the scan
  (nested buffers), it is forcibly rejected. The successfully scanned
  buffer is executed with NORMAL user privileges (key difference from
  legacy usage).

  Modern usermode does not use the Blitter on later hardware, having
  switched over to using the 3D engine instead for performance reasons.
  There are however some legacy usermode apps that rely on Blitter,
  notably the SNA X-Server. There are no known usermode applications
  that require nested command buffers on the Blitter, so the forcible
  rejection of such buffers in this patch series is considered an
  acceptable limitation"

* Intel graphics fixes in emailed bundle from Jon Bloomfield <jon.bloomfield@intel.com>:
  drm/i915/cmdparser: Fix jump whitelist clearing
  drm/i915/gen8+: Add RC6 CTX corruption WA
  drm/i915: Lower RM timeout to avoid DSI hard hangs
  drm/i915/cmdparser: Ignore Length operands during command matching
  drm/i915/cmdparser: Add support for backward jumps
  drm/i915/cmdparser: Use explicit goto for error paths
  drm/i915: Add gen9 BCS cmdparsing
  drm/i915: Allow parsing of unsized batches
  drm/i915: Support ro ppgtt mapped cmdparser shadow buffers
  drm/i915: Add support for mandatory cmdparsing
  drm/i915: Remove Master tables from cmdparser
  drm/i915: Disable Secure Batches for gen6+
  drm/i915: Rename gen7 cmdparser tables
  • Loading branch information
Linus Torvalds committed Nov 12, 2019
2 parents de620fb + ea0b163 commit 100d46b
Show file tree
Hide file tree
Showing 13 changed files with 595 additions and 172 deletions.
5 changes: 5 additions & 0 deletions drivers/gpu/drm/i915/gem/i915_gem_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,8 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
free_engines(rcu_access_pointer(ctx->engines));
mutex_destroy(&ctx->engines_mutex);

kfree(ctx->jump_whitelist);

if (ctx->timeline)
intel_timeline_put(ctx->timeline);

Expand Down Expand Up @@ -441,6 +443,9 @@ __create_context(struct drm_i915_private *i915)
for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;

ctx->jump_whitelist = NULL;
ctx->jump_whitelist_cmds = 0;

return ctx;

err_free:
Expand Down
7 changes: 7 additions & 0 deletions drivers/gpu/drm/i915/gem/i915_gem_context_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ struct i915_gem_context {
* per vm, which may be one per context or shared with the global GTT)
*/
struct radix_tree_root handles_vma;

/** jump_whitelist: Bit array for tracking cmds during cmdparsing
* Guarded by struct_mutex
*/
unsigned long *jump_whitelist;
/** jump_whitelist_cmds: No of cmd slots available */
u32 jump_whitelist_cmds;
};

#endif /* __I915_GEM_CONTEXT_TYPES_H__ */
111 changes: 80 additions & 31 deletions drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,9 @@ static inline u64 gen8_noncanonical_addr(u64 address)

static inline bool eb_use_cmdparser(const struct i915_execbuffer *eb)
{
return intel_engine_needs_cmd_parser(eb->engine) && eb->batch_len;
return intel_engine_requires_cmd_parser(eb->engine) ||
(intel_engine_using_cmd_parser(eb->engine) &&
eb->args->batch_len);
}

static int eb_create(struct i915_execbuffer *eb)
Expand Down Expand Up @@ -1955,40 +1957,94 @@ static int i915_reset_gen7_sol_offsets(struct i915_request *rq)
return 0;
}

static struct i915_vma *eb_parse(struct i915_execbuffer *eb, bool is_master)
static struct i915_vma *
shadow_batch_pin(struct i915_execbuffer *eb, struct drm_i915_gem_object *obj)
{
struct drm_i915_private *dev_priv = eb->i915;
struct i915_vma * const vma = *eb->vma;
struct i915_address_space *vm;
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.vm;
} else if (vma->vm->has_read_only) {
flags = PIN_USER;
vm = vma->vm;
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 *eb_parse(struct i915_execbuffer *eb)
{
struct intel_engine_pool_node *pool;
struct i915_vma *vma;
u64 batch_start;
u64 shadow_batch_start;
int err;

pool = intel_engine_pool_get(&eb->engine->pool, eb->batch_len);
if (IS_ERR(pool))
return ERR_CAST(pool);

err = intel_engine_cmd_parser(eb->engine,
vma = shadow_batch_pin(eb, pool->obj);
if (IS_ERR(vma))
goto err;

batch_start = gen8_canonical_addr(eb->batch->node.start) +
eb->batch_start_offset;

shadow_batch_start = gen8_canonical_addr(vma->node.start);

err = intel_engine_cmd_parser(eb->gem_context,
eb->engine,
eb->batch->obj,
pool->obj,
batch_start,
eb->batch_start_offset,
eb->batch_len,
is_master);
pool->obj,
shadow_batch_start);

if (err) {
if (err == -EACCES) /* unhandled chained batch */
i915_vma_unpin(vma);

/*
* 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) && (err == -EACCES))
/* Execute original buffer non-secure */
vma = NULL;
else
vma = ERR_PTR(err);
goto err;
}

vma = i915_gem_object_ggtt_pin(pool->obj, NULL, 0, 0, 0);
if (IS_ERR(vma))
goto err;

eb->vma[eb->buffer_count] = i915_vma_get(vma);
eb->flags[eb->buffer_count] =
__EXEC_OBJECT_HAS_PIN | __EXEC_OBJECT_HAS_REF;
vma->exec_flags = &eb->flags[eb->buffer_count];
eb->buffer_count++;

eb->batch_start_offset = 0;
eb->batch = vma;

if (CMDPARSER_USES_GGTT(eb->i915))
eb->batch_flags |= I915_DISPATCH_SECURE;

/* eb->batch_len unchanged */

vma->private = pool;
return vma;

Expand Down Expand Up @@ -2421,6 +2477,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
struct drm_i915_gem_exec_object2 *exec,
struct drm_syncobj **fences)
{
struct drm_i915_private *i915 = to_i915(dev);
struct i915_execbuffer eb;
struct dma_fence *in_fence = NULL;
struct dma_fence *exec_fence = NULL;
Expand All @@ -2432,7 +2489,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
BUILD_BUG_ON(__EXEC_OBJECT_INTERNAL_FLAGS &
~__EXEC_OBJECT_UNKNOWN_FLAGS);

eb.i915 = to_i915(dev);
eb.i915 = i915;
eb.file = file;
eb.args = args;
if (DBG_FORCE_RELOC || !(args->flags & I915_EXEC_NO_RELOC))
Expand All @@ -2452,8 +2509,15 @@ i915_gem_do_execbuffer(struct drm_device *dev,

eb.batch_flags = 0;
if (args->flags & I915_EXEC_SECURE) {
if (INTEL_GEN(i915) >= 11)
return -ENODEV;

/* Return -EPERM to trigger fallback code on old binaries. */
if (!HAS_SECURE_BATCHES(i915))
return -EPERM;

if (!drm_is_current_master(file) || !capable(CAP_SYS_ADMIN))
return -EPERM;
return -EPERM;

eb.batch_flags |= I915_DISPATCH_SECURE;
}
Expand Down Expand Up @@ -2530,34 +2594,19 @@ i915_gem_do_execbuffer(struct drm_device *dev,
goto err_vma;
}

if (eb.batch_len == 0)
eb.batch_len = eb.batch->size - eb.batch_start_offset;

if (eb_use_cmdparser(&eb)) {
struct i915_vma *vma;

vma = eb_parse(&eb, drm_is_current_master(file));
vma = eb_parse(&eb);
if (IS_ERR(vma)) {
err = PTR_ERR(vma);
goto err_vma;
}

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.
*/
eb.batch_flags |= I915_DISPATCH_SECURE;
eb.batch_start_offset = 0;
eb.batch = vma;
}
}

if (eb.batch_len == 0)
eb.batch_len = eb.batch->size - eb.batch_start_offset;

/*
* snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure
* batch" bit. Hence we need to pin secure batches into the global gtt.
Expand Down
13 changes: 10 additions & 3 deletions drivers/gpu/drm/i915/gt/intel_engine_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -475,12 +475,13 @@ struct intel_engine_cs {

struct intel_engine_hangcheck hangcheck;

#define I915_ENGINE_NEEDS_CMD_PARSER BIT(0)
#define I915_ENGINE_USING_CMD_PARSER BIT(0)
#define I915_ENGINE_SUPPORTS_STATS BIT(1)
#define I915_ENGINE_HAS_PREEMPTION BIT(2)
#define I915_ENGINE_HAS_SEMAPHORES BIT(3)
#define I915_ENGINE_NEEDS_BREADCRUMB_TASKLET BIT(4)
#define I915_ENGINE_IS_VIRTUAL BIT(5)
#define I915_ENGINE_REQUIRES_CMD_PARSER BIT(7)
unsigned int flags;

/*
Expand Down Expand Up @@ -541,9 +542,15 @@ struct intel_engine_cs {
};

static inline bool
intel_engine_needs_cmd_parser(const struct intel_engine_cs *engine)
intel_engine_using_cmd_parser(const struct intel_engine_cs *engine)
{
return engine->flags & I915_ENGINE_NEEDS_CMD_PARSER;
return engine->flags & I915_ENGINE_USING_CMD_PARSER;
}

static inline bool
intel_engine_requires_cmd_parser(const struct intel_engine_cs *engine)
{
return engine->flags & I915_ENGINE_REQUIRES_CMD_PARSER;
}

static inline bool
Expand Down
8 changes: 8 additions & 0 deletions drivers/gpu/drm/i915/gt/intel_gt_pm.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ static int __gt_unpark(struct intel_wakeref *wf)
gt->awake = intel_display_power_get(i915, POWER_DOMAIN_GT_IRQ);
GEM_BUG_ON(!gt->awake);

if (NEEDS_RC6_CTX_CORRUPTION_WA(i915))
intel_uncore_forcewake_get(&i915->uncore, FORCEWAKE_ALL);

intel_enable_gt_powersave(i915);

i915_update_gfx_val(i915);
Expand Down Expand Up @@ -67,6 +70,11 @@ static int __gt_park(struct intel_wakeref *wf)
if (INTEL_GEN(i915) >= 6)
gen6_rps_idle(i915);

if (NEEDS_RC6_CTX_CORRUPTION_WA(i915)) {
i915_rc6_ctx_wa_check(i915);
intel_uncore_forcewake_put(&i915->uncore, FORCEWAKE_ALL);
}

/* Everything switched off, flush any residual interrupt just in case */
intel_synchronize_irq(i915);

Expand Down
Loading

0 comments on commit 100d46b

Please sign in to comment.