| Message ID | 20220624110821.29190-1-nirmoy.das@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915: Fix a lockdep warning at error capture | expand |
On 24.06.2022 13:08, Nirmoy Das wrote: > For some platfroms we use stop_machine version of > gen8_ggtt_insert_page/gen8_ggtt_insert_entries to avoid a > concurrent GGTT access bug but this causes a circular locking > dependency warning: > > Possible unsafe locking scenario: > CPU0 CPU1 > ---- ---- > lock(&ggtt->error_mutex); > lock(dma_fence_map); > lock(&ggtt->error_mutex); > lock(cpu_hotplug_lock); > > Fix this by calling gen8_ggtt_insert_page/gen8_ggtt_insert_entries > directly at error capture which is concurrent GGTT access safe because > reset path make sure of that. > > v2: Fix rebase conflict and added a comment. > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5595 > Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_ggtt.c | 10 ++++++++++ > drivers/gpu/drm/i915/gt/intel_gtt.h | 9 +++++++++ > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 ++++- > drivers/gpu/drm/i915/i915_gpu_error.c | 8 ++++++-- > 4 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c > index ffff96180313..15a915bb4088 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c > @@ -960,6 +960,16 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) > if (intel_vm_no_concurrent_access_wa(i915)) { > ggtt->vm.insert_entries = bxt_vtd_ggtt_insert_entries__BKL; > ggtt->vm.insert_page = bxt_vtd_ggtt_insert_page__BKL; > + > + /* > + * Calling stop_machine() version of GGTT update function > + * at error capture/reset path will raise lockdep warning. > + * Allow calling gen8_ggtt_insert_* directly at reset path > + * which is safe from parallel GGTT updates. > + */ > + ggtt->vm.raw_insert_page = gen8_ggtt_insert_page; > + ggtt->vm.raw_insert_entries = gen8_ggtt_insert_entries; > + > ggtt->vm.bind_async_flags = > I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND; > } > diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h > index 29fd3a9e8b2e..e639434e97fd 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gtt.h > +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h > @@ -306,6 +306,15 @@ struct i915_address_space { > struct i915_vma_resource *vma_res, > enum i915_cache_level cache_level, > u32 flags); > + void (*raw_insert_page)(struct i915_address_space *vm, > + dma_addr_t addr, > + u64 offset, > + enum i915_cache_level cache_level, > + u32 flags); > + void (*raw_insert_entries)(struct i915_address_space *vm, > + struct i915_vma_resource *vma_res, > + enum i915_cache_level cache_level, > + u32 flags); I would expect rather extra flag to insert_(page|entries) instead of extra callbacks. Anyway both should work. Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Regards Andrzej > void (*cleanup)(struct i915_address_space *vm); > > void (*foreach)(struct i915_address_space *vm, > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > index d2c5c9367cc4..c06e83872c34 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > @@ -493,7 +493,10 @@ static void uc_fw_bind_ggtt(struct intel_uc_fw *uc_fw) > if (i915_gem_object_is_lmem(obj)) > pte_flags |= PTE_LM; > > - ggtt->vm.insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, pte_flags); > + if (ggtt->vm.raw_insert_entries) > + ggtt->vm.raw_insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, pte_flags); > + else > + ggtt->vm.insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, pte_flags); > } > > static void uc_fw_unbind_ggtt(struct intel_uc_fw *uc_fw) > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index bff8a111424a..f9b1969ed7ed 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -1104,8 +1104,12 @@ i915_vma_coredump_create(const struct intel_gt *gt, > > for_each_sgt_daddr(dma, iter, vma_res->bi.pages) { > mutex_lock(&ggtt->error_mutex); > - ggtt->vm.insert_page(&ggtt->vm, dma, slot, > - I915_CACHE_NONE, 0); > + if (ggtt->vm.raw_insert_page) > + ggtt->vm.raw_insert_page(&ggtt->vm, dma, slot, > + I915_CACHE_NONE, 0); > + else > + ggtt->vm.insert_page(&ggtt->vm, dma, slot, > + I915_CACHE_NONE, 0); > mb(); > > s = io_mapping_map_wc(&ggtt->iomap, slot, PAGE_SIZE);
On 6/24/2022 2:49 PM, Patchwork wrote: > Project List - Patchwork *Patch Details* > *Series:* drm/i915: Fix a lockdep warning at error capture (rev2) > *URL:* https://patchwork.freedesktop.org/series/105291/ > *State:* failure > *Details:* > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/index.html > > > CI Bug Log - changes from CI_DRM_11803 -> Patchwork_105291v2 > > > Summary > > *FAILURE* > > Serious unknown changes coming with Patchwork_105291v2 absolutely need > to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_105291v2, please notify your bug team to allow > them > to document this new failure mode, which will reduce false positives > in CI. > > External URL: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/index.html > > > Participating hosts (36 -> 37) > > Additional (4): fi-kbl-soraka bat-adlm-1 fi-bdw-5557u bat-adlp-6 > Missing (3): bat-dg2-8 fi-bdw-samus fi-pnv-d510 > > > Possible new issues > > Here are the unknown changes that may have been introduced in > Patchwork_105291v2: > > > IGT changes > > > Possible regressions > > * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions: > > o fi-bsw-kefka: PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html> > -> FAIL > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html> > This is not related and the test is failing without the patch as well see: https://intel-gfx-ci.01.org/tree/drm-tip/fi-bsw-kefka.html Nirmoy > * > > > Suppressed > > The following results come from untrusted machines, tests, or statuses. > They do not affect the overall result. > > * > > igt@i915_module_load@reload: > > o {bat-adln-1}: PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/bat-adln-1/igt@i915_module_load@reload.html> > -> DMESG-WARN > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/bat-adln-1/igt@i915_module_load@reload.html> > * > > igt@i915_selftest@live@hangcheck: > > o {bat-adlm-1}: NOTRUN -> INCOMPLETE > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/bat-adlm-1/igt@i915_selftest@live@hangcheck.html> > * > > igt@kms_pipe_crc_basic@hang-read-crc: > > o {bat-adlm-1}: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/bat-adlm-1/igt@kms_pipe_crc_basic@hang-read-crc.html> > +8 similar issues > > > Known issues > > Here are the changes found in Patchwork_105291v2 that come from known > issues: > > > IGT changes > > > Issues hit > > * > > igt@gem_exec_gttfill@basic: > > o fi-kbl-soraka: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-kbl-soraka/igt@gem_exec_gttfill@basic.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271>) +8 > similar issues > * > > igt@gem_huc_copy@huc-copy: > > o fi-kbl-soraka: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271> / > i915#2190 <https://gitlab.freedesktop.org/drm/intel/issues/2190>) > * > > igt@gem_lmem_swapping@basic: > > o > > fi-apl-guc: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-apl-guc/igt@gem_lmem_swapping@basic.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271> / > i915#4613 > <https://gitlab.freedesktop.org/drm/intel/issues/4613>) +3 > similar issues > > o > > fi-kbl-soraka: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271> / > i915#4613 > <https://gitlab.freedesktop.org/drm/intel/issues/4613>) +3 > similar issues > > * > > igt@i915_selftest@live@gt_pm: > > o fi-kbl-soraka: NOTRUN -> DMESG-FAIL > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html> > (i915#1886 <https://gitlab.freedesktop.org/drm/intel/issues/1886>) > * > > igt@i915_selftest@live@hangcheck: > > o > > fi-hsw-g3258: PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html> > -> INCOMPLETE > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html> > (i915#4785 <https://gitlab.freedesktop.org/drm/intel/issues/4785>) > > o > > fi-bdw-5557u: NOTRUN -> INCOMPLETE > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-bdw-5557u/igt@i915_selftest@live@hangcheck.html> > (i915#3921 <https://gitlab.freedesktop.org/drm/intel/issues/3921>) > > o > > bat-dg1-6: NOTRUN -> DMESG-FAIL > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/bat-dg1-6/igt@i915_selftest@live@hangcheck.html> > (i915#4494 > <https://gitlab.freedesktop.org/drm/intel/issues/4494> / > i915#4957 <https://gitlab.freedesktop.org/drm/intel/issues/4957>) > > * > > igt@i915_suspend@basic-s2idle-without-i915: > > o bat-dg1-6: NOTRUN -> INCOMPLETE > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/bat-dg1-6/igt@i915_suspend@basic-s2idle-without-i915.html> > (i915#6011 <https://gitlab.freedesktop.org/drm/intel/issues/6011>) > * > > igt@kms_chamelium@common-hpd-after-suspend: > > o > > fi-hsw-4770: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271> / > fdo#111827 <https://bugs.freedesktop.org/show_bug.cgi?id=111827>) > > o > > fi-snb-2600: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-snb-2600/igt@kms_chamelium@common-hpd-after-suspend.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271> / > fdo#111827 <https://bugs.freedesktop.org/show_bug.cgi?id=111827>) > > * > > igt@kms_chamelium@dp-crc-fast: > > o fi-bdw-5557u: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-bdw-5557u/igt@kms_chamelium@dp-crc-fast.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271> / > fdo#111827 > <https://bugs.freedesktop.org/show_bug.cgi?id=111827>) +7 > similar issues > * > > igt@kms_chamelium@hdmi-crc-fast: > > o fi-apl-guc: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-apl-guc/igt@kms_chamelium@hdmi-crc-fast.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271> / > fdo#111827 > <https://bugs.freedesktop.org/show_bug.cgi?id=111827>) +8 > similar issues > * > > igt@kms_chamelium@hdmi-hpd-fast: > > o fi-kbl-soraka: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-kbl-soraka/igt@kms_chamelium@hdmi-hpd-fast.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271> / > fdo#111827 > <https://bugs.freedesktop.org/show_bug.cgi?id=111827>) +7 > similar issues > * > > igt@kms_flip@basic-plain-flip@a-edp1: > > o fi-tgl-u2: PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/fi-tgl-u2/igt@kms_flip@basic-plain-flip@a-edp1.html> > -> DMESG-WARN > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-tgl-u2/igt@kms_flip@basic-plain-flip@a-edp1.html> > (i915#402 > <https://gitlab.freedesktop.org/drm/intel/issues/402>) +1 > similar issue > * > > igt@kms_force_connector_basic@force-connector-state: > > o fi-apl-guc: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-apl-guc/igt@kms_force_connector_basic@force-connector-state.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271>) +13 > similar issues > * > > igt@kms_psr@primary_page_flip: > > o fi-bdw-5557u: NOTRUN -> SKIP > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-bdw-5557u/igt@kms_psr@primary_page_flip.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271>) +13 > similar issues > * > > igt@runner@aborted: > > o > > fi-skl-guc: NOTRUN -> FAIL > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-skl-guc/igt@runner@aborted.html> > (i915#4312 <https://gitlab.freedesktop.org/drm/intel/issues/4312>) > > o > > fi-hsw-g3258: NOTRUN -> FAIL > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-hsw-g3258/igt@runner@aborted.html> > (fdo#109271 > <https://bugs.freedesktop.org/show_bug.cgi?id=109271> / > i915#4312 > <https://gitlab.freedesktop.org/drm/intel/issues/4312> / > i915#6246 <https://gitlab.freedesktop.org/drm/intel/issues/6246>) > > > Possible fixes > > * > > igt@debugfs_test@read_all_entries: > > o fi-apl-guc: DMESG-WARN > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/fi-apl-guc/igt@debugfs_test@read_all_entries.html> > (i915#1610 > <https://gitlab.freedesktop.org/drm/intel/issues/1610> / > i915#5595 > <https://gitlab.freedesktop.org/drm/intel/issues/5595>) -> > PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-apl-guc/igt@debugfs_test@read_all_entries.html> > * > > igt@i915_selftest@live@gt_engines: > > o bat-dg1-6: INCOMPLETE > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/bat-dg1-6/igt@i915_selftest@live@gt_engines.html> > (i915#4418 > <https://gitlab.freedesktop.org/drm/intel/issues/4418>) -> > PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/bat-dg1-6/igt@i915_selftest@live@gt_engines.html> > * > > igt@i915_selftest@live@hangcheck: > > o > > fi-hsw-4770: INCOMPLETE > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html> > (i915#3303 > <https://gitlab.freedesktop.org/drm/intel/issues/3303> / > i915#4785 > <https://gitlab.freedesktop.org/drm/intel/issues/4785>) -> > PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html> > > o > > fi-snb-2600: INCOMPLETE > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/fi-snb-2600/igt@i915_selftest@live@hangcheck.html> > (i915#3921 > <https://gitlab.freedesktop.org/drm/intel/issues/3921>) -> > PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-snb-2600/igt@i915_selftest@live@hangcheck.html> > > * > > igt@kms_busy@basic@modeset: > > o {bat-adln-1}: DMESG-WARN > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/bat-adln-1/igt@kms_busy@basic@modeset.html> > (i915#3576 > <https://gitlab.freedesktop.org/drm/intel/issues/3576>) -> > PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/bat-adln-1/igt@kms_busy@basic@modeset.html> > * > > igt@kms_flip@basic-flip-vs-dpms@a-edp1: > > o fi-tgl-u2: DMESG-WARN > <https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11803/fi-tgl-u2/igt@kms_flip@basic-flip-vs-dpms@a-edp1.html> > (i915#402 > <https://gitlab.freedesktop.org/drm/intel/issues/402>) -> PASS > <https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105291v2/fi-tgl-u2/igt@kms_flip@basic-flip-vs-dpms@a-edp1.html> > > {name}: This element is suppressed. This means it is ignored when > computing > the status of the difference (SUCCESS, WARNING, or FAILURE). > > > Build changes > > * Linux: CI_DRM_11803 -> Patchwork_105291v2 > > CI-20190529: 20190529 > CI_DRM_11803: 23f2707225ca8a6c43526d1d7c46a7dd1a5f02cf @ > git://anongit.freedesktop.org/gfx-ci/linux > IGT_6541: 02153f109bd422d93cfce7f5aa9d7b0e22fab13c @ > https://gitlab.freedesktop.org/drm/igt-gpu-tools.git > Patchwork_105291v2: 23f2707225ca8a6c43526d1d7c46a7dd1a5f02cf @ > git://anongit.freedesktop.org/gfx-ci/linux > > > Linux commits > > 731487d4ddf7 drm/i915: Fix a lockdep warning at error capture >
On 6/24/2022 2:46 PM, Andrzej Hajda wrote: > On 24.06.2022 13:08, Nirmoy Das wrote: >> For some platfroms we use stop_machine version of >> gen8_ggtt_insert_page/gen8_ggtt_insert_entries to avoid a >> concurrent GGTT access bug but this causes a circular locking >> dependency warning: >> >> Possible unsafe locking scenario: >> CPU0 CPU1 >> ---- ---- >> lock(&ggtt->error_mutex); >> lock(dma_fence_map); >> lock(&ggtt->error_mutex); >> lock(cpu_hotplug_lock); >> >> Fix this by calling gen8_ggtt_insert_page/gen8_ggtt_insert_entries >> directly at error capture which is concurrent GGTT access safe because >> reset path make sure of that. >> >> v2: Fix rebase conflict and added a comment. >> >> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5595 >> Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> >> --- >> drivers/gpu/drm/i915/gt/intel_ggtt.c | 10 ++++++++++ >> drivers/gpu/drm/i915/gt/intel_gtt.h | 9 +++++++++ >> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 ++++- >> drivers/gpu/drm/i915/i915_gpu_error.c | 8 ++++++-- >> 4 files changed, 29 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c >> b/drivers/gpu/drm/i915/gt/intel_ggtt.c >> index ffff96180313..15a915bb4088 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c >> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c >> @@ -960,6 +960,16 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) >> if (intel_vm_no_concurrent_access_wa(i915)) { >> ggtt->vm.insert_entries = bxt_vtd_ggtt_insert_entries__BKL; >> ggtt->vm.insert_page = bxt_vtd_ggtt_insert_page__BKL; >> + >> + /* >> + * Calling stop_machine() version of GGTT update function >> + * at error capture/reset path will raise lockdep warning. >> + * Allow calling gen8_ggtt_insert_* directly at reset path >> + * which is safe from parallel GGTT updates. >> + */ >> + ggtt->vm.raw_insert_page = gen8_ggtt_insert_page; >> + ggtt->vm.raw_insert_entries = gen8_ggtt_insert_entries; >> + >> ggtt->vm.bind_async_flags = >> I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND; >> } >> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h >> b/drivers/gpu/drm/i915/gt/intel_gtt.h >> index 29fd3a9e8b2e..e639434e97fd 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h >> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h >> @@ -306,6 +306,15 @@ struct i915_address_space { >> struct i915_vma_resource *vma_res, >> enum i915_cache_level cache_level, >> u32 flags); >> + void (*raw_insert_page)(struct i915_address_space *vm, >> + dma_addr_t addr, >> + u64 offset, >> + enum i915_cache_level cache_level, >> + u32 flags); >> + void (*raw_insert_entries)(struct i915_address_space *vm, >> + struct i915_vma_resource *vma_res, >> + enum i915_cache_level cache_level, >> + u32 flags); > > > I would expect rather extra flag to insert_(page|entries) instead of > extra callbacks. Anyway both should work. I was about to do that but then realized that those flags are closely related to PTE attributes so I think it makes sense to keep it that way. > > Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Thanks Andrzej. > > Regards > Andrzej > > >> void (*cleanup)(struct i915_address_space *vm); >> void (*foreach)(struct i915_address_space *vm, >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> index d2c5c9367cc4..c06e83872c34 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> @@ -493,7 +493,10 @@ static void uc_fw_bind_ggtt(struct intel_uc_fw >> *uc_fw) >> if (i915_gem_object_is_lmem(obj)) >> pte_flags |= PTE_LM; >> - ggtt->vm.insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, >> pte_flags); >> + if (ggtt->vm.raw_insert_entries) >> + ggtt->vm.raw_insert_entries(&ggtt->vm, dummy, >> I915_CACHE_NONE, pte_flags); >> + else >> + ggtt->vm.insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, >> pte_flags); >> } >> static void uc_fw_unbind_ggtt(struct intel_uc_fw *uc_fw) >> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c >> b/drivers/gpu/drm/i915/i915_gpu_error.c >> index bff8a111424a..f9b1969ed7ed 100644 >> --- a/drivers/gpu/drm/i915/i915_gpu_error.c >> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c >> @@ -1104,8 +1104,12 @@ i915_vma_coredump_create(const struct intel_gt >> *gt, >> for_each_sgt_daddr(dma, iter, vma_res->bi.pages) { >> mutex_lock(&ggtt->error_mutex); >> - ggtt->vm.insert_page(&ggtt->vm, dma, slot, >> - I915_CACHE_NONE, 0); >> + if (ggtt->vm.raw_insert_page) >> + ggtt->vm.raw_insert_page(&ggtt->vm, dma, slot, >> + I915_CACHE_NONE, 0); >> + else >> + ggtt->vm.insert_page(&ggtt->vm, dma, slot, >> + I915_CACHE_NONE, 0); >> mb(); >> s = io_mapping_map_wc(&ggtt->iomap, slot, PAGE_SIZE); >
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index ffff96180313..15a915bb4088 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -960,6 +960,16 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) if (intel_vm_no_concurrent_access_wa(i915)) { ggtt->vm.insert_entries = bxt_vtd_ggtt_insert_entries__BKL; ggtt->vm.insert_page = bxt_vtd_ggtt_insert_page__BKL; + + /* + * Calling stop_machine() version of GGTT update function + * at error capture/reset path will raise lockdep warning. + * Allow calling gen8_ggtt_insert_* directly at reset path + * which is safe from parallel GGTT updates. + */ + ggtt->vm.raw_insert_page = gen8_ggtt_insert_page; + ggtt->vm.raw_insert_entries = gen8_ggtt_insert_entries; + ggtt->vm.bind_async_flags = I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND; } diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h index 29fd3a9e8b2e..e639434e97fd 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.h +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h @@ -306,6 +306,15 @@ struct i915_address_space { struct i915_vma_resource *vma_res, enum i915_cache_level cache_level, u32 flags); + void (*raw_insert_page)(struct i915_address_space *vm, + dma_addr_t addr, + u64 offset, + enum i915_cache_level cache_level, + u32 flags); + void (*raw_insert_entries)(struct i915_address_space *vm, + struct i915_vma_resource *vma_res, + enum i915_cache_level cache_level, + u32 flags); void (*cleanup)(struct i915_address_space *vm); void (*foreach)(struct i915_address_space *vm, diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c index d2c5c9367cc4..c06e83872c34 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c @@ -493,7 +493,10 @@ static void uc_fw_bind_ggtt(struct intel_uc_fw *uc_fw) if (i915_gem_object_is_lmem(obj)) pte_flags |= PTE_LM; - ggtt->vm.insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, pte_flags); + if (ggtt->vm.raw_insert_entries) + ggtt->vm.raw_insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, pte_flags); + else + ggtt->vm.insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, pte_flags); } static void uc_fw_unbind_ggtt(struct intel_uc_fw *uc_fw) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index bff8a111424a..f9b1969ed7ed 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1104,8 +1104,12 @@ i915_vma_coredump_create(const struct intel_gt *gt, for_each_sgt_daddr(dma, iter, vma_res->bi.pages) { mutex_lock(&ggtt->error_mutex); - ggtt->vm.insert_page(&ggtt->vm, dma, slot, - I915_CACHE_NONE, 0); + if (ggtt->vm.raw_insert_page) + ggtt->vm.raw_insert_page(&ggtt->vm, dma, slot, + I915_CACHE_NONE, 0); + else + ggtt->vm.insert_page(&ggtt->vm, dma, slot, + I915_CACHE_NONE, 0); mb(); s = io_mapping_map_wc(&ggtt->iomap, slot, PAGE_SIZE);
For some platfroms we use stop_machine version of gen8_ggtt_insert_page/gen8_ggtt_insert_entries to avoid a concurrent GGTT access bug but this causes a circular locking dependency warning: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&ggtt->error_mutex); lock(dma_fence_map); lock(&ggtt->error_mutex); lock(cpu_hotplug_lock); Fix this by calling gen8_ggtt_insert_page/gen8_ggtt_insert_entries directly at error capture which is concurrent GGTT access safe because reset path make sure of that. v2: Fix rebase conflict and added a comment. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5595 Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com> --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 10 ++++++++++ drivers/gpu/drm/i915/gt/intel_gtt.h | 9 +++++++++ drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 5 ++++- drivers/gpu/drm/i915/i915_gpu_error.c | 8 ++++++-- 4 files changed, 29 insertions(+), 3 deletions(-)