| Message ID | 1736498887-28180-1-git-send-email-yangge1116@126.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: SEV: Pin SEV guest memory out of CMA area | expand |
On 10.01.25 09:48, yangge1116@126.com wrote: > From: yangge <yangge1116@126.com> I would suggest the title to be something like "KVM: SEV: fix wrong pinning of pages" Then you can describe that without FOLL_LONGTERM, the pages will not get migrated out of MIGRATE_CMA/ZONE_MOVABLE, violating these mechanisms to avoid fragmentation with unmovable pages, for example making CMA allocations fail. (CMA allocations failing is only one symptom of the missed usage of FOLL_LONGTERM) > > When pin_user_pages_fast() pins SEV guest memory without the > FOLL_LONGTERM flag, the pinned pages may inadvertently end up in the > CMA (Contiguous Memory Allocator) area. This can subsequently cause > cma_alloc() to fail in allocating these pages, due to the fact that > the pinned pages are not migratable. > > To address the aforementioned problem, we propose adding the > FOLL_LONGTERM flag to the pin_user_pages_fast() function. By doing > so, we ensure that the pages allocated will not occupy space within > the CMA area, thereby preventing potential allocation failures. > > Signed-off-by: yangge <yangge1116@126.com> > --- > arch/x86/kvm/svm/sev.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 943bd07..35d0714 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -630,6 +630,7 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, > unsigned long locked, lock_limit; > struct page **pages; > unsigned long first, last; > + unsigned int flags = 0; Why not set unsigned int flags = FOLL_LONGTERM; > int ret; > > lockdep_assert_held(&kvm->lock); > @@ -662,8 +663,10 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, > if (!pages) > return ERR_PTR(-ENOMEM); > > + flags = write ? FOLL_WRITE : 0; and here do flags |= write ? FOLL_WRITE : 0; > + > /* Pin the user virtual address. */ > - npinned = pin_user_pages_fast(uaddr, npages, write ? FOLL_WRITE : 0, pages); > + npinned = pin_user_pages_fast(uaddr, npages, flags | FOLL_LONGTERM, pages); > if (npinned != npages) { > pr_err("SEV: Failure locking %lu pages.\n", npages); > ret = -ENOMEM;
在 2025/1/10 16:58, David Hildenbrand 写道: > On 10.01.25 09:48, yangge1116@126.com wrote: >> From: yangge <yangge1116@126.com> > > I would suggest the title to be something like > > "KVM: SEV: fix wrong pinning of pages" > > Then you can describe that without FOLL_LONGTERM, the pages will not get > migrated out of MIGRATE_CMA/ZONE_MOVABLE, violating these mechanisms to > avoid fragmentation with unmovable pages, for example making CMA > allocations fail. > > (CMA allocations failing is only one symptom of the missed usage of > FOLL_LONGTERM) > >> >> When pin_user_pages_fast() pins SEV guest memory without the >> FOLL_LONGTERM flag, the pinned pages may inadvertently end up in the >> CMA (Contiguous Memory Allocator) area. This can subsequently cause >> cma_alloc() to fail in allocating these pages, due to the fact that >> the pinned pages are not migratable. >> >> To address the aforementioned problem, we propose adding the >> FOLL_LONGTERM flag to the pin_user_pages_fast() function. By doing >> so, we ensure that the pages allocated will not occupy space within >> the CMA area, thereby preventing potential allocation failures. >> >> Signed-off-by: yangge <yangge1116@126.com> >> --- >> arch/x86/kvm/svm/sev.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >> index 943bd07..35d0714 100644 >> --- a/arch/x86/kvm/svm/sev.c >> +++ b/arch/x86/kvm/svm/sev.c >> @@ -630,6 +630,7 @@ static struct page **sev_pin_memory(struct kvm >> *kvm, unsigned long uaddr, >> unsigned long locked, lock_limit; >> struct page **pages; >> unsigned long first, last; >> + unsigned int flags = 0; > > Why not set > > unsigned int flags = FOLL_LONGTERM; > >> int ret; >> lockdep_assert_held(&kvm->lock); >> @@ -662,8 +663,10 @@ static struct page **sev_pin_memory(struct kvm >> *kvm, unsigned long uaddr, >> if (!pages) >> return ERR_PTR(-ENOMEM); >> + flags = write ? FOLL_WRITE : 0; > > > and here do > > flags |= write ? FOLL_WRITE : 0; > >> + >> /* Pin the user virtual address. */ >> - npinned = pin_user_pages_fast(uaddr, npages, write ? FOLL_WRITE : >> 0, pages); >> + npinned = pin_user_pages_fast(uaddr, npages, flags | >> FOLL_LONGTERM, pages); >> if (npinned != npages) { >> pr_err("SEV: Failure locking %lu pages.\n", npages); >> ret = -ENOMEM; > > Ok, thanks.
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 943bd07..35d0714 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -630,6 +630,7 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, unsigned long locked, lock_limit; struct page **pages; unsigned long first, last; + unsigned int flags = 0; int ret; lockdep_assert_held(&kvm->lock); @@ -662,8 +663,10 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr, if (!pages) return ERR_PTR(-ENOMEM); + flags = write ? FOLL_WRITE : 0; + /* Pin the user virtual address. */ - npinned = pin_user_pages_fast(uaddr, npages, write ? FOLL_WRITE : 0, pages); + npinned = pin_user_pages_fast(uaddr, npages, flags | FOLL_LONGTERM, pages); if (npinned != npages) { pr_err("SEV: Failure locking %lu pages.\n", npages); ret = -ENOMEM;