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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.10.16 at 10:30, <feng.wu@xxxxxxxxx> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: Wednesday, September 28, 2016 5:39 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 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).
> 
> I use pcidevs_lock()/pcidevs_unlock() in vmx_pi_switch_to() since this lock
> is held while device assignment. However, I got the following call trace 
> during
> booting the guest. From the message, seems we cannot acquire that lock
> in this function.

Ah, yes - quite obviously, as we're running with interrupts disabled
there.

> Back to the original question, maybe it is worth remain the "to" hooks, and
> we might go too far if we really want to zap it.

Well, I would have preferred to clear such hooks if possible, and
it still does look to be possible (albeit not straightforward), but
since you don't appear to have much interest in this, and I don't
have the time to repeatedly try and suggest possibilities to try /
investigate (as I'd really expect you to do such investigations), I
guess I'll give up here. I'd like you to make very clear (in a code
comment) though that keeping the hook in place is not a functional
requirement, but just a means to ease the current implementation.
That way anyone with more interest in efficiency will know that it's
permissible to (carefully) remove that hook once no longer needed.

Jan


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