netdev - Re: AM43x boot broken on linux-next

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] [day] [month] [year] [list]

Date:   Wed, 10 Jan 2018 16:08:16 +0530
From:   Keerthy <j-keerthy@...com>
To:     <linux-omap@...r.kernel.org>, <rmk+kernel@...linux.org.uk>,
        <tony@...mide.com>
CC:     Grygorii Strashko <grygorii.strashko@...com>,
        <netdev@...r.kernel.org>, Sekhar Nori <nsekhar@...com>, <nm@...com>
Subject: Re: AM43x boot broken on linux-next



On Wednesday 10 January 2018 03:47 PM, Keerthy wrote:
> 
> 
> On Tuesday 09 January 2018 03:22 PM, Keerthy wrote:
>> On Tuesday 09 January 2018 02:03 PM, Keerthy wrote:
>>> Hi,
>>
>> Adding netdev@...r.kernel.org
>>
>>>
>>> Seems like AM437x boot is broken on latest linux-next.
>>> Log here:
>>>
>>> https://pastebin.ubuntu.com/26351906/
> 
> Git bisect pointed me to this patch:
> 
> commit fea23fb591cce99546baca043d2a068228e87a79
> Author: Russell King <rmk+kernel@...linux.org.uk>
> Date:   Tue Jan 2 10:58:58 2018 +0000
> 
>     net: phy: convert read-modify-write to phy_modify()
> 
> I saw that there was a double negation issue there and i moved to
> Russell's fix patch:
> 
> commit f102852f980eac250855504c18f195900616deec
> Author: Russell King <rmk+kernel@...linux.org.uk>
> Date:   Fri Jan 5 16:07:10 2018 +0000
> 
>     net: phy: fix wrong masks to phy_modify()
> 
> Even with this fix commit i am seeing issues.
> 
> I reverted the fix and the original patch on linux-next and that solves
> the issue.
> 
> So there is more to fix in the original patch IMHO.

Russell,

I found at least one issue with the logic of the original patch here:

@@ -1368,9 +1368,8 @@ static int genphy_config_eee_advert(struct
phy_device *phydev)
  */
 int genphy_setup_forced(struct phy_device *phydev)
 {
-	int ctl = phy_read(phydev, MII_BMCR);
+	u16 ctl = 0;

-	ctl &= BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN;
 	phydev->pause = 0;
 	phydev->asym_pause = 0;

@@ -1382,7 +1381,8 @@ int genphy_setup_forced(struct phy_device *phydev)
 	if (DUPLEX_FULL == phydev->duplex)
 		ctl |= BMCR_FULLDPLX;

-	return phy_write(phydev, MII_BMCR, ctl);
+	return phy_modify(phydev, MII_BMCR,
+			  BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN, ctl);

IMO the original intent of the genphy_setup_forced function was to
preserve the states of BMCR_LOOPBACK | BMCR_ISOLATE | BMCR_PDOWN bits
and clear all other bits..

But after your patch we are deliberately clearing those flags.

So the mask here needs to be negated.

return phy_modify(phydev, MII_BMCR, ~(BMCR_LOOPBACK | BMCR_ISOLATE
    |BMCR_PDOWN), ctl);

This is at least one more fix. Even after this fix boot is still broken.
So there is more to dig.

Regards,
Keerthy

> 
>>>
>>> "Unable to handle kernel paging request at virtual address 000013e8
>>> [    2.367045] pgd = c0350bf7
>>> [    2.370019] [000013e8] *pgd=00000000
>>> [    2.373823] Internal error: Oops: 5 [#1] SMP ARM
>>> [    2.378709] Modules linked in:
>>> [    2.381949] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
>>> 4.15.0-rc6-next-20180108-38410-g895c0dd #4
>>> [    2.391242] Hardware name: Generic AM43 (Flattened Device Tree)
>>> [    2.397523] PC is at phy_attached_print+0xc/0x10c
>>> [    2.402506] LR is at cpsw_slave_open+0x16c/0x258"
>>>
>>> Regards,
>>> Keerthy
>>>