Re: [PATCH v2 1/3] irqdomain: simplify simple and legacy domain creation

From: Jiaxun Yang
Date: Tue Aug 13 2024 - 06:20:11 EST




On 2024/8/8 13:34, Matti Vaittinen wrote:
Move a bit more logic in the generic __irq_domain_instantiate() from the
irq_domain_create_simple() and the irq_domain_create_legacy(). This does
simplify the irq_domain_create_simple() and irq_domain_create_legacy().
It will also ease the use of irq_domain_instantiate() instead of the
irq_domain_create_simple() or irq_domain_create_legacy() by allowing the
callers of irq_domain_instantiate() to omit the IRQ association and
irq_desc allocation code.

Reduce code duplication by introducing the hwirq_base and virq_base
members in the irq_domain_info structure, creating helper function
for allocating irq_descs, and moving logic from the .._legacy() and
the .._simple() to the more generic irq_domain_instantiate().

Hi all,

This patch currently in next had caused regression on MIPS systems.

[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:1008 __irq_set_handler+0x64/0xa8
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.11.0-rc3-next-20240812 #10
[    0.000000] Hardware name: img,boston
[    0.000000] Stack : 0000000000000034 0000000000000000 0000000000000018 ffffffff80cbba58
[    0.000000]         ffffffff80cbba58 ffffffff80cbbb88 0000000000000000 0000000000000000
[    0.000000]         0000000000000000 0000000000000001 0000000000000000 0000000000000000
[    0.000000]         0000000000000000 0000000000000000 ffffffff80a7ed04 0000000000002000
[    0.000000]         ffffffff80cbb824 0000000000000000 0000000000000000 ffffffff80c35aa0
[    0.000000]         ffffffff80ce0000 ffffffff80ce0000 0000000000000000 0000000000000100
[    0.000000]         ffffffff80ce0000 0000000000000000 ffffffffffffffff 0000000016200001
[    0.000000]         ffffffffb6100088 ffffffff80cb8000 ffffffff80cbba50 ffffffff80c90000
[    0.000000]         ffffffff801096d4 0000000000000000 0000000000000000 0000000000000000
[    0.000000]         0000000000000000 0000000000000000 ffffffff801096ec 0000000000000000
[    0.000000]         ...
[    0.000000] Call Trace:
[    0.000000] [<ffffffff801096ec>] show_stack+0x5c/0x150
[    0.000000] [<ffffffff80a8e2e4>] dump_stack_lvl+0x6c/0xb4
[    0.000000] [<ffffffff80a80e20>] __warn+0x9c/0xcc
[    0.000000] [<ffffffff80135bd8>] warn_slowpath_fmt+0x158/0x1c0
[    0.000000] [<ffffffff801a4d04>] __irq_set_handler+0x64/0xa8
[    0.000000] [<ffffffff80dba50c>] gic_of_init+0x2a8/0x55c
[    0.000000] [<ffffffff80dc4430>] of_irq_init+0x1c8/0x324
[    0.000000] [<ffffffff80da092c>] init_IRQ+0x60/0x120
[    0.000000] [<ffffffff80d9dbd4>] start_kernel+0x4dc/0x658
[    0.000000]
[    0.000000] ---[ end trace 0000000000000000 ]---

This might be caused by drivers/irqchip/irq-mips-cpu.c has irq_domain_add_legacy() called with
first_irq set to 0.

Do you have any idea on how should we fix it?

Thanks
- Jiaxun


Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
Suggested-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>

---
Revision history:
v1 => v2:
- New patch

Please note that this patch has received only limited testing. Any and
all testing with devices using the legacy domains is greatly appreciated
:)
---
include/linux/irqdomain.h | 5 +++
kernel/irq/irqdomain.c | 73 +++++++++++++++++++++------------------
2 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index de6105f68fec..bfcffa2c7047 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -291,6 +291,9 @@ struct irq_domain_chip_generic_info;
* @hwirq_max: Maximum number of interrupts supported by controller
* @direct_max: Maximum value of direct maps;
* Use ~0 for no limit; 0 for no direct mapping
+ * @hwirq_base: The first hardware interrupt number (legacy domains only)
+ * @virq_base: The first Linux interrupt number for legacy domains to
+ * immediately associate the interrupts after domain creation
* @bus_token: Domain bus token
* @ops: Domain operation callbacks
* @host_data: Controller private data pointer
@@ -307,6 +310,8 @@ struct irq_domain_info {
unsigned int size;
irq_hw_number_t hwirq_max;
int direct_max;
+ unsigned int hwirq_base;
+ unsigned int virq_base;
enum irq_domain_bus_token bus_token;
const struct irq_domain_ops *ops;
void *host_data;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index cea8f6874b1f..5af5e4028de2 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -267,13 +267,20 @@ static void irq_domain_free(struct irq_domain *domain)
kfree(domain);
}
-/**
- * irq_domain_instantiate() - Instantiate a new irq domain data structure
- * @info: Domain information pointer pointing to the information for this domain
- *
- * Return: A pointer to the instantiated irq domain or an ERR_PTR value.
- */
-struct irq_domain *irq_domain_instantiate(const struct irq_domain_info *info)
+static void irq_domain_instantiate_descs(const struct irq_domain_info *info)
+{
+ if (!IS_ENABLED(CONFIG_SPARSE_IRQ))
+ return;
+
+ if (irq_alloc_descs(info->virq_base, info->virq_base, info->size,
+ of_node_to_nid(to_of_node(info->fwnode))) < 0) {
+ pr_info("Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n",
+ info->virq_base);
+ }
+}
+
+static struct irq_domain *__irq_domain_instantiate(const struct irq_domain_info *info,
+ bool cond_alloc_descs)
{
struct irq_domain *domain;
int err;
@@ -306,6 +313,14 @@ struct irq_domain *irq_domain_instantiate(const struct irq_domain_info *info)
__irq_domain_publish(domain);
+ if (cond_alloc_descs && info->virq_base > 0)
+ irq_domain_instantiate_descs(info);
+
+ /* Legacy interrupt domains have a fixed Linux interrupt number */
+ if (info->virq_base > 0)
+ irq_domain_associate_many(domain, info->virq_base, info->hwirq_base,
+ info->size - info->hwirq_base);
+
return domain;
err_domain_gc_remove:
@@ -315,6 +330,17 @@ struct irq_domain *irq_domain_instantiate(const struct irq_domain_info *info)
irq_domain_free(domain);
return ERR_PTR(err);
}
+
+/**
+ * irq_domain_instantiate() - Instantiate a new irq domain data structure
+ * @info: Domain information pointer pointing to the information for this domain
+ *
+ * Return: A pointer to the instantiated irq domain or an ERR_PTR value.
+ */
+struct irq_domain *irq_domain_instantiate(const struct irq_domain_info *info)
+{
+ return __irq_domain_instantiate(info, false);
+}
EXPORT_SYMBOL_GPL(irq_domain_instantiate);
/**
@@ -413,28 +439,13 @@ struct irq_domain *irq_domain_create_simple(struct fwnode_handle *fwnode,
.fwnode = fwnode,
.size = size,
.hwirq_max = size,
+ .virq_base = first_irq,
.ops = ops,
.host_data = host_data,
};
- struct irq_domain *domain;
+ struct irq_domain *domain = __irq_domain_instantiate(&info, true);
- domain = irq_domain_instantiate(&info);
- if (IS_ERR(domain))
- return NULL;
-
- if (first_irq > 0) {
- if (IS_ENABLED(CONFIG_SPARSE_IRQ)) {
- /* attempt to allocated irq_descs */
- int rc = irq_alloc_descs(first_irq, first_irq, size,
- of_node_to_nid(to_of_node(fwnode)));
- if (rc < 0)
- pr_info("Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n",
- first_irq);
- }
- irq_domain_associate_many(domain, first_irq, 0, size);
- }
-
- return domain;
+ return IS_ERR(domain) ? NULL : domain;
}
EXPORT_SYMBOL_GPL(irq_domain_create_simple);
@@ -476,18 +487,14 @@ struct irq_domain *irq_domain_create_legacy(struct fwnode_handle *fwnode,
.fwnode = fwnode,
.size = first_hwirq + size,
.hwirq_max = first_hwirq + size,
+ .hwirq_base = first_hwirq,
+ .virq_base = first_irq,
.ops = ops,
.host_data = host_data,
};
- struct irq_domain *domain;
+ struct irq_domain *domain = irq_domain_instantiate(&info);
- domain = irq_domain_instantiate(&info);
- if (IS_ERR(domain))
- return NULL;
-
- irq_domain_associate_many(domain, first_irq, first_hwirq, size);
-
- return domain;
+ return IS_ERR(domain) ? NULL : domain;
}
EXPORT_SYMBOL_GPL(irq_domain_create_legacy);

Attachment: OpenPGP_0xC111949073FC0F67.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature