Dong, Eddie wrote: > >> benefit is >> removing the lapic calculations from the critical section. >> >> But I still don't understand your object. The interrupt has to be >> committed sometime. We move the commit point earlier by the time it >> takes to ->prepare_guest_switch() (normally zero, but sometimes some >> thousand cycles), and kvm_load_guest_fpu() (usually zero as >> well). The >> timing is sometimes worse, but there was always a window where a high >> priority interrupt is ignored over low priority interrupts. >> > > Yes even in rare native when 2 irqs arrives at almost same time. > But now we have frequent happenings. Guest frequently see > higher irq pending. Let us keep an eye to see if it will hurt something. > > Then probably need this patch? > > diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c > index aab465d..46ad3b7 100644 > --- a/drivers/kvm/kvm_main.c > +++ b/drivers/kvm/kvm_main.c > @@ -2165,6 +2165,7 @@ again: > if (unlikely(r)) > goto out; > > + vcpu->guest_mode = 1; > kvm_inject_pending_timer_irqs(vcpu); > clear_bit(KVM_REQ_INTR, &vcpu->requests); > if (irqchip_in_kernel(vcpu->kvm)) > @@ -2183,6 +2184,7 @@ again: > local_irq_enable(); > preempt_enable(); > r = -EINTR; > + vcpu->guest_mode = 0; > kvm_run->exit_reason = KVM_EXIT_INTR; > ++vcpu->stat.signal_exits; > goto out; > @@ -2192,6 +2194,7 @@ again: > if (test_and_clear_bit(KVM_REQ_TLB_FLUSH, > &vcpu->requests)) > kvm_x86_ops->tlb_flush(vcpu); > if (test_bit(KVM_REQ_INTR, &vcpu->requests)) { > + vcpu->guest_mode = 0; > local_irq_enable(); > preempt_enable(); > r = 1; > @@ -2199,7 +2202,6 @@ again: > } > } > > - vcpu->guest_mode = 1; > kvm_guest_enter(); > > kvm_x86_ops->run(vcpu, kvm_run); > >
I don't see why this is needed. The interrupt code will set_bit(KVM_REQ_INTR) even if the vcpu is not in guest mode. > >> [we can try to improve it by using vm86 interrupt redirection >> which may >> allow event injection using VT instead of writing to the guest stack]. >> > > :) Love this much more personaly, or lock pages. > > We could lock the pages like this: - do the interrupt injection - if a real mode interrupt, abort, and set a flag requesting to lock the guest pages - exit the critical section - notice the flag is set, lock guest stack - retry - otherwise, proceed with injection but using vm86 interrupt redirection is better. >>> Another serious issue in current code is that due to previous >>> injection, the irq is already consumed, (IRR->ISR). So if there >>> are higher priority IRQs arrive later after the previous injection, >>> now guest will see 2 IRQs consumed, i.e. 2 ISR bits are set. >>> >>> >>> >> I don't understand why. If an event is pending in >> VM_ENTRY_INTR_INFO_FIELD, then we don't call >> kvm_cpu_get_interrupt() at all. >> >> > This is for protect mode. real mode is not setting this one, > and thus double consumed. > > Good point. That is also fixed by using vm86 interrupt redirection. I'll read some more about it. > But this is fixable with other additional logic. My point is that > we may see more potential issues now :-( > > > You're right. Well, that's the price of progress (swapping). -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel