Re: [RFC] KVM: arm64: Don't force broadcast tlbi when guest is running

From: Marc Zyngier <maz@kernel.org>
To: Shaokun Zhang <zhangshaokun@hisilicon.com>
Cc: kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC] KVM: arm64: Don't force broadcast tlbi when guest is running
Date: Thu, 22 Oct 2020 13:03:10 +0100	[thread overview]
Message-ID: <be1d84d42df6203ad993e8d20d9bd6da@kernel.org> (raw)
In-Reply-To: <1603331829-33879-1-git-send-email-zhangshaokun@hisilicon.com>

On 2020-10-22 02:57, Shaokun Zhang wrote:
> From: Nianyao Tang <tangnianyao@huawei.com>
> 
> Now HCR_EL2.FB is set to force broadcast tlbi to all online pcpus,
> even those vcpu do not resident on. It would get worse as system
> gets larger and more pcpus get online.
> Let's disable force-broadcast. We flush tlbi when move vcpu to a
> new pcpu, in case old page mapping still exists on new pcpu.
> 
> Cc: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Nianyao Tang <tangnianyao@huawei.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
>  arch/arm64/include/asm/kvm_arm.h | 2 +-
>  arch/arm64/kvm/arm.c             | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h 
> b/arch/arm64/include/asm/kvm_arm.h
> index 64ce29378467..f85ea9c649cb 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -75,7 +75,7 @@
>   * PTW:		Take a stage2 fault if a stage1 walk steps in device memory
>   */
>  #define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | 
> HCR_VM | \
> -			 HCR_BSU_IS | HCR_FB | HCR_TAC | \
> +			 HCR_BSU_IS | HCR_TAC | \
>  			 HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
>  			 HCR_FMO | HCR_IMO | HCR_PTW )
>  #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index acf9a993dfb6..845be911f885 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -334,8 +334,10 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int 
> cpu)
>  	/*
>  	 * We might get preempted before the vCPU actually runs, but
>  	 * over-invalidation doesn't affect correctness.
> +	 * Dirty tlb might still exist when vcpu ran on other pcpu
> +	 * and modified page mapping.
>  	 */
> -	if (*last_ran != vcpu->vcpu_id) {
> +	if (*last_ran != vcpu->vcpu_id || vcpu->cpu != cpu) {
>  		kvm_call_hyp(__kvm_tlb_flush_local_vmid, mmu);
>  		*last_ran = vcpu->vcpu_id;
>  	}

This breaks uniprocessor semantics for I-cache invalidation. What could
possibly go wrong? You also fail to provide any data that would back up
your claim, as usual.

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm