Re: [RFC/POC PATCH 01/12] drm/amdgpu: add SVM UAPI definitions

Christian König
2026-04-20T12:15:19Z

On 4/20/26 14:07, Honglei Huang wrote:
> From: Honglei Huang <honghuan@amd.com>
>
> Add amdgpu drm SVM API definitions built on the
> DRM GPUSVM framework.
>
> This includes:
> - DRM_AMDGPU_GEM_SVM ioctl
> - AMDGPU_SVM_FLAG_* flags
> - AMDGPU_SVM_OP_SET_ATTR / AMDGPU_SVM_OP_GET_ATTR operations
> - AMDGPU_SVM_ATTR_* attribute types
> - AMDGPU_SVM_LOCATION_SYSMEM / AMDGPU_SVM_LOCATION_UNDEFINED
> - struct drm_amdgpu_svm_attribute and struct drm_amdgpu_gem_svm
>
> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
> include/uapi/drm/amdgpu_drm.h | 39 +++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 406a42be4..bed71ed9b 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -58,6 +58,7 @@ extern "C" {
> #define DRM_AMDGPU_USERQ_SIGNAL 0x17
> #define DRM_AMDGPU_USERQ_WAIT 0x18
> #define DRM_AMDGPU_GEM_LIST_HANDLES 0x19
> +#define DRM_AMDGPU_GEM_SVM 0x1a
>
> #define DRM_IOCTL_AMDGPU_GEM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
> #define DRM_IOCTL_AMDGPU_GEM_MMAP DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
> @@ -79,6 +80,7 @@ extern "C" {
> #define DRM_IOCTL_AMDGPU_USERQ_SIGNAL DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ_SIGNAL, struct drm_amdgpu_userq_signal)
> #define DRM_IOCTL_AMDGPU_USERQ_WAIT DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_USERQ_WAIT, struct drm_amdgpu_userq_wait)
> #define DRM_IOCTL_AMDGPU_GEM_LIST_HANDLES DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_LIST_HANDLES, struct drm_amdgpu_gem_list_handles)
> +#define DRM_IOCTL_AMDGPU_GEM_SVM DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_SVM, struct drm_amdgpu_gem_svm)
>
> /**
> * DOC: memory domains
> @@ -1665,6 +1667,43 @@ struct drm_color_ctm_3x4 {
> __u64 matrix[12];
> };
>
> +#define AMDGPU_SVM_FLAG_HOST_ACCESS 0x00000001
> +#define AMDGPU_SVM_FLAG_COHERENT 0x00000002
> +#define AMDGPU_SVM_FLAG_HIVE_LOCAL 0x00000004
> +#define AMDGPU_SVM_FLAG_GPU_RO 0x00000008
> +#define AMDGPU_SVM_FLAG_GPU_EXEC 0x00000010
> +#define AMDGPU_SVM_FLAG_GPU_READ_MOSTLY 0x00000020
> +#define AMDGPU_SVM_FLAG_GPU_ALWAYS_MAPPED 0x00000040
> +#define AMDGPU_SVM_FLAG_EXT_COHERENT 0x00000080
> +
> +#define AMDGPU_SVM_OP_SET_ATTR 0
> +#define AMDGPU_SVM_OP_GET_ATTR 1
> +
> +#define AMDGPU_SVM_ATTR_PREFERRED_LOC 0
> +#define AMDGPU_SVM_ATTR_PREFETCH_LOC 1

Up till here the interface makes perfect sense, but then it becomes a bit fuzzy.

> +#define AMDGPU_SVM_ATTR_ACCESS 2
> +#define AMDGPU_SVM_ATTR_ACCESS_IN_PLACE 3
> +#define AMDGPU_SVM_ATTR_NO_ACCESS 4

Why are those separate attributes? What is the difference between those?

> +#define AMDGPU_SVM_ATTR_SET_FLAGS 5
> +#define AMDGPU_SVM_ATTR_CLR_FLAGS 6

Why is that separated into set and clear flags?

> +#define AMDGPU_SVM_ATTR_GRANULARITY 7
> +
> +#define AMDGPU_SVM_LOCATION_SYSMEM 0
> +#define AMDGPU_SVM_LOCATION_UNDEFINED 0xffffffff

No location for device local memory?

> +
> +struct drm_amdgpu_svm_attribute {
> + __u32 type;
> + __u32 value;
> +};
> +
> +struct drm_amdgpu_gem_svm {
> + __u64 start_addr;
> + __u64 size;
> + __u32 operation;
> + __u32 nattr;
> + __u64 attrs_ptr;
> +};

Those struct make perfect sense but clearly need documentation. Preferable as kerneldoc.

And we usually use unions in this header to separate the input from the output parameters.

Regards,
Christian.

> +
> #if defined(__cplusplus)
> }
> #endif