All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: SEV: drop FOLL_LONGTERM for encrypted region registration
@ 2026-07-01 14:45 Pankaj Gupta
  2026-07-01 14:58 ` sashiko-bot
  2026-07-01 16:25 ` David Hildenbrand (Arm)
  0 siblings, 2 replies; 10+ 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, &region->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] 10+ 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 14:58 ` sashiko-bot
  2026-07-01 16:25 ` David Hildenbrand (Arm)
  1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-01 14:58 UTC (permalink / raw)
  To: Pankaj Gupta; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Dropping FOLL_LONGTERM creates a silent long-term pin, bypassing GUP safety checks for file-backed mappings and breaking CMA isolation for anonymous RAM.

Pre-existing issues:
- [Critical] Data race and unprotected state modification on `mirror->enc_context_owner` during SEV VM migration leads to list corruption and Use-After-Free.
--

commit a611d4c7ad0ddfba6a16536a997563da04d1672f
Author: Pankaj Gupta <pankaj.gupta@amd.com>

KVM: SEV: drop FOLL_LONGTERM for encrypted region registration

This commit removes the FOLL_LONGTERM flag from sev_pin_memory() when
registering encrypted memory regions. This aims to restore virtio-pmem
compatibility by allowing GUP to succeed on file-backed host mappings.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 74fb15551e83f..053b59068b580 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
[ ... ]
> @@ -2751,7 +2751,7 @@ int sev_mem_enc_register_region(struct kvm *kvm,
>  		return -ENOMEM;
>  
>  	region->pages = sev_pin_memory(kvm, range->addr, range->size, &region->npages,
> -				       FOLL_WRITE | FOLL_LONGTERM);
> +				       FOLL_WRITE);

[Severity: High]
Does removing FOLL_LONGTERM here create a silent long-term pin?

By dropping FOLL_LONGTERM, it appears we bypass GUP safety checks for
file-backed mappings and break CMA isolation for anonymous RAM. This could
allow guest VMs to permanently pin movable pages, leading to CMA allocation
failures and breaking memory hot-unplug.

For file-backed mappings, could this cause filesystem operations like
truncate() to loop indefinitely waiting for page refcounts to drop?


[Severity: Critical]
This is a pre-existing issue, but is there a data race on
mirror->enc_context_owner during SEV VM migration?

Looking at sev_migrate_from(), it updates mirror->enc_context_owner without
synchronization with concurrent readers. If a mirror VM is destroyed via
sev_vm_destroy() concurrently with its owner being migrated, it can read
the old owner_kvm pointer locklessly:

arch/x86/kvm/svm/sev.c:sev_vm_destroy() {
...
		struct kvm *owner_kvm = sev->enc_context_owner;

		mutex_lock(&owner_kvm->lock);
		list_del(&sev->mirror_entry);
		mutex_unlock(&owner_kvm->lock);
		kvm_put_kvm(owner_kvm);
...
}

Could this block on the old source KVM lock while the migration thread moves
the mirror to the new destination KVM list?

When the migration thread releases the lock, sev_vm_destroy() would wake up,
attempt to delete the mirror from the list (corrupting the destination KVM
list), and incorrectly drop a second reference to the source KVM, causing a
use-after-free regression.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701144543.39582-1-pankaj.gupta@amd.com?part=1

^ permalink raw reply	[flat|nested] 10+ 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 14:58 ` sashiko-bot
@ 2026-07-01 16:25 ` David Hildenbrand (Arm)
  2026-07-01 16:30   ` Sean Christopherson
  2026-07-06 12:03   ` Gupta, Pankaj
  1 sibling, 2 replies; 10+ 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] 10+ 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; 10+ 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, &region->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,
+                                              &region->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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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
  0 siblings, 1 reply; 10+ 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, 
&region->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] 10+ 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
  0 siblings, 0 replies; 10+ 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,
> &region->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] 10+ messages in thread

end of thread, other threads:[~2026-07-07 13:59 UTC | newest]

Thread overview: 10+ 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 14:58 ` sashiko-bot
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.