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

[sashiko-bot@kernel.org]

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.

> 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.

> +}

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