[PATCH v2 13/15] drm/ttm: Add BO and offset arguments for vm_access and vm_fault ttm handlers.

[Thomas Hellström]

On 5/18/21 7:17 PM, Christian König wrote:
> Am 18.05.21 um 19:10 schrieb Thomas Hellström:
>>
>> On 5/18/21 5:29 PM, Christian König wrote:
>>>
>>>
>>> Am 18.05.21 um 17:25 schrieb Thomas Hellström:
>>>>
>>>> On 5/18/21 5:17 PM, Christian König wrote:
>>>>>
>>>>>
>>>>> Am 18.05.21 um 17:11 schrieb Thomas Hellström:
>>>>>>
>>>>>> On 5/18/21 5:07 PM, Christian König wrote:
>>>>>>> Am 18.05.21 um 16:55 schrieb Thomas Hellström:
>>>>>>>> From: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>>>>>>
>>>>>>>> This allows other drivers that may not setup the vma in the 
>>>>>>>> same way
>>>>>>>> to use the ttm bo helpers.
>>>>>>>
>>>>>>> Uff can you please explain why exactly you need that?
>>>>>>>
>>>>>>> Providing the BO is not much of a problem, but having the BO at 
>>>>>>> different VMA offsets is really a no-go with TTM.
>>>>>>>
>>>>>>> Christian.
>>>>>>
>>>>>> The current i915 uapi is using different offsets for different 
>>>>>> caching :/. We're currently working around that by using 
>>>>>> ttm_bo_type_kernel (no TTM vma offset at all) and i915's offset.
>>>>>
>>>>> Can you instead adjust the offset in the mmap callback like we do 
>>>>> for dma-buf?
>>>> Will have to take a look.
>>>>>
>>>>> That's really a no-go what you describe here because it will mess 
>>>>> up reverse mapping lockup for buffer movement.
>>>>
>>>> You mean the unmap_mapping_range() stuff? That's not a problem 
>>>> since it's a NOP for kernel ttm buffers, and the i915 move() / 
>>>> swap_notify() takes care of killing the ptes.
>>>
>>> That design is a certain NAK from my side for upstreaming this.
>>>
>>> PTE handling is the domain of TTM, drivers should never mess with 
>>> that directly.
>>
>> Hmm. May I humbly suggest a different view on this:
>>
>> I agree fully for ttm_bo_type_device bos but for ttm_bo_type_kernel, 
>> TTM has no business whatsoever with user-space PTEs. That's really 
>> why that bo type exists in the first place. But otoh one can of 
>> course argue that then i915 has no business calling the TTM fault 
>> helper for these bos.
>
> Well the real question is why does i915 wants to expose kernel BOs to 
> userspace? As the name says only the kernel should be able to access 
> them.

I'd say "kernel" of course refers to how TTM handles them.

>
>>
>> So for discrete we can probably do the right thing with 
>> ttm_bo_type_device. What worries me a bit is when we get to older 
>> hardware support because whatever we do is by definition going to be 
>> ugly. At best we might be able to split the address space between 
>> i915's mmos, and hand the rest to TTM, modifying offsets as you 
>> suggest. That way a TTM call to unmap_mapping_range() would do the 
>> right thing, I think.
>
> Well we do all kind of nasty stuff with the offset in DMA-buf, KFD, 
> overlayfs etc. So adjusting the offset inside the kernel is already 
> well supported and used.
>
> What I don't fully understand is your use case here? Can you briefly 
> describe that?
>
> Do you use different bits of the offset to signal what caching 
> behavior you have? And then based on this adjust the pgprot_t in the vma?
>
> Thanks,
> Christian.

TBH I'm not completely sure about the history of this. I think the basic 
idea is that you want to support different vmas with different caching 
modes, so you never change the vma pgprot after mmap time, like TTM 
does. Needless to say this only works with Intel processors on 
integrated and the more I write about this the more I'm convinced that 
we should perhaps not concern TTM with that at all.

/Thomas


>
>>
>> /Thomas
>>
>>>
>>> Christian.
>>>
>>>>
>>>> While we're in the process of killing that offset flexibility for 
>>>> discrete, we can't do so for older hardware unfortunately.
>>>>
>>>> /Thomas
>>>>
>>>>
>>>>>
>>>>> Christian.
>>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>> /Thomas
>>>>>>
>>>>>
>>>
>