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%2FArticles%2F653585%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C1cdcfe9d20e740308c9e08d92019785b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637576116034492654%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=e2BFGQJcElwVxrvHcnALDWscHN7ernLekGvXHqWBcwY%3D&reserved=0 > > > > > > > > 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@xxxxxxx> > 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 > > > > > > > > > > > > > > > > > > > >
