Hi all,
Sorry for my late reply. I don't know if you still remember this thread, let me
give a quick summary:
1. We want to implement the dGPU prime feature in guest VM. But we
encountered this issue: virtio-gpu doesn’t have ->get_sg_table implemented
which is required by drm_gem_map_attach(). This is modified by: 207395da5a97
(“drm/prime: reject DMA-BUF attach when get_sg_table is missing”).
2. To fix this, I override the function virtgpu_gem_device_attach() to not
call drm_gem_map_attach() for vram object so drm_gem_map_attach() will not
return -ENOSYS for not having ->get_sg_table.
3. Then you think this is incorrect and drm_gem_map_attach() requires
get_sg_table to be implemented is intentional. I should either implement
->attach or ->get_sg_table for virtio-gpu.
4. As discussed, I implemented ->attach for virtio-gpu, but you suggested
that I should check peer2peer flag first.
5. Now I have the implementation to get p2p_distance and check the p2p flag
already, but I found that Rob Clark merged a patch to fix above patch:
207395da5a97 (“drm/prime: reject DMA-BUF attach when get_sg_table is missing”)
* Rob’s patch: https://patchwork.freedesktop.org/patch/584318/
6. With Rob’s patch, ->get_sg_table isn’t required for virtio-gpu anymore
and it seems p2p flag also doesn’t need to be checked anymore.
So I want to rediscuss if we still need to do p2p checking now?
If so, I will send out my implementation soon.
Best regards,
Julia
On 2024/1/31 22:32, Christian König wrote:
Am 31.01.24 um 11:20 schrieb Zhang, Julia:
On 2024/1/30 22:23, Christian König wrote:
Am 30.01.24 um 12:16 schrieb Daniel Vetter:
On Tue, Jan 30, 2024 at 12:10:31PM +0100, Daniel Vetter wrote:
[SNIP]
Hi Sima, Christian,
Yeah, that is really just speculative. All importers need to set the peer2peer
flag just in case.
I see, I will modify this.
What happens under the hood is that IOMMU redirects the "VRAM" memory access to
whatever address the DMA-buf on the host is pointing to (system, VRAM,
doorbell, IOMMU, whatever).
I'm also not 100% sure if all the cache snooping is done correctly in all
cases, but for now it seems to work.
Frankly the more I look at the original patch that added vram export
support the more this just looks like a "pls revert, this is just too
broken".
The commit I mean is this one: ea5ea3d8a117 ("drm/virtio: support mapping
exported vram"). The commit message definitely needs to cite that one, and
also needs a cc: stable because not rejecting invalid imports is a pretty
big deal.
Yeah, I've pointed out that commit in an internal discussion as well. I was
just not aware that it's that severely broken.
Yeah we have mentioned this patch before, but I don't totally understand why
this is too broken. Without exporting vram objects, dGPU prime feature would
not be realized.
Would you mind to explain more about it. Thanks!
One reason is that using sg tables without struct pages is actually a hack we
came up with because we couldn't hope to clean up the sg table structure any
time soon to not include struct page pointers.
Another reason is that using this with devices which don't expect a DMA address
pointing into a virtual PCI BAR. So doing this without checking the peer2peer
flag can most likely cause quite a bit of trouble.
Regards,
Christian.
Best regards,
Julia
Regards,
Christian.
