Linux Power Management development
 help / color / mirror / Atom feed
* Re: [RFC][RFT][PATCH 0/3] arm64: Enable asympacking for minor CPPC asymmetry
From: Vincent Guittot @ 2026-03-26 13:04 UTC (permalink / raw)
  To: Christian Loehle
  Cc: arighi, peterz, dietmar.eggemann, valentin.schneider, mingo,
	rostedt, segall, mgorman, catalin.marinas, will, sudeep.holla,
	rafael, linux-pm, linux-kernel, juri.lelli, kobak, fabecassis
In-Reply-To: <7eccf54f-5a99-40ce-8fbc-b755b4e2d312@arm.com>

On Thu, 26 Mar 2026 at 10:24, Christian Loehle <christian.loehle@arm.com> wrote:
>
> On 3/26/26 08:24, Vincent Guittot wrote:
> > On Thu, 26 Mar 2026 at 09:16, Christian Loehle <christian.loehle@arm.com> wrote:
> >>
> >> On 3/26/26 07:53, Vincent Guittot wrote:
> >>> On Wed, 25 Mar 2026 at 19:13, Christian Loehle <christian.loehle@arm.com> wrote:
> >>>>
> >>>> The scheduler currently handles CPU performance asymmetry via either:
> >>>>
> >>>> - SD_ASYM_PACKING: simple priority-based task placement (x86 ITMT)
> >>>> - SD_ASYM_CPUCAPACITY: capacity-aware scheduling
> >>>>
> >>>> On arm64, capacity-aware scheduling is used for any detected capacity
> >>>> differences.
> >>>>
> >>>> Some systems expose small per-CPU performance differences via CPPC
> >>>> highest_perf (e.g. due to chip binning), resulting in slightly different
> >>>> capacities (<~5%). These differences are sufficient to trigger
> >>>> SD_ASYM_CPUCAPACITY, even though the system is otherwise effectively
> >>>> symmetric.
> >>>>
> >>>> For such small deltas, capacity-aware scheduling is unnecessarily
> >>>> complex. A simpler priority-based approach, similar to x86 ITMT, is
> >>>> sufficient.
> >>>
> >>> I'm not convinced that moving to  SD_ASYM_PACKING is the right way to
> >>> move forward.
> >>> t
> >>> 1st of all, do you target all kind of system or only SMT? It's not
> >>> clear in your cover letter
> >>
> >> AFAIK only Andrea has access to an unreleased asymmetric SMT system,
> >> I haven't done any tests on such a system (as the cover-letter mentions
> >> under RFT section).
> >>
> >>>
> >>> Moving on asym pack for !SMT doesn't make sense to me. If you don't
> >>> want EAS enabled, you can disable it with
> >>> /proc/sys/kernel/sched_energy_aware
> >>
> >> Sorry, what's EAS got to do with it? The system I care about here
> >> (primarily nvidia grace) has no EM.
> >
> > I tried to understand the end goal of this patch
> >
> > SD_ASYM_CPUCAPACITY works fine with !SMT system so why enabling
> > SD_ASYM_PACKING for <5% diff ?
> >
> > That doesn't make sense to me
> I don't know if "works fine" describes the situation accurately.
> I guess I should've included the context in the cover letter, but you
> are aware of them (you've replied to them anyway):
> https://lore.kernel.org/lkml/20260324005509.1134981-1-arighi@nvidia.com/
> https://lore.kernel.org/lkml/20260318092214.130908-1-arighi@nvidia.com/
>
> Andrea sees an improvement even when force-equalizing CPUs to remove
> SD_ASYM_CPUCAPACITY, so I'd argue it doesn't "work fine" on these platforms.

IIUC this was for SMT systems not for !SMT ones but I might have
missed some emails in the thread.

> To me it seems more reasonable to attempt to get these minor improvements
> of minor asymmetries through asympacking and leave SD_ASYM_CPUCAPACITY
> to the actual 'true' asymmetry (e.g. different uArch or vastly different
> performance levels).
> SD_ASYM_CPUCAPACITY handling is also arguably broken if no CPU pair in
> the system fulfills capacity_greater(), the call sites in fair.c give
> a good overview.
> Is $subject the right approach to deal with these platforms instead?
> I don't know, that's why it's marked RFC and RFT.

^ permalink raw reply

* [PATCH v1 08/10] devfreq: Allow find_devfreq_governor() to get module refcount
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

Add a 'get' parameter for find_devfreq_governor() to optionally get a
refcount of the governor module.

Getting refcount in try_then_request_governor() prevents the governor
module from being removed during the governor setting phase.

However, in devfreq_add/remove_governor(), it doesn't need to get a
refcount of the governor module because it just checks whether a
governor registered with the same name exists or not.

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index ba09948915ba..c6b670b8fd22 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -254,11 +254,13 @@ EXPORT_SYMBOL(devfreq_update_status);
 /**
  * find_devfreq_governor() - find devfreq governor from name
  * @name:	name of the governor
+ * @get:	whether to get a refcount of the governor module
  *
  * Search the list of devfreq governors and return the matched
  * governor's pointer.
  */
-static struct devfreq_governor *find_devfreq_governor(const char *name)
+static struct devfreq_governor *find_devfreq_governor(const char *name,
+						      bool get)
 {
 	struct devfreq_governor *tmp_governor;
 
@@ -269,8 +271,13 @@ static struct devfreq_governor *find_devfreq_governor(const char *name)
 
 	guard(mutex)(&devfreq_gov_lock);
 	list_for_each_entry(tmp_governor, &devfreq_governor_list, node) {
-		if (!strncmp(tmp_governor->name, name, DEVFREQ_NAME_LEN))
-			return tmp_governor;
+		if (strncmp(tmp_governor->name, name, DEVFREQ_NAME_LEN))
+			continue;
+
+		if (get && !try_module_get(tmp_governor->owner))
+			return ERR_PTR(-EBUSY);
+
+		return tmp_governor;
 	}
 
 	return ERR_PTR(-ENODEV);
@@ -285,6 +292,9 @@ static struct devfreq_governor *find_devfreq_governor(const char *name)
  * if is not found. This can happen when both drivers (the governor driver
  * and the driver that call devfreq_add_device) are built as modules.
  * Returns the matched governor's pointer or an error pointer.
+ * On success, this holds a refcount of the governor module to prevent the
+ * module from being unloaded during usage, so the caller should put a module
+ * refcount after using it.
  */
 static struct devfreq_governor *try_then_request_governor(const char *name)
 {
@@ -296,7 +306,7 @@ static struct devfreq_governor *try_then_request_governor(const char *name)
 		return ERR_PTR(-EINVAL);
 	}
 
-	governor = find_devfreq_governor(name);
+	governor = find_devfreq_governor(name, true);
 	if (IS_ERR(governor)) {
 		if (!strncmp(name, DEVFREQ_GOV_SIMPLE_ONDEMAND,
 			     DEVFREQ_NAME_LEN))
@@ -307,7 +317,7 @@ static struct devfreq_governor *try_then_request_governor(const char *name)
 		if (err)
 			return (err < 0) ? ERR_PTR(err) : ERR_PTR(-EINVAL);
 
-		governor = find_devfreq_governor(name);
+		governor = find_devfreq_governor(name, true);
 	}
 
 	return governor;
@@ -1007,6 +1017,8 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	if (err)
 		goto err_devfreq;
 
+	module_put(governor->owner);
+
 	list_add(&devfreq->node, &devfreq_list);
 	mutex_unlock(&devfreq_list_lock);
 
@@ -1312,7 +1324,7 @@ int __devfreq_add_governor(struct devfreq_governor *governor,
 		return -EINVAL;
 	}
 
-	g = find_devfreq_governor(governor->name);
+	g = find_devfreq_governor(governor->name, false);
 	if (!IS_ERR(g)) {
 		pr_err("%s: governor %s already registered\n", __func__,
 		       g->name);
@@ -1361,7 +1373,7 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 		return -EINVAL;
 	}
 
-	g = find_devfreq_governor(governor->name);
+	g = find_devfreq_governor(governor->name, false);
 	if (IS_ERR(g)) {
 		pr_err("%s: governor %s not registered\n", __func__,
 		       governor->name);
@@ -1436,6 +1448,8 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 
 	ret = devfreq_set_governor(df, governor);
 
+	module_put(governor->owner);
+
 	return ret ? ret : count;
 }
 static DEVICE_ATTR_RW(governor);
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 07/10] devfreq: Get and put module refcount when switching governor
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

When compiled as a kernel module, the governor module can be dynamically
inserted or removed.  'devfreq->governor' would become NULL if the
governor module is removed when it's in use.

To prevent the governor module from being removed (except for force
unload) when it's in use, get and put a refcount of the governor module
when starting and stopping the governor.

Now, unloading a governor module that is set for a devfreq device
returns an error, for example:
  # cat governor
  performance
  # rmmod governor_performance
  rmmod: ERROR: Module governor_performance is in use

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 9b078458d129..ba09948915ba 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -338,22 +338,31 @@ static int devfreq_set_governor_locked(struct devfreq *df,
 				 __func__, df->governor->name, ret);
 			return ret;
 		}
+		module_put(old_gov->owner);
 	}
 
 	/* Start the new governor */
+	if (!try_module_get(new_gov->owner))
+		return -EINVAL;
+
 	df->governor = new_gov;
 	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
 	if (ret) {
 		dev_warn(dev, "%s: Governor %s not started(%d)\n",
 			 __func__, df->governor->name, ret);
 
+		module_put(new_gov->owner);
 		if (!old_gov)
 			return ret;
 
 		/* Restore previous governor */
+		if (!try_module_get(old_gov->owner))
+			return -EINVAL;
+
 		df->governor = old_gov;
 		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
 		if (ret) {
+			module_put(old_gov->owner);
 			dev_err(dev, "%s: restore Governor %s failed (%d)\n",
 				__func__, old_gov->name, ret);
 			df->governor = NULL;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 04/10] devfreq: Factor out devfreq_set_governor[_locked]()
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

Factor out common governor setting logic into devfreq_set_governor() to
reduce code duplication in governor_store() and devfreq_add_device().

Additionally, devfreq_set_governor_locked() is used when
'devfreq_list_lock' is already held by the caller, e.g. in
devfreq_set_governor(), to avoid an immediate relock after unlock.

Note that the new functions support setting a governor even if
devfreq->governor is NULL.

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c | 129 +++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 66 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index e54e3092e0e0..378a01f71165 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -313,6 +313,64 @@ static struct devfreq_governor *try_then_request_governor(const char *name)
 	return governor;
 }
 
+static int devfreq_set_governor_locked(struct devfreq *df,
+				       const struct devfreq_governor *new_gov)
+{
+	const struct devfreq_governor *old_gov;
+	struct device *dev;
+	int ret;
+
+	old_gov = df->governor;
+	dev = &df->dev;
+
+	if (old_gov) {
+		if (old_gov == new_gov)
+			return 0;
+
+		if (IS_SUPPORTED_FLAG(old_gov->flags, IMMUTABLE) ||
+		    IS_SUPPORTED_FLAG(new_gov->flags, IMMUTABLE))
+			return -EINVAL;
+
+		/* Stop the current governor */
+		ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
+		if (ret) {
+			dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
+				 __func__, df->governor->name, ret);
+			return ret;
+		}
+	}
+
+	/* Start the new governor */
+	df->governor = new_gov;
+	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
+	if (ret) {
+		dev_warn(dev, "%s: Governor %s not started(%d)\n",
+			 __func__, df->governor->name, ret);
+
+		if (!old_gov)
+			return ret;
+
+		/* Restore previous governor */
+		df->governor = old_gov;
+		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
+		if (ret) {
+			dev_err(dev, "%s: restore Governor %s failed (%d)\n",
+				__func__, old_gov->name, ret);
+			df->governor = NULL;
+			return ret;
+		}
+	}
+
+	return sysfs_update_group(&df->dev.kobj, &gov_attr_group);
+}
+
+static int devfreq_set_governor(struct devfreq *df,
+				const struct devfreq_governor *new_gov)
+{
+	guard(mutex)(&devfreq_list_lock);
+	return devfreq_set_governor_locked(df, new_gov);
+}
+
 static int devfreq_notify_transition(struct devfreq *devfreq,
 		struct devfreq_freqs *freqs, unsigned int state)
 {
@@ -936,22 +994,11 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	}
 
 	mutex_lock(&devfreq_list_lock);
