* [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
@ 2021-11-09 1:37 Ramesh Errabolu
2021-11-09 4:51 ` Felix Kuehling
2021-11-09 5:26 ` Lang Yu
0 siblings, 2 replies; 10+ messages in thread
From: Ramesh Errabolu @ 2021-11-09 1:37 UTC (permalink / raw)
To: amd-gfx; +Cc: Ramesh Errabolu
MMIO/DOORBELL BOs encode control data and should be pinned in GTT
domain before enabling PCIe connected peer devices in accessing it
Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 25 +++++++++
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 55 +++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 4c1d6536a7a5..d9a1cfd7876f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -300,6 +300,31 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
uint64_t va, void *drm_priv,
struct kgd_mem **mem, uint64_t *size,
uint64_t *mmap_offset);
+
+/**
+ * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
+ * @bo: Handle of buffer object being pinned
+ * @domain: Domain into which BO should be pinned
+ *
+ * - USERPTR BOs are UNPINNABLE and will return error
+ * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
+ * PIN count incremented. It is valid to PIN a BO multiple times
+ *
+ * Return: ZERO if successful in pinning, Non-Zero in case of error.
+ * Will return -EINVAL if input BO parameter is a USERPTR type.
+ */
+int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain);
+
+/**
+ * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following criteria
+ * @bo: Handle of buffer object being unpinned
+ *
+ * - Is a illegal request for USERPTR BOs and is ignored
+ * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
+ * PIN count decremented. Calls to UNPIN must balance calls to PIN
+ */
+void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo);
+
int amdgpu_amdkfd_get_tile_config(struct amdgpu_device *adev,
struct tile_config *config);
void amdgpu_amdkfd_ras_poison_consumption_handler(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 4fa814358552..f4ffc41873dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1299,6 +1299,36 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
return ret;
}
+int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain)
+{
+ int ret = 0;
+
+ ret = amdgpu_bo_reserve(bo, false);
+ if (unlikely(ret))
+ return ret;
+
+ ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
+ if (ret)
+ pr_err("Error in Pinning BO to domain: %d\n", domain);
+
+ amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
+ amdgpu_bo_unreserve(bo);
+
+ return ret;
+}
+
+void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo)
+{
+ int ret = 0;
+
+ ret = amdgpu_bo_reserve(bo, false);
+ if (unlikely(ret))
+ return;
+
+ amdgpu_bo_unpin(bo);
+ amdgpu_bo_unreserve(bo);
+}
+
int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
struct file *filp, u32 pasid,
void **process_info,
@@ -1525,6 +1555,23 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
if (offset)
*offset = amdgpu_bo_mmap_offset(bo);
+ if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
+ KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
+ ret = amdgpu_amdkfd_bo_validate(bo, AMDGPU_GEM_DOMAIN_GTT, false);
+ if (ret) {
+ pr_err("Validating MMIO/DOORBELL BO during ALLOC FAILED\n");
+ goto err_node_allow;
+ }
+
+ ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
+ if (ret) {
+ pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
+ goto err_node_allow;
+ }
+ bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
+ bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
+ }
+
return 0;
allocate_init_user_pages_failed:
@@ -1561,6 +1608,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
bool is_imported = false;
mutex_lock(&mem->lock);
+
+ /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
+ if (mem->alloc_flags &
+ (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
+ KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
+ amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
+ }
+
mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
is_imported = mem->is_imported;
mutex_unlock(&mem->lock);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
2021-11-09 1:37 [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain Ramesh Errabolu
@ 2021-11-09 4:51 ` Felix Kuehling
2021-11-09 5:13 ` Errabolu, Ramesh
2021-11-09 5:26 ` Lang Yu
1 sibling, 1 reply; 10+ messages in thread
From: Felix Kuehling @ 2021-11-09 4:51 UTC (permalink / raw)
To: amd-gfx, Errabolu, Ramesh
Am 2021-11-08 um 8:37 p.m. schrieb Ramesh Errabolu:
> MMIO/DOORBELL BOs encode control data and should be pinned in GTT
> domain before enabling PCIe connected peer devices in accessing it
The PCIe connected peer device access isn't an issue on the upstream
branch (yet). But in general, it is a good idea to pin these SG BOs.
There is no good reason to have them on the TTM LRU list. And they do
reference fixed physical addresses.
See one more comment inline.
>
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 25 +++++++++
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 55 +++++++++++++++++++
> 2 files changed, 80 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 4c1d6536a7a5..d9a1cfd7876f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -300,6 +300,31 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
> uint64_t va, void *drm_priv,
> struct kgd_mem **mem, uint64_t *size,
> uint64_t *mmap_offset);
> +
> +/**
> + * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
> + * @bo: Handle of buffer object being pinned
> + * @domain: Domain into which BO should be pinned
> + *
> + * - USERPTR BOs are UNPINNABLE and will return error
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count incremented. It is valid to PIN a BO multiple times
> + *
> + * Return: ZERO if successful in pinning, Non-Zero in case of error.
> + * Will return -EINVAL if input BO parameter is a USERPTR type.
> + */
> +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain);
> +
> +/**
> + * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following criteria
> + * @bo: Handle of buffer object being unpinned
> + *
> + * - Is a illegal request for USERPTR BOs and is ignored
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count decremented. Calls to UNPIN must balance calls to PIN
> + */
> +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo);
> +
These declarations aren't needed here. The functions should be static
because they are only used in the same source file.
> int amdgpu_amdkfd_get_tile_config(struct amdgpu_device *adev,
> struct tile_config *config);
> void amdgpu_amdkfd_ras_poison_consumption_handler(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 4fa814358552..f4ffc41873dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1299,6 +1299,36 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
> return ret;
> }
>
> +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain)
static
> +{
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return ret;
> +
> + ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
> + if (ret)
> + pr_err("Error in Pinning BO to domain: %d\n", domain);
> +
> + amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
> + amdgpu_bo_unreserve(bo);
> +
> + return ret;
> +}
> +
> +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo)
static
> +{
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return;
> +
> + amdgpu_bo_unpin(bo);
> + amdgpu_bo_unreserve(bo);
> +}
> +
> int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> struct file *filp, u32 pasid,
> void **process_info,
> @@ -1525,6 +1555,23 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> if (offset)
> *offset = amdgpu_bo_mmap_offset(bo);
>
> + if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + ret = amdgpu_amdkfd_bo_validate(bo, AMDGPU_GEM_DOMAIN_GTT, false);
> + if (ret) {
> + pr_err("Validating MMIO/DOORBELL BO during ALLOC FAILED\n");
> + goto err_node_allow;
Actually, I think this is wrong. You need a new label before
drm_vma_node_revoke to make sure the entry in the node permission
structure is not leaked.
> + }
> +
> + ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
> + if (ret) {
> + pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
> + goto err_node_allow;
Same as above.
Regards,
Felix
> + }
> + bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
> + bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
> + }
> +
> return 0;
>
> allocate_init_user_pages_failed:
> @@ -1561,6 +1608,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> bool is_imported = false;
>
> mutex_lock(&mem->lock);
> +
> + /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
> + if (mem->alloc_flags &
> + (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
> + }
> +
> mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
> is_imported = mem->is_imported;
> mutex_unlock(&mem->lock);
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
2021-11-09 4:51 ` Felix Kuehling
@ 2021-11-09 5:13 ` Errabolu, Ramesh
0 siblings, 0 replies; 10+ messages in thread
From: Errabolu, Ramesh @ 2021-11-09 5:13 UTC (permalink / raw)
To: Kuehling, Felix, amd-gfx
[AMD Official Use Only]
Responses in line
Regards,
Ramesh
-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling@amd.com>
Sent: Monday, November 8, 2021 10:51 PM
To: amd-gfx@lists.freedesktop.org; Errabolu, Ramesh <Ramesh.Errabolu@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
Am 2021-11-08 um 8:37 p.m. schrieb Ramesh Errabolu:
> MMIO/DOORBELL BOs encode control data and should be pinned in GTT
> domain before enabling PCIe connected peer devices in accessing it
The PCIe connected peer device access isn't an issue on the upstream branch (yet). But in general, it is a good idea to pin these SG BOs.
There is no good reason to have them on the TTM LRU list. And they do reference fixed physical addresses.
See one more comment inline.
>
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 25 +++++++++
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 55
> +++++++++++++++++++
> 2 files changed, 80 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 4c1d6536a7a5..d9a1cfd7876f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -300,6 +300,31 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
> uint64_t va, void *drm_priv,
> struct kgd_mem **mem, uint64_t *size,
> uint64_t *mmap_offset);
> +
> +/**
> + * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
> + * @bo: Handle of buffer object being pinned
> + * @domain: Domain into which BO should be pinned
> + *
> + * - USERPTR BOs are UNPINNABLE and will return error
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count incremented. It is valid to PIN a BO multiple times
> + *
> + * Return: ZERO if successful in pinning, Non-Zero in case of error.
> + * Will return -EINVAL if input BO parameter is a USERPTR type.
> + */
> +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain);
> +
> +/**
> + * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following
> +criteria
> + * @bo: Handle of buffer object being unpinned
> + *
> + * - Is a illegal request for USERPTR BOs and is ignored
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count decremented. Calls to UNPIN must balance calls to PIN
> + */
> +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo);
> +
These declarations aren't needed here. The functions should be static because they are only used in the same source file.
Ramesh: Removed them from the header file
> int amdgpu_amdkfd_get_tile_config(struct amdgpu_device *adev,
> struct tile_config *config);
> void amdgpu_amdkfd_ras_poison_consumption_handler(struct
> amdgpu_device *adev); diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 4fa814358552..f4ffc41873dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1299,6 +1299,36 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
> return ret;
> }
>
> +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain)
Static
Ramesh: Updated function signature to make it static
> +{
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return ret;
> +
> + ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
> + if (ret)
> + pr_err("Error in Pinning BO to domain: %d\n", domain);
> +
> + amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
> + amdgpu_bo_unreserve(bo);
> +
> + return ret;
> +}
> +
> +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo)
static
Ramesh: Same as above
> +{
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return;
> +
> + amdgpu_bo_unpin(bo);
> + amdgpu_bo_unreserve(bo);
> +}
> +
> int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> struct file *filp, u32 pasid,
> void **process_info,
> @@ -1525,6 +1555,23 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> if (offset)
> *offset = amdgpu_bo_mmap_offset(bo);
>
> + if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + ret = amdgpu_amdkfd_bo_validate(bo, AMDGPU_GEM_DOMAIN_GTT, false);
> + if (ret) {
> + pr_err("Validating MMIO/DOORBELL BO during ALLOC FAILED\n");
> + goto err_node_allow;
Actually, I think this is wrong. You need a new label before drm_vma_node_revoke to make sure the entry in the node permission structure is not leaked.
Ramesh: Good catch - using a new label - "err_pin_bo"
> + }
> +
> + ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
> + if (ret) {
> + pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
> + goto err_node_allow;
Same as above.
Regards,
Felix
> + }
> + bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
> + bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
> + }
> +
> return 0;
>
> allocate_init_user_pages_failed:
> @@ -1561,6 +1608,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> bool is_imported = false;
>
> mutex_lock(&mem->lock);
> +
> + /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
> + if (mem->alloc_flags &
> + (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
> + }
> +
> mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
> is_imported = mem->is_imported;
> mutex_unlock(&mem->lock);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
2021-11-09 1:37 [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain Ramesh Errabolu
2021-11-09 4:51 ` Felix Kuehling
@ 2021-11-09 5:26 ` Lang Yu
2021-11-09 6:12 ` Errabolu, Ramesh
1 sibling, 1 reply; 10+ messages in thread
From: Lang Yu @ 2021-11-09 5:26 UTC (permalink / raw)
To: Ramesh Errabolu; +Cc: amd-gfx
On Mon, Nov 08, 2021 at 07:37:44PM -0600, Ramesh Errabolu wrote:
> MMIO/DOORBELL BOs encode control data and should be pinned in GTT
> domain before enabling PCIe connected peer devices in accessing it
>
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 25 +++++++++
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 55 +++++++++++++++++++
> 2 files changed, 80 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 4c1d6536a7a5..d9a1cfd7876f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -300,6 +300,31 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
> uint64_t va, void *drm_priv,
> struct kgd_mem **mem, uint64_t *size,
> uint64_t *mmap_offset);
> +
> +/**
> + * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
> + * @bo: Handle of buffer object being pinned
> + * @domain: Domain into which BO should be pinned
> + *
> + * - USERPTR BOs are UNPINNABLE and will return error
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count incremented. It is valid to PIN a BO multiple times
> + *
> + * Return: ZERO if successful in pinning, Non-Zero in case of error.
> + * Will return -EINVAL if input BO parameter is a USERPTR type.
> + */
> +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain);
> +
> +/**
> + * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following criteria
> + * @bo: Handle of buffer object being unpinned
> + *
> + * - Is a illegal request for USERPTR BOs and is ignored
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count decremented. Calls to UNPIN must balance calls to PIN
> + */
> +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo);
> +
> int amdgpu_amdkfd_get_tile_config(struct amdgpu_device *adev,
> struct tile_config *config);
> void amdgpu_amdkfd_ras_poison_consumption_handler(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 4fa814358552..f4ffc41873dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1299,6 +1299,36 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
> return ret;
> }
>
> +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain)
> +{
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return ret;
> +
> + ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
> + if (ret)
> + pr_err("Error in Pinning BO to domain: %d\n", domain);
> +
> + amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
> + amdgpu_bo_unreserve(bo);
> +
> + return ret;
> +}
> +
> +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo)
> +{
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return;
> +
> + amdgpu_bo_unpin(bo);
> + amdgpu_bo_unreserve(bo);
> +}
> +
> int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> struct file *filp, u32 pasid,
> void **process_info,
> @@ -1525,6 +1555,23 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> if (offset)
> *offset = amdgpu_bo_mmap_offset(bo);
>
> + if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + ret = amdgpu_amdkfd_bo_validate(bo, AMDGPU_GEM_DOMAIN_GTT, false);
> + if (ret) {
> + pr_err("Validating MMIO/DOORBELL BO during ALLOC FAILED\n");
> + goto err_node_allow;
> + }
amdgpu_amdkfd_gpuvm_pin_bo() will do ttm_bo_validate(),
amdgpu_amdkfd_bo_validate() seems redundant here.
Regards,
Lang
> +
> + ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
> + if (ret) {
> + pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
> + goto err_node_allow;
> + }
> + bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
> + bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
> + }
> +
> return 0;
>
> allocate_init_user_pages_failed:
> @@ -1561,6 +1608,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> bool is_imported = false;
>
> mutex_lock(&mem->lock);
> +
> + /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
> + if (mem->alloc_flags &
> + (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
> + }
> +
> mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
> is_imported = mem->is_imported;
> mutex_unlock(&mem->lock);
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
2021-11-09 5:26 ` Lang Yu
@ 2021-11-09 6:12 ` Errabolu, Ramesh
2021-11-09 6:44 ` Lang Yu
0 siblings, 1 reply; 10+ messages in thread
From: Errabolu, Ramesh @ 2021-11-09 6:12 UTC (permalink / raw)
To: Yu, Lang; +Cc: amd-gfx
[AMD Official Use Only]
Responses in line
-----Original Message-----
From: Yu, Lang <Lang.Yu@amd.com>
Sent: Monday, November 8, 2021 11:27 PM
To: Errabolu, Ramesh <Ramesh.Errabolu@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
On Mon, Nov 08, 2021 at 07:37:44PM -0600, Ramesh Errabolu wrote:
> MMIO/DOORBELL BOs encode control data and should be pinned in GTT
> domain before enabling PCIe connected peer devices in accessing it
>
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 25 +++++++++
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 55
> +++++++++++++++++++
> 2 files changed, 80 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 4c1d6536a7a5..d9a1cfd7876f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -300,6 +300,31 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
> uint64_t va, void *drm_priv,
> struct kgd_mem **mem, uint64_t *size,
> uint64_t *mmap_offset);
> +
> +/**
> + * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
> + * @bo: Handle of buffer object being pinned
> + * @domain: Domain into which BO should be pinned
> + *
> + * - USERPTR BOs are UNPINNABLE and will return error
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count incremented. It is valid to PIN a BO multiple times
> + *
> + * Return: ZERO if successful in pinning, Non-Zero in case of error.
> + * Will return -EINVAL if input BO parameter is a USERPTR type.
> + */
> +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain);
> +
> +/**
> + * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following
> +criteria
> + * @bo: Handle of buffer object being unpinned
> + *
> + * - Is a illegal request for USERPTR BOs and is ignored
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count decremented. Calls to UNPIN must balance calls to PIN
> + */
> +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo);
> +
> int amdgpu_amdkfd_get_tile_config(struct amdgpu_device *adev,
> struct tile_config *config);
> void amdgpu_amdkfd_ras_poison_consumption_handler(struct
> amdgpu_device *adev); diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 4fa814358552..f4ffc41873dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1299,6 +1299,36 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
> return ret;
> }
>
> +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain) {
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return ret;
> +
> + ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
> + if (ret)
> + pr_err("Error in Pinning BO to domain: %d\n", domain);
> +
> + amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
> + amdgpu_bo_unreserve(bo);
> +
> + return ret;
> +}
> +
> +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo) {
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return;
> +
> + amdgpu_bo_unpin(bo);
> + amdgpu_bo_unreserve(bo);
> +}
> +
> int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> struct file *filp, u32 pasid,
> void **process_info,
> @@ -1525,6 +1555,23 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> if (offset)
> *offset = amdgpu_bo_mmap_offset(bo);
>
> + if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + ret = amdgpu_amdkfd_bo_validate(bo, AMDGPU_GEM_DOMAIN_GTT, false);
> + if (ret) {
> + pr_err("Validating MMIO/DOORBELL BO during ALLOC FAILED\n");
> + goto err_node_allow;
> + }
amdgpu_amdkfd_gpuvm_pin_bo() will do ttm_bo_validate(),
amdgpu_amdkfd_bo_validate() seems redundant here.
Ramesh: In my experiments I recall seeing an issue if BO was not validated in GTT domain prior to pinning. If my memory serves me correctly, the call to PIN will fail
Regards,
Lang
> +
> + ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
> + if (ret) {
> + pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
> + goto err_node_allow;
> + }
> + bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
> + bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
> + }
> +
> return 0;
>
> allocate_init_user_pages_failed:
> @@ -1561,6 +1608,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> bool is_imported = false;
>
> mutex_lock(&mem->lock);
> +
> + /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
> + if (mem->alloc_flags &
> + (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
> + }
> +
> mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
> is_imported = mem->is_imported;
> mutex_unlock(&mem->lock);
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
2021-11-09 6:12 ` Errabolu, Ramesh
@ 2021-11-09 6:44 ` Lang Yu
2021-11-09 7:31 ` Errabolu, Ramesh
0 siblings, 1 reply; 10+ messages in thread
From: Lang Yu @ 2021-11-09 6:44 UTC (permalink / raw)
To: Errabolu, Ramesh; +Cc: amd-gfx
On Tue, Nov 09, 2021 at 02:12:00PM +0800, Errabolu, Ramesh wrote:
> [AMD Official Use Only]
>
> Responses in line
>
> -----Original Message-----
> From: Yu, Lang <Lang.Yu@amd.com>
> Sent: Monday, November 8, 2021 11:27 PM
> To: Errabolu, Ramesh <Ramesh.Errabolu@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
>
> On Mon, Nov 08, 2021 at 07:37:44PM -0600, Ramesh Errabolu wrote:
> > MMIO/DOORBELL BOs encode control data and should be pinned in GTT
> > domain before enabling PCIe connected peer devices in accessing it
> >
> > Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 25 +++++++++
> > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 55
> > +++++++++++++++++++
> > 2 files changed, 80 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > index 4c1d6536a7a5..d9a1cfd7876f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > @@ -300,6 +300,31 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
> > uint64_t va, void *drm_priv,
> > struct kgd_mem **mem, uint64_t *size,
> > uint64_t *mmap_offset);
> > +
> > +/**
> > + * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
> > + * @bo: Handle of buffer object being pinned
> > + * @domain: Domain into which BO should be pinned
> > + *
> > + * - USERPTR BOs are UNPINNABLE and will return error
> > + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> > + * PIN count incremented. It is valid to PIN a BO multiple times
> > + *
> > + * Return: ZERO if successful in pinning, Non-Zero in case of error.
> > + * Will return -EINVAL if input BO parameter is a USERPTR type.
> > + */
> > +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain);
> > +
> > +/**
> > + * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following
> > +criteria
> > + * @bo: Handle of buffer object being unpinned
> > + *
> > + * - Is a illegal request for USERPTR BOs and is ignored
> > + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> > + * PIN count decremented. Calls to UNPIN must balance calls to PIN
> > + */
> > +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo);
> > +
> > int amdgpu_amdkfd_get_tile_config(struct amdgpu_device *adev,
> > struct tile_config *config);
> > void amdgpu_amdkfd_ras_poison_consumption_handler(struct
> > amdgpu_device *adev); diff --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index 4fa814358552..f4ffc41873dd 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -1299,6 +1299,36 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
> > return ret;
> > }
> >
> > +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain) {
> > + int ret = 0;
> > +
> > + ret = amdgpu_bo_reserve(bo, false);
> > + if (unlikely(ret))
> > + return ret;
> > +
> > + ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
> > + if (ret)
> > + pr_err("Error in Pinning BO to domain: %d\n", domain);
> > +
> > + amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
> > + amdgpu_bo_unreserve(bo);
> > +
> > + return ret;
> > +}
> > +
> > +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo) {
> > + int ret = 0;
> > +
> > + ret = amdgpu_bo_reserve(bo, false);
> > + if (unlikely(ret))
> > + return;
> > +
> > + amdgpu_bo_unpin(bo);
> > + amdgpu_bo_unreserve(bo);
> > +}
> > +
> > int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> > struct file *filp, u32 pasid,
> > void **process_info,
> > @@ -1525,6 +1555,23 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> > if (offset)
> > *offset = amdgpu_bo_mmap_offset(bo);
> >
> > + if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> > + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> > + ret = amdgpu_amdkfd_bo_validate(bo, AMDGPU_GEM_DOMAIN_GTT, false);
> > + if (ret) {
> > + pr_err("Validating MMIO/DOORBELL BO during ALLOC FAILED\n");
> > + goto err_node_allow;
> > + }
>
> amdgpu_amdkfd_gpuvm_pin_bo() will do ttm_bo_validate(),
> amdgpu_amdkfd_bo_validate() seems redundant here.
>
> Ramesh: In my experiments I recall seeing an issue if BO was not validated in GTT domain prior to pinning. If my memory serves me correctly, the call to PIN will fail
From amdgpu_bo_pin_restricted() we can see, pin operation will
If not pinned,
1, validate the BO with requested domain
2, increase pin count and update stats
If already pinned,
1, increase pin count
So seems not necessarily to validate it twice.
Regards,
Lang
> > +
> > + ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
> > + if (ret) {
> > + pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
> > + goto err_node_allow;
> > + }
> > + bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
> > + bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
> > + }
> > +
> > return 0;
> >
> > allocate_init_user_pages_failed:
> > @@ -1561,6 +1608,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> > bool is_imported = false;
> >
> > mutex_lock(&mem->lock);
> > +
> > + /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
> > + if (mem->alloc_flags &
> > + (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> > + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> > + amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
> > + }
> > +
> > mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
> > is_imported = mem->is_imported;
> > mutex_unlock(&mem->lock);
> > --
> > 2.31.1
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
2021-11-09 6:44 ` Lang Yu
@ 2021-11-09 7:31 ` Errabolu, Ramesh
2021-11-09 19:24 ` Errabolu, Ramesh
0 siblings, 1 reply; 10+ messages in thread
From: Errabolu, Ramesh @ 2021-11-09 7:31 UTC (permalink / raw)
To: Yu, Lang; +Cc: amd-gfx
[-- Attachment #1: Type: text/plain, Size: 6816 bytes --]
[AMD Official Use Only]
I will experiment to see if it is not needed. Will update patch based on the results.
Regards,
Ramesh
________________________________
From: Yu, Lang <Lang.Yu@amd.com>
Sent: Tuesday, November 9, 2021 12:44 AM
To: Errabolu, Ramesh <Ramesh.Errabolu@amd.com>
Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
On Tue, Nov 09, 2021 at 02:12:00PM +0800, Errabolu, Ramesh wrote:
> [AMD Official Use Only]
>
> Responses in line
>
> -----Original Message-----
> From: Yu, Lang <Lang.Yu@amd.com>
> Sent: Monday, November 8, 2021 11:27 PM
> To: Errabolu, Ramesh <Ramesh.Errabolu@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
>
> On Mon, Nov 08, 2021 at 07:37:44PM -0600, Ramesh Errabolu wrote:
> > MMIO/DOORBELL BOs encode control data and should be pinned in GTT
> > domain before enabling PCIe connected peer devices in accessing it
> >
> > Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 25 +++++++++
> > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 55
> > +++++++++++++++++++
> > 2 files changed, 80 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > index 4c1d6536a7a5..d9a1cfd7876f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > @@ -300,6 +300,31 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
> > uint64_t va, void *drm_priv,
> > struct kgd_mem **mem, uint64_t *size,
> > uint64_t *mmap_offset);
> > +
> > +/**
> > + * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
> > + * @bo: Handle of buffer object being pinned
> > + * @domain: Domain into which BO should be pinned
> > + *
> > + * - USERPTR BOs are UNPINNABLE and will return error
> > + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> > + * PIN count incremented. It is valid to PIN a BO multiple times
> > + *
> > + * Return: ZERO if successful in pinning, Non-Zero in case of error.
> > + * Will return -EINVAL if input BO parameter is a USERPTR type.
> > + */
> > +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain);
> > +
> > +/**
> > + * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following
> > +criteria
> > + * @bo: Handle of buffer object being unpinned
> > + *
> > + * - Is a illegal request for USERPTR BOs and is ignored
> > + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> > + * PIN count decremented. Calls to UNPIN must balance calls to PIN
> > + */
> > +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo);
> > +
> > int amdgpu_amdkfd_get_tile_config(struct amdgpu_device *adev,
> > struct tile_config *config);
> > void amdgpu_amdkfd_ras_poison_consumption_handler(struct
> > amdgpu_device *adev); diff --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index 4fa814358552..f4ffc41873dd 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -1299,6 +1299,36 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
> > return ret;
> > }
> >
> > +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain) {
> > + int ret = 0;
> > +
> > + ret = amdgpu_bo_reserve(bo, false);
> > + if (unlikely(ret))
> > + return ret;
> > +
> > + ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
> > + if (ret)
> > + pr_err("Error in Pinning BO to domain: %d\n", domain);
> > +
> > + amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
> > + amdgpu_bo_unreserve(bo);
> > +
> > + return ret;
> > +}
> > +
> > +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo) {
> > + int ret = 0;
> > +
> > + ret = amdgpu_bo_reserve(bo, false);
> > + if (unlikely(ret))
> > + return;
> > +
> > + amdgpu_bo_unpin(bo);
> > + amdgpu_bo_unreserve(bo);
> > +}
> > +
> > int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> > struct file *filp, u32 pasid,
> > void **process_info,
> > @@ -1525,6 +1555,23 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> > if (offset)
> > *offset = amdgpu_bo_mmap_offset(bo);
> >
> > + if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> > + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> > + ret = amdgpu_amdkfd_bo_validate(bo, AMDGPU_GEM_DOMAIN_GTT, false);
> > + if (ret) {
> > + pr_err("Validating MMIO/DOORBELL BO during ALLOC FAILED\n");
> > + goto err_node_allow;
> > + }
>
> amdgpu_amdkfd_gpuvm_pin_bo() will do ttm_bo_validate(),
> amdgpu_amdkfd_bo_validate() seems redundant here.
>
> Ramesh: In my experiments I recall seeing an issue if BO was not validated in GTT domain prior to pinning. If my memory serves me correctly, the call to PIN will fail
From amdgpu_bo_pin_restricted() we can see, pin operation will
If not pinned,
1, validate the BO with requested domain
2, increase pin count and update stats
If already pinned,
1, increase pin count
So seems not necessarily to validate it twice.
Regards,
Lang
> > +
> > + ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
> > + if (ret) {
> > + pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
> > + goto err_node_allow;
> > + }
> > + bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
> > + bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
> > + }
> > +
> > return 0;
> >
> > allocate_init_user_pages_failed:
> > @@ -1561,6 +1608,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> > bool is_imported = false;
> >
> > mutex_lock(&mem->lock);
> > +
> > + /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
> > + if (mem->alloc_flags &
> > + (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> > + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> > + amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
> > + }
> > +
> > mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
> > is_imported = mem->is_imported;
> > mutex_unlock(&mem->lock);
> > --
> > 2.31.1
> >
[-- Attachment #2: Type: text/html, Size: 12296 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
2021-11-09 7:31 ` Errabolu, Ramesh
@ 2021-11-09 19:24 ` Errabolu, Ramesh
0 siblings, 0 replies; 10+ messages in thread
From: Errabolu, Ramesh @ 2021-11-09 19:24 UTC (permalink / raw)
To: Yu, Lang; +Cc: amd-gfx
[-- Attachment #1: Type: text/plain, Size: 7667 bytes --]
[AMD Official Use Only]
Based on my experiments I am able conclude that I can avoid validating the BO prior to pinning it. I don't have the code history that led me to validating the BO in the first place. In any case I posted an updated patch to the DRM-NEXT branch in addition to a standalone patch for the DKMS branch as well.
Thanks for pointing this out.
Regards,
Ramesh
From: Errabolu, Ramesh <Ramesh.Errabolu@amd.com>
Sent: Tuesday, November 9, 2021 1:32 AM
To: Yu, Lang <Lang.Yu@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
I will experiment to see if it is not needed. Will update patch based on the results.
Regards,
Ramesh
________________________________
From: Yu, Lang <Lang.Yu@amd.com<mailto:Lang.Yu@amd.com>>
Sent: Tuesday, November 9, 2021 12:44 AM
To: Errabolu, Ramesh <Ramesh.Errabolu@amd.com<mailto:Ramesh.Errabolu@amd.com>>
Cc: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
On Tue, Nov 09, 2021 at 02:12:00PM +0800, Errabolu, Ramesh wrote:
> [AMD Official Use Only]
>
> Responses in line
>
> -----Original Message-----
> From: Yu, Lang <Lang.Yu@amd.com<mailto:Lang.Yu@amd.com>>
> Sent: Monday, November 8, 2021 11:27 PM
> To: Errabolu, Ramesh <Ramesh.Errabolu@amd.com<mailto:Ramesh.Errabolu@amd.com>>
> Cc: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
>
> On Mon, Nov 08, 2021 at 07:37:44PM -0600, Ramesh Errabolu wrote:
> > MMIO/DOORBELL BOs encode control data and should be pinned in GTT
> > domain before enabling PCIe connected peer devices in accessing it
> >
> > Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com<mailto:Ramesh.Errabolu@amd.com>>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 25 +++++++++
> > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 55
> > +++++++++++++++++++
> > 2 files changed, 80 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > index 4c1d6536a7a5..d9a1cfd7876f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> > @@ -300,6 +300,31 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct amdgpu_device *adev,
> > uint64_t va, void *drm_priv,
> > struct kgd_mem **mem, uint64_t *size,
> > uint64_t *mmap_offset);
> > +
> > +/**
> > + * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
> > + * @bo: Handle of buffer object being pinned
> > + * @domain: Domain into which BO should be pinned
> > + *
> > + * - USERPTR BOs are UNPINNABLE and will return error
> > + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> > + * PIN count incremented. It is valid to PIN a BO multiple times
> > + *
> > + * Return: ZERO if successful in pinning, Non-Zero in case of error.
> > + * Will return -EINVAL if input BO parameter is a USERPTR type.
> > + */
> > +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain);
> > +
> > +/**
> > + * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following
> > +criteria
> > + * @bo: Handle of buffer object being unpinned
> > + *
> > + * - Is a illegal request for USERPTR BOs and is ignored
> > + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> > + * PIN count decremented. Calls to UNPIN must balance calls to PIN
> > + */
> > +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo);
> > +
> > int amdgpu_amdkfd_get_tile_config(struct amdgpu_device *adev,
> > struct tile_config *config);
> > void amdgpu_amdkfd_ras_poison_consumption_handler(struct
> > amdgpu_device *adev); diff --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index 4fa814358552..f4ffc41873dd 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -1299,6 +1299,36 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
> > return ret;
> > }
> >
> > +int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain) {
> > + int ret = 0;
> > +
> > + ret = amdgpu_bo_reserve(bo, false);
> > + if (unlikely(ret))
> > + return ret;
> > +
> > + ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
> > + if (ret)
> > + pr_err("Error in Pinning BO to domain: %d\n", domain);
> > +
> > + amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
> > + amdgpu_bo_unreserve(bo);
> > +
> > + return ret;
> > +}
> > +
> > +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo) {
> > + int ret = 0;
> > +
> > + ret = amdgpu_bo_reserve(bo, false);
> > + if (unlikely(ret))
> > + return;
> > +
> > + amdgpu_bo_unpin(bo);
> > + amdgpu_bo_unreserve(bo);
> > +}
> > +
> > int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> > struct file *filp, u32 pasid,
> > void **process_info,
> > @@ -1525,6 +1555,23 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> > if (offset)
> > *offset = amdgpu_bo_mmap_offset(bo);
> >
> > + if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> > + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> > + ret = amdgpu_amdkfd_bo_validate(bo, AMDGPU_GEM_DOMAIN_GTT, false);
> > + if (ret) {
> > + pr_err("Validating MMIO/DOORBELL BO during ALLOC FAILED\n");
> > + goto err_node_allow;
> > + }
>
> amdgpu_amdkfd_gpuvm_pin_bo() will do ttm_bo_validate(),
> amdgpu_amdkfd_bo_validate() seems redundant here.
>
> Ramesh: In my experiments I recall seeing an issue if BO was not validated in GTT domain prior to pinning. If my memory serves me correctly, the call to PIN will fail
From amdgpu_bo_pin_restricted() we can see, pin operation will
If not pinned,
1, validate the BO with requested domain
2, increase pin count and update stats
If already pinned,
1, increase pin count
So seems not necessarily to validate it twice.
Regards,
Lang
> > +
> > + ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
> > + if (ret) {
> > + pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
> > + goto err_node_allow;
> > + }
> > + bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
> > + bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
> > + }
> > +
> > return 0;
> >
> > allocate_init_user_pages_failed:
> > @@ -1561,6 +1608,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> > bool is_imported = false;
> >
> > mutex_lock(&mem->lock);
> > +
> > + /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
> > + if (mem->alloc_flags &
> > + (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> > + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> > + amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
> > + }
> > +
> > mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
> > is_imported = mem->is_imported;
> > mutex_unlock(&mem->lock);
> > --
> > 2.31.1
> >
[-- Attachment #2: Type: text/html, Size: 16043 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
2021-11-09 19:12 Ramesh Errabolu
@ 2021-11-10 3:06 ` Felix Kuehling
0 siblings, 0 replies; 10+ messages in thread
From: Felix Kuehling @ 2021-11-10 3:06 UTC (permalink / raw)
To: Ramesh Errabolu, amd-gfx
On 2021-11-09 2:12 p.m., Ramesh Errabolu wrote:
> MMIO/DOORBELL BOs encode control data and should be pinned in GTT
> domain before enabling PCIe connected peer devices in accessing it
>
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 70 +++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 08675f89bfb2..5e063fac0250 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1297,6 +1297,56 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
> return ret;
> }
>
> +/**
> + * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
> + * @bo: Handle of buffer object being pinned
> + * @domain: Domain into which BO should be pinned
> + *
> + * - USERPTR BOs are UNPINNABLE and will return error
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count incremented. It is valid to PIN a BO multiple times
> + *
> + * Return: ZERO if successful in pinning, Non-Zero in case of error.
> + * Will return -EINVAL if input BO parameter is a USERPTR type.
> + */
> +static int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain)
> +{
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return ret;
> +
> + ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
> + if (ret)
> + pr_err("Error in Pinning BO to domain: %d\n", domain);
> +
> + amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
> + amdgpu_bo_unreserve(bo);
> +
> + return ret;
> +}
> +
> +/**
> + * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following criteria
> + * @bo: Handle of buffer object being unpinned
> + *
> + * - Is a illegal request for USERPTR BOs and is ignored
> + * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
> + * PIN count decremented. Calls to UNPIN must balance calls to PIN
> + */
> +void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo)
> +{
> + int ret = 0;
> +
> + ret = amdgpu_bo_reserve(bo, false);
> + if (unlikely(ret))
> + return;
> +
> + amdgpu_bo_unpin(bo);
> + amdgpu_bo_unreserve(bo);
> +}
> +
> int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
> struct file *filp, u32 pasid,
> void **process_info,
> @@ -1523,10 +1573,22 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> if (offset)
> *offset = amdgpu_bo_mmap_offset(bo);
>
> + if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
> + if (ret) {
> + pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
> + goto err_pin_bo;
> + }
> + bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
> + bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
> + }
> +
> return 0;
>
> allocate_init_user_pages_failed:
> remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
> +err_pin_bo:
> drm_vma_node_revoke(&gobj->vma_node, drm_priv);
> err_node_allow:
> drm_gem_object_put(gobj);
> @@ -1559,6 +1621,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> bool is_imported = false;
>
> mutex_lock(&mem->lock);
> +
> + /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
> + if (mem->alloc_flags &
> + (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
> + KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
> + amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
> + }
> +
> mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
> is_imported = mem->is_imported;
> mutex_unlock(&mem->lock);
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain
@ 2021-11-09 19:12 Ramesh Errabolu
2021-11-10 3:06 ` Felix Kuehling
0 siblings, 1 reply; 10+ messages in thread
From: Ramesh Errabolu @ 2021-11-09 19:12 UTC (permalink / raw)
To: amd-gfx; +Cc: Ramesh Errabolu
MMIO/DOORBELL BOs encode control data and should be pinned in GTT
domain before enabling PCIe connected peer devices in accessing it
Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 70 +++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 08675f89bfb2..5e063fac0250 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1297,6 +1297,56 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
return ret;
}
+/**
+ * amdgpu_amdkfd_gpuvm_pin_bo() - Pins a BO using following criteria
+ * @bo: Handle of buffer object being pinned
+ * @domain: Domain into which BO should be pinned
+ *
+ * - USERPTR BOs are UNPINNABLE and will return error
+ * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
+ * PIN count incremented. It is valid to PIN a BO multiple times
+ *
+ * Return: ZERO if successful in pinning, Non-Zero in case of error.
+ * Will return -EINVAL if input BO parameter is a USERPTR type.
+ */
+static int amdgpu_amdkfd_gpuvm_pin_bo(struct amdgpu_bo *bo, u32 domain)
+{
+ int ret = 0;
+
+ ret = amdgpu_bo_reserve(bo, false);
+ if (unlikely(ret))
+ return ret;
+
+ ret = amdgpu_bo_pin_restricted(bo, domain, 0, 0);
+ if (ret)
+ pr_err("Error in Pinning BO to domain: %d\n", domain);
+
+ amdgpu_bo_sync_wait(bo, AMDGPU_FENCE_OWNER_KFD, false);
+ amdgpu_bo_unreserve(bo);
+
+ return ret;
+}
+
+/**
+ * amdgpu_amdkfd_gpuvm_unpin_bo() - Unpins BO using following criteria
+ * @bo: Handle of buffer object being unpinned
+ *
+ * - Is a illegal request for USERPTR BOs and is ignored
+ * - All other BO types (GTT, VRAM, MMIO and DOORBELL) will have their
+ * PIN count decremented. Calls to UNPIN must balance calls to PIN
+ */
+void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo)
+{
+ int ret = 0;
+
+ ret = amdgpu_bo_reserve(bo, false);
+ if (unlikely(ret))
+ return;
+
+ amdgpu_bo_unpin(bo);
+ amdgpu_bo_unreserve(bo);
+}
+
int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev,
struct file *filp, u32 pasid,
void **process_info,
@@ -1523,10 +1573,22 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
if (offset)
*offset = amdgpu_bo_mmap_offset(bo);
+ if (flags & (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
+ KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
+ ret = amdgpu_amdkfd_gpuvm_pin_bo(bo, AMDGPU_GEM_DOMAIN_GTT);
+ if (ret) {
+ pr_err("Pinning MMIO/DOORBELL BO during ALLOC FAILED\n");
+ goto err_pin_bo;
+ }
+ bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
+ bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
+ }
+
return 0;
allocate_init_user_pages_failed:
remove_kgd_mem_from_kfd_bo_list(*mem, avm->process_info);
+err_pin_bo:
drm_vma_node_revoke(&gobj->vma_node, drm_priv);
err_node_allow:
drm_gem_object_put(gobj);
@@ -1559,6 +1621,14 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
bool is_imported = false;
mutex_lock(&mem->lock);
+
+ /* Unpin MMIO/DOORBELL BO's that were pinnned during allocation */
+ if (mem->alloc_flags &
+ (KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL |
+ KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP)) {
+ amdgpu_amdkfd_gpuvm_unpin_bo(mem->bo);
+ }
+
mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
is_imported = mem->is_imported;
mutex_unlock(&mem->lock);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-10 3:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09 1:37 [PATCH] drm/amdgpu: Pin MMIO/DOORBELL BO's in GTT domain Ramesh Errabolu
2021-11-09 4:51 ` Felix Kuehling
2021-11-09 5:13 ` Errabolu, Ramesh
2021-11-09 5:26 ` Lang Yu
2021-11-09 6:12 ` Errabolu, Ramesh
2021-11-09 6:44 ` Lang Yu
2021-11-09 7:31 ` Errabolu, Ramesh
2021-11-09 19:24 ` Errabolu, Ramesh
2021-11-09 19:12 Ramesh Errabolu
2021-11-10 3:06 ` Felix Kuehling
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.