[PATCH v2 0/2] x86: Get rid of custom DMA functions

[PATCH v2 0/2] x86: Get rid of custom DMA functions

[Nicolas Saenz Julienne]

sta2x11 is the only x86 device that depends custom DMA direct functions.
It turns out it can be made standard by carefully setting the device's
DMA masks and offset.

Originally only patch #2 was sent but I realised patch #1 is also
needed, which is a good addition as it's also a prerequisite to get
proper DMA support on the Raspberry Pi 4[1].

[1] https://lkml.org/lkml/2019/10/15/523

---

Changes since v1:
  - Small cleanups in sta2x11-fixup.x
  - add patch checking DMA addresses lower bounds

Nicolas Saenz Julienne (2):
  dma-direct: check for overflows on 32 bit DMA addresses
  x86/PCI: sta2x11: use default DMA address translation ops

 arch/x86/Kconfig                  |   1 -
 arch/x86/include/asm/device.h     |   3 -
 arch/x86/include/asm/dma-direct.h |   9 --
 arch/x86/pci/sta2x11-fixup.c      | 135 ++++++------------------------
 include/linux/dma-direct.h        |   8 ++
 5 files changed, 34 insertions(+), 122 deletions(-)
 delete mode 100644 arch/x86/include/asm/dma-direct.h

-- 
2.23.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 1/2] dma-direct: check for overflows on 32 bit DMA addresses

[Nicolas Saenz Julienne]

As seen on the new Raspberry Pi 4 and sta2x11's DMA implementation it is
possible for a device configured with 32 bit DMA addresses and a partial
DMA mapping located at the end of the address space to overflow. It
happens when a higher physical address, not DMAable, is translated to
it's DMA counterpart.

For example the Raspberry Pi 4, configurable up to 4 GB of memory, has
an interconnect capable of addressing the lower 1 GB of physical memory
with a DMA offset of 0xc0000000. It transpires that, any attempt to
translate physical addresses higher than the first GB will result in an
overflow which dma_capable() can't detect as it only checks for
addresses bigger then the maximum allowed DMA address.

Fix this by verifying in dma_capable() if the DMA address range provided
is at any point lower than the minimum possible DMA address on the bus.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 include/linux/dma-direct.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index adf993a3bd58..af4bb9c81ccc 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -3,6 +3,7 @@
 #define _LINUX_DMA_DIRECT_H 1
 
 #include <linux/dma-mapping.h>
+#include <linux/memblock.h> /* for min_low_pfn */
 #include <linux/mem_encrypt.h>
 
 #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
@@ -27,6 +28,13 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
 	if (!dev->dma_mask)
 		return false;
 
+#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	/* Check if DMA address overflowed */
+	if (min(addr, addr + size - 1) <
+		__phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
+		return false;
+#endif
+
 	return addr + size - 1 <=
 		min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
 }
-- 
2.23.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 1/2] dma-direct: check for overflows on 32 bit DMA addresses

[Christoph Hellwig]

On Fri, Oct 18, 2019 at 01:00:43PM +0200, Nicolas Saenz Julienne wrote:
> +#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +	/* Check if DMA address overflowed */
> +	if (min(addr, addr + size - 1) <
> +		__phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
> +		return false;
> +#endif

Would be nice to use IS_ENABLED and PFN_PHYS here, and I also think we
need to use phys_to_dma to take care of the encryption bit.  If you then
also introduce an end variable we can make the whole thing actually look
nice:

static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
{
	dma_addr_t end = addr + size - 1;

        if (!dev->dma_mask)
                return false;

	if (!IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) &&
	    min(addr, end) < phys_to_dma(dev, PFN_PHYS(min_low_pfn)))
		return false;

        return end <= min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
}

Otherwise this looks sensible to me.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 1/2] dma-direct: check for overflows on 32 bit DMA addresses

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 1099 bytes --]

On Wed, 2019-10-30 at 14:41 -0700, Christoph Hellwig wrote:
> On Fri, Oct 18, 2019 at 01:00:43PM +0200, Nicolas Saenz Julienne wrote:
> > +#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
> > +	/* Check if DMA address overflowed */
> > +	if (min(addr, addr + size - 1) <
> > +		__phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
> > +		return false;
> > +#endif
> 
> Would be nice to use IS_ENABLED and PFN_PHYS here, and I also think we
> need to use phys_to_dma to take care of the encryption bit.  If you then
> also introduce an end variable we can make the whole thing actually look
> nice:
> 
> static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t
> size)
> {
> 	dma_addr_t end = addr + size - 1;
> 
>         if (!dev->dma_mask)
>                 return false;
> 
> 	if (!IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) &&
> 	    min(addr, end) < phys_to_dma(dev, PFN_PHYS(min_low_pfn)))
> 		return false;
> 
>         return end <= min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
> }
> 
> Otherwise this looks sensible to me.

Thanks, noted.


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu