[PATCH] drm/radeon/kms: forbid big bo allocation (fdo 31708) v2

[Thomas Hellstrom]

On 11/22/2010 06:31 PM, Thomas Hellstrom wrote:
> On 11/22/2010 06:07 PM, Jerome Glisse wrote:
>> 2010/11/22 Michel Dänzer<michel at daenzer.net>:
>>> On Fre, 2010-11-19 at 16:34 -0500, jglisse at redhat.com wrote:
>>>> From: Jerome Glisse<jglisse at redhat.com>
>>>>
>>>> Forbid allocating buffer bigger than visible VRAM or GTT, also
>>>> properly set lpfn field.
>>>>
>>>> v2 - use max macro
>>>>     - silence warning
>>>>
>>>> Signed-off-by: Jerome Glisse<jglisse at redhat.com>
>>>> ---
>>>>   drivers/gpu/drm/radeon/radeon_object.c |   34 
>>>> ++++++++++++++++++++++++++-----
>>>>   1 files changed, 28 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
>>>> b/drivers/gpu/drm/radeon/radeon_object.c
>>>> index 1d06774..c2fa64c 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_object.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_object.c
>>>> @@ -69,18 +69,28 @@ void radeon_ttm_placement_from_domain(struct 
>>>> radeon_bo *rbo, u32 domain)
>>>>        u32 c = 0;
>>>>
>>>>        rbo->placement.fpfn = 0;
>>>> -     rbo->placement.lpfn = rbo->rdev->mc.active_vram_size>>  
>>>> PAGE_SHIFT;
>>>> +     rbo->placement.lpfn = 0;
>>>>        rbo->placement.placement = rbo->placements;
>>>>        rbo->placement.busy_placement = rbo->placements;
>>>> -     if (domain&  RADEON_GEM_DOMAIN_VRAM)
>>>> +     if (domain&  RADEON_GEM_DOMAIN_VRAM) {
>>>> +             rbo->placement.lpfn = 
>>>> max((unsigned)rbo->placement.lpfn, 
>>>> (unsigned)rbo->rdev->mc.active_vram_size>>  PAGE_SHIFT);
>>>>                rbo->placements[c++] = TTM_PL_FLAG_WC | 
>>>> TTM_PL_FLAG_UNCACHED |
>>>>                                        TTM_PL_FLAG_VRAM;
>>>> -     if (domain&  RADEON_GEM_DOMAIN_GTT)
>>>> +     }
>>>> +     if (domain&  RADEON_GEM_DOMAIN_GTT) {
>>>> +             rbo->placement.lpfn = 
>>>> max((unsigned)rbo->placement.lpfn, 
>>>> (unsigned)rbo->rdev->mc.gtt_size>>  PAGE_SHIFT);
>>>>                rbo->placements[c++] = TTM_PL_MASK_CACHING | 
>>>> TTM_PL_FLAG_TT;
>>>> -     if (domain&  RADEON_GEM_DOMAIN_CPU)
>>>> +     }
>>>> +     if (domain&  RADEON_GEM_DOMAIN_CPU) {
>>>> +             /* 4G limit for CPU domain */
>>>> +             rbo->placement.lpfn = max(rbo->placement.lpfn, 
>>>> 0xFFFFFFFF>>  PAGE_SHIFT);
>>>>                rbo->placements[c++] = TTM_PL_MASK_CACHING | 
>>>> TTM_PL_FLAG_SYSTEM;
>>>> -     if (!c)
>>>> +     }
>>>> +     if (!c) {
>>>> +             /* 4G limit for CPU domain */
>>>> +             rbo->placement.lpfn = max(rbo->placement.lpfn, 
>>>> 0xFFFFFFFF>>  PAGE_SHIFT);
>>>>                rbo->placements[c++] = TTM_PL_MASK_CACHING | 
>>>> TTM_PL_FLAG_SYSTEM;
>>>> +     }
>>> I don't think taxing the maximum is the right thing to do: If domain is
>>> (RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT) and VRAM doesn't 
>>> happen
>>> to be the same size as GTT, lpfn will end up larger than one of them.
>>>
>>> AFAICT radeon_ttm_placement_from_domain() should just set lpfn to 0
>>> (i.e. unrestricted), the callers that need it to be non-0 already 
>>> set it
>>> afterwards.
>>>
>>> Out of curiosity, where does the 4G limit come from?
>>>
>>>
>> > From my hat, but iirc ttm limit things to 1G anyway (vm size for
>> mapping object in drm file and iirc we will report error if we can't
>> find a mapping for userspace object). I guess at one point we should
>> start thinking about what we want to do on that front.
>
> That limit is taken from *my* hat. Nothing prevents us to increase 
> that limit to whatever.
>
> /Thomas
>
>

Hmm.
Did you guys ever run into trouble with this? Looking at the code, it 
seems the current limit is not 1GB but 1TB? (0x10000000 << PAGE_SHIFT)

/Thomas



>
>> Doing a v3.
>>
>> Cheers,
>> Jerome
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel