| Message ID | 20090303001405.GA5889@amt.cnet (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Marcelo Tosatti wrote: > @@ -72,7 +73,6 @@ struct kvm_vcpu { > struct mutex mutex; > int cpu; > struct kvm_run *run; > - int guest_mode; > unsigned long requests; > unsigned long guest_debug; > int fpu_active; The deletion will break other archs which depend on the filed. Xiantao-- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 03, 2009 at 03:53:50PM +0800, Zhang, Xiantao wrote: > Marcelo Tosatti wrote: > > @@ -72,7 +73,6 @@ struct kvm_vcpu { > > struct mutex mutex; > > int cpu; > > struct kvm_run *run; > > - int guest_mode; > > unsigned long requests; > > unsigned long guest_debug; > > int fpu_active; > > The deletion will break other archs which depend on the filed. Indeed. Will keep it, thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 03, 2009 at 10:25:20AM -0300, Marcelo Tosatti wrote: > On Tue, Mar 03, 2009 at 03:53:50PM +0800, Zhang, Xiantao wrote: > > Marcelo Tosatti wrote: > > > @@ -72,7 +73,6 @@ struct kvm_vcpu { > > > struct mutex mutex; > > > int cpu; > > > struct kvm_run *run; > > > - int guest_mode; > > > unsigned long requests; > > > unsigned long guest_debug; > > > int fpu_active; > > > > The deletion will break other archs which depend on the filed. > > Indeed. Will keep it, thanks. The only other arch is ia64 and its kvm_vcpu_kick() should be fixed the same way. No? -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 03, 2009 at 03:25:57PM +0200, Gleb Natapov wrote: > On Tue, Mar 03, 2009 at 10:25:20AM -0300, Marcelo Tosatti wrote: > > On Tue, Mar 03, 2009 at 03:53:50PM +0800, Zhang, Xiantao wrote: > > > Marcelo Tosatti wrote: > > > > @@ -72,7 +73,6 @@ struct kvm_vcpu { > > > > struct mutex mutex; > > > > int cpu; > > > > struct kvm_run *run; > > > > - int guest_mode; > > > > unsigned long requests; > > > > unsigned long guest_debug; > > > > int fpu_active; > > > > > > The deletion will break other archs which depend on the filed. > > > > Indeed. Will keep it, thanks. > The only other arch is ia64 and its kvm_vcpu_kick() should be fixed > the same way. No? Probably yes. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Marcelo Tosatti wrote: > KVM uses a function call IPI to cause the exit of a guest running on a > physical cpu. For virtual interrupt notification there is no need to > wait on IPI receival, or to execute any function. > > This is exactly what the reschedule IPI does, without the overhead > of function IPI. So use it instead of smp_call_function_single in > kvm_vcpu_kick. > > Also change the "guest_mode" variable to a bit in vcpu->requests, and > use that to collapse multiple IPI's that would be issued between the > first one and zeroing of guest mode. > > This allows kvm_vcpu_kick to called from interrupt context. > Looks good. The only worry I have is that we depend on smp_reschedule_interrupt() being a no-op. I guess that's a reasonable assumption though.
* Avi Kivity <avi@redhat.com> wrote: > Marcelo Tosatti wrote: >> KVM uses a function call IPI to cause the exit of a guest running on a >> physical cpu. For virtual interrupt notification there is no need to >> wait on IPI receival, or to execute any function. >> >> This is exactly what the reschedule IPI does, without the overhead >> of function IPI. So use it instead of smp_call_function_single in >> kvm_vcpu_kick. >> >> Also change the "guest_mode" variable to a bit in vcpu->requests, and >> use that to collapse multiple IPI's that would be issued between the >> first one and zeroing of guest mode. >> >> This allows kvm_vcpu_kick to called from interrupt context. >> > > Looks good. The only worry I have is that we depend on > smp_reschedule_interrupt() being a no-op. I guess that's a > reasonable assumption though. It's a reasonable current assumption - but it might change in the future - so please also put it into the changelog that KVM will revert it or fix it differently if the scheduler grows some functionality there. Ingo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index b556b6a..c68168c 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3133,6 +3133,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) local_irq_disable(); + clear_bit(KVM_REQ_KICK, &vcpu->requests); + smp_mb__after_clear_bit(); + if (vcpu->requests || need_resched() || signal_pending(current)) { local_irq_enable(); preempt_enable(); @@ -3140,13 +3143,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) goto out; } - vcpu->guest_mode = 1; - /* - * Make sure that guest_mode assignment won't happen after - * testing the pending IRQ vector bitmap. - */ - smp_wmb(); - if (vcpu->arch.exception.pending) __queue_exception(vcpu); else if (irqchip_in_kernel(vcpu->kvm)) @@ -3188,7 +3184,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) set_debugreg(vcpu->arch.host_dr6, 6); set_debugreg(vcpu->arch.host_dr7, 7); - vcpu->guest_mode = 0; + set_bit(KVM_REQ_KICK, &vcpu->requests); local_irq_enable(); ++vcpu->stat.exits; @@ -4429,28 +4425,19 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu) || vcpu->arch.nmi_pending; } -static void vcpu_kick_intr(void *info) -{ -#ifdef DEBUG - struct kvm_vcpu *vcpu = (struct kvm_vcpu *)info; - printk(KERN_DEBUG "vcpu_kick_intr %p \n", vcpu); -#endif -} - void kvm_vcpu_kick(struct kvm_vcpu *vcpu) { - int ipi_pcpu = vcpu->cpu; - int cpu = get_cpu(); + int me = get_cpu(); + int cpu = vcpu->cpu; if (waitqueue_active(&vcpu->wq)) { wake_up_interruptible(&vcpu->wq); ++vcpu->stat.halt_wakeup; } - /* - * We may be called synchronously with irqs disabled in guest mode, - * So need not to call smp_call_function_single() in that case. - */ - if (vcpu->guest_mode && vcpu->cpu != cpu) - smp_call_function_single(ipi_pcpu, vcpu_kick_intr, vcpu, 0); + + if (cpu != me && (unsigned)cpu < nr_cpu_ids && cpu_online(cpu)) + if (!test_and_set_bit(KVM_REQ_KICK, &vcpu->requests)) + smp_send_reschedule(cpu); + put_cpu(); } diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 3832243..ef7c239 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -38,6 +38,7 @@ #define KVM_REQ_UNHALT 6 #define KVM_REQ_MMU_SYNC 7 #define KVM_REQ_KVMCLOCK_UPDATE 8 +#define KVM_REQ_KICK 9 #define KVM_USERSPACE_IRQ_SOURCE_ID 0 @@ -72,7 +73,6 @@ struct kvm_vcpu { struct mutex mutex; int cpu; struct kvm_run *run; - int guest_mode; unsigned long requests; unsigned long guest_debug; int fpu_active;