[PATCH 0/8] remoteproc: qcom: Support the Nord HPASS ADSP cluster

[PATCH 0/8] remoteproc: qcom: Support the Nord HPASS ADSP cluster

[Shawn Guo]

The HPASS audio subsystem on the Qualcomm Nord SoC has three QDSP6
instances, ADSP0/1/2. They are not independent: ADSP0 owns the resources
the other two need (PLLs, AG_NOC, RSCp, CESTA, THROTTLE, QTMR) and
initializes them as part of its own boot, so ADSP1/ADSP2 can only cold
boot once ADSP0 is up. Their firmware also cannot tolerate one instance
going down on its own -- stopping or restarting a single member leaves
sysmon/glink_ssr notify requests to the siblings timing out, and the
whole cluster unable to recover without manually stopping all three and
restarting them in dependency order.

This series models that constraint. A new "qcom,cluster-root" phandle
groups the instances; every member carries it, the resource-owning
instance included, whose phandle references itself, so both cluster
membership and root-ness are answerable from the node being probed alone.
Instances without the property -- which is every PAS instance on every
other SoC -- keep qcom_pas.cluster NULL and are completely unaffected.

Two invariants fall out of the hardware, and are implemented
independently of each other:

 - Root-first bring-up. A dependent member's start blocks until the root
   has booted.

 - Coupled, two-phase teardown. Stopping or crashing any member takes
   the whole cluster down. Hardware testing showed that whichever
   member's PAS/TZ power-off happens chronologically last always fails
   its own graceful shutdown ack -- both the sysmon SSCTL QMI ack and
   the SMEM-bit/IRQ ack -- regardless of whether that member is the
   root. So phase 1 fans the stop out to every participant concurrently
   and waits at a barrier for all of them to finish requesting and
   awaiting their own ack while the cluster is still fully powered; only
   past the barrier does phase 2 let each member actually power its
   hardware off. That way no member is ever asking firmware to ack a
   shutdown after a sibling's hardware is already gone. On a crash the
   cluster is then auto-restarted root-first; an explicit stop leaves
   the ordered restart to higher-level policy.

The remaining wrinkle is that a cluster member must not send SSR notify
to its siblings, whose firmware cannot handle it, while still notifying
non-cluster peers such as mpss and cdsp. Both notify paths therefore
suppress receiver-side on a cluster id: sysmon (in remoteproc) and
glink_ssr (in rpmsg). Since RPMSG_QCOM_GLINK does not and should not
depend on REMOTEPROC, the id is a plain opaque "void *" cookie on struct
rproc, plumbed down into the glink edge rather than looked up through
struct rproc from the rpmsg side.

Shawn Guo (8):
  dt-bindings: remoteproc: qcom,nord-pas: Add qcom,cluster-root property
  remoteproc: Add cluster field to struct rproc
  remoteproc: qcom: sysmon: Suppress notify between cluster siblings
  rpmsg: qcom_glink_ssr: Suppress cleanup notify between cluster
    siblings
  remoteproc: qcom: pas: Track HPASS ADSP cluster membership
  remoteproc: qcom: pas: Sequence HPASS ADSP cluster boot root-first
  remoteproc: qcom: pas: Enforce coupled stop/crash for HPASS ADSP
    clusters
  remoteproc: qcom: pas: Add Nord ADSP1/2 support

 .../bindings/remoteproc/qcom,nord-pas.yaml    |  17 +
 drivers/remoteproc/qcom_common.c              |   7 +-
 drivers/remoteproc/qcom_common.h              |   2 +-
 drivers/remoteproc/qcom_q6v5_pas.c            | 524 ++++++++++++++++++
 drivers/remoteproc/qcom_sysmon.c              |  18 +-
 drivers/rpmsg/qcom_glink_native.c             |  20 +-
 drivers/rpmsg/qcom_glink_native.h             |   5 +-
 drivers/rpmsg/qcom_glink_rpm.c                |   2 +-
 drivers/rpmsg/qcom_glink_smem.c               |   5 +-
 drivers/rpmsg/qcom_glink_ssr.c                |  36 +-
 include/linux/remoteproc.h                    |   3 +
 include/linux/rpmsg/qcom_glink.h              |  10 +-
 12 files changed, 627 insertions(+), 22 deletions(-)

-- 
2.43.0

[PATCH 1/8] dt-bindings: remoteproc: qcom,nord-pas: Add qcom,cluster-root property

[Shawn Guo]

Document the qcom,cluster-root phandle property used to describe HPASS's
ADSP0/1/2 cluster relationship. Every member of a cluster carries the
property, including the instance owning the shared resources, which
references itself; instances referencing the same node form one cluster
that boots in order and is torn down as a single unit.

The relationship is described with a phandle rather than by containment
in a parent node, as ti,k3-r5f-rproc and xlnx,zynqmp-r5fss do, because
the hardware does not express it by containment either: the three QDSP6SS
blocks are register-independent, with no shared register window for a
container node to describe.

Also add the qcom,nord-adsp1-pas and qcom,nord-adsp2-pas compatible
strings used by the non-root cluster members.

Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 .../bindings/remoteproc/qcom,nord-pas.yaml      | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml
index e90d2953ba69..58d88cb07e52 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml
@@ -17,6 +17,8 @@ properties:
   compatible:
     enum:
       - qcom,nord-adsp-pas
+      - qcom,nord-adsp1-pas
+      - qcom,nord-adsp2-pas
       - qcom,nord-cdsp0-pas
       - qcom,nord-cdsp1-pas
       - qcom,nord-cdsp2-pas
@@ -89,6 +91,19 @@ properties:
     maxItems: 1
     description: The names of the state bits used for SMP2P output
 
+  qcom,cluster-root:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Reference to the Peripheral Authentication Service instance that owns the
+      resources shared across this DSP's cluster. HPASS shares clock/reset/NoC
+      resources between its ADSP0/1/2 QDSP6 instances, and the owning instance
+      (ADSP0) has to boot first to initialize them before ADSP1/ADSP2 can cold
+      boot. Every member of a cluster carries this property, the owning instance
+      included, which references itself; instances whose property references the
+      same node form one cluster. A cluster is always brought down as a single
+      unit; any member crashing or being stopped forces every other member to
+      crash or stop with it.
+
 required:
   - compatible
   - reg
@@ -101,6 +116,8 @@ allOf:
         compatible:
           enum:
             - qcom,nord-adsp-pas
+            - qcom,nord-adsp1-pas
+            - qcom,nord-adsp2-pas
     then:
       properties:
         power-domains:
-- 
2.43.0

Re: [PATCH 1/8] dt-bindings: remoteproc: qcom,nord-pas: Add qcom,cluster-root property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `qcom,cluster-root` property is strictly required for ADSP cluster members but is omitted from the `required` schema list, and the example is not updated.
--

--- Patch [1]: [PATCH 1/8] dt-bindings: remoteproc: qcom,nord-pas: Add qcom,cluster-root property ---
commit fca9b39c4393ef8848adae4c6b55bac6904bfd95
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

dt-bindings: remoteproc: qcom,nord-pas: Add qcom,cluster-root property

Document the qcom,cluster-root phandle property used to describe HPASS's
ADSP0/1/2 cluster relationship. Every member of a cluster carries the
property, including the instance owning the shared resources, which
references itself; instances referencing the same node form one cluster
that boots in order and is torn down as a single unit.

> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml
[ ... ]
> @@ -101,6 +116,8 @@ allOf:
>          compatible:
>            enum:
>              - qcom,nord-adsp-pas
> +            - qcom,nord-adsp1-pas
> +            - qcom,nord-adsp2-pas
>      then:
>        properties:
>          power-domains:

[Severity: Medium]
Since qcom,cluster-root is strictly required for ADSP cluster members,
should it be added to the required list in this conditional block?

If an ADSP node is created without the property, it will silently pass
make dt_binding_check. At runtime, a missing qcom,cluster-root property can
cause the DSP driver to fail to establish the cluster relationship,
potentially hanging the boot process of ADSP1/ADSP2.

