lkml.org 
[lkml]   [2019]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold
On Tue, May 21, 2019 at 02:47:28PM +0200, Christoph Hellwig wrote:
> The dma masks in struct device are always 64-bits wide. But for builds
> using a 32-bit dma_addr_t we need to ensure we don't store an
> unsupportable value. Before Linux 5.0 this was handled at least by
> the ARM dma mapping code by never allowing to set a larger dma_mask,
> but these days we allow the driver to just set the largest supported
> value and never fall back to a smaller one. Ensure this always works
> by truncating the value.

So how does the driver negotiation for >32bit addresses work if we don't
fail for large masks?

I'm thinking about all those PCI drivers that need DAC cycles for >32bit
addresses, such as e1000, which negotiate via (eg):

/* there is a workaround being applied below that limits
* 64-bit DMA addresses to 64-bit hardware. There are some
* 32-bit adapters that Tx hang when given 64-bit DMA addresses
*/
pci_using_dac = 0;
if ((hw->bus_type == e1000_bus_type_pcix) &&
!dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
pci_using_dac = 1;
} else {
err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
if (err) {
pr_err("No usable DMA config, aborting\n");
goto err_dma;
}
}

and similar. If we blindly trunate the 64-bit to 32-bit, aren't we
going to end up with PCI cards using DAC cycles to a host bridge that
do not support DAC cycles?

>
> Fixes: 9eb9e96e97b3 ("Documentation/DMA-API-HOWTO: update dma_mask sections")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> kernel/dma/mapping.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index f7afdadb6770..1f628e7ac709 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -317,6 +317,12 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
>
> int dma_set_mask(struct device *dev, u64 mask)
> {
> + /*
> + * Truncate the mask to the actually supported dma_addr_t width to
> + * avoid generating unsupportable addresses.
> + */
> + mask = (dma_addr_t)mask;
> +
> if (!dev->dma_mask || !dma_supported(dev, mask))
> return -EIO;
>
> @@ -330,6 +336,12 @@ EXPORT_SYMBOL(dma_set_mask);
> #ifndef CONFIG_ARCH_HAS_DMA_SET_COHERENT_MASK
> int dma_set_coherent_mask(struct device *dev, u64 mask)
> {
> + /*
> + * Truncate the mask to the actually supported dma_addr_t width to
> + * avoid generating unsupportable addresses.
> + */
> + mask = (dma_addr_t)mask;
> +
> if (!dma_supported(dev, mask))
> return -EIO;
>
> --
> 2.20.1
>
>

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

\
 
 \ /
  Last update: 2019-05-21 15:05    [W:0.064 / U:5.828 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site