-	devfreq->governor = governor;
-	err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
-						NULL);
-	if (err) {
-		dev_err_probe(dev, err,
-			"%s: Unable to start governor for the device\n",
-			 __func__);
-		goto err_init;
-	}
-
-	err = sysfs_update_group(&devfreq->dev.kobj, &gov_attr_group);
+	err = devfreq_set_governor_locked(devfreq, governor);
 	if (err)
-		goto err_init;
+		goto err_devfreq;
 
 	list_add(&devfreq->node, &devfreq_list);
-
 	mutex_unlock(&devfreq_list_lock);
 
 	if (devfreq->profile->is_cooling_device) {
@@ -962,8 +1009,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
 	return devfreq;
 
-err_init:
-	mutex_unlock(&devfreq_list_lock);
 err_devfreq:
 	devfreq_remove_device(devfreq);
 	devfreq = NULL;
@@ -1409,7 +1454,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	struct devfreq *df = to_devfreq(dev);
 	int ret;
 	char str_governor[DEVFREQ_NAME_LEN + 1];
-	const struct devfreq_governor *governor, *prev_governor;
+	const struct devfreq_governor *governor;
 
 	if (!df->governor)
 		return -EINVAL;
@@ -1422,57 +1467,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	if (IS_ERR(governor))
 		return PTR_ERR(governor);
 
-	guard(mutex)(&devfreq_list_lock);
-	if (df->governor == governor)
-		return count;
-
-	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE) ||
-	    IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE))
-		return -EINVAL;
-
-	/*
-	 * Stop the current governor and remove the specific sysfs files
-	 * which depend on current governor.
-	 */
-	ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
-	if (ret) {
-		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
-			 __func__, df->governor->name, ret);
-		return ret;
-	}
-
-	/*
-	 * Start the new governor and create the specific sysfs files
-	 * which depend on the new governor.
-	 */
-	prev_governor = df->governor;
-	df->governor = governor;
-	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
-	if (ret) {
-		dev_warn(dev, "%s: Governor %s not started(%d)\n",
-			 __func__, df->governor->name, ret);
-
-		/* Restore previous governor */
-		df->governor = prev_governor;
-		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
-		if (ret) {
-			dev_err(dev,
-				"%s: reverting to Governor %s failed (%d)\n",
-				__func__, prev_governor->name, ret);
-			df->governor = NULL;
-			return ret;
-		}
-	}
-
-	/*
-	 * Create the sysfs files for the new governor. But if failed to start
-	 * the new governor, restore the sysfs files of previous governor.
-	 */
-	ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group);
-	if (ret)
-		return ret;
+	ret = devfreq_set_governor(df, governor);
 
-	return count;
+	return ret ? ret : count;
 }
 static DEVICE_ATTR_RW(governor);
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 00/10] devfreq: Fix NULL pointer dereference when a governor module is unloaded
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng

When compiled as a kernel module, the governor module can be dynamically
inserted or removed.  'devfreq->governor' would become NULL if the governor
module is removed when it's in use, and NULL pointer dereference would be
triggered.  A similar issue was also reported in [1].

To address this issue:

Patch 1-5 rework mutex, factor out a common governor setting function, and
clean up some unreachable code.

Patch 6-8 prevent a governor module in use from being removed (except for
force unload) by getting/putting a refcount of the governor's module when
switching governors.

Patch 9-10 allow 'governor' and 'available_governors' to work normally even
when a governor module in use is force unloaded.

Note that this series is based on [1] or devfreq-next, otherwise code
would conflict.

[1] https://lore.kernel.org/all/20260319091409.998397-1-tianyaxiong@kylinos.cn/
[2] https://lore.kernel.org/all/20251216031153.2242306-1-zhangpengjie2@huawei.com/

Jie Zhan (8):
  devfreq: Use mutex guard in governor_store()
  devfreq: Use mutex guard in devfreq_add/remove_governor()
  devfreq: Add a dedicated mutex for the governor list
  devfreq: Factor out devfreq_set_governor[_locked]()
  devfreq: Remove dead code in devfreq_add_governor()
  devfreq: Add module owner to devfreq governor
  devfreq: Get and put module refcount when switching governor
  devfreq: Allow find_devfreq_governor() to get module refcount

Zhi Wang (2):
  devfreq: Allow showing available_governors when device governor is
    NULL
  devfreq: Allow setting governor when device governor is NULL

 drivers/devfreq/devfreq.c        | 287 +++++++++++++------------------
 include/linux/devfreq-governor.h |  26 ++-
 2 files changed, 143 insertions(+), 170 deletions(-)

-- 
2.43.0


^ permalink raw reply

* [PATCH v1 02/10] devfreq: Use mutex guard in devfreq_add/remove_governor()
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

Use mutex guard in devfreq_add/remove_governor() so as to simplify the
locking logic.

No functional impact intended.

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 21aa9661de0b..83f75dc21c99 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1263,20 +1263,18 @@ int devfreq_add_governor(struct devfreq_governor *governor)
 {
 	struct devfreq_governor *g;
 	struct devfreq *devfreq;
-	int err = 0;
 
 	if (!governor) {
 		pr_err("%s: Invalid parameters.\n", __func__);
 		return -EINVAL;
 	}
 
-	mutex_lock(&devfreq_list_lock);
+	guard(mutex)(&devfreq_list_lock);
 	g = find_devfreq_governor(governor->name);
 	if (!IS_ERR(g)) {
 		pr_err("%s: governor %s already registered\n", __func__,
 		       g->name);
-		err = -EINVAL;
-		goto err_out;
+		return -EINVAL;
 	}
 
 	list_add(&governor->node, &devfreq_governor_list);
@@ -1313,10 +1311,7 @@ int devfreq_add_governor(struct devfreq_governor *governor)
 		}
 	}
 
-err_out:
-	mutex_unlock(&devfreq_list_lock);
-
-	return err;
+	return 0;
 }
 EXPORT_SYMBOL(devfreq_add_governor);
 
@@ -1354,21 +1349,20 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 {
 	struct devfreq_governor *g;
 	struct devfreq *devfreq;
-	int err = 0;
 
 	if (!governor) {
 		pr_err("%s: Invalid parameters.\n", __func__);
 		return -EINVAL;
 	}
 
-	mutex_lock(&devfreq_list_lock);
+	guard(mutex)(&devfreq_list_lock);
 	g = find_devfreq_governor(governor->name);
 	if (IS_ERR(g)) {
 		pr_err("%s: governor %s not registered\n", __func__,
 		       governor->name);
-		err = PTR_ERR(g);
-		goto err_out;
+		return PTR_ERR(g);
 	}
+
 	list_for_each_entry(devfreq, &devfreq_list, node) {
 		int ret;
 		struct device *dev = devfreq->dev.parent;
@@ -1390,10 +1384,8 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 	}
 
 	list_del(&governor->node);
-err_out:
-	mutex_unlock(&devfreq_list_lock);
 
-	return err;
+	return 0;
 }
 EXPORT_SYMBOL(devfreq_remove_governor);
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 10/10] devfreq: Allow setting governor when device governor is NULL
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

From: Zhi Wang <wangzhi12@huawei.com>

When a device's governor is set to NULL, the system should allow
reconfiguring the governor to restore normal operation of devfreq

Before:
$: echo simple_ondemand > governor
$: rmmod -f governor_simpleondemand
$: echo performance > governor
-bash: echo: write error: Invalid argument

After:
$: echo simple_ondemand > governor
$: rmmod -f governor_simpleondemand
$: echo performance > governor
$: cat governor
performance

Signed-off-by: Zhi Wang <wangzhi12@huawei.com>
Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 5aebb32e89b0..a61880ff2792 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1435,9 +1435,6 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	char str_governor[DEVFREQ_NAME_LEN + 1];
 	const struct devfreq_governor *governor;
 
-	if (!df->governor)
-		return -EINVAL;
-
 	ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor);
 	if (ret != 1)
 		return -EINVAL;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 09/10] devfreq: Allow showing available_governors when device governor is NULL
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

From: Zhi Wang <wangzhi12@huawei.com>

Allow showing available_governors via sysfs when a device's governor is
NULL, enabling reconfiguration of the device's governor.

Before:
$: echo simple_ondemand > governor
$: rmmod -f governor_simpleondemand
$: cat available_governors
cat: available_governors: Invalid argument

After:
$: echo simple_ondemand > governor
$: rmmod -f governor_simpleondemand
$: cat available_governors
hisi_platform userspace powersave performance

Signed-off-by: Zhi Wang <wangzhi12@huawei.com>
Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index c6b670b8fd22..5aebb32e89b0 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1461,16 +1461,13 @@ static ssize_t available_governors_show(struct device *d,
 	struct devfreq *df = to_devfreq(d);
 	ssize_t count = 0;
 
-	if (!df->governor)
-		return -EINVAL;
-
 	mutex_lock(&devfreq_list_lock);
 
 	/*
 	 * The devfreq with immutable governor (e.g., passive) shows
 	 * only own governor.
 	 */
-	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)) {
+	if (df->governor && IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)) {
 		count = scnprintf(&buf[count], DEVFREQ_NAME_LEN,
 				  "%s ", df->governor->name);
 	/*
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 06/10] devfreq: Add module owner to devfreq governor
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

Add an 'owner' member to struct devfreq_governor, such that we can find
the module that holds the governor code when it's compiled as a kernel
module.  This allows the devfreq core to properly manage the lifecycle
of governors.

Update devfreq_add_governor() and devm_devfreq_add_governor() to
automatically set 'owner' to THIS_MODULE via helper macros.

This is a prerequisite for implementing governor reference counting
to prevent module unloading while a governor is in use.

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c        | 26 +++++++++-----------------
 include/linux/devfreq-governor.h | 26 +++++++++++++++++++++++---
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 7e71e8c76303..9b078458d129 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1293,11 +1293,8 @@ void devfreq_resume(void)
 	mutex_unlock(&devfreq_list_lock);
 }
 
-/**
- * devfreq_add_governor() - Add devfreq governor
- * @governor:	the devfreq governor to be added
- */
-int devfreq_add_governor(struct devfreq_governor *governor)
+int __devfreq_add_governor(struct devfreq_governor *governor,
+			   struct module *mod)
 {
 	struct devfreq_governor *g;
 
@@ -1313,38 +1310,33 @@ int devfreq_add_governor(struct devfreq_governor *governor)
 		return -EINVAL;
 	}
 
+	governor->owner = mod;
 	scoped_guard(mutex, &devfreq_gov_lock)
 		list_add(&governor->node, &devfreq_governor_list);
 
 	return 0;
 }
-EXPORT_SYMBOL(devfreq_add_governor);
+EXPORT_SYMBOL(__devfreq_add_governor);
 
 static void devm_devfreq_remove_governor(void *governor)
 {
 	WARN_ON(devfreq_remove_governor(governor));
 }
 
-/**
- * devm_devfreq_add_governor() - Add devfreq governor
- * @dev:	device which adds devfreq governor
- * @governor:	the devfreq governor to be added
- *
- * This is a resource-managed variant of devfreq_add_governor().
- */
-int devm_devfreq_add_governor(struct device *dev,
-			      struct devfreq_governor *governor)
+int __devm_devfreq_add_governor(struct device *dev,
+				struct devfreq_governor *governor,
+				struct module *mod)
 {
 	int err;
 
-	err = devfreq_add_governor(governor);
+	err = __devfreq_add_governor(governor, mod);
 	if (err)
 		return err;
 
 	return devm_add_action_or_reset(dev, devm_devfreq_remove_governor,
 					governor);
 }
