From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v8 2/4] xen/arm: Check for interrupt controller directly Date: Mon, 09 Mar 2015 12:55:50 +0200 Message-ID: <54FD7C36.1020305@linaro.org> References: <54F5C8EF.5090507@linaro.org> <1425573375.25940.255.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1425573375.25940.255.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Frediano Ziglio , Tim Deegan , xen-devel@lists.xen.org, Stefano Stabellini , zoltan.kiss@huawei.com List-Id: xen-devel@lists.xenproject.org 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. With this version of the patch secondary gics was added to the DOM0 DT > 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. I though I sent an email to review it but it looks like not :/ Regards, -- Julien Grall