* [PATCH v2 0/3] Misc. SCM driver fixes
@ 2026-06-29 14:17 Mukesh Ojha
2026-06-29 14:17 ` [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published Mukesh Ojha
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Mukesh Ojha @ 2026-06-29 14:17 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski,
Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney
Cc: linux-arm-msm, linux-kernel, Mukesh Ojha
Some of the existing issue reported by Sasiko mentioned here
https://lore.kernel.org/all/20260624192213.C82691F000E9@smtp.kernel.org/
https://lore.kernel.org/all/20260624192418.92B761F000E9@smtp.kernel.org/
and the series is about addressing them.
Changes in v2: https://lore.kernel.org/lkml/20260625093644.3918184-1-mukesh.ojha@oss.qualcomm.com/
- Separated the fixes(2/3, 3/3) as per review.
- Added 1/3 as new patch.
Mukesh Ojha (3):
firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm
is published
firmware: qcom: scm: Fix reserved memory cleanup on probe failure
firmware: qcom: scm: Fix tzmem state on probe retry
drivers/firmware/qcom/qcom_scm-smc.c | 2 +-
drivers/firmware/qcom/qcom_scm.c | 43 ++++++++++++++++------------
drivers/firmware/qcom/qcom_scm.h | 2 +-
drivers/firmware/qcom/qcom_tzmem.c | 13 ++++++---
4 files changed, 36 insertions(+), 24 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published 2026-06-29 14:17 [PATCH v2 0/3] Misc. SCM driver fixes Mukesh Ojha @ 2026-06-29 14:17 ` Mukesh Ojha 2026-06-30 9:50 ` Konrad Dybcio 2026-06-29 14:17 ` [PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure Mukesh Ojha ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Mukesh Ojha @ 2026-06-29 14:17 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski, Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney Cc: linux-arm-msm, linux-kernel, Mukesh Ojha In qcom_scm_probe(), devm_request_threaded_irq() is called before smp_store_release(&__scm, scm). Two paths can dereference __scm before it is published, both causing a NULL pointer dereference. The IRQ handler receives scm via its data argument but passes only wq_ctx to qcom_scm_waitq_wakeup() and qcom_scm_get_completion(), which then dereference __scm directly. Thread scm through both functions so the IRQ handler path never touches __scm. Non-atomic SMC calls made during probe (e.g. from qcom_tzmem_init via qcom_scm_shm_bridge_enable) can return WAITQ_SLEEP, causing qcom_scm_wait_for_wq_completion() to run before __scm is published and dereference it. Add platform_set_drvdata(pdev, scm) early in probe and change qcom_scm_wait_for_wq_completion() to take the device pointer and use dev_get_drvdata() to reach scm, removing any dependency on __scm. Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic") Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> --- drivers/firmware/qcom/qcom_scm-smc.c | 2 +- drivers/firmware/qcom/qcom_scm.c | 22 ++++++++++------------ drivers/firmware/qcom/qcom_scm.h | 2 +- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/drivers/firmware/qcom/qcom_scm-smc.c b/drivers/firmware/qcom/qcom_scm-smc.c index 574930729ddd..7abe60fce676 100644 --- a/drivers/firmware/qcom/qcom_scm-smc.c +++ b/drivers/firmware/qcom/qcom_scm-smc.c @@ -105,7 +105,7 @@ static int __scm_smc_do_quirk_handle_waitq(struct device *dev, struct arm_smccc_ wq_ctx = res->a1; smc_call_ctx = res->a2; - ret = qcom_scm_wait_for_wq_completion(wq_ctx); + ret = qcom_scm_wait_for_wq_completion(dev, wq_ctx); if (ret) return ret; diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index 6b601a4b89db..464ae3b4ca43 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -2630,23 +2630,20 @@ static int qcom_scm_get_waitq_irq(struct qcom_scm *scm) return irq_create_fwspec_mapping(&fwspec); } -static struct completion *qcom_scm_get_completion(u32 wq_ctx) +static struct completion *qcom_scm_get_completion(struct qcom_scm *scm, u32 wq_ctx) { - struct completion *wq; - - if (WARN_ON_ONCE(wq_ctx >= __scm->wq_cnt)) + if (WARN_ON_ONCE(wq_ctx >= scm->wq_cnt)) return ERR_PTR(-EINVAL); - wq = &__scm->waitq_comps[wq_ctx]; - - return wq; + return &scm->waitq_comps[wq_ctx]; } -int qcom_scm_wait_for_wq_completion(u32 wq_ctx) +int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx) { + struct qcom_scm *scm = dev_get_drvdata(dev); struct completion *wq; - wq = qcom_scm_get_completion(wq_ctx); + wq = qcom_scm_get_completion(scm, wq_ctx); if (IS_ERR(wq)) return PTR_ERR(wq); @@ -2655,11 +2652,11 @@ int qcom_scm_wait_for_wq_completion(u32 wq_ctx) return 0; } -static int qcom_scm_waitq_wakeup(unsigned int wq_ctx) +static int qcom_scm_waitq_wakeup(struct qcom_scm *scm, unsigned int wq_ctx) { struct completion *wq; - wq = qcom_scm_get_completion(wq_ctx); + wq = qcom_scm_get_completion(scm, wq_ctx); if (IS_ERR(wq)) return PTR_ERR(wq); @@ -2686,7 +2683,7 @@ static irqreturn_t qcom_scm_irq_handler(int irq, void *data) goto out; } - ret = qcom_scm_waitq_wakeup(wq_ctx); + ret = qcom_scm_waitq_wakeup(scm, wq_ctx); if (ret) goto out; } while (more_pending); @@ -2746,6 +2743,7 @@ static int qcom_scm_probe(struct platform_device *pdev) return -ENOMEM; scm->dev = &pdev->dev; + platform_set_drvdata(pdev, scm); ret = qcom_scm_find_dload_address(&pdev->dev, &scm->dload_mode_addr); if (ret < 0) return ret; diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h index caab80a73e17..cf90a565fdfb 100644 --- a/drivers/firmware/qcom/qcom_scm.h +++ b/drivers/firmware/qcom/qcom_scm.h @@ -66,7 +66,7 @@ struct qcom_scm_res { u64 result[MAX_QCOM_SCM_RETS]; }; -int qcom_scm_wait_for_wq_completion(u32 wq_ctx); +int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx); int scm_get_wq_ctx(u32 *wq_ctx, u32 *flags, u32 *more_pending); #define SCM_SMC_FNID(s, c) ((((s) & 0xFF) << 8) | ((c) & 0xFF)) -- 2.53.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published 2026-06-29 14:17 ` [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published Mukesh Ojha @ 2026-06-30 9:50 ` Konrad Dybcio 2026-06-30 10:07 ` Mukesh Ojha 0 siblings, 1 reply; 10+ messages in thread From: Konrad Dybcio @ 2026-06-30 9:50 UTC (permalink / raw) To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski, Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney Cc: linux-arm-msm, linux-kernel On 6/29/26 4:17 PM, Mukesh Ojha wrote: > In qcom_scm_probe(), devm_request_threaded_irq() is called before > smp_store_release(&__scm, scm). Two paths can dereference __scm before > it is published, both causing a NULL pointer dereference. > > The IRQ handler receives scm via its data argument but passes only wq_ctx > to qcom_scm_waitq_wakeup() and qcom_scm_get_completion(), which then > dereference __scm directly. Thread scm through both functions so the IRQ > handler path never touches __scm. > > Non-atomic SMC calls made during probe (e.g. from qcom_tzmem_init via > qcom_scm_shm_bridge_enable) can return WAITQ_SLEEP, causing > qcom_scm_wait_for_wq_completion() to run before __scm is published and > dereference it. Add platform_set_drvdata(pdev, scm) early in probe and > change qcom_scm_wait_for_wq_completion() to take the device pointer and > use dev_get_drvdata() to reach scm, removing any dependency on __scm. > > Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic") > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> > --- [...] > -int qcom_scm_wait_for_wq_completion(u32 wq_ctx) > +int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx) I think it logically makes more sense to pass a struct qcom_scm* here and get the data from the caller, but potayto/potato Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published 2026-06-30 9:50 ` Konrad Dybcio @ 2026-06-30 10:07 ` Mukesh Ojha 0 siblings, 0 replies; 10+ messages in thread From: Mukesh Ojha @ 2026-06-30 10:07 UTC (permalink / raw) To: Konrad Dybcio Cc: Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski, Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney, linux-arm-msm, linux-kernel On Tue, Jun 30, 2026 at 11:50:30AM +0200, Konrad Dybcio wrote: > On 6/29/26 4:17 PM, Mukesh Ojha wrote: > > In qcom_scm_probe(), devm_request_threaded_irq() is called before > > smp_store_release(&__scm, scm). Two paths can dereference __scm before > > it is published, both causing a NULL pointer dereference. > > > > The IRQ handler receives scm via its data argument but passes only wq_ctx > > to qcom_scm_waitq_wakeup() and qcom_scm_get_completion(), which then > > dereference __scm directly. Thread scm through both functions so the IRQ > > handler path never touches __scm. > > > > Non-atomic SMC calls made during probe (e.g. from qcom_tzmem_init via > > qcom_scm_shm_bridge_enable) can return WAITQ_SLEEP, causing > > qcom_scm_wait_for_wq_completion() to run before __scm is published and > > dereference it. Add platform_set_drvdata(pdev, scm) early in probe and > > change qcom_scm_wait_for_wq_completion() to take the device pointer and > > use dev_get_drvdata() to reach scm, removing any dependency on __scm. > > > > Fixes: 6bf325992236 ("firmware: qcom: scm: Add wait-queue handling logic") > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> > > --- > > [...] > > > -int qcom_scm_wait_for_wq_completion(u32 wq_ctx) > > +int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx) > > I think it logically makes more sense to pass a struct qcom_scm* here > and get the data from the caller, but potayto/potato To maintain the abstraction of struct qcom_scm* to qcom_scm-smc.c, had to use a different way. -- -Mukesh Ojha ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure 2026-06-29 14:17 [PATCH v2 0/3] Misc. SCM driver fixes Mukesh Ojha 2026-06-29 14:17 ` [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published Mukesh Ojha @ 2026-06-29 14:17 ` Mukesh Ojha 2026-06-30 10:07 ` Konrad Dybcio 2026-06-29 14:17 ` [PATCH v2 3/3] firmware: qcom: scm: Fix tzmem state on probe retry Mukesh Ojha 2026-06-30 13:50 ` [PATCH v2 0/3] Misc. SCM driver fixes Bartosz Golaszewski 3 siblings, 1 reply; 10+ messages in thread From: Mukesh Ojha @ 2026-06-29 14:17 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski, Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney Cc: linux-arm-msm, linux-kernel, Mukesh Ojha of_reserved_mem_device_init() adds an entry to a global list with no devres counterpart. If qcom_scm_probe() fails after the call the assignment is never cleaned up. A probe retry would add a duplicate entry, leaking the original one permanently. Add an err_rmem label that calls of_reserved_mem_device_release() and route all error paths after of_reserved_mem_device_init() through it. of_reserved_mem_device_release() is safe to call unconditionally as it simply walks an empty list when nothing was assigned. Fixes: a33b2579c8d3 ("firmware: qcom: scm: add support for SHM bridge memory carveout") Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> --- drivers/firmware/qcom/qcom_scm.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index 464ae3b4ca43..f0e19fc314b4 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -2785,9 +2785,11 @@ static int qcom_scm_probe(struct platform_device *pdev) "Failed to setup the reserved memory region for TZ mem\n"); ret = qcom_tzmem_enable(scm->dev); - if (ret) - return dev_err_probe(scm->dev, ret, - "Failed to enable the TrustZone memory allocator\n"); + if (ret) { + dev_err_probe(scm->dev, ret, + "Failed to enable the TrustZone memory allocator\n"); + goto err_rmem; + } memset(&pool_config, 0, sizeof(pool_config)); pool_config.initial_size = 0; @@ -2795,9 +2797,12 @@ static int qcom_scm_probe(struct platform_device *pdev) pool_config.max_size = SZ_256K; scm->mempool = devm_qcom_tzmem_pool_new(scm->dev, &pool_config); - if (IS_ERR(scm->mempool)) - return dev_err_probe(scm->dev, PTR_ERR(scm->mempool), - "Failed to create the SCM memory pool\n"); + if (IS_ERR(scm->mempool)) { + ret = PTR_ERR(scm->mempool); + dev_err_probe(scm->dev, ret, + "Failed to create the SCM memory pool\n"); + goto err_rmem; + } ret = qcom_scm_query_waitq_count(scm); scm->wq_cnt = ret < 0 ? QCOM_SCM_DEFAULT_WAITQ_COUNT : ret; @@ -2868,6 +2873,10 @@ static int qcom_scm_probe(struct platform_device *pdev) qcom_scm_gunyah_wdt_init(scm); return 0; + +err_rmem: + of_reserved_mem_device_release(scm->dev); + return ret; } static void qcom_scm_shutdown(struct platform_device *pdev) -- 2.53.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure 2026-06-29 14:17 ` [PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure Mukesh Ojha @ 2026-06-30 10:07 ` Konrad Dybcio 2026-07-02 10:28 ` Mukesh Ojha 0 siblings, 1 reply; 10+ messages in thread From: Konrad Dybcio @ 2026-06-30 10:07 UTC (permalink / raw) To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski, Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney Cc: linux-arm-msm, linux-kernel On 6/29/26 4:17 PM, Mukesh Ojha wrote: > of_reserved_mem_device_init() adds an entry to a global list with no > devres counterpart. If qcom_scm_probe() fails after the call the > assignment is never cleaned up. A probe retry would add a duplicate > entry, leaking the original one permanently. > > Add an err_rmem label that calls of_reserved_mem_device_release() and > route all error paths after of_reserved_mem_device_init() through it. > of_reserved_mem_device_release() is safe to call unconditionally as it > simply walks an empty list when nothing was assigned. > > Fixes: a33b2579c8d3 ("firmware: qcom: scm: add support for SHM bridge memory carveout") > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> > --- > drivers/firmware/qcom/qcom_scm.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c > index 464ae3b4ca43..f0e19fc314b4 100644 > --- a/drivers/firmware/qcom/qcom_scm.c > +++ b/drivers/firmware/qcom/qcom_scm.c > @@ -2785,9 +2785,11 @@ static int qcom_scm_probe(struct platform_device *pdev) > "Failed to setup the reserved memory region for TZ mem\n"); > > ret = qcom_tzmem_enable(scm->dev); > - if (ret) > - return dev_err_probe(scm->dev, ret, > - "Failed to enable the TrustZone memory allocator\n"); > + if (ret) { > + dev_err_probe(scm->dev, ret, > + "Failed to enable the TrustZone memory allocator\n"); Assign ret = dev_err_probe to preserve the usefulness of this API Alternatively, I cobbled this together, feel free to take it forward (compile-tested only): From 917c849fd21c6660ba0bd55f8b6ce4cb5dfc8299 Mon Sep 17 00:00:00 2001 From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Date: Tue, 30 Jun 2026 12:02:16 +0200 Subject: [PATCH] of: reserved_mem: Introduce devres-managed initialization functions Introduce devres-based helpers for of_reserved_mem_device_init(_by_idx) to help fight dangling references and ever so slightly reduce the number of boilerplate deinitialization calls. Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> --- drivers/of/of_reserved_mem.c | 37 +++++++++++++++++++++++++++++++++ include/linux/of_reserved_mem.h | 25 ++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 82222bd45ac6..79fa04d4cf04 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -787,6 +787,43 @@ void of_reserved_mem_device_release(struct device *dev) } EXPORT_SYMBOL_GPL(of_reserved_mem_device_release); +static void devm_of_reserved_mem_device_release(struct device *dev, void *res) +{ + of_reserved_mem_device_release(*(struct device **)res); +} + +/** + * devm_of_reserved_mem_device_init_by_idx - Resource managed of_reserved_mem_device_init_by_idx() + * @dev: Pointer to the device to configure + * + * This function assigns respective DMA-mapping operations based on the first + * reserved memory region specified by 'memory-region' property in device tree + * node of the given device. + * + * Returns: Negative errno on failure or zero on success. + */ +int devm_of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx) +{ + struct device **ptr; + int ret; + + ptr = devres_alloc(devm_of_reserved_mem_device_release, sizeof(*ptr), GFP_KERNEL); + if (!ptr) + return -ENOMEM; + + ret = of_reserved_mem_device_init_by_idx(dev, np, idx); + if (ret) { + devres_free(ptr); + return ret; + } + + *ptr = dev; + devres_add(dev, ptr); + + return 0; +} +EXPORT_SYMBOL_GPL(devm_of_reserved_mem_device_init_by_idx); + /** * of_reserved_mem_lookup() - acquire reserved_mem from a device node * @np: node pointer of the desired reserved-memory region diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h index e8b20b29fa68..28beeeb91f4e 100644 --- a/include/linux/of_reserved_mem.h +++ b/include/linux/of_reserved_mem.h @@ -47,6 +47,8 @@ int of_reserved_mem_region_to_resource_byname(const struct device_node *np, const char *name, struct resource *res); int of_reserved_mem_region_count(const struct device_node *np); +int devm_of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx); + #else #define RESERVEDMEM_OF_DECLARE(name, compat, ops) \ @@ -91,6 +93,14 @@ static inline int of_reserved_mem_region_count(const struct device_node *np) { return 0; } + +static inline int devm_of_reserved_mem_device_init_by_idx(struct device *dev, + struct device_node *np, + int idx) +{ + return -ENOSYS; +} + #endif /** @@ -108,4 +118,19 @@ static inline int of_reserved_mem_device_init(struct device *dev) return of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0); } +/** + * of_reserved_mem_device_init() - Devres-managed version of of_reserved_mem_device_init() + * @dev: Pointer to the device to configure + * + * This function assigns respective DMA-mapping operations based on the first + * reserved memory region specified by 'memory-region' property in device tree + * node of the given device. + * + * Returns error code or zero on success. + */ +static inline int devm_of_reserved_mem_device_init(struct device *dev) +{ + return devm_of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0); +} + #endif /* __OF_RESERVED_MEM_H */ -- 2.54.0 Konrad ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure 2026-06-30 10:07 ` Konrad Dybcio @ 2026-07-02 10:28 ` Mukesh Ojha 0 siblings, 0 replies; 10+ messages in thread From: Mukesh Ojha @ 2026-07-02 10:28 UTC (permalink / raw) To: Konrad Dybcio Cc: Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski, Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney, linux-arm-msm, linux-kernel On Tue, Jun 30, 2026 at 12:07:26PM +0200, Konrad Dybcio wrote: > On 6/29/26 4:17 PM, Mukesh Ojha wrote: > > of_reserved_mem_device_init() adds an entry to a global list with no > > devres counterpart. If qcom_scm_probe() fails after the call the > > assignment is never cleaned up. A probe retry would add a duplicate > > entry, leaking the original one permanently. > > > > Add an err_rmem label that calls of_reserved_mem_device_release() and > > route all error paths after of_reserved_mem_device_init() through it. > > of_reserved_mem_device_release() is safe to call unconditionally as it > > simply walks an empty list when nothing was assigned. > > > > Fixes: a33b2579c8d3 ("firmware: qcom: scm: add support for SHM bridge memory carveout") > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> > > --- > > drivers/firmware/qcom/qcom_scm.c | 21 +++++++++++++++------ > > 1 file changed, 15 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c > > index 464ae3b4ca43..f0e19fc314b4 100644 > > --- a/drivers/firmware/qcom/qcom_scm.c > > +++ b/drivers/firmware/qcom/qcom_scm.c > > @@ -2785,9 +2785,11 @@ static int qcom_scm_probe(struct platform_device *pdev) > > "Failed to setup the reserved memory region for TZ mem\n"); > > > > ret = qcom_tzmem_enable(scm->dev); > > - if (ret) > > - return dev_err_probe(scm->dev, ret, > > - "Failed to enable the TrustZone memory allocator\n"); > > + if (ret) { > > + dev_err_probe(scm->dev, ret, > > + "Failed to enable the TrustZone memory allocator\n"); > > Assign ret = dev_err_probe to preserve the usefulness of this API > Sure. > Alternatively, I cobbled this together, feel free to take it forward > (compile-tested only): Sure, thank you along with all the clean up. -Mukesh > > From 917c849fd21c6660ba0bd55f8b6ce4cb5dfc8299 Mon Sep 17 00:00:00 2001 > From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > Date: Tue, 30 Jun 2026 12:02:16 +0200 > Subject: [PATCH] of: reserved_mem: Introduce devres-managed initialization > functions > > Introduce devres-based helpers for of_reserved_mem_device_init(_by_idx) > to help fight dangling references and ever so slightly reduce the > number of boilerplate deinitialization calls. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > --- > drivers/of/of_reserved_mem.c | 37 +++++++++++++++++++++++++++++++++ > include/linux/of_reserved_mem.h | 25 ++++++++++++++++++++++ > 2 files changed, 62 insertions(+) > > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c > index 82222bd45ac6..79fa04d4cf04 100644 > --- a/drivers/of/of_reserved_mem.c > +++ b/drivers/of/of_reserved_mem.c > @@ -787,6 +787,43 @@ void of_reserved_mem_device_release(struct device *dev) > } > EXPORT_SYMBOL_GPL(of_reserved_mem_device_release); > > +static void devm_of_reserved_mem_device_release(struct device *dev, void *res) > +{ > + of_reserved_mem_device_release(*(struct device **)res); > +} > + > +/** > + * devm_of_reserved_mem_device_init_by_idx - Resource managed of_reserved_mem_device_init_by_idx() > + * @dev: Pointer to the device to configure > + * > + * This function assigns respective DMA-mapping operations based on the first > + * reserved memory region specified by 'memory-region' property in device tree > + * node of the given device. > + * > + * Returns: Negative errno on failure or zero on success. > + */ > +int devm_of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx) > +{ > + struct device **ptr; > + int ret; > + > + ptr = devres_alloc(devm_of_reserved_mem_device_release, sizeof(*ptr), GFP_KERNEL); > + if (!ptr) > + return -ENOMEM; > + > + ret = of_reserved_mem_device_init_by_idx(dev, np, idx); > + if (ret) { > + devres_free(ptr); > + return ret; > + } > + > + *ptr = dev; > + devres_add(dev, ptr); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(devm_of_reserved_mem_device_init_by_idx); > + > /** > * of_reserved_mem_lookup() - acquire reserved_mem from a device node > * @np: node pointer of the desired reserved-memory region > diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h > index e8b20b29fa68..28beeeb91f4e 100644 > --- a/include/linux/of_reserved_mem.h > +++ b/include/linux/of_reserved_mem.h > @@ -47,6 +47,8 @@ int of_reserved_mem_region_to_resource_byname(const struct device_node *np, > const char *name, struct resource *res); > int of_reserved_mem_region_count(const struct device_node *np); > > +int devm_of_reserved_mem_device_init_by_idx(struct device *dev, struct device_node *np, int idx); > + > #else > > #define RESERVEDMEM_OF_DECLARE(name, compat, ops) \ > @@ -91,6 +93,14 @@ static inline int of_reserved_mem_region_count(const struct device_node *np) > { > return 0; > } > + > +static inline int devm_of_reserved_mem_device_init_by_idx(struct device *dev, > + struct device_node *np, > + int idx) > +{ > + return -ENOSYS; > +} > + > #endif > > /** > @@ -108,4 +118,19 @@ static inline int of_reserved_mem_device_init(struct device *dev) > return of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0); > } > > +/** > + * of_reserved_mem_device_init() - Devres-managed version of of_reserved_mem_device_init() > + * @dev: Pointer to the device to configure > + * > + * This function assigns respective DMA-mapping operations based on the first > + * reserved memory region specified by 'memory-region' property in device tree > + * node of the given device. > + * > + * Returns error code or zero on success. > + */ > +static inline int devm_of_reserved_mem_device_init(struct device *dev) > +{ > + return devm_of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0); > +} > + > #endif /* __OF_RESERVED_MEM_H */ > -- > 2.54.0 > > > Konrad -- -Mukesh Ojha ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] firmware: qcom: scm: Fix tzmem state on probe retry 2026-06-29 14:17 [PATCH v2 0/3] Misc. SCM driver fixes Mukesh Ojha 2026-06-29 14:17 ` [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published Mukesh Ojha 2026-06-29 14:17 ` [PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure Mukesh Ojha @ 2026-06-29 14:17 ` Mukesh Ojha 2026-06-30 10:12 ` Konrad Dybcio 2026-06-30 13:50 ` [PATCH v2 0/3] Misc. SCM driver fixes Bartosz Golaszewski 3 siblings, 1 reply; 10+ messages in thread From: Mukesh Ojha @ 2026-06-29 14:17 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski, Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney Cc: linux-arm-msm, linux-kernel, Mukesh Ojha qcom_tzmem_enable() returns -EBUSY if called a second time, but this causes probe retries to fail permanently if a later step in qcom_scm_probe() defers after qcom_tzmem_enable() has already succeeded. Use DO_ONCE() to ensure qcom_tzmem_init() runs exactly once across all calls in a thread-safe manner. qcom_tzmem_dev is set on every call since probe retries use the same device pointer. The result of the first initialisation is cached and returned to every subsequent caller. Fixes: 40289e35ca52 ("firmware: qcom: scm: enable the TZ mem allocator") Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> --- drivers/firmware/qcom/qcom_tzmem.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c index 0635cbeacfc8..0fd9581275f1 100644 --- a/drivers/firmware/qcom/qcom_tzmem.c +++ b/drivers/firmware/qcom/qcom_tzmem.c @@ -15,6 +15,7 @@ #include <linux/kernel.h> #include <linux/list.h> #include <linux/mm.h> +#include <linux/once.h> #include <linux/radix-tree.h> #include <linux/slab.h> #include <linux/spinlock.h> @@ -507,14 +508,18 @@ phys_addr_t qcom_tzmem_to_phys(void *vaddr) } EXPORT_SYMBOL_GPL(qcom_tzmem_to_phys); +static void qcom_tzmem_do_init(int *result) +{ + *result = qcom_tzmem_init(); +} + int qcom_tzmem_enable(struct device *dev) { - if (qcom_tzmem_dev) - return -EBUSY; + static int result; qcom_tzmem_dev = dev; - - return qcom_tzmem_init(); + DO_ONCE(qcom_tzmem_do_init, &result); + return result; } EXPORT_SYMBOL_GPL(qcom_tzmem_enable); -- 2.53.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] firmware: qcom: scm: Fix tzmem state on probe retry 2026-06-29 14:17 ` [PATCH v2 3/3] firmware: qcom: scm: Fix tzmem state on probe retry Mukesh Ojha @ 2026-06-30 10:12 ` Konrad Dybcio 0 siblings, 0 replies; 10+ messages in thread From: Konrad Dybcio @ 2026-06-30 10:12 UTC (permalink / raw) To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski, Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney Cc: linux-arm-msm, linux-kernel On 6/29/26 4:17 PM, Mukesh Ojha wrote: > qcom_tzmem_enable() returns -EBUSY if called a second time, but this > causes probe retries to fail permanently if a later step in > qcom_scm_probe() defers after qcom_tzmem_enable() has already succeeded. > > Use DO_ONCE() to ensure qcom_tzmem_init() runs exactly once across all > calls in a thread-safe manner. qcom_tzmem_dev is set on every call since > probe retries use the same device pointer. The result of the first > initialisation is cached and returned to every subsequent caller. > > Fixes: 40289e35ca52 ("firmware: qcom: scm: enable the TZ mem allocator") > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/3] Misc. SCM driver fixes 2026-06-29 14:17 [PATCH v2 0/3] Misc. SCM driver fixes Mukesh Ojha ` (2 preceding siblings ...) 2026-06-29 14:17 ` [PATCH v2 3/3] firmware: qcom: scm: Fix tzmem state on probe retry Mukesh Ojha @ 2026-06-30 13:50 ` Bartosz Golaszewski 3 siblings, 0 replies; 10+ messages in thread From: Bartosz Golaszewski @ 2026-06-30 13:50 UTC (permalink / raw) To: Mukesh Ojha Cc: linux-arm-msm, linux-kernel, Bjorn Andersson, Konrad Dybcio, Bartosz Golaszewski, Guru Das Srinagesh, Sibi Sankar, Elliot Berman, Andrew Halaney On Mon, 29 Jun 2026 16:17:47 +0200, Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> said: > Some of the existing issue reported by Sasiko mentioned here > > https://lore.kernel.org/all/20260624192213.C82691F000E9@smtp.kernel.org/ > > https://lore.kernel.org/all/20260624192418.92B761F000E9@smtp.kernel.org/ > > and the series is about addressing them. > > > Changes in v2: https://lore.kernel.org/lkml/20260625093644.3918184-1-mukesh.ojha@oss.qualcomm.com/ > - Separated the fixes(2/3, 3/3) as per review. > - Added 1/3 as new patch. > > Mukesh Ojha (3): > firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm > is published > firmware: qcom: scm: Fix reserved memory cleanup on probe failure > firmware: qcom: scm: Fix tzmem state on probe retry > > drivers/firmware/qcom/qcom_scm-smc.c | 2 +- > drivers/firmware/qcom/qcom_scm.c | 43 ++++++++++++++++------------ > drivers/firmware/qcom/qcom_scm.h | 2 +- > drivers/firmware/qcom/qcom_tzmem.c | 13 ++++++--- > 4 files changed, 36 insertions(+), 24 deletions(-) > > -- > 2.53.0 > > With the issue raised by Konrad for patch 2/3 fixed: Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-07-02 10:28 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-06-29 14:17 [PATCH v2 0/3] Misc. SCM driver fixes Mukesh Ojha 2026-06-29 14:17 ` [PATCH v2 1/3] firmware: qcom: scm: Fix NULL dereference in IRQ handler before __scm is published Mukesh Ojha 2026-06-30 9:50 ` Konrad Dybcio 2026-06-30 10:07 ` Mukesh Ojha 2026-06-29 14:17 ` [PATCH v2 2/3] firmware: qcom: scm: Fix reserved memory cleanup on probe failure Mukesh Ojha 2026-06-30 10:07 ` Konrad Dybcio 2026-07-02 10:28 ` Mukesh Ojha 2026-06-29 14:17 ` [PATCH v2 3/3] firmware: qcom: scm: Fix tzmem state on probe retry Mukesh Ojha 2026-06-30 10:12 ` Konrad Dybcio 2026-06-30 13:50 ` [PATCH v2 0/3] Misc. SCM driver fixes Bartosz Golaszewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®