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

From: Gupta, Pankaj

Date: Thu Jul 09 2026 - 11:26:07 EST

Hi Lorenzo,

Hi David,

Yes, it fails in this path but for file backed mapping, vma_is_fsdax() returns
false because

vma_is_dax() returns false:
Ah, okay, so fsdax is not involved and we really only fail because of the
writable_file_mapping_allowed() check.

I was for a second thinking in terms of nested virt :)

Host side backend is regular file backed memory (no fsdax).
Okay, so we'll end up mapping an ordinary file into VM memory, and expose that
to the VM as part of virtio-pmem device.

That also means that vfio etc. won't be able to longterm-pin such device memory.
So this is not a problem isolated to SEV.

Forbidding to longterm pin is actually the right thing to do if the filesystem
relies on writenotify, as spelled out by Lorenzo's commit:

"
     Writing to file-backed mappings which require folio dirty tracking using
     GUP is a fundamentally broken operation, as kernel write access to GUP
     mappings do not adhere to the semantics expected by a file system.

     A GUP caller uses the direct mapping to access the folio, which does not
     cause write notify to trigger, nor does it enforce that the caller marks
     the folio dirty.

     The problem arises when, after an initial write to the folio, writeback
     results in the folio being cleaned and then the caller, via the GUP
     interface, writes to the folio again.
"

Hmmm
Yes. For file based mapping we don't allow long term pinning.

If we take into account the fragmentation concerns for MIGRATE_CMA and
ZONE_MOVABLE allocations

solvable with FOLL_LONGTERM, I can think of two options(tested) to allow file
based mappings as well:

1. Fallback on FOLL_WRITE when FOLL_LONGTERM fails as suggested by Sean.
That is just not acceptable, as it breaks random other stuff (MIGRATE_CMA, as
one example) besides the file-pinning problems that Lorenzo added.

If we're going to hack something in, then that we bypass the file writeback check.
Not that we don't use FOLL_LONGTERM.

I'd hate to use a GUP flag to indicate "this is a legacy hack", but it clearly isolates the
issue (needs a better name obviously):
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.


diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index ae9bca4eda5ca..e2c531f914d44 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1912,6 +1912,9 @@ enum {
*/
FOLL_HONOR_NUMA_FAULT = 1 << 12,

+ /* TODO */
+ FOLL_LONGTERM = 1 << 13,
+
/* See also internal only FOLL flags in mm/internal.h */
};

diff --git a/mm/gup.c b/mm/gup.c
index 0692119b79043..1fa0aa0cdc99d 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1186,8 +1186,8 @@ static bool writable_file_mapping_allowed(struct vm_area_struct *vma,
* If we aren't pinning then no problematic write can occur. A long term
* pin is the most egregious case so this is the case we disallow.
*/
- if ((gup_flags & (FOLL_PIN | FOLL_LONGTERM)) !=
- (FOLL_PIN | FOLL_LONGTERM))
+ if ((gup_flags & (FOLL_PIN | FOLL_LONGTERM | FOLL_LONGTERM_HACK)) !=
+ (FOLL_PIN | FOLL_LONGTERM | FOLL_LONGTERM_HACK))
return true;
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.


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.


/*
@@ -2746,7 +2746,7 @@ static bool gup_fast_folio_allowed(struct folio *folio, unsigned int flags)
* If we aren't pinning then no problematic write can occur. A long term
* pin is the most egregious case so this is the one we disallow.
*/
- if ((flags & (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE)) ==
+ if ((flags & (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE | FOLL_LONGTERM_HACK)) ==
(FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE))
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.

Please let me know if you have a preference.

Thanks,

Pankaj


reject_file_backed = true;

@@ -3180,7 +3180,7 @@ static int gup_fast_fallback(unsigned long start, unsigned long nr_pages,
int locked = 0;
int ret;

- if (WARN_ON_ONCE(gup_flags & ~(FOLL_WRITE | FOLL_LONGTERM |
+ if (WARN_ON_ONCE(gup_flags & ~(FOLL_WRITE | FOLL_LONGTERM | FOLL_LONGTERM_HACK |
FOLL_FORCE | FOLL_PIN | FOLL_GET |
FOLL_FAST_ONLY | FOLL_NOFAULT |
FOLL_PCI_P2PDMA | FOLL_HONOR_NUMA_FAULT)))


--
Cheers,

David
Thanks, Lorenzo