On Wed, 27 Nov 2019, Julien Grall wrote: > On Tue, 26 Nov 2019, 23:18 Stefano Stabellini, wrote: > On Fri, 15 Nov 2019, Stewart Hildebrand wrote: > > Allow vgic_get_hw_irq_desc to be called with a vcpu argument. > > > > Use vcpu argument in vgic_connect_hw_irq. > > > > vgic_connect_hw_irq is called for PPIs and SPIs, not SGIs. Enforce with > > ASSERTs. > > > > Signed-off-by: Stewart Hildebrand > > > > --- > > v3: new patch > > > > --- > > Note: I have only modified the old vgic to allow delivery of PPIs. > > --- > >  xen/arch/arm/gic-vgic.c | 24 ++++++++++++++++-------- > >  xen/arch/arm/vgic.c     |  6 +++--- > >  2 files changed, 19 insertions(+), 11 deletions(-) > > > > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c > > index 98c021f1a8..2c66a8fa92 100644 > > --- a/xen/arch/arm/gic-vgic.c > > +++ b/xen/arch/arm/gic-vgic.c > > @@ -418,7 +418,7 @@ struct irq_desc *vgic_get_hw_irq_desc(struct domain *d, struct vcpu *v, > >  { > >      struct pending_irq *p; > >  > > -    ASSERT(!v && virq >= 32); > > +    ASSERT((!v && (virq >= 32)) || (!d && v && (virq >= 16) && (virq < 32))); > > I don't think !d is necessary for this to work as intended so I would > limit the ASSERT to > >   ASSERT((!v && (virq >= 32)) || (v && (virq >= 16) && (virq < 32))); > > the caller can always pass v->domain > > But then you have the risk to run into d != v->domain. So at least with the ASSERT you document the expectation. Yes, that was not my intention. It makes sense in certain scenarios for v to be NULL. What I was trying to say is that when v is not-NULL, then also d should be not-NULL for consistency. I don't think it makes sense to pass v corresponding to vcpu1 of domain2 and d == NULL, right? I don't know if you want to add a (d == v->domain) check to the ASSERT as it is pretty busy already. I am OK either way.