[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] Remove a set operation for VCPU_KICK_SOFTIRQ when post interrupt to vm.
On 2015/9/24 11:25, Zhang, Yang Z wrote:
Liuqiming (John) wrote on 2015-09-24:
On 2015/9/23 21:41, Zhang, Yang Z wrote:
Hanweidong (Randy) wrote on 2015-09-23:
Zhang, Yang Z wrote on ent: 2015å9æ23æ 11:51:
VCPU_KICK_SOFTIRQ when post interrupt to vm.
Zhang, Yang Z wrote on 2015-09-08:
Liuqiming (John) wrote on 2015-09-08:
Ok, I will try to explain, correct me if I got anything wrong:
The problem here is not interrupts lost but interrupts not
delivered in time.
there are basically two path to inject an interrupt into VM (or
vCPU to another vCPU):
Path 1, the traditional way:
1) set bit in vlapic IRR field which represent an interrupt,
then kick vcpu 2) a VCPU_KICK_SOFTIRQ softirq raised 3) if
VCPU_KICK_SOFTIRQ bit not set, then set it, otherwise
return
and
do nothing 4) send an EVENT_CHECK_VECTOR IPI to target
vcpu
5)
target vcpu will VMEXIT due to
EXIT_REASON_EXTERNAL_INTERRUPT
6)
the interrupt handler basically do nothing 7) interrupt in IRR
will be evaluated 8) VCPU_KICK_SOFTIRQ will be cleared when
do_softirq 9) there will be an interrupt inject into vcpu when
VMENTRY Path 2, the Posted-interrupt way (current logic):
1)
set
bit in posted-interrupt descriptor which represent an
interrupt 2) if VCPU_KICK_SOFTIRQ bit not set, then set it,
otherwise return and do nothing 3) send an
POSTED_INTR_NOTIFICATION_VECTOR IPI to target vcpu 4) if
target vcpu in non-ROOT mode it will receive the
interrupt immediately otherwise interrupt will be injected when
VMENTRY
As the first operation in both path is setting a interrupt
represent bit, so no interrupts will lost.
The issue is:
in path 2, the first interrupt will cause VCPU_KICK_SOFTIRQ set
to 1, and unless a VMEXIT occured or somewhere called do_softirq
directly, VCPU_KICK_SOFTIRQ will not cleared, that will make the
later interrupts injection ignored at step 2), which will delay
irq handler process in VM.
And because path 2 set VCPU_KICK_SOFTIRQ to 1, the kick vcpu
logic in path 1 will also return in step 3), which make this
vcpu only can handle interrupt when some other reason cause VMEXIT.
Nice catch! But without set the softirq, the interrupt delivery
also will be delay. Look at the code in vmx_do_vmentry:
cli
<---------------the virtual interrupt occurs here will
be delay. Because there is no softirq pending and the following
posted interrupt may consumed by another running VCPU, so the
target VCPU will not aware there is pending virtual interrupt before next
vmexit.
cmp %ecx,(%rdx,%rax,1)
jnz .Lvmx_process_softirqs
I need more time to think it.
Hi Qiming,
Can you help to take a look at this patch? Also, if possible,
please help to do a testing since I don't have machine to test it.
Thanks very much.
diff --git a/xen/arch/x86/hvm/vmx/entry.S
b/xen/arch/x86/hvm/vmx/entry.S index 664ed83..1ebb5d0 100644
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -77,6 +77,9 @@ UNLIKELY_START(ne, realmode)
call vmx_enter_realmode
UNLIKELY_END(realmode)
+ bt $0,VCPU_vmx_pi_ctrl(%rbx)
+ jc .Lvmx_do_vmentry
+
mov %rsp,%rdi
call vmx_vmenter_helper
mov VCPU_hvm_guest_cr2(%rbx),%rax diff --git
a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index
e0e9e75..315ce65 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1678,8 +1678,9 @@ static void
__vmx_deliver_posted_interrupt(struct
vcpu *v)
{
unsigned int cpu = v->processor;
- if ( !test_and_set_bit(VCPU_KICK_SOFTIRQ,
&softirq_pending(cpu))
- && (cpu != smp_processor_id()) )
+ if ( !test_bit(VCPU_KICK_SOFTIRQ, &softirq_pending(cpu))
+ && pi_test_on(&v->arch.hvm_vmx.pi_desc)
Why need pi_test_on() here?
on bit is cleared means the interrupt is consumed by target VCPU
already. So there is no need to send the PI.
Best regards,
Yang
Hi Yang,
I had a question about the "outstanding-notification" bit
(POSTED_INTR_ON) handling.
It's not very clear in Intel manual. From what I learned, this bit is
set by software when send an interrupt to vcpu and cleared by hardware
when interrupt delivered, right?
from the source code, there is a test_and_set op for this bit in
function vmx_deliver_posted_intr,
else if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) )
{
__vmx_deliver_posted_interrupt(v);
return;
}
so when we enter __vmx_deliver_posted_interrupt to send ipi, this bit
is already set on, the "pi_test_on" code is meaningless.
The on bit will be cleared at any time.
That's true, just feel the logic is a little confusing,
the outside test(vmx_deliver_posted_intr) say: "the bit is not 1, lets
set it to 1 and do the delivery"
the inner test(__vmx_deliver_posted_interrupt) say:"the bit is 1, let's
do the delivery"
And another thought, the clear bit action performed only when you send
a ipi to physical cpu or sync_pir_to_irr, there has a chance the bit
is set on and physical cpu not receive a ipi, for example
1) [CPU1] vcpu kicked and VCPU_KICK_SOFTIRQ is set 2) [CPU1] find the
highest irr where call sync_pir_to_irr to clear POSTED_INTR_ON 3) [CPU2]
a posted interrupt delivered to CPU1, it will set POSTED_INTR_ON on 4)
[CPU2] in __vmx_deliver_posted_interrupt will test VCPU_KICK_SOFTIRQ on
CPU1 which is set by 1), so no ipi sent
At this point, CPU1's POSTED_INTR_ON is on but not interrupt is
received. and until next kick on CPU1,
CPU1 will not allow posted interrupt get in.
Why next kick? CPU1 is going to handle the KICK_SOFTIRQ immediately. After it
completed, the following vmentry will pick up the pending interrupt. If you
send the ipi unconditionally, actually it is received by hypervisor and the
interrupt handler dose nothing. You still rely on the vmentry to pick up the
interrupt.
My confusion is: when vmentry, does physical CPU inject all interrupts
on PIR into VM and clear POSTED_INTR_ON bit?
Or just inject the highest index of PIR and may leave POSTED_INTR_ON bit
being set to 1?
In my opinion, if we want best performance, it should just remove the
VCPU_KICK_SOFTIRQ test in posted-interrupt function, or at least we
should change the order of test VCPU_KICK_SOFTIRQ and test POSTED_INTR_ON
Best regards,
Yang
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
