On 6/23/26 13:32, SHANMUGAM, SRINIVASAN wrote: > AMD General > > Hi Christian, > >> -----Original Message----- >> From: Koenig, Christian <[email protected]> >> Sent: Tuesday, June 23, 2026 2:51 PM >> To: SHANMUGAM, SRINIVASAN <[email protected]>; >> Deucher, Alexander <[email protected]> >> Cc: [email protected] >> Subject: Re: [RFC PATCH 1/3] drm/amdgpu: Add PASID fpriv lookup manager >> >> On 6/23/26 10:12, Srinivasan Shanmugam wrote: >>> Current VM fault handling resolves PASIDs through VM state: >>> >>> PASID -> VM >>> >>> That is sufficient for VM fault handling, but process-scoped event >>> delivery paths need a different semantic mapping: >>> >>> PASID -> amdgpu_fpriv >>> >>> Although both mappings are indexed by PASID, they serve different >>> purposes: >>> >>> vm_manager.pasids >>> PASID -> VM fault handling >>> >>> vm_manager.pasid_fprivs >>> PASID -> process-scoped DRM file-private owner >>> >>> Add a PASID fpriv xarray beside the existing PASID to VM xarray. >>> >>> Cc: Alex Deucher <[email protected]> >>> Cc: Christian König <[email protected]> >>> Signed-off-by: Srinivasan Shanmugam <[email protected]> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 +++++ >>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 13 +++++++++++++ >>> 2 files changed, 18 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> index 450bd9dc6bc0..3141ae1383b0 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> @@ -2869,6 +2869,8 @@ void amdgpu_vm_manager_init(struct amdgpu_device >>> *adev) #endif >>> >>> xa_init_flags(&adev->vm_manager.pasids, XA_FLAGS_LOCK_IRQ); >>> + xa_init_flags(&adev->vm_manager.pasid_fprivs, >>> + XA_FLAGS_LOCK_IRQ); >> >> I think we should completely remove &adev->vm_manager.pasids. >> >> Take a look at amdgpu_pasid_alloc(), we already store the PASIDs in the >> global >> amdgpu_pasid_xa xarray. >> >> But instead of a real value we just give xa_mk_value(0) as dummy for the >> entry. >> >> I think we can re-use that array to store pasid->fpriv mapping. It just >> needs proper >> locking and interface to retrive the fpriv for a pasid (while holding the XA >> lock >> irqsave). >> >> Let me know when you have more questions. > > Thanks!. > > Just to be sure, few clarifications please: > > 1. Should amdgpu_pasid_xa become the single PASID ownership table, > replacing adev->vm_manager.pasids?
Yes, exactly that was my thinking. > > 2. For DRM PASIDs, should amdgpu_pasid_alloc() continue inserting the > dummy xa_mk_value(0), and should amdgpu_driver_open_kms() replace that > entry with fpriv after fpriv setup? What do you mean? amdgpu_pasid_alloc() is called from amdgpu_driver_open_kms(). > > 3. For existing VM fault users, should the lookup path become: > > PASID -> fpriv -> vm > > using &fpriv->vm while holding the amdgpu_pasid_xa lock irqsave? Yes, exactly that. It is a bit tricky since we need to dance with the VM root BO refcount and unlocking/relocking etc.. But I think that should be doable. > > I will rework the RFC to use amdgpu_pasid_xa and remove the new > vm_manager.pasid_fprivs xarray. Thanks, Christian. > > Best regards, > Srini > >> >> Regards, >> Christian. >>
