On 11/01/17 07:59, Nikita Yushchenko wrote: >>> + /* >>> + * we don't yet support buses that have a non-zero mapping. >>> + * Let's hope we won't need it >>> + */ >>> + WARN_ON(dma_base != 0); >> >> I believe we now accomodate the bus remap bits on BCM2837 as a DMA >> offset, so unfortunately I think this is no longer true. > > Arnd, this check is from you. Any updates? Perhaps this check can be > just dropped? > > In swiotlb code, dma address (i.e. with offset already applied) is > checked against mask. Not sure what 'dma_base' means in iommu case. > >>> + /* >>> + * Whatever the parent bus can set. A device must not set >>> + * a DMA mask larger than this. >>> + */ >>> + dev->archdata.parent_dma_mask = size - 1; >> >> This will effectively constrain *all* DMA masks to be 32-bit, since for >> 99% of devices we're going to see a size derived from the default mask >> passed in here. I worry that that's liable to lead to performance and >> stability regressions > > That was exactly my concern when I first tried to address this issue. My > first attempt was to alter very locally exact configuration where > problem shows, while ensuring that everything else stays as is. See > https://lkml.org/lkml/2016/12/29/218 > > But looks like people want a generic solution. > >> I reckon the easiest way forward would be to pass in some flag to >> arch_setup_dma_ops to indicate whether it's an explicitly-configured >> range or not - then simply initialising parent_dma_mask to ~0 for the >> default case *should* keep things working as before. > > Currently only arm, arm64 and mips define arch_setup_dma_ops(). > Mips version only checks 'coherent' argument, 'size' is used only by arm > and arm64. > > Maybe move setting the default from caller to callee? > I.e. pass size=0 if no explicit information exists, and let architecture > handle that? Yes, I think that ought to work, although the __iommu_setup_dma_ops() call will still want a real size reflecting the default mask, so something like: if (size == 0) { dev->archdata.parent_dma_mask = ~0; size = 1ULL << 32; } else { dev->archdata.parent_dma_mask = size - 1; } should probably do the trick. Robin.
