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

[Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>]

Jan,

On 01/05/2017 10:51 PM, Jan Beulich wrote:
>>>> On 31.12.16 at 06:45, <suravee.suthikulpanit@amd.com> 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.
>
> Jan
>

I see. I will move the comment into arch/x86/hvm/vmx/vmx.c close to 
where these hooks are implemented.

Thanks,
Suravee

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