Re: [PATCH v20 2/9] firmware: arm_rmm: Check for RMI support at init

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

On 29/09/2026 23:28, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] arm_rmm: unbounded spin loop on RMI_BUSY without scheduling
> - [Medium] arm_rmm: commit message and code mismatch on feature registers count
> --
> 
> commit 5ddd90a4b6dcf005d84ebc9f9003eb3f383394f8
> Author: Steven Price <steven.price@arm.com>
> 
> firmware: arm_rmm: Check for RMI support at init
> 
> This commit queries the RMI version to ensure compatibility and reads
> feature registers from the Realm Management Monitor (RMM). It also exposes
> these feature registers for future use.
> 
>> The first two feature registers are read and exposed for future code to
>> use.
> 
> [Severity: Medium]
> Is this description accurate? The commit message explicitly claims that the
> first two feature registers are read, whereas the actual implementation in
> rmi_read_features() iterates over ARRAY_SIZE(rmi_feat_reg_cache) (which is
> 5) to read all 5 feature registers. This looks like the commit message
> might have missed the v19 code change.

Ack. I have fixed this locally.

> 
>> diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi-cmds.h
>> new file mode 100644
>> index 0000000000000..911489b636522
>> --- /dev/null
>> +++ b/include/linux/arm-rmi-cmds.h
> [ ... ]
>> +static inline void rmi_smccc_invoke(struct arm_smccc_1_2_regs *regs)
>> +{
>> +	struct arm_smccc_1_2_regs args = *regs;
>> +	long status;
>> +	int i = 0;
>> +
>> +	while (i < RMM_BLOCKED_RETRY_COUNT) {
>> +		arm_smccc_1_2_invoke(&args, regs);
>> +
>> +		status = RMI_RESULT_STATUS(regs->a0);
>> +		if (status != RMI_BUSY && status != RMI_BLOCKED)
>> +			break;
>> +		if (status == RMI_BLOCKED)
>> +			i++;
>> +		cpu_relax();
>> +	}
> 
> [Severity: High]
> Does this sequence create an unbounded spin loop if the firmware continuously
> returns RMI_BUSY?
> 
> If the status returned by arm_smccc_1_2_invoke() is RMI_BUSY, the loop
> continues without incrementing the exit counter i. It issues cpu_relax()
> but does not yield the CPU (e.g., via cond_resched()) or impose a timeout.
> If the RMM firmware remains busy, this could cause the host kernel to lock
> up indefinitely.

This is like "spinlock contention". RMM returns EBUSY when a resource is
locked temporarily. We should be able to make progress, unlike the
RMI_BLOCKED, which is blocking due to a long running operation.

Cheers
Suzuki