[PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Kuehling, Felix]

This is an RFC. I'm not sure this is the right solution, but it
highlights the problem I'm trying to solve.

The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a
64-bit system with hundreds of GB of system memory and GPU memory,
this can become a bottle neck. We're seeing TTM memory allocation
failures not because we're truly out of memory, but because we're
out of space in the dma32_zone for the acc_size needed for our BO
book-keeping.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
CC: thellstrom@vmware.com
CC: christian.koenig@amd.com
---
 drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index f1567c3..bb05365 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global *glob,
 	glob->zones[glob->num_zones++] = zone;
 	return 0;
 }
-#else
+#elifndef CONFIG_64BIT
 static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob,
 				   const struct sysinfo *si)
 {
@@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
 	ret = ttm_mem_init_highmem_zone(glob, &si);
 	if (unlikely(ret != 0))
 		goto out_no_zone;
-#else
+#elifndef CONFIG_64BIT
 	ret = ttm_mem_init_dma32_zone(glob, &si);
 	if (unlikely(ret != 0))
 		goto out_no_zone;
-- 
2.7.4

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

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

Hmm,

This zone was intended to stop TTM page allocations from exhausting the 
DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default, which means 
if we drop this check, other devices may stop functioning unexpectedly?

However, in the end I'd expect the kernel page allocation system to make 
sure there are some pages left in the DMA32 zone, otherwise random 
non-IO page allocations would also potentially exhaust the DMA32 zone 
without anybody caring, which means removing this zone wouldn't be any 
worse than whatever other subsystems may be doing already...

/Thomas


On 2/16/19 12:02 AM, Kuehling, Felix wrote:
> This is an RFC. I'm not sure this is the right solution, but it
> highlights the problem I'm trying to solve.
>
> The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a
> 64-bit system with hundreds of GB of system memory and GPU memory,
> this can become a bottle neck. We're seeing TTM memory allocation
> failures not because we're truly out of memory, but because we're
> out of space in the dma32_zone for the acc_size needed for our BO
> book-keeping.
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> CC: thellstrom@vmware.com
> CC: christian.koenig@amd.com
> ---
>   drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
> index f1567c3..bb05365 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global *glob,
>   	glob->zones[glob->num_zones++] = zone;
>   	return 0;
>   }
> -#else
> +#elifndef CONFIG_64BIT
>   static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob,
>   				   const struct sysinfo *si)
>   {
> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
>   	ret = ttm_mem_init_highmem_zone(glob, &si);
>   	if (unlikely(ret != 0))
>   		goto out_no_zone;
> -#else
> +#elifndef CONFIG_64BIT
>   	ret = ttm_mem_init_dma32_zone(glob, &si);
>   	if (unlikely(ret != 0))
>   		goto out_no_zone;


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Koenig, Christian]

Another good question is also why the heck the acc_size counts towards 
the DMA32 zone?

In other words why should the internal bookkeeping pages be allocated in 
the DMA32 zone?

That doesn't sounds valid to me in any way,
Christian.

Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
> Hmm,
>
> This zone was intended to stop TTM page allocations from exhausting 
> the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default, which 
> means if we drop this check, other devices may stop functioning 
> unexpectedly?
>
> However, in the end I'd expect the kernel page allocation system to 
> make sure there are some pages left in the DMA32 zone, otherwise 
> random non-IO page allocations would also potentially exhaust the 
> DMA32 zone without anybody caring, which means removing this zone 
> wouldn't be any worse than whatever other subsystems may be doing 
> already...
>
> /Thomas
>
>
> On 2/16/19 12:02 AM, Kuehling, Felix wrote:
>> This is an RFC. I'm not sure this is the right solution, but it
>> highlights the problem I'm trying to solve.
>>
>> The dma32_zone limits the acc_size of all allocated BOs to 2GB. On a
>> 64-bit system with hundreds of GB of system memory and GPU memory,
>> this can become a bottle neck. We're seeing TTM memory allocation
>> failures not because we're truly out of memory, but because we're
>> out of space in the dma32_zone for the acc_size needed for our BO
>> book-keeping.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>> CC: thellstrom@vmware.com
>> CC: christian.koenig@amd.com
>> ---
>>   drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c 
>> b/drivers/gpu/drm/ttm/ttm_memory.c
>> index f1567c3..bb05365 100644
>> --- a/drivers/gpu/drm/ttm/ttm_memory.c
>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
>> @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct 
>> ttm_mem_global *glob,
>>       glob->zones[glob->num_zones++] = zone;
>>       return 0;
>>   }
>> -#else
>> +#elifndef CONFIG_64BIT
>>   static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob,
>>                      const struct sysinfo *si)
>>   {
>> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global *glob)
>>       ret = ttm_mem_init_highmem_zone(glob, &si);
>>       if (unlikely(ret != 0))
>>           goto out_no_zone;
>> -#else
>> +#elifndef CONFIG_64BIT
>>       ret = ttm_mem_init_dma32_zone(glob, &si);
>>       if (unlikely(ret != 0))
>>           goto out_no_zone;
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
> Another good question is also why the heck the acc_size counts
> towards 
> the DMA32 zone?

DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.

For small persistent allocations using ttm_mem_global_alloc(), they are
accounted also in the DMA32 zone, which may cause over-accounting of
that zone, but that's pretty unlikely to be a big problem..

/Thomas





> 
> In other words why should the internal bookkeeping pages be allocated
> in 
> the DMA32 zone?
> 
> That doesn't sounds valid to me in any way,
> Christian.
> 
> Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
> > Hmm,
> > 
> > This zone was intended to stop TTM page allocations from
> > exhausting 
> > the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default,
> > which 
> > means if we drop this check, other devices may stop functioning 
> > unexpectedly?
> > 
> > However, in the end I'd expect the kernel page allocation system
> > to 
> > make sure there are some pages left in the DMA32 zone, otherwise 
> > random non-IO page allocations would also potentially exhaust the 
> > DMA32 zone without anybody caring, which means removing this zone 
> > wouldn't be any worse than whatever other subsystems may be doing 
> > already...
> > 
> > /Thomas
> > 
> > 
> > On 2/16/19 12:02 AM, Kuehling, Felix wrote:
> > > This is an RFC. I'm not sure this is the right solution, but it
> > > highlights the problem I'm trying to solve.
> > > 
> > > The dma32_zone limits the acc_size of all allocated BOs to 2GB.
> > > On a
> > > 64-bit system with hundreds of GB of system memory and GPU
> > > memory,
> > > this can become a bottle neck. We're seeing TTM memory allocation
> > > failures not because we're truly out of memory, but because we're
> > > out of space in the dma32_zone for the acc_size needed for our BO
> > > book-keeping.
> > > 
> > > Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> > > CC: thellstrom@vmware.com
> > > CC: christian.koenig@amd.com
> > > ---
> > >   drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c 
> > > b/drivers/gpu/drm/ttm/ttm_memory.c
> > > index f1567c3..bb05365 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_memory.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> > > @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct 
> > > ttm_mem_global *glob,
> > >       glob->zones[glob->num_zones++] = zone;
> > >       return 0;
> > >   }
> > > -#else
> > > +#elifndef CONFIG_64BIT
> > >   static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob,
> > >                      const struct sysinfo *si)
> > >   {
> > > @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global
> > > *glob)
> > >       ret = ttm_mem_init_highmem_zone(glob, &si);
> > >       if (unlikely(ret != 0))
> > >           goto out_no_zone;
> > > -#else
> > > +#elifndef CONFIG_64BIT
> > >       ret = ttm_mem_init_dma32_zone(glob, &si);
> > >       if (unlikely(ret != 0))
> > >           goto out_no_zone;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Christian König]

Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
> On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
>> Another good question is also why the heck the acc_size counts
>> towards
>> the DMA32 zone?
> DMA32 TTM pages are accounted in the DMA32 zone. Other pages are not.

Yeah, I'm perfectly aware of this. But this is for the accounting size!

We have an accounting for the stuff needed additional to the pages 
backing the BO (e.g. the page and DMA addr array).

And from the bug description it sounds like we use the DMA32 zone for 
this accounting which of course is completely nonsense.

Christian.

>
> For small persistent allocations using ttm_mem_global_alloc(), they are
> accounted also in the DMA32 zone, which may cause over-accounting of
> that zone, but that's pretty unlikely to be a big problem..
>
> /Thomas
>
>
>
>
>
>> In other words why should the internal bookkeeping pages be allocated
>> in
>> the DMA32 zone?
>>
>> That doesn't sounds valid to me in any way,
>> Christian.
>>
>> Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
>>> Hmm,
>>>
>>> This zone was intended to stop TTM page allocations from
>>> exhausting
>>> the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by default,
>>> which
>>> means if we drop this check, other devices may stop functioning
>>> unexpectedly?
>>>
>>> However, in the end I'd expect the kernel page allocation system
>>> to
>>> make sure there are some pages left in the DMA32 zone, otherwise
>>> random non-IO page allocations would also potentially exhaust the
>>> DMA32 zone without anybody caring, which means removing this zone
>>> wouldn't be any worse than whatever other subsystems may be doing
>>> already...
>>>
>>> /Thomas
>>>
>>>
>>> On 2/16/19 12:02 AM, Kuehling, Felix wrote:
>>>> This is an RFC. I'm not sure this is the right solution, but it
>>>> highlights the problem I'm trying to solve.
>>>>
>>>> The dma32_zone limits the acc_size of all allocated BOs to 2GB.
>>>> On a
>>>> 64-bit system with hundreds of GB of system memory and GPU
>>>> memory,
>>>> this can become a bottle neck. We're seeing TTM memory allocation
>>>> failures not because we're truly out of memory, but because we're
>>>> out of space in the dma32_zone for the acc_size needed for our BO
>>>> book-keeping.
>>>>
>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>> CC: thellstrom@vmware.com
>>>> CC: christian.koenig@amd.com
>>>> ---
>>>>    drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
>>>> b/drivers/gpu/drm/ttm/ttm_memory.c
>>>> index f1567c3..bb05365 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_memory.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
>>>> @@ -363,7 +363,7 @@ static int ttm_mem_init_highmem_zone(struct
>>>> ttm_mem_global *glob,
>>>>        glob->zones[glob->num_zones++] = zone;
>>>>        return 0;
>>>>    }
>>>> -#else
>>>> +#elifndef CONFIG_64BIT
>>>>    static int ttm_mem_init_dma32_zone(struct ttm_mem_global *glob,
>>>>                       const struct sysinfo *si)
>>>>    {
>>>> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct ttm_mem_global
>>>> *glob)
>>>>        ret = ttm_mem_init_highmem_zone(glob, &si);
>>>>        if (unlikely(ret != 0))
>>>>            goto out_no_zone;
>>>> -#else
>>>> +#elifndef CONFIG_64BIT
>>>>        ret = ttm_mem_init_dma32_zone(glob, &si);
>>>>        if (unlikely(ret != 0))
>>>>            goto out_no_zone;
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
> Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
> > On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
> > > Another good question is also why the heck the acc_size counts
> > > towards
> > > the DMA32 zone?
> > DMA32 TTM pages are accounted in the DMA32 zone. Other pages are
> > not.
> 
> Yeah, I'm perfectly aware of this. But this is for the accounting
> size!
> 
> We have an accounting for the stuff needed additional to the pages 
> backing the BO (e.g. the page and DMA addr array).
> 
> And from the bug description it sounds like we use the DMA32 zone
> for 
> this accounting which of course is completely nonsense.

It's actually accounted in all available zones, since it would be
pretty hard to determine exactly where that memory should be accounted.
In particular if it's vmalloced. It might be DMA32, it might not. Given
the objective of stopping malicious user-space from exhausting the
DMA32 zone it was, at the time the code was written, a reasonable
approximation. With ever increasing memory sizes, there might be better
solutions?

/Thomas

> 
> Christian.
> 
> > For small persistent allocations using ttm_mem_global_alloc(), they
> > are
> > accounted also in the DMA32 zone, which may cause over-accounting
> > of
> > that zone, but that's pretty unlikely to be a big problem..
> > 
> > /Thomas
> > 
> > 
> > 
> > 
> > 
> > > In other words why should the internal bookkeeping pages be
> > > allocated
> > > in
> > > the DMA32 zone?
> > > 
> > > That doesn't sounds valid to me in any way,
> > > Christian.
> > > 
> > > Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
> > > > Hmm,
> > > > 
> > > > This zone was intended to stop TTM page allocations from
> > > > exhausting
> > > > the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by
> > > > default,
> > > > which
> > > > means if we drop this check, other devices may stop functioning
> > > > unexpectedly?
> > > > 
> > > > However, in the end I'd expect the kernel page allocation
> > > > system
> > > > to
> > > > make sure there are some pages left in the DMA32 zone,
> > > > otherwise
> > > > random non-IO page allocations would also potentially exhaust
> > > > the
> > > > DMA32 zone without anybody caring, which means removing this
> > > > zone
> > > > wouldn't be any worse than whatever other subsystems may be
> > > > doing
> > > > already...
> > > > 
> > > > /Thomas
> > > > 
> > > > 
> > > > On 2/16/19 12:02 AM, Kuehling, Felix wrote:
> > > > > This is an RFC. I'm not sure this is the right solution, but
> > > > > it
> > > > > highlights the problem I'm trying to solve.
> > > > > 
> > > > > The dma32_zone limits the acc_size of all allocated BOs to
> > > > > 2GB.
> > > > > On a
> > > > > 64-bit system with hundreds of GB of system memory and GPU
> > > > > memory,
> > > > > this can become a bottle neck. We're seeing TTM memory
> > > > > allocation
> > > > > failures not because we're truly out of memory, but because
> > > > > we're
> > > > > out of space in the dma32_zone for the acc_size needed for
> > > > > our BO
> > > > > book-keeping.
> > > > > 
> > > > > Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> > > > > CC: thellstrom@vmware.com
> > > > > CC: christian.koenig@amd.com
> > > > > ---
> > > > >    drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
> > > > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > b/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > index f1567c3..bb05365 100644
> > > > > --- a/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > @@ -363,7 +363,7 @@ static int
> > > > > ttm_mem_init_highmem_zone(struct
> > > > > ttm_mem_global *glob,
> > > > >        glob->zones[glob->num_zones++] = zone;
> > > > >        return 0;
> > > > >    }
> > > > > -#else
> > > > > +#elifndef CONFIG_64BIT
> > > > >    static int ttm_mem_init_dma32_zone(struct ttm_mem_global
> > > > > *glob,
> > > > >                       const struct sysinfo *si)
> > > > >    {
> > > > > @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct
> > > > > ttm_mem_global
> > > > > *glob)
> > > > >        ret = ttm_mem_init_highmem_zone(glob, &si);
> > > > >        if (unlikely(ret != 0))
> > > > >            goto out_no_zone;
> > > > > -#else
> > > > > +#elifndef CONFIG_64BIT
> > > > >        ret = ttm_mem_init_dma32_zone(glob, &si);
> > > > >        if (unlikely(ret != 0))
> > > > >            goto out_no_zone;
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C1a84a2a700cd48b3980a08d695c38ade%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636861064581303965&amp;sdata=6FCAP6YbostgKfEEhRKKh9eQSrfXR%2BfCczYB8WwlPVY%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Kuehling, Felix]

On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
> On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
>> Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
>>> On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
>>>> Another good question is also why the heck the acc_size counts
>>>> towards
>>>> the DMA32 zone?
>>> DMA32 TTM pages are accounted in the DMA32 zone. Other pages are
>>> not.
>> Yeah, I'm perfectly aware of this. But this is for the accounting
>> size!
>>
>> We have an accounting for the stuff needed additional to the pages
>> backing the BO (e.g. the page and DMA addr array).
>>
>> And from the bug description it sounds like we use the DMA32 zone
>> for
>> this accounting which of course is completely nonsense.
> It's actually accounted in all available zones, since it would be
> pretty hard to determine exactly where that memory should be accounted.
> In particular if it's vmalloced. It might be DMA32, it might not. Given
> the objective of stopping malicious user-space from exhausting the
> DMA32 zone it was, at the time the code was written, a reasonable
> approximation. With ever increasing memory sizes, there might be better
> solutions?

As far as I can see, in TTM, ttm_mem_global_alloc is only used for the 
acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems to 
use it to account for a few things that are allocated with kmalloc.

So would a better solution be to change ttm_mem_global_alloc to use only 
the kernel zone?

Regards,
   Felix


>
> /Thomas
>
>> Christian.
>>
>>> For small persistent allocations using ttm_mem_global_alloc(), they
>>> are
>>> accounted also in the DMA32 zone, which may cause over-accounting
>>> of
>>> that zone, but that's pretty unlikely to be a big problem..
>>>
>>> /Thomas
>>>
>>>
>>>
>>>
>>>
>>>> In other words why should the internal bookkeeping pages be
>>>> allocated
>>>> in
>>>> the DMA32 zone?
>>>>
>>>> That doesn't sounds valid to me in any way,
>>>> Christian.
>>>>
>>>> Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
>>>>> Hmm,
>>>>>
>>>>> This zone was intended to stop TTM page allocations from
>>>>> exhausting
>>>>> the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by
>>>>> default,
>>>>> which
>>>>> means if we drop this check, other devices may stop functioning
>>>>> unexpectedly?
>>>>>
>>>>> However, in the end I'd expect the kernel page allocation
>>>>> system
>>>>> to
>>>>> make sure there are some pages left in the DMA32 zone,
>>>>> otherwise
>>>>> random non-IO page allocations would also potentially exhaust
>>>>> the
>>>>> DMA32 zone without anybody caring, which means removing this
>>>>> zone
>>>>> wouldn't be any worse than whatever other subsystems may be
>>>>> doing
>>>>> already...
>>>>>
>>>>> /Thomas
>>>>>
>>>>>
>>>>> On 2/16/19 12:02 AM, Kuehling, Felix wrote:
>>>>>> This is an RFC. I'm not sure this is the right solution, but
>>>>>> it
>>>>>> highlights the problem I'm trying to solve.
>>>>>>
>>>>>> The dma32_zone limits the acc_size of all allocated BOs to
>>>>>> 2GB.
>>>>>> On a
>>>>>> 64-bit system with hundreds of GB of system memory and GPU
>>>>>> memory,
>>>>>> this can become a bottle neck. We're seeing TTM memory
>>>>>> allocation
>>>>>> failures not because we're truly out of memory, but because
>>>>>> we're
>>>>>> out of space in the dma32_zone for the acc_size needed for
>>>>>> our BO
>>>>>> book-keeping.
>>>>>>
>>>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>>>> CC: thellstrom@vmware.com
>>>>>> CC: christian.koenig@amd.com
>>>>>> ---
>>>>>>     drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>> b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>> index f1567c3..bb05365 100644
>>>>>> --- a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>> @@ -363,7 +363,7 @@ static int
>>>>>> ttm_mem_init_highmem_zone(struct
>>>>>> ttm_mem_global *glob,
>>>>>>         glob->zones[glob->num_zones++] = zone;
>>>>>>         return 0;
>>>>>>     }
>>>>>> -#else
>>>>>> +#elifndef CONFIG_64BIT
>>>>>>     static int ttm_mem_init_dma32_zone(struct ttm_mem_global
>>>>>> *glob,
>>>>>>                        const struct sysinfo *si)
>>>>>>     {
>>>>>> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct
>>>>>> ttm_mem_global
>>>>>> *glob)
>>>>>>         ret = ttm_mem_init_highmem_zone(glob, &si);
>>>>>>         if (unlikely(ret != 0))
>>>>>>             goto out_no_zone;
>>>>>> -#else
>>>>>> +#elifndef CONFIG_64BIT
>>>>>>         ret = ttm_mem_init_dma32_zone(glob, &si);
>>>>>>         if (unlikely(ret != 0))
>>>>>>             goto out_no_zone;
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C1a84a2a700cd48b3980a08d695c38ade%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636861064581303965&amp;sdata=6FCAP6YbostgKfEEhRKKh9eQSrfXR%2BfCczYB8WwlPVY%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
> On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
> > On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
> > > Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
> > > > On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
> > > > > Another good question is also why the heck the acc_size
> > > > > counts
> > > > > towards
> > > > > the DMA32 zone?
> > > > DMA32 TTM pages are accounted in the DMA32 zone. Other pages
> > > > are
> > > > not.
> > > Yeah, I'm perfectly aware of this. But this is for the accounting
> > > size!
> > > 
> > > We have an accounting for the stuff needed additional to the
> > > pages
> > > backing the BO (e.g. the page and DMA addr array).
> > > 
> > > And from the bug description it sounds like we use the DMA32 zone
> > > for
> > > this accounting which of course is completely nonsense.
> > It's actually accounted in all available zones, since it would be
> > pretty hard to determine exactly where that memory should be
> > accounted.
> > In particular if it's vmalloced. It might be DMA32, it might not.
> > Given
> > the objective of stopping malicious user-space from exhausting the
> > DMA32 zone it was, at the time the code was written, a reasonable
> > approximation. With ever increasing memory sizes, there might be
> > better
> > solutions?
> 
> As far as I can see, in TTM, ttm_mem_global_alloc is only used for
> the 
> acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems
> to 
> use it to account for a few things that are allocated with kmalloc.
> 
> So would a better solution be to change ttm_mem_global_alloc to use
> only 
> the kernel zone?
> 

IMO we need to determine what functionality to keep and then the best
solution. The current code does its job, but is obviously too
restrictive. Both of the solutions you suggest open up for potential
DOS attacks (DMA32 and kernel zones are not mutually exclusive. They
overlap).


/Thomas




> Regards,
>    Felix
> 
> 
> > /Thomas
> > 
> > > Christian.
> > > 
> > > > For small persistent allocations using ttm_mem_global_alloc(),
> > > > they
> > > > are
> > > > accounted also in the DMA32 zone, which may cause over-
> > > > accounting
> > > > of
> > > > that zone, but that's pretty unlikely to be a big problem..
> > > > 
> > > > /Thomas
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > > In other words why should the internal bookkeeping pages be
> > > > > allocated
> > > > > in
> > > > > the DMA32 zone?
> > > > > 
> > > > > That doesn't sounds valid to me in any way,
> > > > > Christian.
> > > > > 
> > > > > Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
> > > > > > Hmm,
> > > > > > 
> > > > > > This zone was intended to stop TTM page allocations from
> > > > > > exhausting
> > > > > > the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by
> > > > > > default,
> > > > > > which
> > > > > > means if we drop this check, other devices may stop
> > > > > > functioning
> > > > > > unexpectedly?
> > > > > > 
> > > > > > However, in the end I'd expect the kernel page allocation
> > > > > > system
> > > > > > to
> > > > > > make sure there are some pages left in the DMA32 zone,
> > > > > > otherwise
> > > > > > random non-IO page allocations would also potentially
> > > > > > exhaust
> > > > > > the
> > > > > > DMA32 zone without anybody caring, which means removing
> > > > > > this
> > > > > > zone
> > > > > > wouldn't be any worse than whatever other subsystems may be
> > > > > > doing
> > > > > > already...
> > > > > > 
> > > > > > /Thomas
> > > > > > 
> > > > > > 
> > > > > > On 2/16/19 12:02 AM, Kuehling, Felix wrote:
> > > > > > > This is an RFC. I'm not sure this is the right solution,
> > > > > > > but
> > > > > > > it
> > > > > > > highlights the problem I'm trying to solve.
> > > > > > > 
> > > > > > > The dma32_zone limits the acc_size of all allocated BOs
> > > > > > > to
> > > > > > > 2GB.
> > > > > > > On a
> > > > > > > 64-bit system with hundreds of GB of system memory and
> > > > > > > GPU
> > > > > > > memory,
> > > > > > > this can become a bottle neck. We're seeing TTM memory
> > > > > > > allocation
> > > > > > > failures not because we're truly out of memory, but
> > > > > > > because
> > > > > > > we're
> > > > > > > out of space in the dma32_zone for the acc_size needed
> > > > > > > for
> > > > > > > our BO
> > > > > > > book-keeping.
> > > > > > > 
> > > > > > > Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> > > > > > > CC: thellstrom@vmware.com
> > > > > > > CC: christian.koenig@amd.com
> > > > > > > ---
> > > > > > >     drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
> > > > > > >     1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > b/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > index f1567c3..bb05365 100644
> > > > > > > --- a/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > @@ -363,7 +363,7 @@ static int
> > > > > > > ttm_mem_init_highmem_zone(struct
> > > > > > > ttm_mem_global *glob,
> > > > > > >         glob->zones[glob->num_zones++] = zone;
> > > > > > >         return 0;
> > > > > > >     }
> > > > > > > -#else
> > > > > > > +#elifndef CONFIG_64BIT
> > > > > > >     static int ttm_mem_init_dma32_zone(struct
> > > > > > > ttm_mem_global
> > > > > > > *glob,
> > > > > > >                        const struct sysinfo *si)
> > > > > > >     {
> > > > > > > @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct
> > > > > > > ttm_mem_global
> > > > > > > *glob)
> > > > > > >         ret = ttm_mem_init_highmem_zone(glob, &si);
> > > > > > >         if (unlikely(ret != 0))
> > > > > > >             goto out_no_zone;
> > > > > > > -#else
> > > > > > > +#elifndef CONFIG_64BIT
> > > > > > >         ret = ttm_mem_init_dma32_zone(glob, &si);
> > > > > > >         if (unlikely(ret != 0))
> > > > > > >             goto out_no_zone;
> > > > _______________________________________________
> > > > amd-gfx mailing list
> > > > amd-gfx@lists.freedesktop.org
> > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C1357d06244fb499b31dd08d6968ca04b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636861928079462725&amp;sdata=1Bucho93KMzN0z7QbfiNn%2BNWaZs5yi86Ya6vm9Xhbqo%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Christian König]

