Re: [PATCH v10 05/22] drm/amdgpu: add SVM range types and work queue interface

["Huang, Honglei" <honghuan@amd.com>]

On 11/9/2026 9:48 PM, Christian König wrote:
> On 10/21/26 04:58, Huang Rui wrote:
>> From: Honglei Huang <honghuan@amd.com>
>>
>> Add amdgpu_svm_range.h with GPU mapped range types and interfaces:
>> - enum amdgpu_svm_range_queue_state: NOT_QUEUED, IN_GC, PROCESSING
>>    states for queue work
>> - struct amdgpu_svm_range: extends drm_gpusvm_range with gpu_mapped
>>    state, queue_state, attribute flags, work queue node, pending
>>    ops/pages, and validation timestamp
>> - enum amdgpu_svm_range_op: NONE, UNMAP operation types
>> - struct amdgpu_svm_range_op_ctx: dequeue context for GC processing
>> - Inline helper: amdgpu_svm_range_invalidate_gpu_mapping
>> - Range operations API declarations: find_or_insert, get_pages,
>>    update_mapping, update_gpu_range, invalidate
>>
>> Suggested-by: Christian König <christian.koenig@amd.com>
>> Suggested-by: Huang Rui <ray.huang@amd.com>
>> Signed-off-by: Honglei Huang <honghuan@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.h | 129 ++++++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h     |  30 ++++
>>   2 files changed, 159 insertions(+)
>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.h
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.h
>> new file mode 100644
>> index 0000000000000..39db3a18b5f2f
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.h
>> @@ -0,0 +1,129 @@
>> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
>> +/*
>> + * Copyright 2026 Advanced Micro Devices, Inc.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + *
>> + */
>> +
>> +#ifndef __AMDGPU_SVM_RANGE_H__
>> +#define __AMDGPU_SVM_RANGE_H__
>> +
>> +#include <drm/drm_gpusvm.h>
>> +#include <drm/drm_pagemap.h>
>> +
>> +#include "amdgpu_svm.h"
>> +#include "amdgpu_vm.h"
>> +
>> +#include <linux/ktime.h>
>> +#include <linux/list.h>
>> +#include <linux/types.h>
>> +
>> +struct amdgpu_svm;
>> +struct amdgpu_svm_attr_range;
>> +struct amdgpu_svm_attrs;
>> +struct dma_fence;
>> +struct drm_exec;
>> +struct drm_gpusvm_notifier;
>> +struct drm_gpusvm_range;
>> +struct mmu_notifier_range;
>> +
>> +/**
>> + * enum amdgpu_svm_range_queue_state - Work queue state of an SVM range
>> + * @AMDGPU_SVM_RANGE_NOT_QUEUED: Not on any work list.
>> + * @AMDGPU_SVM_RANGE_IN_GC: Queued on the GC list.
>> + * @AMDGPU_SVM_RANGE_PROCESSING: Being processed in workqueue.
>> + */
>> +enum amdgpu_svm_range_queue_state {
>> +	AMDGPU_SVM_RANGE_NOT_QUEUED = 0,
>> +	AMDGPU_SVM_RANGE_IN_GC,
>> +	AMDGPU_SVM_RANGE_PROCESSING,
>> +};
>> +
>> +/**
>> + * struct amdgpu_svm_range - amdgpu GPU mapped SVM range
>> + * @base: Embedded drm_gpusvm_range base.
>> + * @work_node: Links the range into the work list. Protected by amdgpu_svm.work_lock.
>> + * @gpu_mapped: True while the range has a live GPU mapping. Accessed with
>> + *              READ_ONCE/WRITE_ONCE.
>> + * @queue_state: Current work queue state @amdgpu_svm_range_queue_state.
>> + *               Protected by amdgpu_svm.work_lock.
>> + * @pending_ops: Pending amdgpu_svm_range_op bitmask. Protected by work_lock.
>> + * @pending_start_page: First page of the accumulated pending interval.
>> + * @pending_last_page: Last page of the accumulated pending interval.
>> + * @attr_flags: Attribute flags last applied to the GPU mapping.
>> + * @validate_timestamp: Time of the last successful mapping validation.
>> + */
>> +struct amdgpu_svm_range {
>> +	struct drm_gpusvm_range base;
>> +	struct list_head work_node;
>> +	bool gpu_mapped;
> 
>> +	u8 queue_state;
> 
> What is that good for and why is it needed?

Previous SVM implemetation need pop the range task under mmap write lock 
to prevent the UAF. This is not allowed under drmsvm helper.
The current series use queue state to trace the range work op state and 
seralize the different op by using work_lock instead of mmap lock.

Regards,
Honglei

> 
> Regards,
> Christian.
> 
>> +	u8 pending_ops;
>> +	unsigned long pending_start_page;
>> +	unsigned long pending_last_page;
>> +	uint32_t attr_flags;
>> +	ktime_t validate_timestamp;
>> +};
>> +
>> +static inline struct amdgpu_svm_range *
>> +to_amdgpu_svm_range(struct drm_gpusvm_range *range)
>> +{
>> +	return container_of(range, struct amdgpu_svm_range, base);
>> +}
>> +
>> +static inline void
>> +amdgpu_svm_range_invalidate_gpu_mapping(struct amdgpu_svm_range *range)
>> +{
>> +	WRITE_ONCE(range->gpu_mapped, false);
>> +}
>> +
>> +#define AMDGPU_SVM_RANGE_TRACE(r__, op__)				    \
>> +	trace_amdgpu_svm_range((op__),					    \
>> +		to_amdgpu_svm((r__)->base.gpusvm)->vm->pasid,		    \
>> +		(r__)->base.gpusvm, READ_ONCE((r__)->gpu_mapped),	    \
>> +		(r__)->base.pages.notifier_seq,				    \
>> +		drm_gpusvm_range_start(&(r__)->base) >> PAGE_SHIFT,	    \
>> +		drm_gpusvm_range_end(&(r__)->base) >> PAGE_SHIFT)
>> +
>> +/**
>> + * enum amdgpu_svm_range_op - Pending operation bits for an SVM range
>> + * @AMDGPU_SVM_RANGE_OP_NONE: No pending operation.
>> + * @AMDGPU_SVM_RANGE_OP_UNMAP: Range needs to be unmapped/removed by the GC.
>> + */
>> +enum amdgpu_svm_range_op {
>> +	AMDGPU_SVM_RANGE_OP_NONE    = 0,
>> +	AMDGPU_SVM_RANGE_OP_UNMAP   = BIT(0),
>> +};
>> +
>> +/**
>> + * struct amdgpu_svm_range_op_ctx - context of a range workqueue operation
>> + * @range: The dequeued range to process.
>> + * @start_page: First page for workqueue operation.
>> + * @last_page: Last page for workqueue operation.
>> + * @pending_ops: amdgpu_svm_range_op bitmask.
>> + */
>> +struct amdgpu_svm_range_op_ctx {
>> +	struct amdgpu_svm_range *range;
>> +	unsigned long start_page;
>> +	unsigned long last_page;
>> +	uint8_t pending_ops;
>> +};
>> +
>> +#endif /* __AMDGPU_SVM_RANGE_H__ */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>> index 599c1d0e512b5..86ba09cba34c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>> @@ -116,6 +116,36 @@ TRACE_EVENT(amdgpu_iv,
>>   		      __entry->src_data[2], __entry->src_data[3])
>>   );
>>   
>> +TRACE_EVENT(amdgpu_svm_range,
>> +	    TP_PROTO(const char *op, u32 pasid, void *gpusvm, bool mapped,
>> +		     unsigned long seqno, unsigned long start_page,
>> +		     unsigned long end_page),
>> +	    TP_ARGS(op, pasid, gpusvm, mapped, seqno, start_page, end_page),
>> +	    TP_STRUCT__entry(
>> +			     __string(op, op)
>> +			     __field(u32, pasid)
>> +			     __field(void *, gpusvm)
>> +			     __field(bool, mapped)
>> +			     __field(unsigned long, seqno)
>> +			     __field(unsigned long, start_page)
>> +			     __field(unsigned long, end_page)
>> +			    ),
>> +	    TP_fast_assign(
>> +			   __assign_str(op);
>> +			   __entry->pasid = pasid;
>> +			   __entry->gpusvm = gpusvm;
>> +			   __entry->mapped = mapped;
>> +			   __entry->seqno = seqno;
>> +			   __entry->start_page = start_page;
>> +			   __entry->end_page = end_page;
>> +			   ),
>> +	    TP_printk("%s: pasid=%u gpusvm=%p mapped=%d seqno=%lu range=[0x%lx-0x%lx]-0x%lx",
>> +		      __get_str(op), __entry->pasid, __entry->gpusvm,
>> +		      __entry->mapped, __entry->seqno,
>> +		      __entry->start_page, __entry->end_page,
>> +		      __entry->end_page - __entry->start_page)
>> +);
>> +
>>   
>>   TRACE_EVENT(amdgpu_bo_create,
>>   	    TP_PROTO(struct amdgpu_bo *bo),
>