* [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode
@ 2018-06-14 19:24 Chris Wilson
2018-06-14 19:24 ` [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+ Chris Wilson
` (12 more replies)
0 siblings, 13 replies; 21+ messages in thread
From: Chris Wilson @ 2018-06-14 19:24 UTC (permalink / raw)
To: intel-gfx
From: Jon Bloomfield <jon.bloomfield@intel.com>
We can set a bit inside the ppGTT PTE to indicate a page is read-only;
writes from the GPU will be discarded. We can use this to protect pages
and in particular support read-only userptr mappings (necessary for
importing PROT_READ vma).
Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8883e75cb594..c3630abbd260 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -244,10 +244,13 @@ static void clear_pages(struct i915_vma *vma)
}
static gen8_pte_t gen8_pte_encode(dma_addr_t addr,
- enum i915_cache_level level)
+ enum i915_cache_level level,
+ u32 flags)
{
- gen8_pte_t pte = _PAGE_PRESENT | _PAGE_RW;
- pte |= addr;
+ gen8_pte_t pte = addr | _PAGE_PRESENT | _PAGE_RW;
+
+ if (unlikely(flags & PTE_READ_ONLY))
+ pte &= ~_PAGE_RW;
switch (level) {
case I915_CACHE_NONE:
@@ -637,7 +640,7 @@ static void gen8_initialize_pt(struct i915_address_space *vm,
struct i915_page_table *pt)
{
fill_px(vm, pt,
- gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC));
+ gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0));
}
static void gen6_initialize_pt(struct i915_address_space *vm,
@@ -786,7 +789,7 @@ static bool gen8_ppgtt_clear_pt(struct i915_address_space *vm,
unsigned int pte = gen8_pte_index(start);
unsigned int pte_end = pte + num_entries;
const gen8_pte_t scratch_pte =
- gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC);
+ gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
gen8_pte_t *vaddr;
GEM_BUG_ON(num_entries > pt->used_ptes);
@@ -961,7 +964,7 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
enum i915_cache_level cache_level)
{
struct i915_page_directory *pd;
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
gen8_pte_t *vaddr;
bool ret;
@@ -1029,7 +1032,7 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
struct sgt_dma *iter,
enum i915_cache_level cache_level)
{
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
u64 start = vma->node.start;
dma_addr_t rem = iter->sg->length;
@@ -1495,7 +1498,7 @@ static void gen8_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
{
struct i915_address_space *vm = &ppgtt->vm;
const gen8_pte_t scratch_pte =
- gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC);
+ gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
u64 start = 0, length = ppgtt->vm.total;
if (use_4lvl(vm)) {
@@ -2397,7 +2400,7 @@ static void gen8_ggtt_insert_page(struct i915_address_space *vm,
gen8_pte_t __iomem *pte =
(gen8_pte_t __iomem *)ggtt->gsm + (offset >> PAGE_SHIFT);
- gen8_set_pte(pte, gen8_pte_encode(addr, level));
+ gen8_set_pte(pte, gen8_pte_encode(addr, level, 0));
ggtt->invalidate(vm->i915);
}
@@ -2410,7 +2413,7 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
struct sgt_iter sgt_iter;
gen8_pte_t __iomem *gtt_entries;
- const gen8_pte_t pte_encode = gen8_pte_encode(0, level);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
dma_addr_t addr;
gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm;
@@ -2478,7 +2481,7 @@ static void gen8_ggtt_clear_range(struct i915_address_space *vm,
unsigned first_entry = start >> PAGE_SHIFT;
unsigned num_entries = length >> PAGE_SHIFT;
const gen8_pte_t scratch_pte =
- gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC);
+ gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC, 0);
gen8_pte_t __iomem *gtt_base =
(gen8_pte_t __iomem *)ggtt->gsm + first_entry;
const int max_entries = ggtt_total_entries(ggtt) - first_entry;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
@ 2018-06-14 19:24 ` Chris Wilson
2018-06-14 21:32 ` Matthew Auld
` (2 more replies)
2018-06-14 19:24 ` [PATCH 3/5] drm/i915: Prevent writing into a read-only object via a GGTT mmap Chris Wilson
` (11 subsequent siblings)
12 siblings, 3 replies; 21+ messages in thread
From: Chris Wilson @ 2018-06-14 19:24 UTC (permalink / raw)
To: intel-gfx
From: Jon Bloomfield <jon.bloomfield@intel.com>
Hook up the flags to allow read-only ppGTT mappings for gen8+
v2: Include a selftest to check that writes to a readonly PTE are
dropped
Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> #v1
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 45 ++++--
drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +-
.../gpu/drm/i915/selftests/i915_gem_context.c | 138 ++++++++++++++++++
4 files changed, 182 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c3630abbd260..bfe23e10a127 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -204,7 +204,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
return err;
}
- /* Currently applicable only to VLV */
+ /* Applicable to VLV, and gen8+ */
pte_flags = 0;
if (vma->obj->gt_ro)
pte_flags |= PTE_READ_ONLY;
@@ -961,10 +961,11 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
struct i915_page_directory_pointer *pdp,
struct sgt_dma *iter,
struct gen8_insert_pte *idx,
- enum i915_cache_level cache_level)
+ enum i915_cache_level cache_level,
+ u32 flags)
{
struct i915_page_directory *pd;
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
gen8_pte_t *vaddr;
bool ret;
@@ -1015,14 +1016,14 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level cache_level,
- u32 unused)
+ u32 flags)
{
struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
struct sgt_dma iter = sgt_dma(vma);
struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
gen8_ppgtt_insert_pte_entries(ppgtt, &ppgtt->pdp, &iter, &idx,
- cache_level);
+ cache_level, flags);
vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
}
@@ -1030,9 +1031,10 @@ static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
struct i915_page_directory_pointer **pdps,
struct sgt_dma *iter,
- enum i915_cache_level cache_level)
+ enum i915_cache_level cache_level,
+ u32 flags)
{
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
u64 start = vma->node.start;
dma_addr_t rem = iter->sg->length;
@@ -1148,19 +1150,21 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
static void gen8_ppgtt_insert_4lvl(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level cache_level,
- u32 unused)
+ u32 flags)
{
struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
struct sgt_dma iter = sgt_dma(vma);
struct i915_page_directory_pointer **pdps = ppgtt->pml4.pdps;
if (vma->page_sizes.sg > I915_GTT_PAGE_SIZE) {
- gen8_ppgtt_insert_huge_entries(vma, pdps, &iter, cache_level);
+ gen8_ppgtt_insert_huge_entries(vma, pdps, &iter, cache_level,
+ flags);
} else {
struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
while (gen8_ppgtt_insert_pte_entries(ppgtt, pdps[idx.pml4e++],
- &iter, &idx, cache_level))
+ &iter, &idx, cache_level,
+ flags))
GEM_BUG_ON(idx.pml4e >= GEN8_PML4ES_PER_PML4);
vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
@@ -1573,6 +1577,9 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
1ULL << 48 :
1ULL << 32;
+ /* From bdw, there is support for read-only pages in the PPGTT */
+ ppgtt->vm.has_read_only = true;
+
/* There are only few exceptions for gen >=6. chv and bxt.
* And we are not sure about the latter so play safe for now.
*/
@@ -2408,7 +2415,7 @@ static void gen8_ggtt_insert_page(struct i915_address_space *vm,
static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level level,
- u32 unused)
+ u32 flags)
{
struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
struct sgt_iter sgt_iter;
@@ -2416,6 +2423,9 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
dma_addr_t addr;
+ /* The GTT does not support read-only mappings */
+ GEM_BUG_ON(flags & PTE_READ_ONLY);
+
gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm;
gtt_entries += vma->node.start >> PAGE_SHIFT;
for_each_sgt_dma(addr, sgt_iter, vma->pages)
@@ -2542,13 +2552,14 @@ struct insert_entries {
struct i915_address_space *vm;
struct i915_vma *vma;
enum i915_cache_level level;
+ u32 flags;
};
static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
{
struct insert_entries *arg = _arg;
- gen8_ggtt_insert_entries(arg->vm, arg->vma, arg->level, 0);
+ gen8_ggtt_insert_entries(arg->vm, arg->vma, arg->level, arg->flags);
bxt_vtd_ggtt_wa(arg->vm);
return 0;
@@ -2557,9 +2568,9 @@ static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
static void bxt_vtd_ggtt_insert_entries__BKL(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level level,
- u32 unused)
+ u32 flags)
{
- struct insert_entries arg = { vm, vma, level };
+ struct insert_entries arg = { vm, vma, level, flags };
stop_machine(bxt_vtd_ggtt_insert_entries__cb, &arg, NULL);
}
@@ -2650,7 +2661,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
struct drm_i915_gem_object *obj = vma->obj;
u32 pte_flags;
- /* Currently applicable only to VLV */
+ /* Applicable to VLV (gen8+ do not support RO in the GGTT) */
pte_flags = 0;
if (obj->gt_ro)
pte_flags |= PTE_READ_ONLY;
@@ -3530,6 +3541,10 @@ int i915_ggtt_init_hw(struct drm_i915_private *dev_priv)
*/
mutex_lock(&dev_priv->drm.struct_mutex);
i915_address_space_init(&ggtt->vm, dev_priv, "[global]");
+
+ /* Only VLV supports read-only GGTT mappings */
+ ggtt->vm.has_read_only = IS_VALLEYVIEW(dev_priv);
+
if (!HAS_LLC(dev_priv) && !USES_PPGTT(dev_priv))
ggtt->vm.mm.color_adjust = i915_gtt_color_adjust;
mutex_unlock(&dev_priv->drm.struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 46c95d188580..67a13a0709fc 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -325,7 +325,12 @@ struct i915_address_space {
struct list_head unbound_list;
struct pagevec free_pages;
- bool pt_kmap_wc;
+
+ /* Some systems require uncached updates of the page directories */
+ bool pt_kmap_wc:1;
+
+ /* Some systems support read-only mappings for GGTT and/or PPGTT */
+ bool has_read_only:1;
/* FIXME: Need a more generic return type */
gen6_pte_t (*pte_encode)(dma_addr_t addr,
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ef3c76425843..8853a5e6d421 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1097,6 +1097,7 @@ void intel_ring_unpin(struct intel_ring *ring)
static struct i915_vma *
intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
{
+ struct i915_address_space *vm = &dev_priv->ggtt.vm;
struct drm_i915_gem_object *obj;
struct i915_vma *vma;
@@ -1106,10 +1107,14 @@ intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
if (IS_ERR(obj))
return ERR_CAST(obj);
- /* mark ring buffers as read-only from GPU side by default */
- obj->gt_ro = 1;
+ /*
+ * Mark ring buffers as read-only from GPU side (so no stray overwrites)
+ * if supported by the platform's GGTT.
+ */
+ if (vm->has_read_only)
+ obj->gt_ro = 1;
- vma = i915_vma_instance(obj, &dev_priv->ggtt.vm, NULL);
+ vma = i915_vma_instance(obj, vm, NULL);
if (IS_ERR(vma))
goto err;
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 836f1af8b833..5bae52068926 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -23,6 +23,7 @@
*/
#include "../i915_selftest.h"
+#include "i915_random.h"
#include "igt_flush_test.h"
#include "mock_drm.h"
@@ -266,6 +267,41 @@ static int cpu_check(struct drm_i915_gem_object *obj, unsigned int max)
return err;
}
+static int ro_check(struct drm_i915_gem_object *obj, unsigned int max)
+{
+ unsigned int n, m, needs_flush;
+ int err;
+
+ err = i915_gem_obj_prepare_shmem_read(obj, &needs_flush);
+ if (err)
+ return err;
+
+ for (n = 0; n < real_page_count(obj); n++) {
+ u32 *map;
+
+ map = kmap_atomic(i915_gem_object_get_page(obj, n));
+ if (needs_flush & CLFLUSH_BEFORE)
+ drm_clflush_virt_range(map, PAGE_SIZE);
+
+ for (m = 0; m < DW_PER_PAGE; m++) {
+ if (map[m] != 0xdeadbeef) {
+ pr_err("Invalid value (overwritten) at page %d, offset %d: found %08x expected %08x\n",
+ n, m, map[m], 0xdeadbeef);
+ err = -EINVAL;
+ goto out_unmap;
+ }
+ }
+
+out_unmap:
+ kunmap_atomic(map);
+ if (err)
+ break;
+ }
+
+ i915_gem_obj_finish_shmem_access(obj);
+ return err;
+}
+
static int file_add_object(struct drm_file *file,
struct drm_i915_gem_object *obj)
{
@@ -421,6 +457,107 @@ static int igt_ctx_exec(void *arg)
return err;
}
+static int igt_ctx_readonly(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj = NULL;
+ struct drm_file *file;
+ I915_RND_STATE(prng);
+ IGT_TIMEOUT(end_time);
+ LIST_HEAD(objects);
+ struct i915_gem_context *ctx;
+ struct i915_hw_ppgtt *ppgtt;
+ unsigned long ndwords, dw;
+ int err = -ENODEV;
+
+ /* Create a few different contexts (with different mm) and write
+ * through each ctx/mm using the GPU making sure those writes end
+ * up in the expected pages of our obj.
+ */
+
+ file = mock_file(i915);
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+
+ mutex_lock(&i915->drm.struct_mutex);
+
+ ctx = i915_gem_create_context(i915, file->driver_priv);
+ if (IS_ERR(ctx)) {
+ err = PTR_ERR(ctx);
+ goto out_unlock;
+ }
+
+ ppgtt = ctx->ppgtt ?: i915->mm.aliasing_ppgtt;
+ if (!ppgtt || !ppgtt->vm.has_read_only) {
+ err = 0;
+ goto out_unlock;
+ }
+
+ ndwords = 0;
+ dw = 0;
+ while (!time_after(jiffies, end_time)) {
+ struct intel_engine_cs *engine;
+ unsigned int id;
+
+ for_each_engine(engine, i915, id) {
+ if (!intel_engine_can_store_dword(engine))
+ continue;
+
+ if (!obj) {
+ obj = create_test_object(ctx, file, &objects);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ goto out_unlock;
+ }
+
+ obj->gt_ro = prandom_u32_state(&prng);
+ }
+
+ intel_runtime_pm_get(i915);
+ err = gpu_fill(obj, ctx, engine, dw);
+ intel_runtime_pm_put(i915);
+ if (err) {
+ pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) in ctx %u [full-ppgtt? %s], err=%d\n",
+ ndwords, dw, max_dwords(obj),
+ engine->name, ctx->hw_id,
+ yesno(!!ctx->ppgtt), err);
+ goto out_unlock;
+ }
+
+ if (++dw == max_dwords(obj)) {
+ obj = NULL;
+ dw = 0;
+ }
+ ndwords++;
+ }
+ }
+ pr_info("Submitted %lu dwords (across %u engines)\n",
+ ndwords, INTEL_INFO(i915)->num_rings);
+
+ dw = 0;
+ list_for_each_entry(obj, &objects, st_link) {
+ unsigned int rem =
+ min_t(unsigned int, ndwords - dw, max_dwords(obj));
+
+ if (obj->gt_ro)
+ err = ro_check(obj, rem);
+ else
+ err = cpu_check(obj, rem);
+ if (err)
+ break;
+
+ dw += rem;
+ }
+
+out_unlock:
+ if (igt_flush_test(i915, I915_WAIT_LOCKED))
+ err = -EIO;
+ mutex_unlock(&i915->drm.struct_mutex);
+
+ mock_file_free(i915, file);
+ return err;
+}
+
static __maybe_unused const char *
__engine_name(struct drm_i915_private *i915, unsigned int engines)
{
@@ -595,6 +732,7 @@ int i915_gem_context_live_selftests(struct drm_i915_private *dev_priv)
static const struct i915_subtest tests[] = {
SUBTEST(igt_switch_to_kernel_context),
SUBTEST(igt_ctx_exec),
+ SUBTEST(igt_ctx_readonly),
};
bool fake_alias = false;
int err;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/5] drm/i915: Prevent writing into a read-only object via a GGTT mmap
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
2018-06-14 19:24 ` [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+ Chris Wilson
@ 2018-06-14 19:24 ` Chris Wilson
2018-06-15 8:08 ` Joonas Lahtinen
2018-06-15 15:26 ` [PATCH v3] " Chris Wilson
2018-06-14 19:24 ` [PATCH 4/5] drm/i915: Reject attempted pwrites into a read-only object Chris Wilson
` (10 subsequent siblings)
12 siblings, 2 replies; 21+ messages in thread
From: Chris Wilson @ 2018-06-14 19:24 UTC (permalink / raw)
To: intel-gfx; +Cc: David Herrmann
If the user has created a read-only object, they should not be allowed
to circumvent the write protection by using a GGTT mmapping. Deny it.
Also most machines do not support read-only GGTT PTEs, so again we have
to reject attempted writes. Fortunately, this is known a priori, so we
can at least reject in the call to create the mmap (with a sanity check
in the fault handler).
v2: Check the vma->vm_flags during mmap() to allow readonly access.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> #v1
Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
---
drivers/gpu/drm/drm_gem.c | 5 +++++
drivers/gpu/drm/i915/i915_gem.c | 4 ++++
drivers/gpu/drm/i915/i915_gem_gtt.c | 12 +++++++-----
drivers/gpu/drm/i915/i915_gem_object.h | 13 ++++++++++++-
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 5 +++--
include/drm/drm_vma_manager.h | 1 +
7 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 4a16d7b26c89..230863813905 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1036,6 +1036,11 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
return -EACCES;
}
+ if (vma->vm_flags & VM_WRITE && node->readonly) {
+ drm_gem_object_put_unlocked(obj);
+ return -EINVAL;
+ }
+
ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT,
vma);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8dd4d35655af..2bfb16e83af2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2009,6 +2009,10 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
unsigned int flags;
int ret;
+ /* Sanity check that we allow writing into this object */
+ if (i915_gem_object_is_readonly(obj) && write)
+ return VM_FAULT_SIGBUS;
+
/* We don't use vmf->pgoff since that has the fake offset */
page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bfe23e10a127..8591e7051f0d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -206,7 +206,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
/* Applicable to VLV, and gen8+ */
pte_flags = 0;
- if (vma->obj->gt_ro)
+ if (i915_gem_object_is_readonly(vma->obj))
pte_flags |= PTE_READ_ONLY;
vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags);
@@ -2423,8 +2423,10 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
dma_addr_t addr;
- /* The GTT does not support read-only mappings */
- GEM_BUG_ON(flags & PTE_READ_ONLY);
+ /*
+ * Note that we ignore PTE_READ_ONLY here. The caller must be careful
+ * not to allow the user to override access to a read only page.
+ */
gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm;
gtt_entries += vma->node.start >> PAGE_SHIFT;
@@ -2663,7 +2665,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
/* Applicable to VLV (gen8+ do not support RO in the GGTT) */
pte_flags = 0;
- if (obj->gt_ro)
+ if (i915_gem_object_is_readonly(obj))
pte_flags |= PTE_READ_ONLY;
intel_runtime_pm_get(i915);
@@ -2701,7 +2703,7 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
/* Currently applicable only to VLV */
pte_flags = 0;
- if (vma->obj->gt_ro)
+ if (i915_gem_object_is_readonly(vma->obj))
pte_flags |= PTE_READ_ONLY;
if (flags & I915_VMA_LOCAL_BIND) {
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 54f00b350779..fd703d768b70 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -141,7 +141,6 @@ struct drm_i915_gem_object {
* Is the object to be mapped as read-only to the GPU
* Only honoured if hardware has relevant pte bit
*/
- unsigned long gt_ro:1;
unsigned int cache_level:3;
unsigned int cache_coherent:2;
#define I915_BO_CACHE_COHERENT_FOR_READ BIT(0)
@@ -367,6 +366,18 @@ static inline void i915_gem_object_unlock(struct drm_i915_gem_object *obj)
reservation_object_unlock(obj->resv);
}
+static inline void
+i915_gem_object_set_readonly(struct drm_i915_gem_object *obj)
+{
+ obj->base.vma_node.readonly = true;
+}
+
+static inline bool
+i915_gem_object_is_readonly(const struct drm_i915_gem_object *obj)
+{
+ return obj->base.vma_node.readonly;
+}
+
static inline bool
i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
{
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8853a5e6d421..9d54c3c24b10 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1112,7 +1112,7 @@ intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
* if supported by the platform's GGTT.
*/
if (vm->has_read_only)
- obj->gt_ro = 1;
+ i915_gem_object_set_readonly(obj);
vma = i915_vma_instance(obj, vm, NULL);
if (IS_ERR(vma))
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 5bae52068926..fea447b1a74f 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -510,7 +510,8 @@ static int igt_ctx_readonly(void *arg)
goto out_unlock;
}
- obj->gt_ro = prandom_u32_state(&prng);
+ if (prandom_u32_state(&prng) & 1)
+ i915_gem_object_set_readonly(obj);
}
intel_runtime_pm_get(i915);
@@ -539,7 +540,7 @@ static int igt_ctx_readonly(void *arg)
unsigned int rem =
min_t(unsigned int, ndwords - dw, max_dwords(obj));
- if (obj->gt_ro)
+ if (i915_gem_object_is_readonly(obj))
err = ro_check(obj, rem);
else
err = cpu_check(obj, rem);
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 8758df94e9a0..c7987daeaed0 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -41,6 +41,7 @@ struct drm_vma_offset_node {
rwlock_t vm_lock;
struct drm_mm_node vm_node;
struct rb_root vm_files;
+ bool readonly:1;
};
struct drm_vma_offset_manager {
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/5] drm/i915: Reject attempted pwrites into a read-only object
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
2018-06-14 19:24 ` [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+ Chris Wilson
2018-06-14 19:24 ` [PATCH 3/5] drm/i915: Prevent writing into a read-only object via a GGTT mmap Chris Wilson
@ 2018-06-14 19:24 ` Chris Wilson
2018-06-14 19:24 ` [PATCH 5/5] drm/i915/userptr: Enable read-only support on gen8+ Chris Wilson
` (9 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2018-06-14 19:24 UTC (permalink / raw)
To: intel-gfx
If the user created a read-only object, they should not be allowed to
circumvent the write protection using the pwrite ioctl.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
---
drivers/gpu/drm/i915/i915_gem.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2bfb16e83af2..47afe13638da 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1623,6 +1623,12 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
goto err;
}
+ /* Writes not allowed into this read-only object */
+ if (i915_gem_object_is_readonly(obj)) {
+ ret = -EINVAL;
+ goto err;
+ }
+
trace_i915_gem_object_pwrite(obj, args->offset, args->size);
ret = -ENODEV;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/5] drm/i915/userptr: Enable read-only support on gen8+
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (2 preceding siblings ...)
2018-06-14 19:24 ` [PATCH 4/5] drm/i915: Reject attempted pwrites into a read-only object Chris Wilson
@ 2018-06-14 19:24 ` Chris Wilson
2018-06-14 19:41 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Patchwork
` (8 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2018-06-14 19:24 UTC (permalink / raw)
To: intel-gfx
On gen8 and onwards, we can mark GPU accesses through the ppGTT as being
read-only, that is cause any GPU write onto that page to be discarded
(not triggering a fault). This is all that we need to finally support
the read-only flag for userptr!
Testcase: igt/gem_userptr_blits/readonly*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
---
drivers/gpu/drm/i915/i915_gem_object.h | 1 -
drivers/gpu/drm/i915/i915_gem_userptr.c | 15 +++++++++------
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index fd703d768b70..b2dc2ece3ca3 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -267,7 +267,6 @@ struct drm_i915_gem_object {
union {
struct i915_gem_userptr {
uintptr_t ptr;
- unsigned read_only :1;
struct i915_mm_struct *mm;
struct i915_mmu_object *mmu_object;
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 854bd51b9478..045db5ef17ac 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -507,7 +507,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
struct mm_struct *mm = obj->userptr.mm->mm;
unsigned int flags = 0;
- if (!obj->userptr.read_only)
+ if (!i915_gem_object_is_readonly(obj))
flags |= FOLL_WRITE;
ret = -EFAULT;
@@ -643,7 +643,7 @@ static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
if (pvec) /* defer to worker if malloc fails */
pinned = __get_user_pages_fast(obj->userptr.ptr,
num_pages,
- !obj->userptr.read_only,
+ !i915_gem_object_is_readonly(obj),
pvec);
}
@@ -789,10 +789,12 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
return -EFAULT;
if (args->flags & I915_USERPTR_READ_ONLY) {
- /* On almost all of the current hw, we cannot tell the GPU that a
- * page is readonly, so this is just a placeholder in the uAPI.
+ /*
+ * On almost all of the older hw, we cannot tell the GPU that
+ * a page is readonly.
*/
- return -ENODEV;
+ if (INTEL_GEN(dev_priv) < 8 || !USES_PPGTT(dev_priv))
+ return -ENODEV;
}
obj = i915_gem_object_alloc(dev_priv);
@@ -806,7 +808,8 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
i915_gem_object_set_cache_coherency(obj, I915_CACHE_LLC);
obj->userptr.ptr = args->user_ptr;
- obj->userptr.read_only = !!(args->flags & I915_USERPTR_READ_ONLY);
+ if (args->flags & I915_USERPTR_READ_ONLY)
+ i915_gem_object_set_readonly(obj);
/* And keep a pointer to the current->mm for resolving the user pages
* at binding. This means that we need to hook into the mmu_notifier
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (3 preceding siblings ...)
2018-06-14 19:24 ` [PATCH 5/5] drm/i915/userptr: Enable read-only support on gen8+ Chris Wilson
@ 2018-06-14 19:41 ` Patchwork
2018-06-14 19:43 ` ✗ Fi.CI.SPARSE: " Patchwork
` (7 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-14 19:41 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode
URL : https://patchwork.freedesktop.org/series/44776/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
a55b1e45685a drm/i915/gtt: Add read only pages to gen8_pte_encode
6ac74b295f59 drm/i915/gtt: Read-only pages for insert_entries on bdw+
-:188: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>
#188: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:330:
+ bool pt_kmap_wc:1;
-:191: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>
#191: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:333:
+ bool has_read_only:1;
-:288: WARNING:LINE_SPACING: Missing a blank line after declarations
#288: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:465:
+ struct drm_file *file;
+ I915_RND_STATE(prng);
-:358: ERROR:CODE_INDENT: code indent should use tabs where possible
#358: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:535:
+^I ^Indwords, INTEL_INFO(i915)->num_rings);$
-:358: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#358: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:535:
+^I ^Indwords, INTEL_INFO(i915)->num_rings);$
-:358: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#358: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:535:
+ pr_info("Submitted %lu dwords (across %u engines)\n",
+ ndwords, INTEL_INFO(i915)->num_rings);
total: 1 errors, 4 warnings, 1 checks, 342 lines checked
35e5b57aef3d drm/i915: Prevent writing into a read-only object via a GGTT mmap
-:176: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>
#176: FILE: include/drm/drm_vma_manager.h:44:
+ bool readonly:1;
total: 0 errors, 1 warnings, 0 checks, 114 lines checked
fd5cf6c7adcd drm/i915: Reject attempted pwrites into a read-only object
2a4d290f8b3e drm/i915/userptr: Enable read-only support on gen8+
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✗ Fi.CI.SPARSE: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (4 preceding siblings ...)
2018-06-14 19:41 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Patchwork
@ 2018-06-14 19:43 ` Patchwork
2018-06-14 20:00 ` ✓ Fi.CI.BAT: success " Patchwork
` (6 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-14 19:43 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode
URL : https://patchwork.freedesktop.org/series/44776/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Commit: drm/i915/gtt: Add read only pages to gen8_pte_encode
Okay!
Commit: drm/i915/gtt: Read-only pages for insert_entries on bdw+
+drivers/gpu/drm/i915/selftests/i915_gem_context.c:540:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/i915_gem_context.c:540:25: warning: expression using sizeof(void)
Commit: drm/i915: Prevent writing into a read-only object via a GGTT mmap
-O:drivers/gpu/drm/i915/selftests/i915_gem_context.c:540:25: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/selftests/i915_gem_context.c:540:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/i915_gem_context.c:541:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/i915_gem_context.c:541:25: warning: expression using sizeof(void)
Commit: drm/i915: Reject attempted pwrites into a read-only object
Okay!
Commit: drm/i915/userptr: Enable read-only support on gen8+
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (5 preceding siblings ...)
2018-06-14 19:43 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-06-14 20:00 ` Patchwork
2018-06-15 3:26 ` ✗ Fi.CI.IGT: failure " Patchwork
` (5 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-14 20:00 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode
URL : https://patchwork.freedesktop.org/series/44776/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4320 -> Patchwork_9308 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9308 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9308, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/44776/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9308:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_gttfill@basic:
fi-pnv-d510: SKIP -> PASS
== Known issues ==
Here are the changes found in Patchwork_9308 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_gttfill@basic:
fi-byt-n2820: PASS -> FAIL (fdo#106744)
igt@kms_flip@basic-flip-vs-modeset:
fi-glk-j4005: PASS -> DMESG-WARN (fdo#106097)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
fi-bxt-dsi: PASS -> INCOMPLETE (fdo#103927)
==== Possible fixes ====
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-glk-j4005: FAIL (fdo#100368) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
fdo#106744 https://bugs.freedesktop.org/show_bug.cgi?id=106744
== Participating hosts (43 -> 38) ==
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4320 -> Patchwork_9308
CI_DRM_4320: c104379da03410c3c07b336da92b51fb8bb67fa8 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4519: 3381a56be31defb3b5c23a4fbc19ac26a000c35b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9308: 2a4d290f8b3e4b26872aef908c57edf54c1d69f2 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
2a4d290f8b3e drm/i915/userptr: Enable read-only support on gen8+
fd5cf6c7adcd drm/i915: Reject attempted pwrites into a read-only object
35e5b57aef3d drm/i915: Prevent writing into a read-only object via a GGTT mmap
6ac74b295f59 drm/i915/gtt: Read-only pages for insert_entries on bdw+
a55b1e45685a drm/i915/gtt: Add read only pages to gen8_pte_encode
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9308/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+
2018-06-14 19:24 ` [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+ Chris Wilson
@ 2018-06-14 21:32 ` Matthew Auld
2018-06-15 6:31 ` Chris Wilson
2018-06-15 8:06 ` Joonas Lahtinen
2018-06-15 8:26 ` [PATCH v4] " Chris Wilson
2 siblings, 1 reply; 21+ messages in thread
From: Matthew Auld @ 2018-06-14 21:32 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel Graphics Development
On 14 June 2018 at 20:24, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> From: Jon Bloomfield <jon.bloomfield@intel.com>
>
> Hook up the flags to allow read-only ppGTT mappings for gen8+
>
> v2: Include a selftest to check that writes to a readonly PTE are
> dropped
>
> Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1
> Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> #v1
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 45 ++++--
> drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +-
> drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +-
> .../gpu/drm/i915/selftests/i915_gem_context.c | 138 ++++++++++++++++++
> 4 files changed, 182 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index c3630abbd260..bfe23e10a127 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -204,7 +204,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
> return err;
> }
>
> - /* Currently applicable only to VLV */
> + /* Applicable to VLV, and gen8+ */
> pte_flags = 0;
> if (vma->obj->gt_ro)
> pte_flags |= PTE_READ_ONLY;
> @@ -961,10 +961,11 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
> struct i915_page_directory_pointer *pdp,
> struct sgt_dma *iter,
> struct gen8_insert_pte *idx,
> - enum i915_cache_level cache_level)
> + enum i915_cache_level cache_level,
> + u32 flags)
> {
> struct i915_page_directory *pd;
> - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
> + const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
> gen8_pte_t *vaddr;
> bool ret;
>
> @@ -1015,14 +1016,14 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
> static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
> struct i915_vma *vma,
> enum i915_cache_level cache_level,
> - u32 unused)
> + u32 flags)
> {
> struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> struct sgt_dma iter = sgt_dma(vma);
> struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
>
> gen8_ppgtt_insert_pte_entries(ppgtt, &ppgtt->pdp, &iter, &idx,
> - cache_level);
> + cache_level, flags);
>
> vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
> }
> @@ -1030,9 +1031,10 @@ static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
> static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
> struct i915_page_directory_pointer **pdps,
> struct sgt_dma *iter,
> - enum i915_cache_level cache_level)
> + enum i915_cache_level cache_level,
> + u32 flags)
> {
> - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
> + const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
> u64 start = vma->node.start;
> dma_addr_t rem = iter->sg->length;
>
> @@ -1148,19 +1150,21 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
> static void gen8_ppgtt_insert_4lvl(struct i915_address_space *vm,
> struct i915_vma *vma,
> enum i915_cache_level cache_level,
> - u32 unused)
> + u32 flags)
> {
> struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> struct sgt_dma iter = sgt_dma(vma);
> struct i915_page_directory_pointer **pdps = ppgtt->pml4.pdps;
>
> if (vma->page_sizes.sg > I915_GTT_PAGE_SIZE) {
> - gen8_ppgtt_insert_huge_entries(vma, pdps, &iter, cache_level);
> + gen8_ppgtt_insert_huge_entries(vma, pdps, &iter, cache_level,
> + flags);
> } else {
> struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
>
> while (gen8_ppgtt_insert_pte_entries(ppgtt, pdps[idx.pml4e++],
> - &iter, &idx, cache_level))
> + &iter, &idx, cache_level,
> + flags))
> GEM_BUG_ON(idx.pml4e >= GEN8_PML4ES_PER_PML4);
>
> vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
> @@ -1573,6 +1577,9 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
> 1ULL << 48 :
> 1ULL << 32;
>
> + /* From bdw, there is support for read-only pages in the PPGTT */
> + ppgtt->vm.has_read_only = true;
> +
> /* There are only few exceptions for gen >=6. chv and bxt.
> * And we are not sure about the latter so play safe for now.
> */
> @@ -2408,7 +2415,7 @@ static void gen8_ggtt_insert_page(struct i915_address_space *vm,
> static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> struct i915_vma *vma,
> enum i915_cache_level level,
> - u32 unused)
> + u32 flags)
> {
> struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> struct sgt_iter sgt_iter;
> @@ -2416,6 +2423,9 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
> dma_addr_t addr;
>
> + /* The GTT does not support read-only mappings */
> + GEM_BUG_ON(flags & PTE_READ_ONLY);
> +
> gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm;
> gtt_entries += vma->node.start >> PAGE_SHIFT;
> for_each_sgt_dma(addr, sgt_iter, vma->pages)
> @@ -2542,13 +2552,14 @@ struct insert_entries {
> struct i915_address_space *vm;
> struct i915_vma *vma;
> enum i915_cache_level level;
> + u32 flags;
> };
>
> static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
> {
> struct insert_entries *arg = _arg;
>
> - gen8_ggtt_insert_entries(arg->vm, arg->vma, arg->level, 0);
> + gen8_ggtt_insert_entries(arg->vm, arg->vma, arg->level, arg->flags);
> bxt_vtd_ggtt_wa(arg->vm);
>
> return 0;
> @@ -2557,9 +2568,9 @@ static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
> static void bxt_vtd_ggtt_insert_entries__BKL(struct i915_address_space *vm,
> struct i915_vma *vma,
> enum i915_cache_level level,
> - u32 unused)
> + u32 flags)
> {
> - struct insert_entries arg = { vm, vma, level };
> + struct insert_entries arg = { vm, vma, level, flags };
>
> stop_machine(bxt_vtd_ggtt_insert_entries__cb, &arg, NULL);
> }
> @@ -2650,7 +2661,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
> struct drm_i915_gem_object *obj = vma->obj;
> u32 pte_flags;
>
> - /* Currently applicable only to VLV */
> + /* Applicable to VLV (gen8+ do not support RO in the GGTT) */
> pte_flags = 0;
> if (obj->gt_ro)
> pte_flags |= PTE_READ_ONLY;
> @@ -3530,6 +3541,10 @@ int i915_ggtt_init_hw(struct drm_i915_private *dev_priv)
> */
> mutex_lock(&dev_priv->drm.struct_mutex);
> i915_address_space_init(&ggtt->vm, dev_priv, "[global]");
> +
> + /* Only VLV supports read-only GGTT mappings */
> + ggtt->vm.has_read_only = IS_VALLEYVIEW(dev_priv);
> +
> if (!HAS_LLC(dev_priv) && !USES_PPGTT(dev_priv))
> ggtt->vm.mm.color_adjust = i915_gtt_color_adjust;
> mutex_unlock(&dev_priv->drm.struct_mutex);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 46c95d188580..67a13a0709fc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -325,7 +325,12 @@ struct i915_address_space {
> struct list_head unbound_list;
>
> struct pagevec free_pages;
> - bool pt_kmap_wc;
> +
> + /* Some systems require uncached updates of the page directories */
> + bool pt_kmap_wc:1;
> +
> + /* Some systems support read-only mappings for GGTT and/or PPGTT */
> + bool has_read_only:1;
>
> /* FIXME: Need a more generic return type */
> gen6_pte_t (*pte_encode)(dma_addr_t addr,
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index ef3c76425843..8853a5e6d421 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1097,6 +1097,7 @@ void intel_ring_unpin(struct intel_ring *ring)
> static struct i915_vma *
> intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
> {
> + struct i915_address_space *vm = &dev_priv->ggtt.vm;
> struct drm_i915_gem_object *obj;
> struct i915_vma *vma;
>
> @@ -1106,10 +1107,14 @@ intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
> if (IS_ERR(obj))
> return ERR_CAST(obj);
>
> - /* mark ring buffers as read-only from GPU side by default */
> - obj->gt_ro = 1;
> + /*
> + * Mark ring buffers as read-only from GPU side (so no stray overwrites)
> + * if supported by the platform's GGTT.
> + */
> + if (vm->has_read_only)
> + obj->gt_ro = 1;
>
> - vma = i915_vma_instance(obj, &dev_priv->ggtt.vm, NULL);
> + vma = i915_vma_instance(obj, vm, NULL);
> if (IS_ERR(vma))
> goto err;
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index 836f1af8b833..5bae52068926 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -23,6 +23,7 @@
> */
>
> #include "../i915_selftest.h"
> +#include "i915_random.h"
> #include "igt_flush_test.h"
>
> #include "mock_drm.h"
> @@ -266,6 +267,41 @@ static int cpu_check(struct drm_i915_gem_object *obj, unsigned int max)
> return err;
> }
>
> +static int ro_check(struct drm_i915_gem_object *obj, unsigned int max)
> +{
> + unsigned int n, m, needs_flush;
> + int err;
> +
> + err = i915_gem_obj_prepare_shmem_read(obj, &needs_flush);
> + if (err)
> + return err;
> +
> + for (n = 0; n < real_page_count(obj); n++) {
> + u32 *map;
> +
> + map = kmap_atomic(i915_gem_object_get_page(obj, n));
> + if (needs_flush & CLFLUSH_BEFORE)
> + drm_clflush_virt_range(map, PAGE_SIZE);
> +
> + for (m = 0; m < DW_PER_PAGE; m++) {
m < max; or don't pass max?
> + if (map[m] != 0xdeadbeef) {
> + pr_err("Invalid value (overwritten) at page %d, offset %d: found %08x expected %08x\n",
> + n, m, map[m], 0xdeadbeef);
> + err = -EINVAL;
> + goto out_unmap;
> + }
> + }
> +
> +out_unmap:
> + kunmap_atomic(map);
> + if (err)
> + break;
> + }
> +
> + i915_gem_obj_finish_shmem_access(obj);
> + return err;
> +}
> +
> static int file_add_object(struct drm_file *file,
> struct drm_i915_gem_object *obj)
> {
> @@ -421,6 +457,107 @@ static int igt_ctx_exec(void *arg)
> return err;
> }
>
> +static int igt_ctx_readonly(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct drm_i915_gem_object *obj = NULL;
> + struct drm_file *file;
> + I915_RND_STATE(prng);
> + IGT_TIMEOUT(end_time);
> + LIST_HEAD(objects);
> + struct i915_gem_context *ctx;
> + struct i915_hw_ppgtt *ppgtt;
> + unsigned long ndwords, dw;
> + int err = -ENODEV;
> +
> + /* Create a few different contexts (with different mm) and write
> + * through each ctx/mm using the GPU making sure those writes end
> + * up in the expected pages of our obj.
> + */
/*\n
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (6 preceding siblings ...)
2018-06-14 20:00 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-06-15 3:26 ` Patchwork
2018-06-15 8:36 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev2) Patchwork
` (4 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-15 3:26 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode
URL : https://patchwork.freedesktop.org/series/44776/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4320_full -> Patchwork_9308_full =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_9308_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9308_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9308_full:
=== IGT changes ===
==== Possible regressions ====
igt@gem_userptr_blits@usage-restrictions:
shard-kbl: PASS -> FAIL
shard-glk: PASS -> FAIL
shard-apl: PASS -> FAIL
==== Warnings ====
igt@gem_exec_schedule@deep-bsd2:
shard-kbl: PASS -> SKIP
igt@gem_mocs_settings@mocs-rc6-vebox:
shard-kbl: SKIP -> PASS +2
== Known issues ==
Here are the changes found in Patchwork_9308_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@drv_selftest@live_gtt:
shard-glk: PASS -> INCOMPLETE (fdo#103359, k.org#198133)
igt@drv_suspend@shrink:
shard-hsw: PASS -> INCOMPLETE (fdo#103540)
igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
shard-glk: PASS -> FAIL (fdo#105703) +1
igt@kms_flip@modeset-vs-vblank-race-interruptible:
shard-hsw: PASS -> FAIL (fdo#103060)
igt@prime_busy@wait-after-bsd:
shard-snb: NOTRUN -> INCOMPLETE (fdo#105411)
==== Possible fixes ====
igt@kms_cursor_legacy@flip-vs-cursor-legacy:
shard-hsw: FAIL (fdo#102670) -> PASS
igt@kms_flip@2x-wf_vblank-ts-check-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS
igt@kms_flip_tiling@flip-to-y-tiled:
shard-glk: FAIL (fdo#104724) -> PASS +1
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
igt@perf_pmu@other-init-2:
shard-snb: INCOMPLETE (fdo#105411) -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4320 -> Patchwork_9308
CI_DRM_4320: c104379da03410c3c07b336da92b51fb8bb67fa8 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4519: 3381a56be31defb3b5c23a4fbc19ac26a000c35b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9308: 2a4d290f8b3e4b26872aef908c57edf54c1d69f2 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9308/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+
2018-06-14 21:32 ` Matthew Auld
@ 2018-06-15 6:31 ` Chris Wilson
0 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2018-06-15 6:31 UTC (permalink / raw)
To: Matthew Auld; +Cc: Intel Graphics Development
Quoting Matthew Auld (2018-06-14 22:32:09)
> On 14 June 2018 at 20:24, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > From: Jon Bloomfield <jon.bloomfield@intel.com>
> >
> > Hook up the flags to allow read-only ppGTT mappings for gen8+
> >
> > v2: Include a selftest to check that writes to a readonly PTE are
> > dropped
> >
> > Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1
> > Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> #v1
> > ---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 45 ++++--
> > drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +-
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +-
> > .../gpu/drm/i915/selftests/i915_gem_context.c | 138 ++++++++++++++++++
> > 4 files changed, 182 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index c3630abbd260..bfe23e10a127 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -204,7 +204,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
> > return err;
> > }
> >
> > - /* Currently applicable only to VLV */
> > + /* Applicable to VLV, and gen8+ */
> > pte_flags = 0;
> > if (vma->obj->gt_ro)
> > pte_flags |= PTE_READ_ONLY;
> > @@ -961,10 +961,11 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
> > struct i915_page_directory_pointer *pdp,
> > struct sgt_dma *iter,
> > struct gen8_insert_pte *idx,
> > - enum i915_cache_level cache_level)
> > + enum i915_cache_level cache_level,
> > + u32 flags)
> > {
> > struct i915_page_directory *pd;
> > - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
> > + const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
> > gen8_pte_t *vaddr;
> > bool ret;
> >
> > @@ -1015,14 +1016,14 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
> > static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
> > struct i915_vma *vma,
> > enum i915_cache_level cache_level,
> > - u32 unused)
> > + u32 flags)
> > {
> > struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> > struct sgt_dma iter = sgt_dma(vma);
> > struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
> >
> > gen8_ppgtt_insert_pte_entries(ppgtt, &ppgtt->pdp, &iter, &idx,
> > - cache_level);
> > + cache_level, flags);
> >
> > vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
> > }
> > @@ -1030,9 +1031,10 @@ static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
> > static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
> > struct i915_page_directory_pointer **pdps,
> > struct sgt_dma *iter,
> > - enum i915_cache_level cache_level)
> > + enum i915_cache_level cache_level,
> > + u32 flags)
> > {
> > - const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
> > + const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
> > u64 start = vma->node.start;
> > dma_addr_t rem = iter->sg->length;
> >
> > @@ -1148,19 +1150,21 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
> > static void gen8_ppgtt_insert_4lvl(struct i915_address_space *vm,
> > struct i915_vma *vma,
> > enum i915_cache_level cache_level,
> > - u32 unused)
> > + u32 flags)
> > {
> > struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> > struct sgt_dma iter = sgt_dma(vma);
> > struct i915_page_directory_pointer **pdps = ppgtt->pml4.pdps;
> >
> > if (vma->page_sizes.sg > I915_GTT_PAGE_SIZE) {
> > - gen8_ppgtt_insert_huge_entries(vma, pdps, &iter, cache_level);
> > + gen8_ppgtt_insert_huge_entries(vma, pdps, &iter, cache_level,
> > + flags);
> > } else {
> > struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
> >
> > while (gen8_ppgtt_insert_pte_entries(ppgtt, pdps[idx.pml4e++],
> > - &iter, &idx, cache_level))
> > + &iter, &idx, cache_level,
> > + flags))
> > GEM_BUG_ON(idx.pml4e >= GEN8_PML4ES_PER_PML4);
> >
> > vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
> > @@ -1573,6 +1577,9 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
> > 1ULL << 48 :
> > 1ULL << 32;
> >
> > + /* From bdw, there is support for read-only pages in the PPGTT */
> > + ppgtt->vm.has_read_only = true;
> > +
> > /* There are only few exceptions for gen >=6. chv and bxt.
> > * And we are not sure about the latter so play safe for now.
> > */
> > @@ -2408,7 +2415,7 @@ static void gen8_ggtt_insert_page(struct i915_address_space *vm,
> > static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> > struct i915_vma *vma,
> > enum i915_cache_level level,
> > - u32 unused)
> > + u32 flags)
> > {
> > struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> > struct sgt_iter sgt_iter;
> > @@ -2416,6 +2423,9 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> > const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
> > dma_addr_t addr;
> >
> > + /* The GTT does not support read-only mappings */
> > + GEM_BUG_ON(flags & PTE_READ_ONLY);
> > +
> > gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm;
> > gtt_entries += vma->node.start >> PAGE_SHIFT;
> > for_each_sgt_dma(addr, sgt_iter, vma->pages)
> > @@ -2542,13 +2552,14 @@ struct insert_entries {
> > struct i915_address_space *vm;
> > struct i915_vma *vma;
> > enum i915_cache_level level;
> > + u32 flags;
> > };
> >
> > static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
> > {
> > struct insert_entries *arg = _arg;
> >
> > - gen8_ggtt_insert_entries(arg->vm, arg->vma, arg->level, 0);
> > + gen8_ggtt_insert_entries(arg->vm, arg->vma, arg->level, arg->flags);
> > bxt_vtd_ggtt_wa(arg->vm);
> >
> > return 0;
> > @@ -2557,9 +2568,9 @@ static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
> > static void bxt_vtd_ggtt_insert_entries__BKL(struct i915_address_space *vm,
> > struct i915_vma *vma,
> > enum i915_cache_level level,
> > - u32 unused)
> > + u32 flags)
> > {
> > - struct insert_entries arg = { vm, vma, level };
> > + struct insert_entries arg = { vm, vma, level, flags };
> >
> > stop_machine(bxt_vtd_ggtt_insert_entries__cb, &arg, NULL);
> > }
> > @@ -2650,7 +2661,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
> > struct drm_i915_gem_object *obj = vma->obj;
> > u32 pte_flags;
> >
> > - /* Currently applicable only to VLV */
> > + /* Applicable to VLV (gen8+ do not support RO in the GGTT) */
> > pte_flags = 0;
> > if (obj->gt_ro)
> > pte_flags |= PTE_READ_ONLY;
> > @@ -3530,6 +3541,10 @@ int i915_ggtt_init_hw(struct drm_i915_private *dev_priv)
> > */
> > mutex_lock(&dev_priv->drm.struct_mutex);
> > i915_address_space_init(&ggtt->vm, dev_priv, "[global]");
> > +
> > + /* Only VLV supports read-only GGTT mappings */
> > + ggtt->vm.has_read_only = IS_VALLEYVIEW(dev_priv);
> > +
> > if (!HAS_LLC(dev_priv) && !USES_PPGTT(dev_priv))
> > ggtt->vm.mm.color_adjust = i915_gtt_color_adjust;
> > mutex_unlock(&dev_priv->drm.struct_mutex);
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > index 46c95d188580..67a13a0709fc 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > @@ -325,7 +325,12 @@ struct i915_address_space {
> > struct list_head unbound_list;
> >
> > struct pagevec free_pages;
> > - bool pt_kmap_wc;
> > +
> > + /* Some systems require uncached updates of the page directories */
> > + bool pt_kmap_wc:1;
> > +
> > + /* Some systems support read-only mappings for GGTT and/or PPGTT */
> > + bool has_read_only:1;
> >
> > /* FIXME: Need a more generic return type */
> > gen6_pte_t (*pte_encode)(dma_addr_t addr,
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index ef3c76425843..8853a5e6d421 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1097,6 +1097,7 @@ void intel_ring_unpin(struct intel_ring *ring)
> > static struct i915_vma *
> > intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
> > {
> > + struct i915_address_space *vm = &dev_priv->ggtt.vm;
> > struct drm_i915_gem_object *obj;
> > struct i915_vma *vma;
> >
> > @@ -1106,10 +1107,14 @@ intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
> > if (IS_ERR(obj))
> > return ERR_CAST(obj);
> >
> > - /* mark ring buffers as read-only from GPU side by default */
> > - obj->gt_ro = 1;
> > + /*
> > + * Mark ring buffers as read-only from GPU side (so no stray overwrites)
> > + * if supported by the platform's GGTT.
> > + */
> > + if (vm->has_read_only)
> > + obj->gt_ro = 1;
> >
> > - vma = i915_vma_instance(obj, &dev_priv->ggtt.vm, NULL);
> > + vma = i915_vma_instance(obj, vm, NULL);
> > if (IS_ERR(vma))
> > goto err;
> >
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> > index 836f1af8b833..5bae52068926 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> > @@ -23,6 +23,7 @@
> > */
> >
> > #include "../i915_selftest.h"
> > +#include "i915_random.h"
> > #include "igt_flush_test.h"
> >
> > #include "mock_drm.h"
> > @@ -266,6 +267,41 @@ static int cpu_check(struct drm_i915_gem_object *obj, unsigned int max)
> > return err;
> > }
> >
> > +static int ro_check(struct drm_i915_gem_object *obj, unsigned int max)
> > +{
> > + unsigned int n, m, needs_flush;
> > + int err;
> > +
> > + err = i915_gem_obj_prepare_shmem_read(obj, &needs_flush);
> > + if (err)
> > + return err;
> > +
> > + for (n = 0; n < real_page_count(obj); n++) {
> > + u32 *map;
> > +
> > + map = kmap_atomic(i915_gem_object_get_page(obj, n));
> > + if (needs_flush & CLFLUSH_BEFORE)
> > + drm_clflush_virt_range(map, PAGE_SIZE);
> > +
> > + for (m = 0; m < DW_PER_PAGE; m++) {
>
> m < max; or don't pass max?
max is as far as we try to write, the rest should be filled with
0xdeadbeef from initialisation anyway. Or I should just pull this into
cpu_check() and stop the duplication. (At first it seemed sensible, then
mixed in some writes just to check the code.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+
2018-06-14 19:24 ` [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+ Chris Wilson
2018-06-14 21:32 ` Matthew Auld
@ 2018-06-15 8:06 ` Joonas Lahtinen
2018-06-15 8:26 ` [PATCH v4] " Chris Wilson
2 siblings, 0 replies; 21+ messages in thread
From: Joonas Lahtinen @ 2018-06-15 8:06 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Quoting Chris Wilson (2018-06-14 22:24:01)
> From: Jon Bloomfield <jon.bloomfield@intel.com>
>
> Hook up the flags to allow read-only ppGTT mappings for gen8+
>
> v2: Include a selftest to check that writes to a readonly PTE are
> dropped
>
> Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1
> Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> #v1
<SNIP>
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -23,6 +23,7 @@
> */
>
> #include "../i915_selftest.h"
> +#include "i915_random.h"
> #include "igt_flush_test.h"
>
> #include "mock_drm.h"
> @@ -266,6 +267,41 @@ static int cpu_check(struct drm_i915_gem_object *obj, unsigned int max)
> return err;
> }
>
> +static int ro_check(struct drm_i915_gem_object *obj, unsigned int max)
> +{
> + unsigned int n, m, needs_flush;
> + int err;
> +
> + err = i915_gem_obj_prepare_shmem_read(obj, &needs_flush);
> + if (err)
> + return err;
> +
> + for (n = 0; n < real_page_count(obj); n++) {
> + u32 *map;
> +
> + map = kmap_atomic(i915_gem_object_get_page(obj, n));
> + if (needs_flush & CLFLUSH_BEFORE)
> + drm_clflush_virt_range(map, PAGE_SIZE);
> +
> + for (m = 0; m < DW_PER_PAGE; m++) {
> + if (map[m] != 0xdeadbeef) {
One could have #define MAGIC 0xdeadbeef
> + pr_err("Invalid value (overwritten) at page %d, offset %d: found %08x expected %08x\n",
> + n, m, map[m], 0xdeadbeef);
> + err = -EINVAL;
> + goto out_unmap;
> + }
> + }
> +
> +out_unmap:
> + kunmap_atomic(map);
> + if (err)
> + break;
> + }
> +
> + i915_gem_obj_finish_shmem_access(obj);
> + return err;
> +}
<SNIP>
> +static int igt_ctx_readonly(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct drm_i915_gem_object *obj = NULL;
> + struct drm_file *file;
> + I915_RND_STATE(prng);
> + IGT_TIMEOUT(end_time);
> + LIST_HEAD(objects);
> + struct i915_gem_context *ctx;
> + struct i915_hw_ppgtt *ppgtt;
> + unsigned long ndwords, dw;
> + int err = -ENODEV;
> +
> + /* Create a few different contexts (with different mm) and write
As noted by Matt.
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] drm/i915: Prevent writing into a read-only object via a GGTT mmap
2018-06-14 19:24 ` [PATCH 3/5] drm/i915: Prevent writing into a read-only object via a GGTT mmap Chris Wilson
@ 2018-06-15 8:08 ` Joonas Lahtinen
2018-06-15 8:33 ` Chris Wilson
2018-06-15 15:26 ` [PATCH v3] " Chris Wilson
1 sibling, 1 reply; 21+ messages in thread
From: Joonas Lahtinen @ 2018-06-15 8:08 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: David Herrmann
Quoting Chris Wilson (2018-06-14 22:24:02)
> If the user has created a read-only object, they should not be allowed
> to circumvent the write protection by using a GGTT mmapping. Deny it.
>
> Also most machines do not support read-only GGTT PTEs, so again we have
> to reject attempted writes. Fortunately, this is known a priori, so we
> can at least reject in the call to create the mmap (with a sanity check
> in the fault handler).
>
> v2: Check the vma->vm_flags during mmap() to allow readonly access.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1
> Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> #v1
> Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
<SNIP>
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1036,6 +1036,11 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> return -EACCES;
> }
>
> + if (vma->vm_flags & VM_WRITE && node->readonly) {
> + drm_gem_object_put_unlocked(obj);
> + return -EINVAL;
> + }
> +
Pretty sure you want to split this patch and Cc: dri-devel. With that,
both are:
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4] drm/i915/gtt: Read-only pages for insert_entries on bdw+
2018-06-14 19:24 ` [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+ Chris Wilson
2018-06-14 21:32 ` Matthew Auld
2018-06-15 8:06 ` Joonas Lahtinen
@ 2018-06-15 8:26 ` Chris Wilson
2 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2018-06-15 8:26 UTC (permalink / raw)
To: intel-gfx
From: Jon Bloomfield <jon.bloomfield@intel.com>
Hook up the flags to allow read-only ppGTT mappings for gen8+
v2: Include a selftest to check that writes to a readonly PTE are
dropped
v3: Don't duplicate cpu_check() as we can just reuse it, and even worse
don't wholesale copy the theory-of-operation comment from igt_ctx_exec
without changing it to explain the intention behind the new test!
v4: Joonas really likes magic mystery values
Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 45 ++++---
drivers/gpu/drm/i915/i915_gem_gtt.h | 7 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +-
.../gpu/drm/i915/selftests/i915_gem_context.c | 112 +++++++++++++++++-
4 files changed, 153 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 705cf1c04bba..deddf15db24a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -196,7 +196,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
return err;
}
- /* Currently applicable only to VLV */
+ /* Applicable to VLV, and gen8+ */
pte_flags = 0;
if (vma->obj->gt_ro)
pte_flags |= PTE_READ_ONLY;
@@ -952,10 +952,11 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
struct i915_page_directory_pointer *pdp,
struct sgt_dma *iter,
struct gen8_insert_pte *idx,
- enum i915_cache_level cache_level)
+ enum i915_cache_level cache_level,
+ u32 flags)
{
struct i915_page_directory *pd;
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
gen8_pte_t *vaddr;
bool ret;
@@ -1006,14 +1007,14 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level cache_level,
- u32 unused)
+ u32 flags)
{
struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
struct sgt_dma iter = sgt_dma(vma);
struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
gen8_ppgtt_insert_pte_entries(ppgtt, &ppgtt->pdp, &iter, &idx,
- cache_level);
+ cache_level, flags);
vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
}
@@ -1021,9 +1022,10 @@ static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
struct i915_page_directory_pointer **pdps,
struct sgt_dma *iter,
- enum i915_cache_level cache_level)
+ enum i915_cache_level cache_level,
+ u32 flags)
{
- const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, 0);
+ const gen8_pte_t pte_encode = gen8_pte_encode(0, cache_level, flags);
u64 start = vma->node.start;
dma_addr_t rem = iter->sg->length;
@@ -1139,19 +1141,21 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
static void gen8_ppgtt_insert_4lvl(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level cache_level,
- u32 unused)
+ u32 flags)
{
struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
struct sgt_dma iter = sgt_dma(vma);
struct i915_page_directory_pointer **pdps = ppgtt->pml4.pdps;
if (vma->page_sizes.sg > I915_GTT_PAGE_SIZE) {
- gen8_ppgtt_insert_huge_entries(vma, pdps, &iter, cache_level);
+ gen8_ppgtt_insert_huge_entries(vma, pdps, &iter, cache_level,
+ flags);
} else {
struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
while (gen8_ppgtt_insert_pte_entries(ppgtt, pdps[idx.pml4e++],
- &iter, &idx, cache_level))
+ &iter, &idx, cache_level,
+ flags))
GEM_BUG_ON(idx.pml4e >= GEN8_PML4ES_PER_PML4);
vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
@@ -1564,6 +1568,9 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
1ULL << 48 :
1ULL << 32;
+ /* From bdw, there is support for read-only pages in the PPGTT */
+ ppgtt->vm.has_read_only = true;
+
/* There are only few exceptions for gen >=6. chv and bxt.
* And we are not sure about the latter so play safe for now.
*/
@@ -2400,7 +2407,7 @@ static void gen8_ggtt_insert_page(struct i915_address_space *vm,
static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level level,
- u32 unused)
+ u32 flags)
{
struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
struct sgt_iter sgt_iter;
@@ -2408,6 +2415,9 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
dma_addr_t addr;
+ /* The GTT does not support read-only mappings */
+ GEM_BUG_ON(flags & PTE_READ_ONLY);
+
gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm;
gtt_entries += vma->node.start >> PAGE_SHIFT;
for_each_sgt_dma(addr, sgt_iter, vma->pages)
@@ -2534,13 +2544,14 @@ struct insert_entries {
struct i915_address_space *vm;
struct i915_vma *vma;
enum i915_cache_level level;
+ u32 flags;
};
static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
{
struct insert_entries *arg = _arg;
- gen8_ggtt_insert_entries(arg->vm, arg->vma, arg->level, 0);
+ gen8_ggtt_insert_entries(arg->vm, arg->vma, arg->level, arg->flags);
bxt_vtd_ggtt_wa(arg->vm);
return 0;
@@ -2549,9 +2560,9 @@ static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
static void bxt_vtd_ggtt_insert_entries__BKL(struct i915_address_space *vm,
struct i915_vma *vma,
enum i915_cache_level level,
- u32 unused)
+ u32 flags)
{
- struct insert_entries arg = { vm, vma, level };
+ struct insert_entries arg = { vm, vma, level, flags };
stop_machine(bxt_vtd_ggtt_insert_entries__cb, &arg, NULL);
}
@@ -2642,7 +2653,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
struct drm_i915_gem_object *obj = vma->obj;
u32 pte_flags;
- /* Currently applicable only to VLV */
+ /* Applicable to VLV (gen8+ do not support RO in the GGTT) */
pte_flags = 0;
if (obj->gt_ro)
pte_flags |= PTE_READ_ONLY;
@@ -3522,6 +3533,10 @@ int i915_ggtt_init_hw(struct drm_i915_private *dev_priv)
*/
mutex_lock(&dev_priv->drm.struct_mutex);
i915_address_space_init(&ggtt->vm, dev_priv, "[global]");
+
+ /* Only VLV supports read-only GGTT mappings */
+ ggtt->vm.has_read_only = IS_VALLEYVIEW(dev_priv);
+
if (!HAS_LLC(dev_priv) && !USES_PPGTT(dev_priv))
ggtt->vm.mm.color_adjust = i915_gtt_color_adjust;
mutex_unlock(&dev_priv->drm.struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 9a4824cae68d..f1f6d7076f12 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -325,7 +325,12 @@ struct i915_address_space {
struct list_head unbound_list;
struct pagevec free_pages;
- bool pt_kmap_wc;
+
+ /* Some systems require uncached updates of the page directories */
+ bool pt_kmap_wc:1;
+
+ /* Some systems support read-only mappings for GGTT and/or PPGTT */
+ bool has_read_only:1;
/* FIXME: Need a more generic return type */
gen6_pte_t (*pte_encode)(dma_addr_t addr,
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ef3c76425843..8853a5e6d421 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1097,6 +1097,7 @@ void intel_ring_unpin(struct intel_ring *ring)
static struct i915_vma *
intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
{
+ struct i915_address_space *vm = &dev_priv->ggtt.vm;
struct drm_i915_gem_object *obj;
struct i915_vma *vma;
@@ -1106,10 +1107,14 @@ intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
if (IS_ERR(obj))
return ERR_CAST(obj);
- /* mark ring buffers as read-only from GPU side by default */
- obj->gt_ro = 1;
+ /*
+ * Mark ring buffers as read-only from GPU side (so no stray overwrites)
+ * if supported by the platform's GGTT.
+ */
+ if (vm->has_read_only)
+ obj->gt_ro = 1;
- vma = i915_vma_instance(obj, &dev_priv->ggtt.vm, NULL);
+ vma = i915_vma_instance(obj, vm, NULL);
if (IS_ERR(vma))
goto err;
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 836f1af8b833..6fe4682eacf3 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -23,6 +23,7 @@
*/
#include "../i915_selftest.h"
+#include "i915_random.h"
#include "igt_flush_test.h"
#include "mock_drm.h"
@@ -248,9 +249,9 @@ static int cpu_check(struct drm_i915_gem_object *obj, unsigned int max)
}
for (; m < DW_PER_PAGE; m++) {
- if (map[m] != 0xdeadbeef) {
+ if (map[m] != STACK_MAGIC) {
pr_err("Invalid value at page %d, offset %d: found %x expected %x\n",
- n, m, map[m], 0xdeadbeef);
+ n, m, map[m], STACK_MAGIC);
err = -EINVAL;
goto out_unmap;
}
@@ -306,7 +307,7 @@ create_test_object(struct i915_gem_context *ctx,
if (err)
return ERR_PTR(err);
- err = cpu_fill(obj, 0xdeadbeef);
+ err = cpu_fill(obj, STACK_MAGIC);
if (err) {
pr_err("Failed to fill object with cpu, err=%d\n",
err);
@@ -421,6 +422,110 @@ static int igt_ctx_exec(void *arg)
return err;
}
+static int igt_ctx_readonly(void *arg)
+{
+ struct drm_i915_private *i915 = arg;
+ struct drm_i915_gem_object *obj = NULL;
+ struct drm_file *file;
+ I915_RND_STATE(prng);
+ IGT_TIMEOUT(end_time);
+ LIST_HEAD(objects);
+ struct i915_gem_context *ctx;
+ struct i915_hw_ppgtt *ppgtt;
+ unsigned long ndwords, dw;
+ int err = -ENODEV;
+
+ /*
+ * Create a few read-only objects (with the occasional writable object)
+ * and try to write into these object checking that the GPU discards
+ * any write to a read-only object.
+ */
+
+ file = mock_file(i915);
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+
+ mutex_lock(&i915->drm.struct_mutex);
+
+ ctx = i915_gem_create_context(i915, file->driver_priv);
+ if (IS_ERR(ctx)) {
+ err = PTR_ERR(ctx);
+ goto out_unlock;
+ }
+
+ ppgtt = ctx->ppgtt ?: i915->mm.aliasing_ppgtt;
+ if (!ppgtt || !ppgtt->vm.has_read_only) {
+ err = 0;
+ goto out_unlock;
+ }
+
+ ndwords = 0;
+ dw = 0;
+ while (!time_after(jiffies, end_time)) {
+ struct intel_engine_cs *engine;
+ unsigned int id;
+
+ for_each_engine(engine, i915, id) {
+ if (!intel_engine_can_store_dword(engine))
+ continue;
+
+ if (!obj) {
+ obj = create_test_object(ctx, file, &objects);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ goto out_unlock;
+ }
+
+ obj->gt_ro = prandom_u32_state(&prng);
+ }
+
+ intel_runtime_pm_get(i915);
+ err = gpu_fill(obj, ctx, engine, dw);
+ intel_runtime_pm_put(i915);
+ if (err) {
+ pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) in ctx %u [full-ppgtt? %s], err=%d\n",
+ ndwords, dw, max_dwords(obj),
+ engine->name, ctx->hw_id,
+ yesno(!!ctx->ppgtt), err);
+ goto out_unlock;
+ }
+
+ if (++dw == max_dwords(obj)) {
+ obj = NULL;
+ dw = 0;
+ }
+ ndwords++;
+ }
+ }
+ pr_info("Submitted %lu dwords (across %u engines)\n",
+ ndwords, INTEL_INFO(i915)->num_rings);
+
+ dw = 0;
+ list_for_each_entry(obj, &objects, st_link) {
+ unsigned int rem =
+ min_t(unsigned int, ndwords - dw, max_dwords(obj));
+ unsigned int num_writes;
+
+ num_writes = rem;
+ if (obj->gt_ro)
+ num_writes = 0;
+
+ err = cpu_check(obj, num_writes);
+ if (err)
+ break;
+
+ dw += rem;
+ }
+
+out_unlock:
+ if (igt_flush_test(i915, I915_WAIT_LOCKED))
+ err = -EIO;
+ mutex_unlock(&i915->drm.struct_mutex);
+
+ mock_file_free(i915, file);
+ return err;
+}
+
static __maybe_unused const char *
__engine_name(struct drm_i915_private *i915, unsigned int engines)
{
@@ -595,6 +700,7 @@ int i915_gem_context_live_selftests(struct drm_i915_private *dev_priv)
static const struct i915_subtest tests[] = {
SUBTEST(igt_switch_to_kernel_context),
SUBTEST(igt_ctx_exec),
+ SUBTEST(igt_ctx_readonly),
};
bool fake_alias = false;
int err;
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] drm/i915: Prevent writing into a read-only object via a GGTT mmap
2018-06-15 8:08 ` Joonas Lahtinen
@ 2018-06-15 8:33 ` Chris Wilson
0 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2018-06-15 8:33 UTC (permalink / raw)
To: Joonas Lahtinen, intel-gfx; +Cc: David Herrmann
Quoting Joonas Lahtinen (2018-06-15 09:08:54)
> Quoting Chris Wilson (2018-06-14 22:24:02)
> > If the user has created a read-only object, they should not be allowed
> > to circumvent the write protection by using a GGTT mmapping. Deny it.
> >
> > Also most machines do not support read-only GGTT PTEs, so again we have
> > to reject attempted writes. Fortunately, this is known a priori, so we
> > can at least reject in the call to create the mmap (with a sanity check
> > in the fault handler).
> >
> > v2: Check the vma->vm_flags during mmap() to allow readonly access.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > Cc: David Herrmann <dh.herrmann@gmail.com>
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v1
> > Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> #v1
> > Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
>
> <SNIP>
>
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -1036,6 +1036,11 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> > return -EACCES;
> > }
> >
> > + if (vma->vm_flags & VM_WRITE && node->readonly) {
> > + drm_gem_object_put_unlocked(obj);
> > + return -EINVAL;
> > + }
> > +
>
> Pretty sure you want to split this patch and Cc: dri-devel. With that,
> both are:
I thought I cc'ed dri-devel on the previous sending. (At least I thought
about it.) Besides until ttm differentiates between read/write access,
it's a moot point.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev2)
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (7 preceding siblings ...)
2018-06-15 3:26 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-06-15 8:36 ` Patchwork
2018-06-15 15:35 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3) Patchwork
` (3 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-15 8:36 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev2)
URL : https://patchwork.freedesktop.org/series/44776/
State : failure
== Summary ==
Applying: drm/i915/gtt: Add read only pages to gen8_pte_encode
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/i915_gem_gtt.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_gem_gtt.c
Applying: drm/i915/gtt: Read-only pages for insert_entries on bdw+
Applying: drm/i915: Prevent writing into a read-only object via a GGTT mmap
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/i915_gem_gtt.c
M drivers/gpu/drm/i915/selftests/i915_gem_context.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/selftests/i915_gem_context.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/selftests/i915_gem_context.c
Auto-merging drivers/gpu/drm/i915/i915_gem_gtt.c
error: Failed to merge in the changes.
Patch failed at 0003 drm/i915: Prevent writing into a read-only object via a GGTT mmap
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3] drm/i915: Prevent writing into a read-only object via a GGTT mmap
2018-06-14 19:24 ` [PATCH 3/5] drm/i915: Prevent writing into a read-only object via a GGTT mmap Chris Wilson
2018-06-15 8:08 ` Joonas Lahtinen
@ 2018-06-15 15:26 ` Chris Wilson
1 sibling, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2018-06-15 15:26 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel, David Herrmann
If the user has created a read-only object, they should not be allowed
to circumvent the write protection by using a GGTT mmapping. Deny it.
Also most machines do not support read-only GGTT PTEs, so again we have
to reject attempted writes. Fortunately, this is known a priori, so we
can at least reject in the call to create the mmap (with a sanity check
in the fault handler).
v2: Check the vma->vm_flags during mmap() to allow readonly access.
v3: Remove VM_MAYWRITE to curtail mprotect()
Testcase: igt/gem_userptr_blits/readonly_mmap*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: David Herrmann <dh.herrmann@gmail.com>
Reviewed-by: Matthew Auld <matthew.william.auld@gmail.com> #v1
Reviewed-by: Jon Bloomfield <jon.bloomfield@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/drm_gem.c | 9 +++++++++
drivers/gpu/drm/i915/i915_gem.c | 4 ++++
drivers/gpu/drm/i915/i915_gem_gtt.c | 12 +++++++-----
drivers/gpu/drm/i915/i915_gem_object.h | 13 ++++++++++++-
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 5 +++--
include/drm/drm_vma_manager.h | 1 +
7 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 4a16d7b26c89..bf90625df3c5 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1036,6 +1036,15 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
return -EACCES;
}
+ if (node->readonly) {
+ if (vma->vm_flags & VM_WRITE) {
+ drm_gem_object_put_unlocked(obj);
+ return -EINVAL;
+ }
+
+ vma->vm_flags &= ~VM_MAYWRITE;
+ }
+
ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node) << PAGE_SHIFT,
vma);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a3515cad0b51..3c475c9fcdd9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2009,6 +2009,10 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
unsigned int flags;
int ret;
+ /* Sanity check that we allow writing into this object */
+ if (i915_gem_object_is_readonly(obj) && write)
+ return VM_FAULT_SIGBUS;
+
/* We don't use vmf->pgoff since that has the fake offset */
page_offset = (vmf->address - area->vm_start) >> PAGE_SHIFT;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 64f11cc9c3f2..a28943ae8d40 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -198,7 +198,7 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
/* Applicable to VLV, and gen8+ */
pte_flags = 0;
- if (vma->obj->gt_ro)
+ if (i915_gem_object_is_readonly(vma->obj))
pte_flags |= PTE_READ_ONLY;
vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags);
@@ -2413,8 +2413,10 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
const gen8_pte_t pte_encode = gen8_pte_encode(0, level, 0);
dma_addr_t addr;
- /* The GTT does not support read-only mappings */
- GEM_BUG_ON(flags & PTE_READ_ONLY);
+ /*
+ * Note that we ignore PTE_READ_ONLY here. The caller must be careful
+ * not to allow the user to override access to a read only page.
+ */
gtt_entries = (gen8_pte_t __iomem *)ggtt->gsm;
gtt_entries += vma->node.start >> PAGE_SHIFT;
@@ -2653,7 +2655,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
/* Applicable to VLV (gen8+ do not support RO in the GGTT) */
pte_flags = 0;
- if (obj->gt_ro)
+ if (i915_gem_object_is_readonly(obj))
pte_flags |= PTE_READ_ONLY;
intel_runtime_pm_get(i915);
@@ -2691,7 +2693,7 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
/* Currently applicable only to VLV */
pte_flags = 0;
- if (vma->obj->gt_ro)
+ if (i915_gem_object_is_readonly(vma->obj))
pte_flags |= PTE_READ_ONLY;
if (flags & I915_VMA_LOCAL_BIND) {
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 54f00b350779..fd703d768b70 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -141,7 +141,6 @@ struct drm_i915_gem_object {
* Is the object to be mapped as read-only to the GPU
* Only honoured if hardware has relevant pte bit
*/
- unsigned long gt_ro:1;
unsigned int cache_level:3;
unsigned int cache_coherent:2;
#define I915_BO_CACHE_COHERENT_FOR_READ BIT(0)
@@ -367,6 +366,18 @@ static inline void i915_gem_object_unlock(struct drm_i915_gem_object *obj)
reservation_object_unlock(obj->resv);
}
+static inline void
+i915_gem_object_set_readonly(struct drm_i915_gem_object *obj)
+{
+ obj->base.vma_node.readonly = true;
+}
+
+static inline bool
+i915_gem_object_is_readonly(const struct drm_i915_gem_object *obj)
+{
+ return obj->base.vma_node.readonly;
+}
+
static inline bool
i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
{
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8853a5e6d421..9d54c3c24b10 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1112,7 +1112,7 @@ intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
* if supported by the platform's GGTT.
*/
if (vm->has_read_only)
- obj->gt_ro = 1;
+ i915_gem_object_set_readonly(obj);
vma = i915_vma_instance(obj, vm, NULL);
if (IS_ERR(vma))
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 74fa99860d1b..ad3e4f5b6994 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -480,7 +480,8 @@ static int igt_ctx_readonly(void *arg)
goto out_unlock;
}
- obj->gt_ro = prandom_u32_state(&prng);
+ if (prandom_u32_state(&prng) & 1)
+ i915_gem_object_set_readonly(obj);
}
intel_runtime_pm_get(i915);
@@ -511,7 +512,7 @@ static int igt_ctx_readonly(void *arg)
unsigned int num_writes;
num_writes = rem;
- if (obj->gt_ro)
+ if (i915_gem_object_is_readonly(obj))
num_writes = 0;
err = cpu_check(obj, num_writes);
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 8758df94e9a0..c7987daeaed0 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -41,6 +41,7 @@ struct drm_vma_offset_node {
rwlock_t vm_lock;
struct drm_mm_node vm_node;
struct rb_root vm_files;
+ bool readonly:1;
};
struct drm_vma_offset_manager {
--
2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 21+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3)
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (8 preceding siblings ...)
2018-06-15 8:36 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev2) Patchwork
@ 2018-06-15 15:35 ` Patchwork
2018-06-15 15:37 ` ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-15 15:35 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3)
URL : https://patchwork.freedesktop.org/series/44776/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
0358c9fd1b9b drm/i915/gtt: Add read only pages to gen8_pte_encode
765448876d70 drm/i915/gtt: Read-only pages for insert_entries on bdw+
-:192: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>
#192: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:330:
+ bool pt_kmap_wc:1;
-:195: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>
#195: FILE: drivers/gpu/drm/i915/i915_gem_gtt.h:333:
+ bool has_read_only:1;
-:271: WARNING:LINE_SPACING: Missing a blank line after declarations
#271: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:430:
+ struct drm_file *file;
+ I915_RND_STATE(prng);
-:342: ERROR:CODE_INDENT: code indent should use tabs where possible
#342: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:501:
+^I ^Indwords, INTEL_INFO(i915)->num_rings);$
-:342: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#342: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:501:
+^I ^Indwords, INTEL_INFO(i915)->num_rings);$
-:342: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#342: FILE: drivers/gpu/drm/i915/selftests/i915_gem_context.c:501:
+ pr_info("Submitted %lu dwords (across %u engines)\n",
+ ndwords, INTEL_INFO(i915)->num_rings);
total: 1 errors, 4 warnings, 1 checks, 323 lines checked
9468f84d4626 drm/i915: Prevent writing into a read-only object via a GGTT mmap
-:182: WARNING:BOOL_BITFIELD: Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>
#182: FILE: include/drm/drm_vma_manager.h:44:
+ bool readonly:1;
total: 0 errors, 1 warnings, 0 checks, 118 lines checked
7927204dc8b7 drm/i915: Reject attempted pwrites into a read-only object
af7db8229431 drm/i915/userptr: Enable read-only support on gen8+
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✗ Fi.CI.SPARSE: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3)
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (9 preceding siblings ...)
2018-06-15 15:35 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3) Patchwork
@ 2018-06-15 15:37 ` Patchwork
2018-06-15 15:55 ` ✓ Fi.CI.BAT: success " Patchwork
2018-06-16 1:50 ` ✗ Fi.CI.IGT: failure " Patchwork
12 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-15 15:37 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3)
URL : https://patchwork.freedesktop.org/series/44776/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Commit: drm/i915/gtt: Add read only pages to gen8_pte_encode
Okay!
Commit: drm/i915/gtt: Read-only pages for insert_entries on bdw+
+drivers/gpu/drm/i915/selftests/i915_gem_context.c:506:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/i915_gem_context.c:506:25: warning: expression using sizeof(void)
Commit: drm/i915: Prevent writing into a read-only object via a GGTT mmap
Okay!
Commit: drm/i915: Reject attempted pwrites into a read-only object
Okay!
Commit: drm/i915/userptr: Enable read-only support on gen8+
Okay!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3)
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (10 preceding siblings ...)
2018-06-15 15:37 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-06-15 15:55 ` Patchwork
2018-06-16 1:50 ` ✗ Fi.CI.IGT: failure " Patchwork
12 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-15 15:55 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3)
URL : https://patchwork.freedesktop.org/series/44776/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4325 -> Patchwork_9330 =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_9330 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9330, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/44776/revisions/3/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9330:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_gttfill@basic:
fi-pnv-d510: SKIP -> PASS
== Known issues ==
Here are the changes found in Patchwork_9330 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_gttfill@basic:
fi-byt-n2820: PASS -> FAIL (fdo#106744)
==== Possible fixes ====
igt@kms_flip@basic-plain-flip:
fi-glk-j4005: DMESG-WARN (fdo#106097) -> PASS +1
igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence:
fi-glk-j4005: DMESG-WARN (fdo#106000, fdo#106097) -> PASS
fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097
fdo#106744 https://bugs.freedesktop.org/show_bug.cgi?id=106744
== Participating hosts (43 -> 38) ==
Missing (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u
== Build changes ==
* Linux: CI_DRM_4325 -> Patchwork_9330
CI_DRM_4325: 4275ebe85ad179007c49b7bcf78d340b7681871e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4520: 91f5d4665b07f073c78abd3cd4b8e0e347dbf638 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9330: af7db8229431c36770ba54e8e5003451794c8151 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
af7db8229431 drm/i915/userptr: Enable read-only support on gen8+
7927204dc8b7 drm/i915: Reject attempted pwrites into a read-only object
9468f84d4626 drm/i915: Prevent writing into a read-only object via a GGTT mmap
765448876d70 drm/i915/gtt: Read-only pages for insert_entries on bdw+
0358c9fd1b9b drm/i915/gtt: Add read only pages to gen8_pte_encode
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9330/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3)
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
` (11 preceding siblings ...)
2018-06-15 15:55 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-06-16 1:50 ` Patchwork
12 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2018-06-16 1:50 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3)
URL : https://patchwork.freedesktop.org/series/44776/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4325_full -> Patchwork_9330_full =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_9330_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_9330_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_9330_full:
=== IGT changes ===
==== Possible regressions ====
igt@gem_userptr_blits@usage-restrictions:
shard-kbl: PASS -> FAIL
shard-glk: PASS -> FAIL
shard-apl: PASS -> FAIL
==== Warnings ====
igt@drv_selftest@live_execlists:
shard-apl: SKIP -> PASS +1
== Known issues ==
Here are the changes found in Patchwork_9330_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
shard-glk: PASS -> FAIL (fdo#105703)
igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
shard-glk: PASS -> FAIL (fdo#105189)
igt@kms_flip@2x-modeset-vs-vblank-race-interruptible:
shard-glk: PASS -> FAIL (fdo#103060)
igt@kms_flip@2x-plain-flip-ts-check-interruptible:
shard-glk: PASS -> FAIL (fdo#100368)
igt@kms_flip@basic-flip-vs-wf_vblank:
shard-hsw: PASS -> FAIL (fdo#100368)
igt@kms_flip_tiling@flip-y-tiled:
shard-glk: PASS -> FAIL (fdo#104724)
igt@kms_setmode@basic:
shard-apl: PASS -> FAIL (fdo#99912)
==== Possible fixes ====
igt@drv_selftest@live_hangcheck:
shard-apl: DMESG-FAIL -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: FAIL (fdo#105363) -> PASS
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS
igt@kms_flip_tiling@flip-to-x-tiled:
shard-glk: FAIL (fdo#104724, fdo#103822) -> PASS
igt@kms_flip_tiling@flip-x-tiled:
shard-glk: FAIL (fdo#104724) -> PASS
igt@kms_vblank@pipe-c-accuracy-idle:
shard-glk: FAIL (fdo#102583) -> PASS
==== Warnings ====
igt@drv_selftest@live_gtt:
shard-glk: FAIL (fdo#105347) -> INCOMPLETE (k.org#198133, fdo#103359)
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133
== Participating hosts (5 -> 5) ==
No changes in participating hosts
== Build changes ==
* Linux: CI_DRM_4325 -> Patchwork_9330
CI_DRM_4325: 4275ebe85ad179007c49b7bcf78d340b7681871e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4520: 91f5d4665b07f073c78abd3cd4b8e0e347dbf638 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_9330: af7db8229431c36770ba54e8e5003451794c8151 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9330/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-06-16 1:50 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-14 19:24 [PATCH 1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Chris Wilson
2018-06-14 19:24 ` [PATCH 2/5] drm/i915/gtt: Read-only pages for insert_entries on bdw+ Chris Wilson
2018-06-14 21:32 ` Matthew Auld
2018-06-15 6:31 ` Chris Wilson
2018-06-15 8:06 ` Joonas Lahtinen
2018-06-15 8:26 ` [PATCH v4] " Chris Wilson
2018-06-14 19:24 ` [PATCH 3/5] drm/i915: Prevent writing into a read-only object via a GGTT mmap Chris Wilson
2018-06-15 8:08 ` Joonas Lahtinen
2018-06-15 8:33 ` Chris Wilson
2018-06-15 15:26 ` [PATCH v3] " Chris Wilson
2018-06-14 19:24 ` [PATCH 4/5] drm/i915: Reject attempted pwrites into a read-only object Chris Wilson
2018-06-14 19:24 ` [PATCH 5/5] drm/i915/userptr: Enable read-only support on gen8+ Chris Wilson
2018-06-14 19:41 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode Patchwork
2018-06-14 19:43 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-06-14 20:00 ` ✓ Fi.CI.BAT: success " Patchwork
2018-06-15 3:26 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-06-15 8:36 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev2) Patchwork
2018-06-15 15:35 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/gtt: Add read only pages to gen8_pte_encode (rev3) Patchwork
2018-06-15 15:37 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-06-15 15:55 ` ✓ Fi.CI.BAT: success " Patchwork
2018-06-16 1:50 ` ✗ Fi.CI.IGT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.