Re: [Xen-devel] [PATCH v2 01/10] x86/HVM: Introduce struct hvm_pi_ops

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.01.17 at 07:51, <Suravee.Suthikulpanit@xxxxxxx> wrote:
> On 01/05/2017 10:51 PM, Jan Beulich wrote:
>>>>> On 31.12.16 at 06:45, <suravee.suthikulpanit@xxxxxxx> wrote:
>>> --- a/xen/include/asm-x86/hvm/domain.h
>>> +++ b/xen/include/asm-x86/hvm/domain.h
>>> @@ -72,6 +72,67 @@ struct hvm_ioreq_server {
>>>      bool_t                 bufioreq_atomic;
>>>  };
>>>
>>> +struct hvm_pi_ops {
>>> +    /*
>>> +     * To handle posted interrupts correctly, we need to set the following
>>> +     * state:
>>> +     *
>>> +     * * The PI notification vector (NV)
>>> +     * * The PI notification destination processor (NDST)
>>> +     * * The PI "suppress notification" bit (SN)
>>> +     * * The vcpu pi "blocked" list
>>> +     *
>>> +     * If a VM is currently running, we want the PI delivered to the guest 
>>> vcpu
>>> +     * on the proper pcpu (NDST = v->processor, SN clear).
>>> +     *
>>> +     * If the vm is blocked, we want the PI delivered to Xen so that it can
>>> +     * wake it up  (SN clear, NV = pi_wakeup_vector, vcpu on block list).
>>> +     *
>>> +     * If the VM is currently either preempted or offline (i.e., not 
>>> running
>>> +     * because of some reason other than blocking waiting for an 
>>> interrupt),
>>> +     * there's nothing Xen can do -- we want the interrupt pending bit set 
>>> in
>>> +     * the guest, but we don't want to bother Xen with an interrupt (SN 
>>> clear).
>>> +     *
>>> +     * There's a brief window of time between vmx_intr_assist() and 
>>> checking
>>> +     * softirqs where if an interrupt comes in it may be lost; so we need 
>>> Xen
>>> +     * to get an interrupt and raise a softirq so that it will go through 
>>> the
>>> +     * vmx_intr_assist() path again (SN clear, NV = posted_interrupt).
>>> +     *
>>> +     * The way we implement this now is by looking at what needs to happen 
>>> on
>>> +     * the following runstate transitions:
>>> +     *
>>> +     * A: runnable -> running
>>> +     *  - SN = 0
>>> +     *  - NDST = v->processor
>>> +     * B: running -> runnable
>>> +     *  - SN = 1
>>> +     * C: running -> blocked
>>> +     *  - NV = pi_wakeup_vector
>>> +     *  - Add vcpu to blocked list
>>> +     * D: blocked -> runnable
>>> +     *  - NV = posted_intr_vector
>>> +     *  - Take vcpu off blocked list
>>> +     *
>>> +     * For transitions A and B, we add hooks into vmx_ctxt_switch_{from,to}
>>> +     * paths.
>>> +     *
>>> +     * For transition C, we add a new arch hook, arch_vcpu_block(), which 
>>> is
>>> +     * called from vcpu_block() and vcpu_do_poll().
>>> +     *
>>> +     * For transition D, rather than add an extra arch hook on vcpu_wake, 
>>> we
>>> +     * add a hook on the vmentry path which checks to see if either of the 
>>> two
>>> +     * actions need to be taken.
>>> +     *
>>> +     * These hooks only need to be called when the domain in question 
>>> actually
>>> +     * has a physical device assigned to it, so we set and clear the 
>>> callbacks
>>> +     * as appropriate when device assignment changes.
>>> +     */
>>> +    void (*vcpu_block) (struct vcpu *);
>>> +    void (*pi_switch_from) (struct vcpu *v);
>>> +    void (*pi_switch_to) (struct vcpu *v);
>>> +    void (*pi_do_resume) (struct vcpu *v);
>>> +};
>>
>> While the hooks (as said, with the pi_ prefixes dropped) are
>> certainly fine to move here, the comment is extremely VMX
>> centric, and hence doesn't fit in this file. It either needs to be
>> generalized, or it should remain in VMX specific code, perhaps
>> with a referral to it added here.
> 
> I see. I will move the comment into arch/x86/hvm/vmx/vmx.c close to 
> where these hooks are implemented.

So you see no way of generalizing it? (I confess I didn't look closely
enough yet at the [dis]similarities between VMX/VT-d PI and AVIC
to be able to easily tell myself.)

Jan


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