16.01.2023 18:11, Christian König пишет: > >>>>> >>>>>> mmapping the memory with that new offset should still work. The >>>>>> imported BO is created with ttm_bo_type_sg, and AFAICT ttm_bo_vm.c >>>>>> supports mapping of SG BOs. >>>>> >>>>> Actually it shouldn't. This can go boom really easily. >>>> >>>> OK. I don't think we're doing this, but after Xiaogang raised the >>>> question I went looking through the code whether it's theoretically >>>> possible. I didn't find anything in the code that says that mmapping >>>> imported dmabufs would be prohibited or even dangerous. On the >>>> contrary, I found that ttm_bo_vm explicitly supports mmapping SG BOs. >>>> >>>> >>>>> >>>>> When you have imported a BO the only correct way of to mmap() it is >>>>> to do so on the original exporter. >>>> >>>> That seems sensible, and this is what we do today. That said, if >>>> mmapping an imported BO is dangerous, I'm missing a mechanism to >>>> protect against this. It could be as simple as setting >>>> AMDGPU_GEM_CREATE_NO_CPU_ACCESS in amdgpu_dma_buf_create_obj. >>> >>> At least for the GEM mmap() handler this is double checked very early >>> by looking at obj->import_attach and then either rejecting it or >>> redirecting the request to the DMA-buf file instead. >> >> Can you point me at where this check is? I see a check for >> obj->import_attach in drm_gem_dumb_map_offset. But I can't see how >> this function is called in amdgpu. I don't think it is used at all. > > Uff, good question! @Thomas and @Dmitry: I clearly remember that one of > you guys was involved in the DRM/GEM mmap cleanup and DMA-buf with > workarounds for the KFD and DMA-buf. > > What was the final solution to this? I can't find it of hand any more. I was looking at it. The AMDGPU indeed allows to map imported GEMs, but then touching the mapped area by CPU results in a bus fault. You, Christian, suggested that this an AMDGPU bug that should be fixed by prohibiting the mapping in the first place and I was going to fix it, but then the plan changed from prohibiting the mapping into fixing it. The first proposal was to make DRM core to handle the dma-buf mappings for all drivers universally [1]. Then we decided that will be better to prohibit mapping of imported GEMs [2]. In the end, Rob Clark argued that better to implement the [1], otherwise current userspace (Android) will be broken if mapping will be prohibited. The last question was about the cache syncing of imported dma-bufs, how to ensure that drivers will do the cache maintenance/syncing properly. Rob suggested that it should be a problem for drivers and not for DRM core. I was going to re-send the [1], but other things were getting priority. It's good that you reminded me about it :) I may re-send it sometime soon if there are no new objections. [1] https://patchwork.freedesktop.org/patch/487481/ [2] https://lore.kernel.org/all/20220701090240.1896131-1-dmitry.osipenko@xxxxxxxxxxxxx/
