netdev - RE: [PATCH v1 1/1] net: phy: micrel: Add workaround for incomplete autonegotiation

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:
 <PH7PR12MB7282617140D3D2F2F84869DBD7702@PH7PR12MB7282.namprd12.prod.outlook.com>
Date: Fri, 19 Jan 2024 20:18:00 +0000
From: Asmaa Mnebhi <asmaa@...dia.com>
To: Andrew Lunn <andrew@...n.ch>
CC: "Russell King (Oracle)" <linux@...linux.org.uk>, "davem@...emloft.net"
	<davem@...emloft.net>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	David Thompson <davthompson@...dia.com>
Subject: RE: [PATCH v1 1/1] net: phy: micrel: Add workaround for incomplete
 autonegotiation

> > >
> > Hi Andrew,
> >
> > Thank you for your feedback.
> 
> Lets try to figure out some more about the situation when it fails to link up.
> 
> What is the value of BMSR when it fails to report complete? You say you are
> using interrupts, so i just want to make sure its not an interrupt problem, you
> are using edge interrupts instead of level, etc.  Maybe i'm remembering wrong,
> but i though i made a comment about this once when reviewing one of your
> drivers. What about the contents of registers 0x1b and 0x1f?
> 
Yes I dumped all PHY registers and didn't see anything suspicious besides the autonegotiation not completing:
root@...alhost:~/phytool# ./phytool/phytool read oob_net0/0x3/0x0
0x1140

root@...alhost:~/phytool# ./phytool/phytool read oob_net0/0x3/0x1
0x7949 //aneg didnt complete and link failed

root@...alhost:~/phytool# ./phytool/phytool read oob_net0/0x3/0x9
0x0200 // correct advertisement from PHY

root@...alhost:~/phytool# ./phytool/phytool read oob_net0/0x3/0xA
0000 // nothing detected on link partner 

root@...alhost:~/phytool# ./phytool/phytool read oob_net0/0x3/0xF
0x3000 // correct ability advertised

root@...alhost:~/phytool# ./phytool/phytool read oob_net0/0x3/0x15
0000 // no errors

root@...alhost:~/phytool# ./phytool/phytool read oob_net0/0x3/0x1B
0x0500 // no pending interrupts

root@...alhost:~/phytool# ./phytool/phytool read oob_net0/0x3/0x1F
0x0300 


I also added the following debug prints. Please see comment next to them if they were printed or not during the reproduction.

--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
static int ksz9031_read_status(struct phy_device *phydev)
        int regval; 
        err = genphy_read_status(phydev);
+        if (err) {
+            printk("ksz9031 genphy_read_status failed"); //not printed
....
          regval = phy_read(phydev, MII_STAT1000);
+        if ((regval & 0xFF) == 0xFF) {
+          printk("ksz9031 err"); //not printed


--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
      void phy_state_machine(struct work_struct *work)
        if (needs_aneg) {
+        printk("needs_aneg"); //printed
          err = phy_start_aneg(phydev);


--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1718,6 +1718,8 @@ int __genphy_config_aneg(struct phy_device *phydev, bool changed)
                        changed = true; /* do restart aneg */
        }
 
+       printk("restart = %d", restart); // prints "restart = 1" so autonegotiation is restarted by the line below.

        /* Only restart aneg if we are advertising something different
         * than we were before.
   

The above prints proved that the micrel PHY started autonegotiation but the result is that it failed to complete it. I also noticed that the KSZ9031 PHY takes ~5 full seconds to complete aneg which is much longer than other PHYs like VSC8221 (which we use with BlueField-3 systems).

Regarding this comment in your next email:

"Please could you check that /proc/interrupts do show level interrupts are being used."
I don't think the problem is the interrupt. The interrupt for link up is issued only when autonegotiation is completed. If autonegotiation is not completed the link just stays down as indicated in PHY register 1, and no interrupt is issued.

Thanks.
Asmaa