Re: [Xen-devel] [PATCH v4 1/6] VMX: Statically assign two PI hooks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.09.16 at 08:48, <feng.wu@xxxxxxxxx> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: Monday, September 26, 2016 8:10 PM
>> To: Wu, Feng <feng.wu@xxxxxxxxx>
>> Cc: andrew.cooper3@xxxxxxxxxx; dario.faggioli@xxxxxxxxxx;
>> george.dunlap@xxxxxxxxxxxxx; Tian, Kevin <kevin.tian@xxxxxxxxx>; xen-
>> devel@xxxxxxxxxxxxx 
>> Subject: Re: [Xen-devel] [PATCH v4 1/6] VMX: Statically assign two PI hooks
>> 
>> >>> On 26.09.16 at 13:37, <JBeulich@xxxxxxxx> wrote:
>> >>>> On 21.09.16 at 04:37, <feng.wu@xxxxxxxxx> wrote:
>> >> PI hooks: vmx_pi_switch_from() and vmx_pi_switch_to() are
>> >> needed even when any previously assigned device is detached
>> >> from the domain. Since 'SN' bit is also used to control the
>> >> CPU side PI and we change the state of SN bit in these two
>> >> functions, then evaluate this bit in vmx_deliver_posted_intr()
>> >> when trying to deliver the interrupt in posted way via software.
>> >> The problem is if we deassign the hooks while the vCPU is runnable
>> >> in the runqueue with 'SN' set, all the furture notificaton event
>> >> will be suppressed. This patch makes these two hooks statically
>> >> assigned.
>> >
>> > So if only SN left set is a problem, why do you need to also keep
>> > vmx_pi_switch_from in place? It's vmx_pi_switch_to() which clears
>> > the bit, and vmx_deliver_posted_intr() doesn't actively change it.
>> 
>> And it doesn't appear completely unreasonable for
>> vmx_pi_switch_to() to remove itself (when it gets run with
>> the "from" hook still NULL and no new device being in the
>> process of getting assigned).
> 
> I think this may introduce extra complex to the situation:
> 1. Especially for "no new device being in the process of getting assigned",
> since device assignment can be happened simultaneous when this function
> gets called, so does it mean we need to use a lock to protect it?

Since device addition/removal is already protected by a lock, this
would at least seem not impossible to do without causing lock
conflicts (and would certainly be required, yes).

> 2. vmx_pi_switch_to() will update the 'NDST' field to the current pCPU,
> so if we zap vmx_pi_switch_to() here, that means, the 'NDST' filed may
> contain some valid pCPU information. So the method you suggested
> for [4/6] of the series doesn't work anymore, since we cannot use
> an invalid value for the judgement.

But why does the function need to continue to do these adjustments
when there's no assigned device anymore? I.e. couldn't it reset the
field to the "invalid" value along with removing itself?

Jan


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