Re: [PATCH] KVM: arm64: Always invalidate TLB for stage-2 permission faults

[Oliver Upton <oliver.upton@linux.dev>]

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@vger.kernel.org
> Fixes: a12ab1378a88 ("KVM: arm64: Use local TLBI on permission relaxation")

I'll drop the stable tag.

> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  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