On 21 August 2024 22:28:09 CEST, David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote:
>From: David Woodhouse <dwmw@xxxxxxxxxxxx>
>
>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@xxxxxxxxxxxx>
>---
> 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?