Additionally, the existing example node at the end of the file lacks the
qcom,cluster-root property:

Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml:
examples:
  - |
    ...
    remoteproc@4c00000 {
        compatible = "qcom,nord-adsp-pas";
        reg = <0x04c00000 0x10000>;
    ...

Could the example be updated to include a self-referencing
qcom,cluster-root property to conform to this new requirement?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815132541.1575121-1-shengchao.guo@oss.qualcomm.com?part=1

[PATCH 2/8] remoteproc: Add cluster field to struct rproc

[Shawn Guo]

Some Qualcomm SoCs group multiple remote processors into a cluster
(e.g. Nord's ADSP0/1/2, which share HPASS-domain PLLs, AG_NOC, RSCp
and other resources owned by ADSP0). Firmware on these clusters cannot
tolerate the peer-shutdown notifications that qcom_sysmon and
qcom_glink_ssr send between independently started/stopped remote
processors.

Add an opaque cluster identifier to struct rproc so that those
notification paths, which live in different subsystems (remoteproc
and rpmsg), can recognize siblings in the same cluster and suppress
notifications between them, without introducing a module dependency
between the two subsystems.

Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 include/linux/remoteproc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index a44368737b39..53e56d53641e 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -254,6 +254,8 @@ enum rproc_features {
  * @cdev: character device of the rproc
  * @cdev_put_on_release: flag to indicate if remoteproc should be shutdown on @char_dev release
  * @features: indicate remoteproc features
+ * @cluster: opaque identifier shared by remoteprocs whose SSR notifications
+ *	     should be coalesced (set by the owning rproc driver), or NULL
  */
 struct rproc {
 	struct list_head node;
@@ -299,6 +301,7 @@ struct rproc {
 	struct cdev cdev;
 	bool cdev_put_on_release;
 	DECLARE_BITMAP(features, RPROC_MAX_FEATURES);
+	void *cluster;
 };
 
 /**
-- 
2.43.0

[PATCH 3/8] remoteproc: qcom: sysmon: Suppress notify between cluster siblings

[Shawn Guo]

Firmware on clustered remote processors (e.g. Nord's HPASS ADSP0/1/2)
cannot handle the SSCTL peer-shutdown notification sysmon sends
whenever a sibling in the same cluster starts or stops, and the QMI
request instead times out.

Skip sysmon_notify() when the notifying and the notified instance share
the cluster identifier their owning rproc driver put in rproc->cluster.
The suppression is done on the receiving side, so that a cluster member
keeps notifying, and being notified by, remote processors outside its
cluster. Instances whose rproc has no cluster set are completely
unaffected.

Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_sysmon.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
index 61e1038328e8..5d4443ef3bc1 100644
--- a/drivers/remoteproc/qcom_sysmon.c
+++ b/drivers/remoteproc/qcom_sysmon.c
@@ -67,6 +67,7 @@ static const char * const sysmon_state_string[] = {
 struct sysmon_event {
 	const char *subsys_name;
 	u32 ssr_event;
+	void *cluster;
 };
 
 static DEFINE_MUTEX(sysmon_lock);
@@ -473,7 +474,8 @@ static int sysmon_prepare(struct rproc_subdev *subdev)
 						  subdev);
 	struct sysmon_event event = {
 		.subsys_name = sysmon->name,
-		.ssr_event = SSCTL_SSR_EVENT_BEFORE_POWERUP
+		.ssr_event = SSCTL_SSR_EVENT_BEFORE_POWERUP,
+		.cluster = sysmon->rproc->cluster,
 	};
 
 	mutex_lock(&sysmon->state_lock);
@@ -500,7 +502,8 @@ static int sysmon_start(struct rproc_subdev *subdev)
 	struct qcom_sysmon *target;
 	struct sysmon_event event = {
 		.subsys_name = sysmon->name,
-		.ssr_event = SSCTL_SSR_EVENT_AFTER_POWERUP
+		.ssr_event = SSCTL_SSR_EVENT_AFTER_POWERUP,
+		.cluster = sysmon->rproc->cluster,
 	};
 
 	reinit_completion(&sysmon->ssctl_comp);
@@ -536,7 +539,8 @@ static void sysmon_stop(struct rproc_subdev *subdev, bool crashed)
 	struct qcom_sysmon *sysmon = container_of(subdev, struct qcom_sysmon, subdev);
 	struct sysmon_event event = {
 		.subsys_name = sysmon->name,
-		.ssr_event = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN
+		.ssr_event = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN,
+		.cluster = sysmon->rproc->cluster,
 	};
 
 	sysmon->shutdown_acked = false;
@@ -567,7 +571,8 @@ static void sysmon_unprepare(struct rproc_subdev *subdev)
 						  subdev);
 	struct sysmon_event event = {
 		.subsys_name = sysmon->name,
-		.ssr_event = SSCTL_SSR_EVENT_AFTER_SHUTDOWN
+		.ssr_event = SSCTL_SSR_EVENT_AFTER_SHUTDOWN,
+		.cluster = sysmon->rproc->cluster,
 	};
 
 	mutex_lock(&sysmon->state_lock);
@@ -588,6 +593,11 @@ static int sysmon_notify(struct notifier_block *nb, unsigned long event,
 	struct qcom_sysmon *sysmon = container_of(nb, struct qcom_sysmon, nb);
 	struct sysmon_event *sysmon_event = data;
 
+	/* Cluster siblings' firmware can't handle peer SSR notify; skip it */
+	if (sysmon->rproc->cluster &&
+	    sysmon->rproc->cluster == sysmon_event->cluster)
+		return NOTIFY_DONE;
+
 	/* Skip non-running rprocs and the originating instance */
 	if (sysmon->state != SSCTL_SSR_EVENT_AFTER_POWERUP ||
 	    !strcmp(sysmon_event->subsys_name, sysmon->name)) {
-- 
2.43.0

Re: [PATCH 3/8] remoteproc: qcom: sysmon: Suppress notify between cluster siblings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch's fix for the QMI timeout is incomplete. The patch adds a cluster-based suppression mechanism in `sysmon_notify()` to prevent QMI timeouts caused by peer SSR notifications between remote processors in the same cluster. However, it fails to add the same suppression check in the state synchronization loop inside `sysmon_start()`. When a remote processor powers up, `sysmon_start()` iterates over all already running instances and directly calls `ssctl_send_event()` (or `sysmon_send_event()`). If a sibling in the same cluster is already running, the newly starting processor will explicitly receive a peer notification about its sibling. This will trigger the exact same unhandled QMI message and timeout the patch intended to fix, making the timeout persist depending on the boot order of cluster members.
--

--- Patch [3]: [PATCH 3/8] remoteproc: qcom: sysmon: Suppress notify between cluster siblings ---
commit 8023059af35265ed9a7ebd6a255bd4e8dfa00c5b
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

remoteproc: qcom: sysmon: Suppress notify between cluster siblings

Firmware on clustered remote processors (e.g. Nord's HPASS ADSP0/1/2)
cannot handle the SSCTL peer-shutdown notification sysmon sends
whenever a sibling in the same cluster starts or stops, and the QMI
request instead times out.

> diff --git a/drivers/remoteproc/qcom_sysmon.c b/drivers/remoteproc/qcom_sysmon.c
> index 61e1038328e83..5d4443ef3bc18 100644
> --- a/drivers/remoteproc/qcom_sysmon.c
> +++ b/drivers/remoteproc/qcom_sysmon.c

[ ... ]

> @@ -500,7 +502,8 @@ static int sysmon_start(struct rproc_subdev *subdev)
>  	struct qcom_sysmon *target;
>  	struct sysmon_event event = {
>  		.subsys_name = sysmon->name,
> -		.ssr_event = SSCTL_SSR_EVENT_AFTER_POWERUP
> +		.ssr_event = SSCTL_SSR_EVENT_AFTER_POWERUP,
> +		.cluster = sysmon->rproc->cluster,
>  	};
>  
>  	reinit_completion(&sysmon->ssctl_comp);

[Severity: High]
This is a pre-existing issue, but does the patch leave the QMI timeout
unresolved during power-up?

When the second processor in a cluster starts, sysmon_start() iterates over
all running instances in sysmon_list and directly sends them events:

drivers/remoteproc/qcom_sysmon.c:sysmon_start() {
    ...
	list_for_each_entry(target, &sysmon_list, node) {
		mutex_lock(&target->state_lock);
		if (target == sysmon || target->state != SSCTL_SSR_EVENT_AFTER_POWERUP) {
			mutex_unlock(&target->state_lock);
			continue;
		}

		event.subsys_name = target->name;
		event.ssr_event = target->state;

		if (sysmon->ssctl_version == 2)
			ssctl_send_event(sysmon, &event);
		else if (sysmon->ept)
			sysmon_send_event(sysmon, &event);
		mutex_unlock(&target->state_lock);
	}
    ...
}

Because this manual synchronization loop bypasses sysmon_notify() and
directly calls ssctl_send_event() or sysmon_send_event(), it misses the
new cluster sibling suppression logic.

If a sibling in the same cluster is already running, will the newly starting
processor explicitly receive a peer notification about its sibling and
trigger the exact same unhandled QMI message and timeout this patch intends
to fix?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815132541.1575121-1-shengchao.guo@oss.qualcomm.com?part=3

[PATCH 4/8] rpmsg: qcom_glink_ssr: Suppress cleanup notify between cluster siblings

[Shawn Guo]

Like qcom_sysmon's SSCTL notify, clustered remote processors (e.g.
Nord's HPASS ADSP0/1/2) cannot handle the glink_ssr DO_CLEANUP
broadcast sent to siblings whenever one of them starts or stops,
leaving qcom_glink_ssr_notifier_call() to time out waiting for
CLEANUP_DONE.

Pass the stopping remoteproc's cluster identifier through
qcom_glink_ssr_notify(), and plumb each edge's own cluster identifier
down from the remoteproc that registers it, so that
qcom_glink_ssr_notifier_call() can skip the DO_CLEANUP send when the
sender and the receiving edge share a cluster. As in qcom_sysmon, the
suppression is done on the receiving side, so that a cluster member
keeps sending DO_CLEANUP to, and receiving it from, remote processors
outside its cluster. Edges with no cluster set are unaffected.

The receiving edge's identifier is carried in struct qcom_glink rather
than looked up from the rproc that owns it, so that glink_ssr does not
have to reach into struct remoteproc: RPMSG_QCOM_GLINK does not depend
on REMOTEPROC, and the identifier is deliberately opaque to keep the
two subsystems independent of each other.

Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_common.c  |  7 +++---
 drivers/remoteproc/qcom_common.h  |  2 +-
 drivers/rpmsg/qcom_glink_native.c | 20 ++++++++++++++++-
 drivers/rpmsg/qcom_glink_native.h |  5 ++++-
 drivers/rpmsg/qcom_glink_rpm.c    |  2 +-
 drivers/rpmsg/qcom_glink_smem.c   |  5 +++--
 drivers/rpmsg/qcom_glink_ssr.c    | 36 ++++++++++++++++++++++++++-----
 include/linux/rpmsg/qcom_glink.h  | 10 +++++----
 8 files changed, 69 insertions(+), 18 deletions(-)

diff --git a/drivers/remoteproc/qcom_common.c b/drivers/remoteproc/qcom_common.c
index 5294e327f158..13458bda6b24 100644
--- a/drivers/remoteproc/qcom_common.c
+++ b/drivers/remoteproc/qcom_common.c
@@ -217,7 +217,8 @@ static int glink_subdev_start(struct rproc_subdev *subdev)
 {
 	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
 
-	glink->edge = qcom_glink_smem_register(glink->dev, glink->node);
+	glink->edge = qcom_glink_smem_register(&glink->rproc->dev, glink->node,
+					       glink->rproc->cluster);
 
 	return PTR_ERR_OR_ZERO(glink->edge);
 }
@@ -234,7 +235,7 @@ static void glink_subdev_unprepare(struct rproc_subdev *subdev)
 {
 	struct qcom_rproc_glink *glink = to_glink_subdev(subdev);
 
-	qcom_glink_ssr_notify(glink->ssr_name);
+	qcom_glink_ssr_notify(glink->ssr_name, glink->rproc->cluster);
 }
 
 /**
@@ -258,7 +259,7 @@ void qcom_add_glink_subdev(struct rproc *rproc, struct qcom_rproc_glink *glink,
 		return;
 	}
 
-	glink->dev = dev;
+	glink->rproc = rproc;
 	glink->subdev.start = glink_subdev_start;
 	glink->subdev.stop = glink_subdev_stop;
 	glink->subdev.unprepare = glink_subdev_unprepare;
diff --git a/drivers/remoteproc/qcom_common.h b/drivers/remoteproc/qcom_common.h
index b0e7e336d363..7136494cbfba 100644
--- a/drivers/remoteproc/qcom_common.h
+++ b/drivers/remoteproc/qcom_common.h
@@ -14,7 +14,7 @@ struct qcom_rproc_glink {
 
 	const char *ssr_name;
 
-	struct device *dev;
+	struct rproc *rproc;
 	struct device_node *node;
 	struct qcom_glink_smem *edge;
 };
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 55793fc18293..06494f4d3a4f 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -122,6 +122,8 @@ struct qcom_glink {
 	unsigned long features;
 
 	bool intentless;
+	/* Cluster identifier of the remote processor behind this edge, or NULL */
+	void *cluster;
 	wait_queue_head_t tx_avail_notify;
 	bool sent_read_notify;
 
@@ -1889,7 +1891,8 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 					   unsigned long features,
 					   struct qcom_glink_pipe *rx,
 					   struct qcom_glink_pipe *tx,
-					   bool intentless)
+					   bool intentless,
+					   void *cluster)
 {
 	int ret;
 	struct qcom_glink *glink;
@@ -1904,6 +1907,7 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 
 	glink->features = features;
 	glink->intentless = intentless;
+	glink->cluster = cluster;
 
 	spin_lock_init(&glink->tx_lock);
 	spin_lock_init(&glink->rx_lock);
@@ -1937,6 +1941,20 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(qcom_glink_native_probe);
 
+/**
+ * qcom_glink_ept_cluster() - cluster identifier of an endpoint's edge
+ * @ept:	endpoint to query
+ *
+ * Return: the opaque cluster identifier of the remote processor @ept talks to,
+ * or NULL if it is not part of a cluster.
+ */
+void *qcom_glink_ept_cluster(struct rpmsg_endpoint *ept)
+{
+	struct glink_channel *channel = to_glink_channel(ept);
+
+	return channel->glink->cluster;
+}
+
 static int qcom_glink_remove_device(struct device *dev, void *data)
 {
 	device_unregister(dev);
diff --git a/drivers/rpmsg/qcom_glink_native.h b/drivers/rpmsg/qcom_glink_native.h
index 8dbec24de23e..8544f4f5c545 100644
--- a/drivers/rpmsg/qcom_glink_native.h
+++ b/drivers/rpmsg/qcom_glink_native.h
@@ -29,12 +29,15 @@ struct qcom_glink_pipe {
 
 struct device;
 struct qcom_glink;
+struct rpmsg_endpoint;
 
 struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 					   unsigned long features,
 					   struct qcom_glink_pipe *rx,
 					   struct qcom_glink_pipe *tx,
-					   bool intentless);
+					   bool intentless,
+					   void *cluster);
+void *qcom_glink_ept_cluster(struct rpmsg_endpoint *ept);
 void qcom_glink_native_remove(struct qcom_glink *glink);
 void qcom_glink_native_rx(struct qcom_glink *glink);
 
diff --git a/drivers/rpmsg/qcom_glink_rpm.c b/drivers/rpmsg/qcom_glink_rpm.c
index e3ba2c63a5fc..9c1ff05d4a10 100644
--- a/drivers/rpmsg/qcom_glink_rpm.c
+++ b/drivers/rpmsg/qcom_glink_rpm.c
@@ -346,7 +346,7 @@ static int glink_rpm_probe(struct platform_device *pdev)
 					0,
 					&rpm->rx_pipe.native,
 					&rpm->tx_pipe.native,
-					true);
+					true, NULL);
 	if (IS_ERR(glink)) {
 		mbox_free_channel(rpm->mbox_chan);
 		return PTR_ERR(glink);
diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
index 35bb03e67ae8..938f9575784e 100644
--- a/drivers/rpmsg/qcom_glink_smem.c
+++ b/drivers/rpmsg/qcom_glink_smem.c
@@ -225,7 +225,8 @@ static void qcom_glink_smem_release(struct device *dev)
 }
 
 struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
-						 struct device_node *node)
+						 struct device_node *node,
+						 void *cluster)
 {
 	struct glink_smem_pipe *rx_pipe;
 	struct glink_smem_pipe *tx_pipe;
@@ -345,7 +346,7 @@ struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
 	glink = qcom_glink_native_probe(dev,
 					GLINK_FEATURE_INTENT_REUSE,
 					&rx_pipe->native, &tx_pipe->native,
-					false);
+					false, cluster);
 	if (IS_ERR(glink)) {
 		ret = PTR_ERR(glink);
 		goto err_free_mbox;
diff --git a/drivers/rpmsg/qcom_glink_ssr.c b/drivers/rpmsg/qcom_glink_ssr.c
index e71d3716c55c..74c671a19af0 100644
--- a/drivers/rpmsg/qcom_glink_ssr.c
+++ b/drivers/rpmsg/qcom_glink_ssr.c
@@ -11,6 +11,8 @@
 #include <linux/rpmsg/qcom_glink.h>
 #include <linux/remoteproc/qcom_rproc.h>
 
+#include "qcom_glink_native.h"
+
 /**
  * struct do_cleanup_msg - The data structure for an SSR do_cleanup message
  * @version:	The G-Link SSR protocol version
@@ -55,16 +57,34 @@ struct glink_ssr {
 	struct completion completion;
 };
 
+/**
+ * struct glink_ssr_notify_data - payload of an SSR notification
+ * @ssr_name:	name of the remoteproc that has been stopped
+ * @cluster:	cluster identifier of the remoteproc that has been stopped,
+ *		or NULL if it is not part of a cluster
+ */
+struct glink_ssr_notify_data {
+	const char *ssr_name;
+	void *cluster;
+};
+
 /* Notifier list for all registered glink_ssr instances */
 static BLOCKING_NOTIFIER_HEAD(ssr_notifiers);
 
 /**
  * qcom_glink_ssr_notify() - notify GLINK SSR about stopped remoteproc
  * @ssr_name:	name of the remoteproc that has been stopped
+ * @cluster:	cluster identifier of the remoteproc that has been stopped,
+ *		or NULL if it is not part of a cluster
  */
-void qcom_glink_ssr_notify(const char *ssr_name)
+void qcom_glink_ssr_notify(const char *ssr_name, void *cluster)
 {
-	blocking_notifier_call_chain(&ssr_notifiers, 0, (void *)ssr_name);
+	struct glink_ssr_notify_data data = {
+		.ssr_name = ssr_name,
+		.cluster = cluster,
+	};
+
+	blocking_notifier_call_chain(&ssr_notifiers, 0, &data);
 }
 EXPORT_SYMBOL_GPL(qcom_glink_ssr_notify);
 
@@ -100,18 +120,24 @@ static int qcom_glink_ssr_notifier_call(struct notifier_block *nb,
 					void *data)
 {
 	struct glink_ssr *ssr = container_of(nb, struct glink_ssr, nb);
+	struct glink_ssr_notify_data *notify_data = data;
 	struct do_cleanup_msg msg;
-	char *ssr_name = data;
+	void *cluster;
 	int ret;
 
+	/* Cluster siblings' firmware can't handle peer SSR notify; skip it */
+	cluster = qcom_glink_ept_cluster(ssr->ept);
+	if (cluster && cluster == notify_data->cluster)
+		return NOTIFY_DONE;
+
 	ssr->seq_num++;
 	reinit_completion(&ssr->completion);
 
 	memset(&msg, 0, sizeof(msg));
 	msg.command = cpu_to_le32(GLINK_SSR_DO_CLEANUP);
 	msg.seq_num = cpu_to_le32(ssr->seq_num);
-	msg.name_len = cpu_to_le32(strlen(ssr_name));
-	strscpy(msg.name, ssr_name, sizeof(msg.name));
+	msg.name_len = cpu_to_le32(strlen(notify_data->ssr_name));
+	strscpy(msg.name, notify_data->ssr_name, sizeof(msg.name));
 
 	ret = rpmsg_send(ssr->ept, &msg, sizeof(msg));
 	if (ret < 0)
diff --git a/include/linux/rpmsg/qcom_glink.h b/include/linux/rpmsg/qcom_glink.h
index bfbd48f435fa..6591d016e8cb 100644
--- a/include/linux/rpmsg/qcom_glink.h
+++ b/include/linux/rpmsg/qcom_glink.h
@@ -8,22 +8,24 @@
 struct qcom_glink_smem;
 
 #if IS_ENABLED(CONFIG_RPMSG_QCOM_GLINK)
-void qcom_glink_ssr_notify(const char *ssr_name);
+void qcom_glink_ssr_notify(const char *ssr_name, void *cluster);
 #else
-static inline void qcom_glink_ssr_notify(const char *ssr_name) {}
+static inline void qcom_glink_ssr_notify(const char *ssr_name, void *cluster) {}
 #endif
 
 #if IS_ENABLED(CONFIG_RPMSG_QCOM_GLINK_SMEM)
 
 struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
-						 struct device_node *node);
+						 struct device_node *node,
+						 void *cluster);
 void qcom_glink_smem_unregister(struct qcom_glink_smem *glink);
 
 #else
 
 static inline struct qcom_glink_smem *
 qcom_glink_smem_register(struct device *parent,
-			 struct device_node *node)
+			 struct device_node *node,
+			 void *cluster)
 {
 	return NULL;
 }
-- 
2.43.0

Re: [PATCH 4/8] rpmsg: qcom_glink_ssr: Suppress cleanup notify between cluster siblings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Type confusion and out-of-bounds read when `qcom_glink_ept_cluster()` downcasts a generic `rpmsg_endpoint` without checking its transport backend.
--

--- Patch [4]: [PATCH 4/8] rpmsg: qcom_glink_ssr: Suppress cleanup notify between cluster siblings ---
commit c10027dd4bbd2c7fac42077955228324a4275d43
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

rpmsg: qcom_glink_ssr: Suppress cleanup notify between cluster siblings

This commit passes the remote processor's cluster identifier down to the
GLINK edges and uses it to skip sending cleanup broadcast messages to other
members of the same cluster during start/stop events.

> diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
> index d9d4468e4cbdf..a15d7e6a52423 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
[ ... ]
> @@ -1940,6 +1944,20 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(qcom_glink_native_probe);
>  
> +/**
> + * qcom_glink_ept_cluster() - cluster identifier of an endpoint's edge
> + * @ept:	endpoint to query
> + *
> + * Return: the opaque cluster identifier of the remote processor @ept talks to,
> + * or NULL if it is not part of a cluster.
> + */
> +void *qcom_glink_ept_cluster(struct rpmsg_endpoint *ept)
> +{
> +	struct glink_channel *channel = to_glink_channel(ept);
> +
> +	return channel->glink->cluster;
> +}

