From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH 2/4] KVM: Extended shared_msr_global to per CPU
Date: Wed, 16 Dec 2009 11:21:38 +0200
Message-ID: <4B28A6A2.3090802@redhat.com>
References: <1260942485-19156-1-git-send-email-sheng@linux.intel.com> <1260942485-19156-3-git-send-email-sheng@linux.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Marcelo Tosatti <mtosatti@redhat.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]:8315 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S932265AbZLPJVl (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 16 Dec 2009 04:21:41 -0500
In-Reply-To: <1260942485-19156-3-git-send-email-sheng@linux.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 12/16/2009 07:48 AM, Sheng Yang wrote:
> shared_msr_global saved host value of relevant MSRs, but it have an
> assumption that all MSRs it tracked shared the value across the different
> CPUs. It's not true with some MSRs, e.g. MSR_TSC_AUX.
>
> Extend it to per CPU to provide the support of MSR_TSC_AUX, and more
> alike MSRs.
>
> Notice now the shared_msr_global still have one assumption: it can only deal
> with the MSRs that won't change in host after KVM module loaded.
>
>
>
> -void kvm_define_shared_msr(unsigned slot, u32 msr)
> +static void shared_msr_update(void *data)
>   {
> -	int cpu;
> +	struct kvm_shared_msrs *smsr;
> +	u32 msr = *(u32 *)data;
> +	int slot;
>   	u64 value;
>
> +	smsr =&get_cpu_var(shared_msrs);
>    

Can use __get_cpu_var() since interrupts are disabled.

> +	/* only read, and nobody should modify it at this time,
> +	 * so don't need lock */
> +	for (slot = 0; slot<  shared_msrs_global.nr; slot++)
> +		if (shared_msrs_global.msrs[slot] == msr)
> +			break;
> +	if (slot>= shared_msrs_global.nr) {
> +		printk(KERN_ERR "kvm: can't find the defined MSR!");
> +		return;
> +	}
> +	rdmsrl_safe(msr,&value);
> +	smsr->values[slot].host = value;
> +	smsr->values[slot].curr = value;
> +	put_cpu_var(shared_msrs);
> +}
> +
> +void kvm_define_shared_msr(unsigned slot, u32 msr)
> +{
>   	if (slot>= shared_msrs_global.nr)
>   		shared_msrs_global.nr = slot + 1;
> -	shared_msrs_global.msrs[slot].msr = msr;
> -	rdmsrl_safe(msr,&value);
> -	shared_msrs_global.msrs[slot].value = value;
> -	for_each_online_cpu(cpu)
> -		per_cpu(shared_msrs, cpu).current_value[slot] = value;
> +	shared_msrs_global.msrs[slot] = msr;
> +	/* we need ensured the shared_msr_global have been updated */
> +	smp_wmb();
> +	smp_call_function(shared_msr_update,&msr, 1);
>    

on_each_cpu() is preferred.

> +	shared_msr_update(&msr);
>   }
>   EXPORT_SYMBOL_GPL(kvm_define_shared_msr);
>
>   static void kvm_shared_msr_cpu_online(void)
>   {
>   	unsigned i;
> -	struct kvm_shared_msrs *locals =&__get_cpu_var(shared_msrs);
> +	struct kvm_shared_msrs *smsr =&__get_cpu_var(shared_msrs);
>
>   	for (i = 0; i<  shared_msrs_global.nr; ++i)
> -		locals->current_value[i] = shared_msrs_global.msrs[i].value;
> +		smsr->values[i].curr = smsr->values[i].host;
>    

If the cpu is being onlined for the first time, then .host will not have 
been initialized.  Need to call shared_msr_update().

Also need to verify this is called after all the msrs have been 
initialized by the kernel, or we'll read default values.


-- 
error compiling committee.c: too many arguments to function