Re: [PATCH v2 2/2] drm/amdkfd: Unmap svm range from GPU set to no-access

On 2026-07-27 15:39, Philip Yang wrote:
> When KFD_IOCTL_SVM_ATTR_NO_ACCESS is applied to a GPU that has an SVM
> range mapped, unmap the range from that GPU. Once no GPU maps the range,
> the MMU notifier can skip queue eviction on CPU page faults.
>
> Replace the mapped_to_gpu boolean with bitmap_mapped to track which
> GPUs currently have the range mapped. Set bits in svm_range_map_to_gpus()
> and clear them in svm_range_unmap_from_gpus(). This is separate from
> bitmap_access/bitmap_aip which track user-requested attributes and must
> not be used to determine mapping state.
>
> Add bitmap_needs_unmap to svm_range, set for each GPU given no-access.
> Add svm_range_needs_unmap() to unmap the range from those GPUs when the
> app sets the no-access attribute.
>
> Bump the checkpoint timestamp on unmap so retry faults queued before the
> no-access unmap are dropped instead of restoring the mapping.
>
> v4:
>   - Rename and set prange->mapping_done to false if validate and map not
>     complete successfully (Felix)
> v3:
>   - Correct error handling, support app retry update mapping (Felix)
> v2:
>   - Add bitmap_mapped to not break get_attr (Felix)
>
> Signed-off-by: Philip Yang <[email protected]>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_queue.c |   5 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c   | 107 ++++++++++++++++---------
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.h   |   7 +-
>   3 files changed, 77 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> index b249e7d1af48..25954c2c2d91 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
> @@ -112,12 +112,11 @@ static int kfd_queue_buffer_svm_get(struct kfd_process_device *pdd, u64 addr, u6
>   		if (!prange)
>   			break;
>   
> -		if (!prange->mapped_to_gpu)
> -			break;
> -
>   		r = kfd_process_gpuid_from_node(p, pdd->dev, &gpuid, &gpuidx);
>   		if (r < 0)
>   			break;
> +		if (!test_bit(gpuidx, prange->bitmap_mapped))
> +			break;
>   		if (!test_bit(gpuidx, prange->bitmap_access) &&
>   		    !test_bit(gpuidx, prange->bitmap_aip))
>   			break;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index c10d4edc8813..1a01d089b132 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -826,8 +826,13 @@ svm_range_apply_attrs(struct kfd_process *p, struct svm_range *prange,
>   			gpuidx = kfd_process_gpuidx_from_gpuid(p,
>   							       attrs[i].value);
>   			if (attrs[i].type == KFD_IOCTL_SVM_ATTR_NO_ACCESS) {
> +				svm_range_update_checkpoint_timestamp(p);

In the last version I suggested moving this into svm_range_needs_unmap 
so we only update the checkpoint (and drop interrupts) when anything is 
actually unmapped (after the bitmap_empty check for bitmap_needs_unmap). 
Did that not work for some reason?

Regards,
   Felix


> +
>   				bitmap_clear(prange->bitmap_access, gpuidx, 1);
>   				bitmap_clear(prange->bitmap_aip, gpuidx, 1);
> +
> +				if (test_bit(gpuidx, prange->bitmap_mapped))
> +					bitmap_set(prange->bitmap_needs_unmap, gpuidx, 1);
>   			} else if (attrs[i].type == KFD_IOCTL_SVM_ATTR_ACCESS) {
>   				bitmap_set(prange->bitmap_access, gpuidx, 1);
>   				bitmap_clear(prange->bitmap_aip, gpuidx, 1);
> @@ -1118,9 +1123,10 @@ svm_range_split_adjust(struct svm_range *new, struct svm_range *old,
>   	new->prefetch_loc = old->prefetch_loc;
>   	new->actual_loc = old->actual_loc;
>   	new->granularity = old->granularity;
> -	new->mapped_to_gpu = old->mapped_to_gpu;
> +	new->mapping_done = old->mapping_done;
>   	bitmap_copy(new->bitmap_access, old->bitmap_access, MAX_GPU_INSTANCE);
>   	bitmap_copy(new->bitmap_aip, old->bitmap_aip, MAX_GPU_INSTANCE);
> +	bitmap_copy(new->bitmap_mapped, old->bitmap_mapped, MAX_GPU_INSTANCE);
>   	atomic_set(&new->queue_refcount, atomic_read(&old->queue_refcount));
>   
>   	return 0;
> @@ -1421,7 +1427,8 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   
>   static int
>   svm_range_unmap_from_gpus(struct svm_range *prange, unsigned long start,
> -			  unsigned long last, uint32_t trigger)
> +			  unsigned long last, unsigned long *bitmap_unmap,
> +			  uint32_t trigger)
>   {
>   	struct kfd_process_device *pdd;
>   	struct dma_fence *fence = NULL;
> @@ -1429,21 +1436,15 @@ svm_range_unmap_from_gpus(struct svm_range *prange, unsigned long start,
>   	uint32_t gpuidx;
>   	int r = 0;
>   
> -	if (!prange->mapped_to_gpu) {
> -		pr_debug("prange 0x%p [0x%lx 0x%lx] not mapped to GPU\n",
> -			 prange, prange->start, prange->last);
> -		return 0;
> -	}
> -
> -	if (prange->start == start && prange->last == last) {
> -		pr_debug("unmap svms 0x%p prange 0x%p\n", prange->svms, prange);
> -		prange->mapped_to_gpu = false;
> -	}
> -
>   	p = container_of(prange->svms, struct kfd_process, svms);
>   
> -	for_each_or_bit(gpuidx, prange->bitmap_access, prange->bitmap_aip, MAX_GPU_INSTANCE) {
> -		pr_debug("unmap from gpu idx 0x%x\n", gpuidx);
> +	for_each_set_bit(gpuidx, bitmap_unmap, MAX_GPU_INSTANCE) {
> +		if (prange->start == start && prange->last == last) {
> +			pr_debug("unmap svms 0x%p prange 0x%p from gpu_idx 0x%x\n",
> +				 prange->svms, prange, gpuidx);
> +			clear_bit(gpuidx, prange->bitmap_mapped);
> +		}
> +
>   		pdd = kfd_process_device_from_gpuidx(p, gpuidx);
>   		if (!pdd) {
>   			pr_debug("failed to find device idx %d\n", gpuidx);
> @@ -1596,6 +1597,8 @@ svm_range_map_to_gpus(struct svm_range *prange, unsigned long offset,
>   			continue;
>   		}
>   
> +		set_bit(gpuidx, prange->bitmap_mapped);
> +
>   		r = svm_range_map_to_gpu(pdd, prange, offset, npages, readonly,
>   					 prange->dma_addr[gpuidx],
>   					 bo_adev, wait ? &fence : NULL,
> @@ -1741,7 +1744,9 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   		bitmap_zero(ctx->bitmap, MAX_GPU_INSTANCE);
>   		bitmap_set(ctx->bitmap, gpuidx, 1);
>   	} else if (ctx->process->xnack_enabled) {
> -		bitmap_copy(ctx->bitmap, prange->bitmap_aip, MAX_GPU_INSTANCE);
> +		/* Update mapping on already mapped or access in place GPU */
> +		bitmap_or(ctx->bitmap, prange->bitmap_mapped, prange->bitmap_aip,
> +			  MAX_GPU_INSTANCE);
>   
>   		/* If prefetch range to GPU, or GPU retry fault migrate range to
>   		 * GPU, which has ACCESS attribute to the range, create mapping
> @@ -1761,14 +1766,12 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   		}
>   
>   		/*
> -		 * If prange is already mapped or with always mapped flag,
> -		 * update mapping on GPUs with ACCESS attribute
> +		 * If prange with always mapped flag, update mapping on GPUs with
> +		 * ACCESS attribute
>   		 */
> -		if (bitmap_empty(ctx->bitmap, MAX_GPU_INSTANCE)) {
> -			if (prange->mapped_to_gpu ||
> -			    prange->flags & KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED)
> -				bitmap_copy(ctx->bitmap, prange->bitmap_access, MAX_GPU_INSTANCE);
> -		}
> +		if (prange->flags & KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED)
> +			bitmap_or(ctx->bitmap, ctx->bitmap, prange->bitmap_access,
> +				  MAX_GPU_INSTANCE);
>   	} else {
>   		bitmap_or(ctx->bitmap, prange->bitmap_access,
>   			  prange->bitmap_aip, MAX_GPU_INSTANCE);
> @@ -1834,6 +1837,7 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   				e = min(end, prange->last);
>   				if (e >= s)
>   					r = svm_range_unmap_from_gpus(prange, s, e,
> +						       prange->bitmap_mapped,
>   						       KFD_SVM_UNMAP_TRIGGER_UNMAP_FROM_CPU);
>   				svm_range_unlock(prange);
>   				/* If unmap returns non-zero, we'll bail on the next for loop
> @@ -1896,7 +1900,9 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   		}
>   
>   		if (!r && next == end)
> -			prange->mapped_to_gpu = true;
> +			prange->mapping_done = true;
> +		else
> +			prange->mapping_done = false;
>   
>   		svm_range_unlock(prange);
>   
> @@ -2066,10 +2072,10 @@ svm_range_evict(struct svm_range *prange, struct mm_struct *mm,
>   	if (!p->xnack_enabled ||
>   	    (prange->flags & KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED)) {
>   		int evicted_ranges;
> -		bool mapped = prange->mapped_to_gpu;
> +		bool mapped = !bitmap_empty(prange->bitmap_mapped, MAX_GPU_INSTANCE);
>   
>   		list_for_each_entry(pchild, &prange->child_list, child_list) {
> -			if (!pchild->mapped_to_gpu)
> +			if (bitmap_empty(pchild->bitmap_mapped, MAX_GPU_INSTANCE))
>   				continue;
>   			mapped = true;
>   			mutex_lock_nested(&pchild->lock, 1);
> @@ -2118,13 +2124,14 @@ svm_range_evict(struct svm_range *prange, struct mm_struct *mm,
>   			s = max(start, pchild->start);
>   			l = min(last, pchild->last);
>   			if (l >= s)
> -				svm_range_unmap_from_gpus(pchild, s, l, trigger);
> +				svm_range_unmap_from_gpus(pchild, s, l, prange->bitmap_mapped,
> +							  trigger);
>   			mutex_unlock(&pchild->lock);
>   		}
>   		s = max(start, prange->start);
>   		l = min(last, prange->last);
>   		if (l >= s)
> -			svm_range_unmap_from_gpus(prange, s, l, trigger);
> +			svm_range_unmap_from_gpus(prange, s, l, prange->bitmap_mapped, trigger);
>   	}
>   
>   	return r;
> @@ -2154,10 +2161,11 @@ static struct svm_range *svm_range_clone(struct svm_range *old)
>   	new->prefetch_loc = old->prefetch_loc;
>   	new->actual_loc = old->actual_loc;
>   	new->granularity = old->granularity;
> -	new->mapped_to_gpu = old->mapped_to_gpu;
> +	new->mapping_done = old->mapping_done;
>   	new->vram_pages = old->vram_pages;
>   	bitmap_copy(new->bitmap_access, old->bitmap_access, MAX_GPU_INSTANCE);
>   	bitmap_copy(new->bitmap_aip, old->bitmap_aip, MAX_GPU_INSTANCE);
> +	bitmap_copy(new->bitmap_mapped, old->bitmap_mapped, MAX_GPU_INSTANCE);
>   	atomic_set(&new->queue_refcount, atomic_read(&old->queue_refcount));
>   
>   	return new;
> @@ -2277,7 +2285,7 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
>   		next_start = min(node->last, last) + 1;
>   
>   		if (svm_range_is_same_attrs(p, prange, nattr, attrs) &&
> -		    prange->mapped_to_gpu) {
> +		    prange->mapping_done) {
>   			/* nothing to do */
>   		} else if (node->start < start || node->last > last) {
>   			/* node intersects the update range and its attributes
> @@ -2626,14 +2634,14 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
>   		s = max(start, pchild->start);
>   		l = min(last, pchild->last);
>   		if (l >= s)
> -			svm_range_unmap_from_gpus(pchild, s, l, trigger);
> +			svm_range_unmap_from_gpus(pchild, s, l, prange->bitmap_mapped, trigger);
>   		svm_range_unmap_split(prange, pchild, start, last);
>   		mutex_unlock(&pchild->lock);
>   	}
>   	s = max(start, prange->start);
>   	l = min(last, prange->last);
>   	if (l >= s)
> -		svm_range_unmap_from_gpus(prange, s, l, trigger);
> +		svm_range_unmap_from_gpus(prange, s, l, prange->bitmap_mapped, trigger);
>   	svm_range_unmap_split(prange, prange, start, last);
>   
>   	if (unmap_parent)
> @@ -3723,6 +3731,23 @@ static void svm_range_evict_svm_bo_worker(struct work_struct *work)
>   	svm_range_bo_unref(svm_bo);
>   }
>   
> +static bool svm_range_needs_unmap(struct kfd_process *p, struct svm_range *prange)
> +{
> +	if (bitmap_empty(prange->bitmap_needs_unmap, MAX_GPU_INSTANCE))
> +		return false;
> +
> +	pr_debug("prange 0x%p no access set for [0x%lx 0x%lx]\n",
> +		 prange, prange->start, prange->last);
> +
> +	svm_range_unmap_from_gpus(prange, prange->start,
> +				  prange->last, prange->bitmap_needs_unmap,
> +				  KFD_SVM_UNMAP_TRIGGER_UNMAP_FROM_CPU);
> +
> +	bitmap_clear(prange->bitmap_needs_unmap, 0, MAX_GPU_INSTANCE);
> +
> +	return bitmap_empty(prange->bitmap_mapped, MAX_GPU_INSTANCE);
> +}
> +
>   static int
>   svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
>   		   uint64_t start, uint64_t size, uint32_t nattr,
> @@ -3778,10 +3803,10 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
>   		svm_range_add_to_svms(prange);
>   		svm_range_add_notifier_locked(mm, prange);
>   	}
> -	list_for_each_entry(prange, &update_list, update_list) {
> +
> +	list_for_each_entry(prange, &update_list, update_list)
>   		svm_range_apply_attrs(p, prange, nattr, attrs, &update_mapping);
> -		/* TODO: unmap ranges from GPU that lost access */
> -	}
> +
>   	update_mapping |= !p->xnack_enabled && !list_empty(&remap_list);
>   
>   	list_for_each_entry_safe(prange, next, &remove_list, update_list) {
> @@ -3802,6 +3827,9 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
>   	list_for_each_entry(prange, &update_list, update_list) {
>   		bool migrated;
>   
> +		if (svm_range_needs_unmap(p, prange))
> +			continue;
> +
>   		mutex_lock(&prange->migrate_mutex);
>   
>   		r = svm_range_trigger_migration(mm, prange, &migrated);
> @@ -3810,7 +3838,7 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
>   
>   		if (migrated && (!p->xnack_enabled ||
>   		    (prange->flags & KFD_IOCTL_SVM_FLAG_GPU_ALWAYS_MAPPED)) &&
> -		    prange->mapped_to_gpu) {
> +		    !bitmap_empty(prange->bitmap_mapped, MAX_GPU_INSTANCE)) {
>   			pr_debug("restore_work will update mappings of GPUs\n");
>   			mutex_unlock(&prange->migrate_mutex);
>   			continue;
> @@ -3821,7 +3849,8 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
>   			continue;
>   		}
>   
> -		flush_tlb = !migrated && update_mapping && prange->mapped_to_gpu;
> +		flush_tlb = !migrated && update_mapping &&
> +			    !bitmap_empty(prange->bitmap_mapped, MAX_GPU_INSTANCE);
>   
>   		r = svm_range_validate_and_map(mm, prange->start, prange->last, prange,
>   					       MAX_GPU_INSTANCE, true, true, flush_tlb);
> @@ -3835,11 +3864,13 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
>   	}
>   
>   	list_for_each_entry(prange, &remap_list, update_list) {
> +		flush_tlb = !bitmap_empty(prange->bitmap_mapped, MAX_GPU_INSTANCE);
> +
>   		pr_debug("Remapping prange 0x%p [0x%lx 0x%lx]\n",
>   			 prange, prange->start, prange->last);
>   		mutex_lock(&prange->migrate_mutex);
>   		r = svm_range_validate_and_map(mm,  prange->start, prange->last, prange,
> -					       MAX_GPU_INSTANCE, true, true, prange->mapped_to_gpu);
> +					       MAX_GPU_INSTANCE, true, true, flush_tlb);
>   		if (r)
>   			pr_debug("failed %d on remap svm range\n", r);
>   		mutex_unlock(&prange->migrate_mutex);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> index a63dfc95b602..0da635532aff 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.h
> @@ -100,6 +100,9 @@ struct svm_work_list_item {
>    * @child_list: list header for split ranges which are not added to svms yet
>    * @bitmap_access: index bitmap of GPUs which can access the range
>    * @bitmap_aip: index bitmap of GPUs which can access the range in place
> + * @bitmap_needs_unmap: index bitmap of GPUs which currently set NO_ACCESS
> + * @bitmap_mapped: index bitmap of GPUs which currently have the range mapped
> + * @mapping_done: true if range_validate_and_map complete successfully
>    *
>    * Data structure for virtual memory range shared by CPU and GPUs, it can be
>    * allocated from system memory ram or device vram, and migrate from ram to vram
> @@ -135,7 +138,9 @@ struct svm_range {
>   	struct list_head		child_list;
>   	DECLARE_BITMAP(bitmap_access, MAX_GPU_INSTANCE);
>   	DECLARE_BITMAP(bitmap_aip, MAX_GPU_INSTANCE);
> -	bool				mapped_to_gpu;
> +	DECLARE_BITMAP(bitmap_needs_unmap, MAX_GPU_INSTANCE);
> +	DECLARE_BITMAP(bitmap_mapped, MAX_GPU_INSTANCE);
> +	bool				mapping_done;
>   	atomic_t			queue_refcount;
>   };
>