[Severity: High]
Does this downcast assume that the endpoint is always backed by the GLINK
transport without verifying it?

If a remote processor on a different bus (such as virtio_rpmsg_bus) sends a
Name Service announcement for a channel named glink_ssr, the rpmsg core
could probe the qcom_glink_ssr_driver and store a non-GLINK endpoint.

Later, when a remoteproc stops, qcom_glink_ssr_notifier_call() would pass
this non-GLINK endpoint to qcom_glink_ept_cluster():

qcom_glink_ssr_notifier_call()
  cluster = qcom_glink_ept_cluster(ssr->ept);

Could this blind cast to struct glink_channel and subsequent dereference
of channel->glink->cluster cause a kernel panic or out-of-bounds read?

> diff --git a/drivers/rpmsg/qcom_glink_ssr.c b/drivers/rpmsg/qcom_glink_ssr.c
> index e71d3716c55c5..74c671a19af03 100644
> --- a/drivers/rpmsg/qcom_glink_ssr.c
> +++ b/drivers/rpmsg/qcom_glink_ssr.c
[ ... ]
> @@ -100,18 +120,24 @@ static int qcom_glink_ssr_notifier_call(struct notifier_block *nb,
>  					void *data)
>  {
>  	struct glink_ssr *ssr = container_of(nb, struct glink_ssr, nb);
> +	struct glink_ssr_notify_data *notify_data = data;
>  	struct do_cleanup_msg msg;
> -	char *ssr_name = data;
> +	void *cluster;
>  	int ret;
>  
> +	/* Cluster siblings' firmware can't handle peer SSR notify; skip it */
> +	cluster = qcom_glink_ept_cluster(ssr->ept);
> +	if (cluster && cluster == notify_data->cluster)
> +		return NOTIFY_DONE;
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815132541.1575121-1-shengchao.guo@oss.qualcomm.com?part=4

[PATCH 5/8] remoteproc: qcom: pas: Track HPASS ADSP cluster membership

[Shawn Guo]

On Nord, ADSP0/1/2 share HPASS-domain resources (PLLs, AG_NOC, RSCp,
CESTA, THROTTLE, QTMR) owned by ADSP0, and their firmware cannot
tolerate one member being started or stopped independently of the
others. Modeling that requires each PAS instance to know which other
instances it is grouped with.

Add a small registry of clusters, keyed by the device_node every member's
"qcom,cluster-root" phandle points at, and join it at probe. Membership
is answerable from the node being probed alone: every member carries the
property, and the one referencing itself is the root, so there is no need
to search the device tree for nodes referencing a given instance.

A dependent member is rejected at probe if its root is disabled in DT,
since its boot could never be sequenced after a root that will not bind.

Publish the cluster in rproc->cluster, which is all sysmon and
glink_ssr need to stop notifying siblings of each other's SSR events.
The cluster is left only after rproc_del() has stopped this member, so
that the suppression is still in effect for that stop, and is unwound on
every probe error path, since a member freed while still linked into the
cluster would be walked by its siblings.

Instances without "qcom,cluster-root" are completely unaffected:
qcom_pas.cluster stays NULL for them.

Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 152 +++++++++++++++++++++++++++++
 1 file changed, 152 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 8cff0684fe2f..c8e4230bcf2c 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -66,6 +66,8 @@ struct qcom_pas_data {
 	bool needs_tzmem;
 };
 
+struct qcom_pas_cluster;
+
 struct qcom_pas {
 	struct device *dev;
 	struct rproc *rproc;
@@ -123,8 +125,148 @@ struct qcom_pas {
 
 	struct qcom_pas_context *pas_ctx;
 	struct qcom_pas_context *dtb_pas_ctx;
+
+	struct qcom_pas_cluster *cluster;
+	struct list_head cluster_node;
+	bool is_cluster_root;
+};
+
+/**
+ * struct qcom_pas_cluster - state shared by clustered PAS instances
+ * @node:	device_node of the cluster root, the key into the global list
+ * @list:	linkage in qcom_pas_cluster_list
+ * @lock:	protects @members and @root
+ * @members:	list of struct qcom_pas, linked via cluster_node
+ * @root:	the member whose "qcom,cluster-root" points at itself
+ * @refcount:	number of members currently attached to this cluster
+ *
+ * PAS instances whose "qcom,cluster-root" phandle points at the same node
+ * share one of these.
+ */
+struct qcom_pas_cluster {
+	struct device_node *node;
+	struct list_head list;
+
+	struct mutex lock;
+	struct list_head members;
+	struct qcom_pas *root;
+
+	int refcount;
 };
 
+static LIST_HEAD(qcom_pas_cluster_list);
+static DEFINE_MUTEX(qcom_pas_cluster_list_lock);
+
+static struct qcom_pas_cluster *qcom_pas_cluster_get(struct device_node *node)
+{
+	struct qcom_pas_cluster *cluster;
+
+	mutex_lock(&qcom_pas_cluster_list_lock);
+
+	list_for_each_entry(cluster, &qcom_pas_cluster_list, list) {
+		if (cluster->node == node) {
+			cluster->refcount++;
+			goto out;
+		}
+	}
+
+	cluster = kzalloc(sizeof(*cluster), GFP_KERNEL);
+	if (!cluster)
+		goto out;
+
+	cluster->node = of_node_get(node);
+	mutex_init(&cluster->lock);
+	INIT_LIST_HEAD(&cluster->members);
+	cluster->refcount = 1;
+	list_add_tail(&cluster->list, &qcom_pas_cluster_list);
+
+out:
+	mutex_unlock(&qcom_pas_cluster_list_lock);
+	return cluster;
+}
+
+static void qcom_pas_cluster_put(struct qcom_pas_cluster *cluster)
+{
+	mutex_lock(&qcom_pas_cluster_list_lock);
+	if (--cluster->refcount == 0) {
+		list_del(&cluster->list);
+		mutex_unlock(&qcom_pas_cluster_list_lock);
+		of_node_put(cluster->node);
+		kfree(cluster);
+		return;
+	}
+	mutex_unlock(&qcom_pas_cluster_list_lock);
+}
+
+/**
+ * qcom_pas_cluster_init() - join the cluster referenced by @np, if any
+ * @pas:	PAS instance being probed
+ * @np:		of_node of @pas's platform device
+ *
+ * Devices without a "qcom,cluster-root" property are not part of a cluster;
+ * @pas->cluster is left NULL and this is a no-op (e.g. cdsp0-3). Cluster
+ * members all carry the property, the root included, whose phandle points
+ * back at itself.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+static int qcom_pas_cluster_init(struct qcom_pas *pas, struct device_node *np)
+{
+	struct device_node *root_node;
+	bool is_root;
+
+	root_node = of_parse_phandle(np, "qcom,cluster-root", 0);
+	if (!root_node)
+		return 0;
+
+	is_root = root_node == np;
+
+	/*
+	 * A non-root member is useless without its root: it can never be
+	 * booted, since its boot has to be sequenced after the root's. Reject
+	 * it here rather than at first boot, so that a DT enabling a dependent
+	 * DSP but not the one owning the shared resources fails loudly and
+	 * early.
+	 */
+	if (!is_root && !of_device_is_available(root_node)) {
+		dev_err(pas->dev, "cluster root %pOF is not enabled\n", root_node);
+		of_node_put(root_node);
+		return -ENODEV;
+	}
+
+	pas->cluster = qcom_pas_cluster_get(root_node);
+	of_node_put(root_node);
+	if (!pas->cluster)
+		return -ENOMEM;
+
+	pas->is_cluster_root = is_root;
+
+	mutex_lock(&pas->cluster->lock);
+	list_add_tail(&pas->cluster_node, &pas->cluster->members);
+	if (is_root)
+		pas->cluster->root = pas;
+	mutex_unlock(&pas->cluster->lock);
+
+	return 0;
+}
+
+static void qcom_pas_cluster_exit(struct qcom_pas *pas)
+{
+	struct qcom_pas_cluster *cluster = pas->cluster;
+
+	if (!cluster)
+		return;
+
+	mutex_lock(&cluster->lock);
+	list_del(&pas->cluster_node);
+	if (cluster->root == pas)
+		cluster->root = NULL;
+	mutex_unlock(&cluster->lock);
+
+	qcom_pas_cluster_put(cluster);
+	pas->cluster = NULL;
+}
+
 static void qcom_pas_segment_dump(struct rproc *rproc,
 				  struct rproc_dump_segment *segment,
 				  void *dest, size_t offset, size_t size)
@@ -862,6 +1004,12 @@ static int qcom_pas_probe(struct platform_device *pdev)
 	pas = rproc->priv;
 	pas->dev = &pdev->dev;
 	pas->rproc = rproc;
+
+	ret = qcom_pas_cluster_init(pas, pdev->dev.of_node);
+	if (ret)
+		return ret;
+	rproc->cluster = pas->cluster;
+
 	pas->minidump_id = desc->minidump_id;
 	pas->pas_id = desc->pas_id;
 	pas->lite_pas_id = desc->lite_pas_id;
@@ -951,6 +1099,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
 	qcom_pas_unassign_memory_region(pas);
 free_rproc:
 	device_init_wakeup(pas->dev, false);
+	qcom_pas_cluster_exit(pas);
 
 	return ret;
 }
