> But I'm a bit confused, doesn't his stuff belong into the IOMMU code? PASIDs work on dGPUs without an IOMMU. They're just device-specific process identifiers. On APUs there is an extra step in KFD that registers a process+PASID with the IOMMU driver. That way the IOMMU knows what to do when it sees address translation requests for a specific PASID from a specific device. This is done by calling amd_iommu_bind_pasid: struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev, struct kfd_process *p) { ... err = amd_iommu_bind_pasid(dev->pdev, p->pasid, p->lead_thread); ... } The PASID management is done by the device driver, not the IOMMU driver. I think different devices can use different PASIDs for the same process. Regards, Felix ________________________________________ From: Christian König <deathsimple@xxxxxxxxxxx> Sent: Saturday, August 26, 2017 9:27 AM To: Kuehling, Felix; amd-gfx at lists.freedesktop.org Subject: Re: [PATCH 2/9] drm/amdgpu: Add PASID management Am 26.08.2017 um 09:19 schrieb Felix Kuehling: > Allows assigning a PASID to a VM for identifying VMs involved in page > faults. The global PASID manager is also exported in the KFD > interface so that AMDGPU and KFD can share the PASID space. > > PASIDs of different sizes can be requested. On APUs, the PASID size > is deterined by the capabilities of the IOMMU. So KFD must be able > to allocate PASIDs in a smaller range. > > TODO: > * Actually assign PASIDs to VMs > * Update the PASID-VMID mapping registers during CS > > Change-Id: I88c9357a7c584f10e84b5607ac09eba77c833393 > Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com> The patch itself is Reviewed-by: Christian König <christian.koenig at amd.com>. But I'm a bit confused, doesn't his stuff belong into the IOMMU code? Regards, Christian. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 76 ++++++++++++++++++++++- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 14 ++++- > drivers/gpu/drm/amd/include/kgd_kfd_interface.h | 6 ++ > 8 files changed, 101 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c > index 3e28d2b..0807d52 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c > @@ -188,6 +188,8 @@ static const struct kfd2kgd_calls kfd2kgd = { > .get_local_mem_info = get_local_mem_info, > .get_gpu_clock_counter = get_gpu_clock_counter, > .get_max_engine_clock_in_mhz = get_max_engine_clock_in_mhz, > + .alloc_pasid = amdgpu_vm_alloc_pasid, > + .free_pasid = amdgpu_vm_free_pasid, > .create_process_vm = amdgpu_amdkfd_gpuvm_create_process_vm, > .destroy_process_vm = amdgpu_amdkfd_gpuvm_destroy_process_vm, > .get_process_page_dir = amdgpu_amdkfd_gpuvm_get_process_page_dir, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c > index 3b6b4d9..c20c000 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c > @@ -162,6 +162,8 @@ static const struct kfd2kgd_calls kfd2kgd = { > .get_local_mem_info = get_local_mem_info, > .get_gpu_clock_counter = get_gpu_clock_counter, > .get_max_engine_clock_in_mhz = get_max_engine_clock_in_mhz, > + .alloc_pasid = amdgpu_vm_alloc_pasid, > + .free_pasid = amdgpu_vm_free_pasid, > .create_process_vm = amdgpu_amdkfd_gpuvm_create_process_vm, > .destroy_process_vm = amdgpu_amdkfd_gpuvm_destroy_process_vm, > .create_process_gpumem = create_process_gpumem, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c > index 961369d..bb99c64 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c > @@ -209,6 +209,8 @@ static const struct kfd2kgd_calls kfd2kgd = { > .get_local_mem_info = get_local_mem_info, > .get_gpu_clock_counter = get_gpu_clock_counter, > .get_max_engine_clock_in_mhz = get_max_engine_clock_in_mhz, > + .alloc_pasid = amdgpu_vm_alloc_pasid, > + .free_pasid = amdgpu_vm_free_pasid, > .create_process_vm = amdgpu_amdkfd_gpuvm_create_process_vm, > .destroy_process_vm = amdgpu_amdkfd_gpuvm_destroy_process_vm, > .create_process_gpumem = create_process_gpumem, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > index 35f7d77..462011c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > @@ -1397,7 +1397,7 @@ int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, void **vm, > return -ENOMEM; > > /* Initialize the VM context, allocate the page directory and zero it */ > - ret = amdgpu_vm_init(adev, &new_vm->base, AMDGPU_VM_CONTEXT_COMPUTE); > + ret = amdgpu_vm_init(adev, &new_vm->base, AMDGPU_VM_CONTEXT_COMPUTE, 0); > if (ret != 0) { > pr_err("Failed init vm ret %d\n", ret); > /* Undo everything related to the new VM context */ > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index e390c01..ba3dc4d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -825,7 +825,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv) > } > > r = amdgpu_vm_init(adev, &fpriv->vm, > - AMDGPU_VM_CONTEXT_GFX); > + AMDGPU_VM_CONTEXT_GFX, 0); > if (r) { > kfree(fpriv); > goto out_suspend; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 70d7632..c635699 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -27,12 +27,59 @@ > */ > #include <linux/dma-fence-array.h> > #include <linux/interval_tree_generic.h> > +#include <linux/idr.h> > #include <drm/drmP.h> > #include <drm/amdgpu_drm.h> > #include "amdgpu.h" > #include "amdgpu_trace.h" > > /* > + * PASID manager > + * > + * PASIDs are global address space identifiers that can be shared > + * between the GPU, an IOMMU and the driver. VMs on different devices > + * may use the same PASID if they share the same address > + * space. Therefore PASIDs are allocated using a global IDA. VMs are > + * looked up from the PASID per amdgpu_device. > + */ > +static DEFINE_IDA(amdgpu_vm_pasid_ida); > + > +/** > + * amdgpu_vm_alloc_pasid - Allocate a PASID > + * @bits: Maximum width of the PASID in bits, must be at least 1 > + * > + * Allocates a PASID of the given width while keeping smaller PASIDs > + * available if possible. > + * > + * Returns a positive integer on success. Returns %-EINVAL if bits==0. > + * Returns %-ENOSPC if no PASID was avaliable. Returns %-ENOMEM on > + * memory allocation failure. > + */ > +int amdgpu_vm_alloc_pasid(unsigned int bits) > +{ > + int pasid = -EINVAL; > + > + for (bits = min(bits, 31U); bits > 0; bits--) { > + pasid = ida_simple_get(&amdgpu_vm_pasid_ida, > + 1U << (bits - 1), 1U << bits, > + GFP_KERNEL); > + if (pasid != -ENOSPC) > + break; > + } > + > + return pasid; > +} > + > +/** > + * amdgpu_vm_free_pasid - Free a PASID > + * @pasid: PASID to free > + */ > +void amdgpu_vm_free_pasid(unsigned int pasid) > +{ > + ida_simple_remove(&amdgpu_vm_pasid_ida, pasid); > +} > + > +/* > * GPUVM > * GPUVM is similar to the legacy gart on older asics, however > * rather than there being a single global gart table > @@ -2482,7 +2529,7 @@ void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint64_t vm_size, uint32_ > * Init @vm fields. > */ > int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, > - int vm_context) > + int vm_context, unsigned int pasid) > { > const unsigned align = min(AMDGPU_VM_PTB_ALIGN_SIZE, > AMDGPU_VM_PTE_COUNT(adev) * 8); > @@ -2562,6 +2609,19 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, > if (r) > goto error_free_root; > > + if (pasid) { > + unsigned long flags; > + > + spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags); > + r = idr_alloc(&adev->vm_manager.pasid_idr, vm, pasid, pasid + 1, > + GFP_ATOMIC); > + spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags); > + if (r < 0) > + goto error_free_root; > + > + vm->pasid = pasid; > + } > + > vm->vm_context = vm_context; > if (vm_context == AMDGPU_VM_CONTEXT_COMPUTE) { > mutex_lock(&id_mgr->lock); > @@ -2650,6 +2710,14 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm) > mutex_unlock(&id_mgr->lock); > } > > + if (vm->pasid) { > + unsigned long flags; > + > + spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags); > + idr_remove(&adev->vm_manager.pasid_idr, vm->pasid); > + spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags); > + } > + > amd_sched_entity_fini(vm->entity.sched, &vm->entity); > > if (!RB_EMPTY_ROOT(&vm->va)) { > @@ -2729,6 +2797,9 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev) > adev->vm_manager.vm_update_mode = 0; > #endif > > + idr_init(&adev->vm_manager.pasid_idr); > + spin_lock_init(&adev->vm_manager.pasid_lock); > + > adev->vm_manager.n_compute_vms = 0; > } > > @@ -2743,6 +2814,9 @@ void amdgpu_vm_manager_fini(struct amdgpu_device *adev) > { > unsigned i, j; > > + WARN_ON(!idr_is_empty(&adev->vm_manager.pasid_idr)); > + idr_destroy(&adev->vm_manager.pasid_idr); > + > for (i = 0; i < AMDGPU_MAX_VMHUBS; ++i) { > struct amdgpu_vm_id_manager *id_mgr = > &adev->vm_manager.id_mgr[i]; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > index 49e15d7..692b05c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > @@ -25,6 +25,7 @@ > #define __AMDGPU_VM_H__ > > #include <linux/rbtree.h> > +#include <linux/idr.h> > > #include "gpu_scheduler.h" > #include "amdgpu_sync.h" > @@ -143,8 +144,9 @@ struct amdgpu_vm { > /* Scheduler entity for page table updates */ > struct amd_sched_entity entity; > > - /* client id */ > + /* client id and PASID (TODO: replace client_id with PASID) */ > u64 client_id; > + unsigned int pasid; > /* dedicated to vm */ > struct amdgpu_vm_id *reserved_vmid[AMDGPU_MAX_VMHUBS]; > > @@ -219,14 +221,22 @@ struct amdgpu_vm_manager { > */ > int vm_update_mode; > > + /* PASID to VM mapping, will be used in interrupt context to > + * look up VM of a page fault > + */ > + struct idr pasid_idr; > + spinlock_t pasid_lock; > + > /* Number of Compute VMs, used for detecting Compute activity */ > unsigned n_compute_vms; > }; > > +int amdgpu_vm_alloc_pasid(unsigned int bits); > +void amdgpu_vm_free_pasid(unsigned int pasid); > void amdgpu_vm_manager_init(struct amdgpu_device *adev); > void amdgpu_vm_manager_fini(struct amdgpu_device *adev); > int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm, > - int vm_context); > + int vm_context, unsigned int pasid); > void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm); > void amdgpu_vm_get_pd_bo(struct amdgpu_vm *vm, > struct list_head *validated, > diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h > index e8027b3..5833ef7 100644 > --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h > +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h > @@ -188,6 +188,9 @@ struct tile_config { > * > * @get_max_engine_clock_in_mhz: Retrieves maximum GPU clock in MHz > * > + * @alloc_pasid: Allocate a PASID > + * @free_pasid: Free a PASID > + * > * @program_sh_mem_settings: A function that should initiate the memory > * properties such as main aperture memory type (cache / non cached) and > * secondary aperture base address, size and memory type. > @@ -264,6 +267,9 @@ struct kfd2kgd_calls { > > uint32_t (*get_max_engine_clock_in_mhz)(struct kgd_dev *kgd); > > + int (*alloc_pasid)(unsigned int bits); > + void (*free_pasid)(unsigned int pasid); > + > int (*create_process_vm)(struct kgd_dev *kgd, void **vm, > void **process_info); > void (*destroy_process_vm)(struct kgd_dev *kgd, void *vm);