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
