Re: [PATCH v9 1/8] VMX: Permanently assign PI hook vmx_pi_switch_to()

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

On Tue, Feb 28, 2017 at 09:43:09AM -0700, Jan Beulich wrote:
>>>> On 27.02.17 at 02:45, <chao.gao@intel.com> wrote:
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -260,9 +260,15 @@ void vmx_pi_hooks_deassign(struct domain *d)
>>  
>>      ASSERT(d->arch.hvm_domain.pi_ops.vcpu_block);
>>  
>> +    /*
>> +     * Note that we don't set 'd->arch.hvm_domain.pi_ops.switch_to' to NULL
>> +     * here. If we deassign the hooks while the vCPU is runnable in the
>> +     * runqueue with 'SN' set, all the future notification event will be
>> +     * suppressed. Preserving the 'switch_to' hook function can make sure
>> +     * event time the vCPU is running the 'SN' field is cleared.
>> +     */
>
>Did we now lose the remark indicating that at least in theory
>we could remove the hook after it had run one more time? It's

I also think it only need to run one more time, because the hook
function that set 'SN' bit is already removed.

>also not really becoming clear why SN continues to matter
>after device removal, but perhaps that's just because of my
>so far limited understanding of PI.
>
>Also I think you mean "every time" on the last line, albeit that
>(as per my remark above) is irrelevant - we need it to run just
>once more.

I would like to make the comment as clear as possible.
How about the underlying comment,

Note that we don't set 'd->arch.hvm_domain.pi_ops.switch_to' to NULL
here. If we deassign the hooks while the vCPU is runnable in the
runqueue with 'SN' set, all the future notification event will be
suppressed since vmx_deliver_posted_intr() also use 'SN' bit
as the suppression flag. Preserving the 'switch_to' hook function can
clear the 'SN' bit when the vCPU becomes running next time. After
that, No matter which status(runnable, running or block) the vCPU is in,
the 'SN' bit will keep clear for the 'switch_from' hook function that set
the 'SN' bit has been removed. At that time, the 'switch_to' hook function
is also useless. Considering the function doesn't do harm to the whole
system, leave it here until we find a clean solution to deassign the 
'switch_to' hook function.

Thanks,
Chao

>
>Jan
>

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