On Mon, Jun 7, 2021 at 11:06 AM Sven Peter <sven@xxxxxxxxxxxxx> wrote: > On Mon, Jun 7, 2021, at 10:22, Arnd Bergmann wrote: > > I've looked at Documentation/core-api/dma-api-howto.rst again which mentions that > > By default, the kernel assumes that your device can address 32-bits of DMA > addressing. For a 64-bit capable device, this needs to be increased, and for > a device with limitations, it needs to be decreased. > [...] > These calls usually return zero to indicated your device can perform DMA > properly on the machine given the address mask you provided, but they might > return an error if the mask is too small to be supportable on the given > system. If it returns non-zero, your device cannot perform DMA properly on > this platform, and attempting to do so will result in undefined behavior. > You must not use DMA on this device unless the dma_set_mask family of > functions has returned success. > > which, unless I'm reading this incorrectly, should mean that asking for a 64bit > mask is always fine. In the worst case the mask will just be downgraded to > 32bit if the bus is correctly annotated (the places I looked at that use the mask > take the min of that one and dev->bus_dma_limit). > Only asking for a mask that is too small would be bad. > > I have also found [1],[2] which made changes to that documentation and that also > seems to confirm that it's fine to just ask for a 64 bit mask either way. Indeed, I forgot about that change, this does make it easier. > So for these cases > > > > > This will now fail on machines with dwc3 connected to a 32-bit bus (or a > > > > bus that is accidentally not annotated as supporting 64-bit) when there is > > > > some memory that is not addressable through that bus. > > the call should return success but the final mask used for allocations should > remain at 32bit. Before the change no memory above the 32bit limit was used by > the dwc3 core and after the change we still can't use any memory above the > 32bit limit. Right. > Now if we had a dwc3 controller with > * a quirk that only allows 32bit DMA for the core dwc3 controller > * but support for >32bit DMA for xhci buffers (xhci already asks for a 64bit mask) > * on a bus that's otherwise annotated to support 64bit > this change will break that. > > But that's unrelated to the dma_set_mask_and_coherent return value since > just calling it with a 64bit mask will already cause trouble (and also be successful!). > > The problem I see is that we likely wouldn't know about devices with a quirk like this > since so far everything has been working fine there. I'm not really sure how to guard > against that either since we would only notice on the first DMA transfer above the 32bit > limit. I'm also not sure how likely the existence of such a weird device is. > > This hypothetical dwc3 controller should probably either be confined to a bus with a > proper 32bit limit or get a quirk that enforces allocations from ZONE_DMA32. Doesn't > change the fact that they used to work but would now break after this patch. Makes sense, so for your v3 patch: Reviewed-by: Arnd Bergmann <arnd@xxxxxxxx>
