Re: [Xen-devel] [XEN PATCH v3 07/11] xen: arm: vgic: allow delivery of PPIs to guests

From: Julien Grall <julien.grall@gmail.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Stewart Hildebrand <Stewart.Hildebrand@dornerworks.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Julien Grall <julien@xen.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [Xen-devel] [XEN PATCH v3 07/11] xen: arm: vgic: allow delivery of PPIs to guests
Date: Wed, 27 Nov 2019 00:13:29 +0000	[thread overview]
Message-ID: <CAF3u54B5GHVFkmghDPYhO+W4Z3L6bMqg2MnPvvpr7J+FDNq5Xg@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.21.1911261418240.8205@sstabellini-ThinkPad-T480s>

[-- Attachment #1.1: Type: text/plain, Size: 1690 bytes --]

On Tue, 26 Nov 2019, 23:18 Stefano Stabellini, <sstabellini@kernel.org>
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 <stewart.hildebrand@dornerworks.com>
> >
> > ---
> > 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.

Cheers,

>
> >      if ( !v )
> >          v = d->vcpu[0];
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

[-- Attachment #1.2: Type: text/html, Size: 2907 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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