From b89c0ed09e1189217cd9d516b739627c523d53a4 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Tue, 19 Nov 2024 18:56:36 +0100 Subject: [PATCH 1/6] opp: core: implement dev_pm_opp_get_bw Add and implement dev_pm_opp_get_bw() to retrieve the OPP's bandwidth in the same way as the dev_pm_opp_get_voltage() helper. Retrieving bandwidth is required in the case of the Adreno GPU where the GPU Management Unit can handle the Bandwidth scaling. The helper can get the peak or average bandwidth for any of the interconnect path. Signed-off-by: Neil Armstrong [ Viresh: Fixed commit log and a comment in code ] Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 25 +++++++++++++++++++++++++ include/linux/pm_opp.h | 7 +++++++ 2 files changed, 32 insertions(+) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 0311b18319a45..d4a0030a0228d 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -106,6 +106,31 @@ static bool assert_single_clk(struct opp_table *opp_table) return !WARN_ON(opp_table->clk_count > 1); } +/** + * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an opp + * @opp: opp for which bandwidth has to be returned for + * @peak: select peak or average bandwidth + * @index: bandwidth index + * + * Return: bandwidth in kBps, else return 0 + */ +unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, bool peak, int index) +{ + if (IS_ERR_OR_NULL(opp)) { + pr_err("%s: Invalid parameters\n", __func__); + return 0; + } + + if (index > opp->opp_table->path_count) + return 0; + + if (!opp->bandwidth) + return 0; + + return peak ? opp->bandwidth[index].peak : opp->bandwidth[index].avg; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_get_bw); + /** * dev_pm_opp_get_voltage() - Gets the voltage corresponding to an opp * @opp: opp for which voltage has to be returned for diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 568183e3e641c..414146abfe816 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -102,6 +102,8 @@ struct dev_pm_opp_data { struct opp_table *dev_pm_opp_get_opp_table(struct device *dev); void dev_pm_opp_put_opp_table(struct opp_table *opp_table); +unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, bool peak, int index); + unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp); int dev_pm_opp_get_supplies(struct dev_pm_opp *opp, struct dev_pm_opp_supply *supplies); @@ -205,6 +207,11 @@ static inline struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device * static inline void dev_pm_opp_put_opp_table(struct opp_table *opp_table) {} +static inline unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, bool peak, int index) +{ + return 0; +} + static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp) { return 0; From 402074f0105e93154409e4a86c02d09e5199d9a5 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sat, 30 Nov 2024 13:07:56 +0300 Subject: [PATCH 2/6] opp: core: Fix off by one in dev_pm_opp_get_bw() The "opp->bandwidth" array has "opp->opp_table->path_count" number of elements. It's allocated in _opp_allocate(). So this > needs to be >= to prevent an out of bounds access. Fixes: d78653dcd8bf ("opp: core: implement dev_pm_opp_get_bw") Signed-off-by: Dan Carpenter Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index d4a0030a0228d..09a1432561f63 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -121,7 +121,7 @@ unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, bool peak, int index) return 0; } - if (index > opp->opp_table->path_count) + if (index >= opp->opp_table->path_count) return 0; if (!opp->bandwidth) From d659bc68ed489022ea33342cfbda2911a81e7a0d Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Tue, 3 Dec 2024 09:12:59 +0100 Subject: [PATCH 3/6] OPP: add index check to assert to avoid buffer overflow in _read_freq() Pass the freq index to the assert function to make sure we do not read a freq out of the opp->rates[] table when called from the indexed variants: dev_pm_opp_find_freq_exact_indexed() or dev_pm_opp_find_freq_ceil/floor_indexed(). Add a secondary parameter to the assert function, unused for assert_single_clk() then add assert_clk_index() which will check for the clock index when called from the _indexed() find functions. Fixes: 142e17c1c2b4 ("OPP: Introduce dev_pm_opp_find_freq_{ceil/floor}_indexed() APIs") Fixes: a5893928bb17 ("OPP: Add dev_pm_opp_find_freq_exact_indexed()") Signed-off-by: Neil Armstrong Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 09a1432561f63..1fee2abc0ba66 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -101,11 +101,21 @@ struct opp_table *_find_opp_table(struct device *dev) * representation in the OPP table and manage the clock configuration themselves * in an platform specific way. */ -static bool assert_single_clk(struct opp_table *opp_table) +static bool assert_single_clk(struct opp_table *opp_table, + unsigned int __always_unused index) { return !WARN_ON(opp_table->clk_count > 1); } +/* + * Returns true if clock table is large enough to contain the clock index. + */ +static bool assert_clk_index(struct opp_table *opp_table, + unsigned int index) +{ + return opp_table->clk_count > index; +} + /** * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an opp * @opp: opp for which bandwidth has to be returned for @@ -524,12 +534,12 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table, unsigned long (*read)(struct dev_pm_opp *opp, int index), bool (*compare)(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp, unsigned long opp_key, unsigned long key), - bool (*assert)(struct opp_table *opp_table)) + bool (*assert)(struct opp_table *opp_table, unsigned int index)) { struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); /* Assert that the requirement is met */ - if (assert && !assert(opp_table)) + if (assert && !assert(opp_table, index)) return ERR_PTR(-EINVAL); mutex_lock(&opp_table->lock); @@ -557,7 +567,7 @@ _find_key(struct device *dev, unsigned long *key, int index, bool available, unsigned long (*read)(struct dev_pm_opp *opp, int index), bool (*compare)(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp, unsigned long opp_key, unsigned long key), - bool (*assert)(struct opp_table *opp_table)) + bool (*assert)(struct opp_table *opp_table, unsigned int index)) { struct opp_table *opp_table; struct dev_pm_opp *opp; @@ -580,7 +590,7 @@ _find_key(struct device *dev, unsigned long *key, int index, bool available, static struct dev_pm_opp *_find_key_exact(struct device *dev, unsigned long key, int index, bool available, unsigned long (*read)(struct dev_pm_opp *opp, int index), - bool (*assert)(struct opp_table *opp_table)) + bool (*assert)(struct opp_table *opp_table, unsigned int index)) { /* * The value of key will be updated here, but will be ignored as the @@ -593,7 +603,7 @@ static struct dev_pm_opp *_find_key_exact(struct device *dev, static struct dev_pm_opp *_opp_table_find_key_ceil(struct opp_table *opp_table, unsigned long *key, int index, bool available, unsigned long (*read)(struct dev_pm_opp *opp, int index), - bool (*assert)(struct opp_table *opp_table)) + bool (*assert)(struct opp_table *opp_table, unsigned int index)) { return _opp_table_find_key(opp_table, key, index, available, read, _compare_ceil, assert); @@ -602,7 +612,7 @@ static struct dev_pm_opp *_opp_table_find_key_ceil(struct opp_table *opp_table, static struct dev_pm_opp *_find_key_ceil(struct device *dev, unsigned long *key, int index, bool available, unsigned long (*read)(struct dev_pm_opp *opp, int index), - bool (*assert)(struct opp_table *opp_table)) + bool (*assert)(struct opp_table *opp_table, unsigned int index)) { return _find_key(dev, key, index, available, read, _compare_ceil, assert); @@ -611,7 +621,7 @@ static struct dev_pm_opp *_find_key_ceil(struct device *dev, unsigned long *key, static struct dev_pm_opp *_find_key_floor(struct device *dev, unsigned long *key, int index, bool available, unsigned long (*read)(struct dev_pm_opp *opp, int index), - bool (*assert)(struct opp_table *opp_table)) + bool (*assert)(struct opp_table *opp_table, unsigned int index)) { return _find_key(dev, key, index, available, read, _compare_floor, assert); @@ -672,7 +682,8 @@ struct dev_pm_opp * dev_pm_opp_find_freq_exact_indexed(struct device *dev, unsigned long freq, u32 index, bool available) { - return _find_key_exact(dev, freq, index, available, _read_freq, NULL); + return _find_key_exact(dev, freq, index, available, _read_freq, + assert_clk_index); } EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_exact_indexed); @@ -732,7 +743,8 @@ struct dev_pm_opp * dev_pm_opp_find_freq_ceil_indexed(struct device *dev, unsigned long *freq, u32 index) { - return _find_key_ceil(dev, freq, index, true, _read_freq, NULL); + return _find_key_ceil(dev, freq, index, true, _read_freq, + assert_clk_index); } EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_ceil_indexed); @@ -785,7 +797,7 @@ struct dev_pm_opp * dev_pm_opp_find_freq_floor_indexed(struct device *dev, unsigned long *freq, u32 index) { - return _find_key_floor(dev, freq, index, true, _read_freq, NULL); + return _find_key_floor(dev, freq, index, true, _read_freq, assert_clk_index); } EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor_indexed); @@ -1727,7 +1739,7 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq) if (IS_ERR(opp_table)) return; - if (!assert_single_clk(opp_table)) + if (!assert_single_clk(opp_table, 0)) goto put_table; mutex_lock(&opp_table->lock); @@ -2079,7 +2091,7 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long tol, u_volt = data->u_volt; int ret; - if (!assert_single_clk(opp_table)) + if (!assert_single_clk(opp_table, 0)) return -EINVAL; new_opp = _opp_allocate(opp_table); @@ -2835,7 +2847,7 @@ static int _opp_set_availability(struct device *dev, unsigned long freq, return r; } - if (!assert_single_clk(opp_table)) { + if (!assert_single_clk(opp_table, 0)) { r = -EINVAL; goto put_table; } @@ -2911,7 +2923,7 @@ int dev_pm_opp_adjust_voltage(struct device *dev, unsigned long freq, return r; } - if (!assert_single_clk(opp_table)) { + if (!assert_single_clk(opp_table, 0)) { r = -EINVAL; goto put_table; } From b44b9bc7cab2967c3d6a791b1cd542c89fc07f0e Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Tue, 3 Dec 2024 09:13:00 +0100 Subject: [PATCH 4/6] OPP: fix dev_pm_opp_find_bw_*() when bandwidth table not initialized If a driver calls dev_pm_opp_find_bw_ceil/floor() the retrieve bandwidth from the OPP table but the bandwidth table was not created because the interconnect properties were missing in the OPP consumer node, the kernel will crash with: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004 ... pc : _read_bw+0x8/0x10 lr : _opp_table_find_key+0x9c/0x174 ... Call trace: _read_bw+0x8/0x10 (P) _opp_table_find_key+0x9c/0x174 (L) _find_key+0x98/0x168 dev_pm_opp_find_bw_ceil+0x50/0x88 ... In order to fix the crash, create an assert function to check if the bandwidth table was created before trying to get a bandwidth with _read_bw(). Fixes: add1dc094a74 ("OPP: Use generic key finding helpers for bandwidth key") Signed-off-by: Neil Armstrong Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 1fee2abc0ba66..be3291b537194 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -116,6 +116,15 @@ static bool assert_clk_index(struct opp_table *opp_table, return opp_table->clk_count > index; } +/* + * Returns true if bandwidth table is large enough to contain the bandwidth index. + */ +static bool assert_bandwidth_index(struct opp_table *opp_table, + unsigned int index) +{ + return opp_table->path_count > index; +} + /** * dev_pm_opp_get_bw() - Gets the bandwidth corresponding to an opp * @opp: opp for which bandwidth has to be returned for @@ -915,7 +924,8 @@ struct dev_pm_opp *dev_pm_opp_find_bw_ceil(struct device *dev, unsigned int *bw, unsigned long temp = *bw; struct dev_pm_opp *opp; - opp = _find_key_ceil(dev, &temp, index, true, _read_bw, NULL); + opp = _find_key_ceil(dev, &temp, index, true, _read_bw, + assert_bandwidth_index); *bw = temp; return opp; } @@ -946,7 +956,8 @@ struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev, unsigned long temp = *bw; struct dev_pm_opp *opp; - opp = _find_key_floor(dev, &temp, index, true, _read_bw, NULL); + opp = _find_key_floor(dev, &temp, index, true, _read_bw, + assert_bandwidth_index); *bw = temp; return opp; } From 1d38eb7f7b26261a0b642f6e0923269c7c000a97 Mon Sep 17 00:00:00 2001 From: Joe Hattori Date: Tue, 7 Jan 2025 14:44:53 +0900 Subject: [PATCH 5/6] OPP: OF: Fix an OF node leak in _opp_add_static_v2() _opp_add_static_v2() leaks the obtained OF node reference when _of_opp_alloc_required_opps() fails. Add an of_node_put() call in the error path. Fixes: 3466ea2cd6b6 ("OPP: Don't drop opp->np reference while it is still in use") Signed-off-by: Joe Hattori Signed-off-by: Viresh Kumar --- drivers/opp/of.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index fd5ed28582588..a24f76f5fd017 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -926,7 +926,7 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, ret = _of_opp_alloc_required_opps(opp_table, new_opp); if (ret) - goto free_opp; + goto put_node; if (!of_property_read_u32(np, "clock-latency-ns", &val)) new_opp->clock_latency_ns = val; @@ -976,6 +976,8 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, free_required_opps: _of_opp_free_required_opps(opp_table, new_opp); +put_node: + of_node_put(np); free_opp: _opp_free(new_opp); From b489e7946656ed67fea1a30f5103eb62a8686e04 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Tue, 9 Jan 2024 11:57:48 +0530 Subject: [PATCH 6/6] PM / OPP: Add reference counting helpers for Rust implementation To ensure that resources such as OPP tables or OPP nodes are not freed while in use by the Rust implementation, it is necessary to increment their reference count from Rust code. This commit introduces a new helper function, dev_pm_opp_get_opp_table_ref(), to increment the reference count of an OPP table and declares the existing helper dev_pm_opp_get() in pm_opp.h. Signed-off-by: Viresh Kumar --- drivers/opp/core.c | 17 ++++++++++++----- drivers/opp/opp.h | 1 - include/linux/pm_opp.h | 6 ++++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index be3291b537194..73e9a3b2f29b3 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -1528,11 +1528,6 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index) return ERR_PTR(ret); } -void _get_opp_table_kref(struct opp_table *opp_table) -{ - kref_get(&opp_table->kref); -} - static struct opp_table *_update_opp_table_clk(struct device *dev, struct opp_table *opp_table, bool getclk) @@ -1693,6 +1688,17 @@ static void _opp_table_kref_release(struct kref *kref) kfree(opp_table); } +void _get_opp_table_kref(struct opp_table *opp_table) +{ + kref_get(&opp_table->kref); +} + +void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table) +{ + _get_opp_table_kref(opp_table); +} +EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_table_ref); + void dev_pm_opp_put_opp_table(struct opp_table *opp_table) { kref_put_mutex(&opp_table->kref, _opp_table_kref_release, @@ -1727,6 +1733,7 @@ void dev_pm_opp_get(struct dev_pm_opp *opp) { kref_get(&opp->kref); } +EXPORT_SYMBOL_GPL(dev_pm_opp_get); void dev_pm_opp_put(struct dev_pm_opp *opp) { diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index 430651e7424aa..5c7c81190e41a 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -250,7 +250,6 @@ struct opp_table { }; /* Routines internal to opp core */ -void dev_pm_opp_get(struct dev_pm_opp *opp); bool _opp_remove_all_static(struct opp_table *opp_table); void _get_opp_table_kref(struct opp_table *opp_table); int _get_opp_count(struct opp_table *opp_table); diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 414146abfe816..c247317aae38a 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -100,6 +100,7 @@ struct dev_pm_opp_data { #if defined(CONFIG_PM_OPP) struct opp_table *dev_pm_opp_get_opp_table(struct device *dev); +void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table); void dev_pm_opp_put_opp_table(struct opp_table *opp_table); unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, bool peak, int index); @@ -160,6 +161,7 @@ struct dev_pm_opp *dev_pm_opp_find_bw_ceil(struct device *dev, struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev, unsigned int *bw, int index); +void dev_pm_opp_get(struct dev_pm_opp *opp); void dev_pm_opp_put(struct dev_pm_opp *opp); int dev_pm_opp_add_dynamic(struct device *dev, struct dev_pm_opp_data *opp); @@ -205,6 +207,8 @@ static inline struct opp_table *dev_pm_opp_get_opp_table_indexed(struct device * return ERR_PTR(-EOPNOTSUPP); } +static inline void dev_pm_opp_get_opp_table_ref(struct opp_table *opp_table) {} + static inline void dev_pm_opp_put_opp_table(struct opp_table *opp_table) {} static inline unsigned long dev_pm_opp_get_bw(struct dev_pm_opp *opp, bool peak, int index) @@ -341,6 +345,8 @@ static inline struct dev_pm_opp *dev_pm_opp_find_bw_floor(struct device *dev, return ERR_PTR(-EOPNOTSUPP); } +static inline void dev_pm_opp_get(struct dev_pm_opp *opp) {} + static inline void dev_pm_opp_put(struct dev_pm_opp *opp) {} static inline int