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

[Lorenzo Stoakes <ljs@kernel.org>]

On Fri, Jul 10, 2026 at 02:57:55PM +0200, David Hildenbrand (Arm) wrote:
> 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" ?

I based it on Gupta saying 'without kernel GUP writes, and therefore not
impacting dirty tracking'

I mean I think we definitely need some clarification here yes :)

Not really got the bandwidth to dig deep into GUP again :P

Gupta could you please clarify exactly what's happening here?

>
> --
> Cheers,
>
> David

Thanks, Lorenzo