[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/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.
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.
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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
