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

2020-03-21 Thread Yin, Tianci (Rico)
[AMD Official Use Only - Internal Distribution Only]

I see, thanks your explanation.

Regards,
Rico

From: Koenig, Christian 
Sent: Saturday, March 21, 2020 16:44
To: Yin, Tianci (Rico) 
Cc: amd-gfx@lists.freedesktop.org ; Xu, Feifei 
; Li, Pauline ; Long, Gang 
; Zhang, Hawking 
Subject: Re: [PATCH] drm/amdgpu: fix size validation failure in large buffer 
creation

Correct, yes.

For example if you have a 16GB VRAM Vega10 in a system with just 4GB RAM you 
can only allocate < 4GB VRAM (actually more like ~3GB) in a single BO.

Otherwise we wouldn't be able to evacuate VRAM to system memory and disk during 
suspend/resume or during memory pressure.

Regards,
Christian.

Am 21.03.2020 09:32 schrieb "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 
Sent: Saturday, March 21, 2020 2:27
To: Yin, Tianci (Rico) ; amd-gfx@lists.freedesktop.org 

Cc: Xu, Feifei ; Li, Pauline ; Long, 
Gang ; Zhang, Hawking 
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" 
>
> [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 
> ---
>   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;

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-03-21 Thread Koenig, Christian
Correct, yes.

For example if you have a 16GB VRAM Vega10 in a system with just 4GB RAM you 
can only allocate < 4GB VRAM (actually more like ~3GB) in a single BO.

Otherwise we wouldn't be able to evacuate VRAM to system memory and disk during 
suspend/resume or during memory pressure.

Regards,
Christian.

Am 21.03.2020 09:32 schrieb "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 
Sent: Saturday, March 21, 2020 2:27
To: Yin, Tianci (Rico) ; amd-gfx@lists.freedesktop.org 

Cc: Xu, Feifei ; Li, Pauline ; Long, 
Gang ; Zhang, Hawking 
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" 
>
> [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 
> ---
>   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;

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-03-21 Thread 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 
Sent: Saturday, March 21, 2020 2:27
To: Yin, Tianci (Rico) ; amd-gfx@lists.freedesktop.org 

Cc: Xu, Feifei ; Li, Pauline ; Long, 
Gang ; Zhang, Hawking 
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" 
>
> [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 
> ---
>   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;

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-03-20 Thread Christian König

Am 20.03.20 um 10:46 schrieb Tianci Yin:

From: "Tianci.Yin" 

[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 
---
  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;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

2020-03-20 Thread Tianci Yin
From: "Tianci.Yin" 

[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.

Change-Id: Ic1d31b9b0a4939e6bba0241ff79ae9aa2225ee05
Signed-off-by: Tianci.Yin 
---
 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;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx