A remoteproc could theoretically signal handover twice. This is unexpected
and would break the reference counting for the handover resources (power
domains, clocks, regulators, etc), so add a check to prevent that from
happening.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 drivers/remoteproc/qcom_q6v5.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
index 769c6d6d6a731672eca9f960b05c68f6d4d77af2..58d5b85e58cdadabdd3e23d39c06a39196c3a194 100644
--- a/drivers/remoteproc/qcom_q6v5.c
+++ b/drivers/remoteproc/qcom_q6v5.c
@@ -164,6 +164,11 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
 {
 	struct qcom_q6v5 *q6v5 = data;
 
+	if (q6v5->handover_issued) {
+		dev_err(q6v5->dev, "Handover signaled, but it already happened\n");
+		return IRQ_HANDLED;
+	}
+
 	if (q6v5->handover)
 		q6v5->handover(q6v5);
 

-- 
2.50.1

On Tue, Aug 19, 2025 at 01:08:03PM +0200, Stephan Gerhold wrote:
> A remoteproc could theoretically signal handover twice. This is unexpected

theoretically or practically?

> and would break the reference counting for the handover resources (power
> domains, clocks, regulators, etc), so add a check to prevent that from
> happening.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
>  drivers/remoteproc/qcom_q6v5.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> index 769c6d6d6a731672eca9f960b05c68f6d4d77af2..58d5b85e58cdadabdd3e23d39c06a39196c3a194 100644
> --- a/drivers/remoteproc/qcom_q6v5.c
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -164,6 +164,11 @@ static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
>  {
>  	struct qcom_q6v5 *q6v5 = data;
>  
> +	if (q6v5->handover_issued) {
> +		dev_err(q6v5->dev, "Handover signaled, but it already happened\n");
> +		return IRQ_HANDLED;
> +	}
> +
>  	if (q6v5->handover)
>  		q6v5->handover(q6v5);
>  
> 
> -- 
> 2.50.1
> 

-- 
With best wishes
Dmitry

On Tue, Aug 19, 2025 at 02:41:55PM +0300, Dmitry Baryshkov wrote:
> On Tue, Aug 19, 2025 at 01:08:03PM +0200, Stephan Gerhold wrote:
> > A remoteproc could theoretically signal handover twice. This is unexpected
> 
> theoretically or practically?
> 

You could easily trigger handover again from a custom remoteproc
firmware by setting the handover state to 0 and then back to 1. However,
if you find a firmware version doing this, you might want to have a
serious conversation with the firmware developer. It makes no sense to
do that. :-)

In other words, on technical level it is practical. From a conceptual
point of view it is just theoretical.

In any case, if it happens, we shouldn't mess up reference counters in
my opinion (or risk dereferencing invalid pointers etc).

Thanks,
Stephan

On Tue, Aug 19, 2025 at 05:05:13PM +0200, Stephan Gerhold wrote:
> On Tue, Aug 19, 2025 at 02:41:55PM +0300, Dmitry Baryshkov wrote:
> > On Tue, Aug 19, 2025 at 01:08:03PM +0200, Stephan Gerhold wrote:
> > > A remoteproc could theoretically signal handover twice. This is unexpected
> > 
> > theoretically or practically?
> > 
> 
> You could easily trigger handover again from a custom remoteproc
> firmware by setting the handover state to 0 and then back to 1. However,
> if you find a firmware version doing this, you might want to have a
> serious conversation with the firmware developer. It makes no sense to
> do that. :-)
> 
> In other words, on technical level it is practical. From a conceptual
> point of view it is just theoretical.
> 
> In any case, if it happens, we shouldn't mess up reference counters in
> my opinion (or risk dereferencing invalid pointers etc).

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry