Re: [Xen-devel] [PATCH RFC] xen: arm: context switch vtimer PPI state.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 3 Mar 2015, Ian Campbell wrote:
> On Tue, 2015-03-03 at 11:38 +0000, Stefano Stabellini wrote:
> > >      gic_set_irq_properties(desc, cpumask_of(smp_processor_id()), 
> > > GIC_PRI_IRQ);
> > >  
> > > -    /* Use vcpu0 to retrieve the pending_irq struct. Given that we only
> > > -     * route SPIs to guests, it doesn't make any difference. */
> > > -    p = irq_to_pending(d->vcpu[0], desc->irq);
> > > -    p->desc = desc;
> > > +    if ( d )
> > > +    {
> > > +        struct pending_irq *p;
> > > +
> > > +        /* Use vcpu0 to retrieve the pending_irq struct. Given that we 
> > > only
> > > +         * route SPIs to guests, it doesn't make any difference. */
> > > +        p = irq_to_pending(d->vcpu[0], desc->irq);
> > > +        p->desc = desc;
> > > +    }
> > 
> > I think you need to assign p->desc = desc in this case too, otherwise
> > p->desc == NULL checks in gic.c and related code will succeed (and we
> > want them to fail as this is an hardware interrupt). We should be able
> > to use GICH_V2_LR_HW for it, right?
> > 
> > If you don't set p->desc, it is going to be treated as a virtual irq
> > with no corresponding physical irq.
> 
> How can we lookup p without a d?
> 
> > In fact I am not sure how the code can work as is given that
> > desc->handler is set to gic_hw_ops->gic_guest_irq_type but desc = NULL:
> > who is doing the EOI of the physical interrupt? Who is calling
> > gicv2_dir_irq?
> 
> p->desc is set in gicv2_restore_hwppi for interrupts such as these, so
> that it always correctly points to the running domain.
 
Ah, right! OK then. 

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