<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-US" link="blue" vlink="#954F72"> [AMD Official Use Only - Internal Distribution Only] <div> <div class="WordSection1"> Thanks million! I’ll change the patch. <o:p> </o:p> -Jacob<o:p></o:p> <o:p> </o:p> </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Koenig, Christian <Christian.Koenig@amd.com> Sent: Wednesday, March 4, 2020 12:09:36 AM To: He, Jacob <Jacob.He@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Subject: Re: [PATCH] drm/amdgpu: Update SPM_VMID with the job's vmid when application reserves the vmid <div> </div> </div> <div style="background-color:#FFFFFF"> <div class="x_moz-cite-prefix">Am 03.03.20 um 17:07 schrieb He, Jacob: </div> <blockquote type="cite"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style>  </style> [AMD Official Use Only - Internal Distribution Only] <div> <div class="x_WordSection1"> Oh, you are right! If SPM_VMID is updated by other process while the SPM enabled commands is executing, that will cause VM fault. Is the wait vm idle right before unreserve vmid still necessary if using asynchroneously setting SPM_VMID? </div> </div> </blockquote> No, that are alternative approaches. Updating the VMID asynchronously sounds a bit cleaner to me, but feel free to pick whatever is easier for you to implement. Regards, Christian. <blockquote type="cite"> <div> <div class="x_WordSection1"> Thanks Jacob <div style="border:none; border-top:solid #E1E1E1 1.0pt; padding:3.0pt 0in 0in 0in"> From: <a href="mailto:Christian.Koenig@amd.com">Koenig, Christian</a> Sent: Tuesday, March 3, 2020 11:36 PM To: <a href="mailto:Jacob.He@amd.com">He, Jacob</a>; <a href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a> Subject: Re: [PATCH] drm/amdgpu: Update SPM_VMID with the job's vmid when application reserves the vmid </div> See the SPM buffer address is set using CP commands as well, right? And those execute asynchronously. When we now synchronously update the SPM VMID we risk that we switch from one process to another while the new process is not ready yet with its setup. That could have quite a bunch of unforeseen consequences, including accidentally writing SPM data into the new process address space at whatever buffer address was used before. This is something we at least should try to avoid. Regards, Christian. Am 03.03.20 um 16:28 schrieb He, Jacob: <blockquote style="margin-top:5.0pt; margin-bottom:5.0pt"> [AMD Official Use Only - Internal Distribution Only] <div> Thanks! Could you please take an example of trouble “This way we avoid a bunch of trouble when one process drops the VMID reservation and another one grabs it.”? Thanks Jacob <div style="border:none; border-top:solid #E1E1E1 1.0pt; padding:3.0pt 0in 0in 0in"> From: <a href="mailto:Christian.Koenig@amd.com">Koenig, Christian</a> Sent: Tuesday, March 3, 2020 11:03 PM To: <a href="mailto:Jacob.He@amd.com">He, Jacob</a>; <a href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a> Subject: Re: [PATCH] drm/amdgpu: Update SPM_VMID with the job's vmid when application reserves the vmid </div> Am 03.03.20 um 15:34 schrieb He, Jacob: <blockquote style="margin-top:5.0pt; margin-bottom:5.0pt"> [AMD Official Use Only - Internal Distribution Only] <div> It would be better if we could do that asynchronously with a register write on the ring. Sorry, I don’t get your point. Could you please elaborate more? </div> </blockquote> You pass the ring from amdgpu_vm_flush() to the *_update_spm_vmid() functions. And then instead of using WREG32() you call amdgpu_ring_emit_wreg() to make the write asynchronously on the ring buffer using a CP command. This way we avoid a bunch of trouble when one process drops the VMID reservation and another one grabs it. Regards, Christian. <blockquote style="margin-top:5.0pt; margin-bottom:5.0pt"> <div> Thanks Jacob <div style="border:none; border-top:solid #E1E1E1 1.0pt; padding:3.0pt 0in 0in 0in"> From: <a href="mailto:ckoenig.leichtzumerken@gmail.com">Christian König</a> Sent: Tuesday, March 3, 2020 10:16 PM To: <a href="mailto:Jacob.He@amd.com">He, Jacob</a>; <a href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a> Subject: Re: [PATCH] drm/amdgpu: Update SPM_VMID with the job's vmid when application reserves the vmid </div> Am 02.03.20 um 06:35 schrieb Jacob He: > SPM access the video memory according to SPM_VMID. It should be updated > with the job's vmid right before the job is scheduled. SPM_VMID is a > global resource > > Change-Id: Id3881908960398f87e7c95026a54ff83ff826700 > Signed-off-by: Jacob He <a href="mailto:jacob.he@amd.com"><jacob.he@amd.com></a> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index c00696f3017e..c761d3a0b6e8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1080,8 +1080,12 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job, > struct dma_fence *fence = NULL; > bool pasid_mapping_needed = false; > unsigned patch_offset = 0; > + bool update_spm_vmid_needed = (job->vm && (job->vm->reserved_vmid[vmhub] != NULL)); > int r; > > + if (update_spm_vmid_needed && adev->gfx.rlc.funcs->update_spm_vmid) > + adev->gfx.rlc.funcs->update_spm_vmid(adev, job->vmid); > + It would be better if we could do that asynchronously with a register write on the ring. The alternative is that we block for the VM to be idle in amdgpu_vm_ioctl() before unreserving the VMID. In other words lock the reservation object of the root PD and call amdgpu_vm_wait_idle() before calling amdgpu_vmid_free_reserved(). Regards, Christian. > if (amdgpu_vmid_had_gpu_reset(adev, id)) { > gds_switch_needed = true; > vm_flush_needed = true; </div> </blockquote> </div> </blockquote> </div> </div> </blockquote> </div> </div> </body> </html>