| Message ID | 20240821202814.711673-1-dwmw2@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/5] KVM: pfncache: Add needs_invalidation flag to gfn_to_pfn_cache | expand |
On 21 August 2024 22:28:09 CEST, David Woodhouse <dwmw2@infradead.org> wrote: >From: David Woodhouse <dwmw@amazon.co.uk> > >This will be used to allow hva_to_pfn_retry() to be more selective about >its retry loop, which is currently extremely pessimistic. > >It allows for invalidations to occur even while the PFN is being mapped >(which happens with the lock dropped), before the GPC is fully valid. > >No functional change yet, as the existing mmu_notifier_retry_cache() >function will still return true in all cases where the invalidation >may have triggered. > >Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> >--- > include/linux/kvm_types.h | 1 + > virt/kvm/pfncache.c | 29 ++++++++++++++++++++++++----- > 2 files changed, 25 insertions(+), 5 deletions(-) > >diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h >index 827ecc0b7e10..4d8fbd87c320 100644 >--- a/include/linux/kvm_types.h >+++ b/include/linux/kvm_types.h >@@ -69,6 +69,7 @@ struct gfn_to_pfn_cache { > void *khva; > kvm_pfn_t pfn; > bool active; >+ bool needs_invalidation; > bool valid; > }; > >diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c >index f0039efb9e1e..7007d32d197a 100644 >--- a/virt/kvm/pfncache.c >+++ b/virt/kvm/pfncache.c >@@ -32,7 +32,7 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start, > read_lock_irq(&gpc->lock); > > /* Only a single page so no need to care about length */ >- if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) && >+ if (gpc->needs_invalidation && !is_error_noslot_pfn(gpc->pfn) && > gpc->uhva >= start && gpc->uhva < end) { > read_unlock_irq(&gpc->lock); > >@@ -45,9 +45,11 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start, > */ > > write_lock_irq(&gpc->lock); >- if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) && >- gpc->uhva >= start && gpc->uhva < end) >+ if (gpc->needs_invalidation && !is_error_noslot_pfn(gpc->pfn) && >+ gpc->uhva >= start && gpc->uhva < end) { >+ gpc->needs_invalidation = false; > gpc->valid = false; >+ } > write_unlock_irq(&gpc->lock); > continue; > } >@@ -93,6 +95,9 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len) > if (!gpc->valid) > return false; > >+ /* If it's valid, it needs invalidation! */ >+ WARN_ON_ONCE(!gpc->needs_invalidation); >+ > return true; > } > >@@ -175,6 +180,17 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc) > mmu_seq = gpc->kvm->mmu_invalidate_seq; > smp_rmb(); > >+ /* >+ * The translation made by hva_to_pfn() below could be made >+ * invalid as soon as it's mapped. But the uhva is already >+ * known and that's all that gfn_to_pfn_cache_invalidate() >+ * looks at. So set the 'needs_invalidation' flag to allow >+ * the GPC to be marked invalid from the moment the lock is >+ * dropped, before the corresponding PFN is even found (and, >+ * more to the point, immediately afterwards). >+ */ >+ gpc->needs_invalidation = true; >+ > write_unlock_irq(&gpc->lock); > > /* >@@ -224,7 +240,8 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc) > * attempting to refresh. > */ > WARN_ON_ONCE(gpc->valid); >- } while (mmu_notifier_retry_cache(gpc->kvm, mmu_seq)); >+ } while (!gpc->needs_invalidation || >+ mmu_notifier_retry_cache(gpc->kvm, mmu_seq)); > > gpc->valid = true; > gpc->pfn = new_pfn; >@@ -339,6 +356,7 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l > */ > if (ret) { > gpc->valid = false; >+ gpc->needs_invalidation = false; > gpc->pfn = KVM_PFN_ERR_FAULT; > gpc->khva = NULL; > } >@@ -383,7 +401,7 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm) > gpc->pfn = KVM_PFN_ERR_FAULT; > gpc->gpa = INVALID_GPA; > gpc->uhva = KVM_HVA_ERR_BAD; >- gpc->active = gpc->valid = false; >+ gpc->active = gpc->valid = gpc->needs_invalidation = false; > } > > static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva, >@@ -453,6 +471,7 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc) > write_lock_irq(&gpc->lock); > gpc->active = false; > gpc->valid = false; >+ gpc->needs_invalidation = false; > > /* > * Leave the GPA => uHVA cache intact, it's protected by the Ping?
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h index 827ecc0b7e10..4d8fbd87c320 100644 --- a/include/linux/kvm_types.h +++ b/include/linux/kvm_types.h @@ -69,6 +69,7 @@ struct gfn_to_pfn_cache { void *khva; kvm_pfn_t pfn; bool active; + bool needs_invalidation; bool valid; }; diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c index f0039efb9e1e..7007d32d197a 100644 --- a/virt/kvm/pfncache.c +++ b/virt/kvm/pfncache.c @@ -32,7 +32,7 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start, read_lock_irq(&gpc->lock); /* Only a single page so no need to care about length */ - if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) && + if (gpc->needs_invalidation && !is_error_noslot_pfn(gpc->pfn) && gpc->uhva >= start && gpc->uhva < end) { read_unlock_irq(&gpc->lock); @@ -45,9 +45,11 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start, */ write_lock_irq(&gpc->lock); - if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) && - gpc->uhva >= start && gpc->uhva < end) + if (gpc->needs_invalidation && !is_error_noslot_pfn(gpc->pfn) && + gpc->uhva >= start && gpc->uhva < end) { + gpc->needs_invalidation = false; gpc->valid = false; + } write_unlock_irq(&gpc->lock); continue; } @@ -93,6 +95,9 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len) if (!gpc->valid) return false; + /* If it's valid, it needs invalidation! */ + WARN_ON_ONCE(!gpc->needs_invalidation); + return true; } @@ -175,6 +180,17 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc) mmu_seq = gpc->kvm->mmu_invalidate_seq; smp_rmb(); + /* + * The translation made by hva_to_pfn() below could be made + * invalid as soon as it's mapped. But the uhva is already + * known and that's all that gfn_to_pfn_cache_invalidate() + * looks at. So set the 'needs_invalidation' flag to allow + * the GPC to be marked invalid from the moment the lock is + * dropped, before the corresponding PFN is even found (and, + * more to the point, immediately afterwards). + */ + gpc->needs_invalidation = true; + write_unlock_irq(&gpc->lock); /* @@ -224,7 +240,8 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc) * attempting to refresh. */ WARN_ON_ONCE(gpc->valid); - } while (mmu_notifier_retry_cache(gpc->kvm, mmu_seq)); + } while (!gpc->needs_invalidation || + mmu_notifier_retry_cache(gpc->kvm, mmu_seq)); gpc->valid = true; gpc->pfn = new_pfn; @@ -339,6 +356,7 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l */ if (ret) { gpc->valid = false; + gpc->needs_invalidation = false; gpc->pfn = KVM_PFN_ERR_FAULT; gpc->khva = NULL; } @@ -383,7 +401,7 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm) gpc->pfn = KVM_PFN_ERR_FAULT; gpc->gpa = INVALID_GPA; gpc->uhva = KVM_HVA_ERR_BAD; - gpc->active = gpc->valid = false; + gpc->active = gpc->valid = gpc->needs_invalidation = false; } static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva, @@ -453,6 +471,7 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc) write_lock_irq(&gpc->lock); gpc->active = false; gpc->valid = false; + gpc->needs_invalidation = false; /* * Leave the GPA => uHVA cache intact, it's protected by the