Re: [Intel-gfx] [PATCH 10/11] drm/i915: Add 180 degree primary plane rotation support
On Sat, Feb 01, 2014 at 12:40:46AM +0530, sagar.a.kam...@intel.com wrote:
From: Sagar Kamble sagar.a.kam...@intel.com
Primary planes support 180 degree rotation. Expose the feature
through rotation drm property.
Signed-off-by: Uma Shankar uma.shan...@intel.com
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
Tested-by: Sagar Kamble sagar.a.kam...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 54
++--
drivers/gpu/drm/i915/intel_drv.h | 2 ++
3 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 57906c5..d3000c4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3553,6 +3553,7 @@
#define DISPPLANE_NO_LINE_DOUBLE 0
#define DISPPLANE_STEREO_POLARITY_FIRST0
#define DISPPLANE_STEREO_POLARITY_SECOND (118)
+#define DISPPLANE_ROTATE_180 (115)
#define DISPPLANE_TRICKLE_FEED_DISABLE (114) /* Ironlake */
#define DISPPLANE_TILED(110)
#define _DSPAADDR(dev_priv-info-display_mmio_offset + 0x70184)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4d4a0d9..483de59 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2037,6 +2037,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
unsigned long linear_offset;
u32 dspcntr;
u32 reg;
+ int pixel_size;
switch (plane) {
case 0:
@@ -2047,6 +2048,7 @@ static int i9xx_update_plane(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
return -EINVAL;
}
+ pixel_size = drm_format_plane_cpp(fb-pixel_format, 0);
intel_fb = to_intel_framebuffer(fb);
obj = intel_fb-obj;
@@ -2054,6 +2056,8 @@ static int i9xx_update_plane(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
dspcntr = I915_READ(reg);
/* Mask out pixel format bits in case we change it */
dspcntr = ~DISPPLANE_PIXFORMAT_MASK;
+ dspcntr = ~DISPPLANE_ROTATE_180;
+
switch (fb-pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2095,8 +2099,6 @@ static int i9xx_update_plane(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
- I915_WRITE(reg, dspcntr);
-
linear_offset = y * fb-pitches[0] + x * (fb-bits_per_pixel / 8);
if (INTEL_INFO(dev)-gen = 4) {
@@ -2109,6 +2111,17 @@ static int i9xx_update_plane(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
intel_crtc-dspaddr_offset = linear_offset;
}
+ if (intel_crtc-rotation == BIT(DRM_ROTATE_180)) {
+ dspcntr |= DISPPLANE_ROTATE_180;
+
+ x += (fb-width - 1);
+ y += (fb-height - 1);
+ linear_offset += (fb-height - 1) * fb-pitches[0] +
+ fb-width * pixel_size;
+ }
+
+ I915_WRITE(reg, dspcntr);
+
DRM_DEBUG_KMS(Writing base %08lX %08lX %d %d %d\n,
i915_gem_obj_ggtt_offset(obj), linear_offset, x, y,
fb-pitches[0]);
@@ -8748,6 +8761,31 @@ free_work:
return ret;
}
+static int intel_crtc_set_property(struct drm_crtc *crtc,
+ struct drm_property *prop,
+ uint64_t val)
+{
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ uint64_t old_val;
+ int ret = -ENOENT;
+
+ if (prop == dev_priv-rotation_property) {
+ /* exactly one rotation angle please */
+ if (hweight32(val 0xf) != 1)
+ return -EINVAL;
+
+ old_val = intel_crtc-rotation;
+ intel_crtc-rotation = val;
Couple of other things we'd need to do here is check if the crtc is even
active, since calling .update_plane() on an inactive crtc would oops.
Also we need to wait for pending page flips to make sure we don't
overtake them.
Additionall the FBC code would need some rotation checks since FBC
doesn't work with a rotated plane on certain generations.
+ ret = dev_priv-display.update_plane(crtc, crtc-fb, 0, 0);
+ if (ret)
+ intel_crtc-rotation = old_val;
+ }
+
+ return ret;
+}
+
static struct drm_crtc_helper_funcs intel_helper_funcs = {
.mode_set_base_atomic = intel_pipe_set_base_atomic,
.load_lut = intel_crtc_load_lut,
@@ -10160,6 +10198,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs =
{
.set_config = intel_crtc_set_config,
.destroy = intel_crtc_destroy,
[Intel-gfx] [PATCH 0/4] tests: New userptr test case
From: Tvrtko Ursulin tvrtko.ursu...@intel.com This patch series replaces the old vmap with the new userptr test case. Some refactoring precedes the actual introduction to pull out code from gem_evict_everything which is useful for both. Test case at the moment tests dmabuf sharing of userptr objects which is not currently implemented in the latest kernel patch. Tvrtko Ursulin (4): tests/gem_evict_everything: Factor out eviction logic tests/eviction_common: Avoid submitting duplicate objects tests/gem_userptr_blits: Expanded userptr test cases tests/gem_vmap_blits: Remove obsolete test case tests/.gitignore |2 +- tests/Makefile.sources |2 +- tests/eviction_common.c | 242 + tests/gem_evict_everything.c | 210 ++-- tests/gem_userptr_blits.c| 1185 ++ tests/gem_vmap_blits.c | 344 6 files changed, 1470 insertions(+), 515 deletions(-) create mode 100644 tests/eviction_common.c create mode 100644 tests/gem_userptr_blits.c delete mode 100644 tests/gem_vmap_blits.c -- 1.8.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl
On 01/29/2014 08:34 PM, Daniel Vetter wrote: Actually I've found something else to complain about: On Tue, Jan 28, 2014 at 2:16 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: +#define I915_USERPTR_READ_ONLY 0x1 This smells like an insta-root-exploit: 1. mmap /lib/ld-linux.so as read-only 2. userptr bind that mmap'ed area as READ_ONLY 3. blit exploit code over it 4. profit I also don't see a way we could fix this, at least without the hardware providing read-only modes in the ptes. Which also requires us to actually trust it to follow them, even when they exists ... Would disallowing mapping of shared pages help and be acceptable considering intended use cases? Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: vlv: don't unmask IIR[DISPLAY_PIPE_A/B_VBLANK] interrupt
Bspec and the code suggests that the interrupt signaled by IIR[7,5]
(DISPLAY_PIPE_A/B_VBLANK) is a first level IRQ flag for the second
level PIPEA/BSTAT[2] (Start of Vertical Blank) interrupt. Measuring
the relative timings of when IIR[7] and PIPEASTAT[1,2] get set and
checking the effect of unmasking different pipestat and IIR events
shows that this isn't so:
First, ISR/IIR[7] gets set independently of PIPEASTAT[18] (Start of
Vertical Blank Enable) or any other pipestat enable bit, so it isn't
a first level IRQ bit showing the state of PIPEASTAT[2], but is
connected directly to the timing generator.
Second, setting only PIPEASTAT[18] and leaving all other pipestat events
disabled, IIR[6] (DISPLAY_PIPE_A_EVENT) gets set close to the moment when
PIPEASTAT[2] gets set, so the former is a first level interrupt flag for
the latter. The bspec is rather unclear about this, but I also assume
that IIR[6] signals all pipestat A events, except PIPEASTAT[31] (FIFO
Under-run Status).
Third, IIR[7] is set close to the moment when PIPEASTAT[1] (Framestart
Interrupt) gets set, in the mode I used about 12usec after PIPEASTAT[2]
and IIR[6] gets set. This means the IIR[7] isn't marking the start of
vblank, but rather signals the framestart event.
Based on the above, we don't need to unmask IIR[7] when waiting for
start of vblank events, but we can rely on IIR[6] being always unmasked,
which will signal when PIPEASTAT[2] gets set. Doing this will also get
rid of the overhead of getting an interrupt and servicing IIR[7], which
is atm raised always some time after IIR[6]/PIPEASTAT[2] is raised.
Signed-off-by: Imre Deak imre.d...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b226ae6..137ac65 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2297,18 +2297,11 @@ static int valleyview_enable_vblank(struct drm_device
*dev, int pipe)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
unsigned long irqflags;
- u32 imr;
if (!i915_pipe_enabled(dev, pipe))
return -EINVAL;
spin_lock_irqsave(dev_priv-irq_lock, irqflags);
- imr = I915_READ(VLV_IMR);
- if (pipe == PIPE_A)
- imr = ~I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT;
- else
- imr = ~I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
- I915_WRITE(VLV_IMR, imr);
i915_enable_pipestat(dev_priv, pipe,
PIPE_START_VBLANK_INTERRUPT_ENABLE);
spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
@@ -2366,17 +2359,10 @@ static void valleyview_disable_vblank(struct drm_device
*dev, int pipe)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev-dev_private;
unsigned long irqflags;
- u32 imr;
spin_lock_irqsave(dev_priv-irq_lock, irqflags);
i915_disable_pipestat(dev_priv, pipe,
PIPE_START_VBLANK_INTERRUPT_ENABLE);
- imr = I915_READ(VLV_IMR);
- if (pipe == PIPE_A)
- imr |= I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT;
- else
- imr |= I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT;
- I915_WRITE(VLV_IMR, imr);
spin_unlock_irqrestore(dev_priv-irq_lock, irqflags);
}
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] tests/eviction_common: Avoid submitting duplicate objects
From: Tvrtko Ursulin tvrtko.ursu...@intel.com Make sure selection loop does not generate duplicates when it picks a subset of objects for a single exec buffer. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com --- tests/eviction_common.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/tests/eviction_common.c b/tests/eviction_common.c index efe560c..91fb2df 100644 --- a/tests/eviction_common.c +++ b/tests/eviction_common.c @@ -65,6 +65,13 @@ static int minor_evictions(int fd, struct igt_eviction_test_ops *ops, uint32_t *bo, *sel; int n, m, pass, fail; + /* Make sure nr_surfaces is not divisible by seven +* to avoid duplicates in the selection loop below. +*/ + nr_surfaces /= 7; + nr_surfaces *= 7; + nr_surfaces += 3; + igt_require((uint64_t)nr_surfaces * surface_size / (1024 * 1024) intel_get_total_ram_mb() * 9 / 10); -- 1.8.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] tests/gem_userptr_blits: Expanded userptr test cases
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
A set of userptr test cases to support the new feature.
For the eviction and swapping stress testing I have extracted
some common behaviour from gem_evict_everything and made both
test cases use it to avoid duplicating the code.
Both unsynchronized and synchronized userptr objects are
tested but the latter set of tests will be skipped if kernel
is compiled without MMU_NOTIFIERS.
Also, with 32-bit userspace swapping tests are skipped if
the system has a lot more RAM than process address space.
Forking swapping tests are not skipped since they can still
trigger swapping by cumulative effect.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
---
tests/.gitignore |1 +
tests/Makefile.sources|1 +
tests/gem_userptr_blits.c | 1185 +
3 files changed, 1187 insertions(+)
create mode 100644 tests/gem_userptr_blits.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 7377275..77ab34e 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -92,6 +92,7 @@ gem_tiling_max_stride
gem_unfence_active_buffers
gem_unref_active_buffers
gem_vmap_blits
+gem_userptr_blits
gem_wait_render_timeout
gem_write_read_ring_switch
gen3_mixed_blits
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a8c0c96..7699a84 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -116,6 +116,7 @@ TESTS_progs = \
gem_unfence_active_buffers \
gem_unref_active_buffers \
gem_vmap_blits \
+ gem_userptr_blits \
gem_wait_render_timeout \
gen3_mixed_blits \
gen3_render_linear_blits \
diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
new file mode 100644
index 000..1472b67
--- /dev/null
+++ b/tests/gem_userptr_blits.c
@@ -0,0 +1,1185 @@
+/*
+ * Copyright © 2009-2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Eric Anholt e...@anholt.net
+ *Chris Wilson ch...@chris-wilson.co.uk
+ *Tvrtko Ursulin tvrtko.ursu...@intel.com
+ *
+ */
+
+/** @file gem_userptr_blits.c
+ *
+ * This is a test of doing many blits using a mixture of normal system pages
+ * and uncached linear buffers, with a working set larger than the
+ * aperture size.
+ *
+ * The goal is to simply ensure the basics work.
+ */
+
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/time.h
+#include sys/mman.h
+#include drm.h
+#include i915_drm.h
+#include drmtest.h
+#include intel_bufmgr.h
+#include intel_batchbuffer.h
+#include intel_gpu_tools.h
+
+#include eviction_common.c
+
+#define WIDTH 512
+#define HEIGHT 512
+#define PAGE_SIZE 4096
+
+#define LOCAL_I915_GEM_USERPTR 0x34
+#define LOCAL_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE +
LOCAL_I915_GEM_USERPTR, struct local_i915_gem_userptr)
+struct local_i915_gem_userptr {
+ uint64_t user_ptr;
+ uint64_t user_size;
+ uint32_t flags;
+#define I915_USERPTR_READ_ONLY (10)
+#define I915_USERPTR_UNSYNCHRONIZED (131)
+ uint32_t handle;
+};
+
+static uint32_t userptr_flags;
+
+static uint32_t linear[WIDTH*HEIGHT];
+
+static void gem_userptr_test_unsynchronized(void)
+{
+ userptr_flags = I915_USERPTR_UNSYNCHRONIZED;
+}
+
+static void gem_userptr_test_synchronized(void)
+{
+ userptr_flags = 0;
+}
+
+static int gem_userptr(int fd, void *ptr, int size, int read_only, uint32_t
*handle)
+{
+ struct local_i915_gem_userptr userptr;
+ int ret;
+
+ userptr.user_ptr = (uintptr_t)ptr;
+ userptr.user_size = size;
+ userptr.flags = userptr_flags;
+ if (read_only)
+ userptr.flags |= I915_USERPTR_READ_ONLY;
+
+ ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_USERPTR, userptr);
+ if (ret)
+ ret = errno;
+
[Intel-gfx] [PATCH 1/4] tests/gem_evict_everything: Factor out eviction logic
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
In preparation for userptr test we move the eviction logic
into a common file so it can be used from both test cases.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
---
tests/eviction_common.c | 235 +++
tests/gem_evict_everything.c | 210 --
2 files changed, 276 insertions(+), 169 deletions(-)
create mode 100644 tests/eviction_common.c
diff --git a/tests/eviction_common.c b/tests/eviction_common.c
new file mode 100644
index 000..efe560c
--- /dev/null
+++ b/tests/eviction_common.c
@@ -0,0 +1,235 @@
+/*
+ * Copyright © 2007, 2011, 2013, 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Eric Anholt e...@anholt.net
+ *Daniel Vetter daniel.vet...@ffwll.ch
+ *Tvrtko Ursulin tvrtko.ursu...@intel.com
+ *
+ */
+
+#include stdlib.h
+
+#include drmtest.h
+
+struct igt_eviction_test_ops
+{
+ uint32_t (*create)(int fd, int size);
+ void (*close)(int fd, uint32_t bo);
+ void (*copy)(int fd, uint32_t dst, uint32_t src,
+uint32_t *all_bo, int nr_bos, int error);
+ void (*clear)(int fd, uint32_t bo, int size);
+};
+
+#define FORKING_EVICTIONS_INTERRUPTIBLE (1 0)
+#define FORKING_EVICTIONS_SWAPPING (1 1)
+#define FORKING_EVICTIONS_DUP_DRMFD (1 2)
+#define FORKING_EVICTIONS_MEMORY_PRESSURE (1 3)
+#define ALL_FORKING_EVICTIONS (FORKING_EVICTIONS_INTERRUPTIBLE | \
+FORKING_EVICTIONS_SWAPPING | \
+FORKING_EVICTIONS_DUP_DRMFD | \
+FORKING_EVICTIONS_MEMORY_PRESSURE)
+
+static void exchange_uint32_t(void *array, unsigned i, unsigned j)
+{
+ uint32_t *i_arr = array;
+ uint32_t i_tmp;
+
+ i_tmp = i_arr[i];
+ i_arr[i] = i_arr[j];
+ i_arr[j] = i_tmp;
+}
+
+static int minor_evictions(int fd, struct igt_eviction_test_ops *ops,
+ int surface_size, int nr_surfaces)
+{
+ uint32_t *bo, *sel;
+ int n, m, pass, fail;
+
+ igt_require((uint64_t)nr_surfaces * surface_size / (1024 * 1024)
+intel_get_total_ram_mb() * 9 / 10);
+
+ bo = malloc(3*nr_surfaces*sizeof(*bo));
+ igt_assert(bo);
+
+ for (n = 0; n 2*nr_surfaces; n++)
+ bo[n] = ops-create(fd, surface_size);
+
+ sel = bo + n;
+ for (fail = 0, m = 0; fail 10; fail++) {
+ for (pass = 0; pass 100; pass++) {
+ for (n = 0; n nr_surfaces; n++, m += 7)
+ sel[n] = bo[m%(2*nr_surfaces)];
+ ops-copy(fd, sel[0], sel[1], sel, nr_surfaces, 0);
+ }
+ ops-copy(fd, bo[0], bo[0], bo, 2*nr_surfaces, ENOSPC);
+ }
+
+ for (n = 0; n 2*nr_surfaces; n++)
+ ops-close(fd, bo[n]);
+ free(bo);
+
+ return 0;
+}
+
+static int major_evictions(int fd, struct igt_eviction_test_ops *ops,
+ int surface_size, int nr_surfaces)
+{
+ int n, m, loop;
+ uint32_t *bo;
+
+ igt_require((uint64_t)nr_surfaces * surface_size / (1024 * 1024)
+intel_get_total_ram_mb() * 9 / 10);
+
+ bo = malloc(nr_surfaces*sizeof(*bo));
+ igt_assert(bo);
+
+ for (n = 0; n nr_surfaces; n++)
+ bo[n] = ops-create(fd, surface_size);
+
+ for (loop = 0, m = 0; loop 100; loop++, m += 17) {
+ n = m % nr_surfaces;
+ ops-copy(fd, bo[n], bo[n], bo[n], 1, 0);
+ }
+
+ for (n = 0; n nr_surfaces; n++)
+ ops-close(fd, bo[n]);
+ free(bo);
+
+ return 0;
+}
+
+static int swapping_evictions(int fd, struct igt_eviction_test_ops *ops,
+ int
Re: [Intel-gfx] [PATCH] drm/i915: Introduce mapping of user pages into video memory (userptr) ioctl
On 01/30/2014 11:06 AM, Chris Wilson wrote: On Wed, Jan 29, 2014 at 10:58:48PM +0100, Daniel Vetter wrote: On Wed, Jan 29, 2014 at 10:53 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Jan 29, 2014 at 09:25:51PM +0100, Daniel Vetter wrote: So originally I've thought we need this due to the massive overhead of the mmu notifier. But now with the nice shared mmu notifiers I've thought that overhead is gone I prefer to also ditch this option. Same goes about the MMU_NOTIFIER conditional code, imo we simply should select this - most distros will have it anyway and users will be really suprised if they lose userspace driver features for seemingly irrelevant reasons. Seriously? You think the overhead is magically gone? Well the once-per-process overhead is still there, and imo it's ok to eat that. But the complaints I've heard concerned the per-object overhead, so I wonder how much of that is still relevant. I am still annoyed by the thought of having to enable an extra feature in my kernels, and the extra code that is then run on every mm operation. (Mixing mmu_notifiers + mm debuging was an especially unpleasant experience that I don't wish to ever do again.) Numbers talk though, if we can't demonstrate a significant difference between the two, it can die. Keeping a debug mode to turn off mmu_notifiers would still be good so that we can keep track of any impact over time. Writing a benchmark for this is next on my userptr to do list following completing of the i-g-t test case. Btw, I did not notice you are discussing this sooner since I got dropped from Cc. Only when Rafael mentioned he saw some discussion about potential exploit I went looking. Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] tests/gem_vmap_blits: Remove obsolete test case
From: Tvrtko Ursulin tvrtko.ursu...@intel.com
No need for the old test case once the new one was added.
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
---
tests/.gitignore | 1 -
tests/Makefile.sources | 1 -
tests/gem_vmap_blits.c | 344 -
3 files changed, 346 deletions(-)
delete mode 100644 tests/gem_vmap_blits.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 77ab34e..798eeed 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -91,7 +91,6 @@ gem_tiled_swapping
gem_tiling_max_stride
gem_unfence_active_buffers
gem_unref_active_buffers
-gem_vmap_blits
gem_userptr_blits
gem_wait_render_timeout
gem_write_read_ring_switch
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 7699a84..2d42a18 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -115,7 +115,6 @@ TESTS_progs = \
gem_tiling_max_stride \
gem_unfence_active_buffers \
gem_unref_active_buffers \
- gem_vmap_blits \
gem_userptr_blits \
gem_wait_render_timeout \
gen3_mixed_blits \
diff --git a/tests/gem_vmap_blits.c b/tests/gem_vmap_blits.c
deleted file mode 100644
index 48297af..000
--- a/tests/gem_vmap_blits.c
+++ /dev/null
@@ -1,344 +0,0 @@
-/*
- * Copyright © 2009,2011 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the Software),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- *Eric Anholt e...@anholt.net
- *Chris Wilson ch...@chris-wilson.co.uk
- *
- */
-
-/** @file gem_vmap_blits.c
- *
- * This is a test of doing many blits using a mixture of normal system pages
- * and uncached linear buffers, with a working set larger than the
- * aperture size.
- *
- * The goal is to simply ensure the basics work.
- */
-
-#include stdlib.h
-#include stdio.h
-#include string.h
-#include fcntl.h
-#include inttypes.h
-#include errno.h
-#include sys/stat.h
-#include sys/time.h
-#include drm.h
-#include i915_drm.h
-#include drmtest.h
-#include intel_bufmgr.h
-#include intel_batchbuffer.h
-#include intel_gpu_tools.h
-
-#if !defined(I915_PARAM_HAS_VMAP)
-#pragma message(No vmap support in drm, skipping)
-int main(int argc, char **argv)
-{
- fprintf(stderr, No vmap support in drm.\n);
- return 77;
-}
-#else
-
-#define WIDTH 512
-#define HEIGHT 512
-
-static uint32_t linear[WIDTH*HEIGHT];
-
-static uint32_t gem_vmap(int fd, void *ptr, int size, int read_only)
-{
- struct drm_i915_gem_vmap vmap;
-
- vmap.user_ptr = (uintptr_t)ptr;
- vmap.user_size = size;
- vmap.flags = 0;
- if (read_only)
- vmap.flags |= I915_VMAP_READ_ONLY;
-
- if (drmIoctl(fd, DRM_IOCTL_I915_GEM_VMAP, vmap))
- return 0;
-
- return vmap.handle;
-}
-
-
-static void gem_vmap_sync(int fd, uint32_t handle)
-{
- gem_set_domain(fd, handle, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
-}
-
-static void
-copy(int fd, uint32_t dst, uint32_t src)
-{
- uint32_t batch[10];
- struct drm_i915_gem_relocation_entry reloc[2];
- struct drm_i915_gem_exec_object2 obj[3];
- struct drm_i915_gem_execbuffer2 exec;
- uint32_t handle;
- int ret;
-
- batch[0] = XY_SRC_COPY_BLT_CMD |
- XY_SRC_COPY_BLT_WRITE_ALPHA |
- XY_SRC_COPY_BLT_WRITE_RGB | 6;
- batch[1] = (3 24) | /* 32 bits */
- (0xcc 16) | /* copy ROP */
- WIDTH*4;
- batch[2] = 0; /* dst x1,y1 */
- batch[3] = (HEIGHT 16) | WIDTH; /* dst x2,y2 */
- batch[4] = 0; /* dst reloc */
- batch[5] = 0; /* src x1,y1 */
- batch[6] = WIDTH*4;
- batch[7] = 0; /* src reloc */
- batch[8] = MI_BATCH_BUFFER_END;
- batch[9] = MI_NOOP;
-
- handle = gem_create(fd, 4096);
- gem_write(fd, handle, 0, batch, sizeof(batch));
-
- reloc[0].target_handle = dst;
- reloc[0].delta = 0;
- reloc[0].offset = 4 *
Re: [Intel-gfx] [PATCH 02/13] drm/i915: Implement command buffer parsing logic
Ping. Daniel or Chris, can one of you clarify this request? Thanks.
On Thu, Jan 30, 2014 at 10:05:27AM -0800, Volkin, Bradley D wrote:
On Thu, Jan 30, 2014 at 03:07:15AM -0800, Daniel Vetter wrote:
On Thu, Jan 30, 2014 at 10:12:06AM +0100, Daniel Vetter wrote:
On Thu, Jan 30, 2014 at 10:05:28AM +0100, Daniel Vetter wrote:
On Thu, Jan 30, 2014 at 09:53:28AM +0100, Daniel Vetter wrote:
On Wed, Jan 29, 2014 at 10:28:36PM +, Chris Wilson wrote:
On Wed, Jan 29, 2014 at 01:55:03PM -0800,
bradley.d.vol...@intel.com wrote:
+/*
+ * Returns a pointer to a descriptor for the command specified
by cmd_header.
+ *
+ * The caller must supply space for a default descriptor via the
default_desc
+ * parameter. If no descriptor for the specified command exists
in the ring's
+ * command parser tables, this function fills in default_desc
based on the
+ * ring's default length encoding and returns default_desc.
+ */
+static const struct drm_i915_cmd_descriptor*
+find_cmd(struct intel_ring_buffer *ring,
+ u32 cmd_header,
+ struct drm_i915_cmd_descriptor *default_desc)
+{
+ u32 mask;
+ int i;
+
+ for (i = 0; i ring-cmd_table_count; i++) {
+ const struct drm_i915_cmd_descriptor *desc;
+
+ desc = find_cmd_in_table(ring-cmd_tables[i],
cmd_header);
+ if (desc)
+ return desc;
+ }
+
+ mask = ring-get_cmd_length_mask(cmd_header);
+ if (!mask)
+ return NULL;
+
+ BUG_ON(!default_desc);
+ default_desc-flags = CMD_DESC_SKIP;
+ default_desc-length.mask = mask;
If we turn off all hw validation (through use of the secure bit)
should
we not default to a whitelist of commands? Otherwise it just seems
to be
a case of running a fuzzer until we kill the machine.
Preventing hangs and dos is imo not the attack model, gpus are too
fickle
for that. The attach model here is to prevent priveledge escalation
and
information leaks. I.e. we want just containement of all read/write
access
to the gtt space.
I think for that purpose an explicit whitelist of commands which
target
things outside of the (pp)gtt is sufficient. radeon's checker design
is
completely different, but pretty much the only command they have is
to load register values. Intel gpus otoh have a big set of
special-purpose
commands to load (most) of the rendering pipeline state. So we have
hw built-in register whitelists for all that stuff since you just
can't
load arbitrary registers and state with those commands.
Also note that for raw register access Bradley's scanner _is_
whitelist
based. And for general reads/writes gpu designers confirmed that
those are
all MI_ commands (with very few specific exceptions like
PIPE_CONTROL), so
as long as we check for the exceptions and otherwise only whitelist
MI_
commands we know about we should be covered.
So I think this is sound.
Hm, but while scrolling through the checker I haven't spotted a reject
everything unknown for MI_CLIENT commands. Bradley, have I missed that?
I think submitting an invented MI_CLIENT command would also be a good
testcase.
One more: I think it would be good to have an overview comment at the top
of i915_cmd_parser.c which details the security attack model and the
overall blacklist/whitelist design of the checker. We don't (yet) have
autogenerated documentation for i915, but that's something I'm working on.
And the kerneldoc system can also pull in multi-paragraph overview
comments besides the usual api documentation, so it's good to have things
ready.
Chatted with Chris a bit more on irc about this, and for more paranoia I
guess we should also reject any unknown client and media subclient
commands.
Hmm, not sure I follow. Can you elaborate?
Are you suggesting we add all the MI and Media commands to the tables and
reject
any command from those client/subclients that is not found in the table? Or
that
we look at the client and subclient fields of the command and reject if they
are
not from a set of expected values? Or other?
- Brad
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
