From: Tvrtko Ursulin tvrtko.ursulin@intel.com
With both integrated and discrete Intel GPUs in a system, the current global check of intel_iommu_gfx_mapped, as done from intel_vtd_active() may not be completely accurate.
In this patch we add i915 parameter to intel_vtd_active() in order to prepare it for multiple GPUs and we also change the check away from Intel specific intel_iommu_gfx_mapped (global exported by the Intel IOMMU driver) to probing the presence of IOMMU domain on a specific device using iommu_get_domain_for_dev().
It was suggested to additionally check for __IOMMU_DOMAIN_PAGING bit present in the returned iommu domain, however I opted not to do that at this point.
Checking for this flag would detect whether IOMMU is in address translation mode, with the assumption that is the only relevant question. Downside to that is that in identity mapping (pass-through) mode IOMMU hardware is still active, sitting on the communication path, just not doing address translation.
My rationale was, that for the many intel_vtd_active() checks in our code base, while some clearly are about performance impact of address translation, some may be about working around functional issues when the IOMMU hardware is simply being active. There also may be some performance impact in pass-through mode, but I have not specifically attempted to measure it.
Therefore the safest option feels to be to keep intel_vtd_active() answering the question of "is the IOMMU hardware active" for this device. If in the future we want to expand the set of questions to "is IOMMU active and doing address translation" we can easily do that by adding a new helper to be called from appropriate sites.
v2: * Check for dmar translation specifically, not just iommu domain. (Baolu)
v3: * Go back to plain "any domain" check for now, rewrite commit message.
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: Lu Baolu baolu.lu@linux.intel.com Cc: Lucas De Marchi lucas.demarchi@intel.com Cc: Robin Murphy robin.murphy@arm.com --- drivers/gpu/drm/i915/display/intel_bw.c | 2 +- drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 2 +- drivers/gpu/drm/i915/gem/i915_gemfs.c | 2 +- drivers/gpu/drm/i915/gt/intel_ggtt.c | 4 ++-- drivers/gpu/drm/i915/i915_debugfs.c | 1 + drivers/gpu/drm/i915/i915_driver.c | 7 +++++++ drivers/gpu/drm/i915/i915_drv.h | 13 +++++++------ drivers/gpu/drm/i915/i915_gpu_error.c | 5 +---- drivers/gpu/drm/i915/intel_device_info.c | 14 +------------- drivers/gpu/drm/i915/intel_pm.c | 2 +- 12 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index abec394f6869..2da4aacc956b 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -634,7 +634,7 @@ static unsigned int intel_bw_data_rate(struct drm_i915_private *dev_priv, for_each_pipe(dev_priv, pipe) data_rate += bw_state->data_rate[pipe];
- if (DISPLAY_VER(dev_priv) >= 13 && intel_vtd_active()) + if (DISPLAY_VER(dev_priv) >= 13 && intel_vtd_active(dev_priv)) data_rate = data_rate * 105 / 100;
return data_rate; diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b2d51cd79d6c..1ef77ba7f645 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1293,7 +1293,7 @@ static bool needs_async_flip_vtd_wa(const struct intel_crtc_state *crtc_state) { struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
- return crtc_state->uapi.async_flip && intel_vtd_active() && + return crtc_state->uapi.async_flip && intel_vtd_active(i915) && (DISPLAY_VER(i915) == 9 || IS_BROADWELL(i915) || IS_HASWELL(i915)); }
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index d0c34bc3af6c..614e8697c068 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1677,7 +1677,7 @@ static int intel_sanitize_fbc_option(struct drm_i915_private *i915) static bool need_fbc_vtd_wa(struct drm_i915_private *i915) { /* WaFbcTurnOffFbcWhenHyperVisorIsUsed:skl,bxt */ - if (intel_vtd_active() && + if (intel_vtd_active(i915) && (IS_SKYLAKE(i915) || IS_BROXTON(i915))) { drm_info(&i915->drm, "Disabling framebuffer compression (FBC) to prevent screen flicker with VT-d enabled\n"); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index ddd37ccb1362..cf100c0ea3b7 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -399,7 +399,7 @@ static int i915_gem_init_stolen(struct intel_memory_region *mem) return 0; }
- if (intel_vtd_active() && GRAPHICS_VER(i915) < 8) { + if (intel_vtd_active(i915) && GRAPHICS_VER(i915) < 8) { drm_notice(&i915->drm, "%s, disabling use of stolen memory\n", "DMAR active"); diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c index dbdbdc344d87..11cd66d183e6 100644 --- a/drivers/gpu/drm/i915/gem/i915_gemfs.c +++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c @@ -31,7 +31,7 @@ int i915_gemfs_init(struct drm_i915_private *i915) */
opts = NULL; - if (intel_vtd_active()) { + if (intel_vtd_active(i915)) { if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) { static char huge_opt[] = "huge=within_size"; /* r/w */
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 555111c3bee5..110d3944f9a2 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -106,7 +106,7 @@ static bool needs_idle_maps(struct drm_i915_private *i915) * Query intel_iommu to see if we need the workaround. Presumably that * was loaded first. */ - if (!intel_vtd_active()) + if (!intel_vtd_active(i915)) return false;
if (GRAPHICS_VER(i915) == 5 && IS_MOBILE(i915)) @@ -1233,7 +1233,7 @@ int i915_ggtt_probe_hw(struct drm_i915_private *i915) if (ret) return ret;
- if (intel_vtd_active()) + if (intel_vtd_active(i915)) drm_info(&i915->drm, "VT-d active for gfx access\n");
return 0; diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index fe638b5da7c0..390d541f64ea 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -65,6 +65,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
intel_device_info_print_static(INTEL_INFO(i915), &p); intel_device_info_print_runtime(RUNTIME_INFO(i915), &p); + i915_print_iommu_status(i915, &p); intel_gt_info_print(&i915->gt.info, &p); intel_driver_caps_print(&i915->caps, &p);
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index a13666627dad..bbc99fc5888f 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -741,6 +741,12 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv) i915_gem_driver_unregister(dev_priv); }
+void +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p) +{ + drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915))); +} + static void i915_welcome_messages(struct drm_i915_private *dev_priv) { if (drm_debug_enabled(DRM_UT_DRIVER)) { @@ -756,6 +762,7 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv)
intel_device_info_print_static(INTEL_INFO(dev_priv), &p); intel_device_info_print_runtime(RUNTIME_INFO(dev_priv), &p); + i915_print_iommu_status(dev_priv, &p); intel_gt_info_print(&dev_priv->gt.info, &p); }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1bfadd9127fc..da3b32ae65e0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1760,26 +1760,27 @@ static inline bool run_as_guest(void) #define HAS_D12_PLANE_MINIMIZATION(dev_priv) (IS_ROCKETLAKE(dev_priv) || \ IS_ALDERLAKE_S(dev_priv))
-static inline bool intel_vtd_active(void) +static inline bool intel_vtd_active(struct drm_i915_private *i915) { -#ifdef CONFIG_INTEL_IOMMU - if (intel_iommu_gfx_mapped) + if (iommu_get_domain_for_dev(i915->drm.dev)) return true; -#endif
/* Running as a guest, we assume the host is enforcing VT'd */ return run_as_guest(); }
+void +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p); + static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv) { - return GRAPHICS_VER(dev_priv) >= 6 && intel_vtd_active(); + return GRAPHICS_VER(dev_priv) >= 6 && intel_vtd_active(dev_priv); }
static inline bool intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915) { - return IS_BROXTON(i915) && intel_vtd_active(); + return IS_BROXTON(i915) && intel_vtd_active(i915); }
static inline bool diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 2a2d7643b551..3f39d6641894 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1750,10 +1750,7 @@ static void capture_gen(struct i915_gpu_coredump *error) error->wakelock = atomic_read(&i915->runtime_pm.wakeref_count); error->suspended = i915->runtime_pm.suspended;
- error->iommu = -1; -#ifdef CONFIG_INTEL_IOMMU - error->iommu = intel_iommu_gfx_mapped; -#endif + error->iommu = intel_vtd_active(i915); error->reset_count = i915_reset_count(&i915->gpu_error); error->suspend_count = i915->suspend_count;
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 6e6b317bc33c..e6605b5181a5 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -83,17 +83,6 @@ const char *intel_platform_name(enum intel_platform platform) return platform_names[platform]; }
-static const char *iommu_name(void) -{ - const char *msg = "n/a"; - -#ifdef CONFIG_INTEL_IOMMU - msg = enableddisabled(intel_iommu_gfx_mapped); -#endif - - return msg; -} - void intel_device_info_print_static(const struct intel_device_info *info, struct drm_printer *p) { @@ -114,7 +103,6 @@ void intel_device_info_print_static(const struct intel_device_info *info, drm_printf(p, "display version: %u\n", info->display.ver);
drm_printf(p, "gt: %d\n", info->gt); - drm_printf(p, "iommu: %s\n", iommu_name()); drm_printf(p, "memory-regions: %x\n", info->memory_regions); drm_printf(p, "page-sizes: %x\n", info->page_sizes); drm_printf(p, "platform: %s\n", intel_platform_name(info->platform)); @@ -374,7 +362,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) info->display.has_dsc = 0; }
- if (GRAPHICS_VER(dev_priv) == 6 && intel_vtd_active()) { + if (GRAPHICS_VER(dev_priv) == 6 && intel_vtd_active(dev_priv)) { drm_info(&dev_priv->drm, "Disabling ppGTT for VT-d support\n"); info->ppgtt_type = INTEL_PPGTT_NONE; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 01fa3fac1b57..cff0f32bedc9 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -98,7 +98,7 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) * "Plane N strech max must be programmed to 11b (x1) * when Async flips are enabled on that plane." */ - if (!IS_GEMINILAKE(dev_priv) && intel_vtd_active()) + if (!IS_GEMINILAKE(dev_priv) && intel_vtd_active(dev_priv)) intel_uncore_rmw(&dev_priv->uncore, CHICKEN_PIPESL_1(pipe), SKL_PLANE1_STRETCH_MAX_MASK, SKL_PLANE1_STRETCH_MAX_X1); }
On 2021-11-25 10:42, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin tvrtko.ursulin@intel.com
With both integrated and discrete Intel GPUs in a system, the current global check of intel_iommu_gfx_mapped, as done from intel_vtd_active() may not be completely accurate.
In this patch we add i915 parameter to intel_vtd_active() in order to prepare it for multiple GPUs and we also change the check away from Intel specific intel_iommu_gfx_mapped (global exported by the Intel IOMMU driver) to probing the presence of IOMMU domain on a specific device using iommu_get_domain_for_dev().
FWIW the way you have it now is functionally equivalent to using device_iommu_mapped(), which I think might be slightly clearer for the current intent, but I don't have a significantly strong preference (after all, this *was* the de-facto way of checking before device_iommu_mapped() was introduced, and there are still other examples of it around). So from the IOMMU perspective,
Acked-by: Robin Murphy robin.murphy@arm.com
Perhaps the AGP driver could also be tweaked and intel_iommu_gfx_mapped cleaned away entirely, but I'll leave that for Baolu to think about :)
Cheers, Robin.
It was suggested to additionally check for __IOMMU_DOMAIN_PAGING bit present in the returned iommu domain, however I opted not to do that at this point.
Checking for this flag would detect whether IOMMU is in address translation mode, with the assumption that is the only relevant question. Downside to that is that in identity mapping (pass-through) mode IOMMU hardware is still active, sitting on the communication path, just not doing address translation.
My rationale was, that for the many intel_vtd_active() checks in our code base, while some clearly are about performance impact of address translation, some may be about working around functional issues when the IOMMU hardware is simply being active. There also may be some performance impact in pass-through mode, but I have not specifically attempted to measure it.
Therefore the safest option feels to be to keep intel_vtd_active() answering the question of "is the IOMMU hardware active" for this device. If in the future we want to expand the set of questions to "is IOMMU active and doing address translation" we can easily do that by adding a new helper to be called from appropriate sites.
v2:
- Check for dmar translation specifically, not just iommu domain. (Baolu)
v3:
- Go back to plain "any domain" check for now, rewrite commit message.
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: Lu Baolu baolu.lu@linux.intel.com Cc: Lucas De Marchi lucas.demarchi@intel.com Cc: Robin Murphy robin.murphy@arm.com
drivers/gpu/drm/i915/display/intel_bw.c | 2 +- drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 2 +- drivers/gpu/drm/i915/gem/i915_gemfs.c | 2 +- drivers/gpu/drm/i915/gt/intel_ggtt.c | 4 ++-- drivers/gpu/drm/i915/i915_debugfs.c | 1 + drivers/gpu/drm/i915/i915_driver.c | 7 +++++++ drivers/gpu/drm/i915/i915_drv.h | 13 +++++++------ drivers/gpu/drm/i915/i915_gpu_error.c | 5 +---- drivers/gpu/drm/i915/intel_device_info.c | 14 +------------- drivers/gpu/drm/i915/intel_pm.c | 2 +- 12 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index abec394f6869..2da4aacc956b 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -634,7 +634,7 @@ static unsigned int intel_bw_data_rate(struct drm_i915_private *dev_priv, for_each_pipe(dev_priv, pipe) data_rate += bw_state->data_rate[pipe];
- if (DISPLAY_VER(dev_priv) >= 13 && intel_vtd_active())
if (DISPLAY_VER(dev_priv) >= 13 && intel_vtd_active(dev_priv)) data_rate = data_rate * 105 / 100;
return data_rate;
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b2d51cd79d6c..1ef77ba7f645 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1293,7 +1293,7 @@ static bool needs_async_flip_vtd_wa(const struct intel_crtc_state *crtc_state) { struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
- return crtc_state->uapi.async_flip && intel_vtd_active() &&
- return crtc_state->uapi.async_flip && intel_vtd_active(i915) && (DISPLAY_VER(i915) == 9 || IS_BROADWELL(i915) || IS_HASWELL(i915)); }
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index d0c34bc3af6c..614e8697c068 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1677,7 +1677,7 @@ static int intel_sanitize_fbc_option(struct drm_i915_private *i915) static bool need_fbc_vtd_wa(struct drm_i915_private *i915) { /* WaFbcTurnOffFbcWhenHyperVisorIsUsed:skl,bxt */
- if (intel_vtd_active() &&
- if (intel_vtd_active(i915) && (IS_SKYLAKE(i915) || IS_BROXTON(i915))) { drm_info(&i915->drm, "Disabling framebuffer compression (FBC) to prevent screen flicker with VT-d enabled\n");
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index ddd37ccb1362..cf100c0ea3b7 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -399,7 +399,7 @@ static int i915_gem_init_stolen(struct intel_memory_region *mem) return 0; }
- if (intel_vtd_active() && GRAPHICS_VER(i915) < 8) {
- if (intel_vtd_active(i915) && GRAPHICS_VER(i915) < 8) { drm_notice(&i915->drm, "%s, disabling use of stolen memory\n", "DMAR active");
diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c index dbdbdc344d87..11cd66d183e6 100644 --- a/drivers/gpu/drm/i915/gem/i915_gemfs.c +++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c @@ -31,7 +31,7 @@ int i915_gemfs_init(struct drm_i915_private *i915) */
opts = NULL;
- if (intel_vtd_active()) {
- if (intel_vtd_active(i915)) { if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) { static char huge_opt[] = "huge=within_size"; /* r/w */
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 555111c3bee5..110d3944f9a2 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -106,7 +106,7 @@ static bool needs_idle_maps(struct drm_i915_private *i915) * Query intel_iommu to see if we need the workaround. Presumably that * was loaded first. */
- if (!intel_vtd_active())
if (!intel_vtd_active(i915)) return false;
if (GRAPHICS_VER(i915) == 5 && IS_MOBILE(i915))
@@ -1233,7 +1233,7 @@ int i915_ggtt_probe_hw(struct drm_i915_private *i915) if (ret) return ret;
- if (intel_vtd_active())
if (intel_vtd_active(i915)) drm_info(&i915->drm, "VT-d active for gfx access\n");
return 0;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index fe638b5da7c0..390d541f64ea 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -65,6 +65,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
intel_device_info_print_static(INTEL_INFO(i915), &p); intel_device_info_print_runtime(RUNTIME_INFO(i915), &p);
- i915_print_iommu_status(i915, &p); intel_gt_info_print(&i915->gt.info, &p); intel_driver_caps_print(&i915->caps, &p);
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index a13666627dad..bbc99fc5888f 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -741,6 +741,12 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv) i915_gem_driver_unregister(dev_priv); }
+void +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p) +{
- drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
+}
- static void i915_welcome_messages(struct drm_i915_private *dev_priv) { if (drm_debug_enabled(DRM_UT_DRIVER)) {
@@ -756,6 +762,7 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv)
intel_device_info_print_static(INTEL_INFO(dev_priv), &p); intel_device_info_print_runtime(RUNTIME_INFO(dev_priv), &p);
i915_print_iommu_status(dev_priv, &p);diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1bfadd9127fc..da3b32ae65e0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1760,26 +1760,27 @@ static inline bool run_as_guest(void) #define HAS_D12_PLANE_MINIMIZATION(dev_priv) (IS_ROCKETLAKE(dev_priv) || \ IS_ALDERLAKE_S(dev_priv))
-static inline bool intel_vtd_active(void) +static inline bool intel_vtd_active(struct drm_i915_private *i915) { -#ifdef CONFIG_INTEL_IOMMU
- if (intel_iommu_gfx_mapped)
- if (iommu_get_domain_for_dev(i915->drm.dev)) return true;
-#endif
/* Running as a guest, we assume the host is enforcing VT'd */ return run_as_guest(); }
+void +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
- static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv) {
- return GRAPHICS_VER(dev_priv) >= 6 && intel_vtd_active();
return GRAPHICS_VER(dev_priv) >= 6 && intel_vtd_active(dev_priv); }
static inline bool intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915) {
- return IS_BROXTON(i915) && intel_vtd_active();
return IS_BROXTON(i915) && intel_vtd_active(i915); }
static inline bool
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 2a2d7643b551..3f39d6641894 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1750,10 +1750,7 @@ static void capture_gen(struct i915_gpu_coredump *error) error->wakelock = atomic_read(&i915->runtime_pm.wakeref_count); error->suspended = i915->runtime_pm.suspended;
- error->iommu = -1;
-#ifdef CONFIG_INTEL_IOMMU
- error->iommu = intel_iommu_gfx_mapped;
-#endif
- error->iommu = intel_vtd_active(i915); error->reset_count = i915_reset_count(&i915->gpu_error); error->suspend_count = i915->suspend_count;
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 6e6b317bc33c..e6605b5181a5 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -83,17 +83,6 @@ const char *intel_platform_name(enum intel_platform platform) return platform_names[platform]; }
-static const char *iommu_name(void) -{
- const char *msg = "n/a";
-#ifdef CONFIG_INTEL_IOMMU
- msg = enableddisabled(intel_iommu_gfx_mapped);
-#endif
- return msg;
-}
- void intel_device_info_print_static(const struct intel_device_info *info, struct drm_printer *p) {
@@ -114,7 +103,6 @@ void intel_device_info_print_static(const struct intel_device_info *info, drm_printf(p, "display version: %u\n", info->display.ver);
drm_printf(p, "gt: %d\n", info->gt);
- drm_printf(p, "iommu: %s\n", iommu_name()); drm_printf(p, "memory-regions: %x\n", info->memory_regions); drm_printf(p, "page-sizes: %x\n", info->page_sizes); drm_printf(p, "platform: %s\n", intel_platform_name(info->platform));
@@ -374,7 +362,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) info->display.has_dsc = 0; }
- if (GRAPHICS_VER(dev_priv) == 6 && intel_vtd_active()) {
- if (GRAPHICS_VER(dev_priv) == 6 && intel_vtd_active(dev_priv)) { drm_info(&dev_priv->drm, "Disabling ppGTT for VT-d support\n"); info->ppgtt_type = INTEL_PPGTT_NONE;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 01fa3fac1b57..cff0f32bedc9 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -98,7 +98,7 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) * "Plane N strech max must be programmed to 11b (x1) * when Async flips are enabled on that plane." */
if (!IS_GEMINILAKE(dev_priv) && intel_vtd_active())
if (!IS_GEMINILAKE(dev_priv) && intel_vtd_active(dev_priv)) intel_uncore_rmw(&dev_priv->uncore, CHICKEN_PIPESL_1(pipe), SKL_PLANE1_STRETCH_MAX_MASK, SKL_PLANE1_STRETCH_MAX_X1);
On 2021/11/25 19:47, Robin Murphy wrote:
On 2021-11-25 10:42, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin tvrtko.ursulin@intel.com
With both integrated and discrete Intel GPUs in a system, the current global check of intel_iommu_gfx_mapped, as done from intel_vtd_active() may not be completely accurate.
In this patch we add i915 parameter to intel_vtd_active() in order to prepare it for multiple GPUs and we also change the check away from Intel specific intel_iommu_gfx_mapped (global exported by the Intel IOMMU driver) to probing the presence of IOMMU domain on a specific device using iommu_get_domain_for_dev().
FWIW the way you have it now is functionally equivalent to using device_iommu_mapped(), which I think might be slightly clearer for the current intent, but I don't have a significantly strong preference (after all, this *was* the de-facto way of checking before device_iommu_mapped() was introduced, and there are still other examples of it around). So from the IOMMU perspective,
Acked-by: Robin Murphy robin.murphy@arm.com
Perhaps the AGP driver could also be tweaked and intel_iommu_gfx_mapped cleaned away entirely, but I'll leave that for Baolu to think about :)
I fully agreed with Robin.
I prefer device_iommu_mapped() more than iommu_get_domain_for_dev().
"iommu_get_domain_for_dev(dev) == NULL" simply means that the device does not have any domain attached. Although at present, it is equivalent to device DMAing without IOMMU translation. But I'm sure it will change in the future.
With device_iommu_mapped() replaced,
Reviewed-by: Lu Baolu baolu.lu@linux.intel.com
Best regards, baolu
On 26/11/2021 08:26, Lu Baolu wrote:
On 2021/11/25 19:47, Robin Murphy wrote:
On 2021-11-25 10:42, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin tvrtko.ursulin@intel.com
With both integrated and discrete Intel GPUs in a system, the current global check of intel_iommu_gfx_mapped, as done from intel_vtd_active() may not be completely accurate.
In this patch we add i915 parameter to intel_vtd_active() in order to prepare it for multiple GPUs and we also change the check away from Intel specific intel_iommu_gfx_mapped (global exported by the Intel IOMMU driver) to probing the presence of IOMMU domain on a specific device using iommu_get_domain_for_dev().
FWIW the way you have it now is functionally equivalent to using device_iommu_mapped(), which I think might be slightly clearer for the current intent, but I don't have a significantly strong preference (after all, this *was* the de-facto way of checking before device_iommu_mapped() was introduced, and there are still other examples of it around). So from the IOMMU perspective,
Acked-by: Robin Murphy robin.murphy@arm.com
Perhaps the AGP driver could also be tweaked and intel_iommu_gfx_mapped cleaned away entirely, but I'll leave that for Baolu to think about :)
I fully agreed with Robin.
I prefer device_iommu_mapped() more than iommu_get_domain_for_dev().
"iommu_get_domain_for_dev(dev) == NULL" simply means that the device does not have any domain attached. Although at present, it is equivalent to device DMAing without IOMMU translation. But I'm sure it will change in the future.
With device_iommu_mapped() replaced,
Reviewed-by: Lu Baolu baolu.lu@linux.intel.com
I have empirically established device_iommu_mapped() matches 1:1 with intel_iommu_gfx_mapped and a simple non-NULL check against iommu_get_domain_for_dev(), so okay, I will respin with that change.
We leave the problem of being able to distinguish between pass-through and address translation from the drivers for later. Thanks both!
Regards,
Tvrtko
dri-devel@lists.freedesktop.org
-
Lu Baolu -
Robin Murphy -
Tvrtko Ursulin