netdev - Re: [PATCH v2 net-next 2/2] net: phy: air_en8811h: Add the Airoha EN8811H PHY driver

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: <ZecrGTsBZ9VgsGZ+@shell.armlinux.org.uk>
Date: Tue, 5 Mar 2024 14:24:25 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>
Cc: Eric Woudstra <ericwouds@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Frank Wunderlich <frank-w@...lic-files.de>,
	Daniel Golle <daniel@...rotopia.org>,
	Lucien Jheng <lucien.jheng@...oha.com>,
	Zhi-Jun You <hujy652@...tonmail.com>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH v2 net-next 2/2] net: phy: air_en8811h: Add the Airoha
 EN8811H PHY driver

On Tue, Mar 05, 2024 at 03:06:45PM +0100, Andrew Lunn wrote:
> > The only way I can see around this problem would be to look up the
> > PHY in order to get a pointer to the struct phy_device in the network
> > device's probe function, and attach the PHY there _before_ you register
> > the network device. You can then return EPROBE_DEFER and, because you
> > are returning it in a .probe function, the probe will be retried once
> > other probes in the system (such as your PHY driver) have finished.
> > This also means that userspace doesn't see the appearance of the
> > non-functional network device until it's ready, and thus can use
> > normal hotplug mechanisms to notice the network device.
> 
> What i'm thinking is we add another op to phy_driver dedicated to
> firmware download. We let probe run as is, so the PHY is registered
> and available. But if the firmware op is set, we start a thread and
> call the op in it. Once the op exits, we signal a completion event.
> phy_attach_direct() would then wait on the completion.

That's really not good, because phy_attach_direct() can be called
from .ndo_open, which will result in the rtnl lock being held while
we wait - so this is not much better than having the firmware load
in .config_init.

If we drop the lock, then we need to audit what the effect of that
would be - for example, if the nic is being opened, it may mean
that another attempt to open the nic could be started. Or it may
mean that an attempt to configure the nic down could be started.
Then the original open proceeds and state is now messed up.

I do get the feeling that trying to work around "I don't want the
firmware in the initramfs" is creating more problems and pain than
it's worth.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!