netdev - Re: [PATCH] net: phylink: dsa: mv88e6xxx: Revise irq setup ordering

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:   Mon, 4 Feb 2019 16:38:53 -0500
From:   John David Anglin <dave.anglin@...l.net>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Russell King <linux@....linux.org.uk>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH] net: phylink: dsa: mv88e6xxx: Revise irq setup ordering

On 2019-02-04 3:19 p.m., Andrew Lunn wrote:
> The IRQ core would do this if it was needed.
>
> How many other irq thread work functions can you point to which do
> something similar?
This is comment for handle_edge_irq:

/**
 *    handle_edge_irq - edge type IRQ handler
 *    @desc:    the interrupt description structure for this irq
 *
 *    Interrupt occures on the falling and/or rising edge of a hardware
 *    signal. The occurrence is latched into the irq controller hardware
 *    and must be acked in order to be reenabled. After the ack another
 *    interrupt can happen on the same source even before the first one
 *    is handled by the associated event handler. If this happens it
 *    might be necessary to disable (mask) the interrupt depending on the
 *    controller hardware. This requires to reenable the interrupt inside
 *    of the loop which handles the interrupts which have arrived while
 *    the handler was running. If all pending interrupts are handled, the
 *    loop is left.
 */

As can be seen, the above comment suggests that it may be necessary to
disable (mask) interrupt
as I proposed.

I see no evidence from the Marvell functional specifications for the
88E6341 that it sequences
interrupts from the various sources although it might be that device
interrupts are sequenced
so INTn rises and falls.  I haven't seen any ports fail to link without
the hunk on espressobin
but it is hard to stress test the code.

Disabling and re-enabling interrupts in the global control register does
not affect their status.
Thus, at worst, the hunk adds a bit of unnecessary code.  It could be
skipped if we knew we
were using level interrupts.

Dave

-- 
John David Anglin  dave.anglin@...l.net