|
[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
> -----Original Message-----
> From: Tian, Kevin
> Sent: Wednesday, April 08, 2015 4:54 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: 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?
I cannot find the reason to raise multiple tasklets here. The purpose of the
check
is to make sure the vCPU is not to be blocked when there are interrupts pending
for it. If it happens, we will terminate the block operation. And the 'ON' bit
is clear
along with _all_ the pending interrupts in PIR will being synced to vIRR before
VM-Entry. (once the 'ON' is set by VT-d HW, it will not generate notification
event
When interrupts happens , it only stored the interrupts in the related bit in
PIR.)
Thanks,
Feng
>
> Thanks
> Kevin
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |