Re: [PATCH v17 5/7] firmware: arm_rmm: Activate the RMM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The macro `rmi_sro_memxfer_cmd` uses a large compound literal to initialize the SRO state, forcing the compiler to allocate a ~2.2KB temporary object on the stack.
--

commit c773f8f03d602782404ec2f0103977a8f86f3b3e
Author: Steven Price <steven.price@arm.com>

firmware: arm_rmm: Activate the RMM

Activate the RMM after the basic configuration. This is a memory transferring,
stateful operation.

> diff --git a/drivers/firmware/arm_rmm/rmi.c b/drivers/firmware/arm_rmm/rmi.c
> index 42c973c3a98bb..d969c8738efde 100644
> --- a/drivers/firmware/arm_rmm/rmi.c
> +++ b/drivers/firmware/arm_rmm/rmi.c
[ ... ]
> @@ -656,7 +657,19 @@ static int __init arm64_init_rmi(void)
>  	if (ret)
>  		return ret;
>  
> -	return 0;
> +	/* Activate the RMM */
> +	sro = kmalloc_obj(*sro);
> +	if (!sro)
> +		return -ENOMEM;
> +
> +	ret = rmi_rmm_activate(sro);

[Severity: Medium]
Does expanding rmi_rmm_activate() here cause stack frame bloat in
arm64_init_rmi()?

Looking at include/linux/arm-rmi-cmds.h, rmi_rmm_activate() calls the
rmi_sro_memxfer_cmd() macro, which evaluates a large compound literal:

*__sro = (struct rmi_sro_state){ .regs = {__VA_ARGS__} };

This forces the compiler to allocate a ~2.2KB temporary object on the stack
before copying it to the heap object, which defeats the purpose of the
explicit heap allocation above. Does this risk exceeding the typical
CONFIG_FRAME_WARN threshold of 2048 bytes on arm64 and lead to build
warnings?

> +	if (ret) {
> +		pr_err("RMM activate failed\n");
> +		ret = ret < 0 ? ret : -ENXIO;
> +	}
> +
> +	kfree(sro);
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907095942.1140734-1-suzuki.poulose@arm.com?part=5