@@ -964,6 +1113,9 @@ static void qcom_pas_remove(struct platform_device *pdev)
 
 	rproc_del(pas->rproc);
 
+	pas->rproc->cluster = NULL;
+	qcom_pas_cluster_exit(pas);
+
 	qcom_q6v5_deinit(&pas->q6v5);
 	qcom_pas_unassign_memory_region(pas);
 	qcom_remove_glink_subdev(pas->rproc, &pas->glink_subdev);
-- 
2.43.0

[PATCH 6/8] remoteproc: qcom: pas: Sequence HPASS ADSP cluster boot root-first

[Shawn Guo]

The cluster root owns the HPASS resources shared with its siblings and
initializes them as part of its own boot, so ADSP1/ADSP2 can only cold
boot once ADSP0 is up. Booting a dependent member first leaves it
without the clocks, resets and NoC configuration it needs.

Gate a dependent member's qcom_pas_start() on the root having started.
The root being enabled in DT does not imply it has already bound, as
probe order between the two is not guaranteed, so fail the boot if the
root has not bound rather than dereferencing it. If the root has bound
but is not running yet, wait briefly on a completion the root signals
from its own qcom_pas_start(), so that a member racing the root through
a restart does not fail outright.

The completion starts out signaled, since a member may be attached to
already-running firmware at probe, and is only cleared once the root
actually goes down. A dependent member's boot racing the root's stop can
therefore still observe a stale completion, in which case it falls back
to failing on the root's state.

Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 76 ++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index c8e4230bcf2c..8231f811c568 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -138,6 +138,7 @@ struct qcom_pas {
  * @lock:	protects @members and @root
  * @members:	list of struct qcom_pas, linked via cluster_node
  * @root:	the member whose "qcom,cluster-root" points at itself
+ * @root_booted: signaled once @root has started, cleared when it goes down
  * @refcount:	number of members currently attached to this cluster
  *
  * PAS instances whose "qcom,cluster-root" phandle points at the same node
@@ -150,10 +151,18 @@ struct qcom_pas_cluster {
 	struct mutex lock;
 	struct list_head members;
 	struct qcom_pas *root;
+	struct completion root_booted;
 
 	int refcount;
 };
 
+/*
+ * How long a dependent member waits for its root to finish booting before
+ * giving up, e.g. when it is racing the root through the crash-restart
+ * cascade.
+ */
+#define QCOM_PAS_CLUSTER_ROOT_BOOT_TIMEOUT	(1 * HZ)
+
 static LIST_HEAD(qcom_pas_cluster_list);
 static DEFINE_MUTEX(qcom_pas_cluster_list_lock);
 
@@ -177,6 +186,13 @@ static struct qcom_pas_cluster *qcom_pas_cluster_get(struct device_node *node)
 	cluster->node = of_node_get(node);
 	mutex_init(&cluster->lock);
 	INIT_LIST_HEAD(&cluster->members);
+	init_completion(&cluster->root_booted);
+	/*
+	 * Cluster members may be attached to already-running firmware at
+	 * probe, so the root is presumed up until its own qcom_pas_stop()
+	 * reinit_completion()s this the first time it actually goes down.
+	 */
+	complete_all(&cluster->root_booted);
 	cluster->refcount = 1;
 	list_add_tail(&cluster->list, &qcom_pas_cluster_list);
 
@@ -267,6 +283,51 @@ static void qcom_pas_cluster_exit(struct qcom_pas *pas)
 	pas->cluster = NULL;
 }
 
