netdev - Re: [PATCH 2/2] at803x: double check SGMII side autoneg

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:   Wed, 24 May 2017 14:36:45 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Timur Tabi <timur@....qualcomm.com>, Andrew Lunn <andrew@...n.ch>
Cc:     Matthias May <matthias.may@...atec.com>,
        Zefir Kurtisi <zefir.kurtisi@...atec.com>,
        netdev@...r.kernel.org, David Miller <davem@...emloft.net>,
        Manoj Iyer <manoj.iyer@...onical.com>, jhugo@...eaurora.org
Subject: Re: [PATCH 2/2] at803x: double check SGMII side autoneg

On 05/24/2017 02:32 PM, Timur Tabi wrote:
> On 05/24/2017 04:28 PM, Florian Fainelli wrote:
> 
>> Yes phydev->lock which is used to serialize the state machine state
>> changes.
>>
>> Most PHYs have many more registers than the 15 standard exposed
>> directly, and so you need indirect reads/writes to access these
>> registers, which typically involve switching a particular page, doing
>> the indirect register access, and then flipping the page back. If you
>> interrupt that scheme one way or another, your reads and writes are all
>> messed up.
> 
> Ah, and the at803x is a device like that.
> 
> At worst, the autopoll feature could read a register from the wrong
> page, and think that the link state has changed when it hasn't.  But
> that's still bad, and all my problems do revolve around link states.

Absolutely, just like it's not clear whether autopoll does read
BMSR.LINKSTAT or another at803x specific register? Also how is
BMSR.LINKSTAT defined on at803x when there is SGMII + Copper involved?

> 
>>> I forgot one detail.  Every time you do an MDIO read/write, it
>>> temporarily disables the feature.  Although, I think that's not relevant
>>> to your point.
>>
>> Is that done by the HW itself, or is this under SW control exclusively.
> 
> Software.

OK, and there is no way you can run into the following race condition:

CPU				HW
MDIO read intent
				polling starts
disable HW autopoll
				polling continues
MDIO read is done
				MDIO read done
				polling stops
MDIO read value returned


if you disable autopolling in HW this is guaranteed to immediately stop
by the time the register value is seen in HW and your I/O read/write
returns?
-- 
Florian