Re: [PATCH v10 09/15] KVM: guest_memfd: Add flag to remove from direct map

[Nikita Kalyazin <kalyazin@amazon.com>]

On 06/03/2026 15:16, David Hildenbrand (Arm) wrote:
> On 3/6/26 15:49, Nikita Kalyazin wrote:
>>
>>
>> On 06/03/2026 14:22, David Hildenbrand (Arm) wrote:
>>> [...]
>>>
>>>>
>>>> Dave pointed earlier that the failures were possible [1].  Do you think
>>>> we can document it better?
>>>
>>> I'm fine with checking that somewhere (to catch any future problems).
>>>
>>> Why not do the WARN_ON_ONCE() in folio_restore_direct_map()?
>>>
>>> Then, carefully document (in the new kerneldoc for
>>> folio_restore_direct_map() etc) that folio_restore_direct_map() is only
>>> allowed after a prior successful folio_zap_direct_map(), and add a
>>> helpful comment above the WARN_ON_ONCE() in folio_restore_direct_map()
>>> that we don't expect errors etc.
>>
>> My only concern about that is the assumptions we make in KVM may not
>> apply to the general case and the WARN_ON_ONCE may become too
>> restrictive compared to proper error handling in some (rare) cases.  For
>> example, is it possible for the folio to migrate in between?
> 
> Not without migration support. But then, migration would have to make
> sure that the destination folio has the direct map removed. So I
> wouldn't worry about that.
> 
> Once you return an error from folio_restore_direct_map(), you better
> document when/why it happens and how the caller should handle it.
> 
> There is nothing existing callers could really do, so it's an
> implementation detail of the
> folio_zap_direct_map()/folio_restore_direct_map() functions to make sure
> it keeps working.
> 
> If it ever fails, we'll have to figure out how to keep the interface
> working.
> 
> You can document any restrictions regarding when
> folio_zap_direct_map()/folio_restore_direct_map() is allowed to be used
> in the kerneldoc.

Ok, makes sense to me.

> 
> --
> Cheers,
> 
> David