Re: [Xen-devel] [PATCH v8 2/2] xen/arm: trap guest WFI

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 23 Apr 2013, Ian Campbell wrote:
> On Mon, 2013-04-22 at 18:42 +0100, Stefano Stabellini wrote:
> 
> > +static inline int _local_events_need_delivery(void)
> 
> Can we call this local_events_need_delivery_nomask or something to make
> it clear why it is special (which the leading _ doesn't really do).

Yeah

> > +{
> > +    struct pending_irq *p = irq_to_pending(current, 
> > VGIC_IRQ_EVTCHN_CALLBACK);
> > +
> > +    /* XXX: if the first interrupt has already been delivered, we should
> > +     * check whether any higher priority interrupts are in the
> > +     * lr_pending queue or in the LR registers and return 1 only in that
> > +     * case.
> > +     * In practice the guest interrupt handler should run with
> > +     * interrupts disabled so this shouldn't be a problem in the general
> > +     * case.
> > +     */
> > +    if ( gic_events_need_delivery() )
> > +        return 1;
> > +
> > +    if ( vcpu_info(current, evtchn_upcall_pending) &&
> > +        !vcpu_info(current, evtchn_upcall_mask) &&
> 
> I don't think you need this upcall_mask check.
> 
> > -static inline void local_event_delivery_disable(void)
> > -{
> > -    /* TODO current->vcpu_info->evtchn_upcall_mask = 1; */
> > -}
> > -
> >  static inline void local_event_delivery_enable(void)
> >  {
> >      /* TODO current->vcpu_info->evtchn_upcall_mask = 0; */
> 
> No reason to leave this TODO IMHO.
> 
 
I can remove both, but I would need to add a comment somewhere else
saying that evtchn_upcall_mask is going to be removed.
Where do you think is the right place for it?

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