linux-kernel - Re: [PATCH v5] PCI: Xilinx NWL: Modifying irq chip for legacy interrupts

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: <2196f028-b834-9d32-cc3a-e3eb5ed6c3b1@arm.com>
Date:   Thu, 9 Feb 2017 16:03:14 +0000
From:   Marc Zyngier <marc.zyngier@....com>
To:     Bharat Kumar Gogada <bharat.kumar.gogada@...inx.com>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "robh@...nel.org" <robh@...nel.org>,
        "paul.gortmaker@...driver.com" <paul.gortmaker@...driver.com>,
        "colin.king@...onical.com" <colin.king@...onical.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
Cc:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "michal.simek@...inx.com" <michal.simek@...inx.com>,
        "arnd@...db.de" <arnd@...db.de>,
        Ravikiran Gummaluri <rgummal@...inx.com>
Subject: Re: [PATCH v5] PCI: Xilinx NWL: Modifying irq chip for legacy
 interrupts

On 09/02/17 15:16, Bharat Kumar Gogada wrote:
>>
>> On 09/02/17 12:01, Bharat Kumar Gogada wrote:
>>>> On 06/02/17 07:03, Bharat Kumar Gogada wrote:
>>>>> +static struct irq_chip nwl_leg_irq_chip = {
>>>>> +	.name = "nwl_pcie:legacy",
>>>>> +	.irq_enable = nwl_unmask_leg_irq,
>>>>> +	.irq_disable = nwl_mask_leg_irq,
>>>>
>>>> You don't need these two if they are implemented in terms of mask/unmask.
>>>
>>> These are being invoked by some drivers other than interrupt flow.
>>> Ex: drivers/net/wireless/ath/ath9k/main.c
>>> static int ath_reset_internal(struct ath_softc *sc, struct
>>> ath9k_channel *hchan) {
>>>          ....
>>>          disable_irq(sc->irq);
>>>          tasklet_disable(&sc->intr_tq);
>>>         ...
>>>         ...
>>>         enable_irq(sc->irq);
>>>         spin_unlock_bh(&sc->sc_pcu_lock); } For us masking/unmasking
>>> is the way to enable/disable interrupts.
>>
>> And if you looked at the way disable_irq is implemented, you would have found
>> out that it falls back to masking if there is no disable method, preserving the
>> semantic you expect.
>>
> Yes I did see, but this fall back requires extra "IRQ_DISABLE_UNLAZY" flag to be set to each virq. 

No it doesn't. If you do a disable_irq(), the interrupt is flagged as
disabled, but nothing gets done. If an interrupt actually fires, then
the interrupts gets masked, and the handler is not called.

So just drop these two methods, because if this doesn't work, you have
bigger issues.

	M.
-- 
Jazz is not dead. It just smells funny...