Received-SPF: pass (google.com: domain of linux-kernel+bounces-192371-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 28 May 2024 16:12:17 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Ekansh Gupta <quic_ekangupt@quicinc.com>
Cc: srinivas.kandagatla@linaro.org, linux-arm-msm@vger.kernel.org, 
	gregkh@linuxfoundation.org, quic_bkumar@quicinc.com, linux-kernel@vger.kernel.org, 
	quic_chennak@quicinc.com
Subject: Re: [PATCH v2 8/8] misc: fastrpc: Add system unsigned PD support
Message-ID: <zwlzdhb5zn2wx6bew437hzj7l6nsj2ccekkkabatykmtzihotr@5ocy5wqxtoah>
References: <20240528112956.5979-1-quic_ekangupt@quicinc.com>
 <20240528112956.5979-9-quic_ekangupt@quicinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240528112956.5979-9-quic_ekangupt@quicinc.com>

On Tue, May 28, 2024 at 04:59:54PM +0530, Ekansh Gupta wrote:
> Trusted CPU applications currently offload to signed PDs on CDSP to
> gain some additional services provided by root PD. Unsigned PDs have
> access to limited root PD services that may not be sufficient for
> all use-cases. Signed PDs have a higher dynamic loading latency
> which impacts the performance of applications. Limited root PD
> services could be opened up for unsigned PDs but that should be
> restricted for untrusted processes. For this requirement, System
> unsigned PD is introduced which will be same as Unsigned PD for
> most part but will have access to more root PD services. Add
> changes to offload trusted applications to System unsigned PD
> when unsigned offload is requested.
> 
> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> ---
>  drivers/misc/fastrpc.c      | 9 +++++++++
>  include/uapi/misc/fastrpc.h | 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 7f81a18b8aea..ebe20915392f 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -122,6 +122,7 @@
>  enum fastrpc_userpd_type {
>  	SIGNED_PD			= 1,
>  	UNSIGNED_PD			= 2,
> +	SYSTEM_UNSIGNED_PD	= 3,

Any reason why this is not indented properly? Maybe you have
non-standard tab width?

Also the usage of non-bool type makes sense here, not in the 'unisgned
PD' patch. Please use bool beforehand and switch to an enum here.

>  };
>  
>  static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp",
> @@ -1552,12 +1553,20 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>  	if (init.attrs & FASTRPC_MODE_UNSIGNED_MODULE)
>  		fl->userpd_type = UNSIGNED_PD;
>  
> +	/* Disregard any system unsigned PD attribute from userspace */
> +	init.attrs &= (~FASTRPC_MODE_SYSTEM_UNSIGNED_PD);
>  
>  	if (is_session_rejected(fl, !(fl->userpd_type == SIGNED_PD))) {
>  		err = -EACCES;
>  		goto err;
>  	}
>  
> +	/* Trusted apps will be launched as system unsigned PDs */
> +	if (!fl->untrusted_process && (fl->userpd_type != SIGNED_PD)) {
> +		fl->userpd_type = SYSTEM_UNSIGNED_PD;
> +		init.attrs |= FASTRPC_MODE_SYSTEM_UNSIGNED_PD;
> +	}
> +
>  	if (init.filelen > INIT_FILELEN_MAX) {
>  		err = -EINVAL;
>  		goto err;
> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
> index f33d914d8f46..3b3279bb2cf9 100644
> --- a/include/uapi/misc/fastrpc.h
> +++ b/include/uapi/misc/fastrpc.h
> @@ -62,6 +62,8 @@ enum fastrpc_proc_attr {
>  	FASTRPC_MODE_SYSTEM_PROCESS	= (1 << 5),
>  	/* Macro for Prvileged Process */
>  	FASTRPC_MODE_PRIVILEGED		= (1 << 6),
> +	/* Macro for system unsigned PD */
> +	FASTRPC_MODE_SYSTEM_UNSIGNED_PD	= (1 << 17),
>  };
>  
>  /* Fastrpc attribute for memory protection of buffers */
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry