* [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full [not found] <20261110110724.3614412-1-fuad.tabba@linux.dev> @ 2026-11-10 11:07 ` Fuad Tabba 2026-11-10 14:22 ` Lorenzo Stoakes (ARM) 2026-11-10 15:48 ` Sean Christopherson 0 siblings, 2 replies; 5+ messages in thread From: Fuad Tabba @ 2026-11-10 11:07 UTC (permalink / raw) To: Paolo Bonzini, Sean Christopherson Cc: kvm, linux-kselftest, linux-doc, linux-kernel, Jonathan Corbet, Shuah Khan, Isaku Yamahata, Rick Edgecombe, Lorenzo Stoakes, Marc Zyngier, Oliver Upton, kvmarm, Wei-Lin Chang, Will Deacon, Fuad Tabba, stable Pre-faulting can map a dirty-logged page writable, which pushes it to the vCPU's dirty ring. KVM_RUN exits with KVM_EXIT_DIRTY_RING_FULL before the ring overflows, but the pre-fault loop never checks it, so one call over more such pages than the ring has free trips the WARN in kvm_dirty_ring_push() and overwrites entries userspace hasn't harvested. On x86: WARNING: arch/x86/kvm/../../../virt/kvm/dirty_ring.c:225 at kvm_dirty_ring_push+0x7a/0xd0 Call Trace: make_spte+0x2cc/0x450 kvm_tdp_mmu_map+0x5e1/0x7f0 kvm_tdp_page_fault+0x12c/0x1b0 kvm_mmu_do_page_fault+0x181/0x250 kvm_arch_vcpu_pre_fault_memory+0x136/0x1d0 kvm_vcpu_pre_fault_memory+0xcf/0x180 kvm_vcpu_ioctl+0x455/0x770 Stop the loop once the ring is soft full, returning the pages already processed, or -ENOSPC when there are none, since a retry can't succeed until userspace harvests the ring. kvm_dirty_ring_check_request() fills in a KVM_RUN exit, so test the ring's state directly. The stub for !HAVE_KVM_DIRTY_RING is for s390, which has the ioctl but no ring. Fixes: bc1a5cd002116 ("KVM: Add KVM_PRE_FAULT_MEMORY vcpu ioctl to pre-populate guest memory") Cc: stable@vger.kernel.org Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev> --- Documentation/virt/kvm/api.rst | 9 +++++++-- include/linux/kvm_dirty_ring.h | 6 ++++++ virt/kvm/dirty_ring.c | 2 +- virt/kvm/kvm_main.c | 6 ++++++ 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index e95c9cda085d9..89eb6be9bc6e3 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -6544,6 +6544,7 @@ Errors: ENOENT The specified `gpa` is outside defined memslots. ENOEXEC The vCPU has not been initialised (arm64). EINTR An unmasked signal is pending and no page was processed. + ENOSPC The vCPU's dirty ring is full and no page was processed. EFAULT The parameter address was invalid. EHWPOISON A poisoned host page was encountered. EOPNOTSUPP Mapping memory for a GPA is unsupported by the @@ -6588,7 +6589,9 @@ case, the ioctl can be called in parallel. When the ioctl returns, the input values are updated to point to the remaining range. If `size` > 0 on return, the caller can just issue -the ioctl again with the same `struct kvm_map_memory` argument. +the ioctl again with the same `struct kvm_pre_fault_memory` argument. +If that fails with ENOSPC, the vCPU's dirty ring has to be harvested +and reset first. On x86, shadow page tables cannot support this ioctl because they are indexed by virtual address or nested guest physical address. @@ -8957,7 +8960,9 @@ Therefore, the ioctl must be called *before* reading the content of the dirty pages. The dirty ring can get full. When it happens, the KVM_RUN of the -vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL. +vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL, and +KVM_PRE_FAULT_MEMORY on the vcpu will process no more pages until the +ring has been harvested and reset. The dirty ring interface has a major difference comparing to the KVM_GET_DIRTY_LOG interface in that, when reading the dirty ring from diff --git a/include/linux/kvm_dirty_ring.h b/include/linux/kvm_dirty_ring.h index eb10d87adf7d5..88b4f2a009274 100644 --- a/include/linux/kvm_dirty_ring.h +++ b/include/linux/kvm_dirty_ring.h @@ -60,6 +60,11 @@ static inline void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, { } +static inline bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring) +{ + return false; +} + static inline struct page *kvm_dirty_ring_get_page(struct kvm_dirty_ring *ring, u32 offset) { @@ -81,6 +86,7 @@ int kvm_dirty_ring_alloc(struct kvm *kvm, struct kvm_dirty_ring *ring, int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring, int *nr_entries_reset); void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, u32 slot, u64 offset); +bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring); bool kvm_dirty_ring_check_request(struct kvm_vcpu *vcpu); diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c index 572b854edf740..f73f92102cb7a 100644 --- a/virt/kvm/dirty_ring.c +++ b/virt/kvm/dirty_ring.c @@ -40,7 +40,7 @@ static u32 kvm_dirty_ring_used(struct kvm_dirty_ring *ring) return READ_ONCE(ring->dirty_index) - READ_ONCE(ring->reset_index); } -static bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring) +bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring) { return kvm_dirty_ring_used(ring) >= ring->soft_limit; } diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index cf4cb586f1407..da1b76ceb65e3 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, break; } + /* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */ + if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) { + r = -ENOSPC; + break; + } + r = kvm_arch_vcpu_pre_fault_memory(vcpu, range); if (WARN_ON_ONCE(r == 0 || r == -EIO)) break; -- 2.47.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full 2026-11-10 11:07 ` [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full Fuad Tabba @ 2026-11-10 14:22 ` Lorenzo Stoakes (ARM) 2026-11-10 15:48 ` Sean Christopherson 1 sibling, 0 replies; 5+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-11-10 14:22 UTC (permalink / raw) To: Fuad Tabba Cc: Paolo Bonzini, Sean Christopherson, kvm, linux-kselftest, linux-doc, linux-kernel, Jonathan Corbet, Shuah Khan, Isaku Yamahata, Rick Edgecombe, Marc Zyngier, Oliver Upton, kvmarm, Wei-Lin Chang, Will Deacon, Fuad Tabba, stable On Tue, Nov 10, 2026 at 11:07:23AM +0000, Fuad Tabba wrote: > Pre-faulting can map a dirty-logged page writable, which pushes it to > the vCPU's dirty ring. KVM_RUN exits with KVM_EXIT_DIRTY_RING_FULL > before the ring overflows, but the pre-fault loop never checks it, so > one call over more such pages than the ring has free trips the WARN in > kvm_dirty_ring_push() and overwrites entries userspace hasn't > harvested. On x86: > > WARNING: arch/x86/kvm/../../../virt/kvm/dirty_ring.c:225 at kvm_dirty_ring_push+0x7a/0xd0 > Call Trace: > make_spte+0x2cc/0x450 > kvm_tdp_mmu_map+0x5e1/0x7f0 > kvm_tdp_page_fault+0x12c/0x1b0 > kvm_mmu_do_page_fault+0x181/0x250 > kvm_arch_vcpu_pre_fault_memory+0x136/0x1d0 > kvm_vcpu_pre_fault_memory+0xcf/0x180 > kvm_vcpu_ioctl+0x455/0x770 If you have a local repro maybe worth sending a regression test for this?... and I type that, then check the 2/2 and see that you already did :) ok haha thanks! > > Stop the loop once the ring is soft full, returning the pages already > processed, or -ENOSPC when there are none, since a retry can't succeed > until userspace harvests the ring. kvm_dirty_ring_check_request() fills > in a KVM_RUN exit, so test the ring's state directly. The stub for > !HAVE_KVM_DIRTY_RING is for s390, which has the ioctl but no ring. > > Fixes: bc1a5cd002116 ("KVM: Add KVM_PRE_FAULT_MEMORY vcpu ioctl to pre-populate guest memory") > Cc: stable@vger.kernel.org > Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev> Yikes, nasty. Thanks for addressing this! LGTM so: Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org> > --- > Documentation/virt/kvm/api.rst | 9 +++++++-- > include/linux/kvm_dirty_ring.h | 6 ++++++ > virt/kvm/dirty_ring.c | 2 +- > virt/kvm/kvm_main.c | 6 ++++++ > 4 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index e95c9cda085d9..89eb6be9bc6e3 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -6544,6 +6544,7 @@ Errors: > ENOENT The specified `gpa` is outside defined memslots. > ENOEXEC The vCPU has not been initialised (arm64). > EINTR An unmasked signal is pending and no page was processed. > + ENOSPC The vCPU's dirty ring is full and no page was processed. > EFAULT The parameter address was invalid. > EHWPOISON A poisoned host page was encountered. > EOPNOTSUPP Mapping memory for a GPA is unsupported by the > @@ -6588,7 +6589,9 @@ case, the ioctl can be called in parallel. > > When the ioctl returns, the input values are updated to point to the > remaining range. If `size` > 0 on return, the caller can just issue > -the ioctl again with the same `struct kvm_map_memory` argument. > +the ioctl again with the same `struct kvm_pre_fault_memory` argument. > +If that fails with ENOSPC, the vCPU's dirty ring has to be harvested > +and reset first. > > On x86, shadow page tables cannot support this ioctl because they > are indexed by virtual address or nested guest physical address. > @@ -8957,7 +8960,9 @@ Therefore, the ioctl must be called *before* reading the content of > the dirty pages. > > The dirty ring can get full. When it happens, the KVM_RUN of the > -vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL. > +vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL, and > +KVM_PRE_FAULT_MEMORY on the vcpu will process no more pages until the > +ring has been harvested and reset. > > The dirty ring interface has a major difference comparing to the > KVM_GET_DIRTY_LOG interface in that, when reading the dirty ring from > diff --git a/include/linux/kvm_dirty_ring.h b/include/linux/kvm_dirty_ring.h > index eb10d87adf7d5..88b4f2a009274 100644 > --- a/include/linux/kvm_dirty_ring.h > +++ b/include/linux/kvm_dirty_ring.h > @@ -60,6 +60,11 @@ static inline void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, > { > } > > +static inline bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring) > +{ > + return false; > +} > + > static inline struct page *kvm_dirty_ring_get_page(struct kvm_dirty_ring *ring, > u32 offset) > { > @@ -81,6 +86,7 @@ int kvm_dirty_ring_alloc(struct kvm *kvm, struct kvm_dirty_ring *ring, > int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring, > int *nr_entries_reset); > void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, u32 slot, u64 offset); > +bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring); > > bool kvm_dirty_ring_check_request(struct kvm_vcpu *vcpu); > > diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c > index 572b854edf740..f73f92102cb7a 100644 > --- a/virt/kvm/dirty_ring.c > +++ b/virt/kvm/dirty_ring.c > @@ -40,7 +40,7 @@ static u32 kvm_dirty_ring_used(struct kvm_dirty_ring *ring) > return READ_ONCE(ring->dirty_index) - READ_ONCE(ring->reset_index); > } > > -static bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring) > +bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring) > { > return kvm_dirty_ring_used(ring) >= ring->soft_limit; > } > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index cf4cb586f1407..da1b76ceb65e3 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, > break; > } > > + /* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */ > + if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) { > + r = -ENOSPC; > + break; > + } > + > r = kvm_arch_vcpu_pre_fault_memory(vcpu, range); > if (WARN_ON_ONCE(r == 0 || r == -EIO)) > break; > -- > 2.47.3 > -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full 2026-11-10 11:07 ` [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full Fuad Tabba 2026-11-10 14:22 ` Lorenzo Stoakes (ARM) @ 2026-11-10 15:48 ` Sean Christopherson 2026-11-10 17:18 ` Fuad Tabba 1 sibling, 1 reply; 5+ messages in thread From: Sean Christopherson @ 2026-11-10 15:48 UTC (permalink / raw) To: Fuad Tabba Cc: Paolo Bonzini, kvm, linux-kselftest, linux-doc, linux-kernel, Jonathan Corbet, Shuah Khan, Isaku Yamahata, Rick Edgecombe, Lorenzo Stoakes, Marc Zyngier, Oliver Upton, kvmarm, Wei-Lin Chang, Will Deacon, Fuad Tabba, stable On Tue, Nov 10, 2026, Fuad Tabba wrote: > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index cf4cb586f1407..da1b76ceb65e3 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, > break; > } > > + /* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */ Please don't include AI comments verbatim. > + if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) { Eh, I vote to just use kvm_dirty_ring_check_request(), I don't see any harm in clobbering vcpu->run->exit_reason. Side topic, kvm_dirty_ring_check_request() is rather confusing (clears and then re-sets the requests). AFAICT, it can be this instead: lockdep_assert_held(vcpu->mutex); /* * The vCPU isn't runnable when the dirty ring becomes soft full. Keep * the requests set until the dirty pages are harvested and the dirty * ring is reset by userspace. Note, checking if the ring is soft-full * can get a false positive, but not a false negative, because only the * vCPU can add entries to the ring, and its mutex is held. */ if (kvm_test_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu) && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) { vcpu->run->exit_reason = KVM_EXIT_DIRTY_RING_FULL; trace_kvm_dirty_ring_exit(vcpu); return true; } kvm_clear_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu); return false; > + r = -ENOSPC; > + break; > + } > + > r = kvm_arch_vcpu_pre_fault_memory(vcpu, range); > if (WARN_ON_ONCE(r == 0 || r == -EIO)) > break; > -- > 2.47.3 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full 2026-11-10 15:48 ` Sean Christopherson @ 2026-11-10 17:18 ` Fuad Tabba 2026-11-10 18:35 ` Sean Christopherson 0 siblings, 1 reply; 5+ messages in thread From: Fuad Tabba @ 2026-11-10 17:18 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, kvm, linux-kselftest, linux-doc, linux-kernel, Jonathan Corbet, Shuah Khan, Isaku Yamahata, Rick Edgecombe, Lorenzo Stoakes, Marc Zyngier, Oliver Upton, kvmarm, Wei-Lin Chang, Will Deacon, stable Hi Sean, On Tue, 10 Nov 2026 15:48:37 +0000, Sean Christopherson <seanjc@google.com> wrote: > On Tue, Nov 10, 2026, Fuad Tabba wrote: > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c [...] > > @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, > > break; > > } > > > > + /* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */ > > Please don't include AI comments verbatim. The series is LLM-assisted, but the comment was actually me. > > + if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) { > > Eh, I vote to just use kvm_dirty_ring_check_request(), I don't see any harm in > clobbering vcpu->run->exit_reason. Will do. > Side topic, kvm_dirty_ring_check_request() is rather confusing (clears and then > re-sets the requests). AFAICT, it can be this instead: > > lockdep_assert_held(vcpu->mutex); > > /* > * The vCPU isn't runnable when the dirty ring becomes soft full. Keep > * the requests set until the dirty pages are harvested and the dirty > * ring is reset by userspace. Note, checking if the ring is soft-full > * can get a false positive, but not a false negative, because only the > * vCPU can add entries to the ring, and its mutex is held. > */ > if (kvm_test_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu) && > kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) { > vcpu->run->exit_reason = KVM_EXIT_DIRTY_RING_FULL; > trace_kvm_dirty_ring_exit(vcpu); > return true; > } > > kvm_clear_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu); > return false; I'll send it as its own patch. Cheers, /fuad ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full 2026-11-10 17:18 ` Fuad Tabba @ 2026-11-10 18:35 ` Sean Christopherson 0 siblings, 0 replies; 5+ messages in thread From: Sean Christopherson @ 2026-11-10 18:35 UTC (permalink / raw) To: Fuad Tabba Cc: Paolo Bonzini, kvm, linux-kselftest, linux-doc, linux-kernel, Jonathan Corbet, Shuah Khan, Isaku Yamahata, Rick Edgecombe, Lorenzo Stoakes, Marc Zyngier, Oliver Upton, kvmarm, Wei-Lin Chang, Will Deacon, stable On Tue, Nov 10, 2026, Fuad Tabba wrote: > Hi Sean, > > On Tue, 10 Nov 2026 15:48:37 +0000, Sean Christopherson > <seanjc@google.com> wrote: > > On Tue, Nov 10, 2026, Fuad Tabba wrote: > > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > [...] > > > @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu, > > > break; > > > } > > > > > > + /* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */ > > > > Please don't include AI comments verbatim. > > The series is LLM-assisted, but the comment was actually me. LOL, I think all of us are getting a bit corrupted. :-) ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-10 18:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20261110110724.3614412-1-fuad.tabba@linux.dev>
2026-11-10 11:07 ` [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full Fuad Tabba
2026-11-10 14:22 ` Lorenzo Stoakes (ARM)
2026-11-10 15:48 ` Sean Christopherson
2026-11-10 17:18 ` Fuad Tabba
2026-11-10 18:35 ` Sean Christopherson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox