Received-SPF: pass (google.com: domain of linux-kernel+bounces-80197-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Thomas Gleixner <tglx@linutronix.de>
To: Marc Zyngier <maz@kernel.org>, linux-kernel@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Biju Das
 <biju.das.jz@bp.renesas.com>, x86@kernel.org
Subject: Re: [PATCH] genirq/irqdomain: Don't call ops->select for
 DOMAIN_BUS_ANY tokens
In-Reply-To: <20240220114731.1898534-1-maz@kernel.org>
References: <20240220114731.1898534-1-maz@kernel.org>
Date: Sun, 25 Feb 2024 17:19:52 +0100
Message-ID: <878r38cy8n.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

On Tue, Feb 20 2024 at 11:47, Marc Zyngier wrote:
> Users of the IRQCHIP_PLATFORM_DRIVER_{BEGIN,END} helpers rely
> on a fwspec containing only the fwnode (and crucially a number
> of parameters set to 0) together with a DOMAIN_BUS_ANY token
> to check whether a parent irqchip has probed and registered
> a domain.
>
> Since de1ff306dcf4 ("genirq/irqdomain: Remove the param count
> restriction from select()"), we call ops->select unconditionally,
> meaning that irqchips implementing select now need to handle
> ANY as a match.
>
> Instead of adding more esoteric checks to the individual drivers,
> add that condition to irq_find_matching_fwspec(), and let it
> handle the corner case, as per the comment in the function.
>
> This restores the functionnality of the above helpers.
>
> Reported-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reported-by: Biju Das <biju.das.jz@bp.renesas.com>
> Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count restriction from select()")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Link: https://lore.kernel.org/r/20240219-gic-fix-child-domain-v1-1-09f8fd2d9a8f@linaro.org

Bah. That breaks x86 because it uses DOMAIN_BUS_ANY to find the MSI
parent for a fwspec (IOAPIC and HPET) which gets either picked up by the
interrupt remapping or by the root vector domain.

Fix below.

Thanks,

        tglx
---
Subject: x86/apic/msi: Use DOMAIN_BUS_GENERIC_MSI for HPET/IO-APIC domain search
From: Thomas Gleixner <tglx@linutronix.de>
Date: Sun, 25 Feb 2024 16:56:12 +0100

The recent restriction to invoke irqdomain_ops::select() only when the
domain bus toke is DOMAIN_BUS_ANY breaks the search for the parent MSI
domain of HPET and IO-APIC. The latter causes a full boot fail.

The restriction itself makes sense to avoid adding DOMAIN_BUS_ANY matches
into the various ARM specific select() callbacks. Reverting this change
would obviously break ARM platforms again and require DOMAIN_BUS_ANY
matches added to various places.

A simpler solution is to use the DOMAIN_BUS_GENERIC_MSI token for the HPET
and IO-APIC parent domain search. This works out of the box because the
affected parent domains check only for the firmware specification content
and not for the bus token.

Fixes: 5aa3c0cf5bba ("genirq/irqdomain: Don't call ops->select for DOMAIN_BUS_ANY tokens")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/apic/io_apic.c |    2 +-
 arch/x86/kernel/hpet.c         |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2354,7 +2354,7 @@ static int mp_irqdomain_create(int ioapi
 	fwspec.param_count = 1;
 	fwspec.param[0] = mpc_ioapic_id(ioapic);
 
-	parent = irq_find_matching_fwspec(&fwspec, DOMAIN_BUS_ANY);
+	parent = irq_find_matching_fwspec(&fwspec, DOMAIN_BUS_GENERIC_MSI);
 	if (!parent) {
 		if (!cfg->dev)
 			irq_domain_free_fwnode(fn);
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -568,7 +568,7 @@ static struct irq_domain *hpet_create_ir
 	fwspec.param_count = 1;
 	fwspec.param[0] = hpet_id;
 
-	parent = irq_find_matching_fwspec(&fwspec, DOMAIN_BUS_ANY);
+	parent = irq_find_matching_fwspec(&fwspec, DOMAIN_BUS_GENERIC_MSI);
 	if (!parent) {
 		irq_domain_free_fwnode(fn);
 		kfree(domain_info);