Re: [PATCH v4 3/8] KVM Guest: Add VCPU running/pre-empted state for guest

Nikunj A Dadhania
2012-08-24T05:40:26Z

On Thu, 23 Aug 2012 06:36:43 -0300, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> On Tue, Aug 21, 2012 at 04:56:35PM +0530, Nikunj A. Dadhania wrote:
> >
> > +void kvm_disable_vcpu_state(void)
> > +{
> > + if (!has_vcpu_state)
> > + return;
> > +
> > + wrmsr(MSR_KVM_VCPU_STATE, 0, 0);
>
> wrmsrl (to be consistent).
>
Sure, will change

> > +}
> > +
> > #ifdef CONFIG_SMP
> > static void __init kvm_smp_prepare_boot_cpu(void)
> > {
> > @@ -410,6 +440,7 @@ static void __cpuinit kvm_guest_cpu_online(void *dummy)
> >
> > static void kvm_guest_cpu_offline(void *dummy)
> > {
> > + kvm_disable_vcpu_state();
>
> Should disable MSR at kvm_pv_guest_cpu_reboot.
>
Sure, can you explain the difference for my understanding?

> > kvm_disable_steal_time();
> > if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
> > wrmsrl(MSR_KVM_PV_EOI_EN, 0);
> > @@ -469,6 +500,11 @@ void __init kvm_guest_init(void)
> > if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
> > apic_set_eoi_write(kvm_guest_apic_eoi_write);
> >
> > +#ifdef CONFIG_PARAVIRT_TLB_FLUSH
> > + if (kvm_para_has_feature(KVM_FEATURE_VCPU_STATE))
> > + has_vcpu_state = 1;
> > +#endif
>
> Why only this hunk guarded by CONFIG_PARAVIRT_TLB_FLUSH and not
> the rest of the code?
>
The guest should have been compiled with CONFIG_PARAVIRT_TLB_FLUSH, as
the config also brings in HAVE_RCU_TABLE_FREE code into picture. We
should not enable this code without HAVE_RCU_TABLE_FREE.

Did not want to spray this across all the code, as the compiler will
take care of throwing out the kvm_tlb_flush_others

> Is there a switch to enable/disable this feature on the kernel
> command line?
>
No, havent added it.

> Grep for early_param in kvm.c.
>
Let me know if that is required.

Regards
Nikunj