Re: [Xen-devel] [PATCH v3 6/8] ARM: VGIC: factor out vgic_get_hw_irq_desc()

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 31/01/18 16:24, Andre Przywara wrote:
> Hi,
>
> On 31/01/18 16:16, Julien Grall wrote:
> > Hi,
> >
> > On 24/01/18 18:10, Andre Przywara wrote:
> > > At the moment we happily access the VGIC internal struct pending_irq
> > > (which describes a virtual IRQ) in irq.c.
> > > Factor out the actually needed functionality to learn the associated
> > > hardware IRQ and move that into gic-vgic.c to improve abstraction.
> > >
> > > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> > > Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > ---
> > >    xen/arch/arm/gic-vgic.c    | 15 +++++++++++++++
> > >    xen/arch/arm/irq.c         |  7 ++-----
> > >    xen/include/asm-arm/vgic.h |  2 ++
> > >    3 files changed, 19 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
> > > index d44e4dacd3..3ad98dcd3a 100644
> > > --- a/xen/arch/arm/gic-vgic.c
> > > +++ b/xen/arch/arm/gic-vgic.c
> > > @@ -411,6 +411,21 @@ void gic_dump_vgic_info(struct vcpu *v)
> > >            printk("Pending irq=%d\n", p->irq);
> > >    }
> > >    +struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu
> > > *v,
> > > +                                      unsigned int virq)
> > > +{
> > > +    struct pending_irq *p;
> > > +
> > > +    if ( !v )
> > > +        v = d->vcpu[0];
> > I dislike this new function in the current state. Let's imagine someone
> > decide to pass a PPI but no vCPU. The vCPU would now be default to vCPU0
> > and potentially returning the wrong irq_desc (imagine PPI passthrough
> > such as for the vtimer).
> >
> > So the code needs at least checking the vCPU is passed in the case of a
> > PPI. I would be happy with an ASSERT().
> Yeah, good point. Something like ASSERT(!v && virq >= 32), I guess?
Yes. It should be enough.

Cheers,


> Cheers,
> Andre.
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel