* [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
@ 2026-07-01 14:45 Pankaj Gupta
2026-07-01 16:25 ` David Hildenbrand (Arm)
0 siblings, 1 reply; 22+ messages in thread
From: Pankaj Gupta @ 2026-07-01 14:45 UTC (permalink / raw)
To: seanjc, pbonzini, tglx, mingo, dave.hansen
Cc: bp, x86, thomas.lendacky, hpa, david, yangge1116, kvm,
linux-kernel, pankaj.gupta, stable
commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
FOLL_WRITE | FOLL_LONGTERM since:
commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
Drop FOLL_LONGTERM when registering encrypted memory regions and restore
the previous behavior.
Fixes: 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
Cc: stable@vger.kernel.org
Signed-off-by: Pankaj Gupta <pankaj.gupta@amd.com>
---
arch/x86/kvm/svm/sev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 6c6a6d663e29..c4b53700f69e 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2764,7 +2764,7 @@ int sev_mem_enc_register_region(struct kvm *kvm,
return -ENOMEM;
region->pages = sev_pin_memory(kvm, range->addr, range->size, ®ion->npages,
- FOLL_WRITE | FOLL_LONGTERM);
+ FOLL_WRITE);
if (IS_ERR(region->pages)) {
ret = PTR_ERR(region->pages);
goto e_free;
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-01 14:45 [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration Pankaj Gupta
@ 2026-07-01 16:25 ` David Hildenbrand (Arm)
2026-07-01 16:30 ` Sean Christopherson
2026-07-06 12:03 ` Gupta, Pankaj
0 siblings, 2 replies; 22+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-01 16:25 UTC (permalink / raw)
To: Pankaj Gupta, seanjc, pbonzini, tglx, mingo, dave.hansen
Cc: bp, x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel,
stable
On 7/1/26 16:45, Pankaj Gupta wrote:
> commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
> added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
> migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
> virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
> FOLL_WRITE | FOLL_LONGTERM since:
>
> commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
> commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
>
> Drop FOLL_LONGTERM when registering encrypted memory regions and restore
> the previous behavior.
But that breaks the original issue of breaking ZONE_MOVABLE/CMA?
If it is a longterm pin, it must use FOLL_LONGTERM. :/
I assume we fail in check_vma_flags()
if ((gup_flags & FOLL_LONGTERM) && vma_is_fsdax(vma))
return -EOPNOTSUPP;
IIRC, fsdax cannot tolerate unbounded pins. Is that the case we are running into?
How does vfio deal with that? (does it?)
--
Cheers,
David
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-01 16:25 ` David Hildenbrand (Arm)
@ 2026-07-01 16:30 ` Sean Christopherson
2026-07-01 16:39 ` David Hildenbrand (Arm)
2026-07-06 12:03 ` Gupta, Pankaj
1 sibling, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2026-07-01 16:30 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Pankaj Gupta, pbonzini, tglx, mingo, dave.hansen, bp, x86,
thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
On Wed, Jul 01, 2026, David Hildenbrand (Arm) wrote:
> On 7/1/26 16:45, Pankaj Gupta wrote:
> > commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
> > added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
> > migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
> > virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
> > FOLL_WRITE | FOLL_LONGTERM since:
> >
> > commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
> > commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
> >
> > Drop FOLL_LONGTERM when registering encrypted memory regions and restore
> > the previous behavior.
>
> But that breaks the original issue of breaking ZONE_MOVABLE/CMA?
Ya.
> If it is a longterm pin, it must use FOLL_LONGTERM. :/
Heh, well, KVM showed that that's not entirely true for many years :-)
Assuming we can't solve this some other way, and that there are "real" use cases
that were broken by adding FOLL_LONGTERM, maybe this as a hack-a-fix?
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 74fb15551e83..ea136d79c963 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2752,6 +2752,25 @@ int sev_mem_enc_register_region(struct kvm *kvm,
region->pages = sev_pin_memory(kvm, range->addr, range->size, ®ion->npages,
FOLL_WRITE | FOLL_LONGTERM);
+
+ /*
+ * On failure, attempt a "short"-term pin for backwards compatibility,
+ * in quotes because this isn't actually a short-term pin. The kernel
+ * disallows long-term writable pins on file-backed memory as a partial
+ * defense against the fundamental problem that most filesystems don't
+ * play nice with kernel writes via GUP (true short-term pins are much
+ * less likely to be problematic).
+ *
+ * Unfortunately, KVM (incorrectly) used a short-term pin for years,
+ * and so can't *require* a long-term pin. And for this use case, the
+ * potential filesystem crashes that occur with kernel writes are a
+ * non-issue, as KVM isn't using this pin to access guest memory, the
+ * pin is performed purely to prevent the memory from being migrated.
+ */
+ if (IS_ERR(region->pages))
+ region->pages = sev_pin_memory(kvm, range->addr, range->size,
+ ®ion->npages, FOLL_WRITE);
+
if (IS_ERR(region->pages)) {
ret = PTR_ERR(region->pages);
goto e_free;
> I assume we fail in check_vma_flags()
>
> if ((gup_flags & FOLL_LONGTERM) && vma_is_fsdax(vma))
> return -EOPNOTSUPP;
>
> IIRC, fsdax cannot tolerate unbounded pins. Is that the case we are running into?
>
> How does vfio deal with that? (does it?)
>
> --
> Cheers,
>
> David
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-01 16:30 ` Sean Christopherson
@ 2026-07-01 16:39 ` David Hildenbrand (Arm)
2026-07-01 16:56 ` Sean Christopherson
0 siblings, 1 reply; 22+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-01 16:39 UTC (permalink / raw)
To: Sean Christopherson
Cc: Pankaj Gupta, pbonzini, tglx, mingo, dave.hansen, bp, x86,
thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
On 7/1/26 18:30, Sean Christopherson wrote:
> On Wed, Jul 01, 2026, David Hildenbrand (Arm) wrote:
>> On 7/1/26 16:45, Pankaj Gupta wrote:
>>> commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
>>> added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
>>> migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
>>> virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
>>> FOLL_WRITE | FOLL_LONGTERM since:
>>>
>>> commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
>>> commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
>>>
>>> Drop FOLL_LONGTERM when registering encrypted memory regions and restore
>>> the previous behavior.
>>
>> But that breaks the original issue of breaking ZONE_MOVABLE/CMA?
>
> Ya.
>
>> If it is a longterm pin, it must use FOLL_LONGTERM. :/
>
> Heh, well, KVM showed that that's not entirely true for many years :-)
What exactly do you mean? KVM MMUs sync through memory notifiers and doesn't
need this.
It's only our "interesting" CoCo code :)
>
> Assuming we can't solve this some other way, and that there are "real" use cases
> that were broken by adding FOLL_LONGTERM, maybe this as a hack-a-fix?
Well, it's not a driver's decision to make. :P
But, can we actually whitelist virtio-pmem in GUP code somehow?
I mean, it does not suffer from the documented writeback issue, that we wanted
to protect from. We similarly allow shmem and hugetlb there.
--
Cheers,
David
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-01 16:39 ` David Hildenbrand (Arm)
@ 2026-07-01 16:56 ` Sean Christopherson
0 siblings, 0 replies; 22+ messages in thread
From: Sean Christopherson @ 2026-07-01 16:56 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Pankaj Gupta, pbonzini, tglx, mingo, dave.hansen, bp, x86,
thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
On Wed, Jul 01, 2026, David Hildenbrand (Arm) wrote:
> On 7/1/26 18:30, Sean Christopherson wrote:
> > On Wed, Jul 01, 2026, David Hildenbrand (Arm) wrote:
> >> On 7/1/26 16:45, Pankaj Gupta wrote:
> >>> commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
> >>> added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
> >>> migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
> >>> virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
> >>> FOLL_WRITE | FOLL_LONGTERM since:
> >>>
> >>> commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
> >>> commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
> >>>
> >>> Drop FOLL_LONGTERM when registering encrypted memory regions and restore
> >>> the previous behavior.
> >>
> >> But that breaks the original issue of breaking ZONE_MOVABLE/CMA?
> >
> > Ya.
> >
> >> If it is a longterm pin, it must use FOLL_LONGTERM. :/
> >
> > Heh, well, KVM showed that that's not entirely true for many years :-)
>
> What exactly do you mean? KVM MMUs sync through memory notifiers and doesn't
> need this.
>
> It's only our "interesting" CoCo code :)
Yeah, I'm just being cheeky and saying that it's obviously possible to do what
is effectively a long-term pint without specifying FOLL_LONGTERM, i.e. saying
it "must" use FOLL_LONGTERM isn't super duper strictly true.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-01 16:25 ` David Hildenbrand (Arm)
2026-07-01 16:30 ` Sean Christopherson
@ 2026-07-06 12:03 ` Gupta, Pankaj
2026-07-07 10:04 ` David Hildenbrand (Arm)
1 sibling, 1 reply; 22+ messages in thread
From: Gupta, Pankaj @ 2026-07-06 12:03 UTC (permalink / raw)
To: David Hildenbrand (Arm), seanjc, pbonzini, tglx, mingo,
dave.hansen
Cc: bp, x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel,
stable
Hi David,
>> commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted memory regions")
>> added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
>> migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
>> virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
>> FOLL_WRITE | FOLL_LONGTERM since:
>>
>> commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to file-backed mappings")
>> commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings").
>>
>> Drop FOLL_LONGTERM when registering encrypted memory regions and restore
>> the previous behavior.
> But that breaks the original issue of breaking ZONE_MOVABLE/CMA?
>
> If it is a longterm pin, it must use FOLL_LONGTERM. :/
>
> I assume we fail in check_vma_flags()
>
> if ((gup_flags & FOLL_LONGTERM) && vma_is_fsdax(vma))
> return -EOPNOTSUPP;
Yes, it fails in this path but for file backed mapping, vma_is_fsdax()
returns false because
vma_is_dax() returns false:
>
> IIRC, fsdax cannot tolerate unbounded pins. Is that the case we are running into?
Host side backend is regular file backed memory (no fsdax).
Thanks,
Pankaj
>
> How does vfio deal with that? (does it?)
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-06 12:03 ` Gupta, Pankaj
@ 2026-07-07 10:04 ` David Hildenbrand (Arm)
2026-07-07 13:45 ` Gupta, Pankaj
0 siblings, 1 reply; 22+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-07 10:04 UTC (permalink / raw)
To: Gupta, Pankaj, seanjc, pbonzini, tglx, mingo, dave.hansen
Cc: bp, x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel,
stable, Lorenzo Stoakes (Oracle)
On 7/6/26 14:03, Gupta, Pankaj wrote:
> Hi David,
>
>>> commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted
>>> memory regions")
>>> added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
>>> migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
>>> virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
>>> FOLL_WRITE | FOLL_LONGTERM since:
>>>
>>> commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to
>>> file-backed mappings")
>>> commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to
>>> file-backed mappings").
>>>
>>> Drop FOLL_LONGTERM when registering encrypted memory regions and restore
>>> the previous behavior.
>> But that breaks the original issue of breaking ZONE_MOVABLE/CMA?
>>
>> If it is a longterm pin, it must use FOLL_LONGTERM. :/
>>
>> I assume we fail in check_vma_flags()
>>
>> if ((gup_flags & FOLL_LONGTERM) && vma_is_fsdax(vma))
>> return -EOPNOTSUPP;
>
> 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 :)
>
>>
>> IIRC, fsdax cannot tolerate unbounded pins. Is that the case we are running into?
>
> 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
--
Cheers,
David
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-07 10:04 ` David Hildenbrand (Arm)
@ 2026-07-07 13:45 ` Gupta, Pankaj
2026-07-07 13:59 ` Sean Christopherson
2026-07-07 14:58 ` David Hildenbrand (Arm)
0 siblings, 2 replies; 22+ messages in thread
From: Gupta, Pankaj @ 2026-07-07 13:45 UTC (permalink / raw)
To: David Hildenbrand (Arm), seanjc, pbonzini, tglx, mingo,
dave.hansen
Cc: bp, x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel,
stable, Lorenzo Stoakes (Oracle)
>> Hi David,
>>
>>>> commit 7e066cb9b71a ("KVM: SEV: Use long-term pin when registering encrypted
>>>> memory regions")
>>>> added FOLL_LONGTERM to sev_mem_enc_register_region() so anonymous guest RAM is
>>>> migrated out of MIGRATE_CMA/ZONE_MOVABLE before a long term pin. This breaks
>>>> virtio-pmem which has file backed (MAP_SHARED) host mapping where GUP rejects
>>>> FOLL_WRITE | FOLL_LONGTERM since:
>>>>
>>>> commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast writing to
>>>> file-backed mappings")
>>>> commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to
>>>> file-backed mappings").
>>>>
>>>> Drop FOLL_LONGTERM when registering encrypted memory regions and restore
>>>> the previous behavior.
>>> But that breaks the original issue of breaking ZONE_MOVABLE/CMA?
>>>
>>> If it is a longterm pin, it must use FOLL_LONGTERM. :/
>>>
>>> I assume we fail in check_vma_flags()
>>>
>>> if ((gup_flags & FOLL_LONGTERM) && vma_is_fsdax(vma))
>>> return -EOPNOTSUPP;
>> 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 :)
>
>>> IIRC, fsdax cannot tolerate unbounded pins. Is that the case we are running into?
>> 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.
2. Explicitly restrict long-term pinning for file-backed mappings with a
change like the patch below [1].
David, Sean,
Do you have a preference between these two approaches? I am leaning
toward towards option 2.
Thank you!
Pankaj
---
[1]
arch/x86/kvm/svm/sev.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 6c6a6d663e29..c4b53700f69e 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2743,6 +2743,29 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void
__user *argp)
return r;
}
+static unsigned int sev_region_gup_flags(unsigned long uaddr, unsigned
long ulen)
+{
+ struct mm_struct *mm = current->mm;
+ unsigned long end = uaddr + ulen;
+ struct vm_area_struct *vma;
+ unsigned int flags = FOLL_WRITE | FOLL_LONGTERM;
+ VMA_ITERATOR(vmi, mm, uaddr);
+
+ if (ulen == 0 || end < uaddr)
+ return FOLL_WRITE;
+
+ mmap_read_lock(mm);
+ for_each_vma_range(vmi, vma, end) {
+ if (!vma_is_anonymous(vma)) {
+ flags = FOLL_WRITE;
+ break;
+ }
+ }
+ mmap_read_unlock(mm);
+
+ return flags;
+}
+
int sev_mem_enc_register_region(struct kvm *kvm,
struct kvm_enc_region *range)
{
@@ -2764,7 +2787,7 @@ int sev_mem_enc_register_region(struct kvm *kvm,
return -ENOMEM;
region->pages = sev_pin_memory(kvm, range->addr, range->size,
®ion->npages,
- FOLL_WRITE | FOLL_LONGTERM);
+ sev_region_gup_flags(range->addr, range->size));
if (IS_ERR(region->pages)) {
ret = PTR_ERR(region->pages);
goto e_free;
(END)
--
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-07 13:45 ` Gupta, Pankaj
@ 2026-07-07 13:59 ` Sean Christopherson
2026-07-07 14:58 ` David Hildenbrand (Arm)
1 sibling, 0 replies; 22+ messages in thread
From: Sean Christopherson @ 2026-07-07 13:59 UTC (permalink / raw)
To: Pankaj Gupta
Cc: David Hildenbrand (Arm), pbonzini, tglx, mingo, dave.hansen, bp,
x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable,
Lorenzo Stoakes (Oracle)
On Tue, Jul 07, 2026, Pankaj Gupta wrote:
> > > > IIRC, fsdax cannot tolerate unbounded pins. Is that the case we are running into?
> > > 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.
The problem that _is_ "isolated" to SEV though, is that this used to work :-/
> > 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.
>
> 2. Explicitly restrict long-term pinning for file-backed mappings with a
> change like the patch below [1].
>
> David, Sean,
>
> Do you have a preference between these two approaches? I am leaning toward
> towards option 2.
Option 1. Option 2 has a TOCTOU bug (the VMA could be changed after dropping
mmap_lock), and I'd rather not bleed any more mm/ details into KVM than is
required: falling back to a non-FOLL_LONGTERM acknowledges that there are mapping
types that don't support WRITE+LONGTERM, but checking vma_is_anonymous() takes
things a few steps further by forcing KVM to know what types of mappings are
problematic.
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 6c6a6d663e29..c4b53700f69e 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2743,6 +2743,29 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user
> *argp)
> return r;
> }
>
> +static unsigned int sev_region_gup_flags(unsigned long uaddr, unsigned long
> ulen)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned long end = uaddr + ulen;
> + struct vm_area_struct *vma;
> + unsigned int flags = FOLL_WRITE | FOLL_LONGTERM;
> + VMA_ITERATOR(vmi, mm, uaddr);
> +
> + if (ulen == 0 || end < uaddr)
> + return FOLL_WRITE;
> +
> + mmap_read_lock(mm);
> + for_each_vma_range(vmi, vma, end) {
> + if (!vma_is_anonymous(vma)) {
> + flags = FOLL_WRITE;
> + break;
> + }
> + }
> + mmap_read_unlock(mm);
> +
> + return flags;
> +}
> +
> int sev_mem_enc_register_region(struct kvm *kvm,
> struct kvm_enc_region *range)
> {
> @@ -2764,7 +2787,7 @@ int sev_mem_enc_register_region(struct kvm *kvm,
> return -ENOMEM;
>
> region->pages = sev_pin_memory(kvm, range->addr, range->size,
> ®ion->npages,
> - FOLL_WRITE | FOLL_LONGTERM);
> + sev_region_gup_flags(range->addr, range->size));
> if (IS_ERR(region->pages)) {
> ret = PTR_ERR(region->pages);
> goto e_free;
> (END)
> --
>
>
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-07 13:45 ` Gupta, Pankaj
2026-07-07 13:59 ` Sean Christopherson
@ 2026-07-07 14:58 ` David Hildenbrand (Arm)
2026-07-08 16:35 ` Gupta, Pankaj
2026-07-09 14:28 ` Lorenzo Stoakes
1 sibling, 2 replies; 22+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-07 14:58 UTC (permalink / raw)
To: Gupta, Pankaj, seanjc, pbonzini, tglx, mingo, dave.hansen
Cc: bp, x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel,
stable, Lorenzo Stoakes (Oracle)
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-07 14:58 ` David Hildenbrand (Arm)
@ 2026-07-08 16:35 ` Gupta, Pankaj
2026-07-09 14:28 ` Lorenzo Stoakes
1 sibling, 0 replies; 22+ messages in thread
From: Gupta, Pankaj @ 2026-07-08 16:35 UTC (permalink / raw)
To: David Hildenbrand (Arm), seanjc, pbonzini, tglx, mingo,
dave.hansen
Cc: bp, x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel,
stable, Lorenzo Stoakes (Oracle)
Hi David, Sean,
Thank you for the discussion!
>> 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)))
>
David,
Yes, the above approach works with few changes [2]. If it looks okay
will send a v2 .
Best regards,
Pankaj
[2]
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index b18c2b2e7d2c..f9af801788b0 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1911,6 +1911,14 @@ enum {
> */
> FOLL_HONOR_NUMA_FAULT = 1 << 12,
>
> + /*
> + * Long-term pin without kernel GUP writes. For callers that pin
> + * writable file-backed mappings only to prevent migration.
> Must be
> + * used with FOLL_PIN and FOLL_LONGTERM. Bypasses the writable
> + * file-backed long-term pin restriction in gup.c.
> + */
> + FOLL_PIN_NO_GUP_WRITE = 1 << 13,
> +
> /* See also internal only FOLL flags in mm/internal.h */
> };
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 0692119b7904..a83d100f7950 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1186,6 +1186,10 @@ 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_NO_GUP_WRITE)) ==
> + (FOLL_PIN | FOLL_LONGTERM | FOLL_PIN_NO_GUP_WRITE))
> + return true;
> +
> if ((gup_flags & (FOLL_PIN | FOLL_LONGTERM)) !=
> (FOLL_PIN | FOLL_LONGTERM))
> return true;
> @@ -2530,6 +2534,11 @@ static bool is_valid_gup_args(struct page
> **pages, int *locked,
> if (WARN_ON_ONCE(!(gup_flags & FOLL_PIN) && (gup_flags &
> FOLL_LONGTERM)))
> return false;
>
> + if (WARN_ON_ONCE((gup_flags & FOLL_PIN_NO_GUP_WRITE) &&
> + (gup_flags & (FOLL_PIN | FOLL_LONGTERM)) !=
> + (FOLL_PIN | FOLL_LONGTERM)))
> + return false;
> +
> /* Pages input must be given if using GET/PIN */
> if (WARN_ON_ONCE((gup_flags & (FOLL_GET | FOLL_PIN)) && !pages))
> return false;
> @@ -2747,7 +2756,8 @@ static bool gup_fast_folio_allowed(struct folio
> *folio, unsigned int flags)
> * pin is the most egregious case so this is the one we disallow.
> */
>
> + if ((gup_flags & (FOLL_PIN | FOLL_LONGTERM |
> FOLL_PIN_NO_GUP_WRITE)) ==
> + (FOLL_PIN | FOLL_LONGTERM | FOLL_PIN_NO_GUP_WRITE))
> + return true;
> +
> if ((gup_flags & (FOLL_PIN | FOLL_LONGTERM)) !=
> (FOLL_PIN | FOLL_LONGTERM))
> return true;
> @@ -2530,6 +2534,11 @@ static bool is_valid_gup_args(struct page
> **pages, int *locked,
> if (WARN_ON_ONCE(!(gup_flags & FOLL_PIN) && (gup_flags &
> FOLL_LONGTERM)))
> return false;
>
> + if (WARN_ON_ONCE((gup_flags & FOLL_PIN_NO_GUP_WRITE) &&
> + (gup_flags & (FOLL_PIN | FOLL_LONGTERM)) !=
> + (FOLL_PIN | FOLL_LONGTERM)))
> + return false;
> +
> /* Pages input must be given if using GET/PIN */
> if (WARN_ON_ONCE((gup_flags & (FOLL_GET | FOLL_PIN)) && !pages))
> return false;
> @@ -2747,7 +2756,8 @@ static bool gup_fast_folio_allowed(struct folio
> *folio, unsigned int flags)
> * pin is the most egregious case so this is the one we disallow.
> */
> if ((flags & (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE)) ==
> - (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE))
> + (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE) &&
> + !(flags & FOLL_PIN_NO_GUP_WRITE))
> reject_file_backed = true;
>
> /* We hold a folio reference, so we can safely access folio
> fields. */
> @@ -3180,7 +3190,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_PIN_NO_GUP_WRITE |
> FOLL_FORCE | FOLL_PIN | FOLL_GET |
> FOLL_FAST_ONLY | FOLL_NOFAULT |
> FOLL_PCI_P2PDMA |
> FOLL_HONOR_NUMA_FAULT)))
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-07 14:58 ` David Hildenbrand (Arm)
2026-07-08 16:35 ` Gupta, Pankaj
@ 2026-07-09 14:28 ` Lorenzo Stoakes
2026-07-09 15:19 ` Gupta, Pankaj
1 sibling, 1 reply; 22+ messages in thread
From: Lorenzo Stoakes @ 2026-07-09 14:28 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Gupta, Pankaj, seanjc, pbonzini, tglx, mingo, dave.hansen, bp,
x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
On Tue, Jul 07, 2026 at 04:58:39PM +0200, David Hildenbrand (Arm) wrote:
> 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):
So under what circumstances are we happy with totally breaking dirty tracking?
:/ seems iffy, and exposing this to drivers generally is a bit worrysome.
>
>
> 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?
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);
}
>
> /*
> @@ -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).
> 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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-09 14:28 ` Lorenzo Stoakes
@ 2026-07-09 15:19 ` Gupta, Pankaj
2026-07-09 15:44 ` Lorenzo Stoakes
0 siblings, 1 reply; 22+ messages in thread
From: Gupta, Pankaj @ 2026-07-09 15:19 UTC (permalink / raw)
To: Lorenzo Stoakes, David Hildenbrand (Arm)
Cc: seanjc, pbonzini, tglx, mingo, dave.hansen, bp, x86,
thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-09 15:19 ` Gupta, Pankaj
@ 2026-07-09 15:44 ` Lorenzo Stoakes
2026-07-10 12:57 ` David Hildenbrand (Arm)
0 siblings, 1 reply; 22+ messages in thread
From: Lorenzo Stoakes @ 2026-07-09 15:44 UTC (permalink / raw)
To: Gupta, Pankaj
Cc: David Hildenbrand (Arm), seanjc, pbonzini, tglx, mingo,
dave.hansen, bp, x86, thomas.lendacky, hpa, yangge1116, kvm,
linux-kernel, stable
On Thu, Jul 09, 2026 at 05:19:10PM +0200, Gupta, Pankaj wrote:
> 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.
OK as long as that's made clear in the patch, commit message, comments etc. :)
>
> > >
> > > 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.
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!
>
> >
> > > /*
> > > @@ -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.
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 :)
>
> 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
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-09 15:44 ` Lorenzo Stoakes
@ 2026-07-10 12:57 ` David Hildenbrand (Arm)
2026-07-10 13:05 ` Lorenzo Stoakes
2026-07-10 13:06 ` Gupta, Pankaj
0 siblings, 2 replies; 22+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-10 12:57 UTC (permalink / raw)
To: Lorenzo Stoakes, Gupta, Pankaj
Cc: seanjc, pbonzini, tglx, mingo, dave.hansen, bp, x86,
thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-10 12:57 ` David Hildenbrand (Arm)
@ 2026-07-10 13:05 ` Lorenzo Stoakes
2026-07-10 15:37 ` David Hildenbrand (Arm)
2026-07-10 13:06 ` Gupta, Pankaj
1 sibling, 1 reply; 22+ messages in thread
From: Lorenzo Stoakes @ 2026-07-10 13:05 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Gupta, Pankaj, seanjc, pbonzini, tglx, mingo, dave.hansen, bp,
x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-10 12:57 ` David Hildenbrand (Arm)
2026-07-10 13:05 ` Lorenzo Stoakes
@ 2026-07-10 13:06 ` Gupta, Pankaj
1 sibling, 0 replies; 22+ messages in thread
From: Gupta, Pankaj @ 2026-07-10 13:06 UTC (permalink / raw)
To: David Hildenbrand (Arm), Lorenzo Stoakes
Cc: seanjc, pbonzini, tglx, mingo, dave.hansen, bp, x86,
thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
>> 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" ?
Honestly, I did not finalize it then, just tossed the name for some
suggestion :-)
> 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" ?
Maybe FOLL_LONGTERM_LEGACY_, still pondering what that name could be ...
Thanks,
Pankaj
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-10 13:05 ` Lorenzo Stoakes
@ 2026-07-10 15:37 ` David Hildenbrand (Arm)
2026-07-10 15:47 ` Sean Christopherson
0 siblings, 1 reply; 22+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-10 15:37 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Gupta, Pankaj, seanjc, pbonzini, tglx, mingo, dave.hansen, bp,
x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
On 7/10/26 15:05, Lorenzo Stoakes wrote:
> On Fri, Jul 10, 2026 at 02:57:55PM +0200, David Hildenbrand (Arm) wrote:
>> On 7/9/26 17:44, Lorenzo Stoakes wrote:
>>>
>>> OK as long as that's made clear in the patch, commit message, comments etc. :)
>>>
>>>
>>> Ack yeah I assumed it was a quick proof of concept and just overlooked it :P
>>>
>>>
>>> Thanks!
>>>
>>>
>>> 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
I think the KVM gueest *will* write to these pages.
By disallowing writable LONGTERM pins on FSes we broke one existing use case
that was relying on that to work.
--
Cheers,
David
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-10 15:37 ` David Hildenbrand (Arm)
@ 2026-07-10 15:47 ` Sean Christopherson
2026-07-10 16:13 ` Lorenzo Stoakes
0 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2026-07-10 15:47 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Lorenzo Stoakes, Pankaj Gupta, pbonzini, tglx, mingo, dave.hansen,
bp, x86, thomas.lendacky, hpa, yangge1116, kvm, linux-kernel,
stable
On Fri, Jul 10, 2026, David Hildenbrand (Arm) wrote:
> On 7/10/26 15:05, Lorenzo Stoakes wrote:
> > On Fri, Jul 10, 2026 at 02:57:55PM +0200, David Hildenbrand (Arm) wrote:
> >> On 7/9/26 17:44, Lorenzo Stoakes wrote:
> >>>
> >>> OK as long as that's made clear in the patch, commit message, comments etc. :)
> >>>
> >>>
> >>> Ack yeah I assumed it was a quick proof of concept and just overlooked it :P
> >>>
> >>>
> >>> Thanks!
> >>>
> >>>
> >>> 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
>
> I think the KVM gueest *will* write to these pages.
Yes, the guest will write these pages, but through KVM's normal mechanism for
mapping memory into guests. The host will NOT write this memory via the GUP
pins though. KVM needs to pin the pages because the memory is (well, technically
may be) encrypted (by the CPU) with a key that is only used/accessible when the
guest is active, and the encryption is salted with the system physical address of
the page. E.g. attempting to migrate the page would corrupt guest memory due to
copying ciphertext that would decrypt different at the new PA.
> By disallowing writable LONGTERM pins on FSes we broke one existing use case
> that was relying on that to work.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-10 15:47 ` Sean Christopherson
@ 2026-07-10 16:13 ` Lorenzo Stoakes
2026-07-10 16:24 ` Sean Christopherson
0 siblings, 1 reply; 22+ messages in thread
From: Lorenzo Stoakes @ 2026-07-10 16:13 UTC (permalink / raw)
To: Sean Christopherson
Cc: David Hildenbrand (Arm), Pankaj Gupta, pbonzini, tglx, mingo,
dave.hansen, bp, x86, thomas.lendacky, hpa, yangge1116, kvm,
linux-kernel, stable
On Fri, Jul 10, 2026 at 08:47:19AM -0700, Sean Christopherson wrote:
> On Fri, Jul 10, 2026, David Hildenbrand (Arm) wrote:
> > On 7/10/26 15:05, Lorenzo Stoakes wrote:
> > > On Fri, Jul 10, 2026 at 02:57:55PM +0200, David Hildenbrand (Arm) wrote:
> > >> On 7/9/26 17:44, Lorenzo Stoakes wrote:
> > >>>
> > >>> OK as long as that's made clear in the patch, commit message, comments etc. :)
> > >>>
> > >>>
> > >>> Ack yeah I assumed it was a quick proof of concept and just overlooked it :P
> > >>>
> > >>>
> > >>> Thanks!
> > >>>
> > >>>
> > >>> 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
> >
> > I think the KVM gueest *will* write to these pages.
>
> Yes, the guest will write these pages, but through KVM's normal mechanism for
> mapping memory into guests. The host will NOT write this memory via the GUP
> pins though. KVM needs to pin the pages because the memory is (well, technically
> may be) encrypted (by the CPU) with a key that is only used/accessible when the
> guest is active, and the encryption is salted with the system physical address of
> the page. E.g. attempting to migrate the page would corrupt guest memory due to
> copying ciphertext that would decrypt different at the new PA.
OK so it's a pinky promise that you won't write to it via GUP?
It's still really crap to just allow drivers to ignore this, which is asking for
abuse.
Is this something we could have a specific GUP helper for that is unexported? Or
does a module have to use this?
If not could we use some whitelisted approach or something to prevent arbitrary
drivers from overriding this?
>
> > By disallowing writable LONGTERM pins on FSes we broke one existing use case
> > that was relying on that to work.
How long ago did I break this though? Why has it taken until now for this to be
reported? :) commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast
writing to file-backed mappings") is from May 2023 :)
Is it vendors moving slow to update distros? Does speak to the usefulness of
testing mainline asap in any case.
I maybe missing details about the actual motivating issue here sorry!
Thanks, Lorenzo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-10 16:13 ` Lorenzo Stoakes
@ 2026-07-10 16:24 ` Sean Christopherson
2026-07-10 16:38 ` David Hildenbrand (Arm)
0 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2026-07-10 16:24 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: David Hildenbrand (Arm), Pankaj Gupta, pbonzini, tglx, mingo,
dave.hansen, bp, x86, thomas.lendacky, hpa, yangge1116, kvm,
linux-kernel, stable
On Fri, Jul 10, 2026, Lorenzo Stoakes wrote:
> On Fri, Jul 10, 2026 at 08:47:19AM -0700, Sean Christopherson wrote:
> > On Fri, Jul 10, 2026, David Hildenbrand (Arm) wrote:
> > > On 7/10/26 15:05, Lorenzo Stoakes wrote:
> > > > On Fri, Jul 10, 2026 at 02:57:55PM +0200, David Hildenbrand (Arm) wrote:
> > > >> On 7/9/26 17:44, Lorenzo Stoakes wrote:
> > > >>>
> > > >>> OK as long as that's made clear in the patch, commit message, comments etc. :)
> > > >>>
> > > >>>
> > > >>> Ack yeah I assumed it was a quick proof of concept and just overlooked it :P
> > > >>>
> > > >>>
> > > >>> Thanks!
> > > >>>
> > > >>>
> > > >>> 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
> > >
> > > I think the KVM gueest *will* write to these pages.
> >
> > Yes, the guest will write these pages, but through KVM's normal mechanism for
> > mapping memory into guests. The host will NOT write this memory via the GUP
> > pins though. KVM needs to pin the pages because the memory is (well, technically
> > may be) encrypted (by the CPU) with a key that is only used/accessible when the
> > guest is active, and the encryption is salted with the system physical address of
> > the page. E.g. attempting to migrate the page would corrupt guest memory due to
> > copying ciphertext that would decrypt different at the new PA.
>
> OK so it's a pinky promise that you won't write to it via GUP?
LOL, yep.
> It's still really crap to just allow drivers to ignore this, which is asking for
> abuse.
Yes, the KVM API in question is garbage, and for all intents and purposes it's
deprecated going forward, but unfortunately we're stuck with it.
> Is this something we could have a specific GUP helper for that is unexported? Or
> does a module have to use this?
Module, but we can limit to KVM modules via EXPORT_SYMBOL_FOR_KVM(), which also
discourages abuse by only providing the export if KVM is actually configured to
be built as a module.
> If not could we use some whitelisted approach or something to prevent arbitrary
> drivers from overriding this?
>
> >
> > > By disallowing writable LONGTERM pins on FSes we broke one existing use case
> > > that was relying on that to work.
>
> How long ago did I break this though? Why has it taken until now for this to be
> reported? :) commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast
> writing to file-backed mappings") is from May 2023 :)
The break didn't come from your changes, it came from commit 7e066cb9b71a ("KVM:
SEV: Use long-term pin when registering encrypted memory regions"). I suggested
falling back to a non-longterm pin, but David didn't like that idea :-)
https://lore.kernel.org/all/akVAnGuiuJttE5-6@google.com
> Is it vendors moving slow to update distros? Does speak to the usefulness of
> testing mainline asap in any case.
>
> I maybe missing details about the actual motivating issue here sorry!
>
> Thanks, Lorenzo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
2026-07-10 16:24 ` Sean Christopherson
@ 2026-07-10 16:38 ` David Hildenbrand (Arm)
0 siblings, 0 replies; 22+ messages in thread
From: David Hildenbrand (Arm) @ 2026-07-10 16:38 UTC (permalink / raw)
To: Sean Christopherson, Lorenzo Stoakes
Cc: Pankaj Gupta, pbonzini, tglx, mingo, dave.hansen, bp, x86,
thomas.lendacky, hpa, yangge1116, kvm, linux-kernel, stable
>>
>> How long ago did I break this though? Why has it taken until now for this to be
>> reported? :) commit 8ac268436e6d ("mm/gup: disallow FOLL_LONGTERM GUP-nonfast
>> writing to file-backed mappings") is from May 2023 :)
>
> The break didn't come from your changes, it came from commit 7e066cb9b71a ("KVM:
> SEV: Use long-term pin when registering encrypted memory regions"). I suggested
> falling back to a non-longterm pin, but David didn't like that idea :-)
Yes, a longterm pin is a longterm pin.
If we don't write to the memory, why do we need a write pin? To make sure that
what we pin was actually unshared?
Well, FOLL_LONGTERM does that nowadays.
... so is maybe dropping the FOLL_WRITE sufficient?
--
Cheers,
David
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2026-07-10 16:38 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 14:45 [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration Pankaj Gupta
2026-07-01 16:25 ` David Hildenbrand (Arm)
2026-07-01 16:30 ` Sean Christopherson
2026-07-01 16:39 ` David Hildenbrand (Arm)
2026-07-01 16:56 ` Sean Christopherson
2026-07-06 12:03 ` Gupta, Pankaj
2026-07-07 10:04 ` David Hildenbrand (Arm)
2026-07-07 13:45 ` Gupta, Pankaj
2026-07-07 13:59 ` Sean Christopherson
2026-07-07 14:58 ` David Hildenbrand (Arm)
2026-07-08 16:35 ` Gupta, Pankaj
2026-07-09 14:28 ` Lorenzo Stoakes
2026-07-09 15:19 ` Gupta, Pankaj
2026-07-09 15:44 ` Lorenzo Stoakes
2026-07-10 12:57 ` David Hildenbrand (Arm)
2026-07-10 13:05 ` Lorenzo Stoakes
2026-07-10 15:37 ` David Hildenbrand (Arm)
2026-07-10 15:47 ` Sean Christopherson
2026-07-10 16:13 ` Lorenzo Stoakes
2026-07-10 16:24 ` Sean Christopherson
2026-07-10 16:38 ` David Hildenbrand (Arm)
2026-07-10 13:06 ` Gupta, Pankaj
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox