[PATCH] amdgpu/ttm: make sure exported objects are populated (v2)

[PATCH] amdgpu/ttm: make sure exported objects are populated (v2)

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

While discussing cgroups we noticed a problem where you could export
a BO to a dma-buf without having it ever being backed or accounted for.

This meant in low memory situations or eventually with cgroups, a
lower privledged process might cause the compositor to try and allocate
a lot of memory on it's behalf and this could fail. At least make
sure the exporter has managed to allocate the RAM at least once
before exporting the object.

This only applies currently to TTM_PL_SYSTEM objects, because
GTT objects get populated on first validate, and VRAM doesn't
use TT.

TODO:
other drivers?
split this into two?

Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Simona Vetter <simona.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>

---
v2: let the user choose interruptible (do we need the whole op ctx?)
stop checking things outside the reservation, just populate always.
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c |  5 +++++
 drivers/gpu/drm/ttm/ttm_bo.c                | 23 +++++++++++++++++++++
 include/drm/ttm/ttm_bo.h                    |  2 ++
 3 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index ce27cb5bb05e..c83289ea3e8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -343,11 +343,16 @@ struct dma_buf *amdgpu_gem_prime_export(struct drm_gem_object *gobj,
 {
 	struct amdgpu_bo *bo = gem_to_amdgpu_bo(gobj);
 	struct dma_buf *buf;
+	int ret;
 
 	if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
 	    bo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID)
 		return ERR_PTR(-EPERM);
 
+	ret = ttm_bo_setup_export(&bo->tbo, true);
+	if (ret)
+		return ERR_PTR(ret);
+
 	buf = drm_gem_prime_export(gobj, flags);
 	if (!IS_ERR(buf))
 		buf->ops = &amdgpu_dmabuf_ops;
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 273757974b9f..3afb577cf327 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1284,3 +1284,26 @@ int ttm_bo_populate(struct ttm_buffer_object *bo,
 	return 0;
 }
 EXPORT_SYMBOL(ttm_bo_populate);
+
+int ttm_bo_setup_export(struct ttm_buffer_object *bo, bool interruptible)
+{
+	struct ttm_operation_ctx ctx = {
+		.interruptible = interruptible,
+		.no_wait_gpu = true,
+		/* We opt to avoid OOM on system pages allocations */
+		.gfp_retry_mayfail = true,
+		.allow_res_evict = false,
+	};
+
+	int ret;
+
+	ret = ttm_bo_reserve(bo, false, false, NULL);
+	if (ret != 0)
+		return ret;
+
+	ret = ttm_bo_populate(bo, bo->resource->placement & TTM_PL_FLAG_MEMCG, &ctx);
+ out:
+	ttm_bo_unreserve(bo);
+	return ret;
+}
+EXPORT_SYMBOL(ttm_bo_setup_export);
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index c33b3667ae76..91913cad879f 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -473,6 +473,8 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo);
 int ttm_bo_populate(struct ttm_buffer_object *bo,
 		    bool memcg_account,
 		    struct ttm_operation_ctx *ctx);
+int ttm_bo_setup_export(struct ttm_buffer_object *bo,
+			bool interruptible);
 
 /* Driver LRU walk helpers initially targeted for shrinking. */
 
-- 
2.50.1

Re: [PATCH] amdgpu/ttm: make sure exported objects are populated (v2)

[Christian König]

On 02.09.25 06:14, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> While discussing cgroups we noticed a problem where you could export
> a BO to a dma-buf without having it ever being backed or accounted for.
> 
> This meant in low memory situations or eventually with cgroups, a
> lower privledged process might cause the compositor to try and allocate
> a lot of memory on it's behalf and this could fail. At least make
> sure the exporter has managed to allocate the RAM at least once
> before exporting the object.
> 
> This only applies currently to TTM_PL_SYSTEM objects, because
> GTT objects get populated on first validate, and VRAM doesn't
> use TT.
> 
> TODO:
> other drivers?

XE and maybe Nouveau are a must have I think.

VMWGFX and hisilicon maybe, I don't know.

Radeon and QXL probably not.

> split this into two?

Potentially better, yes.

> 
> Cc: Christian Koenig <christian.koenig@amd.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Simona Vetter <simona.vetter@ffwll.ch>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Apart from splitting it up Reviewed-by: Christian König <christian.koenig@amd.com>.

Regards,
Christian.