-EXPORT_SYMBOL(devm_devfreq_add_governor);
+EXPORT_SYMBOL(__devm_devfreq_add_governor);
 
 /**
  * devfreq_remove_governor() - Remove devfreq feature from a device.
diff --git a/include/linux/devfreq-governor.h b/include/linux/devfreq-governor.h
index dfdd0160a29f..1c4ff57e24de 100644
--- a/include/linux/devfreq-governor.h
+++ b/include/linux/devfreq-governor.h
@@ -12,6 +12,7 @@
 #define __LINUX_DEVFREQ_DEVFREQ_H__
 
 #include <linux/devfreq.h>
+struct module;
 
 #define DEVFREQ_NAME_LEN			16
 
@@ -50,6 +51,7 @@
 /**
  * struct devfreq_governor - Devfreq policy governor
  * @node:		list node - contains registered devfreq governors
+ * @owner:		Module that this governor belongs to
  * @name:		Governor's name
  * @attrs:		Governor's sysfs attribute flags
  * @flags:		Governor's feature flags
@@ -67,6 +69,7 @@
 struct devfreq_governor {
 	struct list_head node;
 
+	struct module *owner;
 	const char name[DEVFREQ_NAME_LEN];
 	const u64 attrs;
 	const u64 flags;
@@ -81,11 +84,28 @@ void devfreq_monitor_suspend(struct devfreq *devfreq);
 void devfreq_monitor_resume(struct devfreq *devfreq);
 void devfreq_update_interval(struct devfreq *devfreq, unsigned int *delay);
 
-int devfreq_add_governor(struct devfreq_governor *governor);
+/**
+ * devfreq_add_governor() - Add devfreq governor
+ * @governor:	The devfreq governor to be added
+ */
+#define devfreq_add_governor(governor) \
+	__devfreq_add_governor((governor), THIS_MODULE)
+int __devfreq_add_governor(struct devfreq_governor *governor,
+			   struct module *mod);
 int devfreq_remove_governor(struct devfreq_governor *governor);
 
-int devm_devfreq_add_governor(struct device *dev,
-			      struct devfreq_governor *governor);
+/**
+ * devm_devfreq_add_governor() - Add devfreq governor
+ * @dev:	device which adds devfreq governor
+ * @governor:	the devfreq governor to be added
+ *
+ * This is a resource-managed variant of devfreq_add_governor().
+ */
+#define devm_devfreq_add_governor(dev, governor) \
+	__devm_devfreq_add_governor((dev), (governor), THIS_MODULE)
+int __devm_devfreq_add_governor(struct device *dev,
+				struct devfreq_governor *governor,
+				struct module *mod);
 
 int devfreq_update_status(struct devfreq *devfreq, unsigned long freq);
 int devfreq_update_target(struct devfreq *devfreq, unsigned long freq);
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 05/10] devfreq: Remove dead code in devfreq_add_governor()
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

In devfreq_add_governor(), we've already checked the governor list to
see if there's a governor already registered with the same name.

It's impossible that a devfreq device is using such a governor, so the
check can never be true and the error handling is unreachable.

Remove the redundant error handling to simplify the logic.

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 378a01f71165..7e71e8c76303 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1300,7 +1300,6 @@ void devfreq_resume(void)
 int devfreq_add_governor(struct devfreq_governor *governor)
 {
 	struct devfreq_governor *g;
-	struct devfreq *devfreq;
 
 	if (!governor) {
 		pr_err("%s: Invalid parameters.\n", __func__);
@@ -1317,39 +1316,6 @@ int devfreq_add_governor(struct devfreq_governor *governor)
 	scoped_guard(mutex, &devfreq_gov_lock)
 		list_add(&governor->node, &devfreq_governor_list);
 
-	guard(mutex)(&devfreq_list_lock);
-	list_for_each_entry(devfreq, &devfreq_list, node) {
-		int ret = 0;
-		struct device *dev = devfreq->dev.parent;
-
-		if (!strncmp(devfreq->governor->name, governor->name,
-			     DEVFREQ_NAME_LEN)) {
-			/* The following should never occur */
-			if (devfreq->governor) {
-				dev_warn(dev,
-					 "%s: Governor %s already present\n",
-					 __func__, devfreq->governor->name);
-				ret = devfreq->governor->event_handler(devfreq,
-							DEVFREQ_GOV_STOP, NULL);
-				if (ret) {
-					dev_warn(dev,
-						 "%s: Governor %s stop = %d\n",
-						 __func__,
-						 devfreq->governor->name, ret);
-				}
-				/* Fall through */
-			}
-			devfreq->governor = governor;
-			ret = devfreq->governor->event_handler(devfreq,
-						DEVFREQ_GOV_START, NULL);
-			if (ret) {
-				dev_warn(dev, "%s: Governor %s start=%d\n",
-					 __func__, devfreq->governor->name,
-					 ret);
-			}
-		}
-	}
-
 	return 0;
 }
 EXPORT_SYMBOL(devfreq_add_governor);
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 01/10] devfreq: Use mutex guard in governor_store()
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

Use mutex guard in governor_store() so as to simplify the locking logic.

No functional impact intended.

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index fcad3edceeea..21aa9661de0b 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1431,20 +1431,17 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	if (ret != 1)
 		return -EINVAL;
 
-	mutex_lock(&devfreq_list_lock);
+	guard(mutex)(&devfreq_list_lock);
 	governor = try_then_request_governor(str_governor);
-	if (IS_ERR(governor)) {
-		ret = PTR_ERR(governor);
-		goto out;
-	}
-	if (df->governor == governor) {
-		ret = 0;
-		goto out;
-	} else if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)
-		|| IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE)) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (IS_ERR(governor))
+		return PTR_ERR(governor);
+
+	if (df->governor == governor)
+		return count;
+
+	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE) ||
+	    IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE))
+		return -EINVAL;
 
 	/*
 	 * Stop the current governor and remove the specific sysfs files
@@ -1454,7 +1451,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	if (ret) {
 		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
 			 __func__, df->governor->name, ret);
-		goto out;
+		return ret;
 	}
 
 	/*
@@ -1476,7 +1473,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 				"%s: reverting to Governor %s failed (%d)\n",
 				__func__, prev_governor->name, ret);
 			df->governor = NULL;
-			goto out;
+			return ret;
 		}
 	}
 
@@ -1485,13 +1482,10 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	 * the new governor, restore the sysfs files of previous governor.
 	 */
 	ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group);
+	if (ret)
+		return ret;
 
-out:
-	mutex_unlock(&devfreq_list_lock);
-
-	if (!ret)
-		ret = count;
-	return ret;
+	return count;
 }
 static DEVICE_ATTR_RW(governor);
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 03/10] devfreq: Add a dedicated mutex for the governor list
From: Jie Zhan @ 2026-03-26 12:34 UTC (permalink / raw)
  To: cw00.choi, myungjoo.ham, kyungmin.park, tianyaxiong
  Cc: linux-pm, linux-arm-kernel, linuxarm, zhanjie9, jonathan.cameron,
	zhenglifeng1, zhangpengjie2, lihuisong, prime.zeng
In-Reply-To: <20260326123428.800407-1-zhanjie9@hisilicon.com>

Accessing the list of available governors ('devfreq_governor_list') is
currently guarded by 'devfreq_list_lock', but 'devfreq_list_lock' is
supposed to protect the list of devfreq devices ('devfreq_list').

The scope of 'devfreq_list_lock' is too broad to maintain.
'devfreq_governor_list' should have its own mutex lock rather than share
with 'devfreq_list'.

Add a governor mutex lock and lock it when accessing the governor list.
Remove locking of 'devfreq_list_lock' around 'devfreq_governor_list'.

This is also a preparation for further refactoring around
try_then_request_governor().

Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
---
 drivers/devfreq/devfreq.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 83f75dc21c99..e54e3092e0e0 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -49,6 +49,7 @@ static struct workqueue_struct *devfreq_wq;
 
 /* The list of all device-devfreq governors */
 static LIST_HEAD(devfreq_governor_list);
+static DEFINE_MUTEX(devfreq_gov_lock);
 /* The list of all device-devfreq */
 static LIST_HEAD(devfreq_list);
 static DEFINE_MUTEX(devfreq_list_lock);
@@ -255,19 +256,18 @@ EXPORT_SYMBOL(devfreq_update_status);
  * @name:	name of the governor
  *
  * Search the list of devfreq governors and return the matched
