Re: [Xen-devel] [PATCH] xen/arm: Black list everything with a PPI

[Julien Grall <julien.grall@xxxxxxx>]

On 16/05/2019 14:17, Andre Przywara wrote:
> On Thu, 16 May 2019 17:15:36 +0530
> Amit Tomer <amittomer25@xxxxxxxxx> wrote:
> > On Thu, May 16, 2019 at 12:25 AM Oleksandr <olekstysh@xxxxxxxxx> wrote:
> > > On 03.05.19 20:02, Amit Singh Tomar wrote:
> > > > Suggested-by:  Julien Grall <julien.grall@xxxxxxx>
> > > > Signed-off-by: Amit Singh Tomar <amittomer25@xxxxxxxxx>
> > > > ---
> > > >       * This replaces following patch.
> > > >         https://patchwork.kernel.org/patch/10899881/
> > > > ---
> > > >    xen/arch/arm/domain_build.c | 16 +++++++++++++++-
> > > >    1 file changed, 15 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > > index d983677..8f54472 100644
> > > > --- a/xen/arch/arm/domain_build.c
> > > > +++ b/xen/arch/arm/domain_build.c
> > > > @@ -1353,7 +1353,7 @@ static int __init handle_node(struct domain *d, struct 
> > > > kernel_info *kinfo,
> > > >            { /* sentinel */ },
> > > >        };
> > > >        struct dt_device_node *child;
> > > > -    int res;
> > > > +    int res, i, nirq, irq_id;
> > > >        const char *name;
> > > >        const char *path;
> > > >
> > > > @@ -1399,6 +1399,20 @@ static int __init handle_node(struct domain *d, struct 
> > > > kernel_info *kinfo,
> > > >            return 0;
> > > >        }
> > > >
> > > > +    /* Skip the node, using PPI source */
> > > > +    nirq = dt_number_of_irq(node);
> > > > +
> > > > +    for ( i = 0 ; i < nirq ; i++ )
> > > > +    {
> > > > +        irq_id = platform_get_irq(node, i);
> > > Do we need to do something here if platform_get_irq() returns -1?
> > Yeah, I should have done it. some think like following:
> > https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;h=d9836779d17c90e84b94ba32e4a20f028189fc5b;hb=HEAD#l1284
> Why would that (or any actual check against -1) be necessary?
> If irq_id is < 0, then it would surely not match the condition below and
> *nothing* would happen. So I'd say: Keep it like it is, no action necessary.
Worst, depending on the action done with check, you could actively break support 
for platform with multiple interrupt controllers. That's why in handle_device(), 
the interrupt controller is checked before calling platform_get_irq().
Cheers,

--
Julien Grall

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