From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH v6 2/2] KVM: Using workqueue for WBINVD
Date: Wed, 30 Jun 2010 13:41:52 -0300
Message-ID: <20100630164152.GB1628@amt.cnet>
References: <1277871916-8348-1-git-send-email-sheng@linux.intel.com>
 <1277871916-8348-2-git-send-email-sheng@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Avi Kivity <avi@redhat.com>, Jan Kiszka <jan.kiszka@siemens.com>,
	Joerg Roedel <joerg.roedel@amd.com>, kvm@vger.kernel.org
To: Sheng Yang <sheng@linux.intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:26378 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754841Ab0F3RDL (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 30 Jun 2010 13:03:11 -0400
Content-Disposition: inline
In-Reply-To: <1277871916-8348-2-git-send-email-sheng@linux.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, Jun 30, 2010 at 12:25:16PM +0800, Sheng Yang wrote:
> It would buy us the ability to schedule compared to smp_call_function().
> 
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> ---
> 
> But I am not sure if it worth the complexity. Anyway WBINVD itself can't be
> interrupted, so the benefit should come to the caller cpu I think. And that
> would extended the waiting time for the caller cpu, for we still need to wait
> other cpus to complete their works.

Alright, can be done later in case it shows to be necessary.

>  arch/x86/include/asm/kvm_host.h |    1 +
>  arch/x86/kvm/x86.c              |   35 ++++++++++++++++++++++++++++-------
>  2 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 2bda624..6e0b793 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -16,6 +16,7 @@
>  #include <linux/mmu_notifier.h>
>  #include <linux/tracepoint.h>
>  #include <linux/cpumask.h>
> +#include <linux/workqueue.h>
>  
>  #include <linux/kvm.h>
>  #include <linux/kvm_para.h>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index eea75f5..13f7c88 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -153,6 +153,9 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>  
>  u64 __read_mostly host_xcr0;
>  
> +static struct workqueue_struct *wbinvd_wq;
> +static DEFINE_PER_CPU(struct work_struct, wbinvd_work);
> +
>  static inline u32 bit(int bitno)
>  {
>  	return 1 << (bitno & 31);
> @@ -1783,7 +1786,7 @@ out:
>  	return r;
>  }
>  
> -static void wbinvd_ipi(void *garbage)
> +static void wbinvd_do_work(struct work_struct *work)
>  {
>  	wbinvd();
>  }
> @@ -1800,9 +1803,11 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  	if (need_emulate_wbinvd(vcpu)) {
>  		if (kvm_x86_ops->has_wbinvd_exit())
>  			cpumask_set_cpu(cpu, vcpu->arch.wbinvd_dirty_mask);
> -		else if (vcpu->cpu != -1 && vcpu->cpu != cpu)
> -			smp_call_function_single(vcpu->cpu,
> -					wbinvd_ipi, NULL, 1);
> +		else if (vcpu->cpu != -1 && vcpu->cpu != cpu) {
> +			queue_work_on(vcpu->cpu, wbinvd_wq,
> +					&per_cpu(wbinvd_work, vcpu->cpu));
> +			flush_workqueue(wbinvd_wq);
> +		}

Can't schedule here since preemption must be disabled during vcpu_load.

>  	}
>  
>  	kvm_x86_ops->vcpu_load(vcpu, cpu);
> @@ -3672,12 +3677,16 @@ int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address)
>  
>  int kvm_emulate_wbinvd(struct kvm_vcpu *vcpu)
>  {
> +	int cpu;
> +
>  	if (!need_emulate_wbinvd(vcpu))
>  		return X86EMUL_CONTINUE;
>  
>  	if (kvm_x86_ops->has_wbinvd_exit()) {
> -		smp_call_function_many(vcpu->arch.wbinvd_dirty_mask,
> -				wbinvd_ipi, NULL, 1);
> +		for_each_cpu(cpu, vcpu->arch.wbinvd_dirty_mask)
> +			queue_work_on(cpu, wbinvd_wq,
> +					&per_cpu(wbinvd_work, cpu));
> +		flush_workqueue(wbinvd_wq);
>  		cpumask_clear(vcpu->arch.wbinvd_dirty_mask);
>  	}
>  	wbinvd();