Re: [PATCH v5 5/6] xen/arm: support irq delivery to vcpu > 0

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Wed, 18 Jun 2014, Ian Campbell wrote:
> On Wed, 2014-06-11 at 17:27 +0100, Stefano Stabellini wrote:
> > Use vgic_get_target_vcpu to retrieve the target vcpu from do_IRQ.
> > Remove in-code comments about missing implementation of SGI delivery to
> > vcpus other than 0.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > ---
> > 
> > Changes in v4:
> > - the mask in gic_route_irq_to_guest is a physical cpu mask, treat it as
> > such;
> > - export vgic_get_target_vcpu in a previous patch.
> > ---
> >  xen/arch/arm/gic.c        |    1 -
> >  xen/arch/arm/irq.c        |    3 +--
> >  xen/arch/arm/vgic.c       |    6 ++++++
> >  xen/include/asm-arm/gic.h |    1 +
> >  4 files changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> > index 8ed242e..82a0be4 100644
> > --- a/xen/arch/arm/gic.c
> > +++ b/xen/arch/arm/gic.c
> > @@ -287,7 +287,6 @@ void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
> >      gic_set_irq_properties(desc->irq, level, cpumask_of(smp_processor_id()),
> >                             GIC_PRI_IRQ);
> >  
> > -    /* TODO: do not assume delivery to vcpu0 */
> 
> Replace with "Route to vcpu0 by default" ?
> 
> >      p = irq_to_pending(d->vcpu[0], desc->irq);
> >      p->desc = desc;
> >  }
> > diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> > index a33c797..756250c 100644
> > --- a/xen/arch/arm/irq.c
> > +++ b/xen/arch/arm/irq.c
> > @@ -175,8 +175,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
> >          desc->status |= IRQ_INPROGRESS;
> >          desc->arch.eoi_cpu = smp_processor_id();
> >  
> > -        /* XXX: inject irq into all guest vcpus */
> > -        vgic_vcpu_inject_irq(d->vcpu[0], irq);
> > +        vgic_vcpu_inject_spi(d, irq);
> 
> This needs an assert (or at least a comment) that the irq is not a PPI
> because IRQ_GUEST is set and such things cannot be PPIs right now (and
> that last subtlety needs a comment even with the assert)
> 
> >          goto out_no_end;
> >      }
> >  
> > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> > index e640de9..2192a8c 100644
> > --- a/xen/arch/arm/vgic.c
> > +++ b/xen/arch/arm/vgic.c
> > @@ -871,6 +871,12 @@ out:
> >          smp_send_event_check_mask(cpumask_of(v->processor));
> >  }
> >  
> > +void vgic_vcpu_inject_spi(struct domain *d, unsigned int irq)
> > +{
> 
> Or maybe an ASSERT is needed here instead, or as well.
> 
> I'm almost inclined to suggest that vgic_get_target_vcpu should take
> both d and v and that this caller should pass v==NULL and
> vgic_get_target_vcpu should assert that IRQ is a SPI when v==NULL.

I prefer to assert in vgic_vcpu_inject_spi


> > +    struct vcpu *v = vgic_get_target_vcpu(d->vcpu[0], irq);
> > +    vgic_vcpu_inject_irq(v, irq);
> > +}
> > +
> 
>