lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <27033022-cdbe-40d9-8a97-cdc4d5c25dbe@flygoat.com>
Date: Tue, 13 Aug 2024 11:19:46 +0100
From: Jiaxun Yang <jiaxun.yang@...goat.com>
To: Matti Vaittinen <mazziesaccount@...il.com>,
 Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
 Thomas Gleixner <tglx@...utronix.de>
Cc: Mark Brown <broonie@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] irqdomain: simplify simple and legacy domain
 creation



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@...il.com>
> Suggested-by: Thomas Gleixner <tglx@...utronix.de>
>
> ---
> 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);
>   


Download attachment "OpenPGP_0xC111949073FC0F67.asc" of type "application/pgp-keys" (16944 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