From 886a94f008dd1a1702ee66dd035c266f70fd9e90 Mon Sep 17 00:00:00 2001 From: Xilin Wu Date: Fri, 13 Jun 2025 22:53:38 +0800 Subject: [PATCH 1/5] interconnect: qcom: sc7280: Add missing num_links to xm_pcie3_1 node This allows adding interconnect paths for PCIe 1 in device tree later. Fixes: 46bdcac533cc ("interconnect: qcom: Add SC7280 interconnect provider driver") Signed-off-by: Xilin Wu Reviewed-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20250613-sc7280-icc-pcie1-fix-v1-1-0b09813e3b09@radxa.com Signed-off-by: Georgi Djakov --- drivers/interconnect/qcom/sc7280.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/interconnect/qcom/sc7280.c b/drivers/interconnect/qcom/sc7280.c index 346f18d70e9e..905403a3a930 100644 --- a/drivers/interconnect/qcom/sc7280.c +++ b/drivers/interconnect/qcom/sc7280.c @@ -238,6 +238,7 @@ static struct qcom_icc_node xm_pcie3_1 = { .id = SC7280_MASTER_PCIE_1, .channels = 1, .buswidth = 8, + .num_links = 1, .links = { SC7280_SLAVE_ANOC_PCIE_GEM_NOC }, }; From b44f12ae21f661e5d97fb4a8b234d6689f68f706 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 23 Jun 2025 16:24:37 +0200 Subject: [PATCH 2/5] interconnect: exynos: handle node name allocation failure Add the missing error handling in case node name allocation ever fails. Fixes: 2f95b9d5cf0b ("interconnect: Add generic interconnect driver for Exynos SoCs") Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20250623142437.23068-1-johan+linaro@kernel.org Signed-off-by: Georgi Djakov --- drivers/interconnect/samsung/exynos.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/interconnect/samsung/exynos.c b/drivers/interconnect/samsung/exynos.c index 9e041365d909..8e8f56186a36 100644 --- a/drivers/interconnect/samsung/exynos.c +++ b/drivers/interconnect/samsung/exynos.c @@ -134,6 +134,11 @@ static int exynos_generic_icc_probe(struct platform_device *pdev) priv->node = icc_node; icc_node->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%pOFn", bus_dev->of_node); + if (!icc_node->name) { + icc_node_destroy(pdev->id); + return -ENOMEM; + } + if (of_property_read_u32(bus_dev->of_node, "samsung,data-clock-ratio", &priv->bus_clk_ratio)) priv->bus_clk_ratio = EXYNOS_ICC_DEFAULT_BUS_CLK_RATIO; From 1809db75bd1fa71e27e199e4c5cc9072741f6052 Mon Sep 17 00:00:00 2001 From: Gabor Juhos Date: Wed, 25 Jun 2025 15:34:23 +0200 Subject: [PATCH 3/5] interconnect: increase ICC_DYN_ID_START Since commit d30f83d278a9 ("interconnect: core: Add dynamic id allocation support"), interconnect node ids greater than or equal to ICC_DYN_ID_START are reserved for dynamic id allocation. Yet the icc_node_create_nolock() function allows to directly use such ids for creating nodes. This can cause problems by executing dynamic id related codepaths even for nodes intended to use static ids. For example, the 'nsscc-ipq9574' driver creates interconnect nodes with static ids starting from 19148. Because these ids belongs to the dynamic id range, the icc_node_add() function replaces the node names unexpectedly. The node names looked like this before the change: # grep nss_cc /sys/kernel/debug/interconnect/interconnect_summary nss_cc_nssnoc_ppe_clk_master 0 0 nss_cc_nssnoc_ppe_clk_slave 0 0 nss_cc_nssnoc_ppe_cfg_clk_master 0 0 ... And those have an unexpected suffix now: # grep nss_cc /sys/kernel/debug/interconnect/interconnect_summary nss_cc_nssnoc_ppe_clk_master@39b00000.clock-controller 0 0 nss_cc_nssnoc_ppe_clk_slave@39b00000.clock-controller 0 0 nss_cc_nssnoc_ppe_cfg_clk_master@39b00000.clock-controller 0 0 ... Increase the value of ICC_DYN_ID_START to avoid this. Also, add sanity check to the icc_node_create_nolock() function to prevent directly creating nodes with ids reserved for dynamic allocation in order to detect these kind of problems. Fixes: d30f83d278a9 ("interconnect: core: Add dynamic id allocation support") Signed-off-by: Gabor Juhos Reviewed-by: Dmitry Baryshkov Link: https://lore.kernel.org/r/20250625-icc-dyn-id-fix-v1-1-127cb5498449@gmail.com Signed-off-by: Georgi Djakov --- drivers/interconnect/core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 1a41e59c77f8..3a41b2717edd 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -20,7 +20,7 @@ #include "internal.h" -#define ICC_DYN_ID_START 10000 +#define ICC_DYN_ID_START 100000 #define CREATE_TRACE_POINTS #include "trace.h" @@ -819,6 +819,9 @@ static struct icc_node *icc_node_create_nolock(int id) { struct icc_node *node; + if (id >= ICC_DYN_ID_START) + return ERR_PTR(-EINVAL); + /* check if node already exists */ node = node_find(id); if (node) From 618c810a7b2163517ab1875bd56b633ca3cb3328 Mon Sep 17 00:00:00 2001 From: Gabor Juhos Date: Wed, 25 Jun 2025 19:32:35 +0200 Subject: [PATCH 4/5] interconnect: icc-clk: destroy nodes in case of memory allocation failures When memory allocation fails during creating the name of the nodes in icc_clk_register(), the code continues on the error path and it calls icc_nodes_remove() to destroy the already created nodes. However that function only destroys the nodes which were already added to the provider and the newly created nodes are never destroyed in case of error. In order to avoid a memory leaks, change the code to destroy the newly created nodes explicitly in case of memory allocation failures. Fixes: 44c5aa73ccd1 ("interconnect: icc-clk: check return values of devm_kasprintf()") Signed-off-by: Gabor Juhos Reviewed-by: Bartosz Golaszewski Link: https://lore.kernel.org/r/20250625-icc-clk-memleak-fix-v1-1-4151484cd24f@gmail.com Signed-off-by: Georgi Djakov --- drivers/interconnect/icc-clk.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c index 88f311c11020..93c030608d3e 100644 --- a/drivers/interconnect/icc-clk.c +++ b/drivers/interconnect/icc-clk.c @@ -117,6 +117,7 @@ struct icc_provider *icc_clk_register(struct device *dev, node->name = devm_kasprintf(dev, GFP_KERNEL, "%s_master", data[i].name); if (!node->name) { + icc_node_destroy(node->id); ret = -ENOMEM; goto err; } @@ -135,6 +136,7 @@ struct icc_provider *icc_clk_register(struct device *dev, node->name = devm_kasprintf(dev, GFP_KERNEL, "%s_slave", data[i].name); if (!node->name) { + icc_node_destroy(node->id); ret = -ENOMEM; goto err; } From c5b60592886f97b01503c1bb553f88d6a7df42ea Mon Sep 17 00:00:00 2001 From: Gabor Juhos Date: Fri, 27 Jun 2025 09:58:54 +0200 Subject: [PATCH 5/5] interconnect: avoid memory allocation when 'icc_bw_lock' is held The 'icc_bw_lock' mutex is introduced in commit af42269c3523 ("interconnect: Fix locking for runpm vs reclaim") in order to decouple serialization of bw aggregation from codepaths that require memory allocation. However commit d30f83d278a9 ("interconnect: core: Add dynamic id allocation support") added a devm_kasprintf() call into a path protected by the 'icc_bw_lock' which causes the following lockdep warning on machines like the Lenovo ThinkPad X13s: ====================================================== WARNING: possible circular locking dependency detected 6.16.0-rc3 #15 Not tainted ------------------------------------------------------ (udev-worker)/342 is trying to acquire lock: ffffb973f7ec4638 (fs_reclaim){+.+.}-{0:0}, at: __kmalloc_node_track_caller_noprof+0xa0/0x3e0 but task is already holding lock: ffffb973f7f7f0e8 (icc_bw_lock){+.+.}-{4:4}, at: icc_node_add+0x44/0x154 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (icc_bw_lock){+.+.}-{4:4}: icc_init+0x48/0x108 do_one_initcall+0x64/0x30c kernel_init_freeable+0x27c/0x500 kernel_init+0x20/0x1d8 ret_from_fork+0x10/0x20 -> #0 (fs_reclaim){+.+.}-{0:0}: __lock_acquire+0x136c/0x2114 lock_acquire+0x1c8/0x354 fs_reclaim_acquire+0x74/0xa8 __kmalloc_node_track_caller_noprof+0xa0/0x3e0 devm_kmalloc+0x54/0x124 devm_kvasprintf+0x74/0xd4 devm_kasprintf+0x58/0x80 icc_node_add+0xb4/0x154 qcom_osm_l3_probe+0x20c/0x314 [icc_osm_l3] platform_probe+0x68/0xd8 really_probe+0xc0/0x38c __driver_probe_device+0x7c/0x160 driver_probe_device+0x40/0x110 __driver_attach+0xfc/0x208 bus_for_each_dev+0x74/0xd0 driver_attach+0x24/0x30 bus_add_driver+0x110/0x234 driver_register+0x60/0x128 __platform_driver_register+0x24/0x30 osm_l3_driver_init+0x20/0x1000 [icc_osm_l3] do_one_initcall+0x64/0x30c do_init_module+0x58/0x23c load_module+0x1df8/0x1f70 init_module_from_file+0x88/0xc4 idempotent_init_module+0x188/0x280 __arm64_sys_finit_module+0x6c/0xd8 invoke_syscall+0x48/0x110 el0_svc_common.constprop.0+0xc0/0xe0 do_el0_svc+0x1c/0x28 el0_svc+0x4c/0x158 el0t_64_sync_handler+0xc8/0xcc el0t_64_sync+0x198/0x19c other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(icc_bw_lock); lock(fs_reclaim); lock(icc_bw_lock); lock(fs_reclaim); *** DEADLOCK *** The icc_node_add() functions is not designed to fail, and as such it should not do any memory allocation. In order to avoid this, add a new helper function for the name generation to be called by drivers which are using the new dynamic id feature. Fixes: d30f83d278a9 ("interconnect: core: Add dynamic id allocation support") Signed-off-by: Gabor Juhos Link: https://lore.kernel.org/r/20250625-icc-bw-lockdep-v3-1-2b8f8b8987c4@gmail.com Co-developed-by: Johan Hovold Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20250627075854.26943-1-johan+linaro@kernel.org Signed-off-by: Georgi Djakov --- drivers/interconnect/core.c | 29 +++++++++++++++++++++++---- drivers/interconnect/qcom/icc-rpmh.c | 7 ++++++- drivers/interconnect/qcom/osm-l3.c | 7 ++++++- include/linux/interconnect-provider.h | 7 +++++++ 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 3a41b2717edd..3ebf37ddfc18 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -909,10 +909,35 @@ void icc_node_destroy(int id) return; kfree(node->links); + if (node->id >= ICC_DYN_ID_START) + kfree(node->name); kfree(node); } EXPORT_SYMBOL_GPL(icc_node_destroy); +/** + * icc_node_set_name() - set node name + * @node: node + * @provider: node provider + * @name: node name + * + * Return: 0 on success, or -ENOMEM on allocation failure + */ +int icc_node_set_name(struct icc_node *node, const struct icc_provider *provider, const char *name) +{ + if (node->id >= ICC_DYN_ID_START) { + node->name = kasprintf(GFP_KERNEL, "%s@%s", name, + dev_name(provider->dev)); + if (!node->name) + return -ENOMEM; + } else { + node->name = name; + } + + return 0; +} +EXPORT_SYMBOL_GPL(icc_node_set_name); + /** * icc_link_nodes() - create link between two nodes * @src_node: source node @@ -1041,10 +1066,6 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider) node->avg_bw = node->init_avg; node->peak_bw = node->init_peak; - if (node->id >= ICC_DYN_ID_START) - node->name = devm_kasprintf(provider->dev, GFP_KERNEL, "%s@%s", - node->name, dev_name(provider->dev)); - if (node->avg_bw || node->peak_bw) { if (provider->pre_aggregate) provider->pre_aggregate(node); diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c index 41bfc6e7ee1d..001404e91041 100644 --- a/drivers/interconnect/qcom/icc-rpmh.c +++ b/drivers/interconnect/qcom/icc-rpmh.c @@ -293,7 +293,12 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev) goto err_remove_nodes; } - node->name = qn->name; + ret = icc_node_set_name(node, provider, qn->name); + if (ret) { + icc_node_destroy(node->id); + goto err_remove_nodes; + } + node->data = qn; icc_node_add(node, provider); diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c index baecbf2533f7..b33f00da1880 100644 --- a/drivers/interconnect/qcom/osm-l3.c +++ b/drivers/interconnect/qcom/osm-l3.c @@ -236,7 +236,12 @@ static int qcom_osm_l3_probe(struct platform_device *pdev) goto err; } - node->name = qnodes[i]->name; + ret = icc_node_set_name(node, provider, qnodes[i]->name); + if (ret) { + icc_node_destroy(node->id); + goto err; + } + /* Cast away const and add it back in qcom_osm_l3_set() */ node->data = (void *)qnodes[i]; icc_node_add(node, provider); diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h index 55cfebc658e6..8a2f652a05ff 100644 --- a/include/linux/interconnect-provider.h +++ b/include/linux/interconnect-provider.h @@ -119,6 +119,7 @@ int icc_std_aggregate(struct icc_node *node, u32 tag, u32 avg_bw, struct icc_node *icc_node_create_dyn(void); struct icc_node *icc_node_create(int id); void icc_node_destroy(int id); +int icc_node_set_name(struct icc_node *node, const struct icc_provider *provider, const char *name); int icc_link_nodes(struct icc_node *src_node, struct icc_node **dst_node); int icc_link_create(struct icc_node *node, const int dst_id); void icc_node_add(struct icc_node *node, struct icc_provider *provider); @@ -152,6 +153,12 @@ static inline void icc_node_destroy(int id) { } +static inline int icc_node_set_name(struct icc_node *node, const struct icc_provider *provider, + const char *name) +{ + return -EOPNOTSUPP; +} + static inline int icc_link_nodes(struct icc_node *src_node, struct icc_node **dst_node) { return -EOPNOTSUPP;