+/**
+ * qcom_pas_cluster_wait_for_root() - gate a dependent member's boot on its root
+ * @pas:	the non-root cluster member being started
+ *
+ * The cluster root owns the resources its siblings need, and initializes them
+ * as part of its own boot, so a dependent member can only be started once the
+ * root is up.
+ *
+ * Return: 0 if the root is up, negative errno otherwise.
+ */
+static int qcom_pas_cluster_wait_for_root(struct qcom_pas *pas)
+{
+	struct qcom_pas_cluster *cluster = pas->cluster;
+	struct rproc *root;
+
+	/*
+	 * The root's PAS instance is enabled in DT (checked in
+	 * qcom_pas_cluster_init()) but has not necessarily bound yet, and may
+	 * have unbound again. Without it there is nothing to sequence this
+	 * member's boot against.
+	 *
+	 * Note that nothing refcounts members across unbind, so a root freed
+	 * under a concurrent sibling boot remains unhandled.
+	 */
+	mutex_lock(&cluster->lock);
+	root = cluster->root ? cluster->root->rproc : NULL;
+	mutex_unlock(&cluster->lock);
+
+	if (!root) {
+		dev_err(pas->dev, "cluster root not bound\n");
+		return -ENODEV;
+	}
+
+	if (root->state == RPROC_RUNNING || root->state == RPROC_ATTACHED)
+		return 0;
+
+	if (!wait_for_completion_timeout(&cluster->root_booted,
+					 QCOM_PAS_CLUSTER_ROOT_BOOT_TIMEOUT)) {
+		dev_err(pas->dev, "cluster root not started\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static void qcom_pas_segment_dump(struct rproc *rproc,
 				  struct rproc_dump_segment *segment,
 				  void *dest, size_t offset, size_t size)
@@ -424,6 +485,12 @@ static int qcom_pas_start(struct rproc *rproc)
 	struct qcom_pas *pas = rproc->priv;
 	int ret;
 
+	if (pas->cluster && !pas->is_cluster_root) {
+		ret = qcom_pas_cluster_wait_for_root(pas);
+		if (ret)
+			return ret;
+	}
+
 	ret = qcom_q6v5_prepare(&pas->q6v5);
 	if (ret)
 		return ret;
@@ -494,6 +561,9 @@ static int qcom_pas_start(struct rproc *rproc)
 	if (pas->dtb_pas_id)
 		qcom_pas_metadata_release(pas->dtb_pas_ctx);
 
+	if (pas->cluster && pas->is_cluster_root)
+		complete_all(&pas->cluster->root_booted);
+
 	/* firmware is used to pass reference from qcom_pas_start(), drop it now */
 	pas->firmware = NULL;
 
@@ -549,6 +619,12 @@ static int qcom_pas_stop(struct rproc *rproc)
 	int handover;
 	int ret;
 
+	if (pas->cluster && pas->is_cluster_root) {
+		mutex_lock(&pas->cluster->lock);
+		reinit_completion(&pas->cluster->root_booted);
+		mutex_unlock(&pas->cluster->lock);
+	}
+
 	ret = qcom_q6v5_request_stop(&pas->q6v5, pas->sysmon);
 	if (ret == -ETIMEDOUT)
 		dev_err(pas->dev, "timed out on wait\n");
-- 
2.43.0

Re: [PATCH 6/8] remoteproc: qcom: pas: Sequence HPASS ADSP cluster boot root-first

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Accessing `root->state` outside `cluster->lock` without holding an `rproc` device reference causes a Use-After-Free if the root device is unbound concurrently.
- [High] Missing state check after `wait_for_completion_timeout()` completely breaks boot sequencing, allowing dependent members to cold boot before the root.
- [Medium] Concurrent execution of `reinit_completion()` and `wait_for_completion_timeout()` causes a data race on `cluster->root_booted`.
--

--- Patch [6]: [PATCH 6/8] remoteproc: qcom: pas: Sequence HPASS ADSP cluster boot root-first ---
commit 7abf00410ddb06041e837e4d053d61bb3ef43922
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

remoteproc: qcom: pas: Sequence HPASS ADSP cluster boot root-first

This commit gates dependent member boots on the cluster root having started.
It introduces a wait mechanism using a completion to handle racing dependent
member restarts with the root's start and stop paths.

> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index dc6e7d601a9bd..ef38a1929f56b 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
[ ... ]
> @@ -267,6 +283,51 @@ static void qcom_pas_cluster_exit(struct qcom_pas *pas)
>  	pas->cluster = NULL;
>  }
>  
> +/**
> + * qcom_pas_cluster_wait_for_root() - gate a dependent member's boot on its root
> + * @pas:	the non-root cluster member being started
[ ... ]
> +static int qcom_pas_cluster_wait_for_root(struct qcom_pas *pas)
> +{
> +	struct qcom_pas_cluster *cluster = pas->cluster;
> +	struct rproc *root;
> +
> +	/*
> +	 * The root's PAS instance is enabled in DT (checked in
> +	 * qcom_pas_cluster_init()) but has not necessarily bound yet, and may
> +	 * have unbound again. Without it there is nothing to sequence this
> +	 * member's boot against.
> +	 *
> +	 * Note that nothing refcounts members across unbind, so a root freed
> +	 * under a concurrent sibling boot remains unhandled.
> +	 */
> +	mutex_lock(&cluster->lock);
> +	root = cluster->root ? cluster->root->rproc : NULL;
> +	mutex_unlock(&cluster->lock);
> +
> +	if (!root) {
> +		dev_err(pas->dev, "cluster root not bound\n");
> +		return -ENODEV;
> +	}
> +
[Severity: High]
Is there a risk of a use-after-free here if the root device is unbound
concurrently? 

Since the cluster->lock is dropped above and no reference is taken on
the rproc device (e.g., via get_device(&root->dev)), could root be freed via
devres before we evaluate root->state?

> +	if (root->state == RPROC_RUNNING || root->state == RPROC_ATTACHED)
> +		return 0;
> +
[Severity: Medium]
Could this wait_for_completion_timeout() race with the reinit_completion()
called in qcom_pas_stop()? 

In qcom_pas_stop():

	if (pas->cluster && pas->is_cluster_root) {
		mutex_lock(&pas->cluster->lock);
		reinit_completion(&pas->cluster->root_booted);
		mutex_unlock(&pas->cluster->lock);
	}

Since the wait happens outside the cluster->lock but reinit_completion()
modifies the completion directly, does this result in a data race on
cluster->root_booted?

> +	if (!wait_for_completion_timeout(&cluster->root_booted,
> +					 QCOM_PAS_CLUSTER_ROOT_BOOT_TIMEOUT)) {
> +		dev_err(pas->dev, "cluster root not started\n");
> +		return -ENODEV;
> +	}
> +
[Severity: High]
The commit message mentions that the dependent member "falls back to failing
on the root's state" when it observes a stale completion. 

Since cluster->root_booted is initialized as signaled during probe via
complete_all(), wait_for_completion_timeout() will immediately return success
here on a cold boot.

Are we missing the fallback check to re-evaluate root->state before returning
0? Without it, dependent members might successfully bypass the wait and
cold boot before the root.

> +	return 0;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815132541.1575121-1-shengchao.guo@oss.qualcomm.com?part=6

[PATCH 7/8] remoteproc: qcom: pas: Enforce coupled stop/crash for HPASS ADSP clusters

[Shawn Guo]

The firmware of a Nord HPASS ADSP cluster cannot tolerate one member
being stopped or restarted independently of the others: doing so leaves
sysmon/glink_ssr notify requests to siblings timing out, and the whole
cluster unable to recover without manually stopping all three and
restarting them in dependency order.

Coordinate an explicit stop or crash of any member across the whole
cluster in two phases, since hardware testing showed that whichever
member's PAS/TZ hardware power-off happens chronologically last always
fails its own graceful shutdown ack (both the sysmon SSCTL QMI ack and
the SMEM-bit/IRQ ack), regardless of whether that member is the cluster
root.

Phase 1 fans the stop out to every participant concurrently via a
per-member work item and waits, at a barrier, for every participant to
finish requesting and awaiting its own graceful ack while the whole
cluster is still fully powered. Only once every participant has cleared
the barrier does phase 2 let each of them proceed to its actual PAS/TZ
hardware power-off, so no member is ever asking firmware to ack a
shutdown after a sibling's hardware is already gone. The fan-out has to
be deferred to a work item rather than calling rproc_shutdown() inline,
because qcom_pas_stop() runs with the remoteproc core holding the
stopping member's own rproc->lock, and reaching for a sibling's
rproc->lock from inside that critical section would risk an ABBA
deadlock against a concurrent operation on that sibling.

Membership in a round is decided once, by
qcom_pas_cluster_member_stops(), and recorded per member, so that the
participant count and the two counters it seeds can never diverge. The
crashed member that triggered the round is a participant even though its
rproc->state is still RPROC_CRASHED -- the core only sets RPROC_OFFLINE
after ops->stop() returns -- while a sibling crashing concurrently is
deliberately excluded, being already torn down by its own recovery.

On a crash, cascade the stop the same way but also auto-restart the
whole cluster root-first once every participant has fully powered off,
reusing the root_booted completion to keep non-root members'
qcom_pas_start() from racing ahead of the root's boot. An explicit stop
leaves the ordered restart to users or higher-level policy.

Devices without "qcom,cluster-root" are completely unaffected, since
qcom_pas.cluster stays NULL for them.

Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 252 +++++++++++++++++++++++++++++
 1 file changed, 252 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 8231f811c568..0cf0a31f1f1d 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -28,6 +28,7 @@
 #include <linux/soc/qcom/mdt_loader.h>
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/smem_state.h>
+#include <linux/workqueue.h>
 
 #include "qcom_common.h"
 #include "qcom_pil_info.h"
@@ -128,7 +129,9 @@ struct qcom_pas {
 
 	struct qcom_pas_cluster *cluster;
 	struct list_head cluster_node;
+	struct work_struct stop_work;
 	bool is_cluster_root;
+	bool in_cluster_stop;
 };
 
 /**
@@ -139,6 +142,13 @@ struct qcom_pas {
  * @members:	list of struct qcom_pas, linked via cluster_node
  * @root:	the member whose "qcom,cluster-root" points at itself
  * @root_booted: signaled once @root has started, cleared when it goes down
+ * @cascade_work: deferred work that restarts the cluster after a crash
+ * @cascade_origin: member whose stop or crash triggered the current round
+ * @cascade_crashed: true if @cascade_origin crashed, rather than being stopped
+ * @stop_in_progress: re-entrancy guard: a coordinated stop is in flight
+ * @stop_pending: participants still owing a phase-1 graceful-ack attempt
+ * @stop_barrier: released once @stop_pending reaches 0
+ * @stop_done_pending: participants still owing a phase-2 hardware power-off
  * @refcount:	number of members currently attached to this cluster
  *
  * PAS instances whose "qcom,cluster-root" phandle points at the same node
@@ -153,6 +163,15 @@ struct qcom_pas_cluster {
 	struct qcom_pas *root;
 	struct completion root_booted;
 
+	struct work_struct cascade_work;
+	struct qcom_pas *cascade_origin;
+	bool cascade_crashed;
+	bool stop_in_progress;
+
+	int stop_pending;
+	struct completion stop_barrier;
+	int stop_done_pending;
+
 	int refcount;
 };
 
@@ -163,9 +182,78 @@ struct qcom_pas_cluster {
  */
 #define QCOM_PAS_CLUSTER_ROOT_BOOT_TIMEOUT	(1 * HZ)
 
+/*
+ * How long a member waits at the phase-1 barrier for the rest of the cluster,
+ * in case a participant never reaches it at all (e.g. rproc_stop() bails out
+ * before calling ops->stop).
+ */
+#define QCOM_PAS_CLUSTER_STOP_TIMEOUT		(20 * HZ)
+
 static LIST_HEAD(qcom_pas_cluster_list);
 static DEFINE_MUTEX(qcom_pas_cluster_list_lock);
 
+static void qcom_pas_stop_work_fn(struct work_struct *work)
+{
+	struct qcom_pas *pas = container_of(work, struct qcom_pas, stop_work);
+
+	rproc_shutdown(pas->rproc);
+}
+
+/**
+ * qcom_pas_cluster_cascade_work() - restart a crashed cluster, root first
+ * @work:	the cluster's cascade_work
+ *
+ * Only ever scheduled from qcom_pas_cluster_stop_complete(), i.e. only after
+ * every participant has finished its own phase-2 hardware power-off, and only
+ * when the round that just finished was a crash; explicit stops never
+ * auto-restart.
+ */
+static void qcom_pas_cluster_cascade_work(struct work_struct *work)
+{
+	struct qcom_pas_cluster *cluster = container_of(work, struct qcom_pas_cluster,
+						       cascade_work);
+	struct qcom_pas *pas, *origin, *root;
+
+	mutex_lock(&cluster->lock);
+	origin = cluster->cascade_origin;
+	root = cluster->root;
+	mutex_unlock(&cluster->lock);
+
+	/*
+	 * Membership is stable here: qcom_pas_cluster_exit() always
+	 * cancel_work_sync()s this work before touching cluster->members, so no
+	 * member can join or leave while this work item is running.
+	 *
+	 * If @origin is itself the root, its own crash-recovery thread
+	 * (rproc_boot_recovery()) is already booting it directly -- never call
+	 * rproc_boot() on @origin from here. The root has to be booted before
+	 * any other member, since a dependent member's qcom_pas_start() blocks
+	 * on cluster->root_booted: were we to boot it first from this
+	 * single-threaded work item, it would wait out its timeout on a root
+	 * boot this same thread has not issued yet.
+	 */
+	if (root && root != origin) {
+		int ret;
+
+		ret = rproc_boot(root->rproc);
+		if (ret) {
+			dev_err(root->dev, "failed to restart cluster root: %d\n", ret);
+			return;
+		}
+	}
+
+	list_for_each_entry(pas, &cluster->members, cluster_node) {
+		int ret;
+
+		if (pas == origin || pas == root)
+			continue;
+
+		ret = rproc_boot(pas->rproc);
+		if (ret)
+			dev_err(pas->dev, "failed to restart cluster sibling: %d\n", ret);
+	}
+}
+
 static struct qcom_pas_cluster *qcom_pas_cluster_get(struct device_node *node)
 {
 	struct qcom_pas_cluster *cluster;
@@ -187,6 +275,8 @@ static struct qcom_pas_cluster *qcom_pas_cluster_get(struct device_node *node)
 	mutex_init(&cluster->lock);
 	INIT_LIST_HEAD(&cluster->members);
 	init_completion(&cluster->root_booted);
+	init_completion(&cluster->stop_barrier);
+	INIT_WORK(&cluster->cascade_work, qcom_pas_cluster_cascade_work);
 	/*
 	 * Cluster members may be attached to already-running firmware at
 	 * probe, so the root is presumed up until its own qcom_pas_stop()
@@ -256,6 +346,7 @@ static int qcom_pas_cluster_init(struct qcom_pas *pas, struct device_node *np)
 		return -ENOMEM;
 
 	pas->is_cluster_root = is_root;
+	INIT_WORK(&pas->stop_work, qcom_pas_stop_work_fn);
 
 	mutex_lock(&pas->cluster->lock);
 	list_add_tail(&pas->cluster_node, &pas->cluster->members);
@@ -273,6 +364,9 @@ static void qcom_pas_cluster_exit(struct qcom_pas *pas)
 	if (!cluster)
 		return;
 
+	cancel_work_sync(&pas->stop_work);
+	cancel_work_sync(&cluster->cascade_work);
+
 	mutex_lock(&cluster->lock);
 	list_del(&pas->cluster_node);
 	if (cluster->root == pas)
@@ -328,6 +422,153 @@ static int qcom_pas_cluster_wait_for_root(struct qcom_pas *pas)
 	return 0;
 }
 
+/*
+ * A member takes part in a coordinated stop if its hardware is still powered:
+ * either it is running or attached, or it is the crashed member that triggered
+ * the round, whose rproc->state is RPROC_CRASHED and only becomes
+ * RPROC_OFFLINE once its ops->stop() has returned.
+ *
+ * A sibling that crashes concurrently is deliberately not a participant: it is
+ * already being torn down by its own recovery, and pulling it into this round
+ * would leave the two rounds fighting over the same counters.
+ */
+static bool qcom_pas_cluster_member_stops(struct qcom_pas *member,
+					  struct qcom_pas *origin)
+{
+	if (member == origin)
+		return true;
+
+	return member->rproc->state == RPROC_RUNNING ||
+	       member->rproc->state == RPROC_ATTACHED;
+}
+
+/**
+ * qcom_pas_cluster_trigger_stop() - begin a coordinated cluster stop
+ * @pas:	the member that is being stopped or has crashed
+ * @crashed:	true if @pas crashed, rather than being stopped explicitly
+ *
+ * Marks every member whose hardware is still powered as a participant of this
+ * round and fires off each *other* participant's own full stop concurrently
+ * via its stop_work, instead of one after another. This lets every
+ * participant's phase-1 graceful-ack attempt (see qcom_pas_stop()) run while
+ * all of them are still fully powered, so nobody is asking firmware to ack a
+ * shutdown after a sibling's hardware is already gone.
+ *
+ * A no-op if a coordinated stop is already in flight, e.g. when @pas is a
+ * sibling whose own stop was itself triggered by this same round: @pas is
+ * already marked as a participant and just goes on to take part in it.
+ *
+ * Must not call rproc_shutdown()/rproc_boot() directly from here: this runs
+ * from inside qcom_pas_stop(), which the remoteproc core calls with @pas's own
+ * rproc->lock held, and taking a sibling's rproc->lock synchronously from
+ * within that critical section would risk an ABBA deadlock against a
+ * concurrent operation on the sibling.
+ */
+static void qcom_pas_cluster_trigger_stop(struct qcom_pas *pas, bool crashed)
+{
+	struct qcom_pas_cluster *cluster = pas->cluster;
+	struct qcom_pas *member;
+	int active = 0;
+
+	mutex_lock(&cluster->lock);
+	if (cluster->stop_in_progress) {
+		mutex_unlock(&cluster->lock);
+		return;
+	}
+
+	cluster->stop_in_progress = true;
+	cluster->cascade_origin = pas;
+	cluster->cascade_crashed = crashed;
+
+	list_for_each_entry(member, &cluster->members, cluster_node) {
+		member->in_cluster_stop = qcom_pas_cluster_member_stops(member, pas);
+		if (member->in_cluster_stop)
+			active++;
+	}
+
+	cluster->stop_pending = active;
+	cluster->stop_done_pending = active;
+	reinit_completion(&cluster->stop_barrier);
+
+	/*
+	 * Fan out while still holding the lock, so that membership cannot
+	 * change between counting the participants and scheduling them, and so
+	 * that a stop_work running immediately blocks in
+	 * qcom_pas_cluster_stop_barrier() until the counters above are in
+	 * place. schedule_work() does not sleep, so it is safe from here.
+	 */
+	list_for_each_entry(member, &cluster->members, cluster_node) {
+		if (member == pas || !member->in_cluster_stop)
+			continue;
+
+		schedule_work(&member->stop_work);
+	}
+	mutex_unlock(&cluster->lock);
+}
+
+/**
+ * qcom_pas_cluster_stop_barrier() - wait for the whole cluster's phase-1 ack
+ * @pas:	the member calling this from inside its own qcom_pas_stop()
+ *
+ * Blocks this member's own hardware power-off until every other participant
+ * has also finished its phase-1 graceful-ack attempt (see qcom_pas_stop()).
+ * Bounded by QCOM_PAS_CLUSTER_STOP_TIMEOUT, proceeding to phase 2 anyway
+ * rather than hanging forever.
+ *
+ * A member that is not a participant of the current round, having already been
+ * powered off before it started, has no ack to contribute and must not touch
+ * the counters.
+ */
+static void qcom_pas_cluster_stop_barrier(struct qcom_pas *pas)
+{
+	struct qcom_pas_cluster *cluster = pas->cluster;
+
+	mutex_lock(&cluster->lock);
+	if (!pas->in_cluster_stop) {
+		mutex_unlock(&cluster->lock);
+		return;
+	}
+	if (--cluster->stop_pending == 0)
+		complete_all(&cluster->stop_barrier);
+	mutex_unlock(&cluster->lock);
+
+	if (!wait_for_completion_timeout(&cluster->stop_barrier,
+					 QCOM_PAS_CLUSTER_STOP_TIMEOUT))
+		dev_warn(pas->dev, "timed out waiting for cluster stop barrier\n");
+}
+
+/**
+ * qcom_pas_cluster_stop_complete() - record this member's phase-2 completion
+ * @pas:	the member calling this from inside its own qcom_pas_stop()
+ *
+ * The last participant to call this, i.e. the last to finish powering off its
+ * own hardware, ends the round and, if it was triggered by a crash, schedules
+ * the root-first restart cascade.
+ */
+static void qcom_pas_cluster_stop_complete(struct qcom_pas *pas)
+{
+	struct qcom_pas_cluster *cluster = pas->cluster;
+	bool crashed;
+
+	mutex_lock(&cluster->lock);
+	if (!pas->in_cluster_stop) {
+		mutex_unlock(&cluster->lock);
+		return;
+	}
+	pas->in_cluster_stop = false;
+
+	if (--cluster->stop_done_pending != 0) {
+		mutex_unlock(&cluster->lock);
+		return;
+	}
+	cluster->stop_in_progress = false;
+	crashed = cluster->cascade_crashed;
+	mutex_unlock(&cluster->lock);
+
+	if (crashed)
+		schedule_work(&cluster->cascade_work);
+}
+
 static void qcom_pas_segment_dump(struct rproc *rproc,
 				  struct rproc_dump_segment *segment,
 				  void *dest, size_t offset, size_t size)
@@ -625,10 +866,18 @@ static int qcom_pas_stop(struct rproc *rproc)
 		mutex_unlock(&pas->cluster->lock);
 	}
 
+	if (pas->cluster)
+		qcom_pas_cluster_trigger_stop(pas, rproc->state == RPROC_CRASHED);
+
+	/* Phase 1: request and await this member's own graceful ack */
 	ret = qcom_q6v5_request_stop(&pas->q6v5, pas->sysmon);
 	if (ret == -ETIMEDOUT)
 		dev_err(pas->dev, "timed out on wait\n");
 
+	if (pas->cluster)
+		qcom_pas_cluster_stop_barrier(pas);
+
+	/* Phase 2: the whole cluster has acked, power the hardware off */
 	ret = qcom_pas_shutdown(pas->pas_id);
 	if (ret && pas->decrypt_shutdown)
 		ret = qcom_pas_shutdown_poll_decrypt(pas);
@@ -653,6 +902,9 @@ static int qcom_pas_stop(struct rproc *rproc)
 	if (pas->smem_host_id)
 		ret = qcom_smem_bust_hwspin_lock_by_host(pas->smem_host_id);
 
+	if (pas->cluster)
+		qcom_pas_cluster_stop_complete(pas);
+
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH 7/8] remoteproc: qcom: pas: Enforce coupled stop/crash for HPASS ADSP clusters

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Canceling `pas->stop_work` before removing the member from `cluster->members` creates a TOCTOU race window that leads to a Use-After-Free.
- [High] Coordinated stop state machine permanently deadlocks if a participant has multiple power references due to the use of `rproc_shutdown()`.
- [High] Concurrent crashes bypass the coordination barrier and prematurely power off hardware, violating the cluster constraint.
- [High] Sibling auto-restart times out prematurely when the cluster root crashes, leaving siblings permanently offline.
- [High] Lockless iteration of `cluster->members` in `qcom_pas_cluster_cascade_work()` races with `qcom_pas_cluster_init()`, causing list corruption.
- [Medium] Concurrent crash of a cluster member inflates its `rproc->power` refcount during auto-restart, preventing it from being successfully stopped by the user.
--

--- Patch [7]: [PATCH 7/8] remoteproc: qcom: pas: Enforce coupled stop/crash for HPASS ADSP clusters ---
commit 6bd05de74159725dcc440502485f8bb8e3c952cb
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

remoteproc: qcom: pas: Enforce coupled stop/crash for HPASS ADSP clusters

This commit enforces a coupled stop and crash mechanism for Nord HPASS ADSP
clusters. It orchestrates a two-phase stop and hardware power-off sequence
across cluster members to prevent sysmon timeouts and recovery failures. It
also implements an automatic root-first restart sequence upon a crash.

> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index ef38a1929f56b..ac3b92c7e7e79 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c

[ ... ]

> +static void qcom_pas_stop_work_fn(struct work_struct *work)
> +{
> +	struct qcom_pas *pas = container_of(work, struct qcom_pas, stop_work);
> +
> +	rproc_shutdown(pas->rproc);

[Severity: High]
If a sibling has an rproc->power refcount greater than 1 (e.g., booted by
multiple clients), rproc_shutdown() will simply decrement the refcount and
return without invoking qcom_pas_stop().

Since qcom_pas_cluster_stop_complete() would never be called for that
sibling, cluster->stop_done_pending will never reach 0.

Could this leave cluster->stop_in_progress permanently set to true and
deadlock the coordinated stop state machine for all future operations?

> +}
> +
> +/**
> + * qcom_pas_cluster_cascade_work() - restart a crashed cluster, root first
> + * @work:	the cluster's cascade_work

[ ... ]

> +static void qcom_pas_cluster_cascade_work(struct work_struct *work)
> +{
> +	struct qcom_pas_cluster *cluster = container_of(work, struct qcom_pas_cluster,
> +						       cascade_work);
> +	struct qcom_pas *pas, *origin, *root;
> +
> +	mutex_lock(&cluster->lock);
> +	origin = cluster->cascade_origin;
> +	root = cluster->root;
> +	mutex_unlock(&cluster->lock);
> +
> +	/*
> +	 * Membership is stable here: qcom_pas_cluster_exit() always
> +	 * cancel_work_sync()s this work before touching cluster->members, so no

[ ... ]

> +	if (root && root != origin) {
> +		int ret;
> +
> +		ret = rproc_boot(root->rproc);
> +		if (ret) {
> +			dev_err(root->dev, "failed to restart cluster root: %d\n", ret);
> +			return;
> +		}
> +	}
> +
> +	list_for_each_entry(pas, &cluster->members, cluster_node) {

[Severity: High]
Is it safe to iterate cluster->members locklessly here?

If a cluster crash recovery executes concurrently with the probe of a new
cluster member, qcom_pas_cluster_init() could call list_add_tail() while
holding cluster->lock.

Could traversing this list without holding cluster->lock result in list
corruption or infinite loops?

> +		int ret;
> +
> +		if (pas == origin || pas == root)
> +			continue;
> +
> +		ret = rproc_boot(pas->rproc);

[Severity: High]
If the cluster root crashes, its recovery thread will trigger a coordinated
stop and schedule this cascade_work. Siblings are immediately booted here,
which eventually calls qcom_pas_cluster_wait_for_root(), enforcing a 1-second
timeout waiting for the root to boot.

However, the root's recovery thread must still generate a coredump and load
firmware, which typically takes significantly longer than 1 second.

Will this cause the auto-restarting siblings to prematurely time out and
fail to boot?

[Severity: Medium]
When auto-restarting siblings, this loops through the entire cluster without
checking if the members actually participated in the coordinated stop.

If a sibling crashed concurrently and was excluded from the stop (its
stop_work didn't execute and its refcount wasn't decremented), unconditionally
calling rproc_boot() on it here will artificially inflate its rproc->power
refcount.

Could this permanent refcount leak prevent the remote processor from being
successfully shut down by the user later?

> +		if (ret)
> +			dev_err(pas->dev, "failed to restart cluster sibling: %d\n", ret);
> +	}
> +}

[ ... ]

> @@ -273,6 +364,9 @@ static void qcom_pas_cluster_exit(struct qcom_pas *pas)
>  	if (!cluster)
>  		return;
>  
> +	cancel_work_sync(&pas->stop_work);
> +	cancel_work_sync(&cluster->cascade_work);
> +
>  	mutex_lock(&cluster->lock);
>  	list_del(&pas->cluster_node);

[Severity: High]
Does qcom_pas_cluster_exit() have a race condition that could lead to a
use-after-free?

If a member is being removed, cancel_work_sync() is called while the member
is still in the cluster->members list. If a sibling concurrently triggers
a cluster stop, it can find this member in the list and schedule
pas->stop_work via qcom_pas_cluster_trigger_stop():

    list_for_each_entry(member, &cluster->members, cluster_node) {
        if (member == pas || !member->in_cluster_stop)
            continue;
        schedule_work(&member->stop_work);
    }

Could this result in the work executing on a freed pas structure after
device removal completes?

[ ... ]

> @@ -625,10 +866,18 @@ static int qcom_pas_stop(struct rproc *rproc)
>  		mutex_unlock(&pas->cluster->lock);
>  	}
>  
> +	if (pas->cluster)
> +		qcom_pas_cluster_trigger_stop(pas, rproc->state == RPROC_CRASHED);
> +
> +	/* Phase 1: request and await this member's own graceful ack */
>  	ret = qcom_q6v5_request_stop(&pas->q6v5, pas->sysmon);
>  	if (ret == -ETIMEDOUT)
>  		dev_err(pas->dev, "timed out on wait\n");
>  
> +	if (pas->cluster)
> +		qcom_pas_cluster_stop_barrier(pas);
> +
> +	/* Phase 2: the whole cluster has acked, power the hardware off */
>  	ret = qcom_pas_shutdown(pas->pas_id);

