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

[David Hildenbrand (Arm)]

On 7/7/26 15:45, Gupta, Pankaj wrote:
> 
>>> 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):


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;
 
        /*
@@ -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))
                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