Re: [PATCH] drm/amdgpu: fix BO placement overflow with four domain bits

That change doesn't make any sense.

Only CPU|GTT|VRAM can be combined together, that's why AMDGPU_BO_MAX_PLACEMENTS is 3.

DOORBELL, GDS, GWS, OA are special resources without alternatives and can only be specified one at a time.

On 7/24/26 05:12, Li, Candice wrote:
> AMD General
> 
> Ping.
> 
> Cc @Koenig, Christian and @Deucher, Alexander
> 
> 
> Thanks,
> Candice
> 
> -----Original Message-----
> From: Li, Candice <[email protected]>
> Sent: Tuesday, July 21, 2026 5:21 PM
> To: [email protected]
> Cc: Li, Candice <[email protected]>
> Subject: [PATCH] drm/amdgpu: fix BO placement overflow with four domain bits
> 
> AMDGPU_BO_MAX_PLACEMENTS was 3 but amdgpu_bo_placement_from_domain()
> can emit up to four entries for CPU|GTT|VRAM|DOORBELL. Unprivileged
> GEM_CREATE with domains=0x47 writes past placements[] and triggers
> BUG_ON().
> 
> Increase the array to four and normalize mixed GDS/GWS/OA domain
> masks to CPU-only placement on all rebuild paths.
> 
> Signed-off-by: Candice Li <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    |  3 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h    |  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 18 ++++++++++++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 ++-
>  4 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 6a0699746fbcd6..bcf81aba942842 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -431,8 +431,7 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>         flags |= AMDGPU_GEM_CREATE_VRAM_CLEARED;
> 
>         /* create a gem object to contain this object in */
> -       if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
> -           AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
> +       if (args->in.domains & AMDGPU_GEM_DOMAIN_GDS_GWS_OA) {
>                 if (flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID) {
>                         /* if gds bo is created from user space, it must be
>                          * passed to bo list
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> index b558336bc4c6ca..321db2ca29f4a3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> @@ -31,6 +31,8 @@
>   */
> 
>  #define AMDGPU_GEM_DOMAIN_MAX          0x3
> +#define AMDGPU_GEM_DOMAIN_GDS_GWS_OA \
> +       (AMDGPU_GEM_DOMAIN_GDS | AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)
>  #define gem_to_amdgpu_bo(gobj) container_of((gobj), struct amdgpu_bo, tbo.base)
> 
>  extern const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index d4a9d5e8fb429a..bb02858cbebf0d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -99,6 +99,19 @@ bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo)
>         return false;
>  }
> 
> +static u32 amdgpu_bo_filter_placement_domain(u32 domain)
> +{
> +       /*
> +        * GDS/GWS/OA are not normal fallback placement domains. When mixed
> +        * with CPU/GTT/VRAM/DOORBELL, fall back to CPU like amdgpu_bo_create().
> +        */
> +       if ((domain & AMDGPU_GEM_DOMAIN_GDS_GWS_OA) &&
> +           (domain & ~AMDGPU_GEM_DOMAIN_GDS_GWS_OA))
> +               return AMDGPU_GEM_DOMAIN_CPU;

Absolutely clear NAK to that!

That is simply a complete misunderstanding how things work here.

Regards,
Christian.

> +
> +       return domain;
> +}
> +
>  /**
>   * amdgpu_bo_placement_from_domain - set buffer's placement
>   * @abo: &amdgpu_bo buffer object whose placement is to be set
> @@ -115,6 +128,8 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
>         u64 flags = abo->flags;
>         u32 c = 0;
> 
> +       domain = amdgpu_bo_filter_placement_domain(domain);
> +
>         if (domain & AMDGPU_GEM_DOMAIN_VRAM) {
>                 unsigned int visible_pfn = adev->gmc.visible_vram_size >> PAGE_SHIFT;
>                 int8_t mem_id = KFD_XCP_MEM_ID(adev, abo->xcp_id);
> @@ -690,8 +705,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>                 bo->flags &= ~AMDGPU_GEM_CREATE_CPU_GTT_USWC;
> 
>         bo->tbo.bdev = &adev->mman.bdev;
> -       if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA |
> -                         AMDGPU_GEM_DOMAIN_GDS))
> +       if (bp->domain & AMDGPU_GEM_DOMAIN_GDS_GWS_OA)
>                 amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
>         else
>                 amdgpu_bo_placement_from_domain(bo, bp->domain);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index ff11a09034997e..20f4d8c243afdd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -37,7 +37,8 @@
>  #endif
> 
>  #define AMDGPU_BO_INVALID_OFFSET       LONG_MAX
> -#define AMDGPU_BO_MAX_PLACEMENTS       3
> +/* Covers CPU | GTT | VRAM | DOORBELL placement candidates */
> +#define AMDGPU_BO_MAX_PLACEMENTS       4
> 
>  /* BO flag to indicate a KFD userptr BO */
>  #define AMDGPU_AMDKFD_CREATE_USERPTR_BO        (1ULL << 63)
> --
> 2.25.1
>