Am 20.02.19 um 07:41 schrieb Thomas Hellstrom:
> On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
>> On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
>>> On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
>>>> Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
>>>>> On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
>>>>>> Another good question is also why the heck the acc_size
>>>>>> counts
>>>>>> towards
>>>>>> the DMA32 zone?
>>>>> DMA32 TTM pages are accounted in the DMA32 zone. Other pages
>>>>> are
>>>>> not.
>>>> Yeah, I'm perfectly aware of this. But this is for the accounting
>>>> size!
>>>>
>>>> We have an accounting for the stuff needed additional to the
>>>> pages
>>>> backing the BO (e.g. the page and DMA addr array).
>>>>
>>>> And from the bug description it sounds like we use the DMA32 zone
>>>> for
>>>> this accounting which of course is completely nonsense.
>>> It's actually accounted in all available zones, since it would be
>>> pretty hard to determine exactly where that memory should be
>>> accounted.
>>> In particular if it's vmalloced. It might be DMA32, it might not.
>>> Given
>>> the objective of stopping malicious user-space from exhausting the
>>> DMA32 zone it was, at the time the code was written, a reasonable
>>> approximation. With ever increasing memory sizes, there might be
>>> better
>>> solutions?
>> As far as I can see, in TTM, ttm_mem_global_alloc is only used for
>> the
>> acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems
>> to
>> use it to account for a few things that are allocated with kmalloc.
>>
>> So would a better solution be to change ttm_mem_global_alloc to use
>> only
>> the kernel zone?
>>
> IMO we need to determine what functionality to keep and then the best
> solution. The current code does its job, but is obviously too
> restrictive. Both of the solutions you suggest open up for potential
> DOS attacks (DMA32 and kernel zones are not mutually exclusive. They
> overlap).

Yeah and exactly because of this it doesn't make to much sense to 
account the housekeeping stuff in both zones.

IIRC the kernel takes perfectly care by itself that kmalloced memory 
doesn't eat up everything in the DMA32 zone.

Christian.

>
>
> /Thomas
>
>
>
>
>> Regards,
>>     Felix
>>
>>
>>> /Thomas
>>>
>>>> Christian.
>>>>
>>>>> For small persistent allocations using ttm_mem_global_alloc(),
>>>>> they
>>>>> are
>>>>> accounted also in the DMA32 zone, which may cause over-
>>>>> accounting
>>>>> of
>>>>> that zone, but that's pretty unlikely to be a big problem..
>>>>>
>>>>> /Thomas
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> In other words why should the internal bookkeeping pages be
>>>>>> allocated
>>>>>> in
>>>>>> the DMA32 zone?
>>>>>>
>>>>>> That doesn't sounds valid to me in any way,
>>>>>> Christian.
>>>>>>
>>>>>> Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
>>>>>>> Hmm,
>>>>>>>
>>>>>>> This zone was intended to stop TTM page allocations from
>>>>>>> exhausting
>>>>>>> the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by
>>>>>>> default,
>>>>>>> which
>>>>>>> means if we drop this check, other devices may stop
>>>>>>> functioning
>>>>>>> unexpectedly?
>>>>>>>
>>>>>>> However, in the end I'd expect the kernel page allocation
>>>>>>> system
>>>>>>> to
>>>>>>> make sure there are some pages left in the DMA32 zone,
>>>>>>> otherwise
>>>>>>> random non-IO page allocations would also potentially
>>>>>>> exhaust
>>>>>>> the
>>>>>>> DMA32 zone without anybody caring, which means removing
>>>>>>> this
>>>>>>> zone
>>>>>>> wouldn't be any worse than whatever other subsystems may be
>>>>>>> doing
>>>>>>> already...
>>>>>>>
>>>>>>> /Thomas
>>>>>>>
>>>>>>>
>>>>>>> On 2/16/19 12:02 AM, Kuehling, Felix wrote:
>>>>>>>> This is an RFC. I'm not sure this is the right solution,
>>>>>>>> but
>>>>>>>> it
>>>>>>>> highlights the problem I'm trying to solve.
>>>>>>>>
>>>>>>>> The dma32_zone limits the acc_size of all allocated BOs
>>>>>>>> to
>>>>>>>> 2GB.
>>>>>>>> On a
>>>>>>>> 64-bit system with hundreds of GB of system memory and
>>>>>>>> GPU
>>>>>>>> memory,
>>>>>>>> this can become a bottle neck. We're seeing TTM memory
>>>>>>>> allocation
>>>>>>>> failures not because we're truly out of memory, but
>>>>>>>> because
>>>>>>>> we're
>>>>>>>> out of space in the dma32_zone for the acc_size needed
>>>>>>>> for
>>>>>>>> our BO
>>>>>>>> book-keeping.
>>>>>>>>
>>>>>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>>>>>> CC: thellstrom@vmware.com
>>>>>>>> CC: christian.koenig@amd.com
>>>>>>>> ---
>>>>>>>>      drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
>>>>>>>>      1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>> b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>> index f1567c3..bb05365 100644
>>>>>>>> --- a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>> @@ -363,7 +363,7 @@ static int
>>>>>>>> ttm_mem_init_highmem_zone(struct
>>>>>>>> ttm_mem_global *glob,
>>>>>>>>          glob->zones[glob->num_zones++] = zone;
>>>>>>>>          return 0;
>>>>>>>>      }
>>>>>>>> -#else
>>>>>>>> +#elifndef CONFIG_64BIT
>>>>>>>>      static int ttm_mem_init_dma32_zone(struct
>>>>>>>> ttm_mem_global
>>>>>>>> *glob,
>>>>>>>>                         const struct sysinfo *si)
>>>>>>>>      {
>>>>>>>> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct
>>>>>>>> ttm_mem_global
>>>>>>>> *glob)
>>>>>>>>          ret = ttm_mem_init_highmem_zone(glob, &si);
>>>>>>>>          if (unlikely(ret != 0))
>>>>>>>>              goto out_no_zone;
>>>>>>>> -#else
>>>>>>>> +#elifndef CONFIG_64BIT
>>>>>>>>          ret = ttm_mem_init_dma32_zone(glob, &si);
>>>>>>>>          if (unlikely(ret != 0))
>>>>>>>>              goto out_no_zone;
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C1357d06244fb499b31dd08d6968ca04b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636861928079462725&amp;sdata=1Bucho93KMzN0z7QbfiNn%2BNWaZs5yi86Ya6vm9Xhbqo%3D&amp;reserved=0
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

On 2/20/19 9:07 AM, Christian König wrote:
> Am 20.02.19 um 07:41 schrieb Thomas Hellstrom:
>> On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
>>> On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
>>>> On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
>>>>> Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
>>>>>> On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
>>>>>>> Another good question is also why the heck the acc_size
>>>>>>> counts
>>>>>>> towards
>>>>>>> the DMA32 zone?
>>>>>> DMA32 TTM pages are accounted in the DMA32 zone. Other pages
>>>>>> are
>>>>>> not.
>>>>> Yeah, I'm perfectly aware of this. But this is for the accounting
>>>>> size!
>>>>>
>>>>> We have an accounting for the stuff needed additional to the
>>>>> pages
>>>>> backing the BO (e.g. the page and DMA addr array).
>>>>>
>>>>> And from the bug description it sounds like we use the DMA32 zone
>>>>> for
>>>>> this accounting which of course is completely nonsense.
>>>> It's actually accounted in all available zones, since it would be
>>>> pretty hard to determine exactly where that memory should be
>>>> accounted.
>>>> In particular if it's vmalloced. It might be DMA32, it might not.
>>>> Given
>>>> the objective of stopping malicious user-space from exhausting the
>>>> DMA32 zone it was, at the time the code was written, a reasonable
>>>> approximation. With ever increasing memory sizes, there might be
>>>> better
>>>> solutions?
>>> As far as I can see, in TTM, ttm_mem_global_alloc is only used for
>>> the
>>> acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems
>>> to
>>> use it to account for a few things that are allocated with kmalloc.
>>>
>>> So would a better solution be to change ttm_mem_global_alloc to use
>>> only
>>> the kernel zone?
>>>
>> IMO we need to determine what functionality to keep and then the best
>> solution. The current code does its job, but is obviously too
>> restrictive. Both of the solutions you suggest open up for potential
>> DOS attacks (DMA32 and kernel zones are not mutually exclusive. They
>> overlap).
>
> Yeah and exactly because of this it doesn't make to much sense to 
> account the housekeeping stuff in both zones.

IMO it makes sense because (given the assumption that kmalloc/vmalloc 
doesn't care) , accounting only in DMA32 in low memory configurations 
will not guarantee that kernel is not exhausted. Accounting only in 
kernel in huge memory configurations will not guarantee that DMA32 is 
not exhausted.

>
> IIRC the kernel takes perfectly care by itself that kmalloced memory 
> doesn't eat up everything in the DMA32 zone.

If we can somehow verify this, or at least illustrate that it's likely, 
I'm perfectly fine with either patch from the vmwgfx POW.

Thanks,
Thomas



>
> Christian.
>
>>
>>
>> /Thomas
>>
>>
>>
>>
>>> Regards,
>>>     Felix
>>>
>>>
>>>> /Thomas
>>>>
>>>>> Christian.
>>>>>
>>>>>> For small persistent allocations using ttm_mem_global_alloc(),
>>>>>> they
>>>>>> are
>>>>>> accounted also in the DMA32 zone, which may cause over-
>>>>>> accounting
>>>>>> of
>>>>>> that zone, but that's pretty unlikely to be a big problem..
>>>>>>
>>>>>> /Thomas
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> In other words why should the internal bookkeeping pages be
>>>>>>> allocated
>>>>>>> in
>>>>>>> the DMA32 zone?
>>>>>>>
>>>>>>> That doesn't sounds valid to me in any way,
>>>>>>> Christian.
>>>>>>>
>>>>>>> Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
>>>>>>>> Hmm,
>>>>>>>>
>>>>>>>> This zone was intended to stop TTM page allocations from
>>>>>>>> exhausting
>>>>>>>> the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by
>>>>>>>> default,
>>>>>>>> which
>>>>>>>> means if we drop this check, other devices may stop
>>>>>>>> functioning
>>>>>>>> unexpectedly?
>>>>>>>>
>>>>>>>> However, in the end I'd expect the kernel page allocation
>>>>>>>> system
>>>>>>>> to
>>>>>>>> make sure there are some pages left in the DMA32 zone,
>>>>>>>> otherwise
>>>>>>>> random non-IO page allocations would also potentially
>>>>>>>> exhaust
>>>>>>>> the
>>>>>>>> DMA32 zone without anybody caring, which means removing
>>>>>>>> this
>>>>>>>> zone
>>>>>>>> wouldn't be any worse than whatever other subsystems may be
>>>>>>>> doing
>>>>>>>> already...
>>>>>>>>
>>>>>>>> /Thomas
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2/16/19 12:02 AM, Kuehling, Felix wrote:
>>>>>>>>> This is an RFC. I'm not sure this is the right solution,
>>>>>>>>> but
>>>>>>>>> it
>>>>>>>>> highlights the problem I'm trying to solve.
>>>>>>>>>
>>>>>>>>> The dma32_zone limits the acc_size of all allocated BOs
>>>>>>>>> to
>>>>>>>>> 2GB.
>>>>>>>>> On a
>>>>>>>>> 64-bit system with hundreds of GB of system memory and
>>>>>>>>> GPU
>>>>>>>>> memory,
>>>>>>>>> this can become a bottle neck. We're seeing TTM memory
>>>>>>>>> allocation
>>>>>>>>> failures not because we're truly out of memory, but
>>>>>>>>> because
>>>>>>>>> we're
>>>>>>>>> out of space in the dma32_zone for the acc_size needed
>>>>>>>>> for
>>>>>>>>> our BO
>>>>>>>>> book-keeping.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>>>>>>> CC: thellstrom@vmware.com
>>>>>>>>> CC: christian.koenig@amd.com
>>>>>>>>> ---
>>>>>>>>>      drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
>>>>>>>>>      1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>>> b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>>> index f1567c3..bb05365 100644
>>>>>>>>> --- a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>>> @@ -363,7 +363,7 @@ static int
>>>>>>>>> ttm_mem_init_highmem_zone(struct
>>>>>>>>> ttm_mem_global *glob,
>>>>>>>>>          glob->zones[glob->num_zones++] = zone;
>>>>>>>>>          return 0;
>>>>>>>>>      }
>>>>>>>>> -#else
>>>>>>>>> +#elifndef CONFIG_64BIT
>>>>>>>>>      static int ttm_mem_init_dma32_zone(struct
>>>>>>>>> ttm_mem_global
>>>>>>>>> *glob,
>>>>>>>>>                         const struct sysinfo *si)
>>>>>>>>>      {
>>>>>>>>> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct
>>>>>>>>> ttm_mem_global
>>>>>>>>> *glob)
>>>>>>>>>          ret = ttm_mem_init_highmem_zone(glob, &si);
>>>>>>>>>          if (unlikely(ret != 0))
>>>>>>>>>              goto out_no_zone;
>>>>>>>>> -#else
>>>>>>>>> +#elifndef CONFIG_64BIT
>>>>>>>>>          ret = ttm_mem_init_dma32_zone(glob, &si);
>>>>>>>>>          if (unlikely(ret != 0))
>>>>>>>>>              goto out_no_zone;
>>>>>> _______________________________________________
>>>>>> amd-gfx mailing list
>>>>>> amd-gfx@lists.freedesktop.org
>>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C1357d06244fb499b31dd08d6968ca04b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636861928079462725&amp;sdata=1Bucho93KMzN0z7QbfiNn%2BNWaZs5yi86Ya6vm9Xhbqo%3D&amp;reserved=0 
>>>>>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Koenig, Christian]

