[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC v1 13/15] Update Posted-Interrupts Descriptor during vCPU scheduling



> From: Wu, Feng
> Sent: Thursday, April 02, 2015 4:40 PM
> 
> > -----Original Message-----
> > From: Tian, Kevin
> > Sent: Thursday, April 02, 2015 2:25 PM
> > To: Wu, Feng; xen-devel@xxxxxxxxxxxxx
> > Cc: JBeulich@xxxxxxxx; keir@xxxxxxx; Zhang, Yang Z
> > Subject: RE: [RFC v1 13/15] Update Posted-Interrupts Descriptor during vCPU
> > scheduling
> >
> > > From: Wu, Feng
> > > Sent: Wednesday, March 25, 2015 8:32 PM
> > >
> > > The basic idea here is:
> > > 1. When vCPU's state is RUNSTATE_running,
> > >         - set 'NV' to 'Notification Vector'.
> > >         - Clear 'SN' to accpet PI.
> > >         - set 'NDST' to the right pCPU.
> > > 2. When vCPU's state is RUNSTATE_blocked,
> > >         - set 'NV' to 'Wake-up Vector', so we can wake up the
> > >           related vCPU when posted-interrupt happens for it.
> > >         - Clear 'SN' to accpet PI.
> > > 3. When vCPU's state is RUNSTATE_runnable/RUNSTATE_offline,
> > >         - Set 'SN' to suppress non-urgent interrupts.
> > >           (Current, we only support non-urgent interrupts)
> > >         - Set 'NV' back to 'Notification Vector' if needed.
> > >
> > > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > > ---
> > >  xen/arch/x86/hvm/vmx/vmx.c | 108
> > > +++++++++++++++++++++++++++++++++++++++++++++
> > >  xen/common/schedule.c      |   3 ++
> > >  2 files changed, 111 insertions(+)
> > >
> > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> > > index b30392c..6323bd6 100644
> > > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > > @@ -1710,6 +1710,113 @@ static void vmx_handle_eoi(u8 vector)
> > >      __vmwrite(GUEST_INTR_STATUS, status);
> > >  }
> > >
> > > +static void vmx_pi_desc_update(struct vcpu *v, int new_state)
> > > +{
> > > +    struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc;
> > > +    struct pi_desc old, new;
> > > +    int old_state = v->runstate.state;
> > > +    unsigned long flags;
> > > +
> > > +    if ( !iommu_intpost )
> > > +        return;
> > > +
> > > +    switch ( new_state )
> > > +    {
> > > +    case RUNSTATE_runnable:
> > > +    case RUNSTATE_offline:
> > > +        /*
> > > +         * We don't need to send notification event to a non-running
> > > +         * vcpu, the interrupt information will be delivered to it before
> > > +         * VM-ENTRY when the vcpu is scheduled to run next time.
> > > +         */
> > > +        pi_set_sn(pi_desc);
> > > +
> > > +        /*
> > > +         * If the state is transferred from RUNSTATE_blocked,
> > > +         * we should set 'NV' feild back to posted_intr_vector,
> > > +         * so the Posted-Interrupts can be delivered to the vCPU
> > > +         * by VT-d HW after it is scheduled to run.
> > > +         */
> > > +        if ( old_state == RUNSTATE_blocked )
> > > +        {
> > > +            do
> > > +            {
> > > +                old.control = new.control = pi_desc->control;
> > > +                new.nv = posted_intr_vector;
> > > +            }
> > > +            while ( cmpxchg(&pi_desc->control, old.control,
> new.control)
> > > +                    != old.control );
> > > +
> > > +           /*
> > > +            * Delete the vCPU from the related wakeup queue
> > > +            * if we are resuming from blocked state
> > > +            */
> > > +           spin_lock_irqsave(&per_cpu(blocked_vcpu_on_cpu_lock,
> > > +                             v->processor), flags);
> > > +           list_del(&v->blocked_vcpu_list);
> > > +
> spin_unlock_irqrestore(&per_cpu(blocked_vcpu_on_cpu_lock,
> > > +                                  v->processor), flags);
> > > +        }
> > > +        break;
> > > +
> > > +    case RUNSTATE_blocked:
> > > +        /*
> > > +         * The vCPU is blocked on the wait queue.
> > > +         * Store the blocked vCPU on the list of the
> > > +         * vcpu->wakeup_cpu, which is the destination
> > > +         * of the wake-up notification event.
> > > +         */
> > > +        spin_lock_irqsave(&per_cpu(blocked_vcpu_on_cpu_lock,
> > > +                          v->processor), flags);
> > > +        list_add_tail(&v->blocked_vcpu_list,
> > > +                      &per_cpu(blocked_vcpu_on_cpu,
> v->processor));
> > > +        spin_unlock_irqrestore(&per_cpu(blocked_vcpu_on_cpu_lock,
> > > +                               v->processor), flags);
> > > +
> > > +        do
> > > +        {
> > > +            old.control = new.control = pi_desc->control;
> > > +
> > > +            /*
> > > +             * We should not block the vCPU if
> > > +             * an interrupt is posted for it.
> > > +             */
> > > +
> > > +            if ( pi_test_on(&old) == 1 )
> > > +            {
> > > +                tasklet_schedule(&v->vcpu_wakeup_tasklet);
> > > +                return;
> > > +            }
> >
> > so you also need to remove the vcpu from the blocked list, right?
> 
> Yes, I need to remove the vcpu here. I thought it could be removed in
> another place in this patch, however, I feel it cannot do it after
> more thinking about it. I think we can fix this issue in the following two 
> ways:
> #1) Just add the remove logic here.
> or
> #2) In function vcpu_runstate_change(), call 'hvm_funcs.pi_desc_update()'
> after
> v->runstate.state = new_state; and pass the old_state to
> 'hvm_funcs.pi_desc_update()'.
> then here is the code path:
> 
> tasklet_schedule(&v->vcpu_wakeup_tasklet) -> vcpu_unblock() -> vcpu_wake()
> ->
> vcpu_runstate_change(v, RUNSTATE_runnable, NOW()) ->
> vmx_pi_desc_update()
> 
> So, the vCPU will be removed in 'case RUNSTATE_runnable:' of function
> vmx_pi_desc_update().

either way is OK. It's difficult to judge it now based on above function names
w/o re-reading the whole series. we can check it in your new version. :-)

> 
> >
> > and how do you handle ON is set after above check? looks this is better
> > handled behind cmpxchg loop...
> 
> - If 'ON' is set before 'if ( pi_test_on(&old) == 1 )', return
> - If 'ON' is not set before it, and is set after it, ' 
> cmpxchg(&pi_desc->control,
> old.control, new.control) != old.control ' returns ture,
> so, we will do the while again, at this time, 'if ( pi_test_on(&old) == 1 )' 
> is true.
> 

but do we need to raise multiple tasklets in the loop? can they be combined
into one notification after the loop?

Thanks
Kevin

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.