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

[Chao Gao <chao.gao@intel.com>]

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@huawei.com]
>>> 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.
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.

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

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