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 *** if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests ^^^^^^^^^^^^^^ || 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. > I might miss something in your comments, if so please point out. Thanks a lot! > > Thanks, > Feng > > > > > > > > > "if (kvm_x86_ops->hwapic_irr_update) > > > kvm_x86_ops->hwapic_irr_update(vcpu, > > > kvm_lapic_find_highest_irr(vcpu)); > > > > > > > kvm_lapic_find_highest_irr(vcpu) eats some cache > > > > (4 cachelines) versus 1 cacheline for reading ON bit. > > > > > > > > > > > > Please remove blocked and wakeup_cpu, they should not be > > necessary. > > > > > > > > > > > > > > Why do you think wakeup_cpu is not needed, when vCPU is blocked, > > > > > > > wakeup_cpu saves the cpu which the vCPU is blocked on, after vCPU > > > > > > > is woken up, it can run on a different cpu, so we need wakeup_cpu > > > > > > > to > > > > > > > find the right list to wake up the vCPU. > > > > > > > > > > > > If the vCPU was moved it should have updated IRTE destination field > > > > > > to the pCPU which it has moved to? > > > > > > > > > > Every time a vCPU is scheduled to a new pCPU, the IRTE destination > > > > > filed > > > > > would be updated accordingly. > > > > > > > > > > When vCPU is blocked. To wake up the blocked vCPU, we need to find > > which > > > > > list the vCPU is blocked on, and this is what wakeup_cpu used for? > > > > > > > > Right, perhaps prev_vcpu is a better name. > > > > > > Do you mean "prev_pcpu"? > > > > Yes. > > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html