[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <qbuccwnfljpnxvpp7vl4weoecx6ujg3cy2lwwgoz42b3ux5o3k@mi5fxhplgrt7>
Date: Mon, 26 Jan 2026 14:53:46 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: Xingjing Deng <micro6947@...il.com>, srini@...nel.org,
amahesh@....qualcomm.com, arnd@...db.de, dri-devel@...ts.freedesktop.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, Xingjing Deng <xjdeng@...a.edu.cn>,
stable@...r.kernel.org
Subject: Re: [PATCH v5] misc: fastrpc: check qcom_scm_assign_mem() return in
rpmsg_probe
On Mon, Jan 26, 2026 at 04:24:55PM +0100, Greg KH wrote:
> On Sat, Jan 17, 2026 at 10:03:51PM +0800, Xingjing Deng wrote:
> > In the SDSP probe path, qcom_scm_assign_mem() is used to assign the
> > reserved memory to the configured VMIDs, but its return value was not
> > checked.
> >
> > Fail the probe if the SCM call fails to avoid continuing with an
> > unexpected/incorrect memory permission configuration.
> >
> > The file has passed the check of checkpatch.
> >
> > Fixes: c3c0363bc72d4 ("misc: fastrpc: support complete DMA pool access to the DSP")
> > Cc: stable@...r.kernel.org # 6.11-rc1
> > Signed-off-by: Xingjing Deng <xjdeng@...a.edu.cn>
> > ---
> > v5:
> > - Squash the functional change and indentation fix into a single patch.
> > - Link to v4: https://lore.kernel.org/linux-arm-msm/2026011637-statute-showy-2c3f@gregkh/T/#t
> >
> > v4:
> > - Format the indentation
> > - Link to v3: https://lore.kernel.org/linux-arm-msm/20260113084352.72itrloj5w7qb5o3@hu-mojha-hyd.qualcomm.com/T/#t
> >
> > v3:
> > - Add missing linux-kernel@...r.kernel.org to cc list.
> > - Standarlize changelog placement/format.
> > - Link to v2: https://lore.kernel.org/linux-arm-msm/20260113063618.e2ke47gy3hnfi67e@hu-mojha-hyd.qualcomm.com/T/#t
> >
> > v2:
> > - Add Fixes: and Cc: stable tags.
> > - Link to v1: https://lore.kernel.org/linux-arm-msm/20260113022550.4029635-1-xjdeng@buaa.edu.cn/T/#u
> > ---
> > drivers/misc/fastrpc.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > index fb3b54e05928..d9650efa443f 100644
> > --- a/drivers/misc/fastrpc.c
> > +++ b/drivers/misc/fastrpc.c
> > @@ -2338,8 +2338,13 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> > if (!err) {
> > src_perms = BIT(QCOM_SCM_VMID_HLOS);
> >
> > - qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> > - data->vmperms, data->vmcount);
> > + err = qcom_scm_assign_mem(res.start, resource_size(&res), &src_perms,
> > + data->vmperms, data->vmcount);
> > + if (err) {
> > + dev_err(rdev, "Failed to assign memory phys 0x%llx size 0x%llx err %d",
> > + res.start, resource_size(&res), err);
>
> Shouldn't the caller function report the error?
>
That is correct, all codepaths through qcom_scm_assign_mem() will either
be -ENOMEM or print an error message, so we shouldn't print yet another
message in the log here.
(The usefulness of the error message in qcom_scm_assign_mem() could
certainly be improved, but that's a separate matter/patch).
> How as this found and tested?
>
Looking forward to Xingjing's answer here.
But failing to handle errors here means that we're ignoring the failure
to map memory to the DSP, which will fail us later. So, that part is
correct. Exiting through err_free_data looks good as well.
Regards,
Bjorn
> thanks,
>
> greg k-h
>
Powered by blists - more mailing lists