| 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: <20250828214403.1765311-3-niklas.soderlund+renesas@ragnatech.se>
Date: Thu, 28 Aug 2025 23:44:03 +0200
From: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
To: Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
linux-kernel@...r.kernel.org,
linux-renesas-soc@...r.kernel.org
Cc: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
Subject: [PATCH 2/2] clocksource/drivers/sh_cmt: Do not power down channels used for events
The CMT do runtime PM and call clk_enable()/clk_disable() when a new
clock event is register and the CMT is not already started. This is not
always possible as a spinlock is also needed to sync the internals of
the CMT. Running with PROVE_LOCKING uncovers one such issue.
=============================
[ BUG: Invalid wait context ]
6.17.0-rc3-arm64-renesas-03071-gb3c4f4122b28-dirty #21 Not tainted
-----------------------------
swapper/1/0 is trying to lock:
ffff00000898d180 (&dev->power.lock){-...}-{3:3}, at: __pm_runtime_resume+0x38/0x88
ccree e6601000.crypto: ARM CryptoCell 630P Driver: HW version 0xAF400001/0xDCC63000, Driver version 5.0
other info that might help us debug this:
ccree e6601000.crypto: ARM ccree device initialized
context-{5:5}
2 locks held by swapper/1/0:
#0: ffff80008173c298 (tick_broadcast_lock){-...}-{2:2}, at: __tick_broadcast_oneshot_control+0xa4/0x3a8
#1: ffff0000089a5858 (&ch->lock){....}-{2:2}
usbcore: registered new interface driver usbhid
, at: sh_cmt_start+0x30/0x364
stack backtrace:
CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 6.17.0-rc3-arm64-renesas-03071-gb3c4f4122b28-dirty #21 PREEMPT
Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT)
Call trace:
show_stack+0x14/0x1c (C)
dump_stack_lvl+0x6c/0x90
dump_stack+0x14/0x1c
__lock_acquire+0x904/0x1584
lock_acquire+0x220/0x34c
_raw_spin_lock_irqsave+0x58/0x80
__pm_runtime_resume+0x38/0x88
sh_cmt_start+0x54/0x364
sh_cmt_clock_event_set_oneshot+0x64/0xb8
clockevents_switch_state+0xfc/0x13c
tick_broadcast_set_event+0x30/0xa4
__tick_broadcast_oneshot_control+0x1e0/0x3a8
tick_broadcast_oneshot_control+0x30/0x40
cpuidle_enter_state+0x40c/0x680
cpuidle_enter+0x30/0x40
do_idle+0x1f4/0x26c
cpu_startup_entry+0x34/0x40
secondary_start_kernel+0x11c/0x13c
__secondary_switched+0x74/0x78
Fix this by unconditionally powering on and enabling the needed clocks
for all CMT channels which are used for clock events.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>
---
drivers/clocksource/sh_cmt.c | 87 ++++++++++++++++--------------------
1 file changed, 38 insertions(+), 49 deletions(-)
diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c
index 385eb94bbe7c..a57d5523f63b 100644
--- a/drivers/clocksource/sh_cmt.c
+++ b/drivers/clocksource/sh_cmt.c
@@ -5,6 +5,7 @@
* Copyright (C) 2008 Magnus Damm
*/
+#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/clockchips.h>
#include <linux/clocksource.h>
@@ -623,51 +624,6 @@ static void sh_cmt_stop_clocksource(struct sh_cmt_channel *ch)
pm_runtime_put(&ch->cmt->pdev->dev);
}
-static int sh_cmt_start_clockevent(struct sh_cmt_channel *ch)
-{
- int ret = 0;
- unsigned long flags;
-
- raw_spin_lock_irqsave(&ch->lock, flags);
-
- if (!(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) {
- pm_runtime_get_sync(&ch->cmt->pdev->dev);
- ret = sh_cmt_enable(ch);
- }
-
- if (ret)
- goto out;
-
- ch->flags |= FLAG_CLOCKEVENT;
- out:
- raw_spin_unlock_irqrestore(&ch->lock, flags);
-
- return ret;
-}
-
-static void sh_cmt_stop_clockevent(struct sh_cmt_channel *ch)
-{
- unsigned long flags;
- unsigned long f;
-
- raw_spin_lock_irqsave(&ch->lock, flags);
-
- f = ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE);
-
- ch->flags &= ~FLAG_CLOCKEVENT;
-
- if (f && !(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) {
- sh_cmt_disable(ch);
- pm_runtime_put(&ch->cmt->pdev->dev);
- }
-
- /* adjust the timeout to maximum if only clocksource left */
- if (ch->flags & FLAG_CLOCKSOURCE)
- __sh_cmt_set_next(ch, ch->max_match_value);
-
- raw_spin_unlock_irqrestore(&ch->lock, flags);
-}
-
static struct sh_cmt_channel *cs_to_sh_cmt(struct clocksource *cs)
{
return container_of(cs, struct sh_cmt_channel, cs);
@@ -774,19 +730,30 @@ static struct sh_cmt_channel *ced_to_sh_cmt(struct clock_event_device *ced)
static void sh_cmt_clock_event_start(struct sh_cmt_channel *ch, int periodic)
{
- sh_cmt_start_clockevent(ch);
-
if (periodic)
sh_cmt_set_next(ch, ((ch->cmt->rate + HZ/2) / HZ) - 1);
else
sh_cmt_set_next(ch, ch->max_match_value);
}
+static void sh_cmt_clock_event_stop(struct sh_cmt_channel *ch)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&ch->lock, flags);
+
+ /* adjust the timeout to maximum if only clocksource left */
+ if (ch->flags & FLAG_CLOCKSOURCE)
+ __sh_cmt_set_next(ch, ch->max_match_value);
+
+ raw_spin_unlock_irqrestore(&ch->lock, flags);
+}
+
static int sh_cmt_clock_event_shutdown(struct clock_event_device *ced)
{
struct sh_cmt_channel *ch = ced_to_sh_cmt(ced);
- sh_cmt_stop_clockevent(ch);
+ sh_cmt_clock_event_stop(ch);
return 0;
}
@@ -797,7 +764,7 @@ static int sh_cmt_clock_event_set_state(struct clock_event_device *ced,
/* deal with old setting first */
if (clockevent_state_oneshot(ced) || clockevent_state_periodic(ced))
- sh_cmt_stop_clockevent(ch);
+ sh_cmt_clock_event_stop(ch);
dev_info(&ch->cmt->pdev->dev, "ch%u: used for %s clock events\n",
ch->index, periodic ? "periodic" : "oneshot");
@@ -908,6 +875,28 @@ static int sh_cmt_register(struct sh_cmt_channel *ch, const char *name,
ret = sh_cmt_register_clockevent(ch, name);
if (ret < 0)
return ret;
+
+ /*
+ * To support clock events the CMT must always be ready to
+ * accept new events, start it and leave no way for it to be
+ * turned off.
+ *
+ * This is OK as we can't never unregister a CMT device. If this
+ * is fixed in future an equal unconditional shutdown is needed.
+ */
+ pm_runtime_get_sync(&ch->cmt->pdev->dev);
+
+ scoped_guard(raw_spinlock_irqsave, &ch->lock) {
+ ret = sh_cmt_enable(ch);
+ if (ret)
+ return ret;
+
+ /*
+ * Flag that the channel is used as a clock event, it's not
+ * allowed to be powered off!
+ */
+ ch->flags |= FLAG_CLOCKEVENT;
+ }
}
if (clocksource) {
--
2.51.0
Powered by blists - more mailing lists