On 30/06/2026 14:18, Christian König wrote:
On 6/23/26 16:12, Tvrtko Ursulin wrote:Saving and restoring buffer object content poses a challenge for the checkpoint and restore process for at least two reasons. For example not all objects can be exported as dma-buf to enable copying from a separate client context, neither can any objects be easily copied from the same context since the injected CRIU code is unaware of the GPU virtual memory free and allocated ranges. Lets bypass both problems by simply exposing access to the alrady presentTypo "already".
Ack.
fast kernel copy via a new DRM_IOCTL_AMDGPU_GEM_COPY_BUFFER ioctl. By giving the kernel simply the source and destination handles it is able to copy them without the need for objects to be mapped, or shared with a separate client.For now this is implemented fully synchronous but can easily be extended for more parallelsim.I think that sentence can be dropped now.
Yep, forgot to update for v2 changes.
Signed-off-by: Tvrtko Ursulin <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 104 ++++++++++++++++++++++++ drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 14 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 ++ include/uapi/drm/amdgpu_drm.h | 16 ++++ 6 files changed, 137 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index f61f0ddd3ccf..993f38d70086 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -3077,6 +3077,7 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_GEM_LIST_HANDLES, amdgpu_gem_list_handles_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_GEM_LIST_CONTEXTS, amdgpu_gem_list_contexts_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), + DRM_IOCTL_DEF_DRV(AMDGPU_GEM_COPY_BUFFER, amdgpu_gem_copy_buffer_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), };static const struct drm_driver amdgpu_kms_driver = {diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index e75026aba874..ccc0ecfe6603 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -32,6 +32,7 @@ #include <linux/pci.h> #include <linux/dma-buf.h> #include <linux/dma-fence-unwrap.h> +#include <linux/sync_file.h> #include <linux/uaccess.h>#include <drm/amdgpu_drm.h>@@ -1234,6 +1235,109 @@ int amdgpu_gem_list_handles_ioctl(struct drm_device *dev, void *data, return ret; }+/**+ * amdgpu_gem_copy_buffer_ioctl - copy buffer object content + * + * @dev: drm device pointer + * @data: drm_amdgpu_gem_copy_buffer + * @filp: drm file pointer + * + * Returns: + * 0 for success, -errno for errors. + */ +int amdgpu_gem_copy_buffer_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp) +{ + struct amdgpu_copy_mem src_mem = {}, dst_mem = {}; + union drm_amdgpu_gem_copy_buffer *args = data; + struct amdgpu_device *adev = drm_to_adev(dev); + struct drm_gem_object *src_gobj, *dst_gobj; + struct amdgpu_bo *src_bo, *dst_bo; + struct dma_fence *fence = NULL; + struct sync_file *sync_file; + struct drm_exec exec; + unsigned int e; + int r; + + src_gobj = drm_gem_object_lookup(filp, args->in.src_handle); + if (!src_gobj) + return -ENOENT; + + dst_gobj = drm_gem_object_lookup(filp, args->in.dst_handle); + if (!dst_gobj) { + r = -ENOENT; + goto err_dst; + } + + src_bo = gem_to_amdgpu_bo(src_gobj); + dst_bo = gem_to_amdgpu_bo(dst_gobj); + + if (amdgpu_bo_size(src_bo) < amdgpu_bo_size(dst_bo)) { + r = -E2BIG; + goto err_sizes; + }No sub range copy? I would expect something like a ~8MiB transfer buffer to copy a 1GiB buffer in chunks of 2MiB.
To preemptively make it generic or you already have an use case in mind? But sure, I can add that no problem.
+ + drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0); + + drm_exec_until_all_locked(&exec) { + r = drm_exec_prepare_obj(&exec, &src_bo->tbo.base, 1); + drm_exec_retry_on_contention(&exec); + if (r) + goto err_unlock; + + r = drm_exec_prepare_obj(&exec, &dst_bo->tbo.base, 1); + drm_exec_retry_on_contention(&exec); + if (r) + goto err_unlock; + } + + src_mem.bo = &src_bo->tbo; + src_mem.mem = src_bo->tbo.resource; + dst_mem.bo = &dst_bo->tbo; + dst_mem.mem = dst_bo->tbo.resource; + e = atomic_inc_return(&adev->mman.next_move_entity) % + adev->mman.num_move_entities; + r = amdgpu_ttm_copy_mem_to_mem(adev, &adev->mman.move_entities[e], + &src_mem, &dst_mem, + amdgpu_bo_size(src_bo), + amdgpu_bo_encrypted(src_bo), + dst_bo->tbo.base.resv, &fence); + if (r) { + goto err_unlock; + } else if (!fence) { + r = -ENXIO; + goto err_unlock; + } + + dma_resv_add_fence(src_bo->tbo.base.resv, fence, DMA_RESV_USAGE_READ); + dma_resv_add_fence(dst_bo->tbo.base.resv, fence, DMA_RESV_USAGE_WRITE);That should be DMA_RESV_USAGE_BOOKKEEP. READ/WRITE are only for implicit sync here and we clearly don't want that here.
Ack.
+ + r = get_unused_fd_flags(O_CLOEXEC); + if (r < 0) + goto err_put_fence; + + sync_file = sync_file_create(fence);Oh, please no sync_file! Use a drm_syncobj, e.g. drm_syncobj_find() + dma_fence_chain_alloc() before the copy and then drm_syncobj_add_point() when it is done.
Why not, is sync_file so out of fashion? Syncobj feels heavy weight for what this is and it would require the CRIU side to create and destroy the syncobj.
Regards, Tvrtko
+ if (!sync_file) { + put_unused_fd(r); + r = -ENOMEM; + goto err_put_fence; + } + + fd_install(r, sync_file->file); + args->out.fence_fd = r; + r = 0; + +err_put_fence: + dma_fence_put(fence); +err_unlock: + drm_exec_fini(&exec); +err_sizes: + drm_gem_object_put(dst_gobj); +err_dst: + drm_gem_object_put(src_gobj); + return r; +} + static unsigned int amdgpu_gem_align_pitch(struct amdgpu_device *adev, unsigned int width, unsigned int cpp, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h index 0e17d9fc665f..64529680fdad 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h @@ -71,6 +71,8 @@ int amdgpu_gem_list_handles_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); int amdgpu_gem_list_contexts_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); +int amdgpu_gem_copy_buffer_ioctl(struct drm_device *dev, void *data, + struct drm_file *filp);int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data,struct drm_file *filp); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 7ce9e0e194c3..fb1e4d7a87ec 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -294,13 +294,13 @@ static int amdgpu_ttm_map_buffer(struct amdgpu_ttm_buffer_entity *entity, * */ __attribute__((nonnull)) -static int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev, - struct amdgpu_ttm_buffer_entity *entity, - const struct amdgpu_copy_mem *src, - const struct amdgpu_copy_mem *dst, - uint64_t size, bool tmz, - struct dma_resv *resv, - struct dma_fence **f) +int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev, + struct amdgpu_ttm_buffer_entity *entity, + const struct amdgpu_copy_mem *src, + const struct amdgpu_copy_mem *dst, + uint64_t size, bool tmz, + struct dma_resv *resv, + struct dma_fence **f) { struct amdgpu_res_cursor src_mm, dst_mm; struct dma_fence *fence = NULL; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h index e1b3495b902f..0c3aefaca085 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h @@ -284,4 +284,11 @@ void amdgpu_ttm_mmio_remap_free_sgt(struct device *dev, enum dma_data_direction dir, struct sg_table *sgt);+int amdgpu_ttm_copy_mem_to_mem(struct amdgpu_device *adev,+ struct amdgpu_ttm_buffer_entity *entity, + const struct amdgpu_copy_mem *src, + const struct amdgpu_copy_mem *dst, + uint64_t size, bool tmz, + struct dma_resv *resv, + struct dma_fence **f); #endif diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h index 7f5c4de7f9a5..b79c3d4c2ec2 100644 --- a/include/uapi/drm/amdgpu_drm.h +++ b/include/uapi/drm/amdgpu_drm.h @@ -59,6 +59,7 @@ extern "C" { #define DRM_AMDGPU_USERQ_WAIT 0x18 #define DRM_AMDGPU_GEM_LIST_HANDLES 0x19 #define DRM_AMDGPU_GEM_LIST_CONTEXTS 0x20 +#define DRM_AMDGPU_GEM_COPY_BUFFER 0x21#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) @@ -81,6 +82,7 @@ extern "C" { #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_LIST_CONTEXTS DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_LIST_CONTEXTS, struct drm_amdgpu_gem_list_contexts) +#define DRM_IOCTL_AMDGPU_GEM_COPY_BUFFER DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_COPY_BUFFER, union drm_amdgpu_gem_copy_buffer)/*** DOC: memory domains @@ -206,6 +208,20 @@ union drm_amdgpu_gem_create { struct drm_amdgpu_gem_create_out out; };+struct drm_amdgpu_gem_copy_buffer_in {+ __u32 src_handle; + __u32 dst_handle; +}; + +struct drm_amdgpu_gem_copy_buffer_out { + __u32 fence_fd; +}; + +union drm_amdgpu_gem_copy_buffer { + struct drm_amdgpu_gem_copy_buffer_in in; + struct drm_amdgpu_gem_copy_buffer_out out; +}; + /** Opcode to create new residency list. */ #define AMDGPU_BO_LIST_OP_CREATE 0 /** Opcode to destroy previously created residency list */
