Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration

[David Hildenbrand (Arm)]

On 7/9/26 17:44, Lorenzo Stoakes wrote:
> On Thu, Jul 09, 2026 at 05:19:10PM +0200, Gupta, Pankaj wrote:
>> Hi Lorenzo,
>>
>>> So under what circumstances are we happy with totally breaking dirty tracking?
>>> :/ seems iffy, and exposing this to drivers generally is a bit worrysome.
>>
>> The intention is to allow long-term pinning of file-backed mappings only for
>> migration avoidance,
>>
>> without kernel GUP writes, and therefore not impacting dirty tracking.
> 
> OK as long as that's made clear in the patch, commit message, comments etc. :)
> 
>>
>>> Hmm I'm confused, you're then allowing FOLL_PIN | FOLL_LONGTERM, but disallowing
>>> FOLL_PIN | FOLL_LONGTERM | FOLL_LONGTERM_HACK?
>>
>> Yes, I addressed this in my reply, but it wasn't a clean inline response.
> 
> Ack yeah I assumed it was a quick proof of concept and just overlooked it :P
> 
>>
>>>
>>> By the way I think this should be expressed better if I criticise myself here :)
>>>
>>> So like:
>>>
>>> 	if ((gup_flags & FOLL_PIN) && (gup_flags & FOLL_LONGTERM))
>>>
>>> Or even:
>>>
>>> 	/* Only an issue if we pin... */
>>> 	if (!(gup_flags & FOLL_PIN))
>>> 		return false;
>>> 	/* ...and that pin is longterm... */
>>> 	if (!(gup_flags & FOLL_LONGTERM))
>>> 		return false;
>>>
>>> But I'm confused as to why we are suddenly allowing something broken and what
>>> this hack flag is supposed to achieve?
>>>
>>> Shouldn't this rather be:
>>>
>>> 	/* Only an issue if we pin... */
>>> 	if (!(gup_flags & FOLL_PIN))
>>> 		return true;
>>> 	/* ...and that pin is longterm... */
>>> 	if (!(gup_flags & FOLL_LONGTERM))
>>> 		return true;
>>> 	/* ...and not overridden... */
>>> 	if (gup_flags & FOLL_LONGTERM_HACK)
>>> 		return true;
>>> 	/* ...and dirty tracking is required. */
>>> 	return !vma_needs_dirty_tracking(vma);
>>> }
>>
>> Yes, this looks much better. Will incorporate this.
> 
> Thanks!
> 
>>
>>>
>>> Yeah this is just a bit horrid having to stare at a this a while... So
>>> FOLL_LONGTERM_HACK would enable here.
>>>
>>> Be nice to avoid this form of it as it's difficult to understand, do something
>>> like above or a clearer version anyway (probably best abstracted to a small
>>> function).
>>
>> Sure.
>>
>> Also, I am also planning to rename (FOLL_LONGTERM_HACK ->
>> FOLL_PIN_NO_GUP_WRITE) in v2.
> 
> hmm but we have FOLL_LONGTERM as an adjunct to FOLL_PIN (doesn't make sense
> without - any checks that exist for that btw should be extended to this noew
> flag).
> 
> Also don't we want to encode the legacy aspect here?
> 
> Maybe FOLL_LONGTERM_LEGACY_READONLY? Naming is hard :)

I'm confused about the _READONLY, well. and the FOLL_PIN_NO_GUP_WRITE.

We want to longterm write-pin.

@Pankaj, how come you would call this "FOLL_PIN_NO_GUP_WRITE" -- why "no GUP
write" ?

I agree that someting like FOLL_LONGTERM_LEGACY_* is the right thing to do, but
I don't see where this is "no write" or "readonly" ?

-- 
Cheers,

David