linux-kernel - Re: [PATCH] irqchip/loongson-pch-lpc: Use legacy domain for PCH-LPC IRQ controller

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] [day] [month] [year] [list]

Message-ID: <fb4e8ee2-ff13-4446-aa23-57f0332d0342@loongson.cn>
Date: Thu, 11 Sep 2025 15:29:27 +0800
From: Ming Wang <wangming01@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: Jiaxun Yang <jiaxun.yang@...goat.com>,
 Thomas Gleixner <tglx@...utronix.de>, linux-mips@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] irqchip/loongson-pch-lpc: Use legacy domain for PCH-LPC
 IRQ controller

Hi Huacai,

On 9/11/25 12:49, Huacai Chen wrote:
> Hi, Wangming,
> 
> On Tue, Sep 9, 2025 at 8:58 PM Ming Wang <wangming01@...ngson.cn> wrote:
>>
>> On certain Loongson platforms, drivers attempting to request a legacy
>> ISA IRQ directly via request_irq() (e.g., IRQ 4) may fail. The
>> virtual IRQ descriptor is not fully initialized and lacks a valid irqchip.
>>
>> This issue does not affect ACPI-enumerated devices described in DSDT,
>> as their interrupts are properly mapped via the GSI translation path.
>> This indicates the LPC irqdomain itself is functional but is not correctly
>> handling direct VIRQ-to-HWIRQ mappings.
>>
>> The root cause is the use of irq_domain_create_linear(). This API sets
>> up a domain for dynamic, on-demand mapping, typically triggered by a GSI
>> request. It does not pre-populate the mappings for the legacy VIRQ range
>> (0-15). Consequently, if no ACPI device claims a specific GSI
>> (e.g., GSI 4), the corresponding VIRQ (e.g., VIRQ 4) is never mapped to
>> the LPC domain. A direct call to request_irq(4, ...) then fails because
>> the kernel cannot resolve this VIRQ to a hardware interrupt managed by
>> the LPC controller.
> If we create a legacy domain, the VIRQ0~15 are pre-allocated, then an
> ACPI device claims GSI 4, what will happen?
There will be no conflict. When the ACPI GSI mapping function is called,
it first uses irq_find_mapping() to check for an existing mapping.

Since the legacy domain has already created the mapping, the lookup
will succeed. The function will then simply return the existing VIRQ 4,
bypassing any new allocation logic.

The simplified call flow for an ACPI device claiming GSI 4 (after my patch)
is as follows:

acpi_register_gsi(..., .gsi=4, ...)
   -> irq_create_fwspec_mapping(fwspec)
     -> domain = irq_find_matching_fwspec(...)  // Finds lpc_domain
     -> irq_domain_translate(...)               // Translates fwspec to 
hwirq=4
     -> virq = irq_find_mapping(lpc_domain, 4)  // SUCCESS, finds 
existing VIRQ 4
     -> return virq;                            // Returns 4

> 
>>
>> The PCH-LPC interrupt controller is an i8259-compatible legacy device
>> that requires a deterministic, static 1-to-1 mapping for IRQs 0-15 to
>> support legacy drivers.
>>
>> Fix this by replacing irq_domain_create_linear() with
>> irq_domain_create_legacy(). This API is specifically designed for such
>> controllers. It establishes the required static 1-to-1 VIRQ-to-HWIRQ
>> mapping for the entire legacy range (0-15) immediately upon domain
>> creation. This ensures that any VIRQ in this range is always resolvable,
>> making direct calls to request_irq() for legacy IRQs function correctly.
>>
>> Signed-off-by: Ming Wang <wangming01@...ngson.cn>
>> ---
>>   drivers/irqchip/irq-loongson-pch-lpc.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-pch-lpc.c b/drivers/irqchip/irq-loongson-pch-lpc.c
>> index 2d4c3ec128b8..68b09cc8c400 100644
>> --- a/drivers/irqchip/irq-loongson-pch-lpc.c
>> +++ b/drivers/irqchip/irq-loongson-pch-lpc.c
>> @@ -200,8 +200,13 @@ int __init pch_lpc_acpi_init(struct irq_domain *parent,
>>                  goto iounmap_base;
>>          }
>>
>> -       priv->lpc_domain = irq_domain_create_linear(irq_handle, LPC_COUNT,
>> -                                       &pch_lpc_domain_ops, priv);
>> +       /*
>> +        * The LPC interrupt controller is a legacy i8259-compatible device,
>> +        * which requires a static 1:1 mapping for IRQs 0-15.
>> +        * Use irq_domain_create_legacy to establish this static mapping early.
> irq_domain_create_legacy()
OK, I will fix it in the v2 patch.

> 
>> +        */
>> +       priv->lpc_domain = irq_domain_create_legacy(irq_handle, LPC_COUNT, 0, 0,
>> +                       &pch_lpc_domain_ops, priv);
> The recommended indentation is:
>         priv->lpc_domain = irq_domain_create_legacy(irq_handle, LPC_COUNT, 0, 0,
> 
>                     &pch_lpc_domain_ops, priv);
> 
> which means "&" is below "irq_handle".

You are right. I will fix the indentation in the v2 patch.

Thanks,
Ming

> 
> Huacai
> 
>>          if (!priv->lpc_domain) {
>>                  pr_err("Failed to create IRQ domain\n");
>>                  goto free_irq_handle;
>> --
>> 2.43.0
>>