Re: [PATCH v9 15/17] vmx: VT-d posted-interrupt core logic handling

["Wu, Feng" <feng.wu@intel.com>]

> -----Original Message-----
> From: Wu, Feng
> Sent: Wednesday, November 11, 2015 9:43 AM
> To: Dario Faggioli <dario.faggioli@citrix.com>; xen-devel@lists.xen.org
> Cc: Tian, Kevin <kevin.tian@intel.com>; Keir Fraser <keir@xen.org>; George
> Dunlap <george.dunlap@eu.citrix.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Jan Beulich <jbeulich@suse.com>; Wu, Feng
> <feng.wu@intel.com>
> Subject: RE: [Xen-devel] [PATCH v9 15/17] vmx: VT-d posted-interrupt core logic
> handling
> 
> 
> 
> > -----Original Message-----
> > From: Dario Faggioli [mailto:dario.faggioli@citrix.com]
> > Sent: Wednesday, November 11, 2015 12:59 AM
> > To: Wu, Feng <feng.wu@intel.com>; xen-devel@lists.xen.org
> > Cc: Tian, Kevin <kevin.tian@intel.com>; Keir Fraser <keir@xen.org>; George
> > Dunlap <george.dunlap@eu.citrix.com>; Andrew Cooper
> > <andrew.cooper3@citrix.com>; Jan Beulich <jbeulich@suse.com>
> > Subject: Re: [Xen-devel] [PATCH v9 15/17] vmx: VT-d posted-interrupt core
> logic
> > handling
> >
> > On Tue, 2015-11-03 at 09:07 +0000, Wu, Feng wrote:
> > > BTW, In the previous discussion, we will do the PI state adjustment
> > > in vmx_do_resume, however, seems this is not a good place to do this,
> > > since this function gets called only if the scheduling occurs, no
> > > matter it switches to another vCPU or continue runs the current vCPU.
> > > If no scheduling happens during "VM->Xen->VM", vmx-do_resume() will
> > > not get called.
> > >
> > Mmm... When I first read this, it seemed to me to be a good thing, and
> > a reason for actually putting your logic in there (that would avoid
> > paying the price of going through it during every VMENTRY, which you
> > were yourself hesitant about)!
> >
> > So, maybe I'm missing/misremembering something. Just to be sure, can
> > you tell me...
> >
> > > So I put the PI state adjustment code in vmx_vmenter_helper(), if you
> > > have any other good suggestions, please let me know, thanks a lot!
> > >
> > ... what is the reason(s) why you need to do the update even if no
> > scheduling happened?
> >
> > Looking at the code again, I think one reason may be to cope with when
> > vcpu_block() is called, but then local_events_need_delivery() returns
> > true, as shown here below:
> >
> > void vcpu_block(void)
> > {
> >     struct vcpu *v = current;
> >
> >     set_bit(_VPF_blocked, &v->pause_flags);
> >
> >     arch_vcpu_block(v);                             // nv <--- pi_wakeup_vector
> >
> >     /* Check for events /after/ blocking: avoids wakeup waiting race. */
> >     if ( local_events_need_delivery() )             // <=== TRUE
> >         clear_bit(_VPF_blocked, &v->pause_flags);
> >                                                     // we want nv == posted_intr_vector,
> >                                                        so we need vmx_pi_state_change()
> >                                                        to run, even if we're not scheduling
> > }
> >
> > Is this the case?
> 
> Yes, that is the case, after arch_vcpu_block() is called, we don't
> cancel it even if there are events to be delivered, so we need to check
> whether the NV is the right value, if it isn't, which means after
> after arch_vcpu_block(),local_events_need_delivery() check returns
> true, so we need to adjust the PI state before VM-Entry. As Jan mentioned
> before, we will check the value before actually update it atomically, so
> I think it is okay from performance point of view.

Besides the case you mentioned above, it also covers another case: now
we remove the arch hook in vcpu_wake(), so when vCPU is woken up,
the 'NV' filed is still 'wakeup_vector', hence we need to recover it before
VM Entry.

Thanks,
Feng

> 
> Thanks,
> Feng