Re: allow larger than require DMA masks

[James Bottomley]

On Mon, 2019-09-23 at 23:14 +0200, Sven Schnelle wrote:
> Hi,
> 
> On Fri, Feb 15, 2019 at 03:45:54PM +0100, Christoph Hellwig wrote:
> > Hi all,
> > 
> > this series finishes off converting our dma mask model to split
> > between device capabilities (dev->dma_mask and dev-
> > >coherent_dma_mask) and system limitations (dev->bus_dma_mask).  We 
> > already accept larger than required masks in most dma_map_ops
> > implementation, in case of x86 and implementations based on it
> > since the dawn of time.  Only one parisc and two sparc64 instances
> > failed larger than required DMA masks, and this series fixes that
> > up and updates the documentation that devices don't need to handle
> > DMA mask fallbacks.
> > 
> 
> I just tried latest linux-5.4 git on my hp c8000 (parisc), and got
> the following
> error:
> 
> [   27.246866] sata_sil24 0000:00:01.0: Applying completion IRQ loss
> on PCI-X errata fix
> [   27.336968] sata_sil24 0000:00:01.0: DMA enable failed
> [   27.476922] sata_sil24: probe of 0000:00:01.0 failed with error -5
> 
> This is caused by commit dcc02c19cc06bd7bc1b6db0aa0087a2b6eb05b94:
> Author: Christoph Hellwig <hch@xxxxxx>
> Date:   Mon Aug 26 12:57:24 2019 +0200
> 
>     sata_sil24: use dma_set_mask_and_coherent
> 
>     Use the dma_set_mask_and_coherent helper to set the DMA mask.
> Rely
>     on the relatively recent change that setting a larger than
> required
>     mask will never fail to avoid the need for the boilerplate 32-bit
>     fallback code.
> 
>     Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>     Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
> 
> However, the real problem seems to be in sba_dma_supported():
> 
> Â       /* Documentation/DMA-API-HOWTO.txt tells drivers to try 
> Â        * first, then fall back to 32-bit if that fails.
> Â        * We are just "encouraging" 32-bit DMA masks here since we
> can
> Â        * never allow IOMMU bypass unless we add special support for
> ZX1.
> Â        */
> 	if (mask > ~0U)
> Â     Â       return 0;
> 
> Removing the if() makes the DMA mapping work. It's almost midnight
> here, so i won't look into that any further today. Does anyone have
> an opinion on this behaviour? Otherwise i will look a bit more into
> this in the next days.

The reason for the if was to kick the device into 32 bit descriptors,
which are usually more efficient, especially with older dual descriptor
format cards like we have on parisc systems.

Nothing should go wrong if we don't fail the larger mask request except
that the card uses the inefficient descriptors whereas we'll only
supply it with 32 bit addresses.

James