On Fri, Sep 22, 2023 at 10:32:29PM +0000, Oliver Upton wrote:
> It is possible for multiple vCPUs to fault on the same IPA and attempt
> to resolve the fault. One of the page table walks will actually update
> the PTE and the rest will return -EAGAIN per our race detection scheme.
> KVM elides the TLB invalidation on the racing threads as the return
> value is nonzero.
>
> Before commit a12ab1378a88 ("KVM: arm64: Use local TLBI on permission
> relaxation") KVM always used broadcast TLB invalidations when handling
> permission faults, which had the convenient property of making the
> stage-2 updates visible to all CPUs in the system. However now we do a
> local invalidation, and TLBI elision leads to vCPUs getting stuck in a
> permission fault loop. Remember that the architecture permits the TLB to
> cache translations that precipitate a permission fault.
The effects of this are slightly overstated (got ahead of myself).
EAGAIN only crops up if the cmpxchg() fails, we return 0 if the PTE
didn't need to be updated.
On the subsequent permission fault we'll do the right thing and
invalidate the TLB, so this change is purely an optimization rather than
a correctness issue.
> Invalidate the TLB entry responsible for the permission fault if the
> stage-2 descriptor has been relaxed, regardless of which thread actually
> did the job.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: a12ab1378a88 ("KVM: arm64: Use local TLBI on permission relaxation")
I'll drop the stable tag.
> Signed-off-by: Oliver Upton <oliver.upton@xxxxxxxxx>
> ---
> arch/arm64/kvm/hyp/pgtable.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
> index f155b8c9e98c..286888751793 100644
> --- a/arch/arm64/kvm/hyp/pgtable.c
> +++ b/arch/arm64/kvm/hyp/pgtable.c
> @@ -1314,7 +1314,7 @@ int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
> ret = stage2_update_leaf_attrs(pgt, addr, 1, set, clr, NULL, &level,
> KVM_PGTABLE_WALK_HANDLE_FAULT |
> KVM_PGTABLE_WALK_SHARED);
> - if (!ret)
> + if (!ret || ret == -EAGAIN)
> kvm_call_hyp(__kvm_tlb_flush_vmid_ipa_nsh, pgt->mmu, addr, level);
> return ret;
> }
>
> base-commit: ce9ecca0238b140b88f43859b211c9fdfd8e5b70
> --
> 2.42.0.515.g380fc7ccd1-goog
>
--
Thanks,
Oliver