- * governor's pointer. devfreq_list_lock should be held by the caller.
+ * governor's pointer.
  */
 static struct devfreq_governor *find_devfreq_governor(const char *name)
 {
 	struct devfreq_governor *tmp_governor;
 
-	lockdep_assert_held(&devfreq_list_lock);
-
 	if (IS_ERR_OR_NULL(name)) {
 		pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
 		return ERR_PTR(-EINVAL);
 	}
 
+	guard(mutex)(&devfreq_gov_lock);
 	list_for_each_entry(tmp_governor, &devfreq_governor_list, node) {
 		if (!strncmp(tmp_governor->name, name, DEVFREQ_NAME_LEN))
 			return tmp_governor;
@@ -284,16 +284,13 @@ static struct devfreq_governor *find_devfreq_governor(const char *name)
  * Search the list of devfreq governors and request the module and try again
  * if is not found. This can happen when both drivers (the governor driver
  * and the driver that call devfreq_add_device) are built as modules.
- * devfreq_list_lock should be held by the caller. Returns the matched
- * governor's pointer or an error pointer.
+ * Returns the matched governor's pointer or an error pointer.
  */
 static struct devfreq_governor *try_then_request_governor(const char *name)
 {
 	struct devfreq_governor *governor;
 	int err = 0;
 
-	lockdep_assert_held(&devfreq_list_lock);
-
 	if (IS_ERR_OR_NULL(name)) {
 		pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
 		return ERR_PTR(-EINVAL);
@@ -301,15 +298,12 @@ static struct devfreq_governor *try_then_request_governor(const char *name)
 
 	governor = find_devfreq_governor(name);
 	if (IS_ERR(governor)) {
-		mutex_unlock(&devfreq_list_lock);
-
 		if (!strncmp(name, DEVFREQ_GOV_SIMPLE_ONDEMAND,
 			     DEVFREQ_NAME_LEN))
 			err = request_module("governor_%s", "simpleondemand");
 		else
 			err = request_module("governor_%s", name);
 		/* Restore previous state before return */
-		mutex_lock(&devfreq_list_lock);
 		if (err)
 			return (err < 0) ? ERR_PTR(err) : ERR_PTR(-EINVAL);
 
@@ -933,16 +927,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	if (err)
 		goto err_devfreq;
 
-	mutex_lock(&devfreq_list_lock);
-
 	governor = try_then_request_governor(governor_name);
 	if (IS_ERR(governor)) {
 		dev_err(dev, "%s: Unable to find governor for the device\n",
 			__func__);
 		err = PTR_ERR(governor);
-		goto err_init;
+		goto err_devfreq;
 	}
 
+	mutex_lock(&devfreq_list_lock);
 	devfreq->governor = governor;
 	err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
 						NULL);
@@ -1269,7 +1262,6 @@ int devfreq_add_governor(struct devfreq_governor *governor)
 		return -EINVAL;
 	}
 
-	guard(mutex)(&devfreq_list_lock);
 	g = find_devfreq_governor(governor->name);
 	if (!IS_ERR(g)) {
 		pr_err("%s: governor %s already registered\n", __func__,
@@ -1277,8 +1269,10 @@ int devfreq_add_governor(struct devfreq_governor *governor)
 		return -EINVAL;
 	}
 
-	list_add(&governor->node, &devfreq_governor_list);
+	scoped_guard(mutex, &devfreq_gov_lock)
+		list_add(&governor->node, &devfreq_governor_list);
 
+	guard(mutex)(&devfreq_list_lock);
 	list_for_each_entry(devfreq, &devfreq_list, node) {
 		int ret = 0;
 		struct device *dev = devfreq->dev.parent;
@@ -1355,7 +1349,6 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 		return -EINVAL;
 	}
 
-	guard(mutex)(&devfreq_list_lock);
 	g = find_devfreq_governor(governor->name);
 	if (IS_ERR(g)) {
 		pr_err("%s: governor %s not registered\n", __func__,
@@ -1363,6 +1356,7 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 		return PTR_ERR(g);
 	}
 
+	guard(mutex)(&devfreq_list_lock);
 	list_for_each_entry(devfreq, &devfreq_list, node) {
 		int ret;
 		struct device *dev = devfreq->dev.parent;
@@ -1383,7 +1377,8 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
 		}
 	}
 
-	list_del(&governor->node);
+	scoped_guard(mutex, &devfreq_gov_lock)
+		list_del(&governor->node);
 
 	return 0;
 }
@@ -1423,11 +1418,11 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
 	if (ret != 1)
 		return -EINVAL;
 
-	guard(mutex)(&devfreq_list_lock);
 	governor = try_then_request_governor(str_governor);
 	if (IS_ERR(governor))
 		return PTR_ERR(governor);
 
+	guard(mutex)(&devfreq_list_lock);
 	if (df->governor == governor)
 		return count;
 
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v2 0/2] Support BPF traversal of wakeup sources
From: Puranjay Mohan @ 2026-03-26 12:20 UTC (permalink / raw)
  To: Samuel Wu, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Greg Kroah-Hartman, Danilo Krummrich, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan
  Cc: memxor, Samuel Wu, kernel-team, linux-kernel, linux-pm,
	driver-core, bpf, linux-kselftest
In-Reply-To: <20260326112521.2827500-1-wusamuel@google.com>

Samuel Wu <wusamuel@google.com> writes:

> This patchset adds requisite kfuncs for BPF programs to safely traverse
> wakeup_sources, and puts a config flag around the sysfs interface.
>
> Currently, a traversal of wakeup sources require going through
> /sys/class/wakeup/* or /d/wakeup_sources/*. The repeated syscalls to query
> sysfs is inefficient, as there can be hundreds of wakeup_sources, with each
> wakeup source also having multiple attributes. debugfs is unstable and
> insecure.
>
> Adding kfuncs to lock/unlock wakeup sources allows BPF program to safely
> traverse the wakeup sources list. The head address of wakeup_sources can
> safely be resolved through BPF helper functions or variable attributes.
>
> On a quiescent Pixel 6 traversing 150 wakeup_sources, I am seeing ~34x
> speedup (sampled 75 times in table below). For a device under load, the
> speedup is greater.
> +-------+----+----------+----------+
> |       | n  | AVG (ms) | STD (ms) |
> +-------+----+----------+----------+
> | sysfs | 75 | 44.9     | 12.6     |
> +-------+----+----------+----------+
> | BPF   | 75 | 1.3      | 0.7      |
> +-------+----+----------+----------+
>
> The initial attempts for BPF traversal of wakeup_sources was with BPF
> iterators [1]. However, BPF already allows for traversing of a simple list
> with bpf_for(), and this current patchset has the added benefit of being
> ~2-3x more performant than BPF iterators.

I left some inline comments on patch 1, but the high level concern is
that encoding the SRCU index into a fake pointer to get KF_ACQUIRE/
KF_RELEASE tracking is working against the verifier rather than with it.
Nothing actually prevents a BPF program from walking the list without
the lock, and the whole pointer encoding trick goes away if this is done
as an open-coded iterator instead.

Thanks,
Puranjay

^ permalink raw reply

* Re: [PATCH v3 5/9] mfd: mt6397: Add support for MT6392 pmic
From: Lee Jones @ 2026-03-26 12:10 UTC (permalink / raw)
  To: Luca Leonardo Scorcia
  Cc: linux-mediatek, Fabien Parent, Val Packett, Dmitry Torokhov,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sen Chu,
	Sean Wang, Macpaul Lin, Matthias Brugger,
	AngeloGioacchino Del Regno, Linus Walleij, Liam Girdwood,
	Mark Brown, Julien Massot, Gary Bisson, Louis-Alexis Eyraud,
	Chen Zhong, linux-input, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, linux-gpio
In-Reply-To: <20260317184507.523060-6-l.scorcia@gmail.com>

On Tue, 17 Mar 2026, Luca Leonardo Scorcia wrote:

> From: Fabien Parent <parent.f@gmail.com>
> 
> Update the MT6397 MFD driver to support the MT6392 PMIC.
> 
> Signed-off-by: Fabien Parent <parent.f@gmail.com>
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
>  drivers/mfd/mt6397-core.c            |  46 +++
>  drivers/mfd/mt6397-irq.c             |   8 +
>  include/linux/mfd/mt6392/core.h      |  42 +++
>  include/linux/mfd/mt6392/registers.h | 487 +++++++++++++++++++++++++++
>  include/linux/mfd/mt6397/core.h      |   1 +
>  5 files changed, 584 insertions(+)
>  create mode 100644 include/linux/mfd/mt6392/core.h
>  create mode 100644 include/linux/mfd/mt6392/registers.h
> 
> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index 3e58d0764c7e..c4b86a44c68b 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -18,6 +18,7 @@
>  #include <linux/mfd/mt6357/core.h>
>  #include <linux/mfd/mt6358/core.h>
>  #include <linux/mfd/mt6359/core.h>
> +#include <linux/mfd/mt6392/core.h>
>  #include <linux/mfd/mt6397/core.h>
>  #include <linux/mfd/mt6323/registers.h>
>  #include <linux/mfd/mt6328/registers.h>
> @@ -25,6 +26,7 @@
>  #include <linux/mfd/mt6357/registers.h>
>  #include <linux/mfd/mt6358/registers.h>
>  #include <linux/mfd/mt6359/registers.h>
> +#include <linux/mfd/mt6392/registers.h>
>  #include <linux/mfd/mt6397/registers.h>
>  
>  #define MT6323_RTC_BASE		0x8000
> @@ -39,6 +41,9 @@
>  #define MT6358_RTC_BASE		0x0588
>  #define MT6358_RTC_SIZE		0x3c
>  
> +#define MT6392_RTC_BASE		0x8000
> +#define MT6392_RTC_SIZE		0x3e
> +
>  #define MT6397_RTC_BASE		0xe000
>  #define MT6397_RTC_SIZE		0x3e
>  
> @@ -65,6 +70,11 @@ static const struct resource mt6358_rtc_resources[] = {
>  	DEFINE_RES_IRQ(MT6358_IRQ_RTC),
>  };
>  
> +static const struct resource mt6392_rtc_resources[] = {
> +	DEFINE_RES_MEM(MT6392_RTC_BASE, MT6392_RTC_SIZE),
> +	DEFINE_RES_IRQ(MT6392_IRQ_RTC),
> +};
> +
>  static const struct resource mt6397_rtc_resources[] = {
>  	DEFINE_RES_MEM(MT6397_RTC_BASE, MT6397_RTC_SIZE),
>  	DEFINE_RES_IRQ(MT6397_IRQ_RTC),
> @@ -114,6 +124,11 @@ static const struct resource mt6331_keys_resources[] = {
>  	DEFINE_RES_IRQ_NAMED(MT6331_IRQ_STATUS_HOMEKEY, "homekey"),
>  };
>  
> +static const struct resource mt6392_keys_resources[] = {
> +	DEFINE_RES_IRQ_NAMED(MT6392_IRQ_PWRKEY, "powerkey"),
> +	DEFINE_RES_IRQ_NAMED(MT6392_IRQ_FCHRKEY, "homekey"),
> +};
> +
>  static const struct resource mt6397_keys_resources[] = {
>  	DEFINE_RES_IRQ_NAMED(MT6397_IRQ_PWRKEY, "powerkey"),
>  	DEFINE_RES_IRQ_NAMED(MT6397_IRQ_HOMEKEY, "homekey"),
> @@ -253,6 +268,26 @@ static const struct mfd_cell mt6359_devs[] = {
>  	},
>  };
>  
> +static const struct mfd_cell mt6392_devs[] = {
> +	{
> +		.name = "mt6392-rtc",
> +		.num_resources = ARRAY_SIZE(mt6392_rtc_resources),
> +		.resources = mt6392_rtc_resources,
> +		.of_compatible = "mediatek,mt6392-rtc",
> +	}, {
> +		.name = "mt6392-regulator",
> +		.of_compatible = "mediatek,mt6392-regulator",
> +	}, {
> +		.name = "mt6392-pinctrl",
> +		.of_compatible = "mediatek,mt6392-pinctrl",
> +	}, {
> +		.name = "mt6392-keys",
> +		.num_resources = ARRAY_SIZE(mt6392_keys_resources),
> +		.resources = mt6392_keys_resources,
> +		.of_compatible = "mediatek,mt6392-keys"
> +	},
> +};
> +
>  static const struct mfd_cell mt6397_devs[] = {
>  	{
>  		.name = "mt6397-rtc",
> @@ -335,6 +370,14 @@ static const struct chip_data mt6359_core = {
>  	.irq_init = mt6358_irq_init,
>  };
>  
> +static const struct chip_data mt6392_core = {
> +	.cid_addr = MT6392_CID,
> +	.cid_shift = 0,
> +	.cells = mt6392_devs,

I'm not really sure what came over me when I accepted this 6 years ago,
but I have a _strong_ aversion to MFD data being passed through the OF APIs.

Before this patch lands, please could you refactor this driver to only
pass through an identifier through mt6397_of_match[*].data.  Then, you
can match on that via a switch statement where you can allocate each
device's data structures.

This is how the vast majority of MFD drivers work so there should be
lots of examples to work through to make this trivial.

> +	.cell_size = ARRAY_SIZE(mt6392_devs),
> +	.irq_init = mt6397_irq_init,
> +};

-- 
Lee Jones [李琼斯]

^ permalink raw reply

* Re: [PATCH v4 2/2] cpufreq: Pass the policy to cpufreq_driver->adjust_perf()
From: Gautham R. Shenoy @ 2026-03-26 12:04 UTC (permalink / raw)
  To: K Prateek Nayak
  Cc: Rafael J. Wysocki, Viresh Kumar, Huang Rui, Mario Limonciello,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
	Srinivas Pandruvada, Len Brown, Ingo Molnar, Peter Zijlstra,
	Juri Lelli, Vincent Guittot, Miguel Ojeda, Perry Yuan, linux-pm,
	linux-kernel, rust-for-linux, linux-rt-devel, Dietmar Eggemann,
	Ben Segall, Mel Gorman, Valentin Schneider, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Bert Karwatzki
In-Reply-To: <20260316081849.19368-3-kprateek.nayak@amd.com>

On Mon, Mar 16, 2026 at 08:18:49AM +0000, K Prateek Nayak wrote:
> cpufreq_cpu_get() can sleep on PREEMPT_RT in presence of concurrent
> writer(s), however amd-pstate depends on fetching the cpudata via the
> policy's driver data which necessitates grabbing the reference.
> 
> Since schedutil governor can call "cpufreq_driver->update_perf()"
> during sched_tick/enqueue/dequeue with rq_lock held and IRQs disabled,
> fetching the policy object using the cpufreq_cpu_get() helper in the
> scheduler fast-path leads to "BUG: scheduling while atomic" on
> PREEMPT_RT [1].
> 
> Pass the cached cpufreq policy object in sg_policy to the update_perf()
> instead of just the CPU. The CPU can be inferred using "policy->cpu".
> 
> The lifetime of cpufreq_policy object outlasts that of the governor and
> the cpufreq driver (allocated when the CPU is onlined and only reclaimed
> when the CPU is offlined / the CPU device is removed) which makes it
> safe to be referenced throughout the governor's lifetime.
> 
> Fixes: 1d215f0319c2 ("cpufreq: amd-pstate: Add fast switch function for AMD P-State")
> Reported-by: Bert Karwatzki <spasswolf@web.de>
> Closes:https://lore.kernel.org/all/20250731092316.3191-1-spasswolf@web.de/ [1]
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>


-- 
Thanks and Regards
gautham.
> ---
> changelog v3..v4:
> 
> o Added the Fixes tag. (Gautham, Chris Mason's review-prompts)
> ---
>  drivers/cpufreq/amd-pstate.c     |  3 +--
>  drivers/cpufreq/cpufreq.c        |  6 +++---
>  drivers/cpufreq/intel_pstate.c   |  4 ++--
>  include/linux/cpufreq.h          |  4 ++--
>  kernel/sched/cpufreq_schedutil.c |  5 +++--
>  rust/kernel/cpufreq.rs           | 13 ++++++-------
>  6 files changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5faccb3d6b14..ad4b5f84773a 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -710,13 +710,12 @@ static unsigned int amd_pstate_fast_switch(struct cpufreq_policy *policy,
>  	return policy->cur;
>  }
>  
> -static void amd_pstate_adjust_perf(unsigned int cpu,
> +static void amd_pstate_adjust_perf(struct cpufreq_policy *policy,
>  				   unsigned long _min_perf,
>  				   unsigned long target_perf,
>  				   unsigned long capacity)
>  {
>  	u8 max_perf, min_perf, des_perf, cap_perf;
> -	struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpu);
>  	struct amd_cpudata *cpudata;
>  	union perf_cached perf;
>  
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 2082a9e4384f..17a5b8e0ea1e 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2231,7 +2231,7 @@ EXPORT_SYMBOL_GPL(cpufreq_driver_fast_switch);
>  
>  /**
>   * cpufreq_driver_adjust_perf - Adjust CPU performance level in one go.
> - * @cpu: Target CPU.
> + * @policy: cpufreq policy object of the target CPU.
>   * @min_perf: Minimum (required) performance level (units of @capacity).
>   * @target_perf: Target (desired) performance level (units of @capacity).
>   * @capacity: Capacity of the target CPU.
> @@ -2250,12 +2250,12 @@ EXPORT_SYMBOL_GPL(cpufreq_driver_fast_switch);
>   * parallel with either ->target() or ->target_index() or ->fast_switch() for
>   * the same CPU.
>   */
> -void cpufreq_driver_adjust_perf(unsigned int cpu,
> +void cpufreq_driver_adjust_perf(struct cpufreq_policy *policy,
>  				 unsigned long min_perf,
>  				 unsigned long target_perf,
>  				 unsigned long capacity)
>  {
> -	cpufreq_driver->adjust_perf(cpu, min_perf, target_perf, capacity);
> +	cpufreq_driver->adjust_perf(policy, min_perf, target_perf, capacity);
>  }
>  
>  /**
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 51938c5a47ca..1552b2d32a34 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -3239,12 +3239,12 @@ static unsigned int intel_cpufreq_fast_switch(struct cpufreq_policy *policy,
>  	return target_pstate * cpu->pstate.scaling;
>  }
>  
> -static void intel_cpufreq_adjust_perf(unsigned int cpunum,
> +static void intel_cpufreq_adjust_perf(struct cpufreq_policy *policy,
>  				      unsigned long min_perf,
>  				      unsigned long target_perf,
>  				      unsigned long capacity)
>  {
> -	struct cpudata *cpu = all_cpu_data[cpunum];
> +	struct cpudata *cpu = all_cpu_data[policy->cpu];
>  	u64 hwp_cap = READ_ONCE(cpu->hwp_cap_cached);
>  	int old_pstate = cpu->pstate.current_pstate;
>  	int cap_pstate, min_pstate, max_pstate, target_pstate;
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index cc894fc38971..4317c5a312bd 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -372,7 +372,7 @@ struct cpufreq_driver {
>  	 * conditions) scale invariance can be disabled, which causes the
>  	 * schedutil governor to fall back to the latter.
>  	 */
> -	void		(*adjust_perf)(unsigned int cpu,
> +	void		(*adjust_perf)(struct cpufreq_policy *policy,
>  				       unsigned long min_perf,
>  				       unsigned long target_perf,
>  				       unsigned long capacity);
> @@ -617,7 +617,7 @@ struct cpufreq_governor {
>  /* Pass a target to the cpufreq driver */
>  unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
>  					unsigned int target_freq);
> -void cpufreq_driver_adjust_perf(unsigned int cpu,
> +void cpufreq_driver_adjust_perf(struct cpufreq_policy *policy,
>  				unsigned long min_perf,
>  				unsigned long target_perf,
>  				unsigned long capacity);
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index 153232dd8276..ae9fd211cec1 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -461,6 +461,7 @@ static void sugov_update_single_perf(struct update_util_data *hook, u64 time,
>  				     unsigned int flags)
>  {
>  	struct sugov_cpu *sg_cpu = container_of(hook, struct sugov_cpu, update_util);
> +	struct sugov_policy *sg_policy = sg_cpu->sg_policy;
>  	unsigned long prev_util = sg_cpu->util;
>  	unsigned long max_cap;
>  
> @@ -482,10 +483,10 @@ static void sugov_update_single_perf(struct update_util_data *hook, u64 time,
>  	if (sugov_hold_freq(sg_cpu) && sg_cpu->util < prev_util)
>  		sg_cpu->util = prev_util;
>  
> -	cpufreq_driver_adjust_perf(sg_cpu->cpu, sg_cpu->bw_min,
> +	cpufreq_driver_adjust_perf(sg_policy->policy, sg_cpu->bw_min,
>  				   sg_cpu->util, max_cap);
>  
> -	sg_cpu->sg_policy->last_freq_update_time = time;
> +	sg_policy->last_freq_update_time = time;
>  }
>  
>  static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
> diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
> index 76faa1ac8501..a83aec198336 100644
> --- a/rust/kernel/cpufreq.rs
> +++ b/rust/kernel/cpufreq.rs
> @@ -1256,18 +1256,17 @@ impl<T: Driver> Registration<T> {
>      /// # Safety
>      ///
>      /// - This function may only be called from the cpufreq C infrastructure.
> +    /// - The pointer arguments must be valid pointers.
>      unsafe extern "C" fn adjust_perf_callback(
> -        cpu: c_uint,
> +        ptr: *mut bindings::cpufreq_policy,
>          min_perf: c_ulong,
>          target_perf: c_ulong,
>          capacity: c_ulong,
>      ) {
> -        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
> -        let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) };
> -
> -        if let Ok(mut policy) = PolicyCpu::from_cpu(cpu_id) {
> -            T::adjust_perf(&mut policy, min_perf, target_perf, capacity);
> -        }
> +        // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
> +        // lifetime of `policy`.
> +        let policy = unsafe { Policy::from_raw_mut(ptr) };
> +        T::adjust_perf(policy, min_perf, target_perf, capacity);
>      }
>  
>      /// Driver's `get_intermediate` callback.
> -- 
> 2.34.1
> 

^ permalink raw reply

* Re: (subset) [PATCH v2 4/9] leds: Kconfig: drop unneeded dependency on OF_GPIO
From: Lee Jones @ 2026-03-26 12:03 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Andrew Lunn, Heiner Kallweit,
	Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Linus Walleij, Lee Jones, Pavel Machek,
	Wim Van Sebroeck, Guenter Roeck, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Sebastian Reichel, Bartosz Golaszewski
  Cc: brgl, linux-arm-kernel, linux-kernel, netdev, linux-gpio,
	linux-leds, linux-watchdog, linux-media, linux-staging, linux-pm
In-Reply-To: <20260316-gpio-of-kconfig-v2-4-de2f4b00a0e4@oss.qualcomm.com>

On Mon, 16 Mar 2026 10:45:24 +0100, Bartosz Golaszewski wrote:
> OF_GPIO is selected automatically on all OF systems. Any symbols it
> controls also provide stubs so there's really no reason to select it
> explicitly.

Applied, thanks!

[4/9] leds: Kconfig: drop unneeded dependency on OF_GPIO
      commit: b727ba2560a8a806680b45c9acc5a49bc39b8e43

--
Lee Jones [李琼斯]


^ permalink raw reply

* Re: [PATCH v4 1/2] cpufreq/amd-pstate: Pass the policy to amd_pstate_update()
From: Gautham R. Shenoy @ 2026-03-26 12:03 UTC (permalink / raw)
  To: K Prateek Nayak
  Cc: Rafael J. Wysocki, Viresh Kumar, Huang Rui, Mario Limonciello,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
	Perry Yuan, linux-pm, linux-kernel, rust-for-linux,
	linux-rt-devel, Mario Limonciello
In-Reply-To: <20260316081849.19368-2-kprateek.nayak@amd.com>

On Mon, Mar 16, 2026 at 08:18:48AM +0000, K Prateek Nayak wrote:
> All callers of amd_pstate_update() already have a reference to the
> cpufreq_policy object.
> 
> Pass the entire policy object and grab the cpudata using
> "policy->driver_data" instead of passing the cpudata and unnecessarily
> grabbing another read-side reference to the cpufreq policy object when
> it is already available in the caller.
> 
> No functional changes intended.
> 
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

> ---
> changelog v3..v4:
> 
> o No changes.
> ---
>  drivers/cpufreq/amd-pstate.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 5aa9fcd80cf5..5faccb3d6b14 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -565,15 +565,12 @@ static inline bool amd_pstate_sample(struct amd_cpudata *cpudata)
>  	return true;
>  }
>  
> -static void amd_pstate_update(struct amd_cpudata *cpudata, u8 min_perf,
> +static void amd_pstate_update(struct cpufreq_policy *policy, u8 min_perf,
>  			      u8 des_perf, u8 max_perf, bool fast_switch, int gov_flags)
>  {
> -	struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpudata->cpu);
> +	struct amd_cpudata *cpudata = policy->driver_data;
>  	union perf_cached perf = READ_ONCE(cpudata->perf);
>  
> -	if (!policy)
> -		return;
> -
>  	/* limit the max perf when core performance boost feature is disabled */
>  	if (!cpudata->boost_supported)
>  		max_perf = min_t(u8, perf.nominal_perf, max_perf);
> @@ -688,7 +685,7 @@ static int amd_pstate_update_freq(struct cpufreq_policy *policy,
>  	if (!fast_switch)
>  		cpufreq_freq_transition_begin(policy, &freqs);
>  
> -	amd_pstate_update(cpudata, perf.min_limit_perf, des_perf,
> +	amd_pstate_update(policy, perf.min_limit_perf, des_perf,
>  			  perf.max_limit_perf, fast_switch,
>  			  policy->governor->flags);
>  
> @@ -750,7 +747,7 @@ static void amd_pstate_adjust_perf(unsigned int cpu,
>  	if (max_perf < min_perf)
>  		max_perf = min_perf;
>  
> -	amd_pstate_update(cpudata, min_perf, des_perf, max_perf, true,
> +	amd_pstate_update(policy, min_perf, des_perf, max_perf, true,
>  			policy->governor->flags);
>  }
>  
> -- 
> 2.34.1
> 

-- 
Thanks and Regards
gautham.

^ permalink raw reply

* Re: [PATCH v2 1/2] PM: wakeup: Add kfuncs to traverse over wakeup_sources
From: Puranjay Mohan @ 2026-03-26 11:59 UTC (permalink / raw)
  To: Samuel Wu, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Greg Kroah-Hartman, Danilo Krummrich, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan
  Cc: memxor, Samuel Wu, kernel-team, linux-kernel, linux-pm,
	driver-core, bpf, linux-kselftest
In-Reply-To: <20260326112521.2827500-2-wusamuel@google.com>

Samuel Wu <wusamuel@google.com> writes:

> Iterating through wakeup sources via sysfs or debugfs can be inefficient
> or restricted. Introduce BPF kfuncs to allow high-performance and safe
> in-kernel traversal of the wakeup_sources list.
>
> The new kfuncs include:
> - bpf_wakeup_sources_get_head() to obtain the list head.
> - bpf_wakeup_sources_read_lock/unlock() to manage the SRCU lock.
>
> For verifier safety, the underlying SRCU index is wrapped in an opaque
> 'struct bpf_ws_lock' pointer. This enables the use of KF_ACQUIRE and
> KF_RELEASE flags, allowing the BPF verifier to strictly enforce paired
> lock/unlock cycles and prevent resource leaks.
>
> Signed-off-by: Samuel Wu <wusamuel@google.com>
> ---
>  drivers/base/power/power.h  |  7 ++++
>  drivers/base/power/wakeup.c | 72 +++++++++++++++++++++++++++++++++++--
>  2 files changed, 77 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
> index 922ed457db19..ced563286ac5 100644
> --- a/drivers/base/power/power.h
> +++ b/drivers/base/power/power.h
> @@ -168,3 +168,10 @@ static inline void device_pm_init(struct device *dev)
>  	device_pm_sleep_init(dev);
>  	pm_runtime_init(dev);
>  }
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +struct bpf_ws_lock { };
> +struct bpf_ws_lock *bpf_wakeup_sources_read_lock(void);
> +void bpf_wakeup_sources_read_unlock(struct bpf_ws_lock *lock);
> +struct list_head *bpf_wakeup_sources_get_head(void);
> +#endif
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index b8e48a023bf0..27a26a30b6ee 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -1168,11 +1168,79 @@ static const struct file_operations wakeup_sources_stats_fops = {
>  	.release = seq_release_private,
>  };
>  
> -static int __init wakeup_sources_debugfs_init(void)
> +#ifdef CONFIG_BPF_SYSCALL
> +#include <linux/btf.h>
> +
> +__bpf_kfunc_start_defs();
> +
> +/**
> + * bpf_wakeup_sources_read_lock - Acquire the SRCU lock for wakeup sources
> + *
> + * The underlying SRCU lock returns an integer index. However, the BPF verifier
> + * requires a pointer (PTR_TO_BTF_ID) to strictly track the state of acquired
> + * resources using KF_ACQUIRE and KF_RELEASE semantics. We use an opaque
> + * structure pointer (struct bpf_ws_lock *) to satisfy the verifier while
> + * safely encoding the integer index within the pointer address itself.
> + *
> + * Return: An opaque pointer encoding the SRCU lock index + 1 (to avoid NULL).
> + */
> +__bpf_kfunc struct bpf_ws_lock *bpf_wakeup_sources_read_lock(void)
> +{
> +	return (struct bpf_ws_lock *)(long)(wakeup_sources_read_lock() + 1);
> +}
> +
> +/**
> + * bpf_wakeup_sources_read_unlock - Release the SRCU lock for wakeup sources
> + * @lock: The opaque pointer returned by bpf_wakeup_sources_read_lock()
> + *
> + * The BPF verifier guarantees that @lock is a valid, unreleased pointer from
> + * the acquire function. We decode the pointer back into the integer SRCU index
> + * by subtracting 1 and release the lock.
> + */
> +__bpf_kfunc void bpf_wakeup_sources_read_unlock(struct bpf_ws_lock *lock)
> +{
> +	wakeup_sources_read_unlock((int)(long)lock - 1);
> +}
> +
> +/**
> + * bpf_wakeup_sources_get_head - Get the head of the wakeup sources list
> + *
> + * Return: The head of the wakeup sources list.
> + */
> +__bpf_kfunc struct list_head *bpf_wakeup_sources_get_head(void)
> +{
> +	return &wakeup_sources;
> +}

What stops a bpf program from using this kfunc to get the wakeup_sources
pointer and iterating this list without taking the srcu lock? 

An open-coded iterator would solve this by internalizing the lock:
  // Iterator new() acquires SRCU lock
  // Iterator next() returns next wakeup_source
  // Iterator destroy() releases SRCU lock

> +
> +__bpf_kfunc_end_defs();
> +
> +BTF_KFUNCS_START(wakeup_source_kfunc_ids)
> +BTF_ID_FLAGS(func, bpf_wakeup_sources_read_lock, KF_ACQUIRE)

I think this needs KF_RET_NULL as well.

> +BTF_ID_FLAGS(func, bpf_wakeup_sources_read_unlock, KF_RELEASE)
> +BTF_ID_FLAGS(func, bpf_wakeup_sources_get_head)
> +BTF_KFUNCS_END(wakeup_source_kfunc_ids)
> +
> +static const struct btf_kfunc_id_set wakeup_source_kfunc_set = {
> +	.owner = THIS_MODULE,
> +	.set   = &wakeup_source_kfunc_ids,
> +};
> +
> +static void __init wakeup_sources_bpf_init(void)
> +{
> +	if (register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &wakeup_source_kfunc_set))
> +		pm_pr_dbg("Wakeup: failed to register BTF kfuncs\n");
> +}
> +#else
> +static inline void wakeup_sources_bpf_init(void) {}
> +#endif /* CONFIG_BPF_SYSCALL */
> +
> +static int __init wakeup_sources_init(void)
>  {
>  	debugfs_create_file("wakeup_sources", 0444, NULL, NULL,
>  			    &wakeup_sources_stats_fops);
> +	wakeup_sources_bpf_init();
> +
>  	return 0;
>  }
>  
> -postcore_initcall(wakeup_sources_debugfs_init);
> +postcore_initcall(wakeup_sources_init);
> -- 
> 2.53.0.1018.g2bb0e51243-goog

^ permalink raw reply

* [PATCH v4 12/12] Documentation/amd-pstate: Add documentation for amd_pstate_floor_{freq,count}
From: Gautham R. Shenoy @ 2026-03-26 11:47 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Viresh Kumar,
	K Prateek Nayak
  Cc: linux-kernel, linux-pm, Gautham R. Shenoy, Jonathan Corbet,
	Shuah Khan, Mario Limonciello
In-Reply-To: <20260326114756.20374-1-gautham.shenoy@amd.com>

Add documentation for the sysfs files
/sys/devices/system/cpu/cpufreq/policy*/amd_pstate_floor_freq
and
/sys/devices/system/cpu/cpufreq/policy*/amd_pstate_floor_count.

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 32 +++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index b31a478c28ba..d6c2f233ab23 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -242,6 +242,8 @@ control its functionality at the system level. They are located in the
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_hw_prefcore
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_lowest_nonlinear_freq
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_max_freq
+ /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_floor_freq
+ /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_floor_count
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_prefcore_ranking
 
 
@@ -277,6 +279,36 @@ larger numbers are preferred at the time of reading. This can change at
 runtime based on platform conditions. This attribute is read-only. This file
 is only visible on platforms which support the preferred core feature.
 
+``amd_pstate_floor_freq``
+
+The floor frequency associated with each CPU. Userspace can write any
+value between ``cpuinfo_min_freq`` and ``scaling_max_freq`` into this
+file. When the system is under power or thermal constraints, the
+platform firmware will attempt to throttle the CPU frequency to the
+value specified in ``amd_pstate_floor_freq`` before throttling it
+further. This allows userspace to specify different floor frequencies
+to different CPUs. For optimal results, threads of the same core
+should have the same floor frequency value. This file is only visible
+on platforms that support the CPPC Performance Priority feature.
+
+
+``amd_pstate_floor_count``
+
+The number of distinct Floor Performance levels supported by the
+platform. For example, if this value is 2, then the number of unique
+values obtained from the command ``cat
+/sys/devices/system/cpu/cpufreq/policy*/amd_pstate_floor_freq |
+sort -n | uniq`` should be at most this number for the behavior
+described in ``amd_pstate_floor_freq`` to take effect. A zero value
+implies that the platform supports unlimited floor performance levels.
+This file is only visible on platforms that support the CPPC
+Performance Priority feature.
+
+**Note**: When ``amd_pstate_floor_count`` is non-zero, the frequency to
+which the CPU is throttled under power or thermal constraints is
+undefined when the number of unique values of ``amd_pstate_floor_freq``
+across all CPUs in the system exceeds ``amd_pstate_floor_count``.
+
 ``energy_performance_available_preferences``
 
 A list of all the supported EPP preferences that could be used for
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 10/12] Documentation/amd-pstate: List amd_pstate_hw_prefcore sysfs file
From: Gautham R. Shenoy @ 2026-03-26 11:47 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Viresh Kumar,
	K Prateek Nayak
  Cc: linux-kernel, linux-pm, Gautham R. Shenoy, Jonathan Corbet,
	Shuah Khan, Mario Limonciello
In-Reply-To: <20260326114756.20374-1-gautham.shenoy@amd.com>

Add the missing amd_pstate_hw_prefcore filenames in the sysfs listing
example leading to the descriptions of these parameters. Clarify when
will the file be visible.

Fixes: b96b82d1af7f ("cpufreq: amd-pstate: Add documentation for `amd_pstate_hw_prefcore`")
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index e1771f2225d5..b8c846cbf301 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -239,6 +239,7 @@ control its functionality at the system level. They are located in the
 
  root@hr-test1:/home/ray# ls /sys/devices/system/cpu/cpufreq/policy0/*amd*
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_highest_perf
+ /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_hw_prefcore
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_lowest_nonlinear_freq
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_max_freq
 
@@ -264,8 +265,9 @@ This attribute is read-only.
 
 ``amd_pstate_hw_prefcore``
 
-Whether the platform supports the preferred core feature and it has been
-enabled. This attribute is read-only.
+Whether the platform supports the preferred core feature and it has
+been enabled. This attribute is read-only. This file is only visible
+on platforms which support the preferred core feature.
 
 ``amd_pstate_prefcore_ranking``
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 11/12] Documentation/amd-pstate: List amd_pstate_prefcore_ranking sysfs file
From: Gautham R. Shenoy @ 2026-03-26 11:47 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Viresh Kumar,
	K Prateek Nayak
  Cc: linux-kernel, linux-pm, Gautham R. Shenoy, Jonathan Corbet,
	Shuah Khan, Mario Limonciello
In-Reply-To: <20260326114756.20374-1-gautham.shenoy@amd.com>

Add the missing amd_pstate_prefcore_ranking filenames in the sysfs
listing example leading to the descriptions of these
parameters. Clarify when will the file be visible.

Fixes: 15a2b764ea7c ("amd-pstate: Add missing documentation for `amd_pstate_prefcore_ranking`")
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 Documentation/admin-guide/pm/amd-pstate.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
index b8c846cbf301..b31a478c28ba 100644
--- a/Documentation/admin-guide/pm/amd-pstate.rst
+++ b/Documentation/admin-guide/pm/amd-pstate.rst
@@ -242,6 +242,7 @@ control its functionality at the system level. They are located in the
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_hw_prefcore
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_lowest_nonlinear_freq
  /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_max_freq
+ /sys/devices/system/cpu/cpufreq/policy0/amd_pstate_prefcore_ranking
 
 
 ``amd_pstate_highest_perf / amd_pstate_max_freq``
@@ -273,7 +274,8 @@ on platforms which support the preferred core feature.
 
 The performance ranking of the core. This number doesn't have any unit, but
 larger numbers are preferred at the time of reading. This can change at
-runtime based on platform conditions. This attribute is read-only.
+runtime based on platform conditions. This attribute is read-only. This file
+is only visible on platforms which support the preferred core feature.
 
 ``energy_performance_available_preferences``
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 09/12] amd-pstate-ut: Add a testcase to validate the visibility of driver attributes
From: Gautham R. Shenoy @ 2026-03-26 11:47 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Viresh Kumar,
	K Prateek Nayak
  Cc: linux-kernel, linux-pm, Gautham R. Shenoy
In-Reply-To: <20260326114756.20374-1-gautham.shenoy@amd.com>

amd-pstate driver has per-attribute visibility functions to
dynamically control which sysfs freq_attrs are exposed based on the
platform capabilities and the current amd_pstate mode. However, there
is no test coverage to validate that the driver's live attribute list
matches the expected visibility for each mode.

Add amd_pstate_ut_check_freq_attrs() to the amd-pstate unit test
module. For each enabled mode (passive, active, guided), the test
independently derives the expected visibility of each attribute:
  - Core attributes (max_freq, lowest_nonlinear_freq, highest_perf)
    are always expected.
  - Prefcore attributes (prefcore_ranking, hw_prefcore) are expected
    only when cpudata->hw_prefcore indicates platform support.
  - EPP attributes (energy_performance_preference,
    energy_performance_available_preferences) are expected only in
    active mode.
  - Floor frequency attributes (floor_freq, floor_count) are expected
    only when X86_FEATURE_CPPC_PERF_PRIO is present.

Compare these independent expectations against the live driver's attr
array, catching bugs such as attributes leaking into wrong modes or
visibility functions checking incorrect conditions.

Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate-ut.c | 139 ++++++++++++++++++++++++++++++--
 drivers/cpufreq/amd-pstate.c    |   8 ++
 drivers/cpufreq/amd-pstate.h    |   4 +
 3 files changed, 146 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index 3dcdf56883a6..1f62ab6438b4 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -23,6 +23,8 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/bitfield.h>
+#include <linux/cpufeature.h>
+#include <linux/cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
@@ -53,13 +55,15 @@ static int amd_pstate_ut_check_enabled(u32 index);
 static int amd_pstate_ut_check_perf(u32 index);
 static int amd_pstate_ut_check_freq(u32 index);
 static int amd_pstate_ut_check_driver(u32 index);
+static int amd_pstate_ut_check_freq_attrs(u32 index);
 
 static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
-	{"amd_pstate_ut_acpi_cpc_valid",   amd_pstate_ut_acpi_cpc_valid   },
-	{"amd_pstate_ut_check_enabled",    amd_pstate_ut_check_enabled    },
-	{"amd_pstate_ut_check_perf",       amd_pstate_ut_check_perf       },
-	{"amd_pstate_ut_check_freq",       amd_pstate_ut_check_freq       },
-	{"amd_pstate_ut_check_driver",	   amd_pstate_ut_check_driver     }
+	{"amd_pstate_ut_acpi_cpc_valid",    amd_pstate_ut_acpi_cpc_valid   },
+	{"amd_pstate_ut_check_enabled",     amd_pstate_ut_check_enabled    },
+	{"amd_pstate_ut_check_perf",        amd_pstate_ut_check_perf       },
+	{"amd_pstate_ut_check_freq",        amd_pstate_ut_check_freq       },
+	{"amd_pstate_ut_check_driver",      amd_pstate_ut_check_driver     },
+	{"amd_pstate_ut_check_freq_attrs",  amd_pstate_ut_check_freq_attrs },
 };
 
 static bool test_in_list(const char *list, const char *name)
@@ -293,6 +297,131 @@ static int amd_pstate_ut_check_driver(u32 index)
 	return ret;
 }
 
+enum attr_category {
+	ATTR_ALWAYS,
+	ATTR_PREFCORE,
+	ATTR_EPP,
+	ATTR_FLOOR_FREQ,
+};
+
+static const struct {
+	const char	*name;
+	enum attr_category category;
+} expected_freq_attrs[] = {
+	{"amd_pstate_max_freq",				ATTR_ALWAYS},
+	{"amd_pstate_lowest_nonlinear_freq",		ATTR_ALWAYS},
+	{"amd_pstate_highest_perf",			ATTR_ALWAYS},
+	{"amd_pstate_prefcore_ranking",			ATTR_PREFCORE},
+	{"amd_pstate_hw_prefcore",			ATTR_PREFCORE},
+	{"energy_performance_preference",		ATTR_EPP},
+	{"energy_performance_available_preferences",	ATTR_EPP},
+	{"amd_pstate_floor_freq",			ATTR_FLOOR_FREQ},
+	{"amd_pstate_floor_count",			ATTR_FLOOR_FREQ},
+};
+
+static bool attr_in_driver(struct freq_attr **driver_attrs, const char *name)
+{
+	int j;
+
+	for (j = 0; driver_attrs[j]; j++) {
+		if (!strcmp(driver_attrs[j]->attr.name, name))
+			return true;
+	}
+	return false;
+}
+
+/*
+ * Verify that for each mode the driver's live ->attr array contains exactly
+ * the attributes that should be visible.  Expected visibility is derived
+ * independently from hw_prefcore, cpu features, and the current mode —
+ * not from the driver's own visibility functions.
+ */
+static int amd_pstate_ut_check_freq_attrs(u32 index)
+{
+	enum amd_pstate_mode orig_mode = amd_pstate_get_status();
+	static const enum amd_pstate_mode modes[] = {
+		AMD_PSTATE_PASSIVE, AMD_PSTATE_ACTIVE, AMD_PSTATE_GUIDED,
+	};
+	bool has_prefcore, has_floor_freq;
+	int m, i, ret;
+
+	has_floor_freq = cpu_feature_enabled(X86_FEATURE_CPPC_PERF_PRIO);
+
+	/*
+	 * Determine prefcore support from any online CPU's cpudata.
+	 * hw_prefcore reflects the platform-wide decision made at init.
+	 */
+	has_prefcore = false;
+	for_each_online_cpu(i) {
+		struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
+		struct amd_cpudata *cpudata;
+
+		policy = cpufreq_cpu_get(i);
+		if (!policy)
+			continue;
+		cpudata = policy->driver_data;
+		has_prefcore = cpudata->hw_prefcore;
+		break;
+	}
+
+	for (m = 0; m < ARRAY_SIZE(modes); m++) {
+		struct freq_attr **driver_attrs;
+
+		ret = amd_pstate_set_mode(modes[m]);
+		if (ret)
+			goto out;
+
+		driver_attrs = amd_pstate_get_current_attrs();
+		if (!driver_attrs) {
+			pr_err("%s: no driver attrs in mode %s\n",
+			       __func__, amd_pstate_get_mode_string(modes[m]));
+			ret = -EINVAL;
+			goto out;
+		}
+
+		for (i = 0; i < ARRAY_SIZE(expected_freq_attrs); i++) {
+			bool expected, found;
+
+			switch (expected_freq_attrs[i].category) {
+			case ATTR_ALWAYS:
+				expected = true;
+				break;
+			case ATTR_PREFCORE:
+				expected = has_prefcore;
+				break;
+			case ATTR_EPP:
+				expected = (modes[m] == AMD_PSTATE_ACTIVE);
+				break;
+			case ATTR_FLOOR_FREQ:
+				expected = has_floor_freq;
+				break;
+			default:
+				expected = false;
+				break;
+			}
+
+			found = attr_in_driver(driver_attrs,
+					       expected_freq_attrs[i].name);
+
+			if (expected != found) {
+				pr_err("%s: mode %s: attr %s expected %s but is %s\n",
+				       __func__,
+				       amd_pstate_get_mode_string(modes[m]),
+				       expected_freq_attrs[i].name,
+				       expected ? "visible" : "hidden",
+				       found ? "visible" : "hidden");
+				ret = -EINVAL;
+				goto out;
+			}
+		}
+	}
+
+	ret = 0;
+out:
+	amd_pstate_set_mode(orig_mode);
+	return ret;
+}
+
 static int __init amd_pstate_ut_init(void)
 {
 	u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 5eae74a67aeb..ed9fd4155a25 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1390,6 +1390,14 @@ static struct freq_attr_visibility amd_pstate_attr_visibility[] = {
 	{&amd_pstate_floor_count, floor_freq_visibility},
 };
 
+struct freq_attr **amd_pstate_get_current_attrs(void)
+{
+	if (!current_pstate_driver)
+		return NULL;
+	return current_pstate_driver->attr;
+}
+EXPORT_SYMBOL_GPL(amd_pstate_get_current_attrs);
+
 static struct freq_attr **get_freq_attrs(void)
 {
 	bool attr_visible[ARRAY_SIZE(amd_pstate_attr_visibility)];
diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
index 453adfb445f8..faead0b19a8a 100644
--- a/drivers/cpufreq/amd-pstate.h
+++ b/drivers/cpufreq/amd-pstate.h
@@ -134,4 +134,8 @@ const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode);
 int amd_pstate_get_status(void);
 int amd_pstate_update_status(const char *buf, size_t size);
 
+struct freq_attr;
+
+struct freq_attr **amd_pstate_get_current_attrs(void);
+
 #endif /* _LINUX_AMD_PSTATE_H */
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 08/12] amd-pstate-ut: Add module parameter to select testcases
From: Gautham R. Shenoy @ 2026-03-26 11:47 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Viresh Kumar,
	K Prateek Nayak
  Cc: linux-kernel, linux-pm, Gautham R. Shenoy
In-Reply-To: <20260326114756.20374-1-gautham.shenoy@amd.com>

Currently when amd-pstate-ut test module is loaded, it runs all the
tests from amd_pstate_ut_cases[] array.

Add a module parameter named "test_list" that accepts a
comma-delimited list of test names, allowing users to run a
selected subset of tests. When the parameter is omitted or empty, all
tests are run as before.

Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate-ut.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index 447b9aa5ce40..3dcdf56883a6 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -35,6 +35,10 @@
 
 #include "amd-pstate.h"
 
+static char *test_list;
+module_param(test_list, charp, 0444);
+MODULE_PARM_DESC(test_list,
+	"Comma-delimited list of tests to run (empty means run all tests)");
 
 struct amd_pstate_ut_struct {
 	const char *name;
@@ -58,6 +62,25 @@ static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
 	{"amd_pstate_ut_check_driver",	   amd_pstate_ut_check_driver     }
 };
 
+static bool test_in_list(const char *list, const char *name)
+{
+	size_t name_len = strlen(name);
+	const char *p = list;
+
+	while (*p) {
+		const char *sep = strchr(p, ',');
+		size_t token_len = sep ? sep - p : strlen(p);
+
+		if (token_len == name_len && !strncmp(p, name, token_len))
+			return true;
+		if (!sep)
+			break;
+		p = sep + 1;
+	}
+
+	return false;
+}
+
 static bool get_shared_mem(void)
 {
 	bool result = false;
@@ -275,7 +298,13 @@ static int __init amd_pstate_ut_init(void)
 	u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
 
 	for (i = 0; i < arr_size; i++) {
-		int ret = amd_pstate_ut_cases[i].func(i);
+		int ret;
+
+		if (test_list && *test_list &&
+		    !test_in_list(test_list, amd_pstate_ut_cases[i].name))
+			continue;
+
+		ret = amd_pstate_ut_cases[i].func(i);
 
 		if (ret)
 			pr_err("%-4d %-20s\t fail: %d!\n", i+1, amd_pstate_ut_cases[i].name, ret);
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 07/12] amd-pstate: Introduce a tracepoint trace_amd_pstate_cppc_req2()
From: Gautham R. Shenoy @ 2026-03-26 11:47 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Viresh Kumar,
	K Prateek Nayak
  Cc: linux-kernel, linux-pm, Gautham R. Shenoy
In-Reply-To: <20260326114756.20374-1-gautham.shenoy@amd.com>

Introduce a new tracepoint trace_amd_pstate_cppc_req2() to track
updates to MSR_AMD_CPPC_REQ2.

Invoke this while changing the Floor Perf.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate-trace.h | 35 ++++++++++++++++++++++++++++++
 drivers/cpufreq/amd-pstate.c       | 14 +++++++++---
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate-trace.h b/drivers/cpufreq/amd-pstate-trace.h
index 32e1bdc588c5..91fa073b2be4 100644
--- a/drivers/cpufreq/amd-pstate-trace.h
+++ b/drivers/cpufreq/amd-pstate-trace.h
@@ -133,6 +133,41 @@ TRACE_EVENT(amd_pstate_epp_perf,
 		 )
 );
 
+TRACE_EVENT(amd_pstate_cppc_req2,
+
+	TP_PROTO(unsigned int cpu_id,
+		 u8 floor_perf,
+		 bool changed,
+		 int err_code
+		 ),
+
+	TP_ARGS(cpu_id,
+		floor_perf,
+		changed,
+		err_code),
+
+	TP_STRUCT__entry(
+		__field(unsigned int, cpu_id)
+		__field(u8, floor_perf)
+		__field(bool, changed)
+		__field(int, err_code)
+		),
+
+	TP_fast_assign(
+		__entry->cpu_id = cpu_id;
+		__entry->floor_perf = floor_perf;
+		__entry->changed = changed;
+		__entry->err_code = err_code;
+		),
+
+	TP_printk("cpu%u: floor_perf=%u, changed=%u (error = %d)",
+		  __entry->cpu_id,
+		  __entry->floor_perf,
+		  __entry->changed,
+		  __entry->err_code
+		 )
+);
+
 #endif /* _AMD_PSTATE_TRACE_H */
 
 /* This part must be outside protection */
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index a068c4457a8f..5eae74a67aeb 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -333,6 +333,7 @@ static int amd_pstate_set_floor_perf(struct cpufreq_policy *policy, u8 perf)
 {
 	struct amd_cpudata *cpudata = policy->driver_data;
 	u64 value, prev;
+	bool changed;
 	int ret;
 
 	if (!cpu_feature_enabled(X86_FEATURE_CPPC_PERF_PRIO))
@@ -341,17 +342,24 @@ static int amd_pstate_set_floor_perf(struct cpufreq_policy *policy, u8 perf)
 	value = prev = READ_ONCE(cpudata->cppc_req2_cached);
 	FIELD_MODIFY(AMD_CPPC_FLOOR_PERF_MASK, &value, perf);
 
-	if (value == prev)
-		return 0;
+	changed = value != prev;
+	if (!changed) {
+		ret = 0;
+		goto out_trace;
+	}
 
 	ret = wrmsrq_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ2, value);
 	if (ret) {
+		changed = false;
 		pr_err("failed to set CPPC REQ2 value. Error (%d)\n", ret);
-		return ret;
+		goto out_trace;
 	}
 
 	WRITE_ONCE(cpudata->cppc_req2_cached, value);
 
+out_trace:
+	if (trace_amd_pstate_cppc_req2_enabled())
+		trace_amd_pstate_cppc_req2(cpudata->cpu, perf, changed, ret);
 	return ret;
 }
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 06/12] amd-pstate: Add sysfs support for floor_freq and floor_count
From: Gautham R. Shenoy @ 2026-03-26 11:47 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Viresh Kumar,
	K Prateek Nayak
  Cc: linux-kernel, linux-pm, Gautham R. Shenoy, Mario Limonciello