Am 20.02.19 um 09:14 schrieb Thomas Hellstrom:
> On 2/20/19 9:07 AM, Christian König wrote:
>> Am 20.02.19 um 07:41 schrieb Thomas Hellstrom:
>>> On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
>>>> On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
>>>>> On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
>>>>>> Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
>>>>>>> On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
>>>>>>>> Another good question is also why the heck the acc_size
>>>>>>>> counts
>>>>>>>> towards
>>>>>>>> the DMA32 zone?
>>>>>>> DMA32 TTM pages are accounted in the DMA32 zone. Other pages
>>>>>>> are
>>>>>>> not.
>>>>>> Yeah, I'm perfectly aware of this. But this is for the accounting
>>>>>> size!
>>>>>>
>>>>>> We have an accounting for the stuff needed additional to the
>>>>>> pages
>>>>>> backing the BO (e.g. the page and DMA addr array).
>>>>>>
>>>>>> And from the bug description it sounds like we use the DMA32 zone
>>>>>> for
>>>>>> this accounting which of course is completely nonsense.
>>>>> It's actually accounted in all available zones, since it would be
>>>>> pretty hard to determine exactly where that memory should be
>>>>> accounted.
>>>>> In particular if it's vmalloced. It might be DMA32, it might not.
>>>>> Given
>>>>> the objective of stopping malicious user-space from exhausting the
>>>>> DMA32 zone it was, at the time the code was written, a reasonable
>>>>> approximation. With ever increasing memory sizes, there might be
>>>>> better
>>>>> solutions?
>>>> As far as I can see, in TTM, ttm_mem_global_alloc is only used for
>>>> the
>>>> acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems
>>>> to
>>>> use it to account for a few things that are allocated with kmalloc.
>>>>
>>>> So would a better solution be to change ttm_mem_global_alloc to use
>>>> only
>>>> the kernel zone?
>>>>
>>> IMO we need to determine what functionality to keep and then the best
>>> solution. The current code does its job, but is obviously too
>>> restrictive. Both of the solutions you suggest open up for potential
>>> DOS attacks (DMA32 and kernel zones are not mutually exclusive. They
>>> overlap).
>>
>> Yeah and exactly because of this it doesn't make to much sense to 
>> account the housekeeping stuff in both zones.
>
> IMO it makes sense because (given the assumption that kmalloc/vmalloc 
> doesn't care) , accounting only in DMA32 in low memory configurations 
> will not guarantee that kernel is not exhausted. Accounting only in 
> kernel in huge memory configurations will not guarantee that DMA32 is 
> not exhausted.

DMA32 is not exhausted by TTMs kmalloc/vmalloc because otherwise any 
other kmalloc/vmalloc could exhaust it as well.

I mean its probably 20+ years ago that I last looked at stuff in this 
part of the kernel, but I don't think we have ever changed the handling 
of DMA/DMA32 or otherwise we would be running into massive exhaustion 
problems already on modern systems.

>>
>> IIRC the kernel takes perfectly care by itself that kmalloced memory 
>> doesn't eat up everything in the DMA32 zone.
>
> If we can somehow verify this, or at least illustrate that it's 
> likely, I'm perfectly fine with either patch from the vmwgfx POW.

Well I actually think that the whole accounting of housekeeping towards 
the memory zones is completely pointless in the first place. Even if we 
swap things out we still have the same BO and TTM structures around anyway.

What we should rather to is to make sure that all processes using a BO 
have their RSS increased because they do so. So when a process tries to 
exhaust a memory domain it is simply killed by the OOM killer.

Regards,
Christian.

>
> Thanks,
> Thomas
>
>
>
>>
>> Christian.
>>
>>>
>>>
>>> /Thomas
>>>
>>>
>>>
>>>
>>>> Regards,
>>>>     Felix
>>>>
>>>>
>>>>> /Thomas
>>>>>
>>>>>> Christian.
>>>>>>
>>>>>>> For small persistent allocations using ttm_mem_global_alloc(),
>>>>>>> they
>>>>>>> are
>>>>>>> accounted also in the DMA32 zone, which may cause over-
>>>>>>> accounting
>>>>>>> of
>>>>>>> that zone, but that's pretty unlikely to be a big problem..
>>>>>>>
>>>>>>> /Thomas
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> In other words why should the internal bookkeeping pages be
>>>>>>>> allocated
>>>>>>>> in
>>>>>>>> the DMA32 zone?
>>>>>>>>
>>>>>>>> That doesn't sounds valid to me in any way,
>>>>>>>> Christian.
>>>>>>>>
>>>>>>>> Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
>>>>>>>>> Hmm,
>>>>>>>>>
>>>>>>>>> This zone was intended to stop TTM page allocations from
>>>>>>>>> exhausting
>>>>>>>>> the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by
>>>>>>>>> default,
>>>>>>>>> which
>>>>>>>>> means if we drop this check, other devices may stop
>>>>>>>>> functioning
>>>>>>>>> unexpectedly?
>>>>>>>>>
>>>>>>>>> However, in the end I'd expect the kernel page allocation
>>>>>>>>> system
>>>>>>>>> to
>>>>>>>>> make sure there are some pages left in the DMA32 zone,
>>>>>>>>> otherwise
>>>>>>>>> random non-IO page allocations would also potentially
>>>>>>>>> exhaust
>>>>>>>>> the
>>>>>>>>> DMA32 zone without anybody caring, which means removing
>>>>>>>>> this
>>>>>>>>> zone
>>>>>>>>> wouldn't be any worse than whatever other subsystems may be
>>>>>>>>> doing
>>>>>>>>> already...
>>>>>>>>>
>>>>>>>>> /Thomas
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2/16/19 12:02 AM, Kuehling, Felix wrote:
>>>>>>>>>> This is an RFC. I'm not sure this is the right solution,
>>>>>>>>>> but
>>>>>>>>>> it
>>>>>>>>>> highlights the problem I'm trying to solve.
>>>>>>>>>>
>>>>>>>>>> The dma32_zone limits the acc_size of all allocated BOs
>>>>>>>>>> to
>>>>>>>>>> 2GB.
>>>>>>>>>> On a
>>>>>>>>>> 64-bit system with hundreds of GB of system memory and
>>>>>>>>>> GPU
>>>>>>>>>> memory,
>>>>>>>>>> this can become a bottle neck. We're seeing TTM memory
>>>>>>>>>> allocation
>>>>>>>>>> failures not because we're truly out of memory, but
>>>>>>>>>> because
>>>>>>>>>> we're
>>>>>>>>>> out of space in the dma32_zone for the acc_size needed
>>>>>>>>>> for
>>>>>>>>>> our BO
>>>>>>>>>> book-keeping.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>>>>>>>> CC: thellstrom@vmware.com
>>>>>>>>>> CC: christian.koenig@amd.com
>>>>>>>>>> ---
>>>>>>>>>>      drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
>>>>>>>>>>      1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>>>> b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>>>> index f1567c3..bb05365 100644
>>>>>>>>>> --- a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>>>> @@ -363,7 +363,7 @@ static int
>>>>>>>>>> ttm_mem_init_highmem_zone(struct
>>>>>>>>>> ttm_mem_global *glob,
>>>>>>>>>>          glob->zones[glob->num_zones++] = zone;
>>>>>>>>>>          return 0;
>>>>>>>>>>      }
>>>>>>>>>> -#else
>>>>>>>>>> +#elifndef CONFIG_64BIT
>>>>>>>>>>      static int ttm_mem_init_dma32_zone(struct
>>>>>>>>>> ttm_mem_global
>>>>>>>>>> *glob,
>>>>>>>>>>                         const struct sysinfo *si)
>>>>>>>>>>      {
>>>>>>>>>> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct
>>>>>>>>>> ttm_mem_global
>>>>>>>>>> *glob)
>>>>>>>>>>          ret = ttm_mem_init_highmem_zone(glob, &si);
>>>>>>>>>>          if (unlikely(ret != 0))
>>>>>>>>>>              goto out_no_zone;
>>>>>>>>>> -#else
>>>>>>>>>> +#elifndef CONFIG_64BIT
>>>>>>>>>>          ret = ttm_mem_init_dma32_zone(glob, &si);
>>>>>>>>>>          if (unlikely(ret != 0))
>>>>>>>>>>              goto out_no_zone;
>>>>>>> _______________________________________________
>>>>>>> amd-gfx mailing list
>>>>>>> amd-gfx@lists.freedesktop.org
>>>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C1357d06244fb499b31dd08d6968ca04b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636861928079462725&amp;sdata=1Bucho93KMzN0z7QbfiNn%2BNWaZs5yi86Ya6vm9Xhbqo%3D&amp;reserved=0 
>>>>>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>

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

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

On Wed, 2019-02-20 at 08:35 +0000, Koenig, Christian wrote:
> Am 20.02.19 um 09:14 schrieb Thomas Hellstrom:
> > On 2/20/19 9:07 AM, Christian König wrote:
> > > Am 20.02.19 um 07:41 schrieb Thomas Hellstrom:
> > > > On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
> > > > > On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
> > > > > > On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
> > > > > > > Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
> > > > > > > > On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian
> > > > > > > > wrote:
> > > > > > > > > Another good question is also why the heck the
> > > > > > > > > acc_size
> > > > > > > > > counts
> > > > > > > > > towards
> > > > > > > > > the DMA32 zone?
> > > > > > > > DMA32 TTM pages are accounted in the DMA32 zone. Other
> > > > > > > > pages
> > > > > > > > are
> > > > > > > > not.
> > > > > > > Yeah, I'm perfectly aware of this. But this is for the
> > > > > > > accounting
> > > > > > > size!
> > > > > > > 
> > > > > > > We have an accounting for the stuff needed additional to
> > > > > > > the
> > > > > > > pages
> > > > > > > backing the BO (e.g. the page and DMA addr array).
> > > > > > > 
> > > > > > > And from the bug description it sounds like we use the
> > > > > > > DMA32 zone
> > > > > > > for
> > > > > > > this accounting which of course is completely nonsense.
> > > > > > It's actually accounted in all available zones, since it
> > > > > > would be
> > > > > > pretty hard to determine exactly where that memory should
> > > > > > be
> > > > > > accounted.
> > > > > > In particular if it's vmalloced. It might be DMA32, it
> > > > > > might not.
> > > > > > Given
> > > > > > the objective of stopping malicious user-space from
> > > > > > exhausting the
> > > > > > DMA32 zone it was, at the time the code was written, a
> > > > > > reasonable
> > > > > > approximation. With ever increasing memory sizes, there
> > > > > > might be
> > > > > > better
> > > > > > solutions?
> > > > > As far as I can see, in TTM, ttm_mem_global_alloc is only
> > > > > used for
> > > > > the
> > > > > acc_size in ttm_bo_init_reserved. Other than that vmwgfx also
> > > > > seems
> > > > > to
> > > > > use it to account for a few things that are allocated with
> > > > > kmalloc.
> > > > > 
> > > > > So would a better solution be to change ttm_mem_global_alloc
> > > > > to use
> > > > > only
> > > > > the kernel zone?
> > > > > 
> > > > IMO we need to determine what functionality to keep and then
> > > > the best
> > > > solution. The current code does its job, but is obviously too
> > > > restrictive. Both of the solutions you suggest open up for
> > > > potential
> > > > DOS attacks (DMA32 and kernel zones are not mutually exclusive.
> > > > They
> > > > overlap).
> > > 
> > > Yeah and exactly because of this it doesn't make to much sense
> > > to 
> > > account the housekeeping stuff in both zones.
> > 
> > IMO it makes sense because (given the assumption that
> > kmalloc/vmalloc 
> > doesn't care) , accounting only in DMA32 in low memory
> > configurations 
> > will not guarantee that kernel is not exhausted. Accounting only
> > in 
> > kernel in huge memory configurations will not guarantee that DMA32
> > is 
> > not exhausted.
> 
> DMA32 is not exhausted by TTMs kmalloc/vmalloc because otherwise any 
> other kmalloc/vmalloc could exhaust it as well.
> 
> I mean its probably 20+ years ago that I last looked at stuff in
> this 
> part of the kernel, but I don't think we have ever changed the
> handling 
> of DMA/DMA32 or otherwise we would be running into massive
> exhaustion 
> problems already on modern systems.
> 
> > > IIRC the kernel takes perfectly care by itself that kmalloced
> > > memory 
> > > doesn't eat up everything in the DMA32 zone.
> > 
> > If we can somehow verify this, or at least illustrate that it's 
> > likely, I'm perfectly fine with either patch from the vmwgfx POW.
> 
> Well I actually think that the whole accounting of housekeeping
> towards 
> the memory zones is completely pointless in the first place. Even if
> we 
> swap things out we still have the same BO and TTM structures around
> anyway.

Well, the whole point of this is to stop malicious user-space apps from
exhausting memory, causing a DOS vulnerability, similaraly for example
to how processes are stopped from creating to many open file
descriptors or other kernel structures. IIRC, for example, you used to
be able to call gem open in an infinite loop with the same bo until you
brought a random big process down using the OOM killer. With vmwgfx you
can't do that. You'd end up swapping out all available BOs and then
you'd get an error.

> 
> What we should rather to is to make sure that all processes using a
> BO 
> have their RSS increased because they do so. So when a process tries
> to 
> exhaust a memory domain it is simply killed by the OOM killer.

Yes, if there is a way to reliably do that and also take care of other
kernel structures that are created by user-space, like in the example
above that indeed sounds like a better solution.

/Thomas



> 
> Regards,
> Christian.
> 
> > Thanks,
> > Thomas
> > 
> > 
> > 
> > > Christian.
> > > 
> > > > 
> > > > /Thomas
> > > > 
> > > > 
> > > > 
> > > > 
> > > > > Regards,
> > > > >     Felix
> > > > > 
> > > > > 
> > > > > > /Thomas
> > > > > > 
> > > > > > > Christian.
> > > > > > > 
> > > > > > > > For small persistent allocations using
> > > > > > > > ttm_mem_global_alloc(),
> > > > > > > > they
> > > > > > > > are
> > > > > > > > accounted also in the DMA32 zone, which may cause over-
> > > > > > > > accounting
> > > > > > > > of
> > > > > > > > that zone, but that's pretty unlikely to be a big
> > > > > > > > problem..
> > > > > > > > 
> > > > > > > > /Thomas
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > > In other words why should the internal bookkeeping
> > > > > > > > > pages be
> > > > > > > > > allocated
> > > > > > > > > in
> > > > > > > > > the DMA32 zone?
> > > > > > > > > 
> > > > > > > > > That doesn't sounds valid to me in any way,
> > > > > > > > > Christian.
> > > > > > > > > 
> > > > > > > > > Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
> > > > > > > > > > Hmm,
> > > > > > > > > > 
> > > > > > > > > > This zone was intended to stop TTM page allocations
> > > > > > > > > > from
> > > > > > > > > > exhausting
> > > > > > > > > > the DMA32 zone. IIRC dma_alloc_coherent() uses
> > > > > > > > > > DMA32 by
> > > > > > > > > > default,
> > > > > > > > > > which
> > > > > > > > > > means if we drop this check, other devices may stop
> > > > > > > > > > functioning
> > > > > > > > > > unexpectedly?
> > > > > > > > > > 
> > > > > > > > > > However, in the end I'd expect the kernel page
> > > > > > > > > > allocation
> > > > > > > > > > system
> > > > > > > > > > to
> > > > > > > > > > make sure there are some pages left in the DMA32
> > > > > > > > > > zone,
> > > > > > > > > > otherwise
> > > > > > > > > > random non-IO page allocations would also
> > > > > > > > > > potentially
> > > > > > > > > > exhaust
> > > > > > > > > > the
> > > > > > > > > > DMA32 zone without anybody caring, which means
> > > > > > > > > > removing
> > > > > > > > > > this
> > > > > > > > > > zone
> > > > > > > > > > wouldn't be any worse than whatever other
> > > > > > > > > > subsystems may be
> > > > > > > > > > doing
> > > > > > > > > > already...
> > > > > > > > > > 
> > > > > > > > > > /Thomas
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > On 2/16/19 12:02 AM, Kuehling, Felix wrote:
> > > > > > > > > > > This is an RFC. I'm not sure this is the right
> > > > > > > > > > > solution,
> > > > > > > > > > > but
> > > > > > > > > > > it
> > > > > > > > > > > highlights the problem I'm trying to solve.
> > > > > > > > > > > 
> > > > > > > > > > > The dma32_zone limits the acc_size of all
> > > > > > > > > > > allocated BOs
> > > > > > > > > > > to
> > > > > > > > > > > 2GB.
> > > > > > > > > > > On a
> > > > > > > > > > > 64-bit system with hundreds of GB of system
> > > > > > > > > > > memory and
> > > > > > > > > > > GPU
> > > > > > > > > > > memory,
> > > > > > > > > > > this can become a bottle neck. We're seeing TTM
> > > > > > > > > > > memory
> > > > > > > > > > > allocation
> > > > > > > > > > > failures not because we're truly out of memory,
> > > > > > > > > > > but
> > > > > > > > > > > because
> > > > > > > > > > > we're
> > > > > > > > > > > out of space in the dma32_zone for the acc_size
> > > > > > > > > > > needed
> > > > > > > > > > > for
> > > > > > > > > > > our BO
> > > > > > > > > > > book-keeping.
> > > > > > > > > > > 
> > > > > > > > > > > Signed-off-by: Felix Kuehling <
> > > > > > > > > > > Felix.Kuehling@amd.com>
> > > > > > > > > > > CC: thellstrom@vmware.com
> > > > > > > > > > > CC: christian.koenig@amd.com
> > > > > > > > > > > ---
> > > > > > > > > > >      drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
> > > > > > > > > > >      1 file changed, 2 insertions(+), 2
> > > > > > > > > > > deletions(-)
> > > > > > > > > > > 
> > > > > > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > > > > > b/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > > > > > index f1567c3..bb05365 100644
> > > > > > > > > > > --- a/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > > > > > +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > > > > > @@ -363,7 +363,7 @@ static int
> > > > > > > > > > > ttm_mem_init_highmem_zone(struct
> > > > > > > > > > > ttm_mem_global *glob,
> > > > > > > > > > >          glob->zones[glob->num_zones++] = zone;
> > > > > > > > > > >          return 0;
> > > > > > > > > > >      }
> > > > > > > > > > > -#else
> > > > > > > > > > > +#elifndef CONFIG_64BIT
> > > > > > > > > > >      static int ttm_mem_init_dma32_zone(struct
> > > > > > > > > > > ttm_mem_global
> > > > > > > > > > > *glob,
> > > > > > > > > > >                         const struct sysinfo *si)
> > > > > > > > > > >      {
> > > > > > > > > > > @@ -441,7 +441,7 @@ int
> > > > > > > > > > > ttm_mem_global_init(struct
> > > > > > > > > > > ttm_mem_global
> > > > > > > > > > > *glob)
> > > > > > > > > > >          ret = ttm_mem_init_highmem_zone(glob,
> > > > > > > > > > > &si);
> > > > > > > > > > >          if (unlikely(ret != 0))
> > > > > > > > > > >              goto out_no_zone;
> > > > > > > > > > > -#else
> > > > > > > > > > > +#elifndef CONFIG_64BIT
> > > > > > > > > > >          ret = ttm_mem_init_dma32_zone(glob,
> > > > > > > > > > > &si);
> > > > > > > > > > >          if (unlikely(ret != 0))
> > > > > > > > > > >              goto out_no_zone;
> > > > > > > > _______________________________________________
> > > > > > > > amd-gfx mailing list
> > > > > > > > amd-gfx@lists.freedesktop.org
> > > > > > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C40edb735811c4c609d5f08d6970e52df%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636862485126215077&amp;sdata=Q4%2Fga%2B8HbwMLRhePkp62uiUMtkpWfp44nMuNumiOskM%3D&amp;reserved=0 
> > > > > > > > 
> > > > _______________________________________________
> > > > amd-gfx mailing list
> > > > amd-gfx@lists.freedesktop.org
> > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C40edb735811c4c609d5f08d6970e52df%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636862485126215077&amp;sdata=Q4%2Fga%2B8HbwMLRhePkp62uiUMtkpWfp44nMuNumiOskM%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Kuehling, Felix]

On 2019-02-20 1:41 a.m., Thomas Hellstrom wrote:
> On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
>> On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
>>> On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
>>>> Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
>>>>> On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
>>>>>> Another good question is also why the heck the acc_size
>>>>>> counts
>>>>>> towards
>>>>>> the DMA32 zone?
>>>>> DMA32 TTM pages are accounted in the DMA32 zone. Other pages
>>>>> are
>>>>> not.
>>>> Yeah, I'm perfectly aware of this. But this is for the accounting
>>>> size!
>>>>
>>>> We have an accounting for the stuff needed additional to the
>>>> pages
>>>> backing the BO (e.g. the page and DMA addr array).
>>>>
>>>> And from the bug description it sounds like we use the DMA32 zone
>>>> for
>>>> this accounting which of course is completely nonsense.
>>> It's actually accounted in all available zones, since it would be
>>> pretty hard to determine exactly where that memory should be
>>> accounted.
>>> In particular if it's vmalloced. It might be DMA32, it might not.
>>> Given
>>> the objective of stopping malicious user-space from exhausting the
>>> DMA32 zone it was, at the time the code was written, a reasonable
>>> approximation. With ever increasing memory sizes, there might be
>>> better
>>> solutions?
>> As far as I can see, in TTM, ttm_mem_global_alloc is only used for
>> the
>> acc_size in ttm_bo_init_reserved. Other than that vmwgfx also seems
>> to
>> use it to account for a few things that are allocated with kmalloc.
>>
>> So would a better solution be to change ttm_mem_global_alloc to use
>> only
>> the kernel zone?
>>
> IMO we need to determine what functionality to keep and then the best
> solution. The current code does its job, but is obviously too
> restrictive. Both of the solutions you suggest open up for potential
> DOS attacks (DMA32 and kernel zones are not mutually exclusive. They
> overlap).
On x86 with HIGHMEM there is no dma32 zone. Why do we need one on 
x86_64? Can we make x86_64 more like HIGHMEM instead?

Regards,
   Felix


>
>
> /Thomas
>
>
>
>
>> Regards,
>>     Felix
>>
>>
>>> /Thomas
>>>
>>>> Christian.
>>>>
>>>>> For small persistent allocations using ttm_mem_global_alloc(),
>>>>> they
>>>>> are
>>>>> accounted also in the DMA32 zone, which may cause over-
>>>>> accounting
>>>>> of
>>>>> that zone, but that's pretty unlikely to be a big problem..
>>>>>
>>>>> /Thomas
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> In other words why should the internal bookkeeping pages be
>>>>>> allocated
>>>>>> in
>>>>>> the DMA32 zone?
>>>>>>
>>>>>> That doesn't sounds valid to me in any way,
>>>>>> Christian.
>>>>>>
>>>>>> Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
>>>>>>> Hmm,
>>>>>>>
>>>>>>> This zone was intended to stop TTM page allocations from
>>>>>>> exhausting
>>>>>>> the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by
>>>>>>> default,
>>>>>>> which
>>>>>>> means if we drop this check, other devices may stop
>>>>>>> functioning
>>>>>>> unexpectedly?
>>>>>>>
>>>>>>> However, in the end I'd expect the kernel page allocation
>>>>>>> system
>>>>>>> to
>>>>>>> make sure there are some pages left in the DMA32 zone,
>>>>>>> otherwise
>>>>>>> random non-IO page allocations would also potentially
>>>>>>> exhaust
>>>>>>> the
>>>>>>> DMA32 zone without anybody caring, which means removing
>>>>>>> this
>>>>>>> zone
>>>>>>> wouldn't be any worse than whatever other subsystems may be
>>>>>>> doing
>>>>>>> already...
>>>>>>>
>>>>>>> /Thomas
>>>>>>>
>>>>>>>
>>>>>>> On 2/16/19 12:02 AM, Kuehling, Felix wrote:
>>>>>>>> This is an RFC. I'm not sure this is the right solution,
>>>>>>>> but
>>>>>>>> it
>>>>>>>> highlights the problem I'm trying to solve.
>>>>>>>>
>>>>>>>> The dma32_zone limits the acc_size of all allocated BOs
>>>>>>>> to
>>>>>>>> 2GB.
>>>>>>>> On a
>>>>>>>> 64-bit system with hundreds of GB of system memory and
>>>>>>>> GPU
>>>>>>>> memory,
>>>>>>>> this can become a bottle neck. We're seeing TTM memory
>>>>>>>> allocation
>>>>>>>> failures not because we're truly out of memory, but
>>>>>>>> because
>>>>>>>> we're
>>>>>>>> out of space in the dma32_zone for the acc_size needed
>>>>>>>> for
>>>>>>>> our BO
>>>>>>>> book-keeping.
>>>>>>>>
>>>>>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>>>>>>>> CC: thellstrom@vmware.com
>>>>>>>> CC: christian.koenig@amd.com
>>>>>>>> ---
>>>>>>>>      drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
>>>>>>>>      1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>> b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>> index f1567c3..bb05365 100644
>>>>>>>> --- a/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
>>>>>>>> @@ -363,7 +363,7 @@ static int
>>>>>>>> ttm_mem_init_highmem_zone(struct
>>>>>>>> ttm_mem_global *glob,
>>>>>>>>          glob->zones[glob->num_zones++] = zone;
>>>>>>>>          return 0;
>>>>>>>>      }
>>>>>>>> -#else
>>>>>>>> +#elifndef CONFIG_64BIT
>>>>>>>>      static int ttm_mem_init_dma32_zone(struct
>>>>>>>> ttm_mem_global
>>>>>>>> *glob,
>>>>>>>>                         const struct sysinfo *si)
>>>>>>>>      {
>>>>>>>> @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct
>>>>>>>> ttm_mem_global
>>>>>>>> *glob)
>>>>>>>>          ret = ttm_mem_init_highmem_zone(glob, &si);
>>>>>>>>          if (unlikely(ret != 0))
>>>>>>>>              goto out_no_zone;
>>>>>>>> -#else
>>>>>>>> +#elifndef CONFIG_64BIT
>>>>>>>>          ret = ttm_mem_init_dma32_zone(glob, &si);
>>>>>>>>          if (unlikely(ret != 0))
>>>>>>>>              goto out_no_zone;
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C1357d06244fb499b31dd08d6968ca04b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636861928079462725&amp;sdata=1Bucho93KMzN0z7QbfiNn%2BNWaZs5yi86Ya6vm9Xhbqo%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

On Wed, 2019-02-20 at 19:23 +0000, Kuehling, Felix wrote:
> On 2019-02-20 1:41 a.m., Thomas Hellstrom wrote:
> > On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
> > > On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
> > > > On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
> > > > > Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
> > > > > > On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
> > > > > > > Another good question is also why the heck the acc_size
> > > > > > > counts
> > > > > > > towards
> > > > > > > the DMA32 zone?
> > > > > > DMA32 TTM pages are accounted in the DMA32 zone. Other
> > > > > > pages
> > > > > > are
> > > > > > not.
> > > > > Yeah, I'm perfectly aware of this. But this is for the
> > > > > accounting
> > > > > size!
> > > > > 
> > > > > We have an accounting for the stuff needed additional to the
> > > > > pages
> > > > > backing the BO (e.g. the page and DMA addr array).
> > > > > 
> > > > > And from the bug description it sounds like we use the DMA32
> > > > > zone
> > > > > for
> > > > > this accounting which of course is completely nonsense.
> > > > It's actually accounted in all available zones, since it would
> > > > be
> > > > pretty hard to determine exactly where that memory should be
> > > > accounted.
> > > > In particular if it's vmalloced. It might be DMA32, it might
> > > > not.
> > > > Given
> > > > the objective of stopping malicious user-space from exhausting
> > > > the
> > > > DMA32 zone it was, at the time the code was written, a
> > > > reasonable
> > > > approximation. With ever increasing memory sizes, there might
> > > > be
> > > > better
> > > > solutions?
> > > As far as I can see, in TTM, ttm_mem_global_alloc is only used
> > > for
> > > the
> > > acc_size in ttm_bo_init_reserved. Other than that vmwgfx also
> > > seems
> > > to
> > > use it to account for a few things that are allocated with
> > > kmalloc.
> > > 
> > > So would a better solution be to change ttm_mem_global_alloc to
> > > use
> > > only
> > > the kernel zone?
> > > 
> > IMO we need to determine what functionality to keep and then the
> > best
> > solution. The current code does its job, but is obviously too
> > restrictive. Both of the solutions you suggest open up for
> > potential
> > DOS attacks (DMA32 and kernel zones are not mutually exclusive.
> > They
> > overlap).
> On x86 with HIGHMEM there is no dma32 zone. Why do we need one on 
> x86_64? Can we make x86_64 more like HIGHMEM instead?
> 
> Regards,
>    Felix
> 

IIRC with x86, the kernel zone is always smaller than any dma32 zone,
so we'd always exhaust the kernel zone before dma32 anyway.

Not sure why we have dma32 on x86 without highmem, though. sounds
superflous but harmless.

/Thomas



> 
> > 
> > /Thomas
> > 
> > 
> > 
> > 
> > > Regards,
> > >     Felix
> > > 
> > > 
> > > > /Thomas
> > > > 
> > > > > Christian.
> > > > > 
> > > > > > For small persistent allocations using
> > > > > > ttm_mem_global_alloc(),
> > > > > > they
> > > > > > are
> > > > > > accounted also in the DMA32 zone, which may cause over-
> > > > > > accounting
> > > > > > of
> > > > > > that zone, but that's pretty unlikely to be a big problem..
> > > > > > 
> > > > > > /Thomas
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > In other words why should the internal bookkeeping pages
> > > > > > > be
> > > > > > > allocated
> > > > > > > in
> > > > > > > the DMA32 zone?
> > > > > > > 
> > > > > > > That doesn't sounds valid to me in any way,
> > > > > > > Christian.
> > > > > > > 
> > > > > > > Am 18.02.19 um 09:02 schrieb Thomas Hellstrom:
> > > > > > > > Hmm,
> > > > > > > > 
> > > > > > > > This zone was intended to stop TTM page allocations
> > > > > > > > from
> > > > > > > > exhausting
> > > > > > > > the DMA32 zone. IIRC dma_alloc_coherent() uses DMA32 by
> > > > > > > > default,
> > > > > > > > which
> > > > > > > > means if we drop this check, other devices may stop
> > > > > > > > functioning
> > > > > > > > unexpectedly?
> > > > > > > > 
> > > > > > > > However, in the end I'd expect the kernel page
> > > > > > > > allocation
> > > > > > > > system
> > > > > > > > to
> > > > > > > > make sure there are some pages left in the DMA32 zone,
> > > > > > > > otherwise
> > > > > > > > random non-IO page allocations would also potentially
> > > > > > > > exhaust
> > > > > > > > the
> > > > > > > > DMA32 zone without anybody caring, which means removing
> > > > > > > > this
> > > > > > > > zone
> > > > > > > > wouldn't be any worse than whatever other subsystems
> > > > > > > > may be
> > > > > > > > doing
> > > > > > > > already...
> > > > > > > > 
> > > > > > > > /Thomas
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On 2/16/19 12:02 AM, Kuehling, Felix wrote:
> > > > > > > > > This is an RFC. I'm not sure this is the right
> > > > > > > > > solution,
> > > > > > > > > but
> > > > > > > > > it
> > > > > > > > > highlights the problem I'm trying to solve.
> > > > > > > > > 
> > > > > > > > > The dma32_zone limits the acc_size of all allocated
> > > > > > > > > BOs
> > > > > > > > > to
> > > > > > > > > 2GB.
> > > > > > > > > On a
> > > > > > > > > 64-bit system with hundreds of GB of system memory
> > > > > > > > > and
> > > > > > > > > GPU
> > > > > > > > > memory,
> > > > > > > > > this can become a bottle neck. We're seeing TTM
> > > > > > > > > memory
> > > > > > > > > allocation
> > > > > > > > > failures not because we're truly out of memory, but
> > > > > > > > > because
> > > > > > > > > we're
> > > > > > > > > out of space in the dma32_zone for the acc_size
> > > > > > > > > needed
> > > > > > > > > for
> > > > > > > > > our BO
> > > > > > > > > book-keeping.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com
> > > > > > > > > >
> > > > > > > > > CC: thellstrom@vmware.com
> > > > > > > > > CC: christian.koenig@amd.com
> > > > > > > > > ---
> > > > > > > > >      drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
> > > > > > > > >      1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > > > b/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > > > index f1567c3..bb05365 100644
> > > > > > > > > --- a/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > > > +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> > > > > > > > > @@ -363,7 +363,7 @@ static int
> > > > > > > > > ttm_mem_init_highmem_zone(struct
> > > > > > > > > ttm_mem_global *glob,
> > > > > > > > >          glob->zones[glob->num_zones++] = zone;
> > > > > > > > >          return 0;
> > > > > > > > >      }
> > > > > > > > > -#else
> > > > > > > > > +#elifndef CONFIG_64BIT
> > > > > > > > >      static int ttm_mem_init_dma32_zone(struct
> > > > > > > > > ttm_mem_global
> > > > > > > > > *glob,
> > > > > > > > >                         const struct sysinfo *si)
> > > > > > > > >      {
> > > > > > > > > @@ -441,7 +441,7 @@ int ttm_mem_global_init(struct
> > > > > > > > > ttm_mem_global
> > > > > > > > > *glob)
> > > > > > > > >          ret = ttm_mem_init_highmem_zone(glob, &si);
> > > > > > > > >          if (unlikely(ret != 0))
> > > > > > > > >              goto out_no_zone;
> > > > > > > > > -#else
> > > > > > > > > +#elifndef CONFIG_64BIT
> > > > > > > > >          ret = ttm_mem_init_dma32_zone(glob, &si);
> > > > > > > > >          if (unlikely(ret != 0))
> > > > > > > > >              goto out_no_zone;
> > > > > > _______________________________________________
> > > > > > amd-gfx mailing list
> > > > > > amd-gfx@lists.freedesktop.org
> > > > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cthellstrom%40vmware.com%7C5521905ed2f94144c64208d69768eda0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636862874256927858&amp;sdata=2PqTuwA75OoJTDBZ%2BtXnuEfrI3Lham5uYTWq2rT%2BN24%3D&amp;reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Koenig, Christian]

Am 21.02.19 um 07:47 schrieb Thomas Hellstrom:
> On Wed, 2019-02-20 at 19:23 +0000, Kuehling, Felix wrote:
>> On 2019-02-20 1:41 a.m., Thomas Hellstrom wrote:
>>> On Tue, 2019-02-19 at 17:06 +0000, Kuehling, Felix wrote:
>>>> On 2019-02-18 3:39 p.m., Thomas Hellstrom wrote:
>>>>> On Mon, 2019-02-18 at 18:07 +0100, Christian König wrote:
>>>>>> Am 18.02.19 um 10:47 schrieb Thomas Hellstrom:
>>>>>>> On Mon, 2019-02-18 at 09:20 +0000, Koenig, Christian wrote:
>>>>>>>> Another good question is also why the heck the acc_size
>>>>>>>> counts
>>>>>>>> towards
>>>>>>>> the DMA32 zone?
>>>>>>> DMA32 TTM pages are accounted in the DMA32 zone. Other
>>>>>>> pages
>>>>>>> are
>>>>>>> not.
>>>>>> Yeah, I'm perfectly aware of this. But this is for the
>>>>>> accounting
>>>>>> size!
>>>>>>
>>>>>> We have an accounting for the stuff needed additional to the
>>>>>> pages
>>>>>> backing the BO (e.g. the page and DMA addr array).
>>>>>>
>>>>>> And from the bug description it sounds like we use the DMA32
>>>>>> zone
>>>>>> for
>>>>>> this accounting which of course is completely nonsense.
>>>>> It's actually accounted in all available zones, since it would
>>>>> be
>>>>> pretty hard to determine exactly where that memory should be
>>>>> accounted.
>>>>> In particular if it's vmalloced. It might be DMA32, it might
>>>>> not.
>>>>> Given
>>>>> the objective of stopping malicious user-space from exhausting
>>>>> the
>>>>> DMA32 zone it was, at the time the code was written, a
>>>>> reasonable
>>>>> approximation. With ever increasing memory sizes, there might
>>>>> be
>>>>> better
>>>>> solutions?
>>>> As far as I can see, in TTM, ttm_mem_global_alloc is only used
>>>> for
>>>> the
>>>> acc_size in ttm_bo_init_reserved. Other than that vmwgfx also
>>>> seems
>>>> to
>>>> use it to account for a few things that are allocated with
>>>> kmalloc.
>>>>
>>>> So would a better solution be to change ttm_mem_global_alloc to
>>>> use
>>>> only
>>>> the kernel zone?
>>>>
>>> IMO we need to determine what functionality to keep and then the
>>> best
>>> solution. The current code does its job, but is obviously too
>>> restrictive. Both of the solutions you suggest open up for
>>> potential
>>> DOS attacks (DMA32 and kernel zones are not mutually exclusive.
>>> They
>>> overlap).
>> On x86 with HIGHMEM there is no dma32 zone. Why do we need one on
>> x86_64? Can we make x86_64 more like HIGHMEM instead?
>>
>> Regards,
>>     Felix
>>
> IIRC with x86, the kernel zone is always smaller than any dma32 zone,
> so we'd always exhaust the kernel zone before dma32 anyway.
>
> Not sure why we have dma32 on x86 without highmem, though. sounds
> superflous but harmless.

Well DMA32 denotes memory which is accessible by devices who can only do 
32bit addressing. And IIRC we can actually do DMA32 to highmem since 
something like 2.4.*.

Because of this it is actually irrelevant if you have highmem or not, 
what matters for DMA32 is if you have an IOMMU or not.

So even on x86_64 you actually do need the DMA32 zone if you don't have 
an IOMMU which remaps all memory for devices which can't directly 
address it.

Regards,
Christian.

>
> /Thomas
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Kuehling, Felix]

On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
> On x86 with HIGHMEM there is no dma32 zone. Why do we need one on
>>> x86_64? Can we make x86_64 more like HIGHMEM instead?
>>>
>>> Regards,
>>>      Felix
>>>
>> IIRC with x86, the kernel zone is always smaller than any dma32 zone,
>> so we'd always exhaust the kernel zone before dma32 anyway.
>>
>> Not sure why we have dma32 on x86 without highmem, though. sounds
>> superflous but harmless.
> Well DMA32 denotes memory which is accessible by devices who can only do
> 32bit addressing. And IIRC we can actually do DMA32 to highmem since
> something like 2.4.*.
>
> Because of this it is actually irrelevant if you have highmem or not,
> what matters for DMA32 is if you have an IOMMU or not.

Are you saying we should have a dma32_zone even on x86 with HIGHMEM?


>
> So even on x86_64 you actually do need the DMA32 zone if you don't have
> an IOMMU which remaps all memory for devices which can't directly
> address it.

Why is DMA32 special in this way? For example AMD GFX8 GPUs support 
40-bit DMA. But we don't have a special zone for that.

How common is it to have devices that need DMA32 on an x86_64 system?

Regards,
   Felix


>
> Regards,
> Christian.
>
>> /Thomas
>>
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
> On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
> > On x86 with HIGHMEM there is no dma32 zone. Why do we need one on
> > > > x86_64? Can we make x86_64 more like HIGHMEM instead?
> > > > 
> > > > Regards,
> > > >      Felix
> > > > 
> > > IIRC with x86, the kernel zone is always smaller than any dma32
> > > zone,
> > > so we'd always exhaust the kernel zone before dma32 anyway.
> > > 
> > > Not sure why we have dma32 on x86 without highmem, though. sounds
> > > superflous but harmless.
> > Well DMA32 denotes memory which is accessible by devices who can
> > only do
> > 32bit addressing. And IIRC we can actually do DMA32 to highmem
> > since
> > something like 2.4.*.
> > 
> > Because of this it is actually irrelevant if you have highmem or
> > not,
> > what matters for DMA32 is if you have an IOMMU or not.
> 
> Are you saying we should have a dma32_zone even on x86 with HIGHMEM?
> 
> 
> > So even on x86_64 you actually do need the DMA32 zone if you don't
> > have
> > an IOMMU which remaps all memory for devices which can't directly
> > address it.
> 
> Why is DMA32 special in this way? For example AMD GFX8 GPUs support 
> 40-bit DMA. But we don't have a special zone for that.

If you're running on a non-IOMMU system with physical memory addresses
> 40 bits, and tell the DMA subsystem that you need to restrict to 40
bits, it will probably start using bounce buffers for streaming DMA
(which won't work with most graphics drivers), or for
dma_alloc_coherent(), it will probably use memory from the DMA32 zone.

> 
> How common is it to have devices that need DMA32 on an x86_64 system?

IIRC All devices using dma_alloc_coherent() allocate DMA32 memory
unless they explicitly set the dma coherent mask to something larger.
Like Christian says, if an IOMMU is present and enabled, the need for
the DMA32 zone goes away. In theory at least.

/Thomas


> 
> Regards,
>    Felix
> 
> 
> > Regards,
> > Christian.
> > 
> > > /Thomas
> > > 
> > > 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Kuehling, Felix]

On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
> On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
>> On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
>>> On x86 with HIGHMEM there is no dma32 zone. Why do we need one on
>>>>> x86_64? Can we make x86_64 more like HIGHMEM instead?
>>>>>
>>>>> Regards,
>>>>>       Felix
>>>>>
>>>> IIRC with x86, the kernel zone is always smaller than any dma32
>>>> zone,
>>>> so we'd always exhaust the kernel zone before dma32 anyway.
>>>>
>>>> Not sure why we have dma32 on x86 without highmem, though. sounds
>>>> superflous but harmless.
>>> Well DMA32 denotes memory which is accessible by devices who can
>>> only do
>>> 32bit addressing. And IIRC we can actually do DMA32 to highmem
>>> since
>>> something like 2.4.*.
>>>
>>> Because of this it is actually irrelevant if you have highmem or
>>> not,
>>> what matters for DMA32 is if you have an IOMMU or not.
>> Are you saying we should have a dma32_zone even on x86 with HIGHMEM?
>>
>>
>>> So even on x86_64 you actually do need the DMA32 zone if you don't
>>> have
>>> an IOMMU which remaps all memory for devices which can't directly
>>> address it.
>> Why is DMA32 special in this way? For example AMD GFX8 GPUs support
>> 40-bit DMA. But we don't have a special zone for that.
> If you're running on a non-IOMMU system with physical memory addresses
>> 40 bits, and tell the DMA subsystem that you need to restrict to 40
> bits, it will probably start using bounce buffers for streaming DMA
> (which won't work with most graphics drivers), or for
> dma_alloc_coherent(), it will probably use memory from the DMA32 zone.

OK, then why is it not needed when CONFIG_HIGHMEM is defined?

I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we should use 
that to decide whether to account for the DMA32 zone in TTM. It is set 
on x86_64 and a number of other 64-bit architectures.


>> How common is it to have devices that need DMA32 on an x86_64 system?
> IIRC All devices using dma_alloc_coherent() allocate DMA32 memory
> unless they explicitly set the dma coherent mask to something larger.
> Like Christian says, if an IOMMU is present and enabled, the need for
> the DMA32 zone goes away. In theory at least.

Thanks. I read up a bit on DMA32 and memory zones in general. I found 
that there is a lowmem_reserve_ratio feature that protects against 
normal page allocations overflowing into lowmem zones. There is some 
documentation in Documentation/scsctl/vm.txt (search for 
lowmem_reserve_ratio). The protected amount of memory in each zone can 
be seen in /proc/zoneinfo.

With that, can we conclude that we don't need to count 
ttm_mem_global_alloc against the dma32 zone.

Thanks,
   Felix


>
> /Thomas
>
>
>> Regards,
>>     Felix
>>
>>
>>> Regards,
>>> Christian.
>>>
>>>> /Thomas
>>>>
>>>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

Hi,

On Thu, 2019-02-21 at 20:24 +0000, Kuehling, Felix wrote:
> On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
> > On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
> > > On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
> > > > On x86 with HIGHMEM there is no dma32 zone. Why do we need one
> > > > on
> > > > > > x86_64? Can we make x86_64 more like HIGHMEM instead?
> > > > > > 
> > > > > > Regards,
> > > > > >       Felix
> > > > > > 
> > > > > IIRC with x86, the kernel zone is always smaller than any
> > > > > dma32
> > > > > zone,
> > > > > so we'd always exhaust the kernel zone before dma32 anyway.
> > > > > 
> > > > > Not sure why we have dma32 on x86 without highmem, though.
> > > > > sounds
> > > > > superflous but harmless.
> > > > Well DMA32 denotes memory which is accessible by devices who
> > > > can
> > > > only do
> > > > 32bit addressing. And IIRC we can actually do DMA32 to highmem
> > > > since
> > > > something like 2.4.*.
> > > > 
> > > > Because of this it is actually irrelevant if you have highmem
> > > > or
> > > > not,
> > > > what matters for DMA32 is if you have an IOMMU or not.
> > > Are you saying we should have a dma32_zone even on x86 with
> > > HIGHMEM?
> > > 
> > > 
> > > > So even on x86_64 you actually do need the DMA32 zone if you
> > > > don't
> > > > have
> > > > an IOMMU which remaps all memory for devices which can't
> > > > directly
> > > > address it.
> > > Why is DMA32 special in this way? For example AMD GFX8 GPUs
> > > support
> > > 40-bit DMA. But we don't have a special zone for that.
> > If you're running on a non-IOMMU system with physical memory
> > addresses
> > > 40 bits, and tell the DMA subsystem that you need to restrict to
> > > 40
> > bits, it will probably start using bounce buffers for streaming DMA
> > (which won't work with most graphics drivers), or for
> > dma_alloc_coherent(), it will probably use memory from the DMA32
> > zone.
> 
> OK, then why is it not needed when CONFIG_HIGHMEM is defined?
> 
> I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we should
> use 
> that to decide whether to account for the DMA32 zone in TTM. It is
> set 
> on x86_64 and a number of other 64-bit architectures.
> 
> 
> > > How common is it to have devices that need DMA32 on an x86_64
> > > system?
> > IIRC All devices using dma_alloc_coherent() allocate DMA32 memory
> > unless they explicitly set the dma coherent mask to something
> > larger.
> > Like Christian says, if an IOMMU is present and enabled, the need
> > for
> > the DMA32 zone goes away. In theory at least.
> 
> Thanks. I read up a bit on DMA32 and memory zones in general. I
> found 
> that there is a lowmem_reserve_ratio feature that protects against 
> normal page allocations overflowing into lowmem zones. There is some 
> documentation in Documentation/scsctl/vm.txt (search for 
> lowmem_reserve_ratio). The protected amount of memory in each zone
> can 
> be seen in /proc/zoneinfo.
> 
> With that, can we conclude that we don't need to count 
> ttm_mem_global_alloc against the dma32 zone.

Yes, it indeed looks like that.
But then I would suggest removing the DMA32 zone entirely.

/Thomas


> 
> Thanks,
>    Felix
> 
> 
> > /Thomas
> > 
> > 
> > > Regards,
> > >     Felix
> > > 
> > > 
> > > > Regards,
> > > > Christian.
> > > > 
> > > > > /Thomas
> > > > > 
> > > > > 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Koenig, Christian]

Am 21.02.19 um 22:02 schrieb Thomas Hellstrom:
> Hi,
>
> On Thu, 2019-02-21 at 20:24 +0000, Kuehling, Felix wrote:
>> On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
>>> On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
>>>> On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
>>>>> On x86 with HIGHMEM there is no dma32 zone. Why do we need one
>>>>> on
>>>>>>> x86_64? Can we make x86_64 more like HIGHMEM instead?
>>>>>>>
>>>>>>> Regards,
>>>>>>>        Felix
>>>>>>>
>>>>>> IIRC with x86, the kernel zone is always smaller than any
>>>>>> dma32
>>>>>> zone,
>>>>>> so we'd always exhaust the kernel zone before dma32 anyway.
>>>>>>
>>>>>> Not sure why we have dma32 on x86 without highmem, though.
>>>>>> sounds
>>>>>> superflous but harmless.
>>>>> Well DMA32 denotes memory which is accessible by devices who
>>>>> can
>>>>> only do
>>>>> 32bit addressing. And IIRC we can actually do DMA32 to highmem
>>>>> since
>>>>> something like 2.4.*.
>>>>>
>>>>> Because of this it is actually irrelevant if you have highmem
>>>>> or
>>>>> not,
>>>>> what matters for DMA32 is if you have an IOMMU or not.
>>>> Are you saying we should have a dma32_zone even on x86 with
>>>> HIGHMEM?
>>>>
>>>>
>>>>> So even on x86_64 you actually do need the DMA32 zone if you
>>>>> don't
>>>>> have
>>>>> an IOMMU which remaps all memory for devices which can't
>>>>> directly
>>>>> address it.
>>>> Why is DMA32 special in this way? For example AMD GFX8 GPUs
>>>> support
>>>> 40-bit DMA. But we don't have a special zone for that.
>>> If you're running on a non-IOMMU system with physical memory
>>> addresses
>>>> 40 bits, and tell the DMA subsystem that you need to restrict to
>>>> 40
>>> bits, it will probably start using bounce buffers for streaming DMA
>>> (which won't work with most graphics drivers), or for
>>> dma_alloc_coherent(), it will probably use memory from the DMA32
>>> zone.
>> OK, then why is it not needed when CONFIG_HIGHMEM is defined?
>>
>> I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we should
>> use
>> that to decide whether to account for the DMA32 zone in TTM. It is
>> set
>> on x86_64 and a number of other 64-bit architectures.
>>
>>
>>>> How common is it to have devices that need DMA32 on an x86_64
>>>> system?
>>> IIRC All devices using dma_alloc_coherent() allocate DMA32 memory
>>> unless they explicitly set the dma coherent mask to something
>>> larger.
>>> Like Christian says, if an IOMMU is present and enabled, the need
>>> for
>>> the DMA32 zone goes away. In theory at least.
>> Thanks. I read up a bit on DMA32 and memory zones in general. I
>> found
>> that there is a lowmem_reserve_ratio feature that protects against
>> normal page allocations overflowing into lowmem zones. There is some
>> documentation in Documentation/scsctl/vm.txt (search for
>> lowmem_reserve_ratio). The protected amount of memory in each zone
>> can
>> be seen in /proc/zoneinfo.
>>
>> With that, can we conclude that we don't need to count
>> ttm_mem_global_alloc against the dma32 zone.
> Yes, it indeed looks like that.
> But then I would suggest removing the DMA32 zone entirely.

We still need it for the pages we allocate, but we should just stop 
accounting all the housekeeping to it.

To answer Felix earlier question we really still need DMA32 without 
IOMMU even on a 64bit system. The problem is simply that you have tons 
of PCIe hardware which can do only 32bit addressing, for example the 
audio codecs even on our newest Vega chips.

Regards,
Christian.

>
> /Thomas
>
>
>> Thanks,
>>     Felix
>>
>>
>>> /Thomas
>>>
>>>
>>>> Regards,
>>>>      Felix
>>>>
>>>>
>>>>> Regards,
>>>>> Christian.
>>>>>
>>>>>> /Thomas
>>>>>>
>>>>>>

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

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Thomas Hellstrom]

On Fri, 2019-02-22 at 07:10 +0000, Koenig, Christian wrote:
> Am 21.02.19 um 22:02 schrieb Thomas Hellstrom:
> > Hi,
> > 
> > On Thu, 2019-02-21 at 20:24 +0000, Kuehling, Felix wrote:
> > > On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
> > > > On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
> > > > > On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
> > > > > > On x86 with HIGHMEM there is no dma32 zone. Why do we need
> > > > > > one
> > > > > > on
> > > > > > > > x86_64? Can we make x86_64 more like HIGHMEM instead?
> > > > > > > > 
> > > > > > > > Regards,
> > > > > > > >        Felix
> > > > > > > > 
> > > > > > > IIRC with x86, the kernel zone is always smaller than any
> > > > > > > dma32
> > > > > > > zone,
> > > > > > > so we'd always exhaust the kernel zone before dma32
> > > > > > > anyway.
> > > > > > > 
> > > > > > > Not sure why we have dma32 on x86 without highmem,
> > > > > > > though.
> > > > > > > sounds
> > > > > > > superflous but harmless.
> > > > > > Well DMA32 denotes memory which is accessible by devices
> > > > > > who
> > > > > > can
> > > > > > only do
> > > > > > 32bit addressing. And IIRC we can actually do DMA32 to
> > > > > > highmem
> > > > > > since
> > > > > > something like 2.4.*.
> > > > > > 
> > > > > > Because of this it is actually irrelevant if you have
> > > > > > highmem
> > > > > > or
> > > > > > not,
> > > > > > what matters for DMA32 is if you have an IOMMU or not.
> > > > > Are you saying we should have a dma32_zone even on x86 with
> > > > > HIGHMEM?
> > > > > 
> > > > > 
> > > > > > So even on x86_64 you actually do need the DMA32 zone if
> > > > > > you
> > > > > > don't
> > > > > > have
> > > > > > an IOMMU which remaps all memory for devices which can't
> > > > > > directly
> > > > > > address it.
> > > > > Why is DMA32 special in this way? For example AMD GFX8 GPUs
> > > > > support
> > > > > 40-bit DMA. But we don't have a special zone for that.
> > > > If you're running on a non-IOMMU system with physical memory
> > > > addresses
> > > > > 40 bits, and tell the DMA subsystem that you need to restrict
> > > > > to
> > > > > 40
> > > > bits, it will probably start using bounce buffers for streaming
> > > > DMA
> > > > (which won't work with most graphics drivers), or for
> > > > dma_alloc_coherent(), it will probably use memory from the
> > > > DMA32
> > > > zone.
> > > OK, then why is it not needed when CONFIG_HIGHMEM is defined?
> > > 
> > > I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we
> > > should
> > > use
> > > that to decide whether to account for the DMA32 zone in TTM. It
> > > is
> > > set
> > > on x86_64 and a number of other 64-bit architectures.
> > > 
> > > 
> > > > > How common is it to have devices that need DMA32 on an x86_64
> > > > > system?
> > > > IIRC All devices using dma_alloc_coherent() allocate DMA32
> > > > memory
> > > > unless they explicitly set the dma coherent mask to something
> > > > larger.
> > > > Like Christian says, if an IOMMU is present and enabled, the
> > > > need
> > > > for
> > > > the DMA32 zone goes away. In theory at least.
> > > Thanks. I read up a bit on DMA32 and memory zones in general. I
> > > found
> > > that there is a lowmem_reserve_ratio feature that protects
> > > against
> > > normal page allocations overflowing into lowmem zones. There is
> > > some
> > > documentation in Documentation/scsctl/vm.txt (search for
> > > lowmem_reserve_ratio). The protected amount of memory in each
> > > zone
> > > can
> > > be seen in /proc/zoneinfo.
> > > 
> > > With that, can we conclude that we don't need to count
> > > ttm_mem_global_alloc against the dma32 zone.
> > Yes, it indeed looks like that.
> > But then I would suggest removing the DMA32 zone entirely.
> 
> We still need it for the pages we allocate, but we should just stop 
> accounting all the housekeeping to it.

Why is that? Can't we just account all pages in the kernel zone, and
leave it up to the kernel to make sure there are still DMA32 pages
left?

/Thomas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] [RFC] drm/ttm: Don't init dma32_zone on 64-bit systems

[Kuehling, Felix]

On 2019-02-22 8:45 a.m., Thomas Hellstrom wrote:
> On Fri, 2019-02-22 at 07:10 +0000, Koenig, Christian wrote:
>> Am 21.02.19 um 22:02 schrieb Thomas Hellstrom:
>>> Hi,
>>>
>>> On Thu, 2019-02-21 at 20:24 +0000, Kuehling, Felix wrote:
>>>> On 2019-02-21 12:34 p.m., Thomas Hellstrom wrote:
>>>>> On Thu, 2019-02-21 at 16:57 +0000, Kuehling, Felix wrote:
>>>>>> On 2019-02-21 2:59 a.m., Koenig, Christian wrote:
>>>>>>> On x86 with HIGHMEM there is no dma32 zone. Why do we need
>>>>>>> one
>>>>>>> on
>>>>>>>>> x86_64? Can we make x86_64 more like HIGHMEM instead?
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>>         Felix
>>>>>>>>>
>>>>>>>> IIRC with x86, the kernel zone is always smaller than any
>>>>>>>> dma32
>>>>>>>> zone,
>>>>>>>> so we'd always exhaust the kernel zone before dma32
>>>>>>>> anyway.
>>>>>>>>
>>>>>>>> Not sure why we have dma32 on x86 without highmem,
>>>>>>>> though.
>>>>>>>> sounds
>>>>>>>> superflous but harmless.
>>>>>>> Well DMA32 denotes memory which is accessible by devices
>>>>>>> who
>>>>>>> can
>>>>>>> only do
>>>>>>> 32bit addressing. And IIRC we can actually do DMA32 to
>>>>>>> highmem
>>>>>>> since
>>>>>>> something like 2.4.*.
>>>>>>>
>>>>>>> Because of this it is actually irrelevant if you have
>>>>>>> highmem
>>>>>>> or
>>>>>>> not,
>>>>>>> what matters for DMA32 is if you have an IOMMU or not.
>>>>>> Are you saying we should have a dma32_zone even on x86 with
>>>>>> HIGHMEM?
>>>>>>
>>>>>>
>>>>>>> So even on x86_64 you actually do need the DMA32 zone if
>>>>>>> you
>>>>>>> don't
>>>>>>> have
>>>>>>> an IOMMU which remaps all memory for devices which can't
>>>>>>> directly
>>>>>>> address it.
>>>>>> Why is DMA32 special in this way? For example AMD GFX8 GPUs
>>>>>> support
>>>>>> 40-bit DMA. But we don't have a special zone for that.
>>>>> If you're running on a non-IOMMU system with physical memory
>>>>> addresses
>>>>>> 40 bits, and tell the DMA subsystem that you need to restrict
>>>>>> to
>>>>>> 40
>>>>> bits, it will probably start using bounce buffers for streaming
>>>>> DMA
>>>>> (which won't work with most graphics drivers), or for
>>>>> dma_alloc_coherent(), it will probably use memory from the
>>>>> DMA32
>>>>> zone.
>>>> OK, then why is it not needed when CONFIG_HIGHMEM is defined?
>>>>
>>>> I found that there is a CONFIG_ZONE_DMA32 parameter. Maybe we
>>>> should
>>>> use
>>>> that to decide whether to account for the DMA32 zone in TTM. It
>>>> is
>>>> set
>>>> on x86_64 and a number of other 64-bit architectures.
>>>>
>>>>
>>>>>> How common is it to have devices that need DMA32 on an x86_64
>>>>>> system?
>>>>> IIRC All devices using dma_alloc_coherent() allocate DMA32
>>>>> memory
>>>>> unless they explicitly set the dma coherent mask to something
>>>>> larger.
>>>>> Like Christian says, if an IOMMU is present and enabled, the
>>>>> need
>>>>> for
>>>>> the DMA32 zone goes away. In theory at least.
>>>> Thanks. I read up a bit on DMA32 and memory zones in general. I
>>>> found
>>>> that there is a lowmem_reserve_ratio feature that protects
>>>> against
>>>> normal page allocations overflowing into lowmem zones. There is
>>>> some
>>>> documentation in Documentation/scsctl/vm.txt (search for
>>>> lowmem_reserve_ratio). The protected amount of memory in each
>>>> zone
>>>> can
>>>> be seen in /proc/zoneinfo.
>>>>
>>>> With that, can we conclude that we don't need to count
>>>> ttm_mem_global_alloc against the dma32 zone.
>>> Yes, it indeed looks like that.
>>> But then I would suggest removing the DMA32 zone entirely.
>> We still need it for the pages we allocate, but we should just stop
>> accounting all the housekeeping to it.
> Why is that? Can't we just account all pages in the kernel zone, and
> leave it up to the kernel to make sure there are still DMA32 pages
> left?

ttm_page_alloc and ttm_page_alloc_dma support allocating from DMA32 
explicitly (setting GFP_DMA32). Such allocations could exhaust DMA32 
memory, which TTM should prevent by limiting its DMA32 usage. This would 
still be counted against the dma32 zone by ttm_mem_global_alloc_page.

I'll send out a new patch that counts general kernel allocations against 
the kernel zone only. I hope this would be acceptable.

Regards,
   Felix


>
> /Thomas
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx