* [PATCH] drm/i915: Use per device iommu check
@ 2021-11-09 12:17 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-09 12:17 UTC (permalink / raw)
To: Intel-gfx; +Cc: Lu Baolu, dri-devel, Tvrtko Ursulin
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
and probe presence of iommu domain per device to accurately reflect its
status.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Lu Baolu <baolu.lu@linux.intel.com>
---
Baolu, is my understanding here correct? Maybe I am confused by both
intel_iommu_gfx_mapped and dmar_map_gfx being globals in the intel_iommu
driver. But it certainly appears the setup can assign some iommu ops (and
assign the discrete i915 to iommu group) when those two are set to off.
---
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_drv.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 8d9d888e9316..a4d8088e4f71 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -490,7 +490,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 29392dfc46c8..80d206f3e9da 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1315,7 +1315,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 834eb4cc7c10..0ccbfc9e4101 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1539,7 +1539,7 @@ static int intel_sanitize_fbc_option(struct drm_i915_private *dev_priv)
static bool need_fbc_vtd_wa(struct drm_i915_private *dev_priv)
{
/* WaFbcTurnOffFbcWhenHyperVisorIsUsed:skl,bxt */
- if (intel_vtd_active() &&
+ if (intel_vtd_active(dev_priv) &&
(IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))) {
drm_info(&dev_priv->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 1fb4a03d7ac3..ddb4a9d039d4 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -104,7 +104,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))
@@ -1231,7 +1231,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_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f184b44d05f2..8efd0ad5bef0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -733,6 +733,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)) {
@@ -748,6 +754,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 e967cd08f23e..9fb38a54f1fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1763,26 +1763,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 aa2b3aad9643..7672927ed10b 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 59adf0ce6719..f5b567f87c24 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);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-09 12:17 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-09 12:17 UTC (permalink / raw)
To: Intel-gfx; +Cc: Lu Baolu, dri-devel
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
and probe presence of iommu domain per device to accurately reflect its
status.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Lu Baolu <baolu.lu@linux.intel.com>
---
Baolu, is my understanding here correct? Maybe I am confused by both
intel_iommu_gfx_mapped and dmar_map_gfx being globals in the intel_iommu
driver. But it certainly appears the setup can assign some iommu ops (and
assign the discrete i915 to iommu group) when those two are set to off.
---
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_drv.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 8d9d888e9316..a4d8088e4f71 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -490,7 +490,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 29392dfc46c8..80d206f3e9da 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1315,7 +1315,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 834eb4cc7c10..0ccbfc9e4101 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1539,7 +1539,7 @@ static int intel_sanitize_fbc_option(struct drm_i915_private *dev_priv)
static bool need_fbc_vtd_wa(struct drm_i915_private *dev_priv)
{
/* WaFbcTurnOffFbcWhenHyperVisorIsUsed:skl,bxt */
- if (intel_vtd_active() &&
+ if (intel_vtd_active(dev_priv) &&
(IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))) {
drm_info(&dev_priv->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 1fb4a03d7ac3..ddb4a9d039d4 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -104,7 +104,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))
@@ -1231,7 +1231,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_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f184b44d05f2..8efd0ad5bef0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -733,6 +733,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)) {
@@ -748,6 +754,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 e967cd08f23e..9fb38a54f1fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1763,26 +1763,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 aa2b3aad9643..7672927ed10b 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 59adf0ce6719..f5b567f87c24 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);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Use per device iommu check
2021-11-09 12:17 ` [Intel-gfx] " Tvrtko Ursulin
(?)
@ 2021-11-09 14:02 ` Patchwork
-1 siblings, 0 replies; 39+ messages in thread
From: Patchwork @ 2021-11-09 14:02 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 3938 bytes --]
== Series Details ==
Series: drm/i915: Use per device iommu check
URL : https://patchwork.freedesktop.org/series/96715/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_10857 -> Patchwork_21546
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/index.html
Participating hosts (35 -> 34)
------------------------------
Additional (2): fi-tgl-u2 fi-pnv-d510
Missing (3): fi-ctg-p8600 bat-dg1-6 bat-adlp-4
Known issues
------------
Here are the changes found in Patchwork_21546 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s0:
- fi-tgl-1115g4: [PASS][1] -> [FAIL][2] ([i915#1888])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s0.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s0.html
* igt@gem_huc_copy@huc-copy:
- fi-tgl-u2: NOTRUN -> [SKIP][3] ([i915#2190])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/fi-tgl-u2/igt@gem_huc_copy@huc-copy.html
* igt@i915_selftest@live@hangcheck:
- fi-snb-2600: [PASS][4] -> [INCOMPLETE][5] ([i915#3921])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
* igt@kms_chamelium@dp-hpd-fast:
- fi-tgl-u2: NOTRUN -> [SKIP][6] ([fdo#109284] / [fdo#111827]) +8 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/fi-tgl-u2/igt@kms_chamelium@dp-hpd-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-tgl-u2: NOTRUN -> [SKIP][7] ([i915#4103]) +1 similar issue
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/fi-tgl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
* igt@kms_force_connector_basic@force-load-detect:
- fi-tgl-u2: NOTRUN -> [SKIP][8] ([fdo#109285])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/fi-tgl-u2/igt@kms_force_connector_basic@force-load-detect.html
* igt@prime_vgem@basic-userptr:
- fi-pnv-d510: NOTRUN -> [SKIP][9] ([fdo#109271]) +53 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/fi-pnv-d510/igt@prime_vgem@basic-userptr.html
- fi-tgl-u2: NOTRUN -> [SKIP][10] ([i915#3301])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/fi-tgl-u2/igt@prime_vgem@basic-userptr.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
[i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
Build changes
-------------
* Linux: CI_DRM_10857 -> Patchwork_21546
CI-20190529: 20190529
CI_DRM_10857: 2f005a829cd05b317c5b497a6941b88d981d22e6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6275: 6d172a5cf51ffff5f2780e2837860d613db5067f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_21546: 4fa0c25ec337523add2859b0cedb0ec78c1831f2 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
4fa0c25ec337 drm/i915: Use per device iommu check
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/index.html
[-- Attachment #2: Type: text/html, Size: 4728 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Use per device iommu check
2021-11-09 12:17 ` [Intel-gfx] " Tvrtko Ursulin
(?)
(?)
@ 2021-11-09 16:23 ` Patchwork
-1 siblings, 0 replies; 39+ messages in thread
From: Patchwork @ 2021-11-09 16:23 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 30259 bytes --]
== Series Details ==
Series: drm/i915: Use per device iommu check
URL : https://patchwork.freedesktop.org/series/96715/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_10857_full -> Patchwork_21546_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Known issues
------------
Here are the changes found in Patchwork_21546_full that come from known issues:
### CI changes ###
#### Issues hit ####
* boot:
- shard-snb: ([PASS][1], [PASS][2], [PASS][3], [PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25]) -> ([PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [FAIL][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50]) ([i915#4338])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb7/boot.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb7/boot.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb7/boot.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb7/boot.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb7/boot.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb6/boot.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb6/boot.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb6/boot.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb6/boot.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb6/boot.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb5/boot.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb5/boot.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb5/boot.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb5/boot.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb5/boot.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb4/boot.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb4/boot.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb4/boot.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb4/boot.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb4/boot.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb2/boot.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb2/boot.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb2/boot.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb2/boot.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb2/boot.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb7/boot.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb7/boot.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb7/boot.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb7/boot.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb7/boot.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb6/boot.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb6/boot.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb6/boot.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb6/boot.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb6/boot.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb5/boot.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb5/boot.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb5/boot.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb5/boot.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb5/boot.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb4/boot.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb4/boot.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb4/boot.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb4/boot.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb4/boot.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb2/boot.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb2/boot.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb2/boot.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb2/boot.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb2/boot.html
### IGT changes ###
#### Issues hit ####
* igt@feature_discovery@psr2:
- shard-iclb: [PASS][51] -> [SKIP][52] ([i915#658])
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-iclb2/igt@feature_discovery@psr2.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb7/igt@feature_discovery@psr2.html
* igt@gem_create@create-massive:
- shard-skl: NOTRUN -> [DMESG-WARN][53] ([i915#3002])
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl3/igt@gem_create@create-massive.html
* igt@gem_ctx_isolation@preservation-s3@bcs0:
- shard-tglb: [PASS][54] -> [INCOMPLETE][55] ([i915#456]) +1 similar issue
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-tglb1/igt@gem_ctx_isolation@preservation-s3@bcs0.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb7/igt@gem_ctx_isolation@preservation-s3@bcs0.html
* igt@gem_eio@in-flight-contexts-10ms:
- shard-tglb: [PASS][56] -> [TIMEOUT][57] ([i915#3063])
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-tglb7/igt@gem_eio@in-flight-contexts-10ms.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb3/igt@gem_eio@in-flight-contexts-10ms.html
* igt@gem_exec_fair@basic-deadline:
- shard-skl: NOTRUN -> [FAIL][58] ([i915#2846])
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl3/igt@gem_exec_fair@basic-deadline.html
* igt@gem_exec_fair@basic-none@vecs0:
- shard-apl: [PASS][59] -> [FAIL][60] ([i915#2842] / [i915#3468])
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-apl3/igt@gem_exec_fair@basic-none@vecs0.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl8/igt@gem_exec_fair@basic-none@vecs0.html
* igt@gem_exec_fair@basic-pace@bcs0:
- shard-tglb: [PASS][61] -> [FAIL][62] ([i915#2842]) +1 similar issue
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-tglb6/igt@gem_exec_fair@basic-pace@bcs0.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb8/igt@gem_exec_fair@basic-pace@bcs0.html
* igt@gem_exec_fair@basic-pace@vecs0:
- shard-kbl: [PASS][63] -> [FAIL][64] ([i915#2842])
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-kbl6/igt@gem_exec_fair@basic-pace@vecs0.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl3/igt@gem_exec_fair@basic-pace@vecs0.html
- shard-iclb: [PASS][65] -> [FAIL][66] ([i915#2842])
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-iclb4/igt@gem_exec_fair@basic-pace@vecs0.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb5/igt@gem_exec_fair@basic-pace@vecs0.html
* igt@gem_pxp@regular-baseline-src-copy-readible:
- shard-tglb: NOTRUN -> [SKIP][67] ([i915#4270]) +2 similar issues
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb3/igt@gem_pxp@regular-baseline-src-copy-readible.html
* igt@gem_render_copy@y-tiled-mc-ccs-to-vebox-y-tiled:
- shard-iclb: NOTRUN -> [SKIP][68] ([i915#768])
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@gem_render_copy@y-tiled-mc-ccs-to-vebox-y-tiled.html
* igt@gem_userptr_blits@dmabuf-sync:
- shard-iclb: NOTRUN -> [SKIP][69] ([i915#3323])
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@gem_userptr_blits@dmabuf-sync.html
* igt@gem_userptr_blits@input-checking:
- shard-kbl: NOTRUN -> [DMESG-WARN][70] ([i915#3002])
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl2/igt@gem_userptr_blits@input-checking.html
* igt@gen9_exec_parse@allowed-all:
- shard-tglb: NOTRUN -> [SKIP][71] ([i915#2856])
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb6/igt@gen9_exec_parse@allowed-all.html
* igt@gen9_exec_parse@bb-start-param:
- shard-iclb: NOTRUN -> [SKIP][72] ([i915#2856])
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@gen9_exec_parse@bb-start-param.html
* igt@i915_pm_dc@dc6-dpms:
- shard-iclb: [PASS][73] -> [FAIL][74] ([i915#454])
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-iclb4/igt@i915_pm_dc@dc6-dpms.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb3/igt@i915_pm_dc@dc6-dpms.html
* igt@i915_pm_rpm@modeset-lpsp-stress-no-wait:
- shard-snb: NOTRUN -> [SKIP][75] ([fdo#109271]) +51 similar issues
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb5/igt@i915_pm_rpm@modeset-lpsp-stress-no-wait.html
* igt@i915_pm_rpm@pc8-residency:
- shard-iclb: NOTRUN -> [SKIP][76] ([fdo#109293] / [fdo#109506])
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@i915_pm_rpm@pc8-residency.html
* igt@kms_async_flips@crc:
- shard-skl: NOTRUN -> [FAIL][77] ([i915#4272])
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl3/igt@kms_async_flips@crc.html
* igt@kms_big_fb@x-tiled-32bpp-rotate-180:
- shard-glk: [PASS][78] -> [DMESG-WARN][79] ([i915#118])
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-glk4/igt@kms_big_fb@x-tiled-32bpp-rotate-180.html
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-glk1/igt@kms_big_fb@x-tiled-32bpp-rotate-180.html
* igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip:
- shard-apl: NOTRUN -> [SKIP][80] ([fdo#109271] / [i915#3777])
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl1/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip.html
* igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-hflip:
- shard-skl: NOTRUN -> [SKIP][81] ([fdo#109271] / [i915#3777]) +1 similar issue
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl7/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-hflip.html
* igt@kms_big_fb@yf-tiled-32bpp-rotate-0:
- shard-tglb: NOTRUN -> [SKIP][82] ([fdo#111615])
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb3/igt@kms_big_fb@yf-tiled-32bpp-rotate-0.html
* igt@kms_big_joiner@invalid-modeset:
- shard-iclb: NOTRUN -> [SKIP][83] ([i915#2705])
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@kms_big_joiner@invalid-modeset.html
* igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc:
- shard-skl: NOTRUN -> [SKIP][84] ([fdo#109271] / [i915#3886]) +2 similar issues
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl7/igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc.html
* igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc:
- shard-iclb: NOTRUN -> [SKIP][85] ([fdo#109278] / [i915#3886])
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html
* igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc:
- shard-kbl: NOTRUN -> [SKIP][86] ([fdo#109271] / [i915#3886]) +6 similar issues
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl3/igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc.html
- shard-apl: NOTRUN -> [SKIP][87] ([fdo#109271] / [i915#3886]) +4 similar issues
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl2/igt@kms_ccs@pipe-b-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc.html
* igt@kms_chamelium@dp-crc-multiple:
- shard-skl: NOTRUN -> [SKIP][88] ([fdo#109271] / [fdo#111827]) +6 similar issues
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl7/igt@kms_chamelium@dp-crc-multiple.html
* igt@kms_chamelium@dp-frame-dump:
- shard-tglb: NOTRUN -> [SKIP][89] ([fdo#109284] / [fdo#111827])
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb8/igt@kms_chamelium@dp-frame-dump.html
* igt@kms_chamelium@dp-hpd-storm-disable:
- shard-apl: NOTRUN -> [SKIP][90] ([fdo#109271] / [fdo#111827]) +7 similar issues
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl1/igt@kms_chamelium@dp-hpd-storm-disable.html
* igt@kms_chamelium@hdmi-hpd-after-suspend:
- shard-iclb: NOTRUN -> [SKIP][91] ([fdo#109284] / [fdo#111827]) +2 similar issues
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@kms_chamelium@hdmi-hpd-after-suspend.html
* igt@kms_chamelium@hdmi-mode-timings:
- shard-snb: NOTRUN -> [SKIP][92] ([fdo#109271] / [fdo#111827]) +1 similar issue
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb5/igt@kms_chamelium@hdmi-mode-timings.html
* igt@kms_chamelium@vga-hpd-without-ddc:
- shard-kbl: NOTRUN -> [SKIP][93] ([fdo#109271] / [fdo#111827]) +12 similar issues
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl3/igt@kms_chamelium@vga-hpd-without-ddc.html
* igt@kms_color@pipe-c-ctm-blue-to-red:
- shard-skl: [PASS][94] -> [DMESG-WARN][95] ([i915#1982]) +1 similar issue
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-skl7/igt@kms_color@pipe-c-ctm-blue-to-red.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl6/igt@kms_color@pipe-c-ctm-blue-to-red.html
* igt@kms_content_protection@atomic-dpms:
- shard-apl: NOTRUN -> [TIMEOUT][96] ([i915#1319])
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl1/igt@kms_content_protection@atomic-dpms.html
* igt@kms_cursor_crc@pipe-a-cursor-512x512-sliding:
- shard-iclb: NOTRUN -> [SKIP][97] ([fdo#109278] / [fdo#109279])
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@kms_cursor_crc@pipe-a-cursor-512x512-sliding.html
* igt@kms_cursor_crc@pipe-c-cursor-512x512-random:
- shard-tglb: NOTRUN -> [SKIP][98] ([fdo#109279] / [i915#3359])
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb8/igt@kms_cursor_crc@pipe-c-cursor-512x512-random.html
* igt@kms_cursor_legacy@cursorb-vs-flipb-atomic:
- shard-iclb: NOTRUN -> [SKIP][99] ([fdo#109274] / [fdo#109278])
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@kms_cursor_legacy@cursorb-vs-flipb-atomic.html
* igt@kms_cursor_legacy@pipe-d-torture-bo:
- shard-apl: NOTRUN -> [SKIP][100] ([fdo#109271] / [i915#533]) +1 similar issue
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl1/igt@kms_cursor_legacy@pipe-d-torture-bo.html
* igt@kms_fbcon_fbt@fbc-suspend:
- shard-kbl: [PASS][101] -> [INCOMPLETE][102] ([i915#180] / [i915#636])
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-kbl4/igt@kms_fbcon_fbt@fbc-suspend.html
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl6/igt@kms_fbcon_fbt@fbc-suspend.html
* igt@kms_fbcon_fbt@psr-suspend:
- shard-tglb: [PASS][103] -> [INCOMPLETE][104] ([i915#2411] / [i915#456])
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-tglb3/igt@kms_fbcon_fbt@psr-suspend.html
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb7/igt@kms_fbcon_fbt@psr-suspend.html
* igt@kms_flip@2x-flip-vs-panning:
- shard-apl: NOTRUN -> [SKIP][105] ([fdo#109271]) +62 similar issues
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl1/igt@kms_flip@2x-flip-vs-panning.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1:
- shard-glk: [PASS][106] -> [FAIL][107] ([i915#79])
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-glk4/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1.html
[107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-glk7/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1.html
* igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1:
- shard-glk: [PASS][108] -> [FAIL][109] ([i915#2122])
[108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-glk2/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html
[109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-glk1/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html
* igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
- shard-kbl: [PASS][110] -> [DMESG-WARN][111] ([i915#180]) +9 similar issues
[110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
[111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl6/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
- shard-apl: [PASS][112] -> [DMESG-WARN][113] ([i915#180]) +2 similar issues
[112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-apl8/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
[113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
* igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1:
- shard-skl: [PASS][114] -> [FAIL][115] ([i915#2122])
[114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-skl7/igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1.html
[115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl6/igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1.html
* igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-gtt:
- shard-iclb: NOTRUN -> [SKIP][116] ([fdo#109280]) +9 similar issues
[116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-gtt.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-shrfb-fliptrack-mmap-gtt:
- shard-skl: NOTRUN -> [SKIP][117] ([fdo#109271]) +54 similar issues
[117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl3/igt@kms_frontbuffer_tracking@fbcpsr-1p-shrfb-fliptrack-mmap-gtt.html
* igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-move:
- shard-tglb: NOTRUN -> [SKIP][118] ([fdo#111825]) +4 similar issues
[118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-move.html
* igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc:
- shard-kbl: NOTRUN -> [SKIP][119] ([fdo#109271]) +122 similar issues
[119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl2/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc.html
* igt@kms_pipe_crc_basic@hang-read-crc-pipe-d:
- shard-kbl: NOTRUN -> [SKIP][120] ([fdo#109271] / [i915#533])
[120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl7/igt@kms_pipe_crc_basic@hang-read-crc-pipe-d.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
- shard-kbl: [PASS][121] -> [INCOMPLETE][122] ([i915#3614])
[121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-kbl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
[122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
* igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
- shard-kbl: NOTRUN -> [FAIL][123] ([fdo#108145] / [i915#265])
[123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl3/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
* igt@kms_plane_lowres@pipe-b-tiling-x:
- shard-tglb: NOTRUN -> [SKIP][124] ([i915#3536])
[124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb8/igt@kms_plane_lowres@pipe-b-tiling-x.html
* igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-3:
- shard-apl: NOTRUN -> [SKIP][125] ([fdo#109271] / [i915#658])
[125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl1/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-3.html
* igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-1:
- shard-kbl: NOTRUN -> [SKIP][126] ([fdo#109271] / [i915#658]) +3 similar issues
[126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl7/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-1.html
* igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-5:
- shard-tglb: NOTRUN -> [SKIP][127] ([i915#2920])
[127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb8/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-5.html
* igt@kms_psr2_su@frontbuffer:
- shard-iclb: [PASS][128] -> [SKIP][129] ([fdo#109642] / [fdo#111068] / [i915#658])
[128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
[129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb7/igt@kms_psr2_su@frontbuffer.html
* igt@kms_psr@psr2_basic:
- shard-iclb: [PASS][130] -> [SKIP][131] ([fdo#109441]) +2 similar issues
[130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-iclb2/igt@kms_psr@psr2_basic.html
[131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb6/igt@kms_psr@psr2_basic.html
* igt@kms_psr@psr2_sprite_plane_onoff:
- shard-tglb: NOTRUN -> [FAIL][132] ([i915#132] / [i915#3467])
[132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb6/igt@kms_psr@psr2_sprite_plane_onoff.html
* igt@kms_vblank@pipe-d-ts-continuation-modeset-rpm:
- shard-iclb: NOTRUN -> [SKIP][133] ([fdo#109278]) +9 similar issues
[133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@kms_vblank@pipe-d-ts-continuation-modeset-rpm.html
* igt@kms_writeback@writeback-check-output:
- shard-skl: NOTRUN -> [SKIP][134] ([fdo#109271] / [i915#2437])
[134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl3/igt@kms_writeback@writeback-check-output.html
* igt@kms_writeback@writeback-invalid-parameters:
- shard-kbl: NOTRUN -> [SKIP][135] ([fdo#109271] / [i915#2437])
[135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl3/igt@kms_writeback@writeback-invalid-parameters.html
- shard-apl: NOTRUN -> [SKIP][136] ([fdo#109271] / [i915#2437])
[136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl2/igt@kms_writeback@writeback-invalid-parameters.html
* igt@prime_nv_pcopy@test_semaphore:
- shard-tglb: NOTRUN -> [SKIP][137] ([fdo#109291])
[137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb8/igt@prime_nv_pcopy@test_semaphore.html
* igt@prime_nv_test@nv_write_i915_cpu_mmap_read:
- shard-iclb: NOTRUN -> [SKIP][138] ([fdo#109291])
[138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@prime_nv_test@nv_write_i915_cpu_mmap_read.html
* igt@prime_vgem@coherency-gtt:
- shard-iclb: NOTRUN -> [SKIP][139] ([fdo#109292])
[139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@prime_vgem@coherency-gtt.html
* igt@sysfs_clients@busy:
- shard-kbl: NOTRUN -> [SKIP][140] ([fdo#109271] / [i915#2994])
[140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl3/igt@sysfs_clients@busy.html
* igt@sysfs_clients@create:
- shard-skl: NOTRUN -> [SKIP][141] ([fdo#109271] / [i915#2994])
[141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-skl3/igt@sysfs_clients@create.html
* igt@sysfs_clients@recycle:
- shard-tglb: NOTRUN -> [SKIP][142] ([i915#2994])
[142]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb3/igt@sysfs_clients@recycle.html
* igt@sysfs_clients@sema-25:
- shard-apl: NOTRUN -> [SKIP][143] ([fdo#109271] / [i915#2994])
[143]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl1/igt@sysfs_clients@sema-25.html
#### Possible fixes ####
* igt@gem_exec_fair@basic-none-share@rcs0:
- shard-iclb: [FAIL][144] ([i915#2842]) -> [PASS][145]
[144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-iclb3/igt@gem_exec_fair@basic-none-share@rcs0.html
[145]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb7/igt@gem_exec_fair@basic-none-share@rcs0.html
- shard-apl: [SKIP][146] ([fdo#109271]) -> [PASS][147]
[146]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-apl7/igt@gem_exec_fair@basic-none-share@rcs0.html
[147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl6/igt@gem_exec_fair@basic-none-share@rcs0.html
* igt@gem_exec_fair@basic-none@vcs0:
- shard-kbl: [FAIL][148] ([i915#2842]) -> [PASS][149] +2 similar issues
[148]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-kbl6/igt@gem_exec_fair@basic-none@vcs0.html
[149]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-kbl3/igt@gem_exec_fair@basic-none@vcs0.html
- shard-apl: [FAIL][150] ([i915#2842]) -> [PASS][151]
[150]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-apl3/igt@gem_exec_fair@basic-none@vcs0.html
[151]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-apl8/igt@gem_exec_fair@basic-none@vcs0.html
* igt@gem_exec_fair@basic-pace-share@rcs0:
- shard-glk: [FAIL][152] ([i915#2842]) -> [PASS][153] +1 similar issue
[152]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-glk6/igt@gem_exec_fair@basic-pace-share@rcs0.html
[153]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-glk5/igt@gem_exec_fair@basic-pace-share@rcs0.html
* igt@gem_exec_fair@basic-pace-solo@rcs0:
- shard-tglb: [FAIL][154] ([i915#2842]) -> [PASS][155] +1 similar issue
[154]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-tglb1/igt@gem_exec_fair@basic-pace-solo@rcs0.html
[155]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb7/igt@gem_exec_fair@basic-pace-solo@rcs0.html
* igt@gem_exec_reloc@basic-scanout@vecs0:
- {shard-rkl}: [SKIP][156] ([i915#3639]) -> [PASS][157] +3 similar issues
[156]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-rkl-1/igt@gem_exec_reloc@basic-scanout@vecs0.html
[157]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-rkl-6/igt@gem_exec_reloc@basic-scanout@vecs0.html
* igt@gem_exec_whisper@basic-contexts-priority:
- shard-iclb: [INCOMPLETE][158] ([i915#1895]) -> [PASS][159]
[158]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-iclb1/igt@gem_exec_whisper@basic-contexts-priority.html
[159]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-iclb8/igt@gem_exec_whisper@basic-contexts-priority.html
* igt@gem_exec_whisper@basic-queues-forked-all:
- shard-glk: [DMESG-WARN][160] ([i915#118]) -> [PASS][161] +1 similar issue
[160]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-glk2/igt@gem_exec_whisper@basic-queues-forked-all.html
[161]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-glk2/igt@gem_exec_whisper@basic-queues-forked-all.html
* igt@i915_pm_backlight@fade_with_suspend:
- shard-tglb: [INCOMPLETE][162] ([i915#456]) -> [PASS][163]
[162]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-tglb7/igt@i915_pm_backlight@fade_with_suspend.html
[163]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-tglb8/igt@i915_pm_backlight@fade_with_suspend.html
* igt@i915_pm_rpm@basic-pci-d3-state:
- {shard-rkl}: [SKIP][164] ([fdo#109308]) -> [PASS][165] +1 similar issue
[164]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-rkl-1/igt@i915_pm_rpm@basic-pci-d3-state.html
[165]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-rkl-6/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_selftest@live@hangcheck:
- shard-snb: [INCOMPLETE][166] ([i915#3921]) -> [PASS][167]
[166]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10857/shard-snb6/igt@i915_selftest@live@hangcheck.html
[167]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/shard-snb5/igt@i915_selftest
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21546/index.html
[-- Attachment #2: Type: text/html, Size: 33599 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
2021-11-09 12:17 ` [Intel-gfx] " Tvrtko Ursulin
` (2 preceding siblings ...)
(?)
@ 2021-11-09 17:19 ` Lucas De Marchi
2021-11-09 17:35 ` Tvrtko Ursulin
-1 siblings, 1 reply; 39+ messages in thread
From: Lucas De Marchi @ 2021-11-09 17:19 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx, dri-devel, Lu Baolu
On Tue, Nov 09, 2021 at 12:17:59PM +0000, Tvrtko Ursulin wrote:
>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
>On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>and probe presence of iommu domain per device to accurately reflect its
>status.
nice, I was just starting to look into thus but for another reason: we
are adding support for other archs, like aarch64, and the global from here
was a problem
should we change drivers/gpu/drm/i915/Kconfig.debug to stop selecting
CONFIG_INTEL_IOMMU and CONFIG_INTEL_IOMMU_DEFAULT_ON?
thanks
Lucas De Marchi
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
2021-11-09 17:19 ` [Intel-gfx] [PATCH] " Lucas De Marchi
@ 2021-11-09 17:35 ` Tvrtko Ursulin
2021-11-10 7:25 ` Lu Baolu
0 siblings, 1 reply; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-09 17:35 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: Intel-gfx, dri-devel, Lu Baolu
On 09/11/2021 17:19, Lucas De Marchi wrote:
> On Tue, Nov 09, 2021 at 12:17:59PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>> and probe presence of iommu domain per device to accurately reflect its
>> status.
>
> nice, I was just starting to look into thus but for another reason: we
> are adding support for other archs, like aarch64, and the global from here
> was a problem
Yes I realized the other iommu angle as well. To do this properly we
need to sort the intel_vtd_active call sites into at least two buckets -
which are truly about VT-d and which are just IOMMU.
For instance the THP decision in i915_gemfs.co would be "are we behind
any iommu". Some other call sites are possibly only about the bugs in
the igfx iommu. Not sure if there is a third bucket for any potential
differences between igfx iommu and other Intel iommu in case of dgfx.
I'd like to hear from Baolu as well to confirm if intel_iommu driver is
handling igfx + dgfx correctly in respect to the two global variables I
mention in the commit message.
> should we change drivers/gpu/drm/i915/Kconfig.debug to stop selecting
> CONFIG_INTEL_IOMMU and CONFIG_INTEL_IOMMU_DEFAULT_ON?
Don't know. For debug it is useful since it can catch more issues but
whether or not kconfig can be improved to select the right one for the
platform? I guess select X if X86, select Y if Z?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
2021-11-09 12:17 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-11-10 7:12 ` Lu Baolu
-1 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-10 7:12 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: dri-devel, baolu.lu
Hi Tvrtko,
On 2021/11/9 20:17, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>
> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
> and probe presence of iommu domain per device to accurately reflect its
> status.
>
> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
> Cc: Lu Baolu<baolu.lu@linux.intel.com>
> ---
> Baolu, is my understanding here correct? Maybe I am confused by both
> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the intel_iommu
> driver. But it certainly appears the setup can assign some iommu ops (and
> assign the discrete i915 to iommu group) when those two are set to off.
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index e967cd08f23e..9fb38a54f1fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1763,26 +1763,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();
}
Have you verified this change? I am afraid that
iommu_get_domain_for_dev() always gets a valid iommu domain even
intel_iommu_gfx_mapped == 0.
A possible way could look like this:
static bool intel_vtd_active(struct drm_i915_private *i915)
{
struct iommu_domain *domain;
domain = iommu_get_domain_for_dev(i915->drm.dev);
if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
return true;
... ...
}
Actually I don't like this either since it checks the domain->type out
of the iommu subsystem. We could refactor this later by export an iommu
interface for this check.
Best regards,
baolu
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
@ 2021-11-10 7:12 ` Lu Baolu
0 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-10 7:12 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: Tvrtko Ursulin, dri-devel, baolu.lu
Hi Tvrtko,
On 2021/11/9 20:17, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>
> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
> and probe presence of iommu domain per device to accurately reflect its
> status.
>
> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
> Cc: Lu Baolu<baolu.lu@linux.intel.com>
> ---
> Baolu, is my understanding here correct? Maybe I am confused by both
> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the intel_iommu
> driver. But it certainly appears the setup can assign some iommu ops (and
> assign the discrete i915 to iommu group) when those two are set to off.
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index e967cd08f23e..9fb38a54f1fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1763,26 +1763,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();
}
Have you verified this change? I am afraid that
iommu_get_domain_for_dev() always gets a valid iommu domain even
intel_iommu_gfx_mapped == 0.
A possible way could look like this:
static bool intel_vtd_active(struct drm_i915_private *i915)
{
struct iommu_domain *domain;
domain = iommu_get_domain_for_dev(i915->drm.dev);
if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
return true;
... ...
}
Actually I don't like this either since it checks the domain->type out
of the iommu subsystem. We could refactor this later by export an iommu
interface for this check.
Best regards,
baolu
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
2021-11-09 17:35 ` Tvrtko Ursulin
@ 2021-11-10 7:25 ` Lu Baolu
2021-11-10 9:35 ` Tvrtko Ursulin
0 siblings, 1 reply; 39+ messages in thread
From: Lu Baolu @ 2021-11-10 7:25 UTC (permalink / raw)
To: Tvrtko Ursulin, Lucas De Marchi; +Cc: Intel-gfx, dri-devel, baolu.lu
On 2021/11/10 1:35, Tvrtko Ursulin wrote:
>
> On 09/11/2021 17:19, Lucas De Marchi wrote:
>> On Tue, Nov 09, 2021 at 12:17:59PM +0000, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>> and probe presence of iommu domain per device to accurately reflect its
>>> status.
>>
>> nice, I was just starting to look into thus but for another reason: we
>> are adding support for other archs, like aarch64, and the global from
>> here
>> was a problem
>
> Yes I realized the other iommu angle as well. To do this properly we
> need to sort the intel_vtd_active call sites into at least two buckets -
> which are truly about VT-d and which are just IOMMU.
>
> For instance the THP decision in i915_gemfs.co would be "are we behind
> any iommu". Some other call sites are possibly only about the bugs in
> the igfx iommu. Not sure if there is a third bucket for any potential
> differences between igfx iommu and other Intel iommu in case of dgfx.
>
> I'd like to hear from Baolu as well to confirm if intel_iommu driver is
> handling igfx + dgfx correctly in respect to the two global variables I
> mention in the commit message.
I strongly agree that the drivers should call the IOMMU interface
directly for portability. For Intel graphic driver, we have two issues:
#1) driver asks vt-d driver for identity map with intel_iommu=igfx_off.
#2) driver query the status with a global intel_iommu_gfx_mapped.
We need to solve these two problems step by step. This patch is
definitely a good start point.
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Use per device iommu check (rev2)
2021-11-09 12:17 ` [Intel-gfx] " Tvrtko Ursulin
` (4 preceding siblings ...)
(?)
@ 2021-11-10 8:00 ` Patchwork
-1 siblings, 0 replies; 39+ messages in thread
From: Patchwork @ 2021-11-10 8:00 UTC (permalink / raw)
To: Lu Baolu; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Use per device iommu check (rev2)
URL : https://patchwork.freedesktop.org/series/96715/
State : failure
== Summary ==
Applying: drm/i915: Use per device iommu check
error: git diff header lacks filename information when removing 1 leading pathname component (line 2)
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915: Use per device iommu check
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-10 7:12 ` Lu Baolu
@ 2021-11-10 9:30 ` Tvrtko Ursulin
-1 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-10 9:30 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel, Tvrtko Ursulin
On 10/11/2021 07:12, Lu Baolu wrote:
> Hi Tvrtko,
>
> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>
>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>> and probe presence of iommu domain per device to accurately reflect its
>> status.
>>
>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>> ---
>> Baolu, is my understanding here correct? Maybe I am confused by both
>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the intel_iommu
>> driver. But it certainly appears the setup can assign some iommu ops (and
>> assign the discrete i915 to iommu group) when those two are set to off.
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index e967cd08f23e..9fb38a54f1fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1763,26 +1763,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();
> }
>
> Have you verified this change? I am afraid that
> iommu_get_domain_for_dev() always gets a valid iommu domain even
> intel_iommu_gfx_mapped == 0.
Yes it seems to work as is:
default:
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
intel_iommu=igfx_off:
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
On my system dri device 0 is integrated graphics and 1 is discrete.
Regards,
Tvrtko
>
> A possible way could look like this:
>
> static bool intel_vtd_active(struct drm_i915_private *i915)
> {
> struct iommu_domain *domain;
>
> domain = iommu_get_domain_for_dev(i915->drm.dev);
>
> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
> return true;
>
> ... ...
> }
>
> Actually I don't like this either since it checks the domain->type out
> of the iommu subsystem. We could refactor this later by export an iommu
> interface for this check.
>
> Best regards,
> baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-10 9:30 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-10 9:30 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel
On 10/11/2021 07:12, Lu Baolu wrote:
> Hi Tvrtko,
>
> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>
>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>> and probe presence of iommu domain per device to accurately reflect its
>> status.
>>
>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>> ---
>> Baolu, is my understanding here correct? Maybe I am confused by both
>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the intel_iommu
>> driver. But it certainly appears the setup can assign some iommu ops (and
>> assign the discrete i915 to iommu group) when those two are set to off.
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index e967cd08f23e..9fb38a54f1fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1763,26 +1763,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();
> }
>
> Have you verified this change? I am afraid that
> iommu_get_domain_for_dev() always gets a valid iommu domain even
> intel_iommu_gfx_mapped == 0.
Yes it seems to work as is:
default:
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
intel_iommu=igfx_off:
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
On my system dri device 0 is integrated graphics and 1 is discrete.
Regards,
Tvrtko
>
> A possible way could look like this:
>
> static bool intel_vtd_active(struct drm_i915_private *i915)
> {
> struct iommu_domain *domain;
>
> domain = iommu_get_domain_for_dev(i915->drm.dev);
>
> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
> return true;
>
> ... ...
> }
>
> Actually I don't like this either since it checks the domain->type out
> of the iommu subsystem. We could refactor this later by export an iommu
> interface for this check.
>
> Best regards,
> baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
2021-11-10 7:25 ` Lu Baolu
@ 2021-11-10 9:35 ` Tvrtko Ursulin
2021-11-10 12:16 ` Robin Murphy
2021-11-10 12:26 ` Lu Baolu
0 siblings, 2 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-10 9:35 UTC (permalink / raw)
To: Lu Baolu, Lucas De Marchi; +Cc: Intel-gfx, dri-devel
On 10/11/2021 07:25, Lu Baolu wrote:
> On 2021/11/10 1:35, Tvrtko Ursulin wrote:
>>
>> On 09/11/2021 17:19, Lucas De Marchi wrote:
>>> On Tue, Nov 09, 2021 at 12:17:59PM +0000, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>>> and probe presence of iommu domain per device to accurately reflect its
>>>> status.
>>>
>>> nice, I was just starting to look into thus but for another reason: we
>>> are adding support for other archs, like aarch64, and the global from
>>> here
>>> was a problem
>>
>> Yes I realized the other iommu angle as well. To do this properly we
>> need to sort the intel_vtd_active call sites into at least two buckets
>> - which are truly about VT-d and which are just IOMMU.
>>
>> For instance the THP decision in i915_gemfs.co would be "are we behind
>> any iommu". Some other call sites are possibly only about the bugs in
>> the igfx iommu. Not sure if there is a third bucket for any potential
>> differences between igfx iommu and other Intel iommu in case of dgfx.
>>
>> I'd like to hear from Baolu as well to confirm if intel_iommu driver
>> is handling igfx + dgfx correctly in respect to the two global
>> variables I mention in the commit message.
>
> I strongly agree that the drivers should call the IOMMU interface
> directly for portability. For Intel graphic driver, we have two issues:
>
> #1) driver asks vt-d driver for identity map with intel_iommu=igfx_off.
> #2) driver query the status with a global intel_iommu_gfx_mapped.
>
> We need to solve these two problems step by step. This patch is
> definitely a good start point.
(I should have really consolidated the thread, but never mind now.)
You mean good starting point for the discussion or between your first
and second email you started thinking it may even work?
Because as I wrote in the other email, it appears to work. But I fully
accept it may be by accident and you may suggest a proper API to be
added to the IOMMU core, which I would then be happy to use.
If maybe not immediately, perhaps we could start with this patch and
going forward add something more detailed. Like for instance allowing us
to query the name/id of the iommu driver in case i915 needs to apply
different quirks across them? Not sure how feasible that would be, but
at the moment the need does sound plausible to me.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-10 9:30 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-11-10 12:04 ` Lu Baolu
-1 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-10 12:04 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: Tvrtko Ursulin, dri-devel, baolu.lu
On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>
> On 10/11/2021 07:12, Lu Baolu wrote:
>> Hi Tvrtko,
>>
>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>
>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>> and probe presence of iommu domain per device to accurately reflect its
>>> status.
>>>
>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>> ---
>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the intel_iommu
>>> driver. But it certainly appears the setup can assign some iommu ops
>>> (and
>>> assign the discrete i915 to iommu group) when those two are set to off.
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index e967cd08f23e..9fb38a54f1fe 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1763,26 +1763,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();
>> }
>>
>> Have you verified this change? I am afraid that
>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>> intel_iommu_gfx_mapped == 0.
>
> Yes it seems to work as is:
>
> default:
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>
> intel_iommu=igfx_off:
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>
> On my system dri device 0 is integrated graphics and 1 is discrete.
The drm device 0 has a dedicated iommu. When the user request igfx not
mapped, the VT-d implementation will turn it off to save power. But for
shared iommu, you definitely will get it enabled.
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-10 12:04 ` Lu Baolu
0 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-10 12:04 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: dri-devel, baolu.lu
On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>
> On 10/11/2021 07:12, Lu Baolu wrote:
>> Hi Tvrtko,
>>
>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>
>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>> and probe presence of iommu domain per device to accurately reflect its
>>> status.
>>>
>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>> ---
>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the intel_iommu
>>> driver. But it certainly appears the setup can assign some iommu ops
>>> (and
>>> assign the discrete i915 to iommu group) when those two are set to off.
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index e967cd08f23e..9fb38a54f1fe 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1763,26 +1763,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();
>> }
>>
>> Have you verified this change? I am afraid that
>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>> intel_iommu_gfx_mapped == 0.
>
> Yes it seems to work as is:
>
> default:
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>
> intel_iommu=igfx_off:
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>
> On my system dri device 0 is integrated graphics and 1 is discrete.
The drm device 0 has a dedicated iommu. When the user request igfx not
mapped, the VT-d implementation will turn it off to save power. But for
shared iommu, you definitely will get it enabled.
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-10 12:04 ` [Intel-gfx] " Lu Baolu
@ 2021-11-10 12:08 ` Tvrtko Ursulin
-1 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-10 12:08 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel, Tvrtko Ursulin
On 10/11/2021 12:04, Lu Baolu wrote:
> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 07:12, Lu Baolu wrote:
>>> Hi Tvrtko,
>>>
>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>
>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>>> and probe presence of iommu domain per device to accurately reflect its
>>>> status.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>> ---
>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>> intel_iommu
>>>> driver. But it certainly appears the setup can assign some iommu ops
>>>> (and
>>>> assign the discrete i915 to iommu group) when those two are set to off.
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index e967cd08f23e..9fb38a54f1fe 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -1763,26 +1763,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();
>>> }
>>>
>>> Have you verified this change? I am afraid that
>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>> intel_iommu_gfx_mapped == 0.
>>
>> Yes it seems to work as is:
>>
>> default:
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>
>> intel_iommu=igfx_off:
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>
>> On my system dri device 0 is integrated graphics and 1 is discrete.
>
> The drm device 0 has a dedicated iommu. When the user request igfx not
> mapped, the VT-d implementation will turn it off to save power. But for
> shared iommu, you definitely will get it enabled.
Sorry I am not following, what exactly do you mean? Is there a platform
with integrated graphics without a dedicated iommu, in which case
intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
iommu_get_domain_for_dev returning non-NULL?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-10 12:08 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-10 12:08 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel
On 10/11/2021 12:04, Lu Baolu wrote:
> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 07:12, Lu Baolu wrote:
>>> Hi Tvrtko,
>>>
>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>
>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option only
>>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>>> and probe presence of iommu domain per device to accurately reflect its
>>>> status.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>> ---
>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>> intel_iommu
>>>> driver. But it certainly appears the setup can assign some iommu ops
>>>> (and
>>>> assign the discrete i915 to iommu group) when those two are set to off.
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index e967cd08f23e..9fb38a54f1fe 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -1763,26 +1763,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();
>>> }
>>>
>>> Have you verified this change? I am afraid that
>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>> intel_iommu_gfx_mapped == 0.
>>
>> Yes it seems to work as is:
>>
>> default:
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>
>> intel_iommu=igfx_off:
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>
>> On my system dri device 0 is integrated graphics and 1 is discrete.
>
> The drm device 0 has a dedicated iommu. When the user request igfx not
> mapped, the VT-d implementation will turn it off to save power. But for
> shared iommu, you definitely will get it enabled.
Sorry I am not following, what exactly do you mean? Is there a platform
with integrated graphics without a dedicated iommu, in which case
intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
iommu_get_domain_for_dev returning non-NULL?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
2021-11-10 9:35 ` Tvrtko Ursulin
@ 2021-11-10 12:16 ` Robin Murphy
2021-11-10 12:26 ` Lu Baolu
1 sibling, 0 replies; 39+ messages in thread
From: Robin Murphy @ 2021-11-10 12:16 UTC (permalink / raw)
To: Tvrtko Ursulin, Lu Baolu, Lucas De Marchi; +Cc: Intel-gfx, dri-devel
On 2021-11-10 09:35, Tvrtko Ursulin wrote:
>
> On 10/11/2021 07:25, Lu Baolu wrote:
>> On 2021/11/10 1:35, Tvrtko Ursulin wrote:
>>>
>>> On 09/11/2021 17:19, Lucas De Marchi wrote:
>>>> On Tue, Nov 09, 2021 at 12:17:59PM +0000, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option
>>>>> only
>>>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>>>> and probe presence of iommu domain per device to accurately reflect
>>>>> its
>>>>> status.
>>>>
>>>> nice, I was just starting to look into thus but for another reason: we
>>>> are adding support for other archs, like aarch64, and the global
>>>> from here
>>>> was a problem
>>>
>>> Yes I realized the other iommu angle as well. To do this properly we
>>> need to sort the intel_vtd_active call sites into at least two
>>> buckets - which are truly about VT-d and which are just IOMMU.
>>>
>>> For instance the THP decision in i915_gemfs.co would be "are we
>>> behind any iommu". Some other call sites are possibly only about the
>>> bugs in the igfx iommu. Not sure if there is a third bucket for any
>>> potential differences between igfx iommu and other Intel iommu in
>>> case of dgfx.
>>>
>>> I'd like to hear from Baolu as well to confirm if intel_iommu driver
>>> is handling igfx + dgfx correctly in respect to the two global
>>> variables I mention in the commit message.
>>
>> I strongly agree that the drivers should call the IOMMU interface
>> directly for portability. For Intel graphic driver, we have two issues:
>>
>> #1) driver asks vt-d driver for identity map with intel_iommu=igfx_off.
>> #2) driver query the status with a global intel_iommu_gfx_mapped.
>>
>> We need to solve these two problems step by step. This patch is
>> definitely a good start point.
>
> (I should have really consolidated the thread, but never mind now.)
>
> You mean good starting point for the discussion or between your first
> and second email you started thinking it may even work?
>
> Because as I wrote in the other email, it appears to work. But I fully
> accept it may be by accident and you may suggest a proper API to be
> added to the IOMMU core, which I would then be happy to use.
The "proper" API at the moment is device_iommu_mapped(), but indeed that
only answers the "is this device connected to an IOMMU at all?"
question, it doesn't tell you whether that IOMMU is translating or just
bypassing.
If translation only really matters in terms of DMA API usage - i.e.
you're not interested in using the IOMMU API directly - then I reckon it
would be fairly reasonable to use dma_get_ops() to look at whether
you've got dma-direct or iommu_dma_ops. At the moment that's sufficient
to tell you whether your DMA is translated or not. If a more formal
interface is wanted in future, I'm inclined to think that it would still
belong at the DMA API level, since the public IOMMU API is really all
about explicit translation, whereas what we care about here is reflected
more in its internal interaction with the DMA APIs.
> If maybe not immediately, perhaps we could start with this patch and
> going forward add something more detailed. Like for instance allowing us
> to query the name/id of the iommu driver in case i915 needs to apply
> different quirks across them? Not sure how feasible that would be, but
> at the moment the need does sound plausible to me.
FWIW I'd be wary of trying to get that clever - with iGFX it's easy to
make fine-grained decisions because you've got a known and fixed
integration with a particular IOMMU, but once you get out into the wider
world you'll run into not only multiple different IOMMU implementations
behind the same driver, but even the exact same IOMMU IP having
different characteristics in different SoCs. Even something as
seemingly-innocuous as an "is it worth using 2MB large pages?" quirk
list could effectively become the cross product of various kernel config
options and all PCIe-capable SoCs in existence.
Cheers,
Robin.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
2021-11-10 9:35 ` Tvrtko Ursulin
2021-11-10 12:16 ` Robin Murphy
@ 2021-11-10 12:26 ` Lu Baolu
1 sibling, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-10 12:26 UTC (permalink / raw)
To: Tvrtko Ursulin, Lucas De Marchi; +Cc: Intel-gfx, dri-devel, baolu.lu
On 2021/11/10 17:35, Tvrtko Ursulin wrote:
>
> On 10/11/2021 07:25, Lu Baolu wrote:
>> On 2021/11/10 1:35, Tvrtko Ursulin wrote:
>>>
>>> On 09/11/2021 17:19, Lucas De Marchi wrote:
>>>> On Tue, Nov 09, 2021 at 12:17:59PM +0000, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option
>>>>> only
>>>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>>>> and probe presence of iommu domain per device to accurately reflect
>>>>> its
>>>>> status.
>>>>
>>>> nice, I was just starting to look into thus but for another reason: we
>>>> are adding support for other archs, like aarch64, and the global
>>>> from here
>>>> was a problem
>>>
>>> Yes I realized the other iommu angle as well. To do this properly we
>>> need to sort the intel_vtd_active call sites into at least two
>>> buckets - which are truly about VT-d and which are just IOMMU.
>>>
>>> For instance the THP decision in i915_gemfs.co would be "are we
>>> behind any iommu". Some other call sites are possibly only about the
>>> bugs in the igfx iommu. Not sure if there is a third bucket for any
>>> potential differences between igfx iommu and other Intel iommu in
>>> case of dgfx.
>>>
>>> I'd like to hear from Baolu as well to confirm if intel_iommu driver
>>> is handling igfx + dgfx correctly in respect to the two global
>>> variables I mention in the commit message.
>>
>> I strongly agree that the drivers should call the IOMMU interface
>> directly for portability. For Intel graphic driver, we have two issues:
>>
>> #1) driver asks vt-d driver for identity map with intel_iommu=igfx_off.
>> #2) driver query the status with a global intel_iommu_gfx_mapped.
>>
>> We need to solve these two problems step by step. This patch is
>> definitely a good start point.
>
> (I should have really consolidated the thread, but never mind now.)
>
> You mean good starting point for the discussion or between your first
> and second email you started thinking it may even work?
>
> Because as I wrote in the other email, it appears to work. But I fully
> accept it may be by accident and you may suggest a proper API to be
> added to the IOMMU core, which I would then be happy to use.
>
> If maybe not immediately, perhaps we could start with this patch and
> going forward add something more detailed. Like for instance allowing us
> to query the name/id of the iommu driver in case i915 needs to apply
> different quirks across them? Not sure how feasible that would be, but
> at the moment the need does sound plausible to me.
The whole story in my mind looks like below:
1. The graphic device driver has its own way to let the user specify
iommu-bypass mode. For example, early_param()...
2. If the iommu-bypass mode is set, the graphic device will get below
set:
dev->dma_ops_bypass = true;
3. The iommu probe procedure will use identity domain (non-mapped mode)
for the device because value of dev->dma_ops_bypass is true.
That's all. The device driver doesn't need to check iommu for the
mapping mode. And this framework will be generic and could be used by
other device drivers.
The difficulty in doing so is that step 2 must happen before or during
the device probe process. At present, I haven't figured out how to do
it yet. :-)
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-10 12:08 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-11-10 12:35 ` Lu Baolu
-1 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-10 12:35 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: Tvrtko Ursulin, dri-devel, baolu.lu
On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>
> On 10/11/2021 12:04, Lu Baolu wrote:
>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>> Hi Tvrtko,
>>>>
>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>
>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option
>>>>> only
>>>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>>>> and probe presence of iommu domain per device to accurately reflect
>>>>> its
>>>>> status.
>>>>>
>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>> ---
>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>> intel_iommu
>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>> ops (and
>>>>> assign the discrete i915 to iommu group) when those two are set to
>>>>> off.
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -1763,26 +1763,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();
>>>> }
>>>>
>>>> Have you verified this change? I am afraid that
>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>> intel_iommu_gfx_mapped == 0.
>>>
>>> Yes it seems to work as is:
>>>
>>> default:
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>
>>> intel_iommu=igfx_off:
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>
>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>
>> The drm device 0 has a dedicated iommu. When the user request igfx not
>> mapped, the VT-d implementation will turn it off to save power. But for
>> shared iommu, you definitely will get it enabled.
>
> Sorry I am not following, what exactly do you mean? Is there a platform
> with integrated graphics without a dedicated iommu, in which case
> intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
> iommu_get_domain_for_dev returning non-NULL?
Your code always work for an igfx with a dedicated iommu. This might be
always true on today's platforms. But from driver's point of view, we
should not make such assumption.
For example, if the iommu implementation decides not to turn off the
graphic iommu (perhaps due to some hw quirk or for graphic
virtualization), your code will be broken.
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-10 12:35 ` Lu Baolu
0 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-10 12:35 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: dri-devel, baolu.lu
On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>
> On 10/11/2021 12:04, Lu Baolu wrote:
>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>> Hi Tvrtko,
>>>>
>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>
>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option
>>>>> only
>>>>> disables the igfx iommu. Stop relying on global intel_iommu_gfx_mapped
>>>>> and probe presence of iommu domain per device to accurately reflect
>>>>> its
>>>>> status.
>>>>>
>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>> ---
>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>> intel_iommu
>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>> ops (and
>>>>> assign the discrete i915 to iommu group) when those two are set to
>>>>> off.
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -1763,26 +1763,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();
>>>> }
>>>>
>>>> Have you verified this change? I am afraid that
>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>> intel_iommu_gfx_mapped == 0.
>>>
>>> Yes it seems to work as is:
>>>
>>> default:
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>
>>> intel_iommu=igfx_off:
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>
>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>
>> The drm device 0 has a dedicated iommu. When the user request igfx not
>> mapped, the VT-d implementation will turn it off to save power. But for
>> shared iommu, you definitely will get it enabled.
>
> Sorry I am not following, what exactly do you mean? Is there a platform
> with integrated graphics without a dedicated iommu, in which case
> intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
> iommu_get_domain_for_dev returning non-NULL?
Your code always work for an igfx with a dedicated iommu. This might be
always true on today's platforms. But from driver's point of view, we
should not make such assumption.
For example, if the iommu implementation decides not to turn off the
graphic iommu (perhaps due to some hw quirk or for graphic
virtualization), your code will be broken.
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-10 12:35 ` [Intel-gfx] " Lu Baolu
@ 2021-11-10 14:11 ` Tvrtko Ursulin
-1 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-10 14:11 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: Robin Murphy, dri-devel, Tvrtko Ursulin
On 10/11/2021 12:35, Lu Baolu wrote:
> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 12:04, Lu Baolu wrote:
>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>> Hi Tvrtko,
>>>>>
>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option
>>>>>> only
>>>>>> disables the igfx iommu. Stop relying on global
>>>>>> intel_iommu_gfx_mapped
>>>>>> and probe presence of iommu domain per device to accurately
>>>>>> reflect its
>>>>>> status.
>>>>>>
>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>> ---
>>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>> intel_iommu
>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>> ops (and
>>>>>> assign the discrete i915 to iommu group) when those two are set to
>>>>>> off.
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -1763,26 +1763,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();
>>>>> }
>>>>>
>>>>> Have you verified this change? I am afraid that
>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>> intel_iommu_gfx_mapped == 0.
>>>>
>>>> Yes it seems to work as is:
>>>>
>>>> default:
>>>>
>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>
>>>> intel_iommu=igfx_off:
>>>>
>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>
>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>
>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>> mapped, the VT-d implementation will turn it off to save power. But for
>>> shared iommu, you definitely will get it enabled.
>>
>> Sorry I am not following, what exactly do you mean? Is there a
>> platform with integrated graphics without a dedicated iommu, in which
>> case intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
>> iommu_get_domain_for_dev returning non-NULL?
>
> Your code always work for an igfx with a dedicated iommu. This might be
> always true on today's platforms. But from driver's point of view, we
> should not make such assumption.
>
> For example, if the iommu implementation decides not to turn off the
> graphic iommu (perhaps due to some hw quirk or for graphic
> virtualization), your code will be broken.
If I got it right, this would go back to your earlier recommendation to
have the check look like this:
static bool intel_vtd_active(struct drm_i915_private *i915)
{
struct iommu_domain *domain;
domain = iommu_get_domain_for_dev(i915->drm.dev);
if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
return true;
...
This would be okay as a first step?
Elsewhere in the thread Robin suggested looking at the dec->dma_ops and
comparing against iommu_dma_ops. These two solution would be effectively
the same?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-10 14:11 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-10 14:11 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: Robin Murphy, dri-devel
On 10/11/2021 12:35, Lu Baolu wrote:
> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 12:04, Lu Baolu wrote:
>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>> Hi Tvrtko,
>>>>>
>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option
>>>>>> only
>>>>>> disables the igfx iommu. Stop relying on global
>>>>>> intel_iommu_gfx_mapped
>>>>>> and probe presence of iommu domain per device to accurately
>>>>>> reflect its
>>>>>> status.
>>>>>>
>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>> ---
>>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>> intel_iommu
>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>> ops (and
>>>>>> assign the discrete i915 to iommu group) when those two are set to
>>>>>> off.
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -1763,26 +1763,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();
>>>>> }
>>>>>
>>>>> Have you verified this change? I am afraid that
>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>> intel_iommu_gfx_mapped == 0.
>>>>
>>>> Yes it seems to work as is:
>>>>
>>>> default:
>>>>
>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>
>>>> intel_iommu=igfx_off:
>>>>
>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>
>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>
>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>> mapped, the VT-d implementation will turn it off to save power. But for
>>> shared iommu, you definitely will get it enabled.
>>
>> Sorry I am not following, what exactly do you mean? Is there a
>> platform with integrated graphics without a dedicated iommu, in which
>> case intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
>> iommu_get_domain_for_dev returning non-NULL?
>
> Your code always work for an igfx with a dedicated iommu. This might be
> always true on today's platforms. But from driver's point of view, we
> should not make such assumption.
>
> For example, if the iommu implementation decides not to turn off the
> graphic iommu (perhaps due to some hw quirk or for graphic
> virtualization), your code will be broken.
If I got it right, this would go back to your earlier recommendation to
have the check look like this:
static bool intel_vtd_active(struct drm_i915_private *i915)
{
struct iommu_domain *domain;
domain = iommu_get_domain_for_dev(i915->drm.dev);
if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
return true;
...
This would be okay as a first step?
Elsewhere in the thread Robin suggested looking at the dec->dma_ops and
comparing against iommu_dma_ops. These two solution would be effectively
the same?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-10 14:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-11-10 14:37 ` Robin Murphy
-1 siblings, 0 replies; 39+ messages in thread
From: Robin Murphy @ 2021-11-10 14:37 UTC (permalink / raw)
To: Tvrtko Ursulin, Lu Baolu, Intel-gfx; +Cc: dri-devel, Tvrtko Ursulin
On 2021-11-10 14:11, Tvrtko Ursulin wrote:
>
> On 10/11/2021 12:35, Lu Baolu wrote:
>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>> Hi Tvrtko,
>>>>>>
>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>> option only
>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>> intel_iommu_gfx_mapped
>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>> reflect its
>>>>>>> status.
>>>>>>>
>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>> ---
>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>> intel_iommu
>>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>>> ops (and
>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>> to off.
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> @@ -1763,26 +1763,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();
>>>>>> }
>>>>>>
>>>>>> Have you verified this change? I am afraid that
>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>
>>>>> Yes it seems to work as is:
>>>>>
>>>>> default:
>>>>>
>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>
>>>>> intel_iommu=igfx_off:
>>>>>
>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>
>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>
>>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>>> mapped, the VT-d implementation will turn it off to save power. But for
>>>> shared iommu, you definitely will get it enabled.
>>>
>>> Sorry I am not following, what exactly do you mean? Is there a
>>> platform with integrated graphics without a dedicated iommu, in which
>>> case intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
>>> iommu_get_domain_for_dev returning non-NULL?
>>
>> Your code always work for an igfx with a dedicated iommu. This might be
>> always true on today's platforms. But from driver's point of view, we
>> should not make such assumption.
>>
>> For example, if the iommu implementation decides not to turn off the
>> graphic iommu (perhaps due to some hw quirk or for graphic
>> virtualization), your code will be broken.
>
> If I got it right, this would go back to your earlier recommendation to
> have the check look like this:
>
> static bool intel_vtd_active(struct drm_i915_private *i915)
> {
> struct iommu_domain *domain;
>
> domain = iommu_get_domain_for_dev(i915->drm.dev);
> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
> return true;
> ...
>
> This would be okay as a first step?
>
> Elsewhere in the thread Robin suggested looking at the dec->dma_ops and
> comparing against iommu_dma_ops. These two solution would be effectively
> the same?
Effectively, yes. See iommu_setup_dma_ops() - the only way to end up
with iommu_dma_ops is if a managed translation domain is present; if the
IOMMU is present but the default domain type has been set to passthrough
(either globally or forced for the given device) it will do nothing and
leave you with dma-direct, while if the IOMMU has been ignored entirely
then it should never even be called. Thus it neatly encapsulates what
you're after here.
Cheers,
Robin.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-10 14:37 ` Robin Murphy
0 siblings, 0 replies; 39+ messages in thread
From: Robin Murphy @ 2021-11-10 14:37 UTC (permalink / raw)
To: Tvrtko Ursulin, Lu Baolu, Intel-gfx; +Cc: dri-devel
On 2021-11-10 14:11, Tvrtko Ursulin wrote:
>
> On 10/11/2021 12:35, Lu Baolu wrote:
>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>> Hi Tvrtko,
>>>>>>
>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>> option only
>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>> intel_iommu_gfx_mapped
>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>> reflect its
>>>>>>> status.
>>>>>>>
>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>> ---
>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>> intel_iommu
>>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>>> ops (and
>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>> to off.
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> @@ -1763,26 +1763,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();
>>>>>> }
>>>>>>
>>>>>> Have you verified this change? I am afraid that
>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>
>>>>> Yes it seems to work as is:
>>>>>
>>>>> default:
>>>>>
>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>
>>>>> intel_iommu=igfx_off:
>>>>>
>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>
>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>
>>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>>> mapped, the VT-d implementation will turn it off to save power. But for
>>>> shared iommu, you definitely will get it enabled.
>>>
>>> Sorry I am not following, what exactly do you mean? Is there a
>>> platform with integrated graphics without a dedicated iommu, in which
>>> case intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
>>> iommu_get_domain_for_dev returning non-NULL?
>>
>> Your code always work for an igfx with a dedicated iommu. This might be
>> always true on today's platforms. But from driver's point of view, we
>> should not make such assumption.
>>
>> For example, if the iommu implementation decides not to turn off the
>> graphic iommu (perhaps due to some hw quirk or for graphic
>> virtualization), your code will be broken.
>
> If I got it right, this would go back to your earlier recommendation to
> have the check look like this:
>
> static bool intel_vtd_active(struct drm_i915_private *i915)
> {
> struct iommu_domain *domain;
>
> domain = iommu_get_domain_for_dev(i915->drm.dev);
> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
> return true;
> ...
>
> This would be okay as a first step?
>
> Elsewhere in the thread Robin suggested looking at the dec->dma_ops and
> comparing against iommu_dma_ops. These two solution would be effectively
> the same?
Effectively, yes. See iommu_setup_dma_ops() - the only way to end up
with iommu_dma_ops is if a managed translation domain is present; if the
IOMMU is present but the default domain type has been set to passthrough
(either globally or forced for the given device) it will do nothing and
leave you with dma-direct, while if the IOMMU has been ignored entirely
then it should never even be called. Thus it neatly encapsulates what
you're after here.
Cheers,
Robin.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-10 12:35 ` [Intel-gfx] " Lu Baolu
@ 2021-11-11 15:06 ` Tvrtko Ursulin
-1 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-11 15:06 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel, Tvrtko Ursulin
On 10/11/2021 12:35, Lu Baolu wrote:
> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 12:04, Lu Baolu wrote:
>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>> Hi Tvrtko,
>>>>>
>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option
>>>>>> only
>>>>>> disables the igfx iommu. Stop relying on global
>>>>>> intel_iommu_gfx_mapped
>>>>>> and probe presence of iommu domain per device to accurately
>>>>>> reflect its
>>>>>> status.
>>>>>>
>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>> ---
>>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>> intel_iommu
>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>> ops (and
>>>>>> assign the discrete i915 to iommu group) when those two are set to
>>>>>> off.
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -1763,26 +1763,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();
>>>>> }
>>>>>
>>>>> Have you verified this change? I am afraid that
>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>> intel_iommu_gfx_mapped == 0.
>>>>
>>>> Yes it seems to work as is:
>>>>
>>>> default:
>>>>
>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>
>>>> intel_iommu=igfx_off:
>>>>
>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>
>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>
>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>> mapped, the VT-d implementation will turn it off to save power. But for
>>> shared iommu, you definitely will get it enabled.
>>
>> Sorry I am not following, what exactly do you mean? Is there a
>> platform with integrated graphics without a dedicated iommu, in which
>> case intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
>> iommu_get_domain_for_dev returning non-NULL?
>
> Your code always work for an igfx with a dedicated iommu. This might be
> always true on today's platforms. But from driver's point of view, we
> should not make such assumption.
>
> For example, if the iommu implementation decides not to turn off the
> graphic iommu (perhaps due to some hw quirk or for graphic
> virtualization), your code will be broken.
I tried your suggestion (checking for __IOMMU_DOMAIN_PAGING) and it works better, however I have observed one odd behaviour (for me at least).
In short - why does the DMAR mode for the discrete device change depending on igfx_off parameter?
Consider the laptop has these two graphics cards:
# cat /sys/kernel/debug/dri/0/name
i915 dev=0000:00:02.0 unique=0000:00:02.0 # integrated
# cat /sys/kernel/debug/dri/1/name
i915 dev=0000:03:00.0 unique=0000:03:00.0 # discrete
Booting with different options:
===============================
default / intel_iommu=on
------------------------
# cat /sys/class/iommu/dmar0/devices/0000:00:02.0/iommu_group/type
DMA-FQ
# cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
DMA-FQ
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
All good.
intel_iommu=igfx_off
--------------------
## no dmar0 in sysfs
# cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
identity
Unexpected!?
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled # At least the i915 patch detects it correctly.
intel_iommu=off
---------------
## no dmar0 in sysfs
## no dmar2 in sysfs
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled
All good.
The fact discrete graphics changes from translated to pass-through when igfx_off is set is surprising to me. Is this a bug?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-11 15:06 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-11 15:06 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel
On 10/11/2021 12:35, Lu Baolu wrote:
> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 12:04, Lu Baolu wrote:
>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>> Hi Tvrtko,
>>>>>
>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off option
>>>>>> only
>>>>>> disables the igfx iommu. Stop relying on global
>>>>>> intel_iommu_gfx_mapped
>>>>>> and probe presence of iommu domain per device to accurately
>>>>>> reflect its
>>>>>> status.
>>>>>>
>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>> ---
>>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>> intel_iommu
>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>> ops (and
>>>>>> assign the discrete i915 to iommu group) when those two are set to
>>>>>> off.
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -1763,26 +1763,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();
>>>>> }
>>>>>
>>>>> Have you verified this change? I am afraid that
>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>> intel_iommu_gfx_mapped == 0.
>>>>
>>>> Yes it seems to work as is:
>>>>
>>>> default:
>>>>
>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>
>>>> intel_iommu=igfx_off:
>>>>
>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>
>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>
>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>> mapped, the VT-d implementation will turn it off to save power. But for
>>> shared iommu, you definitely will get it enabled.
>>
>> Sorry I am not following, what exactly do you mean? Is there a
>> platform with integrated graphics without a dedicated iommu, in which
>> case intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
>> iommu_get_domain_for_dev returning non-NULL?
>
> Your code always work for an igfx with a dedicated iommu. This might be
> always true on today's platforms. But from driver's point of view, we
> should not make such assumption.
>
> For example, if the iommu implementation decides not to turn off the
> graphic iommu (perhaps due to some hw quirk or for graphic
> virtualization), your code will be broken.
I tried your suggestion (checking for __IOMMU_DOMAIN_PAGING) and it works better, however I have observed one odd behaviour (for me at least).
In short - why does the DMAR mode for the discrete device change depending on igfx_off parameter?
Consider the laptop has these two graphics cards:
# cat /sys/kernel/debug/dri/0/name
i915 dev=0000:00:02.0 unique=0000:00:02.0 # integrated
# cat /sys/kernel/debug/dri/1/name
i915 dev=0000:03:00.0 unique=0000:03:00.0 # discrete
Booting with different options:
===============================
default / intel_iommu=on
------------------------
# cat /sys/class/iommu/dmar0/devices/0000:00:02.0/iommu_group/type
DMA-FQ
# cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
DMA-FQ
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
All good.
intel_iommu=igfx_off
--------------------
## no dmar0 in sysfs
# cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
identity
Unexpected!?
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled # At least the i915 patch detects it correctly.
intel_iommu=off
---------------
## no dmar0 in sysfs
## no dmar2 in sysfs
# grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
/sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
/sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled
All good.
The fact discrete graphics changes from translated to pass-through when igfx_off is set is surprising to me. Is this a bug?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-10 14:37 ` [Intel-gfx] " Robin Murphy
@ 2021-11-11 15:18 ` Tvrtko Ursulin
-1 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-11 15:18 UTC (permalink / raw)
To: Robin Murphy, Lu Baolu, Intel-gfx; +Cc: dri-devel, Tvrtko Ursulin
On 10/11/2021 14:37, Robin Murphy wrote:
> On 2021-11-10 14:11, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 12:35, Lu Baolu wrote:
>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>> Hi Tvrtko,
>>>>>>>
>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>
>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>> option only
>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>> reflect its
>>>>>>>> status.
>>>>>>>>
>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>> ---
>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by
>>>>>>>> both
>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>> intel_iommu
>>>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>>>> ops (and
>>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>>> to off.
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> @@ -1763,26 +1763,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();
>>>>>>> }
>>>>>>>
>>>>>>> Have you verified this change? I am afraid that
>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>
>>>>>> Yes it seems to work as is:
>>>>>>
>>>>>> default:
>>>>>>
>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>
>>>>>> intel_iommu=igfx_off:
>>>>>>
>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>
>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>
>>>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>>>> mapped, the VT-d implementation will turn it off to save power. But
>>>>> for
>>>>> shared iommu, you definitely will get it enabled.
>>>>
>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>> platform with integrated graphics without a dedicated iommu, in
>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped ==
>>>> 0 and iommu_get_domain_for_dev returning non-NULL?
>>>
>>> Your code always work for an igfx with a dedicated iommu. This might be
>>> always true on today's platforms. But from driver's point of view, we
>>> should not make such assumption.
>>>
>>> For example, if the iommu implementation decides not to turn off the
>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>> virtualization), your code will be broken.
>>
>> If I got it right, this would go back to your earlier recommendation
>> to have the check look like this:
>>
>> static bool intel_vtd_active(struct drm_i915_private *i915)
>> {
>> struct iommu_domain *domain;
>>
>> domain = iommu_get_domain_for_dev(i915->drm.dev);
>> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
>> return true;
>> ...
>>
>> This would be okay as a first step?
>>
>> Elsewhere in the thread Robin suggested looking at the dec->dma_ops
>> and comparing against iommu_dma_ops. These two solution would be
>> effectively the same?
>
> Effectively, yes. See iommu_setup_dma_ops() - the only way to end up
> with iommu_dma_ops is if a managed translation domain is present; if the
> IOMMU is present but the default domain type has been set to passthrough
> (either globally or forced for the given device) it will do nothing and
> leave you with dma-direct, while if the IOMMU has been ignored entirely
> then it should never even be called. Thus it neatly encapsulates what
> you're after here.
One concern I have is whether the pass-through mode truly does nothing
or addresses perhaps still go through the dmar hardware just with no
translation?
If latter then most like for like change is actually exactly what the
first version of my patch did. That is replace intel_iommu_gfx_mapped
with a plain non-NULL check on iommu_get_domain_for_dev.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-11 15:18 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-11 15:18 UTC (permalink / raw)
To: Robin Murphy, Lu Baolu, Intel-gfx; +Cc: dri-devel
On 10/11/2021 14:37, Robin Murphy wrote:
> On 2021-11-10 14:11, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 12:35, Lu Baolu wrote:
>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>> Hi Tvrtko,
>>>>>>>
>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>
>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>> option only
>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>> reflect its
>>>>>>>> status.
>>>>>>>>
>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>> ---
>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by
>>>>>>>> both
>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>> intel_iommu
>>>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>>>> ops (and
>>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>>> to off.
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> @@ -1763,26 +1763,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();
>>>>>>> }
>>>>>>>
>>>>>>> Have you verified this change? I am afraid that
>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>
>>>>>> Yes it seems to work as is:
>>>>>>
>>>>>> default:
>>>>>>
>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>
>>>>>> intel_iommu=igfx_off:
>>>>>>
>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>
>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>
>>>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>>>> mapped, the VT-d implementation will turn it off to save power. But
>>>>> for
>>>>> shared iommu, you definitely will get it enabled.
>>>>
>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>> platform with integrated graphics without a dedicated iommu, in
>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped ==
>>>> 0 and iommu_get_domain_for_dev returning non-NULL?
>>>
>>> Your code always work for an igfx with a dedicated iommu. This might be
>>> always true on today's platforms. But from driver's point of view, we
>>> should not make such assumption.
>>>
>>> For example, if the iommu implementation decides not to turn off the
>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>> virtualization), your code will be broken.
>>
>> If I got it right, this would go back to your earlier recommendation
>> to have the check look like this:
>>
>> static bool intel_vtd_active(struct drm_i915_private *i915)
>> {
>> struct iommu_domain *domain;
>>
>> domain = iommu_get_domain_for_dev(i915->drm.dev);
>> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
>> return true;
>> ...
>>
>> This would be okay as a first step?
>>
>> Elsewhere in the thread Robin suggested looking at the dec->dma_ops
>> and comparing against iommu_dma_ops. These two solution would be
>> effectively the same?
>
> Effectively, yes. See iommu_setup_dma_ops() - the only way to end up
> with iommu_dma_ops is if a managed translation domain is present; if the
> IOMMU is present but the default domain type has been set to passthrough
> (either globally or forced for the given device) it will do nothing and
> leave you with dma-direct, while if the IOMMU has been ignored entirely
> then it should never even be called. Thus it neatly encapsulates what
> you're after here.
One concern I have is whether the pass-through mode truly does nothing
or addresses perhaps still go through the dmar hardware just with no
translation?
If latter then most like for like change is actually exactly what the
first version of my patch did. That is replace intel_iommu_gfx_mapped
with a plain non-NULL check on iommu_get_domain_for_dev.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-11 15:06 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-11-12 0:53 ` Lu Baolu
-1 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-12 0:53 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: Tvrtko Ursulin, dri-devel, baolu.lu
On 11/11/21 11:06 PM, Tvrtko Ursulin wrote:
>
> On 10/11/2021 12:35, Lu Baolu wrote:
>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>> Hi Tvrtko,
>>>>>>
>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>> option only
>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>> intel_iommu_gfx_mapped
>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>> reflect its
>>>>>>> status.
>>>>>>>
>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>> ---
>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>> intel_iommu
>>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>>> ops (and
>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>> to off.
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> @@ -1763,26 +1763,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();
>>>>>> }
>>>>>>
>>>>>> Have you verified this change? I am afraid that
>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>
>>>>> Yes it seems to work as is:
>>>>>
>>>>> default:
>>>>>
>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>
>>>>> intel_iommu=igfx_off:
>>>>>
>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>
>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>
>>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>>> mapped, the VT-d implementation will turn it off to save power. But for
>>>> shared iommu, you definitely will get it enabled.
>>>
>>> Sorry I am not following, what exactly do you mean? Is there a
>>> platform with integrated graphics without a dedicated iommu, in which
>>> case intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
>>> iommu_get_domain_for_dev returning non-NULL?
>>
>> Your code always work for an igfx with a dedicated iommu. This might be
>> always true on today's platforms. But from driver's point of view, we
>> should not make such assumption.
>>
>> For example, if the iommu implementation decides not to turn off the
>> graphic iommu (perhaps due to some hw quirk or for graphic
>> virtualization), your code will be broken.
>
> I tried your suggestion (checking for __IOMMU_DOMAIN_PAGING) and it
> works better, however I have observed one odd behaviour (for me at least).
>
> In short - why does the DMAR mode for the discrete device change
> depending on igfx_off parameter?
>
> Consider the laptop has these two graphics cards:
>
> # cat /sys/kernel/debug/dri/0/name
> i915 dev=0000:00:02.0 unique=0000:00:02.0 # integrated
>
> # cat /sys/kernel/debug/dri/1/name
> i915 dev=0000:03:00.0 unique=0000:03:00.0 # discrete
>
> Booting with different options:
> ===============================
>
> default / intel_iommu=on
> ------------------------
>
> # cat /sys/class/iommu/dmar0/devices/0000:00:02.0/iommu_group/type
> DMA-FQ
> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
> DMA-FQ
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>
> All good.
>
> intel_iommu=igfx_off
> --------------------
>
> ## no dmar0 in sysfs
> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
> identity
>
> Unexpected!?
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled # At least the
> i915 patch detects it correctly.
>
> intel_iommu=off
> ---------------
>
> ## no dmar0 in sysfs
> ## no dmar2 in sysfs
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled
>
> All good.
>
> The fact discrete graphics changes from translated to pass-through when
> igfx_off is set is surprising to me. Is this a bug?
The existing VT-d implementation doesn't distinguish igfx from dgfx. It
only checks whether the device is of a display class:
#define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
When igfx_off is specified, all graphic devices will put into pass-
through (the same meaning as identity mapping) mode. For igfx, since
the iommu is always dedicated, hence it further turn off the iommu
(hence there's no iommu domain) to save power.
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-12 0:53 ` Lu Baolu
0 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-12 0:53 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx; +Cc: dri-devel, baolu.lu
On 11/11/21 11:06 PM, Tvrtko Ursulin wrote:
>
> On 10/11/2021 12:35, Lu Baolu wrote:
>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>> Hi Tvrtko,
>>>>>>
>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>> option only
>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>> intel_iommu_gfx_mapped
>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>> reflect its
>>>>>>> status.
>>>>>>>
>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>> ---
>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by both
>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>> intel_iommu
>>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>>> ops (and
>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>> to off.
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> @@ -1763,26 +1763,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();
>>>>>> }
>>>>>>
>>>>>> Have you verified this change? I am afraid that
>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>
>>>>> Yes it seems to work as is:
>>>>>
>>>>> default:
>>>>>
>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>
>>>>> intel_iommu=igfx_off:
>>>>>
>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>
>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>
>>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>>> mapped, the VT-d implementation will turn it off to save power. But for
>>>> shared iommu, you definitely will get it enabled.
>>>
>>> Sorry I am not following, what exactly do you mean? Is there a
>>> platform with integrated graphics without a dedicated iommu, in which
>>> case intel_iommu=igfx_off results in intel_iommu_gfx_mapped == 0 and
>>> iommu_get_domain_for_dev returning non-NULL?
>>
>> Your code always work for an igfx with a dedicated iommu. This might be
>> always true on today's platforms. But from driver's point of view, we
>> should not make such assumption.
>>
>> For example, if the iommu implementation decides not to turn off the
>> graphic iommu (perhaps due to some hw quirk or for graphic
>> virtualization), your code will be broken.
>
> I tried your suggestion (checking for __IOMMU_DOMAIN_PAGING) and it
> works better, however I have observed one odd behaviour (for me at least).
>
> In short - why does the DMAR mode for the discrete device change
> depending on igfx_off parameter?
>
> Consider the laptop has these two graphics cards:
>
> # cat /sys/kernel/debug/dri/0/name
> i915 dev=0000:00:02.0 unique=0000:00:02.0 # integrated
>
> # cat /sys/kernel/debug/dri/1/name
> i915 dev=0000:03:00.0 unique=0000:03:00.0 # discrete
>
> Booting with different options:
> ===============================
>
> default / intel_iommu=on
> ------------------------
>
> # cat /sys/class/iommu/dmar0/devices/0000:00:02.0/iommu_group/type
> DMA-FQ
> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
> DMA-FQ
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>
> All good.
>
> intel_iommu=igfx_off
> --------------------
>
> ## no dmar0 in sysfs
> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
> identity
>
> Unexpected!?
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled # At least the
> i915 patch detects it correctly.
>
> intel_iommu=off
> ---------------
>
> ## no dmar0 in sysfs
> ## no dmar2 in sysfs
>
> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled
>
> All good.
>
> The fact discrete graphics changes from translated to pass-through when
> igfx_off is set is surprising to me. Is this a bug?
The existing VT-d implementation doesn't distinguish igfx from dgfx. It
only checks whether the device is of a display class:
#define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
When igfx_off is specified, all graphic devices will put into pass-
through (the same meaning as identity mapping) mode. For igfx, since
the iommu is always dedicated, hence it further turn off the iommu
(hence there's no iommu domain) to save power.
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-11 15:18 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-11-12 0:58 ` Lu Baolu
-1 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-12 0:58 UTC (permalink / raw)
To: Tvrtko Ursulin, Robin Murphy, Intel-gfx
Cc: Tvrtko Ursulin, dri-devel, baolu.lu
On 11/11/21 11:18 PM, Tvrtko Ursulin wrote:
>
> On 10/11/2021 14:37, Robin Murphy wrote:
>> On 2021-11-10 14:11, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 12:35, Lu Baolu wrote:
>>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>>
>>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>>> Hi Tvrtko,
>>>>>>>>
>>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>>
>>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>>> option only
>>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>>> reflect its
>>>>>>>>> status.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>>> ---
>>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by
>>>>>>>>> both
>>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>>> intel_iommu
>>>>>>>>> driver. But it certainly appears the setup can assign some
>>>>>>>>> iommu ops (and
>>>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>>>> to off.
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> @@ -1763,26 +1763,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();
>>>>>>>> }
>>>>>>>>
>>>>>>>> Have you verified this change? I am afraid that
>>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>>
>>>>>>> Yes it seems to work as is:
>>>>>>>
>>>>>>> default:
>>>>>>>
>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>
>>>>>>> intel_iommu=igfx_off:
>>>>>>>
>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>
>>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>>
>>>>>> The drm device 0 has a dedicated iommu. When the user request igfx
>>>>>> not
>>>>>> mapped, the VT-d implementation will turn it off to save power.
>>>>>> But for
>>>>>> shared iommu, you definitely will get it enabled.
>>>>>
>>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>>> platform with integrated graphics without a dedicated iommu, in
>>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped
>>>>> == 0 and iommu_get_domain_for_dev returning non-NULL?
>>>>
>>>> Your code always work for an igfx with a dedicated iommu. This might be
>>>> always true on today's platforms. But from driver's point of view, we
>>>> should not make such assumption.
>>>>
>>>> For example, if the iommu implementation decides not to turn off the
>>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>>> virtualization), your code will be broken.
>>>
>>> If I got it right, this would go back to your earlier recommendation
>>> to have the check look like this:
>>>
>>> static bool intel_vtd_active(struct drm_i915_private *i915)
>>> {
>>> struct iommu_domain *domain;
>>>
>>> domain = iommu_get_domain_for_dev(i915->drm.dev);
>>> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
>>> return true;
>>> ...
>>>
>>> This would be okay as a first step?
>>>
>>> Elsewhere in the thread Robin suggested looking at the dec->dma_ops
>>> and comparing against iommu_dma_ops. These two solution would be
>>> effectively the same?
>>
>> Effectively, yes. See iommu_setup_dma_ops() - the only way to end up
>> with iommu_dma_ops is if a managed translation domain is present; if
>> the IOMMU is present but the default domain type has been set to
>> passthrough (either globally or forced for the given device) it will
>> do nothing and leave you with dma-direct, while if the IOMMU has been
>> ignored entirely then it should never even be called. Thus it neatly
>> encapsulates what you're after here.
>
> One concern I have is whether the pass-through mode truly does nothing
> or addresses perhaps still go through the dmar hardware just with no
> translation?
Pass-through mode means the latter.
>
> If latter then most like for like change is actually exactly what the
> first version of my patch did. That is replace intel_iommu_gfx_mapped
> with a plain non-NULL check on iommu_get_domain_for_dev.
Depends on what you want here,
#1) the graphic device works in iommu pass-through mode
- device have an iommu
- but iommu does no translation
- the dma transactions go through iommu with the same destination
memory address specified by the device;
#2) the graphic device works without a system iommu
- the iommu is off
- there's no iommu on the path of DMA transaction.
My suggestion works for #1). Robin's suggestion (device_iommu_mapped())
could work for #2).
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-12 0:58 ` Lu Baolu
0 siblings, 0 replies; 39+ messages in thread
From: Lu Baolu @ 2021-11-12 0:58 UTC (permalink / raw)
To: Tvrtko Ursulin, Robin Murphy, Intel-gfx; +Cc: dri-devel, baolu.lu
On 11/11/21 11:18 PM, Tvrtko Ursulin wrote:
>
> On 10/11/2021 14:37, Robin Murphy wrote:
>> On 2021-11-10 14:11, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 12:35, Lu Baolu wrote:
>>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>>
>>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>>> Hi Tvrtko,
>>>>>>>>
>>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>>
>>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>>> option only
>>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>>> reflect its
>>>>>>>>> status.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>>> ---
>>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by
>>>>>>>>> both
>>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>>> intel_iommu
>>>>>>>>> driver. But it certainly appears the setup can assign some
>>>>>>>>> iommu ops (and
>>>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>>>> to off.
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> @@ -1763,26 +1763,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();
>>>>>>>> }
>>>>>>>>
>>>>>>>> Have you verified this change? I am afraid that
>>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>>
>>>>>>> Yes it seems to work as is:
>>>>>>>
>>>>>>> default:
>>>>>>>
>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>
>>>>>>> intel_iommu=igfx_off:
>>>>>>>
>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>
>>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>>
>>>>>> The drm device 0 has a dedicated iommu. When the user request igfx
>>>>>> not
>>>>>> mapped, the VT-d implementation will turn it off to save power.
>>>>>> But for
>>>>>> shared iommu, you definitely will get it enabled.
>>>>>
>>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>>> platform with integrated graphics without a dedicated iommu, in
>>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped
>>>>> == 0 and iommu_get_domain_for_dev returning non-NULL?
>>>>
>>>> Your code always work for an igfx with a dedicated iommu. This might be
>>>> always true on today's platforms. But from driver's point of view, we
>>>> should not make such assumption.
>>>>
>>>> For example, if the iommu implementation decides not to turn off the
>>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>>> virtualization), your code will be broken.
>>>
>>> If I got it right, this would go back to your earlier recommendation
>>> to have the check look like this:
>>>
>>> static bool intel_vtd_active(struct drm_i915_private *i915)
>>> {
>>> struct iommu_domain *domain;
>>>
>>> domain = iommu_get_domain_for_dev(i915->drm.dev);
>>> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
>>> return true;
>>> ...
>>>
>>> This would be okay as a first step?
>>>
>>> Elsewhere in the thread Robin suggested looking at the dec->dma_ops
>>> and comparing against iommu_dma_ops. These two solution would be
>>> effectively the same?
>>
>> Effectively, yes. See iommu_setup_dma_ops() - the only way to end up
>> with iommu_dma_ops is if a managed translation domain is present; if
>> the IOMMU is present but the default domain type has been set to
>> passthrough (either globally or forced for the given device) it will
>> do nothing and leave you with dma-direct, while if the IOMMU has been
>> ignored entirely then it should never even be called. Thus it neatly
>> encapsulates what you're after here.
>
> One concern I have is whether the pass-through mode truly does nothing
> or addresses perhaps still go through the dmar hardware just with no
> translation?
Pass-through mode means the latter.
>
> If latter then most like for like change is actually exactly what the
> first version of my patch did. That is replace intel_iommu_gfx_mapped
> with a plain non-NULL check on iommu_get_domain_for_dev.
Depends on what you want here,
#1) the graphic device works in iommu pass-through mode
- device have an iommu
- but iommu does no translation
- the dma transactions go through iommu with the same destination
memory address specified by the device;
#2) the graphic device works without a system iommu
- the iommu is off
- there's no iommu on the path of DMA transaction.
My suggestion works for #1). Robin's suggestion (device_iommu_mapped())
could work for #2).
Best regards,
baolu
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-12 0:53 ` [Intel-gfx] " Lu Baolu
@ 2021-11-12 13:40 ` Tvrtko Ursulin
-1 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-12 13:40 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel, Tvrtko Ursulin
On 12/11/2021 00:53, Lu Baolu wrote:
> On 11/11/21 11:06 PM, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 12:35, Lu Baolu wrote:
>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>> Hi Tvrtko,
>>>>>>>
>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>
>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>> option only
>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>> reflect its
>>>>>>>> status.
>>>>>>>>
>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>> ---
>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by
>>>>>>>> both
>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>> intel_iommu
>>>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>>>> ops (and
>>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>>> to off.
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> @@ -1763,26 +1763,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();
>>>>>>> }
>>>>>>>
>>>>>>> Have you verified this change? I am afraid that
>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>
>>>>>> Yes it seems to work as is:
>>>>>>
>>>>>> default:
>>>>>>
>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>
>>>>>> intel_iommu=igfx_off:
>>>>>>
>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>
>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>
>>>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>>>> mapped, the VT-d implementation will turn it off to save power. But
>>>>> for
>>>>> shared iommu, you definitely will get it enabled.
>>>>
>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>> platform with integrated graphics without a dedicated iommu, in
>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped ==
>>>> 0 and iommu_get_domain_for_dev returning non-NULL?
>>>
>>> Your code always work for an igfx with a dedicated iommu. This might be
>>> always true on today's platforms. But from driver's point of view, we
>>> should not make such assumption.
>>>
>>> For example, if the iommu implementation decides not to turn off the
>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>> virtualization), your code will be broken.
>>
>> I tried your suggestion (checking for __IOMMU_DOMAIN_PAGING) and it
>> works better, however I have observed one odd behaviour (for me at
>> least).
>>
>> In short - why does the DMAR mode for the discrete device change
>> depending on igfx_off parameter?
>>
>> Consider the laptop has these two graphics cards:
>>
>> # cat /sys/kernel/debug/dri/0/name
>> i915 dev=0000:00:02.0 unique=0000:00:02.0 # integrated
>>
>> # cat /sys/kernel/debug/dri/1/name
>> i915 dev=0000:03:00.0 unique=0000:03:00.0 # discrete
>>
>> Booting with different options:
>> ===============================
>>
>> default / intel_iommu=on
>> ------------------------
>>
>> # cat /sys/class/iommu/dmar0/devices/0000:00:02.0/iommu_group/type
>> DMA-FQ
>> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
>> DMA-FQ
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>
>> All good.
>>
>> intel_iommu=igfx_off
>> --------------------
>>
>> ## no dmar0 in sysfs
>> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
>> identity
>>
>> Unexpected!?
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled # At least
>> the i915 patch detects it correctly.
>>
>> intel_iommu=off
>> ---------------
>>
>> ## no dmar0 in sysfs
>> ## no dmar2 in sysfs
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled
>>
>> All good.
>>
>> The fact discrete graphics changes from translated to pass-through
>> when igfx_off is set is surprising to me. Is this a bug?
>
> The existing VT-d implementation doesn't distinguish igfx from dgfx. It
> only checks whether the device is of a display class:
>
> #define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
>
> When igfx_off is specified, all graphic devices will put into pass-
> through (the same meaning as identity mapping) mode. For igfx, since
> the iommu is always dedicated, hence it further turn off the iommu
> (hence there's no iommu domain) to save power.
Ah okay. Is this something we want to change/fix?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-12 13:40 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-12 13:40 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel
On 12/11/2021 00:53, Lu Baolu wrote:
> On 11/11/21 11:06 PM, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 12:35, Lu Baolu wrote:
>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>> Hi Tvrtko,
>>>>>>>
>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>
>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>> option only
>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>> reflect its
>>>>>>>> status.
>>>>>>>>
>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>> ---
>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by
>>>>>>>> both
>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>> intel_iommu
>>>>>>>> driver. But it certainly appears the setup can assign some iommu
>>>>>>>> ops (and
>>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>>> to off.
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>> @@ -1763,26 +1763,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();
>>>>>>> }
>>>>>>>
>>>>>>> Have you verified this change? I am afraid that
>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>
>>>>>> Yes it seems to work as is:
>>>>>>
>>>>>> default:
>>>>>>
>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>
>>>>>> intel_iommu=igfx_off:
>>>>>>
>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>
>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>
>>>>> The drm device 0 has a dedicated iommu. When the user request igfx not
>>>>> mapped, the VT-d implementation will turn it off to save power. But
>>>>> for
>>>>> shared iommu, you definitely will get it enabled.
>>>>
>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>> platform with integrated graphics without a dedicated iommu, in
>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped ==
>>>> 0 and iommu_get_domain_for_dev returning non-NULL?
>>>
>>> Your code always work for an igfx with a dedicated iommu. This might be
>>> always true on today's platforms. But from driver's point of view, we
>>> should not make such assumption.
>>>
>>> For example, if the iommu implementation decides not to turn off the
>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>> virtualization), your code will be broken.
>>
>> I tried your suggestion (checking for __IOMMU_DOMAIN_PAGING) and it
>> works better, however I have observed one odd behaviour (for me at
>> least).
>>
>> In short - why does the DMAR mode for the discrete device change
>> depending on igfx_off parameter?
>>
>> Consider the laptop has these two graphics cards:
>>
>> # cat /sys/kernel/debug/dri/0/name
>> i915 dev=0000:00:02.0 unique=0000:00:02.0 # integrated
>>
>> # cat /sys/kernel/debug/dri/1/name
>> i915 dev=0000:03:00.0 unique=0000:03:00.0 # discrete
>>
>> Booting with different options:
>> ===============================
>>
>> default / intel_iommu=on
>> ------------------------
>>
>> # cat /sys/class/iommu/dmar0/devices/0000:00:02.0/iommu_group/type
>> DMA-FQ
>> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
>> DMA-FQ
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>
>> All good.
>>
>> intel_iommu=igfx_off
>> --------------------
>>
>> ## no dmar0 in sysfs
>> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
>> identity
>>
>> Unexpected!?
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled # At least
>> the i915 patch detects it correctly.
>>
>> intel_iommu=off
>> ---------------
>>
>> ## no dmar0 in sysfs
>> ## no dmar2 in sysfs
>>
>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled
>>
>> All good.
>>
>> The fact discrete graphics changes from translated to pass-through
>> when igfx_off is set is surprising to me. Is this a bug?
>
> The existing VT-d implementation doesn't distinguish igfx from dgfx. It
> only checks whether the device is of a display class:
>
> #define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
>
> When igfx_off is specified, all graphic devices will put into pass-
> through (the same meaning as identity mapping) mode. For igfx, since
> the iommu is always dedicated, hence it further turn off the iommu
> (hence there's no iommu domain) to save power.
Ah okay. Is this something we want to change/fix?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
2021-11-12 0:58 ` [Intel-gfx] " Lu Baolu
@ 2021-11-12 14:10 ` Tvrtko Ursulin
-1 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-12 14:10 UTC (permalink / raw)
To: Lu Baolu, Robin Murphy, Intel-gfx; +Cc: dri-devel
On 12/11/2021 00:58, Lu Baolu wrote:
> On 11/11/21 11:18 PM, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 14:37, Robin Murphy wrote:
>>> On 2021-11-10 14:11, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 12:35, Lu Baolu wrote:
>>>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>>>
>>>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>>>> Hi Tvrtko,
>>>>>>>>>
>>>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>>>
>>>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>>>> option only
>>>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>>>> reflect its
>>>>>>>>>> status.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>>>> ---
>>>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused
>>>>>>>>>> by both
>>>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>>>> intel_iommu
>>>>>>>>>> driver. But it certainly appears the setup can assign some
>>>>>>>>>> iommu ops (and
>>>>>>>>>> assign the discrete i915 to iommu group) when those two are
>>>>>>>>>> set to off.
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>>> @@ -1763,26 +1763,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();
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> Have you verified this change? I am afraid that
>>>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>>>
>>>>>>>> Yes it seems to work as is:
>>>>>>>>
>>>>>>>> default:
>>>>>>>>
>>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>>
>>>>>>>> intel_iommu=igfx_off:
>>>>>>>>
>>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>>
>>>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>>>
>>>>>>> The drm device 0 has a dedicated iommu. When the user request
>>>>>>> igfx not
>>>>>>> mapped, the VT-d implementation will turn it off to save power.
>>>>>>> But for
>>>>>>> shared iommu, you definitely will get it enabled.
>>>>>>
>>>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>>>> platform with integrated graphics without a dedicated iommu, in
>>>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped
>>>>>> == 0 and iommu_get_domain_for_dev returning non-NULL?
>>>>>
>>>>> Your code always work for an igfx with a dedicated iommu. This
>>>>> might be
>>>>> always true on today's platforms. But from driver's point of view, we
>>>>> should not make such assumption.
>>>>>
>>>>> For example, if the iommu implementation decides not to turn off the
>>>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>>>> virtualization), your code will be broken.
>>>>
>>>> If I got it right, this would go back to your earlier recommendation
>>>> to have the check look like this:
>>>>
>>>> static bool intel_vtd_active(struct drm_i915_private *i915)
>>>> {
>>>> struct iommu_domain *domain;
>>>>
>>>> domain = iommu_get_domain_for_dev(i915->drm.dev);
>>>> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
>>>> return true;
>>>> ...
>>>>
>>>> This would be okay as a first step?
>>>>
>>>> Elsewhere in the thread Robin suggested looking at the dec->dma_ops
>>>> and comparing against iommu_dma_ops. These two solution would be
>>>> effectively the same?
>>>
>>> Effectively, yes. See iommu_setup_dma_ops() - the only way to end up
>>> with iommu_dma_ops is if a managed translation domain is present; if
>>> the IOMMU is present but the default domain type has been set to
>>> passthrough (either globally or forced for the given device) it will
>>> do nothing and leave you with dma-direct, while if the IOMMU has been
>>> ignored entirely then it should never even be called. Thus it neatly
>>> encapsulates what you're after here.
>>
>> One concern I have is whether the pass-through mode truly does nothing
>> or addresses perhaps still go through the dmar hardware just with no
>> translation?
>
> Pass-through mode means the latter.
>
>>
>> If latter then most like for like change is actually exactly what the
>> first version of my patch did. That is replace intel_iommu_gfx_mapped
>> with a plain non-NULL check on iommu_get_domain_for_dev.
>
> Depends on what you want here,
>
> #1) the graphic device works in iommu pass-through mode
> - device have an iommu
> - but iommu does no translation
> - the dma transactions go through iommu with the same destination
> memory address specified by the device;
Do you have any indications of the slowdown this adds?
> #2) the graphic device works without a system iommu
> - the iommu is off
> - there's no iommu on the path of DMA transaction.
>
> My suggestion works for #1). Robin's suggestion (device_iommu_mapped())
> could work for #2).
On the question of what do I want here. It seems that to preserve
like-for-like with the current and past i915 usage, ie.
intel_iommu_gfx_mapped, the first version of my patch should be used.
In other words if I configure the boot with iommu=pt, then
intel_iommu_gfx_mapped is true. So if I add the __IOMMU_DOMAIN_PAGING
check, the new intel_vtd_active would return false, where the old
version would return true.
So v1 of the patch feels like the safest route given I don't know which
workarounds are due remapping slowdown, and which may be present even in
the pass-through mode.
I would explain the situation in the comment inside intel_vtd_active for
future reference.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
@ 2021-11-12 14:10 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-12 14:10 UTC (permalink / raw)
To: Lu Baolu, Robin Murphy, Intel-gfx; +Cc: dri-devel, Tvrtko Ursulin
On 12/11/2021 00:58, Lu Baolu wrote:
> On 11/11/21 11:18 PM, Tvrtko Ursulin wrote:
>>
>> On 10/11/2021 14:37, Robin Murphy wrote:
>>> On 2021-11-10 14:11, Tvrtko Ursulin wrote:
>>>>
>>>> On 10/11/2021 12:35, Lu Baolu wrote:
>>>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>>>
>>>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>>>> Hi Tvrtko,
>>>>>>>>>
>>>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>>>
>>>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>>>> option only
>>>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>>>> reflect its
>>>>>>>>>> status.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>>>> ---
>>>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused
>>>>>>>>>> by both
>>>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>>>> intel_iommu
>>>>>>>>>> driver. But it certainly appears the setup can assign some
>>>>>>>>>> iommu ops (and
>>>>>>>>>> assign the discrete i915 to iommu group) when those two are
>>>>>>>>>> set to off.
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>>> @@ -1763,26 +1763,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();
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> Have you verified this change? I am afraid that
>>>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>>>
>>>>>>>> Yes it seems to work as is:
>>>>>>>>
>>>>>>>> default:
>>>>>>>>
>>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>>
>>>>>>>> intel_iommu=igfx_off:
>>>>>>>>
>>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>>
>>>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>>>
>>>>>>> The drm device 0 has a dedicated iommu. When the user request
>>>>>>> igfx not
>>>>>>> mapped, the VT-d implementation will turn it off to save power.
>>>>>>> But for
>>>>>>> shared iommu, you definitely will get it enabled.
>>>>>>
>>>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>>>> platform with integrated graphics without a dedicated iommu, in
>>>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped
>>>>>> == 0 and iommu_get_domain_for_dev returning non-NULL?
>>>>>
>>>>> Your code always work for an igfx with a dedicated iommu. This
>>>>> might be
>>>>> always true on today's platforms. But from driver's point of view, we
>>>>> should not make such assumption.
>>>>>
>>>>> For example, if the iommu implementation decides not to turn off the
>>>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>>>> virtualization), your code will be broken.
>>>>
>>>> If I got it right, this would go back to your earlier recommendation
>>>> to have the check look like this:
>>>>
>>>> static bool intel_vtd_active(struct drm_i915_private *i915)
>>>> {
>>>> struct iommu_domain *domain;
>>>>
>>>> domain = iommu_get_domain_for_dev(i915->drm.dev);
>>>> if (domain && (domain->type & __IOMMU_DOMAIN_PAGING))
>>>> return true;
>>>> ...
>>>>
>>>> This would be okay as a first step?
>>>>
>>>> Elsewhere in the thread Robin suggested looking at the dec->dma_ops
>>>> and comparing against iommu_dma_ops. These two solution would be
>>>> effectively the same?
>>>
>>> Effectively, yes. See iommu_setup_dma_ops() - the only way to end up
>>> with iommu_dma_ops is if a managed translation domain is present; if
>>> the IOMMU is present but the default domain type has been set to
>>> passthrough (either globally or forced for the given device) it will
>>> do nothing and leave you with dma-direct, while if the IOMMU has been
>>> ignored entirely then it should never even be called. Thus it neatly
>>> encapsulates what you're after here.
>>
>> One concern I have is whether the pass-through mode truly does nothing
>> or addresses perhaps still go through the dmar hardware just with no
>> translation?
>
> Pass-through mode means the latter.
>
>>
>> If latter then most like for like change is actually exactly what the
>> first version of my patch did. That is replace intel_iommu_gfx_mapped
>> with a plain non-NULL check on iommu_get_domain_for_dev.
>
> Depends on what you want here,
>
> #1) the graphic device works in iommu pass-through mode
> - device have an iommu
> - but iommu does no translation
> - the dma transactions go through iommu with the same destination
> memory address specified by the device;
Do you have any indications of the slowdown this adds?
> #2) the graphic device works without a system iommu
> - the iommu is off
> - there's no iommu on the path of DMA transaction.
>
> My suggestion works for #1). Robin's suggestion (device_iommu_mapped())
> could work for #2).
On the question of what do I want here. It seems that to preserve
like-for-like with the current and past i915 usage, ie.
intel_iommu_gfx_mapped, the first version of my patch should be used.
In other words if I configure the boot with iommu=pt, then
intel_iommu_gfx_mapped is true. So if I add the __IOMMU_DOMAIN_PAGING
check, the new intel_vtd_active would return false, where the old
version would return true.
So v1 of the patch feels like the safest route given I don't know which
workarounds are due remapping slowdown, and which may be present even in
the pass-through mode.
I would explain the situation in the comment inside intel_vtd_active for
future reference.
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] drm/i915: Use per device iommu check
2021-11-12 13:40 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-11-25 10:00 ` Tvrtko Ursulin
-1 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-25 10:00 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel, Tvrtko Ursulin
On 12/11/2021 13:40, Tvrtko Ursulin wrote:
>
> On 12/11/2021 00:53, Lu Baolu wrote:
>> On 11/11/21 11:06 PM, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 12:35, Lu Baolu wrote:
>>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>>
>>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>>> Hi Tvrtko,
>>>>>>>>
>>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>>
>>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>>> option only
>>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>>> reflect its
>>>>>>>>> status.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>>> ---
>>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by
>>>>>>>>> both
>>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>>> intel_iommu
>>>>>>>>> driver. But it certainly appears the setup can assign some
>>>>>>>>> iommu ops (and
>>>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>>>> to off.
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> @@ -1763,26 +1763,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();
>>>>>>>> }
>>>>>>>>
>>>>>>>> Have you verified this change? I am afraid that
>>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>>
>>>>>>> Yes it seems to work as is:
>>>>>>>
>>>>>>> default:
>>>>>>>
>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>
>>>>>>> intel_iommu=igfx_off:
>>>>>>>
>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>
>>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>>
>>>>>> The drm device 0 has a dedicated iommu. When the user request igfx
>>>>>> not
>>>>>> mapped, the VT-d implementation will turn it off to save power.
>>>>>> But for
>>>>>> shared iommu, you definitely will get it enabled.
>>>>>
>>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>>> platform with integrated graphics without a dedicated iommu, in
>>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped
>>>>> == 0 and iommu_get_domain_for_dev returning non-NULL?
>>>>
>>>> Your code always work for an igfx with a dedicated iommu. This might be
>>>> always true on today's platforms. But from driver's point of view, we
>>>> should not make such assumption.
>>>>
>>>> For example, if the iommu implementation decides not to turn off the
>>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>>> virtualization), your code will be broken.
>>>
>>> I tried your suggestion (checking for __IOMMU_DOMAIN_PAGING) and it
>>> works better, however I have observed one odd behaviour (for me at
>>> least).
>>>
>>> In short - why does the DMAR mode for the discrete device change
>>> depending on igfx_off parameter?
>>>
>>> Consider the laptop has these two graphics cards:
>>>
>>> # cat /sys/kernel/debug/dri/0/name
>>> i915 dev=0000:00:02.0 unique=0000:00:02.0 # integrated
>>>
>>> # cat /sys/kernel/debug/dri/1/name
>>> i915 dev=0000:03:00.0 unique=0000:03:00.0 # discrete
>>>
>>> Booting with different options:
>>> ===============================
>>>
>>> default / intel_iommu=on
>>> ------------------------
>>>
>>> # cat /sys/class/iommu/dmar0/devices/0000:00:02.0/iommu_group/type
>>> DMA-FQ
>>> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
>>> DMA-FQ
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>
>>> All good.
>>>
>>> intel_iommu=igfx_off
>>> --------------------
>>>
>>> ## no dmar0 in sysfs
>>> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
>>> identity
>>>
>>> Unexpected!?
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled # At least
>>> the i915 patch detects it correctly.
>>>
>>> intel_iommu=off
>>> ---------------
>>>
>>> ## no dmar0 in sysfs
>>> ## no dmar2 in sysfs
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled
>>>
>>> All good.
>>>
>>> The fact discrete graphics changes from translated to pass-through
>>> when igfx_off is set is surprising to me. Is this a bug?
>>
>> The existing VT-d implementation doesn't distinguish igfx from dgfx. It
>> only checks whether the device is of a display class:
>>
>> #define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) ==
>> PCI_BASE_CLASS_DISPLAY)
>>
>> When igfx_off is specified, all graphic devices will put into pass-
>> through (the same meaning as identity mapping) mode. For igfx, since
>> the iommu is always dedicated, hence it further turn off the iommu
>> (hence there's no iommu domain) to save power.
>
> Ah okay. Is this something we want to change/fix?
Ping on this - don't we want to fix igfx_off option to not apply to
discrete GPUs?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Use per device iommu check
@ 2021-11-25 10:00 ` Tvrtko Ursulin
0 siblings, 0 replies; 39+ messages in thread
From: Tvrtko Ursulin @ 2021-11-25 10:00 UTC (permalink / raw)
To: Lu Baolu, Intel-gfx; +Cc: dri-devel
On 12/11/2021 13:40, Tvrtko Ursulin wrote:
>
> On 12/11/2021 00:53, Lu Baolu wrote:
>> On 11/11/21 11:06 PM, Tvrtko Ursulin wrote:
>>>
>>> On 10/11/2021 12:35, Lu Baolu wrote:
>>>> On 2021/11/10 20:08, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 10/11/2021 12:04, Lu Baolu wrote:
>>>>>> On 2021/11/10 17:30, Tvrtko Ursulin wrote:
>>>>>>>
>>>>>>> On 10/11/2021 07:12, Lu Baolu wrote:
>>>>>>>> Hi Tvrtko,
>>>>>>>>
>>>>>>>> On 2021/11/9 20:17, Tvrtko Ursulin wrote:
>>>>>>>>> From: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>>
>>>>>>>>> On igfx + dgfx setups, it appears that intel_iommu=igfx_off
>>>>>>>>> option only
>>>>>>>>> disables the igfx iommu. Stop relying on global
>>>>>>>>> intel_iommu_gfx_mapped
>>>>>>>>> and probe presence of iommu domain per device to accurately
>>>>>>>>> reflect its
>>>>>>>>> status.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
>>>>>>>>> Cc: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>>>>> ---
>>>>>>>>> Baolu, is my understanding here correct? Maybe I am confused by
>>>>>>>>> both
>>>>>>>>> intel_iommu_gfx_mapped and dmar_map_gfx being globals in the
>>>>>>>>> intel_iommu
>>>>>>>>> driver. But it certainly appears the setup can assign some
>>>>>>>>> iommu ops (and
>>>>>>>>> assign the discrete i915 to iommu group) when those two are set
>>>>>>>>> to off.
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> index e967cd08f23e..9fb38a54f1fe 100644
>>>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>>>> @@ -1763,26 +1763,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();
>>>>>>>> }
>>>>>>>>
>>>>>>>> Have you verified this change? I am afraid that
>>>>>>>> iommu_get_domain_for_dev() always gets a valid iommu domain even
>>>>>>>> intel_iommu_gfx_mapped == 0.
>>>>>>>
>>>>>>> Yes it seems to work as is:
>>>>>>>
>>>>>>> default:
>>>>>>>
>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>
>>>>>>> intel_iommu=igfx_off:
>>>>>>>
>>>>>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>>>>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>>>>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>>>>>
>>>>>>> On my system dri device 0 is integrated graphics and 1 is discrete.
>>>>>>
>>>>>> The drm device 0 has a dedicated iommu. When the user request igfx
>>>>>> not
>>>>>> mapped, the VT-d implementation will turn it off to save power.
>>>>>> But for
>>>>>> shared iommu, you definitely will get it enabled.
>>>>>
>>>>> Sorry I am not following, what exactly do you mean? Is there a
>>>>> platform with integrated graphics without a dedicated iommu, in
>>>>> which case intel_iommu=igfx_off results in intel_iommu_gfx_mapped
>>>>> == 0 and iommu_get_domain_for_dev returning non-NULL?
>>>>
>>>> Your code always work for an igfx with a dedicated iommu. This might be
>>>> always true on today's platforms. But from driver's point of view, we
>>>> should not make such assumption.
>>>>
>>>> For example, if the iommu implementation decides not to turn off the
>>>> graphic iommu (perhaps due to some hw quirk or for graphic
>>>> virtualization), your code will be broken.
>>>
>>> I tried your suggestion (checking for __IOMMU_DOMAIN_PAGING) and it
>>> works better, however I have observed one odd behaviour (for me at
>>> least).
>>>
>>> In short - why does the DMAR mode for the discrete device change
>>> depending on igfx_off parameter?
>>>
>>> Consider the laptop has these two graphics cards:
>>>
>>> # cat /sys/kernel/debug/dri/0/name
>>> i915 dev=0000:00:02.0 unique=0000:00:02.0 # integrated
>>>
>>> # cat /sys/kernel/debug/dri/1/name
>>> i915 dev=0000:03:00.0 unique=0000:03:00.0 # discrete
>>>
>>> Booting with different options:
>>> ===============================
>>>
>>> default / intel_iommu=on
>>> ------------------------
>>>
>>> # cat /sys/class/iommu/dmar0/devices/0000:00:02.0/iommu_group/type
>>> DMA-FQ
>>> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
>>> DMA-FQ
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: enabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: enabled
>>>
>>> All good.
>>>
>>> intel_iommu=igfx_off
>>> --------------------
>>>
>>> ## no dmar0 in sysfs
>>> # cat /sys/class/iommu/dmar2/devices/0000:03:00.0/iommu_group/type
>>> identity
>>>
>>> Unexpected!?
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled # At least
>>> the i915 patch detects it correctly.
>>>
>>> intel_iommu=off
>>> ---------------
>>>
>>> ## no dmar0 in sysfs
>>> ## no dmar2 in sysfs
>>>
>>> # grep -i iommu /sys/kernel/debug/dri/*/i915_capabilities
>>> /sys/kernel/debug/dri/0/i915_capabilities:iommu: disabled
>>> /sys/kernel/debug/dri/1/i915_capabilities:iommu: disabled
>>>
>>> All good.
>>>
>>> The fact discrete graphics changes from translated to pass-through
>>> when igfx_off is set is surprising to me. Is this a bug?
>>
>> The existing VT-d implementation doesn't distinguish igfx from dgfx. It
>> only checks whether the device is of a display class:
>>
>> #define IS_GFX_DEVICE(pdev) ((pdev->class >> 16) ==
>> PCI_BASE_CLASS_DISPLAY)
>>
>> When igfx_off is specified, all graphic devices will put into pass-
>> through (the same meaning as identity mapping) mode. For igfx, since
>> the iommu is always dedicated, hence it further turn off the iommu
>> (hence there's no iommu domain) to save power.
>
> Ah okay. Is this something we want to change/fix?
Ping on this - don't we want to fix igfx_off option to not apply to
discrete GPUs?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2021-11-25 10:00 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09 12:17 [PATCH] drm/i915: Use per device iommu check Tvrtko Ursulin
2021-11-09 12:17 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-09 14:02 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2021-11-09 16:23 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2021-11-09 17:19 ` [Intel-gfx] [PATCH] " Lucas De Marchi
2021-11-09 17:35 ` Tvrtko Ursulin
2021-11-10 7:25 ` Lu Baolu
2021-11-10 9:35 ` Tvrtko Ursulin
2021-11-10 12:16 ` Robin Murphy
2021-11-10 12:26 ` Lu Baolu
2021-11-10 7:12 ` Lu Baolu
2021-11-10 7:12 ` Lu Baolu
2021-11-10 9:30 ` Tvrtko Ursulin
2021-11-10 9:30 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-10 12:04 ` Lu Baolu
2021-11-10 12:04 ` [Intel-gfx] " Lu Baolu
2021-11-10 12:08 ` Tvrtko Ursulin
2021-11-10 12:08 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-10 12:35 ` Lu Baolu
2021-11-10 12:35 ` [Intel-gfx] " Lu Baolu
2021-11-10 14:11 ` Tvrtko Ursulin
2021-11-10 14:11 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-10 14:37 ` Robin Murphy
2021-11-10 14:37 ` [Intel-gfx] " Robin Murphy
2021-11-11 15:18 ` Tvrtko Ursulin
2021-11-11 15:18 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-12 0:58 ` Lu Baolu
2021-11-12 0:58 ` [Intel-gfx] " Lu Baolu
2021-11-12 14:10 ` Tvrtko Ursulin
2021-11-12 14:10 ` Tvrtko Ursulin
2021-11-11 15:06 ` Tvrtko Ursulin
2021-11-11 15:06 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-12 0:53 ` Lu Baolu
2021-11-12 0:53 ` [Intel-gfx] " Lu Baolu
2021-11-12 13:40 ` Tvrtko Ursulin
2021-11-12 13:40 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-25 10:00 ` Tvrtko Ursulin
2021-11-25 10:00 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-10 8:00 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Use per device iommu check (rev2) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.