iommu: fix amd_iommu=force_isolation
PATCH 1/1 iommu: fix amd_iommu=force_isolation
The parameter is still there but it's ignored. We need to check its value before deciding to go into passthrough mode for AMD IOMMU.
--- drivers/iommu/amd_iommu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/iommu/amd_iommu.c +2 -1
@@ -2195,7 +2195,8 @@ static int amd_iommu_add_device(struct device *dev)BUG_ON (!dev_data); -if (iommu_pass_through || dev_data->iommu_v2) +if (iommu_pass_through || + (!amd_iommu_force_isolation && dev_data->iommu_v2))iommu_request_dm_for_dev (dev);/* Domains are initialized for this device - have a look what we ended up with */
Signature · 1 line
2.20.0.rc1.387.gf8505762e3-goog
Joerg Roedel
maintainerQuoted 15 lines · Yu Zhao
The parameter is still there but it's ignored. We need to check its value before deciding to go into passthrough mode for AMD IOMMU.
Fixes aafd8ba0ca74 ("iommu/amd: Implement add_device and remove_device")Signed-off-by Yu Zhao <[email protected]>--- drivers/iommu/amd_iommu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/iommu/amd_iommu.c +0
-0
drivers/iommu/amd_iommu.c static int amd_iommu_add_device(struct device *dev) · line 2199
Quoted 6 lines
drivers/iommu/amd_iommu.c +2
-1@@ -2195,7 +2195,8 @@ static int amd_iommu_add_device(struct device *dev)BUG_ON (!dev_data); -if (iommu_pass_through || dev_data->iommu_v2) +if (iommu_pass_through || + (!amd_iommu_force_isolation && dev_data->iommu_v2))
This breaks the iommu_v2 use-case, as it needs a direct mapping for the devices that support it.
I think the force_isolation parameter does not make sense anymore today and should be removed.
Regards,
Joerg
Quoted 17 lines · Joerg Roedel
Quoted 16 lines · Yu Zhao
The parameter is still there but it's ignored. We need to check its value before deciding to go into passthrough mode for AMD IOMMU.
Fixes aafd8ba0ca74 ("iommu/amd: Implement add_device and remove_device")Signed-off-by Yu Zhao <[email protected]>--- drivers/iommu/amd_iommu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/iommu/amd_iommu.c +0
-0@@ -2195,7 +2195,8 @@ static int amd_iommu_add_device(struct device *dev)
Quoted 8 lines
drivers/iommu/amd_iommu.c static int amd_iommu_add_device(struct device *dev) · line 2199
Quoted 5 lines
drivers/iommu/amd_iommu.c +2
-1BUG_ON (!dev_data); -if (iommu_pass_through || dev_data->iommu_v2) +if (iommu_pass_through || + (!amd_iommu_force_isolation && dev_data->iommu_v2))This breaks the
iommu_v2use-case, as it needs a direct mapping for the devices that support it.
Actually this is what we want. We occasionally use this parameter to isolate v2 device when debugging memory corruption that we suspect is caused by DMA. It helped us before. Now with the patch, we caught another v2 device doing DMA write to memory areas that it's not supposed to.
Quoted 2 lines
I think the
force_isolationparameter does not make sense anymore today and should be removed.
Could you please suggest an alternative? Otherwise, we won't be able to force v2 device out of direct mapping for the reason mentioned above.
Thank you.
PATCH v2 1/1 iommu: fix amd_iommu=force_isolation
In reply to Yu ZhaoThe parameter is still there but it's ignored. We need to check its value before deciding to go into passthrough mode for AMD IOMMU v2 capable device.
We occasionally use this parameter to force v2 capable device into translation mode to debug memory corruption that we suspect is caused by DMA writes.
To address the following comment from Joerg Roedel on the first version, v2 capability of device is completely ignored.
Quoted 2 lines
This breaks the
iommu_v2use-case, as it needs a direct mapping for the devices that support it.
And from Documentation/admin-guide/kernel-parameters.txt:
This option does not override iommu=pt
--- drivers/iommu/amd_iommu.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/iommu/amd_iommu.c +8 -1
@@ -438,7 +438,14 @@ static int iommu_init_device(struct device *dev) dev_data->alias =get_alias (dev); -if (dev_is_pci (dev) &&pci_iommuv2_capable (to_pci_dev (dev))) { +/* +* By default we use passthrough mode for IOMMUv2 capable device. +* But if amd_iommu=force_isolation is set (e.g. to debug DMA to +* invalid address), we ignore the capability for the device so +* it'll be forced to go into translation mode. +*/ +if ((iommu_pass_through || !amd_iommu_force_isolation) && +dev_is_pci (dev) &&pci_iommuv2_capable (to_pci_dev (dev))) {struct amd_iommu *iommu; iommu = amd_iommu_rlookup_table[dev_data->devid];
Signature · 1 line
2.20.0.rc2.403.gdbc3b29805-goog
Quoted 6 lines · Yu Zhao
Fixes aafd8ba0ca74 ("iommu/amd: Implement add_device and remove_device")Signed-off-by Yu Zhao <[email protected]>--- drivers/iommu/amd_iommu.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
Applied, thanks.