From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v8 2/4] xen/arm: Check for interrupt controller directly Date: Thu, 5 Mar 2015 16:36:15 +0000 Message-ID: <1425573375.25940.255.camel@citrix.com> References: <54F5C8EF.5090507@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54F5C8EF.5090507@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: Frediano Ziglio , Tim Deegan , xen-devel@lists.xen.org, Stefano Stabellini , zoltan.kiss@huawei.com List-Id: xen-devel@lists.xenproject.org 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 > > --- > > 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, >