Re: [PATCH v22 02/10] firmware: arm_rmm: Check for RMI support at init
From: Gavin Shan
Date: Wed Oct 14 2026 - 07:04:46 EST
On 10/14/26 7:19 PM, Suzuki K Poulose wrote:
On 14/10/2026 08:26, Gavin Shan wrote:
On 10/13/26 7:58 PM, Suzuki K Poulose wrote:
On 13/10/2026 04:53, Gavin Shan wrote:
Hi Suzuki,
On 10/2/26 4:14 PM, Suzuki K Poulose wrote:
From: Steven Price <steven.price@xxxxxxx>
Query the RMI version number and check if it is a compatible version.
All RMM feature registers are read and exposed for future code to
use. Also restrict the support to SMC conduit.
We only support this for Little Endian kernels, the Big Endian kernel
support is anyway marked BROKEN and is being removed.
Cc: Marc Zyngier <maz@xxxxxxxxxx>
Cc: Oliver Upton <oupton@xxxxxxxxxx>
Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxxxxxxxx>
Reviewed-by: Gavin Shan <gshan@xxxxxxxxxx>
Reviewed-by: Catalin Marinas <catalin.marinas@xxxxxxx>
Tested-by: Gavin Shan <gshan@xxxxxxxxxx>
Signed-off-by: Steven Price <steven.price@xxxxxxx>
Co-developed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
---
[...]
---
MAINTAINERS | 2 +
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/cpufeature.c | 1 +
arch/arm64/kvm/sys_regs.c | 2 +
drivers/firmware/Kconfig | 1 +
drivers/firmware/Makefile | 1 +
drivers/firmware/arm_rmm/Kconfig | 25 +++++++
drivers/firmware/arm_rmm/Makefile | 1 +
drivers/firmware/arm_rmm/rmi.c | 112 ++++++++++++++++++++++++++ ++++
include/linux/arm-rmi-cmds.h | 48 +++++++++++++
10 files changed, 194 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/drivers/firmware/arm_rmm/rmi.c b/drivers/firmware/ arm_rmm/rmi.c
new file mode 100644
index 0000000000000..06f5392c982d2
--- /dev/null
+++ b/drivers/firmware/arm_rmm/rmi.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023-2026 ARM Ltd.
+ */
+
+#include <linux/arm-rmi-cmds.h>
+#include <linux/cpufeature.h>
+#include <linux/memblock.h>
+#include <linux/slab.h>
+
+#include <asm/memory.h>
+#include <asm/pgtable-hwdef.h>
+
+/* RMM v2.0 defines RmiFeatureRegister0 to RmiFeatureRegister4. */
+static unsigned long rmi_feat_reg_cache[5] __ro_after_init;
+
+static int rmi_check_version(void)
+{
+ unsigned short version_major, version_minor;
+ unsigned long host_version = RMI_ABI_VERSION(RMI_ABI_MAJOR_VERSION,
+ RMI_ABI_MINOR_VERSION);
+ unsigned long aa64pfr0 = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
+ struct arm_smccc_1_2_regs res = {
+ SMC_RMI_VERSION, host_version,
+ };
+
+ /* If RME isn't supported, then RMI can't be */
+ if (cpuid_feature_extract_unsigned_field(aa64pfr0, ID_AA64PFR0_EL1_RME_SHIFT) == 0)
+ return -ENXIO;
+
+ if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_SMC)
+ return -ENXIO;
I think we need to ensure SMCCC version is v1.2 or a newer one here? arm_smccc_1_2_smc()
is immediately called by rmi_smccc_invoke() after this check.
if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_SMC ||
arm_smccc_get_version() < ARM_SMCCC_VERSION_1_2)
We could. But I am wondering if that adds any value ? If the RMM
exists and responds to the RMI_ABI_VERSION, it must be so on a
platform with v1.2 or greater ? The extra check doesn't hurt.
It's required as documented in section "B1.1 overview" of RMM-spec-v2.0- bet3.
In the same section, there are more requirements regarding the SMCCC version
when SVE/SME are supported. I don't see a practical problem if the check is
missed since the combination of TF-A and TF-RMM are the only RMM-spec- v2.0-bet3
implementation (as I knew) where the SMCCC version has been set to v1.5 in the
'master' branch.
Reading the RMM spec again, DFNMZ lists how one can detect if the RMM
ABI is present. That specifies that the user checks for SMCCC
version >= 1.1, which we do by using arm_smccc_1_1_get_conduit().
Then checking the R{M,S}I_VERSION call is sufficient. So, I think
we are good here. Do you agree ?
arm_smccc_1_1_get_conduit() ensures SMCCC version >= v1.1, but we need
SMCCC version >= v1.2. We will have problem if the firmware only supports
SMCCC v1.1 and R{M,S}I_VERSION is coincidentally supported. In this
rare (even impossible) case, it's not sufficient to check R{M,S}I_VERSION
as I understand.
As far as TF-A+TF-RMM is concerned, I agree there is no issue. However,
the extra check doesn't hurt and make the code following the spec a bit
more strictly? :-)
Thanks,
Gavin
Suzuki
Thanks,
Gavin
Suzuki
return -ENXIO;
Thanks,
Gavin
+
+ rmi_smccc_invoke(&res);
+ if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
+ return -ENXIO;
+
+ version_major = RMI_ABI_VERSION_GET_MAJOR(res.a1);
+ version_minor = RMI_ABI_VERSION_GET_MINOR(res.a1);
+
+ if (res.a0 != RMI_SUCCESS) {
+ unsigned short high_version_major, high_version_minor;
+
+ high_version_major = RMI_ABI_VERSION_GET_MAJOR(res.a2);
+ high_version_minor = RMI_ABI_VERSION_GET_MINOR(res.a2);
+
+ pr_err("Unsupported RMI ABI (v%d.%d - v%d.%d) we want v%d. %d\n",
+ version_major, version_minor,
+ high_version_major, high_version_minor,
+ RMI_ABI_MAJOR_VERSION,
+ RMI_ABI_MINOR_VERSION);
+ return -ENXIO;
+ }
+
+ pr_info("RMI ABI version %d.%d\n", version_major, version_minor);
+
+ return 0;
+}
+
[...]