linux-kernel - Re: [PATCH v2 2/3] misc: fastrpc: add support for gpdsp remoteproc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <412fe24e-ce70-4733-ace5-d3fbe43476c4@oss.qualcomm.com>
Date: Wed, 2 Apr 2025 14:08:49 +0530
From: Ekansh Gupta <ekansh.gupta@....qualcomm.com>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Ling Xu <quic_lxu5@...cinc.com>, andersson@...nel.org,
        konradybcio@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
        conor+dt@...nel.org, amahesh@....qualcomm.com, arnd@...db.de,
        gregkh@...uxfoundation.org, quic_kuiw@...cinc.com,
        quic_ekangupt@...cinc.com, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v2 2/3] misc: fastrpc: add support for gpdsp remoteproc

On 3/21/2025 5:53 PM, Srinivas Kandagatla wrote:
>
>
> On 20/03/2025 18:43, Dmitry Baryshkov wrote:
>> On Thu, Mar 20, 2025 at 05:11:20PM +0000, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 20/03/2025 09:14, Ling Xu wrote:
>>>> The fastrpc driver has support for 5 types of remoteprocs. There are
>>>> some products which support GPDSP remoteprocs. Add changes to support
>>>> GPDSP remoteprocs.
>>>>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
>>>> Signed-off-by: Ling Xu <quic_lxu5@...cinc.com>
>>>> ---
>>>>    drivers/misc/fastrpc.c | 10 ++++++++--
>>>>    1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>>>> index 7b7a22c91fe4..80aa554b3042 100644
>>>> --- a/drivers/misc/fastrpc.c
>>>> +++ b/drivers/misc/fastrpc.c
>>>> @@ -28,7 +28,9 @@
>>>>    #define SDSP_DOMAIN_ID (2)
>>>>    #define CDSP_DOMAIN_ID (3)
>>>>    #define CDSP1_DOMAIN_ID (4)
>>>> -#define FASTRPC_DEV_MAX        5 /* adsp, mdsp, slpi, cdsp, cdsp1 */
>>>> +#define GDSP0_DOMAIN_ID (5)
>>>> +#define GDSP1_DOMAIN_ID (6)
>>>
>>> We have already made the driver look silly here, Lets not add domain ids for
>>> each instance, which is not a scalable.
>>>
>>> Domain ids are strictly for a domain not each instance.
>>
>> Then CDSP1 should also be gone, correct?
> Its already gone as part of the patch that I shared in this discussion.
>
> I will send a proper patch to list once Ling/Ekansh has agree with it.
>
Thanks, Srini, for sharing this clean-up patch. It looks proper to
me, but I was thinking if we could remove the domain_id dependency
from the fastrpc driver. The addition of any new DSP will frequently
require changes in the driver. Currently, its usage is for creating
different types of device nodes and transferring memory ownership to
SLPI when a memory region is added.

The actual intention behind different types of device nodes can be
defined as follows:

fastrpc-xdsp-secure: Used for signed (privileged) PD offload and for daemons.
fastrpc-xdsp: Should be used only for unsigned (less privileged) PD offload.

The reason for this constraint is to prevent any untrusted process
from communicating with any privileged PD on DSP, which poses a security risk.
The access to different device nodes can be provided/restricted based on UID/GID
(still need to check more on this; on Android-like systems, this is controlled by
SELinux).

There is already a qcom,non-secure-domain device tree property[1] which doesn't
have a proper definition as of today. The actual way to differentiate between
secure and non-secure DSP should be based on its ability to support unsigned PD.

One way to remove the domain_id dependency that I can think of is to use this
property to create different types of device nodes. Essentially, if unsigned PD
is supported (e.g., CDSP, GPDSP), we add this property to the DT node and create
both types of device nodes based on this. Otherwise, only the secure device node
is created.

This raises the question of backward compatibility, but I see that on most older
platform DTs, this property is already added, so both device nodes will be created
there, and applications will work as expected. If any old DT DSP node lacks this
property, we can add it there as well.

Going forward, the qcom-non-secure-property should be added only if unsigned PD
is supported. This way, we can clean up the driver completely to remove the
domain_id dependency.

If this sounds good, I can work on this design and send out a patch.

[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml#n44

--Ekansh

> --srini
>>
>