| 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
| ||
|
Date: Thu, 16 Apr 2020 23:38:56 +0300
From: Lauri Jakku <lauri.jakku@...inet.fi>
To: Heiner Kallweit <hkallweit1@...il.com>
Cc: Leon Romanovsky <leon@...nel.org>, netdev@...r.kernel.org,
nic_swsd@...ltek.com
Subject: Re: NET: r8168/r8169 identifying fix
Hi
On 16.4.2020 23.10, Lauri Jakku wrote:
>
> On 16.4.2020 23.02, Heiner Kallweit wrote:
>> On 16.04.2020 21:58, Lauri Jakku wrote:
>>> Hi,
>>>
>>> On 16.4.2020 21.37, Lauri Jakku wrote:
>>>> Hi,
>>>>
>>>> On 16.4.2020 21.26, Heiner Kallweit wrote:
>>>>> On 16.04.2020 13:30, Lauri Jakku wrote:
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> 5.6.3-2-MANJARO: stock manjaro kernel, without modifications -->
>>>>>> network does not work
>>>>>>
>>>>>> 5.6.3-2-MANJARO-lja: No attach check, modified kernel (r8169 mods
>>>>>> only) --> network does not work
>>>>>>
>>>>>> 5.6.3-2-MANJARO-with-the-r8169-patch: phy patched + r8169 mods ->
>>>>>> devices show up ok, network works
>>>>>>
>>>>>> All different initcpio's have realtek.ko in them.
>>>>>>
>>>>> Thanks for the logs. Based on the logs you're presumable affected
>>>>> by a known BIOS bug.
>>>>> Check bug tickets 202275 and 207203 at bugzilla.kernel.org.
>>>>> In the first referenced tickets it's about the same mainboard
>>>>> (with earlier BIOS version).
>>>>> BIOS on this mainboard seems to not initialize the network chip /
>>>>> PHY correctly, it reports
>>>>> a random number as PHY ID, resulting in no PHY driver being found.
>>>>> Enable "Onboard LAN Boot ROM" in the BIOS, and your problem should
>>>>> be gone.
>>>>>
>>>> OK, I try that, thank you :)
>>>>
>>> It seems that i DO have the ROM's enabled, i'm now testing some
>>> mutex guard for phy state and try to use it as indicator
>>>
>>> that attach has been done. One thing i've noticed is that driver
>>> needs to be reloaded to allow traffic (ie. ping works etc.)
>>>
>> All that shouldn't be needed. Just check with which PHY ID the PHY
>> comes up.
>> And what do you mean with "it seems"? Is the option enabled or not?
>>
> I do have ROM's enabled, and it does not help with my issue.
I check the ID, and revert all other changes, and check how it is
working after adding the PHY id to list.
>>>>>> The problem with old method seems to be, that device does not
>>>>>> have had time to attach before the
>>>>>> PHY driver check.
>>>>>>
>>>>>> The patch:
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c
>>>>>> b/drivers/net/ethernet/realtek/r8169_main.c
>>>>>> index bf5bf05970a2..acd122a88d4a 100644
>>>>>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>>>>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>>>>>> @@ -5172,11 +5172,11 @@ static int r8169_mdio_register(struct
>>>>>> rtl8169_private *tp)
>>>>>> if (!tp->phydev) {
>>>>>> mdiobus_unregister(new_bus);
>>>>>> return -ENODEV;
>>>>>> - } else if (!tp->phydev->drv) {
>>>>>> + } else if (tp->mac_version == RTL_GIGA_MAC_NONE) {
>>>>>> /* Most chip versions fail with the genphy driver.
>>>>>> * Therefore ensure that the dedicated PHY
>>>>>> driver is loaded.
>>>>>> */
>>>>>> - dev_err(&pdev->dev, "realtek.ko not loaded, maybe
>>>>>> it needs to be added to initramfs?\n");
>>>>>> + dev_err(&pdev->dev, "Not known MAC version.\n");
>>>>>> mdiobus_unregister(new_bus);
>>>>>> return -EUNATCH;
>>>>>> }
>>>>>> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
>>>>>> index 66b8c61ca74c..aba2b304b821 100644
>>>>>> --- a/drivers/net/phy/phy-core.c
>>>>>> +++ b/drivers/net/phy/phy-core.c
>>>>>> @@ -704,6 +704,10 @@ EXPORT_SYMBOL_GPL(phy_modify_mmd);
>>>>>> static int __phy_read_page(struct phy_device *phydev)
>>>>>> {
>>>>>> + /* If not attached, do nothing (no warning) */
>>>>>> + if (!phydev->attached_dev)
>>>>>> + return -EOPNOTSUPP;
>>>>>> +
>>>>>> if (WARN_ONCE(!phydev->drv->read_page, "read_page
>>>>>> callback not available, PHY driver not loaded?\n"))
>>>>>> return -EOPNOTSUPP;
>>>>>> @@ -712,12 +716,17 @@ static int __phy_read_page(struct
>>>>>> phy_device *phydev)
>>>>>> static int __phy_write_page(struct phy_device *phydev, int
>>>>>> page)
>>>>>> {
>>>>>> + /* If not attached, do nothing (no warning) */
>>>>>> + if (!phydev->attached_dev)
>>>>>> + return -EOPNOTSUPP;
>>>>>> +
>>>>>> if (WARN_ONCE(!phydev->drv->write_page, "write_page
>>>>>> callback not available, PHY driver not loaded?\n"))
>>>>>> return -EOPNOTSUPP;
>>>>>> return phydev->drv->write_page(phydev, page);
>>>>>> }
>>>>>> +
>>>>>> /**
>>>>>> * phy_save_page() - take the bus lock and save the current page
>>>>>> * @phydev: a pointer to a &struct phy_device
>>>>>>
>>>>>>
>>>>>>
>>>>>> 15. huhtik. 2020, 19.18, Heiner Kallweit <hkallweit1@...il.com
>>>>>> <mailto:hkallweit1@...il.com>> kirjoitti:
>>>>>>
>>>>>> On 15.04.2020 16:39, Lauri Jakku wrote:
>>>>>>
>>>>>> Hi, There seems to he Something odd problem, maybe
>>>>>> timing related. Stripped version not workingas expected. I get
>>>>>> back to you, when i have it working.
>>>>>>
>>>>>>
>>>>>> There's no point in working on your patch. W/o proper
>>>>>> justification it
>>>>>> isn't acceptable anyway. And so far we still don't know
>>>>>> which problem
>>>>>> you actually have.
>>>>>> FIRST please provide the requested logs and explain the
>>>>>> actual problem
>>>>>> (incl. the commit that caused the regression).
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> 13. huhtik. 2020, 14.46, Lauri Jakku
>>>>>> <ljakku77@...il.com <mailto:ljakku77@...il.com>> kirjoitti: Hi,
>>>>>> Fair enough, i'll strip them. -lja On 2020-04-13 14:34, Leon
>>>>>> Romanovsky wrote:
>>>>>>
>>>>>> On Mon, Apr 13, 2020 at 02:02:01PM +0300, Lauri Jakku
>>>>>> wrote: Hi, Comments inline. On 2020-04-13 13:58, Leon Romanovsky
>>>>>> wrote: On Mon, Apr 13, 2020 at 01:30:13PM +0300, Lauri Jakku
>>>>>> wrote: From 2d41edd4e6455187094f3a13d58c46eeee35aa31 Mon Sep 17
>>>>>> 00:00:00 2001 From: Lauri Jakku <lja@....fi> Date: Mon, 13 Apr
>>>>>> 2020 13:18:35 +0300 Subject: [PATCH] NET: r8168/r8169 identifying
>>>>>> fix The driver installation determination made properly by
>>>>>> checking PHY vs DRIVER id's. ---
>>>>>> drivers/net/ethernet/realtek/r8169_main.c | 70
>>>>>> ++++++++++++++++++++--- drivers/net/phy/mdio_bus.c | 11 +++- 2
>>>>>> files changed, 72 insertions(+), 9 deletions(-) I would say that
>>>>>> most of the code is debug prints. I tought that they are helpful
>>>>>> to keep, they are using the debug calls, so they are not visible
>>>>>> if user does not like those. You are missing the point of who are
>>>>>> your users. Users want to have working device and the code. They
>>>>>> don't need or like to debug their kernel. Thanks
>>>>>>
>>>>>>
Powered by blists - more mailing lists