On 7/3/26 12:36, SHANMUGAM, SRINIVASAN wrote: > AMD General > > Hi Christian, > >> -----Original Message----- >> From: Koenig, Christian <[email protected]> >> Sent: Friday, July 3, 2026 1:14 PM >> To: SHANMUGAM, SRINIVASAN <[email protected]>; >> Deucher, Alexander <[email protected]> >> Cc: [email protected] >> Subject: Re: [PATCH v4 3/3] drm/amdgpu: Drop vm_manager PASID to VM >> mapping >> >> On 7/3/26 08:18, Srinivasan Shanmugam wrote: >>> VM lookup users now resolve DRM PASIDs through the global PASID xarray: >>> >>> PASID -> fpriv -> VM >>> >>> The per-device vm_manager.pasids xarray is no longer needed. >>> >>> Remove PASID registration and teardown from VM init/fini paths, drop >>> vm_manager PASID initialization/cleanup, and remove the xarray from >>> struct amdgpu_vm_manager. >>> >>> 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 | 25 ++----------------------- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 4 ---- >>> 2 files changed, 2 insertions(+), 27 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> index 9092ff227a55..74836240edbb 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> @@ -2647,14 +2647,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, >> struct amdgpu_vm *vm, >>> if (r) >>> dev_dbg(adev->dev, "Failed to create task info for VM\n"); >>> >>> - /* Store new PASID in XArray (if non-zero) */ >>> - if (pasid != 0) { >>> - r = xa_err(xa_store_irq(&adev->vm_manager.pasids, pasid, vm, >> GFP_KERNEL)); >>> - if (r < 0) >>> - goto error_free_root; >>> - >>> - vm->pasid = pasid; >>> - } >>> + vm->pasid = pasid; >> >> What do we actually still need the pasid in the VM for? > > I checked the remaining vm->pasid users. It looks like vm->pasid is still > needed for existing hardware programming paths (TLB flushes, PASID mapping > packets, tracepoints, etc.), while this series only removes the separate > vm_manager.pasids lookup table.
Ah, yes. Especially the TLB flushes are a good point. > > My understanding is therefore that: > > vm->pasid > remains as per-VM state > > vm_manager.pasids > can be removed because lookups now go > PASID -> fpriv -> VM > > May I kno pls, does that match your expectation, or were you thinking of > removing vm->pasid as well? Please go ahead with the current plan of removing vm_manager.pasids but keeping vm->pasid. Thanks, Christian. > > Regards, > Srini > >> >> Regards, >> Christian.
