linux-kernel - Re: [PATCH V1 1/4] irqchip/loongson-pch-pic: Fix initialization of HT vector register

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: <5dc1b15b-278e-65b4-7696-e9ae23c0ea59@loongson.cn>
Date:   Mon, 22 May 2023 16:49:01 +0800
From:   Jianmin Lv <lvjianmin@...ngson.cn>
To:     loongson-kernel@...ts.loongnix.cn,
        Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <maz@...nel.org>
Cc:     linux-kernel@...r.kernel.org, loongarch@...ts.linux.dev,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Huacai Chen <chenhuacai@...ngson.cn>, stable@...r.kernel.org
Subject: Re: [PATCH V1 1/4] irqchip/loongson-pch-pic: Fix initialization of HT
 vector register



On 2023/5/21 下午6:29, WANG Xuerui wrote:
> Hi,
> 
> On 2023/5/20 14:38, Jianmin Lv wrote:
>> In a dual-bridge system based ACPI, the IRQ on PCH PIC of
> 
> "In an ACPI-based dual-bridge system"?
> 
> Also what's "the IRQ on PCH PIC of each bridge", is it "IRQ of each 
> bridge's PCH PIC"? Or did I misunderstand it?
> 

Thanks for your comment here, I'll change them in next version.

>> each bridge sent to CPU is always a zero-based number, which
>> means that the IRQ on PCH PIC of each bridge is mapped into
>> vector range from 0 to 63 of upstream irqchip(e.g. EIOINTC).
>>
>>        EIOINTC N: [0 ... 63 | 64 ... 255]
>>                    --------   ----------
>>                        ^          ^
>>                        |          |
>>                    PCH PIC N      |
>>                               PCH MSI N
>>
>> For example, the IRQ vector number of sata controller on
>> PCH PIC of each bridge is 16, which is sent to upstream
>> irqchip of EIOINTC when an interrupt occurs, which will set
>> bit 16 of EIOINTC. Since hwirq of 16 on EIOINTC has been
>> mapped to a irq_desc for sata controller during hierarchy
>> irq allocation, the related mapped IRQ will be found through
>> irq_resolve_mapping() in the IRQ domain of EIOINTC.
>>
>> So, the IRQ number set in HT vector register should be fixed
>> to be a zero-based number.
>>
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Jianmin Lv <lvjianmin@...ngson.cn>
>> Signed-off-by: liuyun <liuyun@...ngson.cn>
> 
> What's the patch's proper authorship, Yun Liu or you? Based on the 
> ordering of the tags it seems you should be the author, but Yun should 
> be submitting (because their tag comes later, the patch should be 
> flowing from your tree to theirs); otherwise they should be the author 
> and the two signoff lines should be reversed.
> 

Ok, I'll change them as following:
Co-developed-by: liuyun <liuyun@...ngson.cn>
Signed-off-by: liuyun <liuyun@...ngson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@...ngson.cn>

Thanks.

>> ---
>>   drivers/irqchip/irq-loongson-pch-pic.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c 
>> b/drivers/irqchip/irq-loongson-pch-pic.c
>> index e5fe4d50be05..921c5c0190d1 100644
>> --- a/drivers/irqchip/irq-loongson-pch-pic.c
>> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
>> @@ -401,14 +401,12 @@ static int __init acpi_cascade_irqdomain_init(void)
>>   int __init pch_pic_acpi_init(struct irq_domain *parent,
>>                       struct acpi_madt_bio_pic *acpi_pchpic)
>>   {
>> -    int ret, vec_base;
>> +    int ret;
>>       struct fwnode_handle *domain_handle;
>>       if (find_pch_pic(acpi_pchpic->gsi_base) >= 0)
>>           return 0;
>> -    vec_base = acpi_pchpic->gsi_base - GSI_MIN_PCH_IRQ;
>> -
>>       domain_handle = irq_domain_alloc_fwnode(&acpi_pchpic->address);
>>       if (!domain_handle) {
>>           pr_err("Unable to allocate domain handle\n");
>> @@ -416,7 +414,7 @@ int __init pch_pic_acpi_init(struct irq_domain 
>> *parent,
>>       }
>>       ret = pch_pic_init(acpi_pchpic->address, acpi_pchpic->size,
>> -                vec_base, parent, domain_handle, acpi_pchpic->gsi_base);
>> +                0, parent, domain_handle, acpi_pchpic->gsi_base);
>>       if (ret < 0) {
>>           irq_domain_free_fwnode(domain_handle);
>