On Mon, May 19, 2014 at 05:55:56PM +0100, Arnd Bergmann wrote: > On Monday 19 May 2014 16:56:08 Catalin Marinas wrote: > > On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote: > > > On Monday 19 May 2014 10:03:40 Catalin Marinas wrote: > > > > On Mon, May 19, 2014 at 09:32:43AM +0100, Arnd Bergmann wrote: > > > > > The more important question is what happens to high buffers allocated elsewhere > > > > > that get passed into dma_map_sg by a device driver. Depending on the DT properties > > > > > of the device and its parents, this needs to do one of three things: > > > > > > > > > > a) translate the 64-bit virtual address into a 64-bit bus address > > > > > b) create an IOMMU entry for the 64-bit address and pass the 32-bit IOMMU > > > > > address to the driver > > > > > c) use the swiotlb code to create a bounce buffer at a 32-bit DMA address > > > > > and copy the data around > > > > > > > > > > It's definitely wrong to just hardcode a DMA mask in the driver because that > > > > > code doesn't know which of the three cases is being used. Moreover, you can't > > > > > do it using an #ifdef CONFIG_ARM64, because it's completely independent of > > > > > the architecture, and we need to do the exact same logic on ARM32 and any > > > > > other architecture. > > > > > > > > I agree. > > > > > > > > The problem we currently have is system topology description to pass the > > > > DMA mask and in a hierarchical way. I can see Santosh's patches > > > > introducing dma-ranges but the coherent dma mask still set as 32-bit. We > > > > can use the dma-ranges to infer a mask but that's only specific to the > > > > device and the driver doesn't know whether it goes through an iommu or > > > > not. > > > > > > We definitely have to fix this very quickly, before people start building > > > real arm64 systems and shipping them. > > > > > > We should not merge any hacks that attempt to work around the problem, > > > but try to come to a conclusion how to handle them properly. > > > My hope was that we could just always set the dma mask to whatever > > > the DT says it should be to keep the burden from device drivers, > > > unless they want to restrict it further (e.g. when the specific > > > peripheral hardware has a bug that prevents us from using high addresses, > > > even though the SoC in theory supports it everywhere). > > > > I agree. > > > > > Rob Herring argued that we should always mimic PCI and call dma_set_mask() > > > in drivers but default to a 32-bit mask otherwise, independent of whether > > > the hardware can do more or less than that, IIRC. > > > > Can we not default to something built up from dma-ranges? Or 32-bit if > > dma-ranges property is missing? > > We probably want to default to 32-bit for arm32 in the absence of dma-ranges. > For arm64, I'd prefer if we could always mandate dma-ranges to be present > for each bus, just like we mandate ranges to be present. > I hope it's not too late for that. > > dma_set_mask should definitely look at the dma-ranges properties, and the > helper that Santosh just introduced should give us all the information > we need. We just need to decide on the correct behavior. Last time I looked at Santosh's patches I thought the dma-ranges is per device rather than per bus. We could make it per bus only and let the device call dma_set_mask() explicitly if it wants to restrict it further. > > > While we currently don't have a set of swiotlb DMA ops on ARM32, we do > > > have it on ARM64, and I think we should be using them properly. It should > > > really not be hard to implement a proper dma_set_mask() function for > > > ARM64 that gets is able to set up the swiotlb based on the dma-ranges > > > properties and always returns success but leaves the mask unchanged. > > > > The swiotlb bounce buffer needs to be pre-allocated at boot, otherwise > > we don't have any guarantees. Since we can't honour random masks anyway, > > we stick to ZONE_DMA which is currently in the 4G limit. But the driver > > calls dma_set_mask() too late for any further swiotlb setup. > > > > With IOMMU we can be more flexible around dma_set_mask(), can be done at > > run-time. > > What we can do with swiotlb is to check if the mask is smaller than ZONE_DMA. > If it ever is, we have to fail dma_set_mask and hope the driver can fall > back to PIO mode or it will have to fail its probe() function. dma_set_(coherent_)mask check swiotlb_dma_supported() which returns false if io_tlb_end goes beyond the device mask. So we just need to ensure that io_tlb is allocated within ZONE_DMA. > For dma_set_coherent_mask(), we also have to fail any call that tries to > set a mask larger than what the device hardware can do. Unlike that, > dma_set_mask() can succeed with any mask, we just have to enable swiotlb > if the mask that the driver wants is larger than what the hardware can > do. Currently we can't satisfy any arbitrarily small dma mask even with swiotlb since the bounce buffer is just guaranteed to be in ZONE_DMA. Swiotlb allows for smaller masks but we need to reserve the io_tlb buffer early during boot and at smaller addresses. For example, swiotlb_alloc_coherent() first tries __get_free_pages(GFP_DMA) and if the coherent_dma_mask isn't matched, it frees the pages and falls back to the io_tlb buffer. However, I don't think it's worth going for masks smaller than 32-bit on arm64. CMA is pretty similar to swiotlb with regards to pre-allocated buffers for coherent dma. We currently don't limit it for arm64 but I think we should just limit it to ZONE_DMA because we can't tell what masks the devices need. We could parse the DT for dma-ranges but we can still have explicit dma_set_coherent_mask() calls to make it smaller. Yet another issue is what we actually mean by ZONE_DMA. If we have devices with different dma_pfn_offset (as per Santosh's patches), ZONE_DMA would mean different things for them since phys_to_dma() may no longer be the same for a single SoC. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
