Hi Robin, On 06.06.2022 14:38, Robin Murphy wrote: > On 2022-06-02 16:58, Marek Szyprowski wrote: >> On 23.05.2022 19:30, Robin Murphy wrote: >>> On 2022-05-11 13:15, Ajay Kumar wrote: >>>> From: Marek Szyprowski <m.szyprow...@samsung.com> >>>> >>>> Zero is a valid DMA and IOVA address on many architectures, so adjust >>>> the >>>> IOVA management code to properly handle it. A new value IOVA_BAD_ADDR >>>> (~0UL) is introduced as a generic value for the error case. Adjust all >>>> callers of the alloc_iova_fast() function for the new return value. >>> >>> And when does anything actually need this? In fact if you were to stop >>> iommu-dma from reserving IOVA 0 - which you don't - it would only show >>> how patch #3 is broken. >> >> I don't get why you says that patch #3 is broken. > > My mistake, in fact it's already just as broken either way - I forgot > that that's done implicitly via iovad->start_pfn rather than an actual > reserve_iova() entry. I see there is an initial calculation attempting > to honour start_pfn in patch #3, but it's always immediately > overwritten. More critically, the rb_first()/rb_next walk means the > starting point can only creep inevitably upwards as older allocations > are freed, so will end up pathologically stuck at or above limit_pfn > and unable to recover. At best it's more "next-fit" than "first-fit", > and TBH the fact that it could ever even allocate anything at all in > an empty domain makes me want to change the definition of IOVA_ANCHOR ;) > >> Them main issue with >> address 0 being reserved appears when one uses first fit allocation >> algorithm. In such case the first allocation will be at the 0 address, >> what causes some issues. This patch simply makes the whole iova related >> code capable of such case. This mimics the similar code already used on >> ARM 32bit. There are drivers that rely on the way the IOVAs are >> allocated. This is especially important for the drivers for devices with >> limited addressing capabilities. And there exists such and they can be >> even behind the IOMMU. >> >> Lets focus on the s5p-mfc driver and the way one of the supported >> hardware does the DMA. The hardware has very limited DMA window (256M) >> and addresses all memory buffers as an offset from the firmware base. >> Currently it has been assumed that the first allocated buffer will be on >> the beginning of the IOVA space. This worked fine on ARM 32bit and >> supporting such case is a target of this patchset. > > I still understand the use-case; I object to a solution where PFN 0 is > always reserved yet sometimes allocated. Not to mention the slightly > more fundamental thing of the whole lot not actually working the way > it's supposed to. > > I also remain opposed to baking in secret ABIs where allocators are > expected to honour a particular undocumented behaviour. FWIW I've had > plans for a while now to make the IOVA walk bidirectional to make the > existing retry case more efficient, at which point it's then almost > trivial to implement "bottom-up" allocation, which I'm thinking I > might then force on by default for CONFIG_ARM to minimise any further > surprises for v2 of the default domain conversion. However I'm > increasingly less convinced that it's really sufficient for your case, > and am leaning back towards the opinion that any driver with really > specific constraints on how its DMA addresses are laid out should not > be passively relying on a generic allocator to do exactly what it > wants. A simple flag saying "try to allocate DMA addresses bottom-up" > doesn't actually mean "allocate this thing on a 256MB-aligned address > and everything subsequent within a 256MB window above it", so let's > not build a fragile house of cards on top of pretending that it does.
Okay, I see your point. I'm fine with adding more of the IOVA allocation logic to the respective driver (s5p-mfc), however I would still like to use the dma-mapping framework and helpers, which depend on it (like v4l2-videobuf2, dma-buf, and so on). Would it be fine for you to let device drivers to access the IOVA domains hidden by the DMA-IOMMU glue code to mark certain parts of IOVA space as reserved or to manually remap the buffer (like firmware) with specific address requirements? Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu