[PATCH 1/2] drm/amdkfd: set prefetch GPU has access attribute

[PATCH 1/2] drm/amdkfd: set prefetch GPU has access attribute

[Philip Yang]

After svm range prefetch and migrate to GPU, if GPU has no access or
access in place attribute, add GPU to range access_bitmap to be able to
update GPU page table.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index d9111fea724b..3b640772d9c5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2869,6 +2869,17 @@ svm_range_set_attr(struct kfd_process *p, uint64_t start, uint64_t size,
 			continue;
 		}
 
+		if (migrated && prange->prefetch_loc) {
+			int gpuidx;
+
+			gpuidx = kfd_process_gpuidx_from_gpuid(p,
+							prange->prefetch_loc);
+			if (gpuidx >= 0 &&
+			    !test_bit(gpuidx, prange->bitmap_access) &&
+			    !test_bit(gpuidx, prange->bitmap_aip))
+				bitmap_set(prange->bitmap_access, gpuidx, 1);
+		}
+
 		r = svm_range_validate_and_map(mm, prange, MAX_GPU_INSTANCE,
 					       true, true);
 		if (r)
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdkfd: set new unregistered range accessible by GPU

[Philip Yang]

GPU retry vm fault on unregistered address, new range is created to
recover the retry vm fault. Instead of setting new range perferred_loc
to GPU, add GPU to new range access_bitmap, to be able to update GPU
page table after new range migrate to VRAM. The new range preferred_loc
is default value KFD_IOCTL_SVM_LOCATION_UNDEFINED.

Correct one typo.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 3b640772d9c5..5a486e0fbf18 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2243,7 +2243,7 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
 
 	prange = svm_range_new(&p->svms, start, last);
 	if (!prange) {
-		pr_debug("Failed to create prange in address [0x%llx]\\n", addr);
+		pr_debug("Failed to create prange in address [0x%llx]\n", addr);
 		return NULL;
 	}
 	if (kfd_process_gpuid_from_kgd(p, adev, &gpuid, &gpuidx)) {
@@ -2251,9 +2251,8 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
 		svm_range_free(prange);
 		return NULL;
 	}
-	prange->preferred_loc = gpuid;
-	prange->actual_loc = 0;
-	/* Gurantee prange is migrate it */
+
+	bitmap_set(prange->bitmap_access, gpuidx, 1);
 	svm_range_add_to_svms(prange);
 	svm_range_add_notifier_locked(mm, prange);
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdkfd: set new unregistered range accessible by GPU

[Felix Kuehling]

Am 2021-05-05 um 1:56 p.m. schrieb Philip Yang:
> GPU retry vm fault on unregistered address, new range is created to
> recover the retry vm fault. Instead of setting new range perferred_loc
> to GPU, add GPU to new range access_bitmap, to be able to update GPU
> page table after new range migrate to VRAM. The new range preferred_loc
> is default value KFD_IOCTL_SVM_LOCATION_UNDEFINED.
>
> Correct one typo.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 3b640772d9c5..5a486e0fbf18 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2243,7 +2243,7 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
>  
>  	prange = svm_range_new(&p->svms, start, last);
>  	if (!prange) {
> -		pr_debug("Failed to create prange in address [0x%llx]\\n", addr);
> +		pr_debug("Failed to create prange in address [0x%llx]\n", addr);
>  		return NULL;
>  	}
>  	if (kfd_process_gpuid_from_kgd(p, adev, &gpuid, &gpuidx)) {
> @@ -2251,9 +2251,8 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
>  		svm_range_free(prange);
>  		return NULL;
>  	}
> -	prange->preferred_loc = gpuid;
> -	prange->actual_loc = 0;
> -	/* Gurantee prange is migrate it */
> +
> +	bitmap_set(prange->bitmap_access, gpuidx, 1);

By default, with XNACK enabled, the default should be that all GPUs have
access to memory (but not access in place).

Regards,
  Felix


>  	svm_range_add_to_svms(prange);
>  	svm_range_add_notifier_locked(mm, prange);
>  
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdkfd: unregistered range accessible by all GPUs

[Philip Yang]

New range is created to recover retry vm fault, set all GPUs have access
to the range. The new range preferred_loc is default value
KFD_IOCTL_SVM_LOCATION_UNDEFINED.

Correct one typo.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index d9111fea724b..537b12e75f54 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2243,7 +2243,7 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
 
 	prange = svm_range_new(&p->svms, start, last);
 	if (!prange) {
-		pr_debug("Failed to create prange in address [0x%llx]\\n", addr);
+		pr_debug("Failed to create prange in address [0x%llx]\n", addr);
 		return NULL;
 	}
 	if (kfd_process_gpuid_from_kgd(p, adev, &gpuid, &gpuidx)) {
@@ -2251,9 +2251,8 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
 		svm_range_free(prange);
 		return NULL;
 	}
-	prange->preferred_loc = gpuid;
-	prange->actual_loc = 0;
-	/* Gurantee prange is migrate it */
+
+	bitmap_fill(prange->bitmap_access, MAX_GPU_INSTANCE);
 	svm_range_add_to_svms(prange);
 	svm_range_add_notifier_locked(mm, prange);
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdkfd: unregistered range accessible by all GPUs

[Felix Kuehling]

Am 2021-05-07 um 3:07 p.m. schrieb Philip Yang:
> New range is created to recover retry vm fault, set all GPUs have access
> to the range. The new range preferred_loc is default value
> KFD_IOCTL_SVM_LOCATION_UNDEFINED.
>
> Correct one typo.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>

Would it be better to move this into svm_range_new, conditional on
p->xnack_enabled? That way it would correctly apply to ranges created
through SVM API calls (e.g. in svm_range_handle_overlap or
svm_range_add) as well?

Regards,
  Felix


> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index d9111fea724b..537b12e75f54 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2243,7 +2243,7 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
>  
>  	prange = svm_range_new(&p->svms, start, last);
>  	if (!prange) {
> -		pr_debug("Failed to create prange in address [0x%llx]\\n", addr);
> +		pr_debug("Failed to create prange in address [0x%llx]\n", addr);
>  		return NULL;
>  	}
>  	if (kfd_process_gpuid_from_kgd(p, adev, &gpuid, &gpuidx)) {
> @@ -2251,9 +2251,8 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
>  		svm_range_free(prange);
>  		return NULL;
>  	}
> -	prange->preferred_loc = gpuid;
> -	prange->actual_loc = 0;
> -	/* Gurantee prange is migrate it */
> +
> +	bitmap_fill(prange->bitmap_access, MAX_GPU_INSTANCE);
>  	svm_range_add_to_svms(prange);
>  	svm_range_add_notifier_locked(mm, prange);
>  
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdkfd: unregistered range accessible by all GPUs

[philip yang]

