linux-kernel - RE: [PATCH net v1] mlxbf-gige: Support workaround for MDIO GPIO degradation bug

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:
 <CH3PR12MB7738E1776CD326A2566254D5D740A@CH3PR12MB7738.namprd12.prod.outlook.com>
Date: Wed, 2 Jul 2025 20:56:30 +0000
From: Asmaa Mnebhi <asmaa@...dia.com>
To: Andrew Lunn <andrew@...n.ch>
CC: "davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, David Thompson <davthompson@...dia.com>
Subject: RE: [PATCH net v1] mlxbf-gige: Support workaround for MDIO GPIO
 degradation bug

> > However, we have another issue. I noticed that even if
> > mdio_read/write() functions are not being called,
> > runtime_resume/suspend() are still called regularly. After
> > investigation, I found out that this is due to ethtool being called
> > regularly. Ethtool automatically triggers the resume/suspend even if
> > we do no MDIO access. A different team wrote a script which monitors
> > "ethtool -S eth0" every 60 seconds. So every minute, we are running
> > resume/suspend and enabling/disabling the MDIO clock. Seems counter
> > productive. That team said that it is a requirement that they
> > collect these statistics about the mlxbf_gige interface.
> 
> > Is there any way to prevent ethtool from calling resume/suspend
> > without changing core kernel code?
> > 
> You need to put the MDIO bus device into its own pm_domain. Try
> calling dev_pm_domain_set() to separate the MDIO bus from the MAC
> driver in terms of power domains. ethtool will then power on/off the
> MAC but leave the MDIO bus alone.
> 
Using dev_pm_domain_set() has the same effect as SET_RUNTIME_PM_OPS. The dev struct is shared so ethtool is still calling the suspend/resume.

int mlxbf_gige_mdio_probe(struct platform_device *pdev, struct mlxbf_gige *priv)
 {
        struct device *dev = &pdev->dev;
@@ -390,14 +418,27 @@ int mlxbf_gige_mdio_probe(struct platform_device *pdev, struct mlxbf_gige *priv)
        snprintf(priv->mdiobus->id, MII_BUS_ID_SIZE, "%s",
                 dev_name(dev));

+       pm_runtime_set_autosuspend_delay(priv->mdiobus->parent, 100);
+       pm_runtime_use_autosuspend(priv->mdiobus->parent);
+       pm_runtime_set_active(priv->mdiobus->parent);
+       pm_runtime_enable(priv->mdiobus->parent);
+       dev_pm_domain_set(priv->mdiobus->parent, &mlxbf_gige_pm_domain);
+
        ret = mdiobus_register(priv->mdiobus);
-       if (ret)
+       if (ret) {
+               pm_runtime_disable(priv->mdiobus->parent);
+               pm_runtime_set_suspended(priv->mdiobus->parent);
+               pm_runtime_dont_use_autosuspend(priv->mdiobus->parent);
                dev_err(dev, "Failed to register MDIO bus\n");
+       }

        return ret;
 }

I removed the pm related apis and just added disabling/reenabling the clock directly in the mdio read/write and it works (see code snippet below). It might be slower and not as efficient as the pm runtime infrastructure since we don't have autosuspend.

@@ -224,6 +243,10 @@ static int mlxbf_gige_mdio_read(struct mii_bus *bus, int phy_add, int phy_reg)
        if (phy_reg & MII_ADDR_C45)
                return -EOPNOTSUPP;

+       spin_lock_irqsave(&priv->lock, flag);
+
+       mlxbf_gige_mdio_enable(priv);
+
        /* Send mdio read request */
        cmd = mlxbf_gige_mdio_create_cmd(priv->mdio_gw, 0, phy_add, phy_reg,
                                         MLXBF_GIGE_MDIO_CL22_READ);
@@ -236,6 +259,8 @@ static int mlxbf_gige_mdio_read(struct mii_bus *bus, int phy_add, int phy_reg)

        if (ret) {
                writel(0, priv->mdio_io + priv->mdio_gw->gw_address);
+               mlxbf_gige_mdio_disable(priv);
+               spin_unlock_irqrestore(&priv->lock, flag);
                return ret;
        }

@@ -246,6 +271,9 @@ static int mlxbf_gige_mdio_read(struct mii_bus *bus, int phy_add, int phy_reg)
        /* The MDIO lock is set on read. To release it, clear gw register */
        writel(0, priv->mdio_io + priv->mdio_gw->gw_address);

+       mlxbf_gige_mdio_disable(priv);
+       spin_unlock_irqrestore(&priv->lock, flag);
+
        return ret;
 }