Virtualization - Aug 2022 - [PATCH v1] drm/ttm: Refcount allocated tail pages

Am 15.08.22 um 12:09 schrieb Dmitry Osipenko:
> On 8/15/22 13:05, Christian K?nig wrote:
>> Am 15.08.22 um 11:54 schrieb Dmitry Osipenko:
>>> Higher order pages allocated using alloc_pages() aren't
refcounted and
>>> they
>>> need to be refcounted, otherwise it's impossible to map them by
KVM. This
>>> patch sets the refcount of the tail pages and fixes the KVM memory
>>> mapping
>>> faults.
>>>
>>> Without this change guest virgl driver can't map host buffers
into guest
>>> and can't provide OpenGL 4.5 profile support to the guest. The
host
>>> mappings are also needed for enabling the Venus driver using host
GPU
>>> drivers that are utilizing TTM.
>>>
>>> Based on a patch proposed by Trigger Huang.
>> Well I can't count how often I have repeated this: This is an
absolutely
>> clear NAK!
>>
>> TTM pages are not reference counted in the first place and because of
>> this giving them to virgl is illegal.
> A? The first page is refcounted when allocated, the tail pages are not.
No they aren't. The first page is just by coincident initialized with a 
refcount of 1. This refcount is completely ignored and not used at all.

Incrementing the reference count and by this mapping the page into some 
other address space is illegal and corrupts the internal state tracking 
of TTM.
>> Please immediately stop this completely broken approach. We have
>> discussed this multiple times now.
> Could you please give me a link to these discussions?
Not of hand, please search the dri-devel list for similar patches. This 
was brought up multiple times now.

Regards,
Christian.

Am 15.08.22 um 12:11 schrieb Christian K?nig:
> Am 15.08.22 um 12:09 schrieb Dmitry Osipenko:
>> On 8/15/22 13:05, Christian K?nig wrote:
>>> Am 15.08.22 um 11:54 schrieb Dmitry Osipenko:
>>>> Higher order pages allocated using alloc_pages() aren't
refcounted and
>>>> they
>>>> need to be refcounted, otherwise it's impossible to map
them by
>>>> KVM. This
>>>> patch sets the refcount of the tail pages and fixes the KVM
memory
>>>> mapping
>>>> faults.
>>>>
>>>> Without this change guest virgl driver can't map host
buffers into
>>>> guest
>>>> and can't provide OpenGL 4.5 profile support to the guest.
The host
>>>> mappings are also needed for enabling the Venus driver using
host GPU
>>>> drivers that are utilizing TTM.
>>>>
>>>> Based on a patch proposed by Trigger Huang.
>>> Well I can't count how often I have repeated this: This is an 
>>> absolutely
>>> clear NAK!
>>>
>>> TTM pages are not reference counted in the first place and because
of
>>> this giving them to virgl is illegal.
>> A? The first page is refcounted when allocated, the tail pages are not.
>
> No they aren't. The first page is just by coincident initialized with 
> a refcount of 1. This refcount is completely ignored and not used at all.
>
> Incrementing the reference count and by this mapping the page into 
> some other address space is illegal and corrupts the internal state 
> tracking of TTM.
See this comment in the source code as well:

 ??????? /* Don't set the __GFP_COMP flag for higher order allocations.
 ???????? * Mapping pages directly into an userspace process and calling
 ???????? * put_page() on a TTM allocated page is illegal.
 ???????? */

I have absolutely no idea how somebody had the idea he could do this.

Regards,
Christian.
>
>>> Please immediately stop this completely broken approach. We have
>>> discussed this multiple times now.
>> Could you please give me a link to these discussions?
>
> Not of hand, please search the dri-devel list for similar patches. 
> This was brought up multiple times now.
>
> Regards,
> Christian.