linux-kernel - Re: [PATCH] irqchip/mbigen: Fix the clear register offset

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]

Date:   Thu, 27 Apr 2017 09:27:14 +0800
From:   "majun (Euler7)" <majun258@...wei.com>
To:     Marc Zyngier <marc.zyngier@....com>,
        Hanjun Guo <hanjun.guo@...aro.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <tglx@...utronix.de>,
        <dingtianhong@...wei.com>, <guohanjun@...wei.com>
CC:     <majun258@...wei.com>
Subject: Re: [PATCH] irqchip/mbigen: Fix the clear register offset

Hi Marc:

在 2017/4/26 16:01, Marc Zyngier 写道:
> On 26/04/17 04:10, Hanjun Guo wrote:
>> Hi Majun,
>>
>> On 2017/4/25 10:16, Majun wrote:
>>> From: MaJun <majun258@...wei.com>
>>>
>>> Don't minus reserved interrupts (64) when get the clear register offset,because
>>> the clear register space includes the space of these 64 interrupts.
>>
>> Could you mention the background that there is a timeout mechanism
>> to clear the register in the mbigen to make the code work even we clear
>> the wrong (and noneffective) register? that will help for review I
>> think.
> 
> A timeout? So if you don't clear the interrupt in a timely manner, it
> will still bypass the masking? That feels very wrong. How is this
> timeout configured? Can it be entirely disabled?

You are right. The timeout should be turn off usually, it's just a debug or
testing function in our chip. Because of configuration mistake in BIOS, this function
is turned on now and covered the bug in clear offset calculate.
Now, it's time to fix this.

Thanks
MaJun

> 
>>
>>>
>>> Signed-off-by: MaJun <majun258@...wei.com>
>>> ---
>>>  drivers/irqchip/irq-mbigen.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>>> index 061cdb8..75818a5 100644
>>> --- a/drivers/irqchip/irq-mbigen.c
>>> +++ b/drivers/irqchip/irq-mbigen.c
>>> @@ -108,7 +108,6 @@ static inline void get_mbigen_clear_reg(irq_hw_number_t hwirq,
>>>  {
>>>  	unsigned int ofst;
>>>
>>> -	hwirq -= RESERVED_IRQ_PER_MBIGEN_CHIP;
>>>  	ofst = hwirq / 32 * 4;
>>>
>>>  	*mask = 1 << (hwirq % 32);
>>
>> How about following to save more lines of code:
>>
>> --- a/drivers/irqchip/irq-mbigen.c
>> +++ b/drivers/irqchip/irq-mbigen.c
>> @@ -106,10 +106,7 @@ static inline void 
>> get_mbigen_type_reg(irq_hw_number_t hwirq,
>>   static inline void get_mbigen_clear_reg(irq_hw_number_t hwirq,
>>                                          u32 *mask, u32 *addr)
>>   {
>> -       unsigned int ofst;
>> -
>> -       hwirq -= RESERVED_IRQ_PER_MBIGEN_CHIP;
>> -       ofst = hwirq / 32 * 4;
>> +       unsigned int ofst = hwirq / 32 * 4;
>>
>>          *mask = 1 << (hwirq % 32);
>>          *addr = ofst + REG_MBIGEN_CLEAR_OFFSET;
> 
> Well, this is not a code deletion contest... ;-)
> 
> 	M.
>