<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=Windows-1252"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="en-CN" link="#467886" vlink="#96607D" style="word-wrap:break-word"> [AMD Official Use Only - AMD Internal Distribution Only] <div> <div class="WordSection1"> <div> Hi <a id="OWAAM4048616300D61C4AB9A5FEB3EFAA44B5" href="mailto:Felix.Kuehling@amd.com">@Kuehling, Felix</a>, Thank you for review. I agree with you that we need to adjust the priority of each access method here. Hi <a id="OWAAM364ABE905E2A804E9F3AF3E7FE765BB0" href="mailto:Jonathan.Kim@amd.com"> @Kim, Jonathan</a>, I also find a new issue in amdgpu_ttm_access_memory_sdma() after my first patch. I sent a second patch to fix the new issue. Please help review. “[PATCH 2/2] drm/amdgpu: fix incomplete access issue in amdgpu_ttm_access_memory_sdma()” Regards Sam <div id="mail-editor-reference-message-container"> <div> <div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm"> From: Kuehling, Felix <Felix.Kuehling@amd.com> Date: Saturday, August 10, 2024 at 07:49 To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>, Zhang, GuoQing (Sam) <GuoQing.Zhang@amd.com>, Kim, Jonathan <Jonathan.Kim@amd.com> Subject: Re: [PATCH] drm/amdgpu: fix KFDMemoryTest.PtraceAccessInvisibleVram fail on SRIOV </div> <div> On 2024-08-07 04:36, Samuel Zhang wrote: > Ptrace access VRAM bo will first try sdma access in > amdgpu_ttm_access_memory_sdma(), if fails, it will fallback to mmio > access. > > Since ptrace only access 8 bytes at a time and > amdgpu_ttm_access_memory_sdma() only allow PAGE_SIZE bytes access, > it returns fail. > On SRIOV, mmio access will also fail as MM_INDEX/MM_DATA register write > is blocked for security reasons. > > The fix is just change len check in amdgpu_ttm_access_memory_sdma() so > that len in (0, PAGE_SIZE] are allowed. This will not fix the ptrace > test case on SRIOV, but also improve the access performance when the > access length is < PAGE_SIZE. > len > PAGE_SIZE case support is not needed as larger size will be break > into chunks of PAGE_SIZE len max in mem_rw(). I'm not convinced that using SDMA for small accesses is the best solution for all cases. For example, on large-BAR GPUs we should fall back to access through the FB BAR before we use indirect register access. That may still perform better than SDMA especially for very small accesses like 4-bytes typical for ptrace accesses. Maybe this needs an SRIOV-VF-specific condition if MMIO register access is not an option there. @Jonathan Kim, can you chime in as well? Thanks, Felix > > Signed-off-by: Samuel Zhang <guoqing.zhang@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index 5daa05e23ddf..a6e90eada367 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -1486,7 +1486,7 @@ static int amdgpu_ttm_access_memory_sdma(struct ttm_buffer_object *bo, > unsigned int num_dw; > int r, idx; > > - if (len != PAGE_SIZE) > + if (len > PAGE_SIZE) > return -EINVAL; > > if (!adev->mman.sdma_access_ptr) > @@ -1514,7 +1514,7 @@ static int amdgpu_ttm_access_memory_sdma(struct ttm_buffer_object *bo, > swap(src_addr, dst_addr); > > amdgpu_emit_copy_buffer(adev, &job->ibs[0], src_addr, dst_addr, > - PAGE_SIZE, 0); > + len, 0); > > amdgpu_ring_pad_ib(adev->mman.buffer_funcs_ring, &job->ibs[0]); > WARN_ON(job->ibs[0].length_dw > num_dw); </div> </div> </div> </div> </div> </div> </body> </html>