* [PATCH v2 1/4] dt-bindings: remoteproc: qcom: Add Glymur OOBMSS PAS
2026-10-15 5:55 [PATCH v2 0/4] Add Glymur OOBMSS support Anvesh Jain P
@ 2026-10-15 5:55 ` Anvesh Jain P
2026-10-15 10:34 ` Krzysztof Kozlowski
2026-10-15 5:55 ` [PATCH v2 2/4] remoteproc: qcom: pas: Support secondary images Anvesh Jain P
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Anvesh Jain P @ 2026-10-15 5:55 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Sibi Sankar, Anvesh Jain P
Document the Glymur OOBMSS PAS node with its secure GLink edge,
shared-memory signaling, firmware carveouts, and power resources.
Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
---
.../remoteproc/qcom,glymur-oobmss-pas.yaml | 154 +++++++++++++++++++++
1 file changed, 154 insertions(+)
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,glymur-oobmss-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,glymur-oobmss-pas.yaml
new file mode 100644
index 000000000000..3aa96d2262f1
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,glymur-oobmss-pas.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/qcom,glymur-oobmss-pas.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Glymur Out-of-Band Management Subsystem (OOBMSS) PAS
+
+maintainers:
+ - Anvesh Jain P <anvesh.p@oss.qualcomm.com>
+
+description: |
+ Qualcomm Glymur Out-of-Band Management Subsystem (OOBMSS) contains two
+ processors running Zephyr, managed together as one remoteproc instance.
+ The Out-of-Band Trusted Execution Environment (OOB-TEE) is the primary
+ processor. The OOB-Non-Secure processor is an auxiliary processor that
+ depends on the primary processor and cannot function independently.
+
+ The primary processor runs the primary image, while the auxiliary
+ processor runs the secondary image. Each processor has its own DTB.
+ The two images and their DTBs have separate PAS identifiers for
+ authentication and dedicated reserved-memory carveouts.
+
+ OOBMSS is booted by earlier-stage firmware and uses an XO clock and the
+ CX and MX power domains. Shared Memory Point-to-Point (SMP2P) provides
+ boot, shutdown, handover and ping/pong signaling, as well as fatal error
+ notifications. The subsystem exposes a secure GLink edge to the host,
+ with doorbell interrupts provided by the Inter-Processor Communication
+ Controller (IPCC).
+
+properties:
+ compatible:
+ enum:
+ - qcom,glymur-oobmss-pas
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ items:
+ - description: Watchdog interrupt
+ - description: Fatal interrupt
+ - description: Ready interrupt
+ - description: Handover interrupt
+ - description: Stop acknowledge interrupt
+ - description: Pong interrupt
+
+ interrupt-names:
+ items:
+ - const: wdog
+ - const: fatal
+ - const: ready
+ - const: handover
+ - const: stop-ack
+ - const: pong
+
+ clocks:
+ items:
+ - description: XO clock
+
+ clock-names:
+ items:
+ - const: xo
+
+ power-domains:
+ items:
+ - description: CX power domain
+ - description: MX power domain
+
+ power-domain-names:
+ items:
+ - const: cx
+ - const: mx
+
+ memory-region:
+ items:
+ - description: Secure firmware carveout
+ - description: Secure device tree carveout
+ - description: Non-secure firmware carveout
+ - description: Non-secure device tree carveout
+
+ firmware-name:
+ items:
+ - description: Secure firmware
+ - description: Secure device tree firmware
+ - description: Non-secure firmware
+ - description: Non-secure device tree firmware
+
+ qcom,smem-states:
+ minItems: 2
+ description: States used by the AP to signal OOBMSS
+
+ qcom,smem-state-names:
+ minItems: 2
+ description: The names of the state bits used for SMP2P output
+
+required:
+ - compatible
+ - reg
+ - power-domains
+ - power-domain-names
+ - memory-region
+
+allOf:
+ - $ref: /schemas/remoteproc/qcom,pas-common.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,rpmh.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/mailbox/qcom-ipcc.h>
+ #include <dt-bindings/power/qcom,rpmhpd.h>
+
+ remoteproc@1f000000 {
+ compatible = "qcom,glymur-oobmss-pas";
+ reg = <0x1f000000 0x30000>;
+
+ interrupts-extended = <&intc GIC_SPI 86 IRQ_TYPE_EDGE_RISING>,
+ <&smp2p_oobmss_in 0 IRQ_TYPE_EDGE_RISING>,
+ <&smp2p_oobmss_in 1 IRQ_TYPE_EDGE_RISING>,
+ <&smp2p_oobmss_in 2 IRQ_TYPE_EDGE_RISING>,
+ <&smp2p_oobmss_in 3 IRQ_TYPE_EDGE_RISING>,
+ <&smp2p_oobmss_in 9 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "wdog", "fatal", "ready", "handover", "stop-ack", "pong";
+
+ clocks = <&rpmhcc RPMH_CXO_CLK>;
+ clock-names = "xo";
+ power-domains = <&rpmhpd RPMHPD_CX>,
+ <&rpmhpd RPMHPD_MX>;
+ power-domain-names = "cx", "mx";
+
+ memory-region = <&oob_secure_mem>, <&oobdtbqc_mem>,
+ <&oob_nonsecure_mem>, <&oobdtboem_mem>;
+ firmware-name = "qcom/glymur/oobmss_tee.mbn",
+ "qcom/glymur/oobmss_qc_dtb.mbn",
+ "qcom/glymur/oobmss_ns.mbn",
+ "qcom/glymur/oobmss_oem_dtb.mbn";
+ qcom,smem-states = <&smp2p_oobmss_out 1>,
+ <&smp2p_oobmss_out 8>;
+ qcom,smem-state-names = "stop", "ping";
+
+ glink-edge {
+ interrupts-extended = <&ipcc 56
+ IPCC_MPROC_SIGNAL_GLINK_QMP
+ IRQ_TYPE_EDGE_RISING>;
+ mboxes = <&ipcc 56 IPCC_MPROC_SIGNAL_GLINK_QMP>;
+ label = "oobmss";
+ qcom,remote-pid = <20>;
+ };
+
+ };
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 1/4] dt-bindings: remoteproc: qcom: Add Glymur OOBMSS PAS
2026-10-15 5:55 ` [PATCH v2 1/4] dt-bindings: remoteproc: qcom: Add Glymur OOBMSS PAS Anvesh Jain P
@ 2026-10-15 10:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2026-10-15 10:34 UTC (permalink / raw)
To: Anvesh Jain P
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio, linux-arm-msm,
linux-remoteproc, devicetree, linux-kernel, Sibi Sankar
On Thu, Oct 15, 2026 at 11:25:49AM +0530, Anvesh Jain P wrote:
> Document the Glymur OOBMSS PAS node with its secure GLink edge,
> shared-memory signaling, firmware carveouts, and power resources.
>
> Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
> ---
> .../remoteproc/qcom,glymur-oobmss-pas.yaml | 154 +++++++++++++++++++++
> 1 file changed, 154 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,glymur-oobmss-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,glymur-oobmss-pas.yaml
> new file mode 100644
> index 000000000000..3aa96d2262f1
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/4] remoteproc: qcom: pas: Support secondary images
2026-10-15 5:55 [PATCH v2 0/4] Add Glymur OOBMSS support Anvesh Jain P
2026-10-15 5:55 ` [PATCH v2 1/4] dt-bindings: remoteproc: qcom: Add Glymur OOBMSS PAS Anvesh Jain P
@ 2026-10-15 5:55 ` Anvesh Jain P
2026-10-15 6:05 ` sashiko-bot
2026-10-15 5:55 ` [PATCH v2 3/4] remoteproc: qcom: pas: Add coredump support for secondary processors Anvesh Jain P
2026-10-15 5:55 ` [PATCH v2 4/4] remoteproc: qcom: pas: Add Glymur OOBMSS PAS parameters Anvesh Jain P
3 siblings, 1 reply; 9+ messages in thread
From: Anvesh Jain P @ 2026-10-15 5:55 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Sibi Sankar, Anvesh Jain P
The Out-of-Band Management Subsystem (OOBMSS) is managed by a common
remote processor and consists of two Zephyr subsystems:
+----------------------------------------------------------+
| OOBMSS |
| |
| +------------------------+ +------------------------+ |
| | OOB-TEE | | OOB-Non-Secure | |
| | Primary processor | | Auxiliary processor | |
| | Primary image and DTB | | Secondary image and DTB| |
| +------------------------+ +------------------------+ |
+----------------------------------------------------------+
The primary processor is the Out-of-Band Trusted Execution Environment
(OOB-TEE) processor. The OOB-Non-Secure processor is an auxiliary processor
that depends on the primary processor and cannot function independently.
The PAS remoteproc driver currently supports one primary image and its
optional DTB. OOBMSS additionally requires the secondary image and DTB
to be loaded, authenticated, mapped, shut down, and released through
their own PAS identifiers and carveouts.
Add generic PAS support for loading, authenticating, and mapping a
secondary image and its DTB, then shutting down and releasing both while
keeping the existing primary-image lifecycle unchanged.
Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
---
drivers/remoteproc/qcom_q6v5_pas.c | 240 ++++++++++++++++++++++++++++++++++---
1 file changed, 222 insertions(+), 18 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index ca8e61254c44..0bce2f0d5b4a 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -42,8 +42,12 @@ struct qcom_pas_data {
int crash_reason_smem;
const char *firmware_name;
const char *dtb_firmware_name;
+ const char *secondary_firmware_name;
+ const char *secondary_dtb_firmware_name;
int pas_id;
int dtb_pas_id;
+ int secondary_pas_id;
+ int secondary_dtb_pas_id;
int lite_pas_id;
int lite_dtb_pas_id;
unsigned int minidump_id;
@@ -83,8 +87,12 @@ struct qcom_pas {
int proxy_pd_count;
const char *dtb_firmware_name;
+ const char *secondary_firmware_name;
+ const char *secondary_dtb_firmware_name;
int pas_id;
int dtb_pas_id;
+ int secondary_pas_id;
+ int secondary_dtb_pas_id;
int lite_pas_id;
int lite_dtb_pas_id;
unsigned int minidump_id;
@@ -95,17 +103,25 @@ struct qcom_pas {
const struct firmware *firmware;
const struct firmware *dtb_firmware;
+ const struct firmware *secondary_firmware;
+ const struct firmware *secondary_dtb_firmware;
phys_addr_t mem_phys;
phys_addr_t dtb_mem_phys;
+ phys_addr_t secondary_mem_phys;
+ phys_addr_t secondary_dtb_mem_phys;
phys_addr_t mem_reloc;
phys_addr_t dtb_mem_reloc;
+ phys_addr_t secondary_mem_reloc;
+ phys_addr_t secondary_dtb_mem_reloc;
phys_addr_t region_assign_phys[MAX_ASSIGN_COUNT];
void __iomem *mem_region;
size_t mem_size;
size_t dtb_mem_size;
+ size_t secondary_mem_size;
+ size_t secondary_dtb_mem_size;
size_t region_assign_size[MAX_ASSIGN_COUNT];
int region_assign_idx;
@@ -123,6 +139,8 @@ struct qcom_pas {
struct qcom_pas_context *pas_ctx;
struct qcom_pas_context *dtb_pas_ctx;
+ struct qcom_pas_context *secondary_pas_ctx;
+ struct qcom_pas_context *secondary_dtb_pas_ctx;
};
static void qcom_pas_segment_dump(struct rproc *rproc,
@@ -198,19 +216,37 @@ static void qcom_pas_pds_disable(struct qcom_pas *pas, struct device **pds,
}
}
-static int qcom_pas_shutdown_poll_decrypt(struct qcom_pas *pas)
+static int qcom_pas_shutdown_poll_decrypt(int pas_id)
{
unsigned int retry_num = 50;
int ret;
do {
msleep(QCOM_PAS_DECRYPT_SHUTDOWN_DELAY_MS);
- ret = qcom_pas_shutdown(pas->pas_id);
+ ret = qcom_pas_shutdown(pas_id);
} while (ret == -EINVAL && --retry_num);
return ret;
}
+static void qcom_pas_release_firmware(struct qcom_pas *pas)
+{
+ if (pas->dtb_firmware) {
+ release_firmware(pas->dtb_firmware);
+ pas->dtb_firmware = NULL;
+ }
+
+ if (pas->secondary_firmware) {
+ release_firmware(pas->secondary_firmware);
+ pas->secondary_firmware = NULL;
+ }
+
+ if (pas->secondary_dtb_firmware) {
+ release_firmware(pas->secondary_dtb_firmware);
+ pas->secondary_dtb_firmware = NULL;
+ }
+}
+
static int qcom_pas_unprepare(struct rproc *rproc)
{
struct qcom_pas *pas = rproc->priv;
@@ -224,6 +260,10 @@ static int qcom_pas_unprepare(struct rproc *rproc)
qcom_pas_metadata_release(pas->pas_ctx);
if (pas->dtb_pas_id)
qcom_pas_metadata_release(pas->dtb_pas_ctx);
+ if (pas->secondary_pas_id)
+ qcom_pas_metadata_release(pas->secondary_pas_ctx);
+ if (pas->secondary_dtb_pas_id)
+ qcom_pas_metadata_release(pas->secondary_dtb_pas_ctx);
return 0;
}
@@ -241,19 +281,29 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
if (pas->lite_dtb_pas_id)
qcom_pas_shutdown(pas->lite_dtb_pas_id);
- if (pas->dtb_pas_id) {
- ret = request_firmware(&pas->dtb_firmware, pas->dtb_firmware_name, pas->dev);
+ if (pas->secondary_dtb_pas_id) {
+ ret = request_firmware(&pas->secondary_dtb_firmware,
+ pas->secondary_dtb_firmware_name, pas->dev);
+ if (ret) {
+ qcom_pas_release_firmware(pas);
+ return ret;
+ }
+ }
+
+ if (pas->secondary_pas_id) {
+ ret = request_firmware(&pas->secondary_firmware,
+ pas->secondary_firmware_name, pas->dev);
if (ret) {
- dev_err(pas->dev, "request_firmware failed for %s: %d\n",
- pas->dtb_firmware_name, ret);
+ qcom_pas_release_firmware(pas);
return ret;
}
+ }
- ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
- pas->dtb_firmware_name, &pas->dtb_mem_reloc);
+ if (pas->dtb_pas_id) {
+ ret = request_firmware(&pas->dtb_firmware, pas->dtb_firmware_name,
+ pas->dev);
if (ret) {
- qcom_pas_metadata_release(pas->dtb_pas_ctx);
- release_firmware(pas->dtb_firmware);
+ qcom_pas_release_firmware(pas);
return ret;
}
}
@@ -310,10 +360,51 @@ static int qcom_pas_start(struct rproc *rproc)
goto disable_cx_supply;
}
+ if (pas->secondary_dtb_pas_id) {
+ ret = qcom_mdt_pas_load(pas->secondary_dtb_pas_ctx,
+ pas->secondary_dtb_firmware,
+ pas->secondary_dtb_firmware_name,
+ &pas->secondary_dtb_mem_reloc);
+ if (ret)
+ goto release_secondary_dtb_metadata;
+
+ ret = qcom_pas_map_carveout(rproc, pas->secondary_dtb_mem_phys,
+ pas->secondary_dtb_mem_size);
+ if (ret)
+ goto release_secondary_dtb_metadata;
+
+ ret = qcom_pas_prepare_and_auth_reset(pas->secondary_dtb_pas_ctx);
+ if (ret)
+ goto unmap_secondary_dtb_carveout;
+ }
+
+ if (pas->secondary_pas_id) {
+ ret = qcom_mdt_pas_load(pas->secondary_pas_ctx,
+ pas->secondary_firmware,
+ pas->secondary_firmware_name,
+ &pas->secondary_mem_reloc);
+ if (ret)
+ goto release_secondary_metadata;
+
+ ret = qcom_pas_map_carveout(rproc, pas->secondary_mem_phys,
+ pas->secondary_mem_size);
+ if (ret)
+ goto release_secondary_metadata;
+
+ ret = qcom_pas_prepare_and_auth_reset(pas->secondary_pas_ctx);
+ if (ret)
+ goto unmap_secondary_carveout;
+ }
+
if (pas->dtb_pas_id) {
+ ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
+ pas->dtb_firmware_name, &pas->dtb_mem_reloc);
+ if (ret)
+ goto release_dtb_metadata;
+
ret = qcom_pas_map_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
if (ret)
- goto disable_px_supply;
+ goto release_dtb_metadata;
ret = qcom_pas_prepare_and_auth_reset(pas->dtb_pas_ctx);
if (ret) {
@@ -344,13 +435,16 @@ static int qcom_pas_start(struct rproc *rproc)
ret = qcom_q6v5_wait_for_start(&pas->q6v5, msecs_to_jiffies(5000));
if (ret == -ETIMEDOUT) {
dev_err(pas->dev, "start timed out\n");
- qcom_pas_shutdown(pas->pas_id);
goto unmap_carveout;
}
qcom_pas_metadata_release(pas->pas_ctx);
if (pas->dtb_pas_id)
qcom_pas_metadata_release(pas->dtb_pas_ctx);
+ if (pas->secondary_pas_id)
+ qcom_pas_metadata_release(pas->secondary_pas_ctx);
+ if (pas->secondary_dtb_pas_id)
+ qcom_pas_metadata_release(pas->secondary_dtb_pas_ctx);
/* firmware is used to pass reference from qcom_pas_start(), drop it now */
pas->firmware = NULL;
@@ -358,16 +452,37 @@ static int qcom_pas_start(struct rproc *rproc)
return 0;
unmap_carveout:
+ qcom_pas_shutdown(pas->pas_id);
qcom_pas_unmap_carveout(rproc, pas->mem_phys, pas->mem_size);
release_pas_metadata:
qcom_pas_metadata_release(pas->pas_ctx);
- if (pas->dtb_pas_id)
- qcom_pas_metadata_release(pas->dtb_pas_ctx);
-
unmap_dtb_carveout:
+ if (pas->dtb_pas_id) {
+ qcom_pas_shutdown(pas->dtb_pas_id);
+ qcom_pas_unmap_carveout(rproc, pas->dtb_mem_phys,
+ pas->dtb_mem_size);
+ }
+release_dtb_metadata:
if (pas->dtb_pas_id)
- qcom_pas_unmap_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
-disable_px_supply:
+ qcom_pas_metadata_release(pas->dtb_pas_ctx);
+unmap_secondary_carveout:
+ if (pas->secondary_pas_id) {
+ qcom_pas_shutdown(pas->secondary_pas_id);
+ qcom_pas_unmap_carveout(rproc, pas->secondary_mem_phys,
+ pas->secondary_mem_size);
+ }
+release_secondary_metadata:
+ if (pas->secondary_pas_id)
+ qcom_pas_metadata_release(pas->secondary_pas_ctx);
+unmap_secondary_dtb_carveout:
+ if (pas->secondary_dtb_pas_id) {
+ qcom_pas_shutdown(pas->secondary_dtb_pas_id);
+ qcom_pas_unmap_carveout(rproc, pas->secondary_dtb_mem_phys,
+ pas->secondary_dtb_mem_size);
+ }
+release_secondary_dtb_metadata:
+ if (pas->secondary_dtb_pas_id)
+ qcom_pas_metadata_release(pas->secondary_dtb_pas_ctx);
if (pas->px_supply)
regulator_disable(pas->px_supply);
disable_cx_supply:
@@ -413,7 +528,7 @@ static int qcom_pas_stop(struct rproc *rproc)
ret = qcom_pas_shutdown(pas->pas_id);
if (ret && pas->decrypt_shutdown)
- ret = qcom_pas_shutdown_poll_decrypt(pas);
+ ret = qcom_pas_shutdown_poll_decrypt(pas->pas_id);
if (ret)
dev_err(pas->dev, "failed to shutdown: %d\n", ret);
@@ -426,6 +541,27 @@ static int qcom_pas_stop(struct rproc *rproc)
qcom_pas_unmap_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
}
+ if (pas->secondary_pas_id) {
+ ret = qcom_pas_shutdown(pas->secondary_pas_id);
+ if (ret && pas->decrypt_shutdown)
+ ret = qcom_pas_shutdown_poll_decrypt(pas->secondary_pas_id);
+ if (ret)
+ dev_err(pas->dev, "failed to shutdown secondary: %d\n", ret);
+
+ qcom_pas_unmap_carveout(rproc, pas->secondary_mem_phys,
+ pas->secondary_mem_size);
+ }
+
+ if (pas->secondary_dtb_pas_id) {
+ ret = qcom_pas_shutdown(pas->secondary_dtb_pas_id);
+ if (ret)
+ dev_err(pas->dev,
+ "failed to shutdown secondary dtb: %d\n", ret);
+
+ qcom_pas_unmap_carveout(rproc, pas->secondary_dtb_mem_phys,
+ pas->secondary_dtb_mem_size);
+ }
+
qcom_pas_unmap_carveout(rproc, pas->mem_phys, pas->mem_size);
handover = qcom_q6v5_unprepare(&pas->q6v5);
@@ -740,6 +876,42 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
if (IS_ERR(pas->dtb_pas_ctx))
return PTR_ERR(pas->dtb_pas_ctx);
+ if (!pas->secondary_pas_id)
+ return 0;
+
+ ret = of_reserved_mem_region_to_resource(pas->dev->of_node, 2, &res);
+ if (ret)
+ return ret;
+
+ pas->secondary_mem_phys = res.start;
+ pas->secondary_mem_reloc = res.start;
+ pas->secondary_mem_size = resource_size(&res);
+
+ pas->secondary_pas_ctx =
+ devm_qcom_pas_context_alloc(pas->dev, pas->secondary_pas_id,
+ pas->secondary_mem_phys,
+ pas->secondary_mem_size);
+ if (IS_ERR(pas->secondary_pas_ctx))
+ return PTR_ERR(pas->secondary_pas_ctx);
+
+ if (!pas->secondary_dtb_pas_id)
+ return 0;
+
+ ret = of_reserved_mem_region_to_resource(pas->dev->of_node, 3, &res);
+ if (ret)
+ return ret;
+
+ pas->secondary_dtb_mem_phys = res.start;
+ pas->secondary_dtb_mem_reloc = res.start;
+ pas->secondary_dtb_mem_size = resource_size(&res);
+
+ pas->secondary_dtb_pas_ctx =
+ devm_qcom_pas_context_alloc(pas->dev, pas->secondary_dtb_pas_id,
+ pas->secondary_dtb_mem_phys,
+ pas->secondary_dtb_mem_size);
+ if (IS_ERR(pas->secondary_dtb_pas_ctx))
+ return PTR_ERR(pas->secondary_dtb_pas_ctx);
+
return 0;
}
@@ -823,6 +995,8 @@ static int qcom_pas_probe(struct platform_device *pdev)
struct rproc *rproc;
struct device_node *node;
const char *fw_name, *dtb_fw_name = NULL;
+ const char *secondary_fw_name = NULL;
+ const char *secondary_dtb_fw_name = NULL;
const struct rproc_ops *ops = &qcom_pas_ops;
int ret;
@@ -847,6 +1021,24 @@ static int qcom_pas_probe(struct platform_device *pdev)
return ret;
}
+ if (desc->secondary_firmware_name) {
+ secondary_fw_name = desc->secondary_firmware_name;
+ ret = of_property_read_string_index(pdev->dev.of_node,
+ "firmware-name", 2,
+ &secondary_fw_name);
+ if (ret < 0 && ret != -EINVAL)
+ return ret;
+ }
+
+ if (desc->secondary_dtb_firmware_name) {
+ secondary_dtb_fw_name = desc->secondary_dtb_firmware_name;
+ ret = of_property_read_string_index(pdev->dev.of_node,
+ "firmware-name", 3,
+ &secondary_dtb_fw_name);
+ if (ret < 0 && ret != -EINVAL)
+ return ret;
+ }
+
if (desc->minidump_id)
ops = &qcom_pas_minidump_ops;
@@ -879,6 +1071,14 @@ static int qcom_pas_probe(struct platform_device *pdev)
pas->dtb_firmware_name = dtb_fw_name;
pas->dtb_pas_id = desc->dtb_pas_id;
}
+ if (secondary_fw_name) {
+ pas->secondary_firmware_name = secondary_fw_name;
+ pas->secondary_pas_id = desc->secondary_pas_id;
+ }
+ if (secondary_dtb_fw_name) {
+ pas->secondary_dtb_firmware_name = secondary_dtb_fw_name;
+ pas->secondary_dtb_pas_id = desc->secondary_dtb_pas_id;
+ }
platform_set_drvdata(pdev, pas);
ret = device_init_wakeup(pas->dev, true);
@@ -925,6 +1125,10 @@ static int qcom_pas_probe(struct platform_device *pdev)
pas->pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
if (pas->dtb_pas_id)
pas->dtb_pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
+ if (pas->secondary_pas_id)
+ pas->secondary_pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
+ if (pas->secondary_dtb_pas_id)
+ pas->secondary_dtb_pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
if (desc->early_boot)
pas->rproc->state = RPROC_DETACHED;
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/4] remoteproc: qcom: pas: Support secondary images
2026-10-15 5:55 ` [PATCH v2 2/4] remoteproc: qcom: pas: Support secondary images Anvesh Jain P
@ 2026-10-15 6:05 ` sashiko-bot
2026-10-15 9:57 ` Anvesh Jain P
0 siblings, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-10-15 6:05 UTC (permalink / raw)
To: Anvesh Jain P; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] qcom_q6v5_pas: NULL pointer dereference due to early returns bypassing context allocations
- [Medium] qcom_q6v5_pas: Memory leak of firmware objects on successful load
--
commit 3f3270df945541b7e62ff38a77d04e462e6bf823
Author: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
remoteproc: qcom: pas: Support secondary images
The PAS remoteproc driver currently supports one primary image and its
optional DTB. OOBMSS additionally requires the secondary image and DTB
to be loaded, authenticated, mapped, shut down, and released through
their own PAS identifiers and carveouts.
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index ca8e61254c449..0bce2f0d5b4aa 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
[ ... ]
> @@ -241,19 +281,29 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
> if (pas->lite_dtb_pas_id)
> qcom_pas_shutdown(pas->lite_dtb_pas_id);
>
> - if (pas->dtb_pas_id) {
> - ret = request_firmware(&pas->dtb_firmware, pas->dtb_firmware_name, pas->dev);
> + if (pas->secondary_dtb_pas_id) {
> + ret = request_firmware(&pas->secondary_dtb_firmware,
> + pas->secondary_dtb_firmware_name, pas->dev);
> + if (ret) {
> + qcom_pas_release_firmware(pas);
> + return ret;
> + }
> + }
> +
> + if (pas->secondary_pas_id) {
> + ret = request_firmware(&pas->secondary_firmware,
> + pas->secondary_firmware_name, pas->dev);
> if (ret) {
> - dev_err(pas->dev, "request_firmware failed for %s: %d\n",
> - pas->dtb_firmware_name, ret);
> + qcom_pas_release_firmware(pas);
> return ret;
> }
> + }
>
> - ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
> - pas->dtb_firmware_name, &pas->dtb_mem_reloc);
> + if (pas->dtb_pas_id) {
> + ret = request_firmware(&pas->dtb_firmware, pas->dtb_firmware_name,
> + pas->dev);
> if (ret) {
> - qcom_pas_metadata_release(pas->dtb_pas_ctx);
> - release_firmware(pas->dtb_firmware);
> + qcom_pas_release_firmware(pas);
> return ret;
> }
> }
[Severity: Medium]
When qcom_pas_load() completes successfully, it returns 0 without calling
release_firmware() or qcom_pas_release_firmware(). Could this leak the
firmware objects and their underlying buffers?
[ ... ]
> @@ -344,13 +435,16 @@ static int qcom_pas_start(struct rproc *rproc)
> ret = qcom_q6v5_wait_for_start(&pas->q6v5, msecs_to_jiffies(5000));
> if (ret == -ETIMEDOUT) {
> dev_err(pas->dev, "start timed out\n");
> - qcom_pas_shutdown(pas->pas_id);
> goto unmap_carveout;
> }
>
> qcom_pas_metadata_release(pas->pas_ctx);
> if (pas->dtb_pas_id)
> qcom_pas_metadata_release(pas->dtb_pas_ctx);
> + if (pas->secondary_pas_id)
> + qcom_pas_metadata_release(pas->secondary_pas_ctx);
> + if (pas->secondary_dtb_pas_id)
> + qcom_pas_metadata_release(pas->secondary_dtb_pas_ctx);
>
> /* firmware is used to pass reference from qcom_pas_start(), drop it now */
> pas->firmware = NULL;
> @@ -358,16 +452,37 @@ static int qcom_pas_start(struct rproc *rproc)
> return 0;
[Severity: Medium]
Similarly, in the success path of qcom_pas_start(), should there be a call
to release the secondary_dtb_firmware, secondary_firmware, and dtb_firmware
objects before returning 0?
[ ... ]
> @@ -740,6 +876,42 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
> if (IS_ERR(pas->dtb_pas_ctx))
> return PTR_ERR(pas->dtb_pas_ctx);
>
> + if (!pas->secondary_pas_id)
> + return 0;
[Severity: High]
In qcom_pas_alloc_memory_region() (just before this diff hunk), there is an
early return:
if (!pas->dtb_pas_id)
return 0;
If a platform defines a secondary_pas_id but lacks a primary dtb_pas_id, will
this early return prevent the allocation of the secondary contexts?
> +
> + ret = of_reserved_mem_region_to_resource(pas->dev->of_node, 2, &res);
> + if (ret)
> + return ret;
> +
> + pas->secondary_mem_phys = res.start;
> + pas->secondary_mem_reloc = res.start;
> + pas->secondary_mem_size = resource_size(&res);
> +
> + pas->secondary_pas_ctx =
> + devm_qcom_pas_context_alloc(pas->dev, pas->secondary_pas_id,
> + pas->secondary_mem_phys,
> + pas->secondary_mem_size);
> + if (IS_ERR(pas->secondary_pas_ctx))
> + return PTR_ERR(pas->secondary_pas_ctx);
[ ... ]
> @@ -925,6 +1125,10 @@ static int qcom_pas_probe(struct platform_device *pdev)
> pas->pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
> if (pas->dtb_pas_id)
> pas->dtb_pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
> + if (pas->secondary_pas_id)
> + pas->secondary_pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
[Severity: High]
If qcom_pas_alloc_memory_region() returned early because dtb_pas_id was
missing, could pas->secondary_pas_ctx be NULL here, leading to a NULL pointer
dereference when setting use_tzmem?
> + if (pas->secondary_dtb_pas_id)
> + pas->secondary_dtb_pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
>
> if (desc->early_boot)
> pas->rproc->state = RPROC_DETACHED;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-oob-support-glymur-v2-0-715435f52dd7@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/4] remoteproc: qcom: pas: Support secondary images
2026-10-15 6:05 ` sashiko-bot
@ 2026-10-15 9:57 ` Anvesh Jain P
0 siblings, 0 replies; 9+ messages in thread
From: Anvesh Jain P @ 2026-10-15 9:57 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, robh, conor+dt
On 10/15/2026 11:35 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] qcom_q6v5_pas: NULL pointer dereference due to early returns bypassing context allocations
> - [Medium] qcom_q6v5_pas: Memory leak of firmware objects on successful load
> --
>
> commit 3f3270df945541b7e62ff38a77d04e462e6bf823
> Author: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
>
> remoteproc: qcom: pas: Support secondary images
>
> The PAS remoteproc driver currently supports one primary image and its
> optional DTB. OOBMSS additionally requires the secondary image and DTB
> to be loaded, authenticated, mapped, shut down, and released through
> their own PAS identifiers and carveouts.
>
>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
>> index ca8e61254c449..0bce2f0d5b4aa 100644
>> --- a/drivers/remoteproc/qcom_q6v5_pas.c
>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
>
> [ ... ]
>
>> @@ -241,19 +281,29 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
>> if (pas->lite_dtb_pas_id)
>> qcom_pas_shutdown(pas->lite_dtb_pas_id);
>>
>> - if (pas->dtb_pas_id) {
>> - ret = request_firmware(&pas->dtb_firmware, pas->dtb_firmware_name, pas->dev);
>> + if (pas->secondary_dtb_pas_id) {
>> + ret = request_firmware(&pas->secondary_dtb_firmware,
>> + pas->secondary_dtb_firmware_name, pas->dev);
>> + if (ret) {
>> + qcom_pas_release_firmware(pas);
>> + return ret;
>> + }
>> + }
>> +
>> + if (pas->secondary_pas_id) {
>> + ret = request_firmware(&pas->secondary_firmware,
>> + pas->secondary_firmware_name, pas->dev);
>> if (ret) {
>> - dev_err(pas->dev, "request_firmware failed for %s: %d\n",
>> - pas->dtb_firmware_name, ret);
>> + qcom_pas_release_firmware(pas);
>> return ret;
>> }
>> + }
>>
>> - ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
>> - pas->dtb_firmware_name, &pas->dtb_mem_reloc);
>> + if (pas->dtb_pas_id) {
>> + ret = request_firmware(&pas->dtb_firmware, pas->dtb_firmware_name,
>> + pas->dev);
>> if (ret) {
>> - qcom_pas_metadata_release(pas->dtb_pas_ctx);
>> - release_firmware(pas->dtb_firmware);
>> + qcom_pas_release_firmware(pas);
>> return ret;
>> }
>> }
>
> [Severity: Medium]
> When qcom_pas_load() completes successfully, it returns 0 without calling
> release_firmware() or qcom_pas_release_firmware(). Could this leak the
> firmware objects and their underlying buffers?
>
The auxiliary firmware is used later by qcom_pas_start().
I will release it after the final qcom_mdt_pas_load() and on error
paths.
> [ ... ]
>
>> @@ -344,13 +435,16 @@ static int qcom_pas_start(struct rproc *rproc)
>> ret = qcom_q6v5_wait_for_start(&pas->q6v5, msecs_to_jiffies(5000));
>> if (ret == -ETIMEDOUT) {
>> dev_err(pas->dev, "start timed out\n");
>> - qcom_pas_shutdown(pas->pas_id);
>> goto unmap_carveout;
>> }
>>
>> qcom_pas_metadata_release(pas->pas_ctx);
>> if (pas->dtb_pas_id)
>> qcom_pas_metadata_release(pas->dtb_pas_ctx);
>> + if (pas->secondary_pas_id)
>> + qcom_pas_metadata_release(pas->secondary_pas_ctx);
>> + if (pas->secondary_dtb_pas_id)
>> + qcom_pas_metadata_release(pas->secondary_dtb_pas_ctx);
>>
>> /* firmware is used to pass reference from qcom_pas_start(), drop it now */
>> pas->firmware = NULL;
>> @@ -358,16 +452,37 @@ static int qcom_pas_start(struct rproc *rproc)
>> return 0;
>
> [Severity: Medium]
> Similarly, in the success path of qcom_pas_start(), should there be a call
> to release the secondary_dtb_firmware, secondary_firmware, and dtb_firmware
> objects before returning 0?
>
Yes, I can release firmware before returning.
> [ ... ]
>
>> @@ -740,6 +876,42 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
>> if (IS_ERR(pas->dtb_pas_ctx))
>> return PTR_ERR(pas->dtb_pas_ctx);
>>
>> + if (!pas->secondary_pas_id)
>> + return 0;
>
> [Severity: High]
> In qcom_pas_alloc_memory_region() (just before this diff hunk), there is an
> early return:
>
> if (!pas->dtb_pas_id)
> return 0;
>
> If a platform defines a secondary_pas_id but lacks a primary dtb_pas_id, will
> this early return prevent the allocation of the secondary contexts?
>
>> +
>> + ret = of_reserved_mem_region_to_resource(pas->dev->of_node, 2, &res);
>> + if (ret)
>> + return ret;
>> +
>> + pas->secondary_mem_phys = res.start;
>> + pas->secondary_mem_reloc = res.start;
>> + pas->secondary_mem_size = resource_size(&res);
>> +
>> + pas->secondary_pas_ctx =
>> + devm_qcom_pas_context_alloc(pas->dev, pas->secondary_pas_id,
>> + pas->secondary_mem_phys,
>> + pas->secondary_mem_size);
>> + if (IS_ERR(pas->secondary_pas_ctx))
>> + return PTR_ERR(pas->secondary_pas_ctx);
>
> [ ... ]
>
>> @@ -925,6 +1125,10 @@ static int qcom_pas_probe(struct platform_device *pdev)
>> pas->pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
>> if (pas->dtb_pas_id)
>> pas->dtb_pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
>> + if (pas->secondary_pas_id)
>> + pas->secondary_pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
>
> [Severity: High]
> If qcom_pas_alloc_memory_region() returned early because dtb_pas_id was
> missing, could pas->secondary_pas_ctx be NULL here, leading to a NULL pointer
> dereference when setting use_tzmem?
>
I will add condition for allocation instead of returning, so that we
won't return early.
>> + if (pas->secondary_dtb_pas_id)
>> + pas->secondary_dtb_pas_ctx->use_tzmem = desc->needs_tzmem || rproc->has_iommu;
>>
>> if (desc->early_boot)
>> pas->rproc->state = RPROC_DETACHED;
>
--
Best Regards,
Anvesh
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/4] remoteproc: qcom: pas: Add coredump support for secondary processors
2026-10-15 5:55 [PATCH v2 0/4] Add Glymur OOBMSS support Anvesh Jain P
2026-10-15 5:55 ` [PATCH v2 1/4] dt-bindings: remoteproc: qcom: Add Glymur OOBMSS PAS Anvesh Jain P
2026-10-15 5:55 ` [PATCH v2 2/4] remoteproc: qcom: pas: Support secondary images Anvesh Jain P
@ 2026-10-15 5:55 ` Anvesh Jain P
2026-10-15 6:13 ` sashiko-bot
2026-10-15 5:55 ` [PATCH v2 4/4] remoteproc: qcom: pas: Add Glymur OOBMSS PAS parameters Anvesh Jain P
3 siblings, 1 reply; 9+ messages in thread
From: Anvesh Jain P @ 2026-10-15 5:55 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Sibi Sankar, Anvesh Jain P
Add coredump support for PAS remoteproc instances with a secondary
processor by registering fixed dump segments for both primary and
secondary carveouts at probe time and mapping them during collection.
The attached-state path is for debug dump collection and does not
provide operational device recovery.
Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
---
drivers/remoteproc/qcom_q6v5_pas.c | 102 +++++++++++++++++++++++++++++++++----
1 file changed, 91 insertions(+), 11 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 0bce2f0d5b4a..374cafd04b62 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -117,6 +117,7 @@ struct qcom_pas {
phys_addr_t region_assign_phys[MAX_ASSIGN_COUNT];
void __iomem *mem_region;
+ void __iomem *secondary_mem_region;
size_t mem_size;
size_t dtb_mem_size;
@@ -162,6 +163,28 @@ static void qcom_pas_segment_dump(struct rproc *rproc,
memcpy_fromio(dest, pas->mem_region + total_offset, size);
}
+static bool qcom_pas_has_secondary(const struct qcom_pas *pas)
+{
+ return pas->secondary_pas_id && pas->secondary_dtb_pas_id;
+}
+
+static int qcom_pas_register_dump_segments(struct rproc *rproc,
+ struct qcom_pas *pas)
+{
+ int ret;
+
+ ret = rproc_coredump_add_segment(rproc, pas->mem_phys, pas->mem_size);
+ if (ret)
+ return ret;
+
+ ret = rproc_coredump_add_segment(rproc, pas->secondary_mem_phys,
+ pas->secondary_mem_size);
+ if (ret)
+ rproc_coredump_cleanup(rproc);
+
+ return ret;
+}
+
static void qcom_pas_minidump(struct rproc *rproc)
{
struct qcom_pas *pas = rproc->priv;
@@ -577,16 +600,38 @@ static int qcom_pas_stop(struct rproc *rproc)
static void *qcom_pas_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is_iomem)
{
struct qcom_pas *pas = rproc->priv;
- int offset;
+ phys_addr_t base;
+ void __iomem *region;
+
+ if (da >= pas->mem_reloc && len <= pas->mem_size &&
+ da - pas->mem_reloc <= pas->mem_size - len) {
+ base = pas->mem_reloc;
+ region = pas->mem_region;
+ } else if (da >= pas->mem_phys && len <= pas->mem_size &&
+ da - pas->mem_phys <= pas->mem_size - len) {
+ base = pas->mem_phys;
+ region = pas->mem_region;
+ } else if (da >= pas->secondary_mem_reloc &&
+ len <= pas->secondary_mem_size &&
+ da - pas->secondary_mem_reloc <= pas->secondary_mem_size - len) {
+ base = pas->secondary_mem_reloc;
+ region = pas->secondary_mem_region;
+ } else if (da >= pas->secondary_mem_phys &&
+ len <= pas->secondary_mem_size &&
+ da - pas->secondary_mem_phys <= pas->secondary_mem_size - len) {
+ base = pas->secondary_mem_phys;
+ region = pas->secondary_mem_region;
+ } else {
+ return NULL;
+ }
- offset = da - pas->mem_reloc;
- if (offset < 0 || offset + len > pas->mem_size)
+ if (!region)
return NULL;
if (is_iomem)
*is_iomem = true;
- return (__force void *)pas->mem_region + offset;
+ return (__force void *)region + (da - base);
}
static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *fw)
@@ -598,10 +643,21 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f
size_t table_sz;
int ret;
- ret = qcom_register_dump_segments(rproc, fw);
- if (ret) {
- dev_err(pas->dev, "Error in registering dump segments\n");
- return ret;
+ if (qcom_pas_has_secondary(pas)) {
+ if (list_empty(&rproc->dump_segments)) {
+ ret = qcom_pas_register_dump_segments(rproc, pas);
+ if (ret) {
+ dev_err(pas->dev,
+ "failed to register dump segments: %d\n", ret);
+ return ret;
+ }
+ }
+ } else {
+ ret = qcom_register_dump_segments(rproc, fw);
+ if (ret) {
+ dev_err(pas->dev, "Error in registering dump segments\n");
+ return ret;
+ }
}
if (!rproc->has_iommu)
@@ -716,13 +772,21 @@ static void qcom_pas_coredump(struct rproc *rproc)
{
struct qcom_pas *pas = rproc->priv;
- pas->mem_region = qcom_pas_ctx_map(pas->pas_ctx);
+ if (!pas->mem_region)
+ pas->mem_region = qcom_pas_ctx_map(pas->pas_ctx);
if (!pas->mem_region)
return;
+ if (qcom_pas_has_secondary(pas) && !pas->secondary_mem_region) {
+ pas->secondary_mem_region =
+ qcom_pas_ctx_map(pas->secondary_pas_ctx);
+ if (!pas->secondary_mem_region) {
+ dev_err(pas->dev, "unable to map secondary memory for coredump\n");
+ return;
+ }
+ }
+
rproc_coredump(rproc);
- iounmap(pas->mem_region);
- pas->mem_region = NULL;
}
static const struct rproc_ops qcom_pas_ops = {
@@ -1133,6 +1197,12 @@ static int qcom_pas_probe(struct platform_device *pdev)
if (desc->early_boot)
pas->rproc->state = RPROC_DETACHED;
+ if (qcom_pas_has_secondary(pas)) {
+ ret = qcom_pas_register_dump_segments(rproc, pas);
+ if (ret)
+ goto remove_ssr_sysmon;
+ }
+
ret = rproc_add(rproc);
if (ret)
goto remove_ssr_sysmon;
@@ -1144,6 +1214,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
return 0;
remove_ssr_sysmon:
+ rproc_coredump_cleanup(rproc);
qcom_remove_ssr_subdev(rproc, &pas->ssr_subdev);
qcom_remove_sysmon_subdev(pas->sysmon);
deinit_remove_pdm_smd_glink:
@@ -1169,6 +1240,15 @@ static void qcom_pas_remove(struct platform_device *pdev)
of_platform_device_destroy(&pas->bam_dmux->dev, NULL);
rproc_del(pas->rproc);
+ rproc_coredump_cleanup(pas->rproc);
+ if (pas->secondary_mem_region) {
+ iounmap(pas->secondary_mem_region);
+ pas->secondary_mem_region = NULL;
+ }
+ if (pas->mem_region) {
+ iounmap(pas->mem_region);
+ pas->mem_region = NULL;
+ }
qcom_q6v5_deinit(&pas->q6v5);
qcom_pas_unassign_memory_region(pas);
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 3/4] remoteproc: qcom: pas: Add coredump support for secondary processors
2026-10-15 5:55 ` [PATCH v2 3/4] remoteproc: qcom: pas: Add coredump support for secondary processors Anvesh Jain P
@ 2026-10-15 6:13 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-10-15 6:13 UTC (permalink / raw)
To: Anvesh Jain P; +Cc: devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] remoteproc: qcom: coredump completely aborted if secondary memory map fails
- [High] mm: UAF of rproc in qcom_pas_probe() if rproc_add() fails
--
commit 78eeb93e0081fe18e5c14c65d05d13fbc15c17c8
Author: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
remoteproc: qcom: pas: Add coredump support for secondary processors
Add coredump support for PAS remoteproc instances with a secondary
processor by registering fixed dump segments for both primary and
secondary carveouts at probe time and mapping them during collection.
The attached-state path is for debug dump collection and does not
provide operational device recovery.
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 0bce2f0d5b4aa..374cafd04b625 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
[ ... ]
> @@ -716,13 +772,21 @@ static void qcom_pas_coredump(struct rproc *rproc)
> {
> struct qcom_pas *pas = rproc->priv;
>
> - pas->mem_region = qcom_pas_ctx_map(pas->pas_ctx);
> + if (!pas->mem_region)
> + pas->mem_region = qcom_pas_ctx_map(pas->pas_ctx);
> if (!pas->mem_region)
> return;
>
> + if (qcom_pas_has_secondary(pas) && !pas->secondary_mem_region) {
> + pas->secondary_mem_region =
> + qcom_pas_ctx_map(pas->secondary_pas_ctx);
> + if (!pas->secondary_mem_region) {
> + dev_err(pas->dev, "unable to map secondary memory for coredump\n");
> + return;
> + }
> + }
> +
> rproc_coredump(rproc);
[Severity: High]
If the secondary memory mapping fails here, does the early return
unintentionally bypass the rproc_coredump() call completely?
Since the primary processor memory was successfully mapped just above,
and the remoteproc framework's segment copy gracefully handles unmapped
segments, should it log the error and still proceed to dump the primary
core's memory rather than aborting?
[ ... ]
> @@ -1133,6 +1197,12 @@ static int qcom_pas_probe(struct platform_device *pdev)
> if (desc->early_boot)
> pas->rproc->state = RPROC_DETACHED;
>
> + if (qcom_pas_has_secondary(pas)) {
> + ret = qcom_pas_register_dump_segments(rproc, pas);
> + if (ret)
> + goto remove_ssr_sysmon;
> + }
> +
> ret = rproc_add(rproc);
> if (ret)
> goto remove_ssr_sysmon;
> @@ -1144,6 +1214,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
> return 0;
>
> remove_ssr_sysmon:
> + rproc_coredump_cleanup(rproc);
[Severity: High]
Can this sequence cause a use-after-free if rproc_add() fails?
Looking at rproc_add(), if device_add() fails internally, it explicitly
calls put_device() which drops the initial reference created by
devm_rproc_alloc(), freeing the rproc structure:
rproc_add() {
...
ret = device_add(dev);
if (ret < 0) {
put_device(dev);
goto rproc_remove_cdev;
}
...
}
If we jump to remove_ssr_sysmon on rproc_add() failure, doesn't
rproc_coredump_cleanup() then try to access the already freed
rproc structure?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-oob-support-glymur-v2-0-715435f52dd7@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 4/4] remoteproc: qcom: pas: Add Glymur OOBMSS PAS parameters
2026-10-15 5:55 [PATCH v2 0/4] Add Glymur OOBMSS support Anvesh Jain P
` (2 preceding siblings ...)
2026-10-15 5:55 ` [PATCH v2 3/4] remoteproc: qcom: pas: Add coredump support for secondary processors Anvesh Jain P
@ 2026-10-15 5:55 ` Anvesh Jain P
3 siblings, 0 replies; 9+ messages in thread
From: Anvesh Jain P @ 2026-10-15 5:55 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio
Cc: linux-arm-msm, linux-remoteproc, devicetree, linux-kernel,
Sibi Sankar, Anvesh Jain P
The OOBMSS consists of separate OOB-TEE and OOB-NS Zephyr subsystems,
each with its own firmware, DTB, and dedicated PAS ID. Add support for
the OOBMSS PAS loader on Glymur platforms.
Signed-off-by: Anvesh Jain P <anvesh.p@oss.qualcomm.com>
---
drivers/remoteproc/qcom_q6v5_pas.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 374cafd04b62..ecefc6a72fd2 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -2080,6 +2080,30 @@ static const struct qcom_pas_data glymur_soccp_resource = {
.needs_tzmem = true,
};
+static const struct qcom_pas_data glymur_oobmss_resource = {
+ .crash_reason_smem = 669,
+ .firmware_name = "oobmss_tee.mbn",
+ .dtb_firmware_name = "oobmss_qc_dtb.mbn",
+ .secondary_firmware_name = "oobmss_ns.mbn",
+ .secondary_dtb_firmware_name = "oobmss_oem_dtb.mbn",
+ .pas_id = 0x42,
+ .dtb_pas_id = 0x43,
+ .secondary_pas_id = 0x44,
+ .secondary_dtb_pas_id = 0x45,
+ .proxy_pd_names = (char *[]){
+ "cx",
+ "mx",
+ NULL
+ },
+ .ssr_name = "oobmss",
+ .sysmon_name = "oobmss",
+ .ssctl_id = 0x21,
+ .auto_boot = true,
+ .early_boot = true,
+ .needs_tzmem = true,
+ .decrypt_shutdown = true,
+};
+
static const struct qcom_pas_data eliza_cdsp_resource = {
.crash_reason_smem = 601,
.firmware_name = "cdsp.mbn",
@@ -2108,6 +2132,7 @@ static const struct qcom_pas_data eliza_cdsp_resource = {
static const struct of_device_id qcom_pas_of_match[] = {
{ .compatible = "qcom,eliza-adsp-pas", .data = &sm8550_adsp_resource },
{ .compatible = "qcom,eliza-cdsp-pas", .data = &eliza_cdsp_resource },
+ { .compatible = "qcom,glymur-oobmss-pas", .data = &glymur_oobmss_resource },
{ .compatible = "qcom,glymur-soccp-pas", .data = &glymur_soccp_resource },
{ .compatible = "qcom,kaanapali-soccp-pas", .data = &kaanapali_soccp_resource },
{ .compatible = "qcom,milos-adsp-pas", .data = &sm8550_adsp_resource },
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread