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

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 07/09/2026 11:17, sashiko-bot@kernel.org wrote:
> 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?

I have switched to using memset() to initialise the sro and then 
initialize the regs from the __VA_ARGS__


diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi-cmds.h
index 47c31808b8a7e..1e4dd02124ebd 100644
--- a/include/linux/arm-rmi-cmds.h
+++ b/include/linux/arm-rmi-cmds.h
@@ -8,6 +8,7 @@

  #include <linux/arm-smccc-rmi.h>
  #include <linux/bug.h>
+#include <linux/string.h>
  #include <linux/types.h>

  #define RMI_MAX_ADDR_LIST      256
@@ -43,7 +44,8 @@ long rmi_sro_execute(struct arm_smccc_1_2_regs *regs);

  #define rmi_sro_memxfer_cmd(sro, gfp, ...) ({                          \
         struct rmi_sro_state *__sro = (sro);                            \
-       *__sro = (struct rmi_sro_state){ .regs = {__VA_ARGS__} };       \
+       memset(__sro, 0, sizeof(*__sro));                               \
+       __sro->regs = (struct arm_smccc_1_2_regs){ __VA_ARGS__ };       \
         long __ret = rmi_sro_memxfer_execute(__sro, gfp);               \
         rmi_sro_free(__sro);                                            \
         __ret;                                                          \

Cheers
Suzuki


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