[PATCH 00/13] Gen7 batch buffer command parser

bradley.d.volkin
2014-02-18T18:17:26Z

From: Brad Volkin <bradley.d.volkin@intel.com>

Certain OpenGL features (e.g. transform feedback, performance monitoring)
require userspace code to submit batches containing commands such as
MI_LOAD_REGISTER_IMM to access various registers. Unfortunately, some
generations of the hardware will noop these commands in "unsecure" batches
(which includes all userspace batches submitted via i915) even though the
commands may be safe and represent the intended programming model of the device.

This series introduces a software command parser similar in operation to the
command parsing done in hardware for unsecure batches. However, the software
parser allows some operations that would be noop'd by hardware, if the parser
determines the operation is safe, and submits the batch as "secure" to prevent
hardware parsing. Currently the series implements this on IVB and HSW.

The series has one piece of prep work, one patch for the parser logic, and a
handful of patches to fill out the tables which drive the parser. There are
follow-up patches to libdrm and to i-g-t. The i-g-t tests are basic and do not
test all of the commands used by the parser on the assumption that I'm likely
to make the same mistakes in both the parser and the test.

I've previously run the i-g-t gem_* tests, the piglit quick tests, and generally
used Ubuntu 13.10 IVB and HSW systems with the parser running. Aside from a
failure described below, I did not see any regressions.

At this point there are a couple of required/potential improvements.

1) Chained batches. The parser currently allows MI_BATCH_BUFFER_START commands
in userspace batches without parsing them. The media driver uses chained
batches, so a solution is required. I'm still working through the
requirements but don't want to continue delaying the review process for what
I have so far.
2) Command buffer copy. To avoid CPU modifications to buffers after parsing, and
to avoid GPU modifications to buffers via EUs or commands in the batch, we
should copy the userspace batch buffer to memory that userspace does not
have access to, map it into GGTT, and execute that batch buffer. I have a
sense of how to do this for 1st-level batches, but it may need changes to
tie in with the chained batch parsing, so I've again held off.
3) Coherency. I've previously found a coherency issue on VLV when reading the
batch buffer from the CPU during execbuffer2. Userspace writes the batch via
pwrite fast path before calling execbuffer2. The parser reads stale data.
This works fine on IVB and HSW, so I believe it's an LLC vs. non-LLC issue.
It's possible that the shmem pread refactoring fixes this, I just have not
been able to retest due to lack of a VLV system.

v2:
- Significantly reorder series
- Scan secure batches (i.e. I915_EXEC_SECURE)
- Check that parser tables are sorted during init
- Fixed gem_cpu_reloc regression
- HAS_CMD_PARSER -> CMD_PARSER_VERSION getparam
- Additional tests

v3:
- Don't actually send batches as secure yet
- Improved documentation and commenting
- Many other small cleanups throughout

Brad Volkin (13):
drm/i915: Refactor shmem pread setup
drm/i915: Implement command buffer parsing logic
drm/i915: Initial command parser table definitions
drm/i915: Reject privileged commands
drm/i915: Allow some privileged commands from master
drm/i915: Add register whitelists for mesa
drm/i915: Add register whitelist for DRM master
drm/i915: Enable register whitelist checks
drm/i915: Reject commands that explicitly generate interrupts
drm/i915: Enable PPGTT command parser checks
drm/i915: Reject commands that would store to global HWS page
drm/i915: Add a CMD_PARSER_VERSION getparam
drm/i915: Enable command parsing by default

drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_cmd_parser.c | 918 +++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_dma.c | 3 +
drivers/gpu/drm/i915/i915_drv.h | 103 ++++
drivers/gpu/drm/i915/i915_gem.c | 51 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 18 +
drivers/gpu/drm/i915/i915_params.c | 5 +
drivers/gpu/drm/i915/i915_reg.h | 96 +++
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +
drivers/gpu/drm/i915/intel_ringbuffer.h | 32 +
include/uapi/drm/i915_drm.h | 1 +
11 files changed, 1216 insertions(+), 14 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_cmd_parser.c

--
1.8.3.2