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

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: <78fabd1a-0c68-4e23-8293-89c56eb9010b@gmail.com>
Date: Tue, 13 Aug 2024 13:54:19 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Jiaxun Yang <jiaxun.yang@...goat.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 8/13/24 13:19, Jiaxun Yang wrote:
> 
> 
> 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.
> 

Right. Thanks for the report! I do appreciate this testing!

first_irq gets assigned to the info->virq_base making it 0.

Later, in the __irq_domain_instantiate() we use the info->virq_base as a 
condition for
	if (info->virq_base > 0)
                 irq_domain_associate_many()
which was previously unconditionally called in the irq_domain_add_legacy().

> Do you have any idea on how should we fix it?
Maybe we could add a flag for domain association similar to the "bool 
cond_alloc_descs", but maybe Thomas has some better ideas...?


Yours,
	-- Matti
-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~