Re: [Xen-devel] [PATCH] x86/apicv: enhance posted-interrupt processing

["Xuquan (Quan Xu)" <xuquan8@xxxxxxxxxx>]

On February 21, 2017 5:55 AM, Chao Gao wrote:
>On Tue, Feb 21, 2017 at 04:11:53AM +0000, Xuquan (Quan Xu) wrote:
>>On February 21, 2017 11:07 AM, Tian, Kevin wrote:
>>>> From: Xuquan (Quan Xu) [mailto:xuquan8@xxxxxxxxxx]
>>>> Sent: Tuesday, February 21, 2017 10:49 AM
>>>> >>Chao, __iiuc__, your question may be from the comments of
>>>> >xen/arch/x86/hvm/vmx/vmx.c :: pi_notification_interrupt() ..
>>>> >>IF VT-d PI is enabled,
>>>> >>   VCPU_KICK_SOFTIRQ bit is set by '
>>>> >>raise_softirq(VCPU_KICK_SOFTIRQ)',
>>>> >at the end of pi_notification_interrupt()..
>>>> >>Else
>>>> >>  Is it possible for your case?
>>>> >>
>>>> >If vcpu is in root mode and is to do VM-entry, it has synced PIR to
>vIRR.
>>>> >Now a interrupt (e.g. PMU_APIC_VECTOR) happens. Thus it goes
>>>> >following the path
>>>> >pmu_apic_interrupt->vpmu_do_interrupt->vlapic_set_irq(assume
>>>> >it will inject a interrupt to current vcpu)
>>>> >-> vmx_deliver_posted_intr( set one bit in PIR )->
>>>> >-> __vmx_deliver_posted_interrupt
>>>> >Assuming that there is no softirq pending, the code after change
>>>> >doesn't generate a IPI for (cpu == smp_processor_id()). In this
>>>> >case, this interrupt would not be injected to guest before this
>VM-entry.
>>>> >Although there are many assumption in the explaination, I think it
>>>> >may be possible.
>>>> >
>>>>
>>>> So far, I agree to add VCPU_KICK_SOFTIRQ bit in a nice way..
>>>> Even we have checked whether the vCPU is running or not at the
>>>> beginning of __vmx_deliver_posted_interrupt(), we can't grantee the
>>>> vcpu is still in guest mode at the point to call this IPI..
>>>> as in an extreme case, at the point to call this IPI, all of vCPUs
>>>> are in root-mode, the posted-interrupt won't be delivered..
>>>
>>>I don't understand of your concern of whether guest is in guest mode
>here.
>>
>>If the vCPUs are in root-mode, whatever we do we can't deliver posted
>>interrupt Successfully.
>>
>>...
>>
>>>The purpose of this function is not to guarantee posted-interrupt is
>>>always used (cannot unless you pause remote cpu). It's just a best effort.
>>
>>...
>>
>>the best effort, you mentioned here, __iiuc__, we will sync PIRR to
>>vIRR before vmentry..
>>
>>if we don't set VCPU_KICK_SOFTIRQ bit, the pending interrupt in PIRR will
>be not synced to vIRR before vm-entry in time.
>>In an extreme case, if there is only one interrupt pending interrupt in
>>PIRR (no other caller to set VCPU_KICK_SOFTIRQ bit), The pending
>interrupt in PIRR will never be delivered to guest later..
>>
>>...
>>
>>> If target
>>>vcpu is in root mode, then this IPI causes a real interrupt on remote
>>>cpu as notification (then the handler pi_notification_interrupt you
>>>copied earlier will jump in to help).
>>>
>>>
>>...
>>The posted_intr_vector handler is not always pi_notification_interrupt.
>>If the vt-d PI is not enabled, the handler is event_check_interrupt..
>>The VCPU_KICK_SOFTIRQ bit is set in  pi_notification_interrupt , but not
>event_check_interrupt..
>>
>OK. you are right. I think we can resolve it by replacing
>event_check_interrupt with pi_notification_interrupt.

Sounds good to me. Note that there is more ' perfc_incr(ipis); ' in 
event_check_interrupt().. I don't know what's the purpose..

>A remote vcpu receives event check notification, the vcpu doesn't know
>where it is. Maybe it hasn't execute the code that handles event, or it has
>already execute the code. So if the vcpu wants to handle the event, it's
>better to jump back to the code that handles event. I guess why the current
>event_check_interrupt doesn't set the softirq flag is it assumes the flag has
>been set by the source vcpu ( by the line you remove ).
>
>Also, for the case that the target vcpu is current vcpu, the vcpu also doesn't
>know about where it is.
>It is better to set a softirq if there is no softirq.
>

A little confused, but the ipi here is to pCPU, instead of vCPU..

Quan

>Thanks,
>Chao
>>
>>
>>-I'd be very sorry if my description is still not clear..
>>Quan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel