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: Mon, 9 Mar 2015 16:08:42 +0000 Message-ID: <1425917322.21353.36.camel@citrix.com> References: <54F5C8EF.5090507@linaro.org> <1425573375.25940.255.camel@citrix.com> <54FD7C36.1020305@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54FD7C36.1020305@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 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 > >>> --- > >>> 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. > With this version of the patch secondary gics was added to the DOM0 > DT How? Does that same check in make_gic_node not prevent it? > > 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. > > The solution on the v9 was the right one. Good. > I though I sent an email to review it but it looks like not :/ I don't recall seeing it. Ian.