> 
> ---
> v2: let the user choose interruptible (do we need the whole op ctx?)
> stop checking things outside the reservation, just populate always.
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c |  5 +++++
>  drivers/gpu/drm/ttm/ttm_bo.c                | 23 +++++++++++++++++++++
>  include/drm/ttm/ttm_bo.h                    |  2 ++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> index ce27cb5bb05e..c83289ea3e8c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> @@ -343,11 +343,16 @@ struct dma_buf *amdgpu_gem_prime_export(struct drm_gem_object *gobj,
>  {
>  	struct amdgpu_bo *bo = gem_to_amdgpu_bo(gobj);
>  	struct dma_buf *buf;
> +	int ret;
>  
>  	if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
>  	    bo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID)
>  		return ERR_PTR(-EPERM);
>  
> +	ret = ttm_bo_setup_export(&bo->tbo, true);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	buf = drm_gem_prime_export(gobj, flags);
>  	if (!IS_ERR(buf))
>  		buf->ops = &amdgpu_dmabuf_ops;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 273757974b9f..3afb577cf327 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1284,3 +1284,26 @@ int ttm_bo_populate(struct ttm_buffer_object *bo,
>  	return 0;
>  }
>  EXPORT_SYMBOL(ttm_bo_populate);
> +
> +int ttm_bo_setup_export(struct ttm_buffer_object *bo, bool interruptible)
> +{
> +	struct ttm_operation_ctx ctx = {
> +		.interruptible = interruptible,
> +		.no_wait_gpu = true,
> +		/* We opt to avoid OOM on system pages allocations */
> +		.gfp_retry_mayfail = true,
> +		.allow_res_evict = false,
> +	};
> +
> +	int ret;
> +
> +	ret = ttm_bo_reserve(bo, false, false, NULL);
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = ttm_bo_populate(bo, bo->resource->placement & TTM_PL_FLAG_MEMCG, &ctx);
> + out:
> +	ttm_bo_unreserve(bo);
> +	return ret;
> +}
> +EXPORT_SYMBOL(ttm_bo_setup_export);
> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
> index c33b3667ae76..91913cad879f 100644
> --- a/include/drm/ttm/ttm_bo.h
> +++ b/include/drm/ttm/ttm_bo.h
> @@ -473,6 +473,8 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo);
>  int ttm_bo_populate(struct ttm_buffer_object *bo,
>  		    bool memcg_account,
>  		    struct ttm_operation_ctx *ctx);
> +int ttm_bo_setup_export(struct ttm_buffer_object *bo,
> +			bool interruptible);
>  
>  /* Driver LRU walk helpers initially targeted for shrinking. */
>  

Re: [PATCH] amdgpu/ttm: make sure exported objects are populated (v2)

[Thomas Hellström]

Hi, Dave,

On Tue, 2025-09-02 at 14:14 +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> While discussing cgroups we noticed a problem where you could export
> a BO to a dma-buf without having it ever being backed or accounted
> for.
> 
> This meant in low memory situations or eventually with cgroups, a
> lower privledged process might cause the compositor to try and
> allocate
> a lot of memory on it's behalf and this could fail. At least make
> sure the exporter has managed to allocate the RAM at least once
> before exporting the object.
> 
> This only applies currently to TTM_PL_SYSTEM objects, because
> GTT objects get populated on first validate, and VRAM doesn't
> use TT.
> 
> TODO:
> other drivers?
> split this into two?
> 
> Cc: Christian Koenig <christian.koenig@amd.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Simona Vetter <simona.vetter@ffwll.ch>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> 
> ---
> v2: let the user choose interruptible (do we need the whole op ctx?)
> stop checking things outside the reservation, just populate always.
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c |  5 +++++
>  drivers/gpu/drm/ttm/ttm_bo.c                | 23
> +++++++++++++++++++++
>  include/drm/ttm/ttm_bo.h                    |  2 ++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> index ce27cb5bb05e..c83289ea3e8c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> @@ -343,11 +343,16 @@ struct dma_buf *amdgpu_gem_prime_export(struct
> drm_gem_object *gobj,
>  {
>  	struct amdgpu_bo *bo = gem_to_amdgpu_bo(gobj);
>  	struct dma_buf *buf;
> +	int ret;
>  
>  	if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
>  	    bo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID)
>  		return ERR_PTR(-EPERM);
>  
> +	ret = ttm_bo_setup_export(&bo->tbo, true);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	buf = drm_gem_prime_export(gobj, flags);
>  	if (!IS_ERR(buf))
>  		buf->ops = &amdgpu_dmabuf_ops;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> b/drivers/gpu/drm/ttm/ttm_bo.c
> index 273757974b9f..3afb577cf327 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1284,3 +1284,26 @@ int ttm_bo_populate(struct ttm_buffer_object
> *bo,
>  	return 0;
>  }
>  EXPORT_SYMBOL(ttm_bo_populate);
> +
> +int ttm_bo_setup_export(struct ttm_buffer_object *bo, bool
> interruptible)
> +{
> +	struct ttm_operation_ctx ctx = {
> +		.interruptible = interruptible,
> +		.no_wait_gpu = true,
> +		/* We opt to avoid OOM on system pages allocations
> */
> +		.gfp_retry_mayfail = true,
> +		.allow_res_evict = false,
> +	};

I still think the ttm_operation_ctx needs to originate from the driver.
It's really the driver's way of telling TTM how backing store
allocation should be done. 

Otherwise patch LGTM.

Thanks,
Thomas



> +
> +	int ret;
> +
> +	ret = ttm_bo_reserve(bo, false, false, NULL);
> +	if (ret != 0)
> +		return ret;
> +
> +	ret = ttm_bo_populate(bo, bo->resource->placement &
> TTM_PL_FLAG_MEMCG, &ctx);
> + out:
> +	ttm_bo_unreserve(bo);
> +	return ret;
> +}
> +EXPORT_SYMBOL(ttm_bo_setup_export);
> diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
> index c33b3667ae76..91913cad879f 100644
> --- a/include/drm/ttm/ttm_bo.h
> +++ b/include/drm/ttm/ttm_bo.h
> @@ -473,6 +473,8 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object
> *bo);
>  int ttm_bo_populate(struct ttm_buffer_object *bo,
>  		    bool memcg_account,
>  		    struct ttm_operation_ctx *ctx);
> +int ttm_bo_setup_export(struct ttm_buffer_object *bo,
> +			bool interruptible);
>  
>  /* Driver LRU walk helpers initially targeted for shrinking. */
>