On Wed, 2021-05-26 at 12:45 +0200, Christian König wrote:
Am 26.05.21 um 09:39 schrieb Thomas Hellström:
[SNIP]
I think the long term goal is to use memremap all over the place, to just not have to bother with the __iomem annotation. But to do that io- mapping.h needs to support memremap. But for now we need to be strict about __iomem unless we're in arch specific code. That's why that dma_buf_map thing was created, but TTM memcpy was never fully adapted.
I don't think that this will work. __iomem annotation is there because we have architectures where you need to use special CPU instructions for iomem access.
That won't go away just because we use memremap().
That's true, but can we ever support those with TTM, given that we allow user-space mmaping that transparently may change to an iomap? Given that, and what's written here
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2F...
To me it sounds like if an architecture can't support memremap, we can't support it with TTM either.
That was also my argument, but this is unfortunately not true.
We already have architectures where the __iomem approach is mandatory for kernel mappings, but work fine for userspace (don't ask me how that works, idk).
Ugh. :/
That's the reason why we had to fix the UVD firmware upload in the kernel:
commit ba0b2275a6781b2f3919d931d63329b5548f6d5f Author: Christian König christian.koenig@amd.com Date: Tue Aug 23 11:00:17 2016 +0200
drm/amdgpu: use memcpy_to/fromio for UVD fw upload
In any case for this particular patch, to avoid potential regressions, OK if I just add an ioremap() in case the memremap fails?
Well because of the issues outlined above I would actually prefer if we can keep the __iomem annotation around.
Well, we'd do that. Since we use the dma_buf_map unconditionally.
So what would happen in the above case, would be:
- memremap would fail. (Otherwise I'd be terribly confused) - we retry with ioremap and the dma-buf-map member is_iomem would thus be set - memcpy would do the right thing, based on is_iomem.
/Thomas
Christian.
/Thomas
Christian.
As for limited arch support for memremap cached, It looks like we only need to or in "backup" mapping modes in the memremap flags, and we'd mimic the previous behaviour.
/Thomas
/Thomas