Sorry for the late response > -----Original Message----- > From: Avi Kivity [mailto:avi@xxxxxxxxxx] > Sent: Friday, September 07, 2012 12:22 AM > To: Li, Jiongxi > Cc: kvm@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 2/5]KVM:x86, apicv: adjust for virtual interrupt delivery > > On 09/05/2012 08:41 AM, Li, Jiongxi wrote: > > Virtual interrupt delivery avoids KVM to inject vAPIC interrupts > > manually, which is fully taken care of by the hardware. This needs > > some special awareness into existing interrupr injection path: > > > > - for pending interrupt, instead of direct injection, we may need > > update architecture specific indicators before resuming to guest. > > > > - A pending interrupt, which is masked by ISR, should be also > > considered in above update action, since hardware will decide > > when to inject it at right time. Current has_interrupt and > > get_interrupt only returns a valid vector from injection p.o.v. > > > > > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -5194,6 +5194,13 @@ static void inject_pending_event(struct > kvm_vcpu *vcpu) > > vcpu->arch.nmi_injected = true; > > kvm_x86_ops->set_nmi(vcpu); > > } > > + } else if (kvm_apic_vid_enabled(vcpu)) { > > + if (kvm_cpu_has_interrupt_apic_vid(vcpu) && > > + kvm_x86_ops->interrupt_allowed(vcpu)) { > > + kvm_queue_interrupt(vcpu, > > + kvm_cpu_get_interrupt_apic_vid(vcpu), false); > > + kvm_x86_ops->set_irq(vcpu); > > + } > > It may be simpler to change kvm_cpu_{has,get}_interrupt to ignore the apic if > virtual interrupt delivery is enabled. OKs > > > @@ -5293,16 +5300,27 @@ static int vcpu_enter_guest(struct kvm_vcpu > *vcpu) > > } > > > > if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) { > > + /* update archtecture specific hints for APIC virtual interrupt delivery > */ > > + if (kvm_apic_vid_enabled(vcpu)) > > + kvm_x86_ops->update_irq(vcpu); > > + > > Not defined. This function is defined in patch 3/5. Because virtual interrupt delivery is not enabled in this patch. So this function is not called. Since we will enable this feature by default, so maybe we can merge PATCH 2,3,4 together into one patch. > > > inject_pending_event(vcpu); > > > > /* enable NMI/IRQ window open exits if needed */ > > if (vcpu->arch.nmi_pending) > > kvm_x86_ops->enable_nmi_window(vcpu); > > - else if (kvm_cpu_has_interrupt(vcpu) || req_int_win) > > + else if (kvm_apic_vid_enabled(vcpu)) { > > + if (kvm_cpu_has_interrupt_apic_vid(vcpu)) > > + kvm_x86_ops->enable_irq_window(vcpu); > > + } else if (kvm_cpu_has_interrupt(vcpu) || req_int_win) > > kvm_x86_ops->enable_irq_window(vcpu); > > > > if (kvm_lapic_enabled(vcpu)) { > > - update_cr8_intercept(vcpu); > > + /* no need for tpr_threshold update if APIC virtual > > + * interrupt delivery is enabled > > + */ > > + if (!kvm_apic_vid_enabled(vcpu)) > > + update_cr8_intercept(vcpu); > > Perhaps the arch function should do the ignoring. You means putting the 'vid_enabled' judgement in 'kvm_x86_ops->update_cr8_intercept'? Is it just out of the reason that reducing the code change in common code? > > > kvm_lapic_sync_to_vapic(vcpu); > > } > > } > > > > > -- > error compiling committee.c: too many arguments to function -- 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