In-Reply-To: <20260326114756.20374-1-gautham.shenoy@amd.com>

When Floor Performance feature is supported by the platform, expose
two sysfs files:

   * amd_pstate_floor_freq to allow userspace to request the floor
     frequency for each CPU.

   * amd_pstate_floor_count which advertises the number of distinct
     levels of floor frequencies supported on this platform.

Reset the floor_perf to bios_floor_perf in the suspend, offline, and
exit paths, and restore the value to the cached user-request
floor_freq on the resume and online paths mirroring how bios_min_perf
is handled for MSR_AMD_CPPC_REQ.

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 93 +++++++++++++++++++++++++++++++++---
 drivers/cpufreq/amd-pstate.h |  2 +
 2 files changed, 89 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 53b8173ff183..a068c4457a8f 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -383,8 +383,10 @@ static int amd_pstate_init_floor_perf(struct cpufreq_policy *policy)
 			return ret;
 	}
 
-	cpudata->bios_floor_perf = floor_perf;
 
+	cpudata->bios_floor_perf = floor_perf;
+	cpudata->floor_freq = perf_to_freq(cpudata->perf, cpudata->nominal_freq,
+					   floor_perf);
 	return 0;
 }
 
@@ -1288,6 +1290,46 @@ static ssize_t show_energy_performance_preference(
 	return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
 }
 
