| 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
| ||
|
Message-Id: <20240509123718.1521924-1-xiaolei.wang@windriver.com>
Date: Thu, 9 May 2024 20:37:18 +0800
From: Xiaolei Wang <xiaolei.wang@...driver.com>
To: alexandre.torgue@...s.st.com, joabreu@...opsys.com, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
mcoquelin.stm32@...il.com, richardcochran@...il.com,
bartosz.golaszewski@...aro.org, horms@...nel.org, ahalaney@...hat.com,
rohan.g.thomas@...el.com, rmk+kernel@...linux.org.uk,
fancer.lancer@...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: [net PATCH v4] net: stmmac: move the EST and EST lock to struct stmmac_priv
Reinitialize the whole EST structure would also reset the mutex lock
which is embedded in the EST structure, and then trigger the following
warning. To address this, move the lock to struct stmmac_priv,
and move EST to struct stmmac_priv, because the EST configs don't look
as the platform config, but EST is enabled in runtime with the settings
retrieved for the TC TAPRIO feature also in runtime. So it's better to
have the EST-data preserved in the driver private date instead of the
platform data storage. We also need to require the mutex lock when doing
this initialization.
DEBUG_LOCKS_WARN_ON(lock->magic != lock)
WARNING: CPU: 3 PID: 505 at kernel/locking/mutex.c:587 __mutex_lock+0xd84/0x1068
Modules linked in:
CPU: 3 PID: 505 Comm: tc Not tainted 6.9.0-rc6-00053-g0106679839f7-dirty #29
Hardware name: NXP i.MX8MPlus EVK board (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __mutex_lock+0xd84/0x1068
lr : __mutex_lock+0xd84/0x1068
sp : ffffffc0864e3570
x29: ffffffc0864e3570 x28: ffffffc0817bdc78 x27: 0000000000000003
x26: ffffff80c54f1808 x25: ffffff80c9164080 x24: ffffffc080d723ac
x23: 0000000000000000 x22: 0000000000000002 x21: 0000000000000000
x20: 0000000000000000 x19: ffffffc083bc3000 x18: ffffffffffffffff
x17: ffffffc08117b080 x16: 0000000000000002 x15: ffffff80d2d40000
x14: 00000000000002da x13: ffffff80d2d404b8 x12: ffffffc082b5a5c8
x11: ffffffc082bca680 x10: ffffffc082bb2640 x9 : ffffffc082bb2698
x8 : 0000000000017fe8 x7 : c0000000ffffefff x6 : 0000000000000001
x5 : ffffff8178fe0d48 x4 : 0000000000000000 x3 : 0000000000000027
x2 : ffffff8178fe0d50 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
__mutex_lock+0xd84/0x1068
mutex_lock_nested+0x28/0x34
tc_setup_taprio+0x118/0x68c
stmmac_setup_tc+0x50/0xf0
taprio_change+0x868/0xc9c
Signed-off-by: Xiaolei Wang <xiaolei.wang@...driver.com>
---
v1 -> v2:
- move the lock to struct plat_stmmacenet_data
v2 -> v3:
- Add require the mutex lock for reinitialization
v3 -> v4
- Move est and est lock to stmmac_priv as suggested by Serge
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 +
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++---
.../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 30 +++++-----
.../net/ethernet/stmicro/stmmac/stmmac_tc.c | 58 +++++++++----------
include/linux/stmmac.h | 2 -
5 files changed, 56 insertions(+), 55 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index dddcaa9220cc..e05a775b463e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -261,6 +261,9 @@ struct stmmac_priv {
struct stmmac_extra_stats xstats ____cacheline_aligned_in_smp;
struct stmmac_safety_stats sstats;
struct plat_stmmacenet_data *plat;
+ /* Protect est parameters */
+ struct mutex est_lock;
+ struct stmmac_est *est;
struct dma_features dma_cap;
struct stmmac_counters mmc;
int hw_cap_support;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7c6fb14b5555..0eafd609bf53 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2491,9 +2491,9 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
if (!xsk_tx_peek_desc(pool, &xdp_desc))
break;
- if (priv->plat->est && priv->plat->est->enable &&
- priv->plat->est->max_sdu[queue] &&
- xdp_desc.len > priv->plat->est->max_sdu[queue]) {
+ if (priv->est && priv->est->enable &&
+ priv->est->max_sdu[queue] &&
+ xdp_desc.len > priv->est->max_sdu[queue]) {
priv->xstats.max_sdu_txq_drop[queue]++;
continue;
}
@@ -4528,9 +4528,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
return stmmac_tso_xmit(skb, dev);
}
- if (priv->plat->est && priv->plat->est->enable &&
- priv->plat->est->max_sdu[queue] &&
- skb->len > priv->plat->est->max_sdu[queue]){
+ if (priv->est && priv->est->enable &&
+ priv->est->max_sdu[queue] &&
+ skb->len > priv->est->max_sdu[queue]){
priv->xstats.max_sdu_txq_drop[queue]++;
goto max_sdu_err;
}
@@ -4909,9 +4909,9 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
if (stmmac_tx_avail(priv, queue) < STMMAC_TX_THRESH(priv))
return STMMAC_XDP_CONSUMED;
- if (priv->plat->est && priv->plat->est->enable &&
- priv->plat->est->max_sdu[queue] &&
- xdpf->len > priv->plat->est->max_sdu[queue]) {
+ if (priv->est && priv->est->enable &&
+ priv->est->max_sdu[queue] &&
+ xdpf->len > priv->est->max_sdu[queue]) {
priv->xstats.max_sdu_txq_drop[queue]++;
return STMMAC_XDP_CONSUMED;
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index e04830a3a1fb..a6b1de9a251d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -68,13 +68,13 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
nsec = reminder;
/* If EST is enabled, disabled it before adjust ptp time. */
- if (priv->plat->est && priv->plat->est->enable) {
+ if (priv->est && priv->est->enable) {
est_rst = true;
- mutex_lock(&priv->plat->est->lock);
- priv->plat->est->enable = false;
- stmmac_est_configure(priv, priv, priv->plat->est,
+ mutex_lock(&priv->est_lock);
+ priv->est->enable = false;
+ stmmac_est_configure(priv, priv, priv->est,
priv->plat->clk_ptp_rate);
- mutex_unlock(&priv->plat->est->lock);
+ mutex_unlock(&priv->est_lock);
}
write_lock_irqsave(&priv->ptp_lock, flags);
@@ -87,24 +87,24 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
ktime_t current_time_ns, basetime;
u64 cycle_time;
- mutex_lock(&priv->plat->est->lock);
+ mutex_lock(&priv->est_lock);
priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time);
current_time_ns = timespec64_to_ktime(current_time);
- time.tv_nsec = priv->plat->est->btr_reserve[0];
- time.tv_sec = priv->plat->est->btr_reserve[1];
+ time.tv_nsec = priv->est->btr_reserve[0];
+ time.tv_sec = priv->est->btr_reserve[1];
basetime = timespec64_to_ktime(time);
- cycle_time = (u64)priv->plat->est->ctr[1] * NSEC_PER_SEC +
- priv->plat->est->ctr[0];
+ cycle_time = (u64)priv->est->ctr[1] * NSEC_PER_SEC +
+ priv->est->ctr[0];
time = stmmac_calc_tas_basetime(basetime,
current_time_ns,
cycle_time);
- priv->plat->est->btr[0] = (u32)time.tv_nsec;
- priv->plat->est->btr[1] = (u32)time.tv_sec;
- priv->plat->est->enable = true;
- ret = stmmac_est_configure(priv, priv, priv->plat->est,
+ priv->est->btr[0] = (u32)time.tv_nsec;
+ priv->est->btr[1] = (u32)time.tv_sec;
+ priv->est->enable = true;
+ ret = stmmac_est_configure(priv, priv, priv->est,
priv->plat->clk_ptp_rate);
- mutex_unlock(&priv->plat->est->lock);
+ mutex_unlock(&priv->est_lock);
if (ret)
netdev_err(priv->dev, "failed to configure EST\n");
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index cce00719937d..222540b55480 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -918,7 +918,6 @@ struct timespec64 stmmac_calc_tas_basetime(ktime_t old_base_time,
static void tc_taprio_map_maxsdu_txq(struct stmmac_priv *priv,
struct tc_taprio_qopt_offload *qopt)
{
- struct plat_stmmacenet_data *plat = priv->plat;
u32 num_tc = qopt->mqprio.qopt.num_tc;
u32 offset, count, i, j;
@@ -933,7 +932,7 @@ static void tc_taprio_map_maxsdu_txq(struct stmmac_priv *priv,
count = qopt->mqprio.qopt.count[i];
for (j = offset; j < offset + count; j++)
- plat->est->max_sdu[j] = qopt->max_sdu[i] + ETH_HLEN - ETH_TLEN;
+ priv->est->max_sdu[j] = qopt->max_sdu[i] + ETH_HLEN - ETH_TLEN;
}
}
@@ -941,7 +940,6 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
struct tc_taprio_qopt_offload *qopt)
{
u32 size, wid = priv->dma_cap.estwid, dep = priv->dma_cap.estdep;
- struct plat_stmmacenet_data *plat = priv->plat;
struct timespec64 time, current_time, qopt_time;
ktime_t current_time_ns;
bool fpe = false;
@@ -998,23 +996,25 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
if (qopt->cycle_time_extension >= BIT(wid + 7))
return -ERANGE;
- if (!plat->est) {
- plat->est = devm_kzalloc(priv->device, sizeof(*plat->est),
+ if (!priv->est) {
+ priv->est = devm_kzalloc(priv->device, sizeof(*priv->est),
GFP_KERNEL);
- if (!plat->est)
+ if (!priv->est)
return -ENOMEM;
- mutex_init(&priv->plat->est->lock);
+ mutex_init(&priv->est_lock);
} else {
- memset(plat->est, 0, sizeof(*plat->est));
+ mutex_lock(&priv->est_lock);
+ memset(priv->est, 0, sizeof(*priv->est));
+ mutex_unlock(&priv->est_lock);
}
size = qopt->num_entries;
- mutex_lock(&priv->plat->est->lock);
- priv->plat->est->gcl_size = size;
- priv->plat->est->enable = qopt->cmd == TAPRIO_CMD_REPLACE;
- mutex_unlock(&priv->plat->est->lock);
+ mutex_lock(&priv->est_lock);
+ priv->est->gcl_size = size;
+ priv->est->enable = qopt->cmd == TAPRIO_CMD_REPLACE;
+ mutex_unlock(&priv->est_lock);
for (i = 0; i < size; i++) {
s64 delta_ns = qopt->entries[i].interval;
@@ -1042,33 +1042,33 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
return -EOPNOTSUPP;
}
- priv->plat->est->gcl[i] = delta_ns | (gates << wid);
+ priv->est->gcl[i] = delta_ns | (gates << wid);
}
- mutex_lock(&priv->plat->est->lock);
+ mutex_lock(&priv->est_lock);
/* Adjust for real system time */
priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, ¤t_time);
current_time_ns = timespec64_to_ktime(current_time);
time = stmmac_calc_tas_basetime(qopt->base_time, current_time_ns,
qopt->cycle_time);
- priv->plat->est->btr[0] = (u32)time.tv_nsec;
- priv->plat->est->btr[1] = (u32)time.tv_sec;
+ priv->est->btr[0] = (u32)time.tv_nsec;
+ priv->est->btr[1] = (u32)time.tv_sec;
qopt_time = ktime_to_timespec64(qopt->base_time);
- priv->plat->est->btr_reserve[0] = (u32)qopt_time.tv_nsec;
- priv->plat->est->btr_reserve[1] = (u32)qopt_time.tv_sec;
+ priv->est->btr_reserve[0] = (u32)qopt_time.tv_nsec;
+ priv->est->btr_reserve[1] = (u32)qopt_time.tv_sec;
ctr = qopt->cycle_time;
- priv->plat->est->ctr[0] = do_div(ctr, NSEC_PER_SEC);
- priv->plat->est->ctr[1] = (u32)ctr;
+ priv->est->ctr[0] = do_div(ctr, NSEC_PER_SEC);
+ priv->est->ctr[1] = (u32)ctr;
- priv->plat->est->ter = qopt->cycle_time_extension;
+ priv->est->ter = qopt->cycle_time_extension;
tc_taprio_map_maxsdu_txq(priv, qopt);
if (fpe && !priv->dma_cap.fpesel) {
- mutex_unlock(&priv->plat->est->lock);
+ mutex_unlock(&priv->est_lock);
return -EOPNOTSUPP;
}
@@ -1077,9 +1077,9 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
*/
priv->plat->fpe_cfg->enable = fpe;
- ret = stmmac_est_configure(priv, priv, priv->plat->est,
+ ret = stmmac_est_configure(priv, priv, priv->est,
priv->plat->clk_ptp_rate);
- mutex_unlock(&priv->plat->est->lock);
+ mutex_unlock(&priv->est_lock);
if (ret) {
netdev_err(priv->dev, "failed to configure EST\n");
goto disable;
@@ -1095,17 +1095,17 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
return 0;
disable:
- if (priv->plat->est) {
- mutex_lock(&priv->plat->est->lock);
- priv->plat->est->enable = false;
- stmmac_est_configure(priv, priv, priv->plat->est,
+ if (priv->est) {
+ mutex_lock(&priv->est_lock);
+ priv->est->enable = false;
+ stmmac_est_configure(priv, priv, priv->est,
priv->plat->clk_ptp_rate);
/* Reset taprio status */
for (i = 0; i < priv->plat->tx_queues_to_use; i++) {
priv->xstats.max_sdu_txq_drop[i] = 0;
priv->xstats.mtl_est_txq_hlbf[i] = 0;
}
- mutex_unlock(&priv->plat->est->lock);
+ mutex_unlock(&priv->est_lock);
}
priv->plat->fpe_cfg->enable = false;
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index dfa1828cd756..8aa255485a35 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -117,7 +117,6 @@ struct stmmac_axi {
#define EST_GCL 1024
struct stmmac_est {
- struct mutex lock;
int enable;
u32 btr_reserve[2];
u32 btr_offset[2];
@@ -246,7 +245,6 @@ struct plat_stmmacenet_data {
struct fwnode_handle *port_node;
struct device_node *mdio_node;
struct stmmac_dma_cfg *dma_cfg;
- struct stmmac_est *est;
struct stmmac_fpe_cfg *fpe_cfg;
struct stmmac_safety_feature_cfg *safety_feat_cfg;
int clk_csr;
--
2.25.1
Powered by blists - more mailing lists