Re: [RFC] iommu/vt-d: set default value of INTEL_IOMMU_FLOPPY_WA to n
From: Robin Murphy
Date: Fri Nov 11 2022 - 07:36:59 EST
On 2022-11-11 02:31, Baolu Lu wrote:
On 2022/11/11 5:00, Konrad Rzeszutek Wilk wrote:
On Thu, Nov 10, 2022 at 02:39:53PM +0800, Baolu Lu wrote:
On 2022/11/10 4:17, Konrad Rzeszutek Wilk wrote:
On Wed, Nov 09, 2022 at 09:16:53PM +0800, Baolu Lu wrote:
On 2022/11/9 20:16, Harshit Mogalapalli wrote:
On 09/11/22 12:35 pm, Baolu Lu wrote:
On 2022/11/8 20:58, Harshit Mogalapalli wrote:
It is likely that modern intel motherboard will not ship with a
floppy connection anymore, so let us disable it by default, as it
gets turned on when we do a make defconfig.
Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Signed-off-by: Harshit Mogalapalli
<harshit.m.mogalapalli@xxxxxxxxxx>
---
drivers/iommu/intel/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/Kconfig
b/drivers/iommu/intel/Kconfig
index b7dff5092fd2..c783ae85ca9b 100644
--- a/drivers/iommu/intel/Kconfig
+++ b/drivers/iommu/intel/Kconfig
@@ -76,7 +76,7 @@ config INTEL_IOMMU_BROKEN_GFX_WA
option is removed in the 2.6.32 kernel.
config INTEL_IOMMU_FLOPPY_WA
- def_bool y
+ def_bool n
depends on X86
help
Floppy disk drivers are known to bypass DMA API calls
Nobody selects or depends on this. How about removing this bool?
Only
less than 10 lines of code are impacted and are not in any
performance
path.
diff --git a/drivers/iommu/intel/Kconfig
b/drivers/iommu/intel/Kconfig
index b7dff5092fd2..5e077d1c5f5d 100644
--- a/drivers/iommu/intel/Kconfig
+++ b/drivers/iommu/intel/Kconfig
@@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA
to use physical addresses for DMA, at least until this
option is removed in the 2.6.32 kernel.
-config INTEL_IOMMU_FLOPPY_WA
- def_bool y
- depends on X86
- help
- Floppy disk drivers are known to bypass DMA API calls
- thereby failing to work when IOMMU is enabled. This
- workaround will setup a 1:1 mapping for the first
- 16MiB to make floppy (an ISA device) work.
-
config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON
bool "Enable Intel IOMMU scalable mode by default"
default y
diff --git a/drivers/iommu/intel/iommu.c
b/drivers/iommu/intel/iommu.c
index 48cdcd0a5cf3..22801850f339 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4567,7 +4567,6 @@ static void
intel_iommu_get_resv_regions(struct device *device,
}
rcu_read_unlock();
-#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
if (dev_is_pci(device)) {
struct pci_dev *pdev = to_pci_dev(device);
@@ -4579,7 +4578,6 @@ static void
intel_iommu_get_resv_regions(struct device *device,
list_add_tail(®->list, head);
}
}
-#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */
reg = iommu_alloc_resv_region(IOAPIC_RANGE_START,
IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1,
Hi Baolu,
I have a question:
Shouldn't we remove the code between ifdef-endif statements?
I mean something like this:
diff --git a/drivers/iommu/intel/Kconfig
b/drivers/iommu/intel/Kconfig
index b7dff5092fd2..5e077d1c5f5d 100644
--- a/drivers/iommu/intel/Kconfig
+++ b/drivers/iommu/intel/Kconfig
@@ -75,15 +75,6 @@ config INTEL_IOMMU_BROKEN_GFX_WA
to use physical addresses for DMA, at least until this
option is removed in the 2.6.32 kernel.
-config INTEL_IOMMU_FLOPPY_WA
- def_bool y
- depends on X86
- help
- Floppy disk drivers are known to bypass DMA API calls
- thereby failing to work when IOMMU is enabled. This
- workaround will setup a 1:1 mapping for the first
- 16MiB to make floppy (an ISA device) work.
-
config INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON
bool "Enable Intel IOMMU scalable mode by default"
default y
diff --git a/drivers/iommu/intel/iommu.c
b/drivers/iommu/intel/iommu.c
index 48cdcd0a5cf3..2c416ad3204e 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4567,20 +4567,6 @@ static void
intel_iommu_get_resv_regions(struct
device *device,
}
rcu_read_unlock();
-#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
- if (dev_is_pci(device)) {
- struct pci_dev *pdev = to_pci_dev(device);
-
- if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
- reg = iommu_alloc_resv_region(0, 1UL <<
24, prot,
- IOMMU_RESV_DIRECT_RELAXABLE,
- GFP_KERNEL);
- if (reg)
- list_add_tail(®->list, head);
- }
- }
-#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */
-
reg = iommu_alloc_resv_region(IOAPIC_RANGE_START,
IOAPIC_RANGE_END -
IOAPIC_RANGE_START + 1,
0, IOMMU_RESV_MSI,
GFP_KERNEL);
This code is introduced in Commit d850c2ee5fe2 ("iommu/vt-d:
Expose ISA
direct mapping region via iommu_get_resv_regions")
As long as floppy driver exists in the tree, we have to include above
code. Otherwise, floppy drivers don't work. At least we can easily
find
drivers/block/floppy.c which is still maintained (check
MAINTAINERS).:-)
But this requires a machine with Intel IOMMU and ISA:
- 16MiB to make floppy (an ISA device) work.
ISA device? I don't believe there are any Intel machines with an IOMMU
and an ISA device?
This workaround was introduced by commit 49a0429e53f2 ("Intel IOMMU:
Iommu floppy workaround") in 2007. I can't remember what happened 15
years ago, but I believe there must have been corresponding hardware
configurations at that time, and the Linux kernel has been maintained it
to now.
At what point can this be removed then?
No floppy block drivers in the tree or all floppy drivers' DMA going
through the kernel DMA APIs.
Presumably the point of the config is that ISA bridges aren't expected
on IA-64, so a tiny code saving can be made there, but at this point is
anyone really that bothered about any more? There's already tons more
code all through the driver to support newer features that aren't
meaningful to old IA-64 hardware, so in my opinion, meh. Given that this
already won't affect systems that truly can't have any ISA devices, I
don't see any issue with simply making it unconditional.
Note that plenty of chipsets still have an "ISA" bridge to an LPC
interface, so ISA DMA is not necessarily as dead as one might like to think.
Robin.