+static ssize_t store_amd_pstate_floor_freq(struct cpufreq_policy *policy,
+					   const char *buf, size_t count)
+{
+	struct amd_cpudata *cpudata = policy->driver_data;
+	union perf_cached perf = READ_ONCE(cpudata->perf);
+	unsigned int freq;
+	u8 floor_perf;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &freq);
+	if (ret)
+		return ret;
+
+	if (freq < policy->cpuinfo.min_freq || freq > policy->max)
+		return -EINVAL;
+
+	floor_perf = freq_to_perf(perf, cpudata->nominal_freq, freq);
+	ret = amd_pstate_set_floor_perf(policy, floor_perf);
+
+	if (!ret)
+		cpudata->floor_freq = freq;
+
+	return ret ?: count;
+}
+
+static ssize_t show_amd_pstate_floor_freq(struct cpufreq_policy *policy, char *buf)
+{
+	struct amd_cpudata *cpudata = policy->driver_data;
+
+	return sysfs_emit(buf, "%u\n", cpudata->floor_freq);
+}
+
+static ssize_t show_amd_pstate_floor_count(struct cpufreq_policy *policy, char *buf)
+{
+	struct amd_cpudata *cpudata = policy->driver_data;
+	u8 count = cpudata->floor_perf_cnt;
+
+	return sysfs_emit(buf, "%u\n", count);
+}
+
 cpufreq_freq_attr_ro(amd_pstate_max_freq);
 cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
 
@@ -1296,6 +1338,8 @@ cpufreq_freq_attr_ro(amd_pstate_prefcore_ranking);
 cpufreq_freq_attr_ro(amd_pstate_hw_prefcore);
 cpufreq_freq_attr_rw(energy_performance_preference);
 cpufreq_freq_attr_ro(energy_performance_available_preferences);
+cpufreq_freq_attr_rw(amd_pstate_floor_freq);
+cpufreq_freq_attr_ro(amd_pstate_floor_count);
 
 struct freq_attr_visibility {
 	struct freq_attr *attr;
@@ -1320,6 +1364,12 @@ static bool epp_visibility(void)
 	return cppc_state == AMD_PSTATE_ACTIVE;
 }
 
+/* Determines whether amd_pstate_floor_freq related attributes should be visible */
+static bool floor_freq_visibility(void)
+{
+	return cpu_feature_enabled(X86_FEATURE_CPPC_PERF_PRIO);
+}
+
 static struct freq_attr_visibility amd_pstate_attr_visibility[] = {
 	{&amd_pstate_max_freq, always_visible},
 	{&amd_pstate_lowest_nonlinear_freq, always_visible},
@@ -1328,6 +1378,8 @@ static struct freq_attr_visibility amd_pstate_attr_visibility[] = {
 	{&amd_pstate_hw_prefcore, prefcore_visibility},
 	{&energy_performance_preference, epp_visibility},
 	{&energy_performance_available_preferences, epp_visibility},
+	{&amd_pstate_floor_freq, floor_freq_visibility},
+	{&amd_pstate_floor_count, floor_freq_visibility},
 };
 
 static struct freq_attr **get_freq_attrs(void)
@@ -1748,24 +1800,39 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
 
 static int amd_pstate_cpu_online(struct cpufreq_policy *policy)
 {
-	return amd_pstate_cppc_enable(policy);
+	struct amd_cpudata *cpudata = policy->driver_data;
+	union perf_cached perf = READ_ONCE(cpudata->perf);
+	u8 cached_floor_perf;
+	int ret;
+
+	ret = amd_pstate_cppc_enable(policy);
+	if (ret)
+		return ret;
+
+	cached_floor_perf = freq_to_perf(perf, cpudata->nominal_freq, cpudata->floor_freq);
+	return amd_pstate_set_floor_perf(policy, cached_floor_perf);
 }
 
 static int amd_pstate_cpu_offline(struct cpufreq_policy *policy)
 {
 	struct amd_cpudata *cpudata = policy->driver_data;
 	union perf_cached perf = READ_ONCE(cpudata->perf);
+	int ret;
 
 	/*
 	 * Reset CPPC_REQ MSR to the BIOS value, this will allow us to retain the BIOS specified
 	 * min_perf value across kexec reboots. If this CPU is just onlined normally after this, the
 	 * limits, epp and desired perf will get reset to the cached values in cpudata struct
 	 */
-	return amd_pstate_update_perf(policy, perf.bios_min_perf,
+	ret = amd_pstate_update_perf(policy, perf.bios_min_perf,
 				     FIELD_GET(AMD_CPPC_DES_PERF_MASK, cpudata->cppc_req_cached),
 				     FIELD_GET(AMD_CPPC_MAX_PERF_MASK, cpudata->cppc_req_cached),
 				     FIELD_GET(AMD_CPPC_EPP_PERF_MASK, cpudata->cppc_req_cached),
 				     false);
+	if (ret)
+		return ret;
+
+	return amd_pstate_set_floor_perf(policy, cpudata->bios_floor_perf);
 }
 
 static int amd_pstate_suspend(struct cpufreq_policy *policy)
@@ -1787,6 +1854,10 @@ static int amd_pstate_suspend(struct cpufreq_policy *policy)
 	if (ret)
 		return ret;
 
+	ret = amd_pstate_set_floor_perf(policy, cpudata->bios_floor_perf);
+	if (ret)
+		return ret;
+
 	/* set this flag to avoid setting core offline*/
 	cpudata->suspended = true;
 
@@ -1798,15 +1869,24 @@ static int amd_pstate_resume(struct cpufreq_policy *policy)
 	struct amd_cpudata *cpudata = policy->driver_data;
 	union perf_cached perf = READ_ONCE(cpudata->perf);
 	int cur_perf = freq_to_perf(perf, cpudata->nominal_freq, policy->cur);
+	u8 cached_floor_perf;
+	int ret;
 
 	/* Set CPPC_REQ to last sane value until the governor updates it */
-	return amd_pstate_update_perf(policy, perf.min_limit_perf, cur_perf, perf.max_limit_perf,
-				      0U, false);
+	ret = amd_pstate_update_perf(policy, perf.min_limit_perf, cur_perf, perf.max_limit_perf,
+				     0U, false);
+	if (ret)
+		return ret;
+
+	cached_floor_perf = freq_to_perf(perf, cpudata->nominal_freq, cpudata->floor_freq);
+	return amd_pstate_set_floor_perf(policy, cached_floor_perf);
 }
 
 static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
 {
 	struct amd_cpudata *cpudata = policy->driver_data;
+	union perf_cached perf = READ_ONCE(cpudata->perf);
+	u8 cached_floor_perf;
 
 	if (cpudata->suspended) {
 		int ret;
@@ -1819,7 +1899,8 @@ static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
 		cpudata->suspended = false;
 	}
 
-	return 0;
+	cached_floor_perf = freq_to_perf(perf, cpudata->nominal_freq, cpudata->floor_freq);
+	return amd_pstate_set_floor_perf(policy, cached_floor_perf);
 }
 
 static struct cpufreq_driver amd_pstate_driver = {
diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
index 303da70b0afa..453adfb445f8 100644
--- a/drivers/cpufreq/amd-pstate.h
+++ b/drivers/cpufreq/amd-pstate.h
@@ -74,6 +74,7 @@ struct amd_aperf_mperf {
  * @max_limit_freq: Cached value of policy->max (in khz)
  * @nominal_freq: the frequency (in khz) that mapped to nominal_perf
  * @lowest_nonlinear_freq: the frequency (in khz) that mapped to lowest_nonlinear_perf
+ * @floor_freq: Cached value of the user requested floor_freq
  * @cur: Difference of Aperf/Mperf/tsc count between last and current sample
  * @prev: Last Aperf/Mperf/tsc count value read from register
  * @freq: current cpu frequency value (in khz)
@@ -103,6 +104,7 @@ struct amd_cpudata {
 	u32	max_limit_freq;
 	u32	nominal_freq;
 	u32	lowest_nonlinear_freq;
+	u32	floor_freq;
 
 	struct amd_aperf_mperf cur;
 	struct amd_aperf_mperf prev;
-- 
2.34.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox