[PATCH] drm/amdgpu: fix size validation failure in large buffer creation

[Yin, Tianci (Rico)]

[AMD Official Use Only - Internal Distribution Only]

Hi Christian,
You mean amdgpu_bo_validate_size() return false is the expectation when GTT < request < VRAM, even if VRAM size can meet the requirement, right?

Thanks!
Rico
________________________________
From: Christian K?nig <ckoenig.leichtzumerken at gmail.com>
Sent: Saturday, March 21, 2020 2:27
To: Yin, Tianci (Rico) <Tianci.Yin at amd.com>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
Cc: Xu, Feifei <Feifei.Xu at amd.com>; Li, Pauline <Pauline.Li at amd.com>; Long, Gang <Gang.Long at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>
Subject: Re: [PATCH] drm/amdgpu: fix size validation failure in large buffer creation

Am 20.03.20 um 10:46 schrieb Tianci Yin:
> From: "Tianci.Yin" <tianci.yin at amd.com>
>
> [why]
> When GTT domain size is smaller than VRAM, if APP apply a very large
> buffer whose size is larger than GTT but smaller than VRAM, the size
> validation will fail.
>
> [how]
> Validate VRAM domain size at first place, then GTT domain.

NAK, this is intended behavior. VRAM allocations larger than GTT
allocations are illegal and can crash the memory management.

Regards,
Christian.

>
> Change-Id: Ic1d31b9b0a4939e6bba0241ff79ae9aa2225ee05
> Signed-off-by: Tianci.Yin <tianci.yin at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 84745f9e7408..bab134b6369f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -464,21 +464,21 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
>   {
>        struct ttm_mem_type_manager *man = NULL;
>
> -     /*
> -      * If GTT is part of requested domains the check must succeed to
> -      * allow fall back to GTT
> -      */
> -     if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> -             man = &adev->mman.bdev.man[TTM_PL_TT];
> +     if (domain & AMDGPU_GEM_DOMAIN_VRAM) {
> +             man = &adev->mman.bdev.man[TTM_PL_VRAM];
>
>                if (size < (man->size << PAGE_SHIFT))
>                        return true;
> -             else
> +             else if (!(domain & AMDGPU_GEM_DOMAIN_GTT))
>                        goto fail;
>        }
>
> -     if (domain & AMDGPU_GEM_DOMAIN_VRAM) {
> -             man = &adev->mman.bdev.man[TTM_PL_VRAM];
> +     /*
> +      * If GTT is part of requested domains the check must succeed to
> +      * allow fall back to GTT
> +      */
> +     if (domain & AMDGPU_GEM_DOMAIN_GTT) {
> +             man = &adev->mman.bdev.man[TTM_PL_TT];
>
>                if (size < (man->size << PAGE_SHIFT))
>                        return true;

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20200321/e880b64b/attachment.htm>