On Thursday 17 December 2009 18:32:08 Avi Kivity wrote:
> 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@xxxxxxxxxxxxxxx>
> > ---
> >
> > 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.
The reason is, after cpu hotplug, the MSR_TSC_AUX would be rewritten by
vsyscall_init again. But in the hotplug notifier chain, KVM has higher
priority(20 vs 0 for vsyscall_init), so maybe the rdmsr() here would get a
bogus value... Then I think prevent it from initializing again should be
safer.
But I just think of another issue: if we hot plug in a cpu(without hot plug
off), it would have a bogus value as well in the same path? Sound
troublesome...
>
> > + smsr->values[slot].initialized = true;
> > + put_cpu_var(shared_msrs);
>
> If you use __get_cpu_var(), you need to remove put_cpu_var().
>
Sure...
--
regards
Yang, Sheng
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
