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 <[email protected]>
Tested-by: Dmitry Baryshkov <[email protected]>
Reported-by: Biju Das <[email protected]>
Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count restriction from select()")
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/irq/irqdomain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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);
--
2.39.2
Hi Marc Zyngier,
> -----Original Message-----
> From: Marc Zyngier <[email protected]>
> Sent: Tuesday, February 20, 2024 11:48 AM
> Subject: [PATCH] genirq/irqdomain: Don't call ops->select for
> DOMAIN_BUS_ANY tokens
>
> 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 <[email protected]>
> Tested-by: Dmitry Baryshkov <[email protected]>
> Reported-by: Biju Das <[email protected]>
Tested-by: Biju Das <[email protected]>
Cheers,
Biju
> Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count restriction
> from select()")
> Signed-off-by: Marc Zyngier <[email protected]>
> Link:
> ---
> kernel/irq/irqdomain.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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);
> --
> 2.39.2
The following commit has been merged into the irq/msi branch of tip:
Commit-ID: 5aa3c0cf5bba6437c9e63a56f684f61de8b503d6
Gitweb: https://git.kernel.org/tip/5aa3c0cf5bba6437c9e63a56f684f61de8b503d6
Author: Marc Zyngier <[email protected]>
AuthorDate: Tue, 20 Feb 2024 11:47:31
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 20 Feb 2024 17:30:57 +01:00
genirq/irqdomain: Don't call ops->select for DOMAIN_BUS_ANY tokens
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()"), ops->select() is called 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 functionality of the above helpers.
Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count restriction from select()")
Reported-by: Dmitry Baryshkov <[email protected]>
Reported-by: Biju Das <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Dmitry Baryshkov <[email protected]>
Tested-by: Biju Das <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/[email protected]
---
kernel/irq/irqdomain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index aeb4165..3dd1c87 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);
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 <[email protected]>
> Tested-by: Dmitry Baryshkov <[email protected]>
> Reported-by: Biju Das <[email protected]>
> Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count restriction from select()")
> Signed-off-by: Marc Zyngier <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
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 <[email protected]>
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 <[email protected]>
---
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);
On Sun, Feb 25, 2024 at 05:19:52PM +0100, Thomas Gleixner wrote:
> 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.
The guest boots fine now, thx.
Tested-by: Borislav Petkov (AMD) <[email protected]>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 2024-02-25 16:19, Thomas Gleixner wrote:
> 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 <[email protected]>
>> Tested-by: Dmitry Baryshkov <[email protected]>
>> Reported-by: Biju Das <[email protected]>
>> Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count
>> restriction from select()")
>> Signed-off-by: Marc Zyngier <[email protected]>
>> Link:
>> https://lore.kernel.org/r/[email protected]
>
> 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 <[email protected]>
> 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 <[email protected]>
Looks good to me.
Reviewed-by: Marc Zyngier <[email protected]>
M.
--
Jazz is not dead. It just smells funny...
The following commit has been merged into the irq/msi branch of tip:
Commit-ID: c147e1ef59d4751a60687074e4268ecc0ef31b5c
Gitweb: https://git.kernel.org/tip/c147e1ef59d4751a60687074e4268ecc0ef31b5c
Author: Thomas Gleixner <[email protected]>
AuthorDate: Sun, 25 Feb 2024 16:56:12 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Sun, 25 Feb 2024 18:53:08 +01:00
x86/apic/msi: Use DOMAIN_BUS_GENERIC_MSI for HPET/IO-APIC domain search
The recent restriction to invoke irqdomain_ops::select() only when the
domain bus token is not 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")
Reported-by: Borislav Petkov (AMD) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/878r38cy8n.ffs@tglx
---
arch/x86/kernel/apic/io_apic.c | 2 +-
arch/x86/kernel/hpet.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 40c7cf1..e66c775 100644
--- 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 ioapic)
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);
diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index a38d0c9..c96ae8f 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -568,7 +568,7 @@ static struct irq_domain *hpet_create_irq_domain(int hpet_id)
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);