[PATCH] drm/ttm: set TTM allocated pages as reserved

[Christian König]

Am 29.03.23 um 17:51 schrieb Paolo Bonzini:
> On 3/29/23 17:29, Christian König wrote:
>>> First, is it a _host_ corruption or a guest corruption/crash?  A 
>>> guest crash would be KVM doing exactly what it's meant to do: it 
>>> detects the non-reserved, non-refcounted page and refuses to map it 
>>> into the guest.
>>
>> Yes I think that this is what happens.
>
> Ok I was worried all the time that this was host corruption/crash; 
> which obviously would have been much worse.
>
>> The use case and mapping is indeed valid as far as I can see, but
>> the handling that KVM does here is really problematic.
>>
>> What needs to happen instead is that when both hva_to_pfn_fast() and 
>> hva_to_pfn_slow() fails you don't try to convert the PFN into a page 
>> and so also don't get a reference to the page.
>>
>> This somehow needs to be communicated to the callers of hva_to_pfn() 
>> so that kvm_release_pfn() knows what to do.
>
> There's a bit more complication here:
>
> 1) in the guest page fault path we can avoid taking the reference 
> altogether
>
> 2) in other MMU-notifier-protected paths, we can also avoid taking the 
> reference but we also must stop using kmap() in virt/kvm/pfncache.c.
>
> 3) other uses of kmap() must switch to MMU-notifier protection.

I would rather suggest to return the page additionally to the pfn from 
hva_to_pfn() when the function was able to grab a reference to it.

When the page is then not available you can't call kmap() on that either.

>
> If the DRM people are okay with SetPageReserved() as a temporary hack, 
> we can change or remove the WARN in kvm_is_zone_device_page(), since 
> that is what you are referring to in the commit message.

Adding Daniel for additional comments on this, but essentially we have 
changed quite some infrastructure to make sure that everybody uses 
VM_PFNMAP to prevent stuff like this from happening.

I would really prefer a proper solution in KVM instead.

Christian.

>
> Paolo
>