[PATCH 2/9] drm/amdgpu: Add PASID management

Christian König deathsimple at vodafone.de
Mon Aug 28 06:45:20 UTC 2017


I agree that the VM code is growing a bit to much. but the crux is it is 
very close to VMID management, so we should keep the code together.

I wanted to avoid it, but you suggested to separate the VMID management 
quite a while ago. How about moving both VMID as well as PASSID into 
amdgpu_hwid_mgr.c?

Regards,
Christian.

Am 28.08.2017 um 05:06 schrieb zhoucm1:
> Could we separate PASID manager to a clean file like amdgpu_pasid.c 
> like what context manager done?
>
> Since VM code is growing, and which looks more and more complex.
>
> btw: really like many comments about PASID explaination. :)
>
> Regards,
> David Zhou
> On 2017年08月26日 15:19, Felix Kuehling wrote:
>> 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>
>> ---
>>   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);
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx




More information about the amd-gfx mailing list