Re: [PATCH v19 2/7] 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: Infinite loop on RMI_BUSY in rmi_smccc_invoke()
- [Low] arm_rmm: Mismatch between RmiFeatureRegister comment and cache array size
--

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

firmware: arm_rmm: Check for RMI support at init

This commit introduces basic RMI support detection and initialization.
It queries the RMI version and reads the initial feature registers
to expose them for later use.

> diff --git a/drivers/firmware/arm_rmm/rmi.c b/drivers/firmware/arm_rmm/rmi.c
> new file mode 100644
> index 0000000000000..3baba931f92e4
> --- /dev/null
> +++ b/drivers/firmware/arm_rmm/rmi.c
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023-2026 ARM Ltd.
> + */
> +
> +#include <linux/cpufeature.h>
> +#include <linux/memblock.h>
> +#include <linux/arm-rmi-cmds.h>
> +#include <linux/slab.h>
> +
> +#include <asm/memory.h>
> +#include <asm/pgtable-hwdef.h>
> +
> +/* RMM defines RmiFeatureRegister0 to RmiFeatureRegister5. */
> +static unsigned long rmi_feat_reg_cache[5] __ro_after_init;

[Severity: Low]
Is the size of this array correct? The comment mentions registers 0 to 5,
which would require 6 elements.

When arm64_init_rmi() calls rmi_read_features(), it iterates up to
ARRAY_SIZE(rmi_feat_reg_cache) which is 5, silently ignoring the 6th
register during initialization. Any attempt to read index 5 via
rmi_feat_reg() will hit a WARN_ON and return 0.

[ ... ]

> diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi-cmds.h
> new file mode 100644
> index 0000000000000..5c6c563c81555
> --- /dev/null
> +++ b/include/linux/arm-rmi-cmds.h
> @@ -0,0 +1,48 @@

[ ... ]

> +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]
Can this loop run infinitely if the firmware continuously returns RMI_BUSY?

If RMM returns RMI_BUSY, the loop continues without incrementing the counter
i. While the retry count bounds RMI_BLOCKED, RMI_BUSY appears completely
unbounded. This could lead to a soft lockup and freeze the host CPU.

> +}

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