linux-kernel - [PATCH 1/2] net: phylink: add sync flag mac_ready to fix resume issue with WoL enabled

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: <20221130111148.1064475-2-xiaoning.wang@nxp.com>
Date:   Wed, 30 Nov 2022 19:11:47 +0800
From:   Clark Wang <xiaoning.wang@....com>
To:     linux@...linux.org.uk, peppe.cavallaro@...com,
        alexandre.torgue@...s.st.com, joabreu@...opsys.com,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, mcoquelin.stm32@...il.com, andrew@...n.ch,
        hkallweit1@...il.com
Cc:     netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: [PATCH 1/2] net: phylink: add sync flag mac_ready to fix resume issue with WoL enabled

Issue we met:
On some platforms, mac cannot work after resumed from the suspend with WoL
enabled.

The cause of the issue:
1. phylink_resolve() is in a workqueue which will not be executed immediately.
   This is the call sequence:
       phylink_resolve()->phylink_link_up()->pl->mac_ops->mac_link_up()
   For stmmac driver, mac_link_up() will set the correct speed/duplex...
   values which are from link_state.
2. In stmmac_resume(), it will call stmmac_hw_setup() after called the
   phylink_resume(). stmmac_core_init() is called in function stmmac_hw_setup(),
   which will reset the mac and set the speed/duplex... to default value.
Conclusion: Because phylink_resolve() cannot determine when it is called, it
            cannot be guaranteed to be called after stmmac_core_init().
	    Once stmmac_core_init() is called after phylink_resolve(),
	    the mac will be misconfigured and cannot be used.

In order to solve this problem, the mac_ready flag is added to the phylink
structure to synchronize the state of the mac in the phylink_resolve()
function. To prevent the correct configuration from being overwritten.

By default, mac_ready will be configured as true, that is, it will not affect
other drivers that do not use this flag.

Signed-off-by: Clark Wang <xiaoning.wang@....com>
---
 drivers/net/phy/phylink.c | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/phylink.h   |  2 ++
 2 files changed, 38 insertions(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 09cc65c0da93..312b47fdc12b 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -74,6 +74,7 @@ struct phylink {
 
 	bool mac_link_dropped;
 	bool using_mac_select_pcs;
+	bool mac_ready;
 
 	struct sfp_bus *sfp_bus;
 	bool sfp_may_have_phy;
@@ -1276,6 +1277,12 @@ static void phylink_resolve(struct work_struct *w)
 	bool retrigger = false;
 	bool cur_link_state;
 
+	/* If mac is not ready, retrigger this work queue to wait it ready*/
+	if (!pl->mac_ready) {
+		queue_work(system_power_efficient_wq, &pl->resolve);
+		return;
+	}
+
 	mutex_lock(&pl->state_mutex);
 	if (pl->netdev)
 		cur_link_state = netif_carrier_ok(ndev);
@@ -1450,6 +1457,34 @@ static int phylink_register_sfp(struct phylink *pl,
 	return ret;
 }
 
+/**
+ * phylink_clear_mac_ready() - clear mac_ready flag
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ *
+ * set mac_ready to false, which means the mac is not ready,
+ * if the pl->mac_ops->mac_link_up function in resolve is called at this time,
+ * the correct speed/duplex and other parameters set in this function will
+ * be reset to the default values by mac.
+ */
+void phylink_clear_mac_ready(struct phylink *pl)
+{
+	pl->mac_ready = false;
+}
+EXPORT_SYMBOL_GPL(phylink_clear_mac_ready);
+
+/**
+ * phylink_set_mac_ready() - set mac_ready flag
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ *
+ * set mac_ready to true, which means the mac is ready now, can reslove the
+ * phylink and set the correct speed/duplex settings for mac.
+ */
+void phylink_set_mac_ready(struct phylink *pl)
+{
+	pl->mac_ready = true;
+}
+EXPORT_SYMBOL_GPL(phylink_set_mac_ready);
+
 /**
  * phylink_create() - create a phylink instance
  * @config: a pointer to the target &struct phylink_config
@@ -1518,6 +1553,7 @@ struct phylink *phylink_create(struct phylink_config *config,
 	pl->link_config.duplex = DUPLEX_UNKNOWN;
 	pl->link_config.an_enabled = true;
 	pl->mac_ops = mac_ops;
+	pl->mac_ready = true;
 	__set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
 	timer_setup(&pl->link_poll, phylink_fixed_poll, 0);
 
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index c492c26202b5..759c8041f3d2 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -570,6 +570,8 @@ void phylink_generic_validate(struct phylink_config *config,
 			      unsigned long *supported,
 			      struct phylink_link_state *state);
 
+void phylink_clear_mac_ready(struct phylink *pl);
+void phylink_set_mac_ready(struct phylink *pl);
 struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
 			       phy_interface_t iface,
 			       const struct phylink_mac_ops *mac_ops);
-- 
2.34.1