On Mon, Mar 30, 2015 at 04:46:55AM +0000, Wu, Feng wrote: > > > > -----Original Message----- > > From: Marcelo Tosatti [mailto:mtosatti@xxxxxxxxxx] > > Sent: Saturday, March 28, 2015 3:30 AM > > To: Wu, Feng > > Cc: hpa@xxxxxxxxx; tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; x86@xxxxxxxxxx; > > gleb@xxxxxxxxxx; pbonzini@xxxxxxxxxx; dwmw2@xxxxxxxxxxxxx; > > joro@xxxxxxxxxx; alex.williamson@xxxxxxxxxx; jiang.liu@xxxxxxxxxxxxxxx; > > eric.auger@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > > iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx > > Subject: Re: [v3 24/26] KVM: Update Posted-Interrupts Descriptor when vCPU > > is blocked > > > > On Fri, Mar 27, 2015 at 06:34:14AM +0000, Wu, Feng wrote: > > > > > Currently, the following code is executed before local_irq_disable() is > > called, > > > > > so do you mean 1)moving local_irq_disable() to the place before it. 2) after > > > > interrupt > > > > > is disabled, set KVM_REQ_EVENT in case the ON bit is set? > > > > > > > > 2) after interrupt is disabled, set KVM_REQ_EVENT in case the ON bit > > > > is set. > > > > > > Here is my understanding about your comments here: > > > - Disable interrupts > > > - Check 'ON' > > > - Set KVM_REQ_EVENT if 'ON' is set > > > > > > Then we can put the above code inside " if > > (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) " > > > just like it used to be. However, I still have some questions about this > > comment: > > > > > > 1. Where should I set KVM_REQ_EVENT? In function vcpu_enter_guest(), or > > other places? > > > > See below: > > > > > If in vcpu_enter_guest(), since currently local_irq_disable() is called after > > 'KVM_REQ_EVENT' > > > is checked, is it helpful to set KVM_REQ_EVENT after local_irq_disable() is > > called? > > > > local_irq_disable(); > > > > *** add code here *** > > So we need add code like the following here, right? > > if ('ON' is set) > kvm_make_request(KVM_REQ_EVENT, vcpu); Yes. > > if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests > > ^^^^^^^^^^^^^^ Point *1. > > || need_resched() || signal_pending(current)) { > > vcpu->mode = OUTSIDE_GUEST_MODE; > > smp_wmb(); > > local_irq_enable(); > > preempt_enable(); > > vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); > > r = 1; > > goto cancel_injection; > > } > > > > > 2. 'ON' is set by VT-d hardware, it can be set even when interrupt is disabled > > (the related bit in PIR is also set). > > > > Yes, we are checking if the HW has set an interrupt in PIR while > > outside VM (which requires PIR->VIRR transfer by software). > > > > If the interrupt it set by hardware after local_irq_disable(), > > VMX-entry will handle the interrupt and perform the PIR->VIRR > > transfer and reevaluate interrupts, injecting to guest > > if necessary, is that correct ? > > > > > So does it make sense to check 'ON' and set KVM_REQ_EVENT accordingly > > after interrupt is disabled? > > > > To replace the costly > > > > + */ > > + if (kvm_x86_ops->hwapic_irr_update) > > + kvm_x86_ops->hwapic_irr_update(vcpu, > > + kvm_lapic_find_highest_irr(vcpu)); > > > > Yes, i think so. > > After adding the "checking ON and setting KVM_REQ_EVENT" operations listed in my > comments above, do you mean we still need to keep the costly code above > inside "if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {}" in function > vcpu_enter_guest() as it used to be? If yes, my question is what is the exact purpose > of "checking ON and setting KVM_REQ_EVENT" operations? Here is the code flow in > vcpu_enter_guest(): > > 1. Check KVM_REQ_EVENT, if it is set, sync pir->virr > 2. Disable interrupts > 3. Check ON and set KVM_REQ_EVENT -- Here, we set KVM_REQ_EVENT, but it is > checked in the step 1, which means, we cannot get any benefits even we set it here, > since the "pir->virr" sync operation was done in step 1, between step 3 and VM-Entry, > we don't synchronize the pir to virr. So even we set KVM_REQ_EVENT here, the interrupts > remaining in PIR cannot be delivered to guest during this VM-Entry, right? Please check point *1 above. The code will go back to "if (kvm_check_request(KVM_REQ_EVENT, vcpu)" And perform the pir->virr sync. -- 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
