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: <f483c036-3b35-46d1-830e-37741b32d7e1@gmail.com>
Date: Thu, 7 Mar 2024 17:50:08 +0100
From: Eric Woudstra <ericwouds@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>,
 Andrew Lunn <andrew@...n.ch>
Cc: "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 3/5/24 15:24, Russell King (Oracle) wrote:
> 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.

Then I'll move it to .probe().

Best regards,

Eric Woudstra