| Message ID | 20210503155748.1961781-24-jason@jlekstrand.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gem: ioctl clean-ups (v5) | expand |
Hi Jason,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next next-20210503]
[cannot apply to tegra-drm/drm/tegra/for-next drm/drm-next v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jason-Ekstrand/drm-i915-gem-ioctl-clean-ups-v5/20210504-000132
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-s002-20210503 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/6af12f5ca765ecd59075344f3be4c4c0b68ef95e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jason-Ekstrand/drm-i915-gem-ioctl-clean-ups-v5/20210504-000132
git checkout 6af12f5ca765ecd59075344f3be4c4c0b68ef95e
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/gpu/drm/i915/gt/intel_reset.c:1329:5: sparse: sparse: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
drivers/gpu/drm/i915/gt/intel_reset.c: note: in included file:
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu *
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space *
--
drivers/gpu/drm/i915/gt/intel_execlists_submission.c: note: in included file (through drivers/gpu/drm/i915/selftests/igt_spinner.h, drivers/gpu/drm/i915/gt/selftest_execlists.c):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu *
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space *
--
drivers/gpu/drm/i915/gem/i915_gem_object.c: note: in included file:
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu *
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space *
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu *
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space *
drivers/gpu/drm/i915/gem/i915_gem_context.h:154:16: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/gpu/drm/i915/gem/i915_gem_context.h:154:16: sparse: struct i915_address_space [noderef] __rcu *
drivers/gpu/drm/i915/gem/i915_gem_context.h:154:16: sparse: struct i915_address_space *
--
drivers/gpu/drm/i915/i915_gem_gtt.c: note: in included file (through drivers/gpu/drm/i915/selftests/i915_gem_gtt.c):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu *
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space *
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu *
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space *
--
drivers/gpu/drm/i915/i915_vma.c: note: in included file:
>> drivers/gpu/drm/i915/selftests/i915_vma.c:42:24: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> drivers/gpu/drm/i915/selftests/i915_vma.c:42:24: sparse: struct i915_address_space [noderef] __rcu *
>> drivers/gpu/drm/i915/selftests/i915_vma.c:42:24: sparse: struct i915_address_space *
drivers/gpu/drm/i915/i915_vma.c: note: in included file (through drivers/gpu/drm/i915/selftests/i915_vma.c):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu *
>> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space *
vim +163 drivers/gpu/drm/i915/gem/i915_gem_context.h
a4e7ccdac38ec8 Chris Wilson 2019-10-04 156
a4e7ccdac38ec8 Chris Wilson 2019-10-04 157 static inline struct i915_address_space *
a4e7ccdac38ec8 Chris Wilson 2019-10-04 158 i915_gem_context_get_vm_rcu(struct i915_gem_context *ctx)
a4e7ccdac38ec8 Chris Wilson 2019-10-04 159 {
a4e7ccdac38ec8 Chris Wilson 2019-10-04 160 struct i915_address_space *vm;
a4e7ccdac38ec8 Chris Wilson 2019-10-04 161
a4e7ccdac38ec8 Chris Wilson 2019-10-04 162 rcu_read_lock();
a4e7ccdac38ec8 Chris Wilson 2019-10-04 @163 vm = rcu_dereference(ctx->vm);
a4e7ccdac38ec8 Chris Wilson 2019-10-04 164 if (!vm)
a4e7ccdac38ec8 Chris Wilson 2019-10-04 165 vm = &ctx->i915->ggtt.vm;
a4e7ccdac38ec8 Chris Wilson 2019-10-04 166 vm = i915_vm_get(vm);
a4e7ccdac38ec8 Chris Wilson 2019-10-04 167 rcu_read_unlock();
a4e7ccdac38ec8 Chris Wilson 2019-10-04 168
a4e7ccdac38ec8 Chris Wilson 2019-10-04 169 return vm;
a4e7ccdac38ec8 Chris Wilson 2019-10-04 170 }
a4e7ccdac38ec8 Chris Wilson 2019-10-04 171
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Tue, May 04, 2021 at 02:52:03AM +0800, kernel test robot wrote: > Hi Jason, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on drm-intel/for-linux-next] > [also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next next-20210503] > [cannot apply to tegra-drm/drm/tegra/for-next drm/drm-next v5.12] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: https://github.com/0day-ci/linux/commits/Jason-Ekstrand/drm-i915-gem-ioctl-clean-ups-v5/20210504-000132 > base: git://anongit.freedesktop.org/drm-intel for-linux-next > config: i386-randconfig-s002-20210503 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 > reproduce: > # apt-get install sparse > # sparse version: v0.6.3-341-g8af24329-dirty > # https://github.com/0day-ci/linux/commit/6af12f5ca765ecd59075344f3be4c4c0b68ef95e > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Jason-Ekstrand/drm-i915-gem-ioctl-clean-ups-v5/20210504-000132 > git checkout 6af12f5ca765ecd59075344f3be4c4c0b68ef95e > # save the attached .config to linux build tree > make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=i386 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> Just from staring at this I have no idea, so I guess you have to reproduce. What sparse does is primarily check these special bit and pointer values the kernel has, like __rcu and __user, where you need special functions to access them. But since the code is using rcu_dereference I have no idea what the complaint is about. -Daniel > > > sparse warnings: (new ones prefixed by >>) > drivers/gpu/drm/i915/gt/intel_reset.c:1329:5: sparse: sparse: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block > drivers/gpu/drm/i915/gt/intel_reset.c: note: in included file: > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu * > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space * > -- > drivers/gpu/drm/i915/gt/intel_execlists_submission.c: note: in included file (through drivers/gpu/drm/i915/selftests/igt_spinner.h, drivers/gpu/drm/i915/gt/selftest_execlists.c): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu * > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space * > -- > drivers/gpu/drm/i915/gem/i915_gem_object.c: note: in included file: > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu * > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space * > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu * > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space * > drivers/gpu/drm/i915/gem/i915_gem_context.h:154:16: sparse: sparse: incompatible types in comparison expression (different address spaces): > drivers/gpu/drm/i915/gem/i915_gem_context.h:154:16: sparse: struct i915_address_space [noderef] __rcu * > drivers/gpu/drm/i915/gem/i915_gem_context.h:154:16: sparse: struct i915_address_space * > -- > drivers/gpu/drm/i915/i915_gem_gtt.c: note: in included file (through drivers/gpu/drm/i915/selftests/i915_gem_gtt.c): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu * > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space * > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu * > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space * > -- > drivers/gpu/drm/i915/i915_vma.c: note: in included file: > >> drivers/gpu/drm/i915/selftests/i915_vma.c:42:24: sparse: sparse: incompatible types in comparison expression (different address spaces): > >> drivers/gpu/drm/i915/selftests/i915_vma.c:42:24: sparse: struct i915_address_space [noderef] __rcu * > >> drivers/gpu/drm/i915/selftests/i915_vma.c:42:24: sparse: struct i915_address_space * > drivers/gpu/drm/i915/i915_vma.c: note: in included file (through drivers/gpu/drm/i915/selftests/i915_vma.c): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: sparse: incompatible types in comparison expression (different address spaces): > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space [noderef] __rcu * > >> drivers/gpu/drm/i915/gem/i915_gem_context.h:163:14: sparse: struct i915_address_space * > > vim +163 drivers/gpu/drm/i915/gem/i915_gem_context.h > > a4e7ccdac38ec8 Chris Wilson 2019-10-04 156 > a4e7ccdac38ec8 Chris Wilson 2019-10-04 157 static inline struct i915_address_space * > a4e7ccdac38ec8 Chris Wilson 2019-10-04 158 i915_gem_context_get_vm_rcu(struct i915_gem_context *ctx) > a4e7ccdac38ec8 Chris Wilson 2019-10-04 159 { > a4e7ccdac38ec8 Chris Wilson 2019-10-04 160 struct i915_address_space *vm; > a4e7ccdac38ec8 Chris Wilson 2019-10-04 161 > a4e7ccdac38ec8 Chris Wilson 2019-10-04 162 rcu_read_lock(); > a4e7ccdac38ec8 Chris Wilson 2019-10-04 @163 vm = rcu_dereference(ctx->vm); > a4e7ccdac38ec8 Chris Wilson 2019-10-04 164 if (!vm) > a4e7ccdac38ec8 Chris Wilson 2019-10-04 165 vm = &ctx->i915->ggtt.vm; > a4e7ccdac38ec8 Chris Wilson 2019-10-04 166 vm = i915_vm_get(vm); > a4e7ccdac38ec8 Chris Wilson 2019-10-04 167 rcu_read_unlock(); > a4e7ccdac38ec8 Chris Wilson 2019-10-04 168 > a4e7ccdac38ec8 Chris Wilson 2019-10-04 169 return vm; > a4e7ccdac38ec8 Chris Wilson 2019-10-04 170 } > a4e7ccdac38ec8 Chris Wilson 2019-10-04 171 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, May 03, 2021 at 10:57:44AM -0500, Jason Ekstrand wrote: > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> First, a bit of commit message please here. Second I accidentally went down the rabbit hole and ended up at try_qad_pin(). Your patch looks good, unfortunately my brain is complete mushroom soup now. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/gem/i915_gem_context.c | 267 ------------------ > .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +- > .../drm/i915/gem/selftests/i915_gem_context.c | 119 -------- > .../drm/i915/selftests/i915_mock_selftests.h | 1 - > 4 files changed, 1 insertion(+), 388 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c > index f0e7ce6b979b4..ad6e98d8a4fbd 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > @@ -1637,121 +1637,6 @@ int i915_gem_vm_destroy_ioctl(struct drm_device *dev, void *data, > return 0; > } > > -struct context_barrier_task { > - struct i915_active base; > - void (*task)(void *data); > - void *data; > -}; > - > -__i915_active_call > -static void cb_retire(struct i915_active *base) > -{ > - struct context_barrier_task *cb = container_of(base, typeof(*cb), base); > - > - if (cb->task) > - cb->task(cb->data); > - > - i915_active_fini(&cb->base); > - kfree(cb); > -} > - > -I915_SELFTEST_DECLARE(static intel_engine_mask_t context_barrier_inject_fault); > -static int context_barrier_task(struct i915_gem_context *ctx, > - intel_engine_mask_t engines, > - bool (*skip)(struct intel_context *ce, void *data), > - int (*pin)(struct intel_context *ce, struct i915_gem_ww_ctx *ww, void *data), > - int (*emit)(struct i915_request *rq, void *data), > - void (*task)(void *data), > - void *data) > -{ > - struct context_barrier_task *cb; > - struct i915_gem_engines_iter it; > - struct i915_gem_engines *e; > - struct i915_gem_ww_ctx ww; > - struct intel_context *ce; > - int err = 0; > - > - GEM_BUG_ON(!task); > - > - cb = kmalloc(sizeof(*cb), GFP_KERNEL); > - if (!cb) > - return -ENOMEM; > - > - i915_active_init(&cb->base, NULL, cb_retire); > - err = i915_active_acquire(&cb->base); > - if (err) { > - kfree(cb); > - return err; > - } > - > - e = __context_engines_await(ctx, NULL); > - if (!e) { > - i915_active_release(&cb->base); > - return -ENOENT; > - } > - > - for_each_gem_engine(ce, e, it) { > - struct i915_request *rq; > - > - if (I915_SELFTEST_ONLY(context_barrier_inject_fault & > - ce->engine->mask)) { > - err = -ENXIO; > - break; > - } > - > - if (!(ce->engine->mask & engines)) > - continue; > - > - if (skip && skip(ce, data)) > - continue; > - > - i915_gem_ww_ctx_init(&ww, true); > -retry: > - err = intel_context_pin_ww(ce, &ww); > - if (err) > - goto err; > - > - if (pin) > - err = pin(ce, &ww, data); > - if (err) > - goto err_unpin; > - > - rq = i915_request_create(ce); > - if (IS_ERR(rq)) { > - err = PTR_ERR(rq); > - goto err_unpin; > - } > - > - err = 0; > - if (emit) > - err = emit(rq, data); > - if (err == 0) > - err = i915_active_add_request(&cb->base, rq); > - > - i915_request_add(rq); > -err_unpin: > - intel_context_unpin(ce); > -err: > - if (err == -EDEADLK) { > - err = i915_gem_ww_ctx_backoff(&ww); > - if (!err) > - goto retry; > - } > - i915_gem_ww_ctx_fini(&ww); > - > - if (err) > - break; > - } > - i915_sw_fence_complete(&e->fence); > - > - cb->task = err ? NULL : task; /* caller needs to unwind instead */ > - cb->data = data; > - > - i915_active_release(&cb->base); > - > - return err; > -} > - > static int get_ppgtt(struct drm_i915_file_private *file_priv, > struct i915_gem_context *ctx, > struct drm_i915_gem_context_param *args) > @@ -1784,154 +1669,6 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv, > return err; > } > > -static void set_ppgtt_barrier(void *data) > -{ > - struct i915_address_space *old = data; > - > - if (INTEL_GEN(old->i915) < 8) > - gen6_ppgtt_unpin_all(i915_vm_to_ppgtt(old)); > - > - i915_vm_close(old); > -} > - > -static int pin_ppgtt_update(struct intel_context *ce, struct i915_gem_ww_ctx *ww, void *data) > -{ > - struct i915_address_space *vm = ce->vm; > - > - if (!HAS_LOGICAL_RING_CONTEXTS(vm->i915)) > - /* ppGTT is not part of the legacy context image */ > - return gen6_ppgtt_pin(i915_vm_to_ppgtt(vm), ww); > - > - return 0; > -} > - > -static int emit_ppgtt_update(struct i915_request *rq, void *data) > -{ > - struct i915_address_space *vm = rq->context->vm; > - struct intel_engine_cs *engine = rq->engine; > - u32 base = engine->mmio_base; > - u32 *cs; > - int i; > - > - if (i915_vm_is_4lvl(vm)) { > - struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm); > - const dma_addr_t pd_daddr = px_dma(ppgtt->pd); > - > - cs = intel_ring_begin(rq, 6); > - if (IS_ERR(cs)) > - return PTR_ERR(cs); > - > - *cs++ = MI_LOAD_REGISTER_IMM(2); > - > - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 0)); > - *cs++ = upper_32_bits(pd_daddr); > - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, 0)); > - *cs++ = lower_32_bits(pd_daddr); > - > - *cs++ = MI_NOOP; > - intel_ring_advance(rq, cs); > - } else if (HAS_LOGICAL_RING_CONTEXTS(engine->i915)) { > - struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm); > - int err; > - > - /* Magic required to prevent forcewake errors! */ > - err = engine->emit_flush(rq, EMIT_INVALIDATE); > - if (err) > - return err; > - > - cs = intel_ring_begin(rq, 4 * GEN8_3LVL_PDPES + 2); > - if (IS_ERR(cs)) > - return PTR_ERR(cs); > - > - *cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES) | MI_LRI_FORCE_POSTED; > - for (i = GEN8_3LVL_PDPES; i--; ) { > - const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i); > - > - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, i)); > - *cs++ = upper_32_bits(pd_daddr); > - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, i)); > - *cs++ = lower_32_bits(pd_daddr); > - } > - *cs++ = MI_NOOP; > - intel_ring_advance(rq, cs); > - } > - > - return 0; > -} > - > -static bool skip_ppgtt_update(struct intel_context *ce, void *data) > -{ > - if (HAS_LOGICAL_RING_CONTEXTS(ce->engine->i915)) > - return !ce->state; > - else > - return !atomic_read(&ce->pin_count); > -} > - > -static int set_ppgtt(struct drm_i915_file_private *file_priv, > - struct i915_gem_context *ctx, > - struct drm_i915_gem_context_param *args) > -{ > - struct i915_address_space *vm, *old; > - int err; > - > - if (args->size) > - return -EINVAL; > - > - if (!rcu_access_pointer(ctx->vm)) > - return -ENODEV; > - > - if (upper_32_bits(args->value)) > - return -ENOENT; > - > - rcu_read_lock(); > - vm = xa_load(&file_priv->vm_xa, args->value); > - if (vm && !kref_get_unless_zero(&vm->ref)) > - vm = NULL; > - rcu_read_unlock(); > - if (!vm) > - return -ENOENT; > - > - err = mutex_lock_interruptible(&ctx->mutex); > - if (err) > - goto out; > - > - if (i915_gem_context_is_closed(ctx)) { > - err = -ENOENT; > - goto unlock; > - } > - > - if (vm == rcu_access_pointer(ctx->vm)) > - goto unlock; > - > - old = __set_ppgtt(ctx, vm); > - > - /* Teardown the existing obj:vma cache, it will have to be rebuilt. */ > - lut_close(ctx); > - > - /* > - * We need to flush any requests using the current ppgtt before > - * we release it as the requests do not hold a reference themselves, > - * only indirectly through the context. > - */ > - err = context_barrier_task(ctx, ALL_ENGINES, > - skip_ppgtt_update, > - pin_ppgtt_update, > - emit_ppgtt_update, > - set_ppgtt_barrier, > - old); > - if (err) { > - i915_vm_close(__set_ppgtt(ctx, old)); > - i915_vm_close(old); > - lut_close(ctx); /* force a rebuild of the old obj:vma cache */ > - } > - > -unlock: > - mutex_unlock(&ctx->mutex); > -out: > - i915_vm_put(vm); > - return err; > -} > - > int > i915_gem_user_to_context_sseu(struct intel_gt *gt, > const struct drm_i915_gem_context_param_sseu *user, > @@ -2468,10 +2205,6 @@ static int ctx_setparam(struct drm_i915_file_private *fpriv, > ret = set_sseu(ctx, args); > break; > > - case I915_CONTEXT_PARAM_VM: > - ret = set_ppgtt(fpriv, ctx, args); > - break; > - > case I915_CONTEXT_PARAM_ENGINES: > ret = set_engines(ctx, args); > break; > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h > index b673061f4f5ba..84c821cca1739 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h > @@ -262,7 +262,7 @@ struct i915_gem_context { > * In other modes, this is a NULL pointer with the expectation that > * the caller uses the shared global GTT. > */ > - struct i915_address_space __rcu *vm; > + struct i915_address_space *vm; > > /** > * @pid: process id of creator > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > index 5fef592390cb5..16ff64ab34a1b 100644 > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > @@ -1882,125 +1882,6 @@ static int igt_vm_isolation(void *arg) > return err; > } > > -static bool skip_unused_engines(struct intel_context *ce, void *data) > -{ > - return !ce->state; > -} > - > -static void mock_barrier_task(void *data) > -{ > - unsigned int *counter = data; > - > - ++*counter; > -} > - > -static int mock_context_barrier(void *arg) > -{ > -#undef pr_fmt > -#define pr_fmt(x) "context_barrier_task():" # x > - struct drm_i915_private *i915 = arg; > - struct i915_gem_context *ctx; > - struct i915_request *rq; > - unsigned int counter; > - int err; > - > - /* > - * The context barrier provides us with a callback after it emits > - * a request; useful for retiring old state after loading new. > - */ > - > - ctx = mock_context(i915, "mock"); > - if (!ctx) > - return -ENOMEM; > - > - counter = 0; > - err = context_barrier_task(ctx, 0, NULL, NULL, NULL, > - mock_barrier_task, &counter); > - if (err) { > - pr_err("Failed at line %d, err=%d\n", __LINE__, err); > - goto out; > - } > - if (counter == 0) { > - pr_err("Did not retire immediately with 0 engines\n"); > - err = -EINVAL; > - goto out; > - } > - > - counter = 0; > - err = context_barrier_task(ctx, ALL_ENGINES, skip_unused_engines, > - NULL, NULL, mock_barrier_task, &counter); > - if (err) { > - pr_err("Failed at line %d, err=%d\n", __LINE__, err); > - goto out; > - } > - if (counter == 0) { > - pr_err("Did not retire immediately for all unused engines\n"); > - err = -EINVAL; > - goto out; > - } > - > - rq = igt_request_alloc(ctx, i915->gt.engine[RCS0]); > - if (IS_ERR(rq)) { > - pr_err("Request allocation failed!\n"); > - goto out; > - } > - i915_request_add(rq); > - > - counter = 0; > - context_barrier_inject_fault = BIT(RCS0); > - err = context_barrier_task(ctx, ALL_ENGINES, NULL, NULL, NULL, > - mock_barrier_task, &counter); > - context_barrier_inject_fault = 0; > - if (err == -ENXIO) > - err = 0; > - else > - pr_err("Did not hit fault injection!\n"); > - if (counter != 0) { > - pr_err("Invoked callback on error!\n"); > - err = -EIO; > - } > - if (err) > - goto out; > - > - counter = 0; > - err = context_barrier_task(ctx, ALL_ENGINES, skip_unused_engines, > - NULL, NULL, mock_barrier_task, &counter); > - if (err) { > - pr_err("Failed at line %d, err=%d\n", __LINE__, err); > - goto out; > - } > - mock_device_flush(i915); > - if (counter == 0) { > - pr_err("Did not retire on each active engines\n"); > - err = -EINVAL; > - goto out; > - } > - > -out: > - mock_context_close(ctx); > - return err; > -#undef pr_fmt > -#define pr_fmt(x) x > -} > - > -int i915_gem_context_mock_selftests(void) > -{ > - static const struct i915_subtest tests[] = { > - SUBTEST(mock_context_barrier), > - }; > - struct drm_i915_private *i915; > - int err; > - > - i915 = mock_gem_device(); > - if (!i915) > - return -ENOMEM; > - > - err = i915_subtests(tests, i915); > - > - mock_destroy_device(i915); > - return err; > -} > - > int i915_gem_context_live_selftests(struct drm_i915_private *i915) > { > static const struct i915_subtest tests[] = { > diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h > index 3db34d3eea58a..52aa91716dc1f 100644 > --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h > +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h > @@ -32,6 +32,5 @@ selftest(vma, i915_vma_mock_selftests) > selftest(evict, i915_gem_evict_mock_selftests) > selftest(gtt, i915_gem_gtt_mock_selftests) > selftest(hugepages, i915_gem_huge_page_mock_selftests) > -selftest(contexts, i915_gem_context_mock_selftests) > selftest(buddy, i915_buddy_mock_selftests) > selftest(memory_region, intel_memory_region_mock_selftests) > -- > 2.31.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index f0e7ce6b979b4..ad6e98d8a4fbd 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -1637,121 +1637,6 @@ int i915_gem_vm_destroy_ioctl(struct drm_device *dev, void *data, return 0; } -struct context_barrier_task { - struct i915_active base; - void (*task)(void *data); - void *data; -}; - -__i915_active_call -static void cb_retire(struct i915_active *base) -{ - struct context_barrier_task *cb = container_of(base, typeof(*cb), base); - - if (cb->task) - cb->task(cb->data); - - i915_active_fini(&cb->base); - kfree(cb); -} - -I915_SELFTEST_DECLARE(static intel_engine_mask_t context_barrier_inject_fault); -static int context_barrier_task(struct i915_gem_context *ctx, - intel_engine_mask_t engines, - bool (*skip)(struct intel_context *ce, void *data), - int (*pin)(struct intel_context *ce, struct i915_gem_ww_ctx *ww, void *data), - int (*emit)(struct i915_request *rq, void *data), - void (*task)(void *data), - void *data) -{ - struct context_barrier_task *cb; - struct i915_gem_engines_iter it; - struct i915_gem_engines *e; - struct i915_gem_ww_ctx ww; - struct intel_context *ce; - int err = 0; - - GEM_BUG_ON(!task); - - cb = kmalloc(sizeof(*cb), GFP_KERNEL); - if (!cb) - return -ENOMEM; - - i915_active_init(&cb->base, NULL, cb_retire); - err = i915_active_acquire(&cb->base); - if (err) { - kfree(cb); - return err; - } - - e = __context_engines_await(ctx, NULL); - if (!e) { - i915_active_release(&cb->base); - return -ENOENT; - } - - for_each_gem_engine(ce, e, it) { - struct i915_request *rq; - - if (I915_SELFTEST_ONLY(context_barrier_inject_fault & - ce->engine->mask)) { - err = -ENXIO; - break; - } - - if (!(ce->engine->mask & engines)) - continue; - - if (skip && skip(ce, data)) - continue; - - i915_gem_ww_ctx_init(&ww, true); -retry: - err = intel_context_pin_ww(ce, &ww); - if (err) - goto err; - - if (pin) - err = pin(ce, &ww, data); - if (err) - goto err_unpin; - - rq = i915_request_create(ce); - if (IS_ERR(rq)) { - err = PTR_ERR(rq); - goto err_unpin; - } - - err = 0; - if (emit) - err = emit(rq, data); - if (err == 0) - err = i915_active_add_request(&cb->base, rq); - - i915_request_add(rq); -err_unpin: - intel_context_unpin(ce); -err: - if (err == -EDEADLK) { - err = i915_gem_ww_ctx_backoff(&ww); - if (!err) - goto retry; - } - i915_gem_ww_ctx_fini(&ww); - - if (err) - break; - } - i915_sw_fence_complete(&e->fence); - - cb->task = err ? NULL : task; /* caller needs to unwind instead */ - cb->data = data; - - i915_active_release(&cb->base); - - return err; -} - static int get_ppgtt(struct drm_i915_file_private *file_priv, struct i915_gem_context *ctx, struct drm_i915_gem_context_param *args) @@ -1784,154 +1669,6 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv, return err; } -static void set_ppgtt_barrier(void *data) -{ - struct i915_address_space *old = data; - - if (INTEL_GEN(old->i915) < 8) - gen6_ppgtt_unpin_all(i915_vm_to_ppgtt(old)); - - i915_vm_close(old); -} - -static int pin_ppgtt_update(struct intel_context *ce, struct i915_gem_ww_ctx *ww, void *data) -{ - struct i915_address_space *vm = ce->vm; - - if (!HAS_LOGICAL_RING_CONTEXTS(vm->i915)) - /* ppGTT is not part of the legacy context image */ - return gen6_ppgtt_pin(i915_vm_to_ppgtt(vm), ww); - - return 0; -} - -static int emit_ppgtt_update(struct i915_request *rq, void *data) -{ - struct i915_address_space *vm = rq->context->vm; - struct intel_engine_cs *engine = rq->engine; - u32 base = engine->mmio_base; - u32 *cs; - int i; - - if (i915_vm_is_4lvl(vm)) { - struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm); - const dma_addr_t pd_daddr = px_dma(ppgtt->pd); - - cs = intel_ring_begin(rq, 6); - if (IS_ERR(cs)) - return PTR_ERR(cs); - - *cs++ = MI_LOAD_REGISTER_IMM(2); - - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 0)); - *cs++ = upper_32_bits(pd_daddr); - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, 0)); - *cs++ = lower_32_bits(pd_daddr); - - *cs++ = MI_NOOP; - intel_ring_advance(rq, cs); - } else if (HAS_LOGICAL_RING_CONTEXTS(engine->i915)) { - struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm); - int err; - - /* Magic required to prevent forcewake errors! */ - err = engine->emit_flush(rq, EMIT_INVALIDATE); - if (err) - return err; - - cs = intel_ring_begin(rq, 4 * GEN8_3LVL_PDPES + 2); - if (IS_ERR(cs)) - return PTR_ERR(cs); - - *cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES) | MI_LRI_FORCE_POSTED; - for (i = GEN8_3LVL_PDPES; i--; ) { - const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i); - - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, i)); - *cs++ = upper_32_bits(pd_daddr); - *cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, i)); - *cs++ = lower_32_bits(pd_daddr); - } - *cs++ = MI_NOOP; - intel_ring_advance(rq, cs); - } - - return 0; -} - -static bool skip_ppgtt_update(struct intel_context *ce, void *data) -{ - if (HAS_LOGICAL_RING_CONTEXTS(ce->engine->i915)) - return !ce->state; - else - return !atomic_read(&ce->pin_count); -} - -static int set_ppgtt(struct drm_i915_file_private *file_priv, - struct i915_gem_context *ctx, - struct drm_i915_gem_context_param *args) -{ - struct i915_address_space *vm, *old; - int err; - - if (args->size) - return -EINVAL; - - if (!rcu_access_pointer(ctx->vm)) - return -ENODEV; - - if (upper_32_bits(args->value)) - return -ENOENT; - - rcu_read_lock(); - vm = xa_load(&file_priv->vm_xa, args->value); - if (vm && !kref_get_unless_zero(&vm->ref)) - vm = NULL; - rcu_read_unlock(); - if (!vm) - return -ENOENT; - - err = mutex_lock_interruptible(&ctx->mutex); - if (err) - goto out; - - if (i915_gem_context_is_closed(ctx)) { - err = -ENOENT; - goto unlock; - } - - if (vm == rcu_access_pointer(ctx->vm)) - goto unlock; - - old = __set_ppgtt(ctx, vm); - - /* Teardown the existing obj:vma cache, it will have to be rebuilt. */ - lut_close(ctx); - - /* - * We need to flush any requests using the current ppgtt before - * we release it as the requests do not hold a reference themselves, - * only indirectly through the context. - */ - err = context_barrier_task(ctx, ALL_ENGINES, - skip_ppgtt_update, - pin_ppgtt_update, - emit_ppgtt_update, - set_ppgtt_barrier, - old); - if (err) { - i915_vm_close(__set_ppgtt(ctx, old)); - i915_vm_close(old); - lut_close(ctx); /* force a rebuild of the old obj:vma cache */ - } - -unlock: - mutex_unlock(&ctx->mutex); -out: - i915_vm_put(vm); - return err; -} - int i915_gem_user_to_context_sseu(struct intel_gt *gt, const struct drm_i915_gem_context_param_sseu *user, @@ -2468,10 +2205,6 @@ static int ctx_setparam(struct drm_i915_file_private *fpriv, ret = set_sseu(ctx, args); break; - case I915_CONTEXT_PARAM_VM: - ret = set_ppgtt(fpriv, ctx, args); - break; - case I915_CONTEXT_PARAM_ENGINES: ret = set_engines(ctx, args); break; diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h index b673061f4f5ba..84c821cca1739 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h @@ -262,7 +262,7 @@ struct i915_gem_context { * In other modes, this is a NULL pointer with the expectation that * the caller uses the shared global GTT. */ - struct i915_address_space __rcu *vm; + struct i915_address_space *vm; /** * @pid: process id of creator diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c index 5fef592390cb5..16ff64ab34a1b 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c @@ -1882,125 +1882,6 @@ static int igt_vm_isolation(void *arg) return err; } -static bool skip_unused_engines(struct intel_context *ce, void *data) -{ - return !ce->state; -} - -static void mock_barrier_task(void *data) -{ - unsigned int *counter = data; - - ++*counter; -} - -static int mock_context_barrier(void *arg) -{ -#undef pr_fmt -#define pr_fmt(x) "context_barrier_task():" # x - struct drm_i915_private *i915 = arg; - struct i915_gem_context *ctx; - struct i915_request *rq; - unsigned int counter; - int err; - - /* - * The context barrier provides us with a callback after it emits - * a request; useful for retiring old state after loading new. - */ - - ctx = mock_context(i915, "mock"); - if (!ctx) - return -ENOMEM; - - counter = 0; - err = context_barrier_task(ctx, 0, NULL, NULL, NULL, - mock_barrier_task, &counter); - if (err) { - pr_err("Failed at line %d, err=%d\n", __LINE__, err); - goto out; - } - if (counter == 0) { - pr_err("Did not retire immediately with 0 engines\n"); - err = -EINVAL; - goto out; - } - - counter = 0; - err = context_barrier_task(ctx, ALL_ENGINES, skip_unused_engines, - NULL, NULL, mock_barrier_task, &counter); - if (err) { - pr_err("Failed at line %d, err=%d\n", __LINE__, err); - goto out; - } - if (counter == 0) { - pr_err("Did not retire immediately for all unused engines\n"); - err = -EINVAL; - goto out; - } - - rq = igt_request_alloc(ctx, i915->gt.engine[RCS0]); - if (IS_ERR(rq)) { - pr_err("Request allocation failed!\n"); - goto out; - } - i915_request_add(rq); - - counter = 0; - context_barrier_inject_fault = BIT(RCS0); - err = context_barrier_task(ctx, ALL_ENGINES, NULL, NULL, NULL, - mock_barrier_task, &counter); - context_barrier_inject_fault = 0; - if (err == -ENXIO) - err = 0; - else - pr_err("Did not hit fault injection!\n"); - if (counter != 0) { - pr_err("Invoked callback on error!\n"); - err = -EIO; - } - if (err) - goto out; - - counter = 0; - err = context_barrier_task(ctx, ALL_ENGINES, skip_unused_engines, - NULL, NULL, mock_barrier_task, &counter); - if (err) { - pr_err("Failed at line %d, err=%d\n", __LINE__, err); - goto out; - } - mock_device_flush(i915); - if (counter == 0) { - pr_err("Did not retire on each active engines\n"); - err = -EINVAL; - goto out; - } - -out: - mock_context_close(ctx); - return err; -#undef pr_fmt -#define pr_fmt(x) x -} - -int i915_gem_context_mock_selftests(void) -{ - static const struct i915_subtest tests[] = { - SUBTEST(mock_context_barrier), - }; - struct drm_i915_private *i915; - int err; - - i915 = mock_gem_device(); - if (!i915) - return -ENOMEM; - - err = i915_subtests(tests, i915); - - mock_destroy_device(i915); - return err; -} - int i915_gem_context_live_selftests(struct drm_i915_private *i915) { static const struct i915_subtest tests[] = { diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h index 3db34d3eea58a..52aa91716dc1f 100644 --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h @@ -32,6 +32,5 @@ selftest(vma, i915_vma_mock_selftests) selftest(evict, i915_gem_evict_mock_selftests) selftest(gtt, i915_gem_gtt_mock_selftests) selftest(hugepages, i915_gem_huge_page_mock_selftests) -selftest(contexts, i915_gem_context_mock_selftests) selftest(buddy, i915_buddy_mock_selftests) selftest(memory_region, intel_memory_region_mock_selftests)
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 267 ------------------ .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +- .../drm/i915/gem/selftests/i915_gem_context.c | 119 -------- .../drm/i915/selftests/i915_mock_selftests.h | 1 - 4 files changed, 1 insertion(+), 388 deletions(-)