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

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

On Mon, 2015-03-09 at 16:41 +0000, Frediano Ziglio wrote:
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Monday, March 09, 2015 4:09 PM
> > 
> > On Mon, 2015-03-09 at 12:55 +0200, Julien Grall wrote:
> > >
> > > On 05/03/2015 18:36, Ian Campbell wrote:
> > > > 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!
> > >
> > > The current code doesn't insert any secondary gic (see the check in
> > > make_gic_node) in the DT.
> > 
> > Ah, I missed that, yes that would avoid the issue for sure.
> > 
> 
> I'm wondering if the final results can always work. I mean, a bare
> metal kernel will see all interrupt controllers while under Xen kernel
> receive only the primary one (virtualized). Now the question is "What
> are topology of secondary controllers?" I mean if a device is supposed
> so send some interrupt throw a secondary controller and nor kernel nor
> xen handle this device we lose the ability to use interrupts from this
> device?

I've never yet seen a GICv2+ system which had multiple interrupt
controllers. Do you know of one?

My gut feeling is that the need to chain GICs was a feature of the GICv1
which isn't virt capable so we don't care much about it.

Ian.