* [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
* 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
* 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
* [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
* [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
* 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
* 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
* [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
* [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
* ✗ 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
* ✗ 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
* ✗ 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.