[-- Attachment #1: Type: text/html, Size: 2640 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 2/2] drm/amdkfd: new range accessible by all GPUs

[Philip Yang]

If xnack is on, new range is created to recover retry vm fault or
created by SVM API calls, set all GPUs have access to the range.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index d9111fea724b..6a677bdfcadb 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -258,6 +258,7 @@ svm_range *svm_range_new(struct svm_range_list *svms, uint64_t start,
 {
 	uint64_t size = last - start + 1;
 	struct svm_range *prange;
+	struct kfd_process *p;
 
 	prange = kzalloc(sizeof(*prange), GFP_KERNEL);
 	if (!prange)
@@ -277,6 +278,11 @@ svm_range *svm_range_new(struct svm_range_list *svms, uint64_t start,
 	prange->validate_timestamp = 0;
 	mutex_init(&prange->migrate_mutex);
 	mutex_init(&prange->lock);
+
+	p = container_of(svms, struct kfd_process, svms);
+	if (p->xnack_enabled)
+		bitmap_fill(prange->bitmap_access, MAX_GPU_INSTANCE);
+
 	svm_range_set_default_attributes(&prange->preferred_loc,
 					 &prange->prefetch_loc,
 					 &prange->granularity, &prange->flags);
@@ -2243,7 +2249,7 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
 
 	prange = svm_range_new(&p->svms, start, last);
 	if (!prange) {
-		pr_debug("Failed to create prange in address [0x%llx]\\n", addr);
+		pr_debug("Failed to create prange in address [0x%llx]\n", addr);
 		return NULL;
 	}
 	if (kfd_process_gpuid_from_kgd(p, adev, &gpuid, &gpuidx)) {
@@ -2251,9 +2257,7 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
 		svm_range_free(prange);
 		return NULL;
 	}
-	prange->preferred_loc = gpuid;
-	prange->actual_loc = 0;
-	/* Gurantee prange is migrate it */
+
 	svm_range_add_to_svms(prange);
 	svm_range_add_notifier_locked(mm, prange);
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/2] drm/amdkfd: new range accessible by all GPUs

[Felix Kuehling]

Am 2021-05-11 um 11:15 a.m. schrieb Philip Yang:
> If xnack is on, new range is created to recover retry vm fault or
> created by SVM API calls, set all GPUs have access to the range.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index d9111fea724b..6a677bdfcadb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -258,6 +258,7 @@ svm_range *svm_range_new(struct svm_range_list *svms, uint64_t start,
>  {
>  	uint64_t size = last - start + 1;
>  	struct svm_range *prange;
> +	struct kfd_process *p;
>  
>  	prange = kzalloc(sizeof(*prange), GFP_KERNEL);
>  	if (!prange)
> @@ -277,6 +278,11 @@ svm_range *svm_range_new(struct svm_range_list *svms, uint64_t start,
>  	prange->validate_timestamp = 0;
>  	mutex_init(&prange->migrate_mutex);
>  	mutex_init(&prange->lock);
> +
> +	p = container_of(svms, struct kfd_process, svms);
> +	if (p->xnack_enabled)
> +		bitmap_fill(prange->bitmap_access, MAX_GPU_INSTANCE);
> +
>  	svm_range_set_default_attributes(&prange->preferred_loc,
>  					 &prange->prefetch_loc,
>  					 &prange->granularity, &prange->flags);
> @@ -2243,7 +2249,7 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
>  
>  	prange = svm_range_new(&p->svms, start, last);
>  	if (!prange) {
> -		pr_debug("Failed to create prange in address [0x%llx]\\n", addr);
> +		pr_debug("Failed to create prange in address [0x%llx]\n", addr);
>  		return NULL;
>  	}
>  	if (kfd_process_gpuid_from_kgd(p, adev, &gpuid, &gpuidx)) {
> @@ -2251,9 +2257,7 @@ svm_range *svm_range_create_unregistered_range(struct amdgpu_device *adev,
>  		svm_range_free(prange);
>  		return NULL;
>  	}
> -	prange->preferred_loc = gpuid;
> -	prange->actual_loc = 0;
> -	/* Gurantee prange is migrate it */
> +
>  	svm_range_add_to_svms(prange);
>  	svm_range_add_notifier_locked(mm, prange);
>  
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdkfd: set prefetch GPU has access attribute

[Felix Kuehling]

Am 2021-05-05 um 1:56 p.m. schrieb Philip Yang:
> After svm range prefetch and migrate to GPU, if GPU has no access or
> access in place attribute, add GPU to range access_bitmap to be able to
> update GPU page table.

I don't think we should change the bitmap_access, because this is
persistent and can be queried by user mode. Changing this would not be
expected by the application. I think instead we should either pass the
prefetch location to the gpuidx parameter of validate_and_map (we should
only do that is xnack is enabled), or change the logic in
validate_and_map to add the prefetch location to the bitmap.

I think to make this reliable we'll need to add more sanity checks to
prefetch to make sure the prefetch location is compatible with all GPUs
that have access_in_place. Otherwise we have to reject the prefetch.

Regards,
  Felix


>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index d9111fea724b..3b640772d9c5 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2869,6 +2869,17 @@ svm_range_set_attr(struct kfd_process *p, uint64_t start, uint64_t size,
>  			continue;
>  		}
>  
> +		if (migrated && prange->prefetch_loc) {
> +			int gpuidx;
> +
> +			gpuidx = kfd_process_gpuidx_from_gpuid(p,
> +							prange->prefetch_loc);
> +			if (gpuidx >= 0 &&
> +			    !test_bit(gpuidx, prange->bitmap_access) &&
> +			    !test_bit(gpuidx, prange->bitmap_aip))
> +				bitmap_set(prange->bitmap_access, gpuidx, 1);
> +		}
> +
>  		r = svm_range_validate_and_map(mm, prange, MAX_GPU_INSTANCE,
>  					       true, true);
>  		if (r)
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdkfd: set prefetch GPU has access attribute

[Felix Kuehling]

Am 2021-05-05 um 4:01 p.m. schrieb Felix Kuehling:
> Am 2021-05-05 um 1:56 p.m. schrieb Philip Yang:
>> After svm range prefetch and migrate to GPU, if GPU has no access or
>> access in place attribute, add GPU to range access_bitmap to be able to
>> update GPU page table.
> I don't think we should change the bitmap_access, because this is
> persistent and can be queried by user mode. Changing this would not be
> expected by the application. I think instead we should either pass the
> prefetch location to the gpuidx parameter of validate_and_map (we should
> only do that is xnack is enabled), or change the logic in
> validate_and_map to add the prefetch location to the bitmap.

Actually, I don't think we need any special rules here. If the memory is
not accessible by the prefetch GPU, the memory should not get mapped. It
doesn't sound like a very useful thing to do, but that's how the API is
defined.

Memory should get mapped to the local GPU (if it has access) and to any
other GPUs that have access-in-place.

If you're working around a problem of a range not being accessible after
it was created in a page fault, then this needs to be fixed elsewhere.
With XNACK enabled, all GPUs should have access to all memory by
default. If the bitmap_access is not set on a new range created in a
page fault, this is a bug in how we created that range.

Regards,
  Felix


>
> I think to make this reliable we'll need to add more sanity checks to
> prefetch to make sure the prefetch location is compatible with all GPUs
> that have access_in_place. Otherwise we have to reject the prefetch.
>
> Regards,
>   Felix
>
>
>> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> index d9111fea724b..3b640772d9c5 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>> @@ -2869,6 +2869,17 @@ svm_range_set_attr(struct kfd_process *p, uint64_t start, uint64_t size,
>>  			continue;
>>  		}
>>  
>> +		if (migrated && prange->prefetch_loc) {
>> +			int gpuidx;
>> +
>> +			gpuidx = kfd_process_gpuidx_from_gpuid(p,
>> +							prange->prefetch_loc);
>> +			if (gpuidx >= 0 &&
>> +			    !test_bit(gpuidx, prange->bitmap_access) &&
>> +			    !test_bit(gpuidx, prange->bitmap_aip))
>> +				bitmap_set(prange->bitmap_access, gpuidx, 1);
>> +		}
>> +
>>  		r = svm_range_validate_and_map(mm, prange, MAX_GPU_INSTANCE,
>>  					       true, true);
>>  		if (r)
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx