Re: [PATCH v18 2/7] firmware: arm_rmm: Check for RMI support at init

[Sudeep Holla]

On Sat, Sep 12, 2026 at 09:36:05AM +0100, Suzuki K Poulose wrote:
> From: Steven Price <steven.price@xxxxxxx>
> 
> Query the RMI version number and check if it is a compatible version.
> The first two feature registers are read and exposed for future code to
> use.
> 
> We only support this for Little Endian kernels, the Big Endian kernel
> support is anyway marked BROKEN and is being removed.
> 
> Signed-off-by: Steven Price <steven.price@xxxxxxx>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> ---
> v18:
>  * Always use arm_smccc_1_2_invoke() for all RMIs making sure the unsused
>    parameters are 0 - Sashiko
>  * Move rmi_features() calls away from the arm-rmi-cmds.h to rmi.c - Gavin
> v17:
>  * Rename ARM_RMM to ARM_RMM_RMI to make it easier to add Guest facing RSI
>    support, which is also in progress
> v16:
>  * Update Kconfig text to include PCIe TDISP.
>  * Export rmi_feat_reg() here rather than in a later commit.
> v15:
>  * The code is moved again, this time into the 'firmware' directory.
> v14:
>  * This moves the basic RMI setup into the 'kernel' directory. This is
>    because RMI will be used for some features outside of KVM so should
>    be available even if KVM isn't compiled in.
> ---
>  arch/arm64/Kconfig                |   1 +
>  arch/arm64/kernel/cpufeature.c    |   1 +
>  drivers/firmware/Kconfig          |   1 +
>  drivers/firmware/Makefile         |   1 +
>  drivers/firmware/arm_rmm/Kconfig  |  26 +++++++
>  drivers/firmware/arm_rmm/Makefile |   2 +
>  drivers/firmware/arm_rmm/rmi.c    | 122 ++++++++++++++++++++++++++++++
>  include/linux/arm-rmi-cmds.h      |  36 +++++++++
>  8 files changed, 190 insertions(+)
>  create mode 100644 drivers/firmware/arm_rmm/Kconfig
>  create mode 100644 drivers/firmware/arm_rmm/Makefile
>  create mode 100644 drivers/firmware/arm_rmm/rmi.c
>  create mode 100644 include/linux/arm-rmi-cmds.h
> 

[...]

> diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi-cmds.h
> new file mode 100644
> index 0000000000000..9792bf0e00cb9
> --- /dev/null
> +++ b/include/linux/arm-rmi-cmds.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2026 ARM Ltd.
> + */
> +
> +#ifndef __LINUX_ARM_RMI_CMDS_H_
> +#define __LINUX_ARM_RMI_CMDS_H_
> +
> +#include <linux/arm-smccc-rmi.h>
> +#include <linux/bug.h>
> +#include <linux/processor.h>
> +#include <linux/types.h>
> +
> +
> +/*
> + * rmi_smccc_invoke: Invoke the RMI call and return the results in @regs_out
> + * @regs_in: Registers with the arguments filled in.
> + * @regs_out: Ouptput results from the call.
> + */
> +static inline void rmi_smccc_invoke(struct arm_smccc_1_2_regs *regs)
> +{
> +	struct arm_smccc_1_2_regs args = *regs;
> +	unsigned long status;
> +
> +	while (1) {
> +		arm_smccc_1_2_invoke(&args, regs);
> +		status = RMI_RETURN_STATUS(regs->a0);
> +		if (status != RMI_BUSY && status != RMI_BLOCKED)
> +			break;
> +		cpu_relax();
> +	}
> +}

I haven't done a detailed review, this is just a drive through comment.
The while(1) gained my attention.

Should RMI_BLOCKED be returned to the caller instead of retried here?

RMM spec defines RMI_BLOCKED as persisting until the Host takes action.
It also says it is returned when another SRO on the same context is
incomplete. You may be running it on different CPUs and hence different
context I assume. But this loop takes no such action and hides the status
from the caller, so a command issued against that context if that can
happen can spin indefinitely while the operation which would unblock it
cannot run. Ignore me if it taken care not to happen elsewhere. I am
just looking at this in isolation.

Could this retry only RMI_BUSY and propagate RMI_BLOCKED so that the caller
can arrange for the incomplete operation to make progress if the above
scenario is possible ?

-- 
Regards,
Sudeep