Re: [PATCH 3/3] KVM: arm64: nv: Punt stage-2 recycling to a vCPU request

[Marc Zyngier <maz@kernel.org>]

On Tue, 01 Oct 2024 01:17:09 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Currently, when a nested MMU is repurposed for some other MMU context,
> KVM unmaps everything during vcpu_load() while holding the MMU lock for
> write. This is quite a performance bottleneck for large nested VMs, as
> all vCPU scheduling will spin until the unmap completes.
> 
> Start punting the MMU cleanup to a vCPU request, where it is then
> possible to periodically release the MMU lock and CPU in the presence of
> contention. To make that possible, take a pin on the hardware MMU while
> unmapping it to guarantee the responsible vCPU finds the MMU on the next
> vcpu_load().
> 
> Ensure that no vCPU winds up using a stale MMU by tracking the pending
> unmap on the S2 MMU itself and requesting an unmap on every vCPU that
> finds it.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/include/asm/kvm_host.h   |  3 +++
>  arch/arm64/include/asm/kvm_nested.h |  2 ++
>  arch/arm64/kvm/arm.c                |  2 ++
>  arch/arm64/kvm/nested.c             | 21 +++++++++++++++++++--
>  4 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 268f69196380..1d310292c623 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -51,6 +51,7 @@
>  #define KVM_REQ_RELOAD_PMU	KVM_ARCH_REQ(5)
>  #define KVM_REQ_SUSPEND		KVM_ARCH_REQ(6)
>  #define KVM_REQ_RESYNC_PMU_EL0	KVM_ARCH_REQ(7)
> +#define KVM_REQ_NESTED_S2_UNMAP	KVM_ARCH_REQ(8)
>  
>  #define KVM_DIRTY_LOG_MANUAL_CAPS   (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE | \
>  				     KVM_DIRTY_LOG_INITIALLY_SET)
> @@ -216,6 +217,8 @@ struct kvm_s2_mmu {
>  	 * >0: Somebody is actively using this.
>  	 */
>  	atomic_t refcnt;
> +
> +	bool	pending_unmap;
>  };
>  
>  struct kvm_arch_memory_slot {
> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> index e74b90dcfac4..233e65522716 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
> @@ -78,6 +78,8 @@ extern void kvm_s2_mmu_iterate_by_vmid(struct kvm *kvm, u16 vmid,
>  extern void kvm_vcpu_load_hw_mmu(struct kvm_vcpu *vcpu);
>  extern void kvm_vcpu_put_hw_mmu(struct kvm_vcpu *vcpu);
>  
> +extern void check_nested_vcpu_requests(struct kvm_vcpu *vcpu);
> +
>  struct kvm_s2_trans {
>  	phys_addr_t output;
>  	unsigned long block_size;
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 367f0d3d3194..ab7d387379a0 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1031,6 +1031,8 @@ static int check_vcpu_requests(struct kvm_vcpu *vcpu)
>  
>  		if (kvm_dirty_ring_check_request(vcpu))
>  			return 0;
> +
> +		check_nested_vcpu_requests(vcpu);
>  	}
>  
>  	return 1;
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 1ba211541290..749babdbda81 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -632,9 +632,9 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
>  	/* Set the scene for the next search */
>  	kvm->arch.nested_mmus_next = (i + 1) % kvm->arch.nested_mmus_size;
>  
> -	/* Clear the old state */
> +	/* Make sure we don't forget to do the laundry */
>  	if (kvm_s2_mmu_valid(s2_mmu))
> -		kvm_stage2_unmap_range(s2_mmu, 0, kvm_phys_size(s2_mmu), false);
> +		s2_mmu->pending_unmap = true;
>  
>  	/*
>  	 * The virtual VMID (modulo CnP) will be used as a key when matching
> @@ -649,6 +649,9 @@ static struct kvm_s2_mmu *get_s2_mmu_nested(struct kvm_vcpu *vcpu)
>  	s2_mmu->nested_stage2_enabled = vcpu_read_sys_reg(vcpu, HCR_EL2) & HCR_VM;
>  
>  out:
> +	if (s2_mmu->pending_unmap)
> +		kvm_make_request(KVM_REQ_NESTED_S2_UNMAP, vcpu);
> +
>  	kvm_get_s2_mmu(s2_mmu);
>  	return s2_mmu;
>  }
> @@ -1186,3 +1189,17 @@ int kvm_init_nv_sysregs(struct kvm *kvm)
>  
>  	return 0;
>  }
> +
> +void check_nested_vcpu_requests(struct kvm_vcpu *vcpu)
> +{
> +	if (kvm_check_request(KVM_REQ_NESTED_S2_UNMAP, vcpu)) {
> +		CLASS(vcpu_hw_mmu, mmu)(vcpu);

I really think a small comment outlining *why* we need to elevate the
refcount would help. This single line is what holds the whole thing
together.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.