| 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
| ||
|
Message-ID: <e2a8528b-fa18-471f-9cb8-da64bb488f2a@quicinc.com>
Date: Mon, 7 Apr 2025 17:13:25 +0800
From: Ling Xu <quic_lxu5@...cinc.com>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
<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>
CC: <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>,
"Dmitry
Baryshkov" <dmitry.baryshkov@....qualcomm.com>
Subject: Re: [PATCH v2 2/3] misc: fastrpc: add support for gpdsp remoteproc
在 3/21/2025 1:11 AM, Srinivas Kandagatla 写道:
>
>
> 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.
>
>
>> +#define FASTRPC_DEV_MAX 7 /* adsp, mdsp, slpi, cdsp, cdsp1, gdsp0, gdsp1 */
>> #define FASTRPC_MAX_SESSIONS 14
>> #define FASTRPC_MAX_VMIDS 16
>> #define FASTRPC_ALIGN 128
>> @@ -107,7 +109,9 @@
>> #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>> static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp",
>> - "sdsp", "cdsp", "cdsp1" };
>> + "sdsp", "cdsp",
>> + "cdsp1", "gdsp0",
>> + "gdsp1" };
>> struct fastrpc_phy_page {
>> u64 addr; /* physical address */
>> u64 size; /* size of contiguous region */
>> @@ -2338,6 +2342,8 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>> break;
>> case CDSP_DOMAIN_ID:
>> case CDSP1_DOMAIN_ID:
>> + case GDSP0_DOMAIN_ID:
>> + case GDSP1_DOMAIN_ID:
>> data->unsigned_support = true;
>> /* Create both device nodes so that we can allow both Signed and Unsigned PD */
>> err = fastrpc_device_register(rdev, data, true, domains[domain_id]);
>
>
> Can you try this patch: only compile tested.
>
> ---------------------------------->cut<---------------------------------------
> From 3f8607557162e16673b26fa253d11cafdc4444cf Mon Sep 17 00:00:00 2001
> From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> Date: Thu, 20 Mar 2025 17:07:05 +0000
> Subject: [PATCH] misc: fastrpc: cleanup the domain names
>
> Currently the domain ids are added for each instance of domain, this is
> totally not scalable approch.
>
> Clean this mess and create domain ids for only domains not its
> instances.
> This patch also moves the domain ids to uapi header as this is required
> for FASTRPC_IOCTL_GET_DSP_INFO ioctl.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> ---
> drivers/misc/fastrpc.c | 45 ++++++++++++++++++++-----------------
> include/uapi/misc/fastrpc.h | 7 ++++++
> 2 files changed, 32 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 7b7a22c91fe4..b3932897a437 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -23,12 +23,6 @@
> #include <uapi/misc/fastrpc.h>
> #include <linux/of_reserved_mem.h>
>
> -#define ADSP_DOMAIN_ID (0)
> -#define MDSP_DOMAIN_ID (1)
> -#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 FASTRPC_MAX_SESSIONS 14
> #define FASTRPC_MAX_VMIDS 16
> #define FASTRPC_ALIGN 128
> @@ -106,8 +100,6 @@
>
> #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>
> -static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp",
> - "sdsp", "cdsp", "cdsp1" };
> struct fastrpc_phy_page {
> u64 addr; /* physical address */
> u64 size; /* size of contiguous region */
> @@ -1769,7 +1761,7 @@ static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp)
> return -EFAULT;
>
> cap.capability = 0;
> - if (cap.domain >= FASTRPC_DEV_MAX) {
> + if (cap.domain >= FASTRPC_DOMAIN_MAX) {
> dev_err(&fl->cctx->rpdev->dev, "Error: Invalid domain id:%d, err:%d\n",
> cap.domain, err);
> return -ECHRNG;
I tested this patch and saw one issue.
Here FASTRPC_DOMAIN_MAX is set to 4, but in userspace, cdsp1 is 4, gdsp0 is 5 and gdsp1 is 6.
For example, if we run a demo on gdsp0, cap.domain copied from userspace will be 5 which could lead to wrong message.
--Ling Xu
> @@ -2255,6 +2247,24 @@ static int fastrpc_device_register(struct device *dev, struct fastrpc_channel_ct
> return err;
> }
>
> +static int fastrpc_get_domain_id(const char *domain)
> +{
> + if (strncmp(domain, "adsp", 4) == 0) {
> + return ADSP_DOMAIN_ID;
> + } else if (strncmp(domain, "cdsp", 4) == 0) {
> + return CDSP_DOMAIN_ID;
> + } else if (strncmp(domain, "mdsp", 4) ==0) {
> + return MDSP_DOMAIN_ID;
> + } else if (strncmp(domain, "sdsp", 4) ==0) {
> + return SDSP_DOMAIN_ID;
> + } else if (strncmp(domain, "gdsp", 4) ==0) {
> + return GDSP_DOMAIN_ID;
> + }
> +
> + return -EINVAL;
> +
> +}
> +
> static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> {
> struct device *rdev = &rpdev->dev;
> @@ -2272,15 +2282,10 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> return err;
> }
>
> - for (i = 0; i < FASTRPC_DEV_MAX; i++) {
> - if (!strcmp(domains[i], domain)) {
> - domain_id = i;
> - break;
> - }
> - }
> + domain_id = fastrpc_get_domain_id(domain);
>
> if (domain_id < 0) {
> - dev_info(rdev, "FastRPC Invalid Domain ID %d\n", domain_id);
> + dev_info(rdev, "FastRPC Domain %s not supported\n", domain);
> return -EINVAL;
> }
>
> @@ -2332,19 +2337,19 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> case SDSP_DOMAIN_ID:
> /* Unsigned PD offloading is only supported on CDSP and CDSP1 */
> data->unsigned_support = false;
> - err = fastrpc_device_register(rdev, data, secure_dsp, domains[domain_id]);
> + err = fastrpc_device_register(rdev, data, secure_dsp, domain);
> if (err)
> goto fdev_error;
> break;
> case CDSP_DOMAIN_ID:
> - case CDSP1_DOMAIN_ID:
> + case GDSP_DOMAIN_ID:
> data->unsigned_support = true;
> /* Create both device nodes so that we can allow both Signed and Unsigned PD */
> - err = fastrpc_device_register(rdev, data, true, domains[domain_id]);
> + err = fastrpc_device_register(rdev, data, true, domain);
> if (err)
> goto fdev_error;
>
> - err = fastrpc_device_register(rdev, data, false, domains[domain_id]);
> + err = fastrpc_device_register(rdev, data, false, domain);
> if (err)
> goto populate_error;
> break;
> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
> index f33d914d8f46..89516abd258f 100644
> --- a/include/uapi/misc/fastrpc.h
> +++ b/include/uapi/misc/fastrpc.h
> @@ -133,6 +133,13 @@ struct fastrpc_mem_unmap {
> __s32 reserved[5];
> };
>
> +#define ADSP_DOMAIN_ID (0)
> +#define MDSP_DOMAIN_ID (1)
> +#define SDSP_DOMAIN_ID (2)
> +#define CDSP_DOMAIN_ID (3)
> +#define GDSP_DOMAIN_ID (4)
> +
> +#define FASTRPC_DOMAIN_MAX 4
> struct fastrpc_ioctl_capability {
> __u32 domain;
> __u32 attribute_id;
--
Thx and BRs,
Ling Xu
Powered by blists - more mailing lists