netdev - Re: [PATCH net-next 0/2] net: phy: sfp: Add single-byte SMBus SFP access

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5e2fef46-f332-4ba1-93af-4e2881ec0c93@lunn.ch>
Date: Tue, 25 Feb 2025 14:29:06 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: Bjørn Mork <bjorn@...k.no>,
	"Russell King (Oracle)" <linux@...linux.org.uk>,
	davem@...emloft.net, Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com,
	Florian Fainelli <f.fainelli@...il.com>,
	Köry Maincent <kory.maincent@...tlin.com>,
	Simon Horman <horms@...nel.org>,
	Romain Gantois <romain.gantois@...tlin.com>,
	Antoine Tenart <atenart@...nel.org>,
	Marek Behún <kabel@...nel.org>
Subject: Re: [PATCH net-next 0/2] net: phy: sfp: Add single-byte SMBus SFP
 access

> > Would SMBus word reads be an alternative for hwmon, if the SMBus
> > controller support those?  Should qualify as "a single two-byte read
> > sequence across the 2-wire interface."
> 
> There are different flavors when it comes to what an SMBus controller
> can do. In the case of what this patchset supports, its really about
> SMBus controllers that can only perform single-byte operations, which
> will cause issues here.
> 
> What I have is a controller that only supports I2C_FUNC_SMBUS_BYTE, in
> that the controller will issue a STOP after reading/writing one byte.
> 
> But if you have a controller that supports, say,
> I2C_FUNC_SMBUS_WORD_DATA (i.e. 16 bits words xfers), that's already a
> different story, as the diags situation Russell mentions will fit in a
> word. That will also make MDIO accesses to embedded PHYs easier, at
> least for C22. 

Agreed.

> > > Whether PHY access works correctly or not is probably module specific.
> > > E.g. reading the MII_BMSR register may not return latched link status
> > > because the reads of the high and low bytes may be interpreted as two
> > > seperate distinct accesses.  
> > 
> > Bear with me.  Trying to learn here.  AFAIU, we only have a defacto
> > specification of the clause 22 phy interface over i2c, based on the
> > 88E1111 implementation.  As Maxime pointed out, this explicitly allows
> > two sequential distinct byte transactions to read or write the 16bit
> > registers. See figures 27 and 30 in
> > https://www.marvell.com/content/dam/marvell/en/public-collateral/transceivers/marvell-phys-transceivers-alaska-88e1111-datasheet.pdf
> > 
> > Looks like the latch timing restrictions are missing, but I still do not
> > think that's enough reason to disallow access to phys over SMBus.  If
> > this is all the interface specification we have?

We are not suggesting it is disallowed. We are simply saying add a
warning that it is possibly broken, bad things can happen, and there
is nothing we can do about it, so don't report issues when it does
break.

It might work enough that users are happy to take the risk, and need
to unplug/plug the cable every so often to get the link back when the
link peer changes etc.

	Andrew