Messages in this thread | | | From | Darren Kenny <> | Subject | Re: [RFC] iommu/vt-d: set default value of INTEL_IOMMU_FLOPPY_WA to n | Date | Fri, 11 Nov 2022 15:09:33 +0000 |
| |
On Friday, 2022-11-11 at 12:36:48 UTC, Robin Murphy wrote: > 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@oracle.com> >>>>>>>>> Signed-off-by: Harshit Mogalapalli >>>>>>>>> <harshit.m.mogalapalli@oracle.com> >>>>>>>>> --- >>>>>>>>> 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. >
Based on what you're saying then, the fix that you agree with is what Baolu first suggested, which is to remove it from Kconfig and just remove the #ifdef/#endif, but not the code that it contains.
Is that correct?
Thanks,
Darren.
| |