<!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> Hi Julia, sorry I totally missed your mail. The basic problem for P2P is what I already described in my previous mail: <blockquote type="cite">Well the problem is the virtualized environment. pci_p2pdma_distance() checks if two physical PCI devices can communicate with each other (and returns how many hops are in between). But inside a VM you don't see the physical devices, you can only see passed through devices plus your virtual device and a bunch of virtual bridges. So what pci_p2pdma_distance() returns inside the VM is actually completely meaningless. It can be that P2P works, but it can also be P2P doesn't work because on the physical system you have a bridge, root complex or whatever which is blacklisted and won't work for some reason. </blockquote> So the basic problem is that you can't figure out inside the VM if P2P is possible or not. As long as you don't fix this it's irrelevant if you have get_sg_table implemented or not, you first need to figure out the basic and not try to implement some detail. Regards, Christian. <div class="moz-cite-prefix">Am 04.12.24 um 04:46 schrieb Zhang, Julia: </div> <blockquote type="cite" cite="mid:IA1PR12MB6532BBDDA6B5DC4744E38232F2372@IA1PR12MB6532.namprd12.prod.outlook.com"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style>@font-face {font-family:"Cambria Math"; panose-1:2 4 5 3 5 4 6 3 2 4;}@font-face {font-family:DengXian; panose-1:2 1 6 0 3 1 1 1 1 1;}@font-face {font-family:Calibri; panose-1:2 15 5 2 2 2 4 3 2 4;}@font-face {font-family:Aptos;}@font-face {font-family:"\@DengXian"; panose-1:2 1 6 0 3 1 1 1 1 1;}@font-face {font-family:Consolas; panose-1:2 11 6 9 2 2 4 3 2 4;}p.MsoNormal, li.MsoNormal, div.MsoNormal {margin:0in; font-size:12.0pt; font-family:"Aptos",sans-serif;}a:link, span.MsoHyperlink {mso-style-priority:99; color:#467886; text-decoration:underline;}pre {mso-style-priority:99; mso-style-link:"HTML Preformatted Char"; margin:0in; font-size:10.0pt; font-family:"Courier New";}span.HTMLPreformattedChar {mso-style-name:"HTML Preformatted Char"; mso-style-priority:99; mso-style-link:"HTML Preformatted"; font-family:Consolas;}span.EmailStyle22 {mso-style-type:personal-reply; font-family:"Arial",sans-serif; color:windowtext;}.MsoChpDefault {mso-style-type:export-only; font-size:10.0pt; mso-ligatures:none;}div.WordSection1 {page:WordSection1;}ol {margin-bottom:0in;}ul {margin-bottom:0in;}</style> [AMD Official Use Only - AMD Internal Distribution Only] <div> <div class="WordSection1"> Hi Sima, Christian,<o:p></o:p> <o:p> </o:p> I would like to rediscuss about p2p in guest VM, can you please take a look. Thanks.<o:p></o:p> <o:p> </o:p> Best regards,<o:p></o:p> Julia<o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Zhang, Julia <a class="moz-txt-link-rfc2396E" href="mailto:Julia.Zhang@amd.com"><Julia.Zhang@amd.com></a> Sent: Friday, November 29, 2024 3:52 PM To: Koenig, Christian <a class="moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a>; Zhang, Julia <a class="moz-txt-link-rfc2396E" href="mailto:Julia.Zhang@amd.com"><Julia.Zhang@amd.com></a>; Gurchetan Singh <a class="moz-txt-link-rfc2396E" href="mailto:gurchetansingh@chromium.org"><gurchetansingh@chromium.org></a>; Chia-I Wu <a class="moz-txt-link-rfc2396E" href="mailto:olvaffe@gmail.com"><olvaffe@gmail.com></a>; David Airlie <a class="moz-txt-link-rfc2396E" href="mailto:airlied@redhat.com"><airlied@redhat.com></a>; Gerd Hoffmann <a class="moz-txt-link-rfc2396E" href="mailto:kraxel@redhat.com"><kraxel@redhat.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:linux-kernel@vger.kernel.org">linux-kernel@vger.kernel.org</a>; <a class="moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a>; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>; <a class="moz-txt-link-abbreviated" href="mailto:virtualization@lists.linux-foundation.org">virtualization@lists.linux-foundation.org</a>; Deucher, Alexander <a class="moz-txt-link-rfc2396E" href="mailto:Alexander.Deucher@amd.com"><Alexander.Deucher@amd.com></a>; David Airlie <a class="moz-txt-link-rfc2396E" href="mailto:airlied@gmail.com"><airlied@gmail.com></a>; Erik Faye-Lund <a class="moz-txt-link-rfc2396E" href="mailto:kusmabite@gmail.com"><kusmabite@gmail.com></a>; Olsak, Marek <a class="moz-txt-link-rfc2396E" href="mailto:Marek.Olsak@amd.com"><Marek.Olsak@amd.com></a>; Pelloux-Prayer, Pierre-Eric <a class="moz-txt-link-rfc2396E" href="mailto:Pierre-eric.Pelloux-prayer@amd.com"><Pierre-eric.Pelloux-prayer@amd.com></a>; Huang, Honglei1 <a class="moz-txt-link-rfc2396E" href="mailto:Honglei1.Huang@amd.com"><Honglei1.Huang@amd.com></a>; Chen, Jiqian <a class="moz-txt-link-rfc2396E" href="mailto:Jiqian.Chen@amd.com"><Jiqian.Chen@amd.com></a>; Huang, Ray <a class="moz-txt-link-rfc2396E" href="mailto:Ray.Huang@amd.com"><Ray.Huang@amd.com></a>; David Stevens <a class="moz-txt-link-rfc2396E" href="mailto:stevensd@chromium.org"><stevensd@chromium.org></a> Cc: Huang, Ray <a class="moz-txt-link-rfc2396E" href="mailto:Ray.Huang@amd.com"><Ray.Huang@amd.com></a>; Zhu, Lingshan <a class="moz-txt-link-rfc2396E" href="mailto:Lingshan.Zhu@amd.com"><Lingshan.Zhu@amd.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:robdclark@chromium.org">robdclark@chromium.org</a> Subject: Re: [PATCH v2 1/1] drm/virtio: Implement device_attach<o:p></o:p> </div> </div> <o:p> </o:p> Hi all,<o:p></o:p> Sorry for my late reply. I don't know if you still remember this thread, let me give a quick summary:<o:p></o:p> <ol type="1" start="1"> <li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l1 level1 lfo3"> 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”).<o:p></o:p></li> <li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l1 level1 lfo3"> 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.<o:p></o:p></li> <li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l1 level1 lfo3"> 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.<o:p></o:p></li> <li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l1 level1 lfo3"> As discussed, I implemented ->attach for virtio-gpu, but you suggested that I should check peer2peer flag first.<o:p></o:p></li> <li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l1 level1 lfo3"> 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”)<o:p></o:p></li> <ol type="a" start="1"> <li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l1 level2 lfo3"> Rob’s patch: <a href="https://patchwork.freedesktop.org/patch/584318/" moz-do-not-send="true" class="moz-txt-link-freetext">https://patchwork.freedesktop.org/patch/584318/</a><o:p></o:p></li> </ol> <li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l1 level1 lfo3"> 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.<o:p></o:p></li> </ol> So I want to rediscuss if we still need to do p2p checking now? <o:p></o:p> If so, I will send out my implementation soon.<o:p></o:p> Best regards,<o:p></o:p> Julia<o:p></o:p> <o:p> </o:p> <div> On 2024/1/31 22:32, Christian König wrote:<o:p></o:p> </div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> Am 31.01.24 um 11:20 schrieb Zhang, Julia:<o:p></o:p> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>On 2024/1/30 22:23, Christian König wrote:<o:p></o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>Am 30.01.24 um 12:16 schrieb Daniel Vetter:<o:p></o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>On Tue, Jan 30, 2024 at 12:10:31PM +0100, Daniel Vetter wrote:<o:p></o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> [SNIP] <o:p></o:p> </blockquote> </blockquote> </blockquote> <pre>Hi Sima, Christian,<o:p></o:p></pre> <pre><o:p> </o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>Yeah, that is really just speculative. All importers need to set the peer2peer flag just in case.<o:p></o:p></pre> </blockquote> <pre>I see, I will modify this.<o:p></o:p></pre> <pre><o:p> </o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>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).<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>I'm also not 100% sure if all the cache snooping is done correctly in all cases, but for now it seems to work.<o:p></o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>Frankly the more I look at the original patch that added vram export<o:p></o:p></pre> <pre>support the more this just looks like a "pls revert, this is just too<o:p></o:p></pre> <pre>broken".<o:p></o:p></pre> </blockquote> <pre>The commit I mean is this one: ea5ea3d8a117 ("drm/virtio: support mapping<o:p></o:p></pre> <pre>exported vram"). The commit message definitely needs to cite that one, and<o:p></o:p></pre> <pre>also needs a cc: stable because not rejecting invalid imports is a pretty<o:p></o:p></pre> <pre>big deal.<o:p></o:p></pre> </blockquote> <pre>Yeah, I've pointed out that commit in an internal discussion as well. I was just not aware that it's that severely broken.<o:p></o:p></pre> <pre><o:p> </o:p></pre> </blockquote> <pre>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.<o:p></o:p></pre> <pre>Would you mind to explain more about it. Thanks!<o:p></o:p></pre> </blockquote> 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. <o:p></o:p> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>Best regards,<o:p></o:p></pre> <pre>Julia<o:p></o:p></pre> <pre><o:p> </o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>Regards,<o:p></o:p></pre> <pre>Christian.<o:p></o:p></pre> <pre><o:p> </o:p></pre> </blockquote> </blockquote> </blockquote> </div> </div> </blockquote> </body> </html>