Re: [PATCH v8 2/4] xen/arm: Check for interrupt controller directly

[Ian Campbell <ian.campbell@citrix.com>]

On Tue, 2015-03-03 at 14:45 +0000, Julien Grall wrote:
> Hello Frediano,
> 
> On 03/03/15 11:19, Frediano Ziglio wrote:
> > This check allow to detect mail interrupt controller even if it does
> 
> main
> 
> > not match one of the standard ones.
> > This allow boards with non standard controllers to be handled correctly
> > without having to manually edit the global list every time.
> > 
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
> > ---
> >  xen/arch/arm/domain_build.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 9f1f59f..83951a3 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -1069,7 +1069,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
> >  
> >      /* Replace these nodes with our own. Note that the original may be
> >       * used_by DOMID_XEN so this check comes first. */
> > -    if ( dt_match_node(gic_matches, node) )
> > +    if ( node == dt_interrupt_controller || dt_match_node(gic_matches, node) )
> >          return make_gic_node(d, kinfo->fdt, node);
> 
> What about if the device tree exposes multiple GICs? By mistake we will
> expose the secondaries GIC if they are not standard.

Does the existing code here not insert a primary gic node into the dom0
tree for every gic node which find, that doesn't sound like it can be
right!

Is the right pattern:
    if ( node == dt_interrupt_controller )
         return make_gic_node(d, kinfo->fdt, node);
    else if ( device_get_class(node) == DEVICE_GIC )
    {
         DPRINT(" Secondary GIC, skip it\n");
         return 0;/* Skip it */
    }
(incorporating the suggestion to match class from further down thread)?

Anyway, I don't think what Frediano proposes in v9 of this series makes
any of this worse, so I don't propose to block the series based on it.

> 
> As I suggested on a previous mail, I would prefer to introduce a new
> callback to check if the node is a GIC.
> 
> Regards,
>