[Severity: High]
If a sibling crashes concurrently with the original stopping member,
qcom_pas_cluster_member_stops() returns false for it, excluding it from
the coordinated stop.

When that sibling's own recovery thread later calls qcom_pas_stop(), it sees
stop_in_progress is true but its in_cluster_stop is false. This causes it
to bypass qcom_pas_cluster_stop_barrier() and immediately call
qcom_pas_shutdown().

Doesn't this power off its hardware instantly while other participants are
still executing Phase 1, violating the constraint that no member shuts down
before the whole cluster has acked?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815132541.1575121-1-shengchao.guo@oss.qualcomm.com?part=7

[PATCH 8/8] remoteproc: qcom: pas: Add Nord ADSP1/2 support

[Shawn Guo]

Add support for additional Hexagon ADSP cores on Nord (adsp1/adsp2),
which sit alongside the existing adsp0 as HPASS cluster members.

Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 44 ++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 0cf0a31f1f1d..96e2a7c74927 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -1937,6 +1937,48 @@ static const struct qcom_pas_data nord_adsp_resource = {
 	.smem_host_id = 2,
 };
 
+static const struct qcom_pas_data nord_adsp1_resource = {
+	.crash_reason_smem = 663,
+	.firmware_name = "adsp1.mbn",
+	.dtb_firmware_name = "adsp1_dtb.mbn",
+	.pas_id = 53,
+	.dtb_pas_id = 55,
+	.minidump_id = 21,
+	.auto_boot = true,
+	.early_boot = true,
+	.proxy_pd_names = (char*[]){
+		"cx",
+		"mx",
+		NULL
+	},
+	.load_state = "adsp1",
+	.ssr_name = "lpass1",
+	.sysmon_name = "adsp1",
+	.ssctl_id = 0x1d,
+	.smem_host_id = 66,
+};
+
+static const struct qcom_pas_data nord_adsp2_resource = {
+	.crash_reason_smem = 664,
+	.firmware_name = "adsp2.mbn",
+	.dtb_firmware_name = "adsp2_dtb.mbn",
+	.pas_id = 54,
+	.dtb_pas_id = 56,
+	.minidump_id = 22,
+	.auto_boot = true,
+	.early_boot = true,
+	.proxy_pd_names = (char*[]){
+		"cx",
+		"mx",
+		NULL
+	},
+	.load_state = "adsp2",
+	.ssr_name = "lpass2",
+	.sysmon_name = "adsp2",
+	.ssctl_id = 0x1e,
+	.smem_host_id = 130,
+};
+
 static const struct qcom_pas_data nord_cdsp0_resource = {
 	.crash_reason_smem = 601,
 	.firmware_name = "cdsp.mbn",
@@ -2316,6 +2358,8 @@ static const struct of_device_id qcom_pas_of_match[] = {
 	{ .compatible = "qcom,msm8998-adsp-pas", .data = &msm8996_adsp_resource },
 	{ .compatible = "qcom,msm8998-slpi-pas", .data = &msm8996_slpi_resource_init },
 	{ .compatible = "qcom,nord-adsp-pas", .data = &nord_adsp_resource },
+	{ .compatible = "qcom,nord-adsp1-pas", .data = &nord_adsp1_resource },
+	{ .compatible = "qcom,nord-adsp2-pas", .data = &nord_adsp2_resource },
 	{ .compatible = "qcom,nord-cdsp0-pas", .data = &nord_cdsp0_resource },
 	{ .compatible = "qcom,nord-cdsp1-pas", .data = &nord_cdsp1_resource },
 	{ .compatible = "qcom,nord-cdsp2-pas", .data = &nord_cdsp2_resource },
-- 
2.43.0