| Message ID | 20240428032020.214616-1-baolu.lu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] iommu/vt-d: Decouple igfx_off from graphic identity mapping | expand |
> From: Lu Baolu <baolu.lu@linux.intel.com> > Sent: Sunday, April 28, 2024 11:20 AM > > A kernel command called igfx_off was introduced in commit <ba39592764ed> > ("Intel IOMMU: Intel IOMMU driver"). This command allows the user to > disable the IOMMU dedicated to SOC-integrated graphic devices. > > Commit <9452618e7462> ("iommu/intel: disable DMAR for g4x integrated > gfx") > used this mechanism to disable the graphic-dedicated IOMMU for some > problematic devices. Later, more problematic graphic devices were added > to the list by commit <1f76249cc3beb> ("iommu/vt-d: Declare Broadwell igfx > dmar support snafu"). > > On the other hand, commit <19943b0e30b05> ("intel-iommu: Unify > hardware > and software passthrough support") uses the identity domain for graphic > devices if CONFIG_DMAR_BROKEN_GFX_WA is selected. > > + if (iommu_pass_through) > + iommu_identity_mapping = 1; > +#ifdef CONFIG_DMAR_BROKEN_GFX_WA > + else > + iommu_identity_mapping = 2; > +#endif > ... > > static int iommu_should_identity_map(struct pci_dev *pdev, int startup) > { > + if (iommu_identity_mapping == 2) > + return IS_GFX_DEVICE(pdev); > ... > > In the following driver evolution, CONFIG_DMAR_BROKEN_GFX_WA and > quirk_iommu_igfx() are mixed together, causing confusion in the driver's > device_def_domain_type callback. On one hand, dmar_map_gfx is used to > turn > off the graphic-dedicated IOMMU as a workaround for some buggy > hardware; > on the other hand, for those graphic devices, IDENTITY mapping is required > for the IOMMU core. > > Commit <4b8d18c0c986> "iommu/vt-d: Remove > INTEL_IOMMU_BROKEN_GFX_WA" has > removed the CONFIG_DMAR_BROKEN_GFX_WA option, so the > IDENTITY_DOMAIN > requirement for graphic devices is no longer needed. Therefore, this > requirement can be removed from device_def_domain_type() and igfx_off > can > be made independent. > > Fixes: 4b8d18c0c986 ("iommu/vt-d: Remove > INTEL_IOMMU_BROKEN_GFX_WA") > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> > --- > drivers/iommu/intel/iommu.c | 19 ++++++------------- > 1 file changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c > index fbbf8fda22f3..57a986524502 100644 > --- a/drivers/iommu/intel/iommu.c > +++ b/drivers/iommu/intel/iommu.c > @@ -217,12 +217,11 @@ int intel_iommu_sm = > IS_ENABLED(CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON); > int intel_iommu_enabled = 0; > EXPORT_SYMBOL_GPL(intel_iommu_enabled); > > -static int dmar_map_gfx = 1; > static int intel_iommu_superpage = 1; > static int iommu_identity_mapping; > static int iommu_skip_te_disable; > +static int disable_igfx_dedicated_iommu; > what about 'no_igfx_iommu"? dedicated is implied for igfx so a shorter name might be slightly better. otherwise it looks good: Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On 2024/4/28 14:45, Tian, Kevin wrote: >> From: Lu Baolu<baolu.lu@linux.intel.com> >> Sent: Sunday, April 28, 2024 11:20 AM >> >> A kernel command called igfx_off was introduced in commit <ba39592764ed> >> ("Intel IOMMU: Intel IOMMU driver"). This command allows the user to >> disable the IOMMU dedicated to SOC-integrated graphic devices. >> >> Commit <9452618e7462> ("iommu/intel: disable DMAR for g4x integrated >> gfx") >> used this mechanism to disable the graphic-dedicated IOMMU for some >> problematic devices. Later, more problematic graphic devices were added >> to the list by commit <1f76249cc3beb> ("iommu/vt-d: Declare Broadwell igfx >> dmar support snafu"). >> >> On the other hand, commit <19943b0e30b05> ("intel-iommu: Unify >> hardware >> and software passthrough support") uses the identity domain for graphic >> devices if CONFIG_DMAR_BROKEN_GFX_WA is selected. >> >> + if (iommu_pass_through) >> + iommu_identity_mapping = 1; >> +#ifdef CONFIG_DMAR_BROKEN_GFX_WA >> + else >> + iommu_identity_mapping = 2; >> +#endif >> ... >> >> static int iommu_should_identity_map(struct pci_dev *pdev, int startup) >> { >> + if (iommu_identity_mapping == 2) >> + return IS_GFX_DEVICE(pdev); >> ... >> >> In the following driver evolution, CONFIG_DMAR_BROKEN_GFX_WA and >> quirk_iommu_igfx() are mixed together, causing confusion in the driver's >> device_def_domain_type callback. On one hand, dmar_map_gfx is used to >> turn >> off the graphic-dedicated IOMMU as a workaround for some buggy >> hardware; >> on the other hand, for those graphic devices, IDENTITY mapping is required >> for the IOMMU core. >> >> Commit <4b8d18c0c986> "iommu/vt-d: Remove >> INTEL_IOMMU_BROKEN_GFX_WA" has >> removed the CONFIG_DMAR_BROKEN_GFX_WA option, so the >> IDENTITY_DOMAIN >> requirement for graphic devices is no longer needed. Therefore, this >> requirement can be removed from device_def_domain_type() and igfx_off >> can >> be made independent. >> >> Fixes: 4b8d18c0c986 ("iommu/vt-d: Remove >> INTEL_IOMMU_BROKEN_GFX_WA") >> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com> >> --- >> drivers/iommu/intel/iommu.c | 19 ++++++------------- >> 1 file changed, 6 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c >> index fbbf8fda22f3..57a986524502 100644 >> --- a/drivers/iommu/intel/iommu.c >> +++ b/drivers/iommu/intel/iommu.c >> @@ -217,12 +217,11 @@ int intel_iommu_sm = >> IS_ENABLED(CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON); >> int intel_iommu_enabled = 0; >> EXPORT_SYMBOL_GPL(intel_iommu_enabled); >> >> -static int dmar_map_gfx = 1; >> static int intel_iommu_superpage = 1; >> static int iommu_identity_mapping; >> static int iommu_skip_te_disable; >> +static int disable_igfx_dedicated_iommu; >> > what about 'no_igfx_iommu"? dedicated is implied for igfx > so a shorter name might be slightly better. I like disable_igfx_iommu more. :-) Best regards, baolu
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index fbbf8fda22f3..57a986524502 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -217,12 +217,11 @@ int intel_iommu_sm = IS_ENABLED(CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON); int intel_iommu_enabled = 0; EXPORT_SYMBOL_GPL(intel_iommu_enabled); -static int dmar_map_gfx = 1; static int intel_iommu_superpage = 1; static int iommu_identity_mapping; static int iommu_skip_te_disable; +static int disable_igfx_dedicated_iommu; -#define IDENTMAP_GFX 2 #define IDENTMAP_AZALIA 4 const struct iommu_ops intel_iommu_ops; @@ -261,7 +260,7 @@ static int __init intel_iommu_setup(char *str) no_platform_optin = 1; pr_info("IOMMU disabled\n"); } else if (!strncmp(str, "igfx_off", 8)) { - dmar_map_gfx = 0; + disable_igfx_dedicated_iommu = 1; pr_info("Disable GFX device mapping\n"); } else if (!strncmp(str, "forcedac", 8)) { pr_warn("intel_iommu=forcedac deprecated; use iommu.forcedac instead\n"); @@ -2196,9 +2195,6 @@ static int device_def_domain_type(struct device *dev) if ((iommu_identity_mapping & IDENTMAP_AZALIA) && IS_AZALIA(pdev)) return IOMMU_DOMAIN_IDENTITY; - - if ((iommu_identity_mapping & IDENTMAP_GFX) && IS_GFX_DEVICE(pdev)) - return IOMMU_DOMAIN_IDENTITY; } return 0; @@ -2499,9 +2495,6 @@ static int __init init_dmars(void) iommu_set_root_entry(iommu); } - if (!dmar_map_gfx) - iommu_identity_mapping |= IDENTMAP_GFX; - check_tylersburg_isoch(); ret = si_domain_init(hw_pass_through); @@ -2592,7 +2585,7 @@ static void __init init_no_remapping_devices(void) /* This IOMMU has *only* gfx devices. Either bypass it or set the gfx_mapped flag, as appropriate */ drhd->gfx_dedicated = 1; - if (!dmar_map_gfx) + if (disable_igfx_dedicated_iommu) drhd->ignored = 1; } } @@ -4621,7 +4614,7 @@ static void quirk_iommu_igfx(struct pci_dev *dev) return; pci_info(dev, "Disabling IOMMU for graphics on this chipset\n"); - dmar_map_gfx = 0; + disable_igfx_dedicated_iommu = 1; } /* G4x/GM45 integrated gfx dmar support is totally busted. */ @@ -4702,8 +4695,8 @@ static void quirk_calpella_no_shadow_gtt(struct pci_dev *dev) if (!(ggc & GGC_MEMORY_VT_ENABLED)) { pci_info(dev, "BIOS has allocated no shadow GTT; disabling IOMMU for graphics\n"); - dmar_map_gfx = 0; - } else if (dmar_map_gfx) { + disable_igfx_dedicated_iommu = 1; + } else if (!disable_igfx_dedicated_iommu) { /* we have to ensure the gfx device is idle before we flush */ pci_info(dev, "Disabling batched IOTLB flush on Ironlake\n"); iommu_set_dma_strict();
A kernel command called igfx_off was introduced in commit <ba39592764ed> ("Intel IOMMU: Intel IOMMU driver"). This command allows the user to disable the IOMMU dedicated to SOC-integrated graphic devices. Commit <9452618e7462> ("iommu/intel: disable DMAR for g4x integrated gfx") used this mechanism to disable the graphic-dedicated IOMMU for some problematic devices. Later, more problematic graphic devices were added to the list by commit <1f76249cc3beb> ("iommu/vt-d: Declare Broadwell igfx dmar support snafu"). On the other hand, commit <19943b0e30b05> ("intel-iommu: Unify hardware and software passthrough support") uses the identity domain for graphic devices if CONFIG_DMAR_BROKEN_GFX_WA is selected. + if (iommu_pass_through) + iommu_identity_mapping = 1; +#ifdef CONFIG_DMAR_BROKEN_GFX_WA + else + iommu_identity_mapping = 2; +#endif ... static int iommu_should_identity_map(struct pci_dev *pdev, int startup) { + if (iommu_identity_mapping == 2) + return IS_GFX_DEVICE(pdev); ... In the following driver evolution, CONFIG_DMAR_BROKEN_GFX_WA and quirk_iommu_igfx() are mixed together, causing confusion in the driver's device_def_domain_type callback. On one hand, dmar_map_gfx is used to turn off the graphic-dedicated IOMMU as a workaround for some buggy hardware; on the other hand, for those graphic devices, IDENTITY mapping is required for the IOMMU core. Commit <4b8d18c0c986> "iommu/vt-d: Remove INTEL_IOMMU_BROKEN_GFX_WA" has removed the CONFIG_DMAR_BROKEN_GFX_WA option, so the IDENTITY_DOMAIN requirement for graphic devices is no longer needed. Therefore, this requirement can be removed from device_def_domain_type() and igfx_off can be made independent. Fixes: 4b8d18c0c986 ("iommu/vt-d: Remove INTEL_IOMMU_BROKEN_GFX_WA") Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> --- drivers/iommu/intel/iommu.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)