Stop losing firmware-set DMA masks

These patches attempt to deal with this in the simplest way possible by generalising the specific quirk for 32-bit bridges into an arbitrary mask which can then also be plumbed into the firmware code. In the interest of being minimally invasive, I've only included a point fix for the IOMMU issue as seen on arm64 - there may be further tweaks needed in DMA ops to catch all possible incarnations of this problem, but this initial RFC is mostly about the impact beyond the dma-mapping subsystem itself.

Robin.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/580804.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/474443.html

Robin Murphy (4):
dma-mapping: Generalise dma_32bit_limit flag
ACPI/IORT: Set bus DMA mask as appropriate
of/device: Set bus DMA mask as appropriate
iommu/dma: Respect bus DMA limit for IOVAs

 arch/x86/kernel/pci-dma.c | 2 +-
 drivers/acpi/arm64/iort.c | 1 +
 drivers/iommu/dma-iommu.c | 3 +++
 drivers/of/device.c       | 1 +
 include/linux/device.h    | 6 +++---
 kernel/dma/direct.c       | 2 +-
 6 files changed, 10 insertions(+), 5 deletions(-)

Signature · 1 line

2.17.1.dirty

When an explicit DMA limit is described by firmware, we need to remember it regardless of how drivers might subsequently update their devices' masks. The new bus_dma_mask field does that.

Quoted 8 lines

These patches attempt to deal with this in the simplest way possible by generalising the specific quirk for 32-bit bridges into an arbitrary mask which can then also be plumbed into the firmware code. In the interest of being minimally invasive, I've only included a point fix for the IOMMU issue as seen on arm64 - there may be further tweaks needed in DMA ops to catch all possible incarnations of this problem, but this initial RFC is mostly about the impact beyond the dma-mapping subsystem itself.

Thanks, this looks very nice to me.

In fact it probably solves the RISC-V/Xiling problem as well if we can just add the dma-ranges property to the device tree for the affected systems. Palmer, do you know how easily the DT could be updated for that case?

Quoted 8 lines

These patches attempt to deal with this in the simplest way possible by generalising the specific quirk for 32-bit bridges into an arbitrary mask which can then also be plumbed into the firmware code. In the interest of being minimally invasive, I've only included a point fix for the IOMMU issue as seen on arm64 - there may be further tweaks needed in DMA ops to catch all possible incarnations of this problem, but this initial RFC is mostly about the impact beyond the dma-mapping subsystem itself.

Thanks, this looks very nice to me.

In fact it probably solves the RISC-V/Xiling problem as well if we can just add the dma-ranges property to the device tree for the affected systems. Palmer, do you know how easily the DT could be updated for that case?

Quoted 13 lines

Robin.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/580804.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/474443.html

Robin Murphy (4):
dma-mapping: Generalise dma_32bit_limit flag
ACPI/IORT: Set bus DMA mask as appropriate
of/device: Set bus DMA mask as appropriate
iommu/dma: Respect bus DMA limit for IOVAs

Quoted 10 lines

  arch/x86/kernel/pci-dma.c | 2 +-
  drivers/acpi/arm64/iort.c | 1 +
  drivers/iommu/dma-iommu.c | 3 +++
  drivers/of/device.c       | 1 +
  include/linux/device.h    | 6 +++---
  kernel/dma/direct.c       | 2 +-
  6 files changed, 10 insertions(+), 5 deletions(-)

Signature · 1 line

2.17.1.dirty

---end quoted text---

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Whilst the common firmware code invoked by dma_configure() initialises devices' DMA masks according to limitations described by the respective properties ("dma-ranges" for OF and _DMA/IORT for ACPI), the nature of the dma_set_mask() API leads to that information getting lost when well-behaved drivers probe and set a 64-bit mask, since in general there's no way to tell the difference between a firmware-described mask (which should be respected) and whatever default may have come from the bus code (which should be replaced outright). This can break DMA on systems with certain IOMMU topologies (e.g. [1]) where the IOMMU driver only knows its maximum supported address size, not how many of those address bits might actually be wired up between any of its input interfaces and the associated DMA master devices. Similarly, some PCIe root complexes only have a 32-bit native interface on their host bridge, which leads to the same DMA-address-truncation problem in systems with a larger physical memory map and RAM above 4GB (e.g. [2]).

These patches attempt to deal with this in the simplest way possible by generalising the specific quirk for 32-bit bridges into an arbitrary mask which can then also be plumbed into the firmware code. In the interest of being minimally invasive, I've only included a point fix for the IOMMU issue as seen on arm64 - there may be further tweaks needed in DMA ops to catch all possible incarnations of this problem, but this initial RFC is mostly about the impact beyond the dma-mapping subsystem itself.

Quoted 17 lines · Robin Murphy

Whilst the common firmware code invoked by dma_configure() initialises devices' DMA masks according to limitations described by the respective properties ("dma-ranges" for OF and _DMA/IORT for ACPI), the nature of the dma_set_mask() API leads to that information getting lost when well-behaved drivers probe and set a 64-bit mask, since in general there's no way to tell the difference between a firmware-described mask (which should be respected) and whatever default may have come from the bus code (which should be replaced outright). This can break DMA on systems with certain IOMMU topologies (e.g. [1]) where the IOMMU driver only knows its maximum supported address size, not how many of those address bits might actually be wired up between any of its input interfaces and the associated DMA master devices. Similarly, some PCIe root complexes only have a 32-bit native interface on their host bridge, which leads to the same DMA-address-truncation problem in systems with a larger physical memory map and RAM above 4GB (e.g. [2]).

Quoted 8 lines

These patches attempt to deal with this in the simplest way possible by generalising the specific quirk for 32-bit bridges into an arbitrary mask which can then also be plumbed into the firmware code. In the interest of being minimally invasive, I've only included a point fix for the IOMMU issue as seen on arm64 - there may be further tweaks needed in DMA ops to catch all possible incarnations of this problem, but this initial RFC is mostly about the impact beyond the dma-mapping subsystem itself.

Couldn't you set and use the device's parent's dma_mask instead. At least for DT, we should always have a parent device representing the bus. That would avoid further bloating of struct device.

kernel/dma/direct.c int dma_direct_supported(struct device *dev, u64 mask) · line 187

Quoted 11 lines · Robin Murphy

kernel/dma/direct.c +1 -1

index 8be8106270c2..95e185347e34 100644
@@ -183,7 +183,7 @@ int dma_direct_supported(struct device *dev, u64 mask)
  	 * Various PCI/PCIe bridges have broken support for > 32bit DMA even
  	 * if the device itself might support it.
  	 */
-	if (dev->dma_32bit_limit && mask > DMA_BIT_MASK(32))
+	if (dev->bus_dma_mask && mask > dev->bus_dma_mask)
  		return 0;

The comment above this check needs an updated (or just be removed).

Also we still have a few architectures not using dma-direct. I guess most were doing fine without such limits anyway, but at least arm will probably need an equivalent check.

Quoted 3 lines · Robin Murphy

When an explicit DMA limit is described by firmware, we need to remember it regardless of how drivers might subsequently update their devices' masks. The new bus_dma_mask field does that.

Shouldn't we also stop presetting the dma mask after this?

Indeed, once we've found an approach that everyone's happy with we can have a more thorough audit of exactly where else it needs to be applied. FWIW I'm not aware of any 32-bit Arm systems affected by this*, but if they do exist then at least there's no risk of regression since they've always been busted.