Re: [PATCH v2 4/4] KVM: arm64: nv: Clarify safety of allowing TLBI unmaps to reschedule

[Sean Christopherson <seanjc@google.com>]

On Mon, Oct 07, 2024, Oliver Upton wrote:
> There's been a decent amount of attention around unmaps of nested MMUs,
> and TLBI handling is no exception to this. Add a comment clarifying why
> it is safe to reschedule during a TLBI unmap, even without a reference
> on the MMU in progress.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/sys_regs.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e0c7cc16466e..b773d107cd35 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2955,6 +2955,29 @@ union tlbi_info {
>  static void s2_mmu_unmap_range(struct kvm_s2_mmu *mmu,
>  			       const union tlbi_info *info)
>  {
> +	/*
> +	 * The unmap operation is allowed to drop the MMU lock and block, which
> +	 * means that @mmu could be used for a different context than the one
> +	 * currently being invalidated.
> +	 *
> +	 * This behavior is still safe, as:
> +	 *
> +	 *  1) The vCPU that recycled the MMU is responsible for invalidating
> +	 *     the entire MMU before reusing it, which still honors the intent
> +	 *     of a TLBI.

I think it's worth calling out that other vCPUs that want to reuse the "new" MMU
will also ensure the entire MMU is invalidated before reusing it, and maybe even
that multiple vCPUs can participate in recyling an MMU.  The "The vCPU" wording
is probably going to be interpreted by most readers as meaning only _one_ vCPU
will ensure the MMU is invalidated.

> +	 *  2) Until the guest TLBI instruction is 'retired' (i.e. increment PC
> +	 *     and ERET to the guest), other vCPUs are allowed to use stale
> +	 *     translations.
> +	 *
> +	 *  3) Accidentally unmapping an unrelated MMU context is nonfatal, and
> +	 *     at worst may cause more aborts for shadow stage-2 fills.
> +	 *
> +	 * Dropping the MMU lock also implies that shadow stage-2 fills could
> +	 * happen behind the back of the TLBI. This is still safe, though, as
> +	 * the L1 needs to put its stage-2 in a consistent state before doing
> +	 * the TLBI.
> +	 */
>  	kvm_stage2_unmap_range(mmu, info->range.start, info->range.size, true);
>  }
>  
> @@ -3050,6 +3073,10 @@ static void s2_mmu_unmap_ipa(struct kvm_s2_mmu *mmu,
>  	max_size = compute_tlb_inval_range(mmu, info->ipa.addr);
>  	base_addr &= ~(max_size - 1);
>  
> +	/*
> +	 * See comment in s2_mmu_unmap_range() for why this is allowed to
> +	 * reschedule.
> +	 */
>  	kvm_stage2_unmap_range(mmu, base_addr, max_size, true);
>  }
>  
> -- 
> 2.47.0.rc0.187.ge670bccf7e-goog
>