* [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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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 0 siblings, 0 replies; 20+ 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] 20+ 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; 20+ 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] 20+ messages in thread
end of thread, other threads:[~2026-07-10 16:13 UTC | newest] Thread overview: 20+ 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 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