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

[Avi Kivity <avi@redhat.com>]

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