[PATCH v2 1/5] drm/i915: Release GT resources on driver remove

Janusz Krzysztofik janusz.krzysztofik at linux.intel.com
Sun Jun 7 16:50:33 UTC 2020


GT scratch page is now released and its DMA mappings revoked on driver
release.  If a device is removed while its file descriptor is still
open, the driver is not released until last device file descriptor
closure.  In that case intel-iommu code may judge late DMA unmapping as
a bug and kernel panic may occur.

Since DMA mapped address space may be no longer usable after device
removal, release GT resources including scratch page as well as a
reference to its address space on driver remove.  Implement that by
just calling intel_gt_driver_release() on GT remove.  Since that step
can be then dropped from driver release path, an issue of the helper
being potentially called twice on driver probe error is also resolved
as a side effect.

v2: Fix intel_gt_driver_release() called twice issue (Michał).

<4> [39.201062] ------------[ cut here ]------------
<2> [39.201074] kernel BUG at drivers/iommu/intel-iommu.c:3717!
<4> [39.201154] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
<4> [39.201162] CPU: 6 PID: 7 Comm: kworker/u16:0 Tainted: G     U  W         5.7.0-rc5-CI-CI_DRM_8485+ #1
<4> [39.201172] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP, BIOS ICLSFWR1.R00.3175.A00.1904261428 04/26/2019
<4> [39.201243] Workqueue: i915 __i915_gem_free_work [i915]
<4> [39.201252] RIP: 0010:intel_unmap+0x1f5/0x230
<4> [39.201260] Code: 01 e8 9f bc a9 ff 85 c0 74 09 80 3d df 60 09 01 00 74 19 65 ff 0d 13 12 97 7e 0f 85 fc fe ff ff e8 82 b0 95 ff e9 f2 fe ff ff <0f> 0b e8 d4 bd a9 ff 85 c0 75 de 48 c7 c2 10 84 2c 82 be 54 00 00
<4> [39.201278] RSP: 0018:ffffc900000dbc98 EFLAGS: 00010246
<4> [39.201285] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffea0021d30000
<4> [39.201293] RDX: 000000000005f000 RSI: 00000000fed00000 RDI: ffff888889eec000
<4> [39.201301] RBP: ffff888889eec0b0 R08: 0000000000000000 R09: 00000000fffffffe
<4> [39.201309] R10: 00000000458139fc R11: 00000000f6c6d8b2 R12: 0000000000000025
<4> [39.201318] R13: 00000000fed00000 R14: 000000000005f000 R15: 0000000000000025
<4> [39.201326] FS:  0000000000000000(0000) GS:ffff888890100000(0000) knlGS:0000000000000000
<4> [39.201335] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4> [39.201342] CR2: 0000560f1308e148 CR3: 0000000881972002 CR4: 0000000000760ee0
<4> [39.201350] PKRU: 55555554
<4> [39.201355] Call Trace:
<4> [39.201361]  intel_unmap_sg+0x7b/0x180
<4> [39.201412]  shmem_put_pages+0x43/0x250 [i915]
<4> [39.201472]  ? __i915_gem_object_unset_pages.part.12+0x11b/0x1d0 [i915]
<4> [39.201531]  ? __i915_gem_object_unset_pages.part.12+0x133/0x1d0 [i915]
<4> [39.201590]  __i915_gem_object_put_pages+0x81/0xc0 [i915]
<4> [39.201646]  __i915_gem_free_objects.isra.21+0x1a7/0x4b0 [i915]
<4> [39.201658]  process_one_work+0x268/0x600
<4> [39.201666]  ? __schedule+0x307/0x8d0
<4> [39.201675]  worker_thread+0x1d0/0x380
<4> [39.201682]  ? process_one_work+0x600/0x600
<4> [39.201689]  kthread+0x140/0x160
<4> [39.201695]  ? kthread_park+0x80/0x80
<4> [39.201703]  ret_from_fork+0x24/0x50
<4> [39.201712] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 mei_hdcp x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul snd_hda_intel snd_intel_dspcfg snd_hda_codec e1000e ax88179_178a usbnet snd_hwdep mii snd_hda_core ghash_clmulni_intel snd_pcm ptp pps_core mei_me mei intel_lpss_pci prime_numbers
<4> [39.201764] ---[ end trace f3ec1bae3de04509 ]---

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski at intel.com> # v1
Cc: Michał Winiarski <michal.winiarski at intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt.c | 38 ++++++++++++++++--------------
 drivers/gpu/drm/i915/gt/intel_gt.h |  1 -
 drivers/gpu/drm/i915/i915_gem.c    |  2 --
 3 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index ebc29b6ee86c..5829a5b61318 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -592,6 +592,24 @@ int intel_gt_init(struct intel_gt *gt)
 	return err;
 }
 
+static void intel_gt_driver_release(struct intel_gt *gt)
+{
+	struct i915_address_space *vm;
+	intel_wakeref_t wakeref;
+
+	/* Scrub all HW state upon release */
+	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+		__intel_gt_reset(gt, ALL_ENGINES);
+
+	vm = fetch_and_zero(&gt->vm);
+	if (!GEM_WARN_ON(!vm))
+		i915_vm_put(vm);
+
+	intel_gt_pm_fini(gt);
+	intel_gt_fini_scratch(gt);
+	intel_gt_fini_buffer_pool(gt);
+}
+
 void intel_gt_driver_remove(struct intel_gt *gt)
 {
 	__intel_gt_disable(gt);
@@ -599,6 +617,8 @@ void intel_gt_driver_remove(struct intel_gt *gt)
 	intel_uc_driver_remove(&gt->uc);
 
 	intel_engines_release(gt);
+
+	intel_gt_driver_release(gt);
 }
 
 void intel_gt_driver_unregister(struct intel_gt *gt)
@@ -613,24 +633,6 @@ void intel_gt_driver_unregister(struct intel_gt *gt)
 	intel_gt_set_wedged(gt);
 }
 
-void intel_gt_driver_release(struct intel_gt *gt)
-{
-	struct i915_address_space *vm;
-	intel_wakeref_t wakeref;
-
-	/* Scrub all HW state upon release */
-	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
-		__intel_gt_reset(gt, ALL_ENGINES);
-
-	vm = fetch_and_zero(&gt->vm);
-	if (vm) /* FIXME being called twice on error paths :( */
-		i915_vm_put(vm);
-
-	intel_gt_pm_fini(gt);
-	intel_gt_fini_scratch(gt);
-	intel_gt_fini_buffer_pool(gt);
-}
-
 void intel_gt_driver_late_release(struct intel_gt *gt)
 {
 	/* We need to wait for inflight RCU frees to release their grip */
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h
index 4fac043750aa..9d6a88fffcba 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt.h
@@ -41,7 +41,6 @@ void intel_gt_driver_register(struct intel_gt *gt);
 
 void intel_gt_driver_unregister(struct intel_gt *gt);
 void intel_gt_driver_remove(struct intel_gt *gt);
-void intel_gt_driver_release(struct intel_gt *gt);
 
 void intel_gt_driver_late_release(struct intel_gt *gt);
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 41553e9e57a9..6453f3036743 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1242,8 +1242,6 @@ void i915_gem_driver_release(struct drm_i915_private *dev_priv)
 {
 	i915_gem_driver_release__contexts(dev_priv);
 
-	intel_gt_driver_release(&dev_priv->gt);
-
 	intel_wa_list_free(&dev_priv->gt_wa_list);
 
 	intel_uc_cleanup_firmwares(&dev_priv->gt.uc);
-- 
2.21.1



More information about the Intel-gfx-trybot mailing list