Re: [PATCH 2/4] KVM: Extended shared_msr_global to per CPU

[Avi Kivity <avi@redhat.com>]

On 12/17/2009 11:32 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.
>
> Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> ---
>
> How about this?
>
> Move the all initialization to hardware_enable(). And only initialized once
> for each cpu.
>
>
> -void kvm_define_shared_msr(unsigned slot, u32 msr)
> +static void shared_msr_update(unsigned slot, u32 msr)
>   {
> -	int cpu;
> +	struct kvm_shared_msrs *smsr;
>   	u64 value;
>
> +	smsr =&__get_cpu_var(shared_msrs);
> +	/* only read, and nobody should modify it at this time,
> +	 * so don't need lock */
> +	if (slot>= shared_msrs_global.nr) {
> +		printk(KERN_ERR "kvm: invalid MSR slot!");
> +		return;
> +	}
> +	if (smsr->values[slot].initialized)
> +		return;
>    

I don't think .initialized is worthwhile.  shared_msr_update is run very 
rarely.

> +	smsr->values[slot].initialized = true;
> +	put_cpu_var(shared_msrs);
>    

If you use __get_cpu_var(), you need to remove put_cpu_var().


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