optee: make read-only array attr static const

[PATCH][next] optee: make read-only array attr static const

Posted by Colin Ian King 4 weeks, 1 day ago

Don't populate the read-only array attr on the stack at run
time, instead make it static const.

Signed-off-by: Colin Ian King <coking@nvidia.com>
---
 drivers/tee/optee/rpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
index 97fc5b14db0c..1758eb7e6e8b 100644
--- a/drivers/tee/optee/rpc.c
+++ b/drivers/tee/optee/rpc.c
@@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
 	struct i2c_msg msg = { };
 	size_t i;
 	int ret = -EOPNOTSUPP;
-	u8 attr[] = {
+	static const u8 attr[] = {
 		TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
 		TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
 		TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
-- 
2.51.0

Re: [PATCH][next] optee: make read-only array attr static const

Posted by David Laight 3 weeks ago

On Fri,  9 Jan 2026 15:44:42 +0000
Colin Ian King <coking@nvidia.com> wrote:

> Don't populate the read-only array attr on the stack at run
> time, instead make it static const.

For a 4 byte array initialising on stack is likely to generate faster code.
In particular it avoid a very likely data-cache miss for the rodata.
The on-stack array access might even shorter code than accessing a global
array on some architectures.
The compiler might, of course, decide to unroll the loop and not instantiate
the array at all (in either case).
Indeed, just unrolling the loop as:
	if (params[0].attr != TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT) goto bad;
	if (params[1].attr != TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT) goto bad;
	if (params[2].attr != TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT) goto bad;
	if (params[3].attr != TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT) goto bad;
is likely to be pretty near optimal.
(Collecting the bytes and doing a single 32bit compare may be better.)

Of course this won't be a massively hot path - so the code choice won't
make a measurable difference.

	David


> 
> Signed-off-by: Colin Ian King <coking@nvidia.com>
> ---
>  drivers/tee/optee/rpc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> index 97fc5b14db0c..1758eb7e6e8b 100644
> --- a/drivers/tee/optee/rpc.c
> +++ b/drivers/tee/optee/rpc.c
> @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
>  	struct i2c_msg msg = { };
>  	size_t i;
>  	int ret = -EOPNOTSUPP;
> -	u8 attr[] = {
> +	static const u8 attr[] = {
>  		TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
>  		TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
>  		TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,

Re: [PATCH][next] optee: make read-only array attr static const

Posted by Sumit Garg 3 weeks, 5 days ago

On Fri, Jan 09, 2026 at 03:44:42PM +0000, Colin Ian King wrote:
> Don't populate the read-only array attr on the stack at run
> time, instead make it static const.

Is there any value add to do this? AFAIK, the static local variables
aren't preffered.

-Sumit

> 
> Signed-off-by: Colin Ian King <coking@nvidia.com>
> ---
>  drivers/tee/optee/rpc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> index 97fc5b14db0c..1758eb7e6e8b 100644
> --- a/drivers/tee/optee/rpc.c
> +++ b/drivers/tee/optee/rpc.c
> @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
>  	struct i2c_msg msg = { };
>  	size_t i;
>  	int ret = -EOPNOTSUPP;
> -	u8 attr[] = {
> +	static const u8 attr[] = {
>  		TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
>  		TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
>  		TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
> -- 
> 2.51.0
>

Re: [PATCH][next] optee: make read-only array attr static const

Posted by Dan Carpenter 3 weeks, 5 days ago

On Mon, Jan 12, 2026 at 04:56:10PM +0530, Sumit Garg wrote:
> On Fri, Jan 09, 2026 at 03:44:42PM +0000, Colin Ian King wrote:
> > Don't populate the read-only array attr on the stack at run
> > time, instead make it static const.
> 
> Is there any value add to do this? AFAIK, the static local variables
> aren't preffered.
> 

I have never heard that static local variables aren't prefered.  I
imagine that because they're often not thread safe?  But if they're
const thread safeness isn't an problem.  Generally, we prefer the
scope as local as possible.

regards,
dan carpenter

Re: [PATCH][next] optee: make read-only array attr static const

Posted by ryan foster 3 weeks, 5 days ago

This looks like a micro-optimization, const makes the lookup array
explicitly immutable, and static keeps it out of the stack frame,
avoiding per-call initialization.

Is there a style preference for read only lookup arrays here, e.g.
Should these variables remain local but not static, or should they be
moved to file scope static const?

On Mon, Jan 12, 2026 at 3:26 AM Sumit Garg <sumit.garg@kernel.org> wrote:
>
> On Fri, Jan 09, 2026 at 03:44:42PM +0000, Colin Ian King wrote:
> > Don't populate the read-only array attr on the stack at run
> > time, instead make it static const.
>
> Is there any value add to do this? AFAIK, the static local variables
> aren't preffered.
>
> -Sumit
>
> >
> > Signed-off-by: Colin Ian King <coking@nvidia.com>
> > ---
> >  drivers/tee/optee/rpc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> > index 97fc5b14db0c..1758eb7e6e8b 100644
> > --- a/drivers/tee/optee/rpc.c
> > +++ b/drivers/tee/optee/rpc.c
> > @@ -43,7 +43,7 @@ static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
> >       struct i2c_msg msg = { };
> >       size_t i;
> >       int ret = -EOPNOTSUPP;
> > -     u8 attr[] = {
> > +     static const u8 attr[] = {
> >               TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> >               TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> >               TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
> > --
> > 2.51.0
> >
>