From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E3DC5C48BC3 for ; Mon, 19 Feb 2024 16:38:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=2+U4gY7xHtIYC0kK+tdko/wuEne4I/w1bOaNPVB3dFw=; b=nk1pBskVeAVt3+ DDDZvlQuxcu8w5IjgZcTYfOCh3qEd47Za6Mhy3xx6zPpxMBPl5htiDhK1MacTBv+Eduwpm7GNj8UV 4/NuPGFf8UfX++O/99KwnQB5VPcfkQAiSKYk+c43rOzEcoFZZnRABgaEVmdOidKNN4CHi1951eJ7R C6nH5MPmPZobDreiCDRkHI+sGO6TG4sBBeFK1f0PJ9DkFuhmlf/uzRlaFvYo8XzIPyF+sByulWLfF WyviOVU5LB36Z8pH1FjQYPeTQU1WjjigNVjGj2HUq7hzlMiHruWN83prL3JLyEw+RFXqni05E5QcL Izl1mtWfGluO776L6WaA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rc6eU-0000000BNQI-0qkM; Mon, 19 Feb 2024 16:37:58 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rc6eS-0000000BNPs-1c2X for linux-arm-kernel@lists.infradead.org; Mon, 19 Feb 2024 16:37:57 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id B22EA60E84; Mon, 19 Feb 2024 16:37:55 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5472FC433F1; Mon, 19 Feb 2024 16:37:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1708360675; bh=Wu+ycCqAdfmKT21VYRn4CyaHzrwn6FfZ+tmCfzmQgIQ=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=UkJf3r+XhS9fLFm2ZLMkdLJLlX5opcAGa4+2jnZeyjozL44LJZuKWTfJh0/zQMzjg HIqiWkXiMlOYgRPN9sFmBR097+dG9XUpRjTqTF7lFEtJrLUG/MVIFx830XYAmCc/lG pdypqOvIJdEOLzqCm00CLw7lBpCSAIefduByvwgDlsO9UJn8DewWztXdiosDCxnBtb JxcUkWlZUjCQ0llFsFLL31fzW8uFDAFhnDzS1CnKr9yw6XiwnyNryd5h924Yh51s4m xSm+Xf3ZFoWByOkTkePeaFt+AOwXqyydi/c6KHqkJLV2ZRvrMCkLub2TX20PqXQJkI B/b52ZNzMOBpw== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1rc6eO-004ddJ-PS; Mon, 19 Feb 2024 16:37:52 +0000 Date: Mon, 19 Feb 2024 16:37:52 +0000 Message-ID: <865xyk4dgf.wl-maz@kernel.org> From: Marc Zyngier To: Dmitry Baryshkov Cc: Thomas Gleixner , Anup Patel , Konrad Dybcio , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org Subject: Re: [PATCH] irqchip/gic-v3: handle DOMAIN_BUS_ANY in gic_irq_domain_select In-Reply-To: References: <20240219-gic-fix-child-domain-v1-1-09f8fd2d9a8f@linaro.org> <868r3g4fhv.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: dmitry.baryshkov@linaro.org, tglx@linutronix.de, apatel@ventanamicro.com, konrad.dybcio@linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240219_083756_551058_E60A5F59 X-CRM114-Status: GOOD ( 40.07 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 19 Feb 2024 16:21:06 +0000, Dmitry Baryshkov wrote: > > On Mon, 19 Feb 2024 at 17:53, Marc Zyngier wrote: > > > > On Mon, 19 Feb 2024 14:47:37 +0000, > > Dmitry Baryshkov wrote: > > > > > > Before the commit de1ff306dcf4 ("genirq/irqdomain: Remove the param > > > count restriction from select()") the irq_find_matching_fwspec() was > > > handling the DOMAIN_BUS_ANY on its own. After this commit it is a job of > > > the select() callback. However the callback of GICv3 (even though it got > > > modified to handle zero param_count) wasn't prepared to return true for > > > DOMAIN_BUS_ANY bus_token. > > > > > > This breaks probing of any of the child IRQ domains, since > > > platform_irqchip_probe() uses irq_find_matching_host(par_np, > > > DOMAIN_BUS_ANY) to check for the presence of the parent IRQ domain. > > > > > > Fixes: 151378251004 ("irqchip/gic-v3: Make gic_irq_domain_select() robust for zero parameter count") > > > Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count restriction from select()") > > > Signed-off-by: Dmitry Baryshkov > > > --- > > > drivers/irqchip/irq-gic-v3.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > > > index 6fb276504bcc..e9e9643c653f 100644 > > > --- a/drivers/irqchip/irq-gic-v3.c > > > +++ b/drivers/irqchip/irq-gic-v3.c > > > @@ -1696,7 +1696,8 @@ static int gic_irq_domain_select(struct irq_domain *d, > > > > > > /* Handle pure domain searches */ > > > if (!fwspec->param_count) > > > - return d->bus_token == bus_token; > > > + return d->bus_token == bus_token || > > > + bus_token == DOMAIN_BUS_ANY; > > > > > > /* If this is not DT, then we have a single domain */ > > > if (!is_of_node(fwspec->fwnode)) > > > > > > > I really dislike the look of this. If that's the case, any irqchip > > that has a 'select' method (such as imx-intmux) should be similarly > > hacked. And at this point, this should be handled by the core code. > > > > Can you try this instead? I don't have any HW that relies on > > behaviour, but I'd expect this to work. > > > > Thanks, > > > > M. > > > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > > index aeb41655d6de..3dd1c871e091 100644 > > --- a/kernel/irq/irqdomain.c > > +++ b/kernel/irq/irqdomain.c > > @@ -449,7 +449,7 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec, > > */ > > mutex_lock(&irq_domain_mutex); > > list_for_each_entry(h, &irq_domain_list, link) { > > - if (h->ops->select) > > + if (h->ops->select && bus_token != DOMAIN_BUS_ANY) > > rc = h->ops->select(h, fwspec, bus_token); > > else if (h->ops->match) > > rc = h->ops->match(h, to_of_node(fwnode), bus_token); > > This works. But I wonder if the following change is even better. WDYT? > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > index aeb41655d6de..2f0d2700709e 100644 > --- a/kernel/irq/irqdomain.c > +++ b/kernel/irq/irqdomain.c > @@ -449,14 +449,17 @@ struct irq_domain > *irq_find_matching_fwspec(struct irq_fwspec *fwspec, > */ > mutex_lock(&irq_domain_mutex); > list_for_each_entry(h, &irq_domain_list, link) { > - if (h->ops->select) > + if (fwnode != NULL && > + h->fwnode == fwnode && > + bus_token == DOMAIN_BUS_ANY) > + rc = true; > + else if (h->ops->select) > rc = h->ops->select(h, fwspec, bus_token); > else if (h->ops->match) > rc = h->ops->match(h, to_of_node(fwnode), bus_token); > else > rc = ((fwnode != NULL) && (h->fwnode == fwnode) && > - ((bus_token == DOMAIN_BUS_ANY) || > - (h->bus_token == bus_token))); > + (h->bus_token == bus_token)); > > if (rc) { > found = h; > Can't say I like it either. It duplicates the existing check without any obvious benefit. Honestly, this code is shit enough that we should try to make it simpler, not more complex... I'd rather we keep the impact as minimal as possible, and use the upcoming weeks to weed out the effects of these changes (there is another report of some Renesas machine falling over itself here[1]). Thanks, M. [1] https://lore.kernel.org/all/170802702416.398.14922976721740218856.tip-bot2@tip-bot2 -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel