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 <[email protected]>
---
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;
-#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();
--
2.34.1
> From: Lu Baolu <[email protected]>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
On 2024/4/28 14:45, Tian, Kevin wrote:
>> From: Lu Baolu<[email protected]>
>> 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<[email protected]>
>> ---
>> 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