Subject: Re: [PATCH v14 06/44] arm64: RMI: Check for RMI support at init

From: Gavin Shan <[email-masked]>
Date: Mon, 25 May 2026 16:58:44 +1000

Hi Steve,

On 5/22/26 1:49 AM, Steven Price wrote:
> On 21/05/2026 01:39, Gavin Shan wrote:
>> On 5/13/26 11:17 PM, Steven Price wrote:
>>> 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.
>>>
>>> Signed-off-by: Steven Price <[email-masked]>
>>> ---
>>> 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/include/asm/rmi_cmds.h |  3 ++
>>>    arch/arm64/kernel/Makefile        |  2 +-
>>>    arch/arm64/kernel/cpufeature.c    |  1 +
>>>    arch/arm64/kernel/rmi.c           | 65 +++++++++++++++++++++++++++++++
>>>    4 files changed, 70 insertions(+), 1 deletion(-)
>>>    create mode 100644 arch/arm64/kernel/rmi.c
>>>
>>
>> [...]
>>
>>> diff --git a/arch/arm64/kernel/rmi.c b/arch/arm64/kernel/rmi.c
>>> new file mode 100644
>>> index 000000000000..99c1ccc35c11
>>> --- /dev/null
>>> +++ b/arch/arm64/kernel/rmi.c
>>> @@ -0,0 +1,65 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2023-2025 ARM Ltd.
>>> + */
>>> +
>>> +#include <linux/memblock.h>
>>> +
>>> +#include <asm/rmi_cmds.h>
>>> +
>>> +unsigned long rmm_feat_reg0;
>>> +unsigned long rmm_feat_reg1;
>>> +
>>> +static int rmi_check_version(void)
>>> +{
>>> +    struct arm_smccc_res res;
>>> +    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);
>>> +
>>> +    /* 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;
>>> +
>>> +    arm_smccc_1_1_invoke(SMC_RMI_VERSION, host_version, &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;
>>> +}
>>> +
>>> +static int __init arm64_init_rmi(void)
>>> +{
>>> +    /* Continue without realm support if we can't agree on a version */
>>> +    if (rmi_check_version())
>>> +        return 0;
>>
>> Is this still a valid point that we have to return zero on errors returned
>> from rmi_check_version() or other other function calls like rmi_features()?
>> arm64_init_rmi() is triggered by subsys_initcall() where the return value
>> needs to indicate success or failure. It's fine to return error code from
>> arm64_init_rmi() in the path.
> 
> Hmm, I guess now this is moved to arm64 code this indeed doesn't need
> to. Within a module I believe an error return can fail the module loading.
> 
> I'm not sure it really makes much difference though - if this
> initialisation fails then it's not really an error - it just means the
> feature is unavailable.
> 

I think the return value would be consistent to the value of 'arm64_rmi_is_available'.
'arm64_rmi_is_available' is true when zero is returned, otherwise, 'arm64_rmi_is_available'
is false.

With the consistency between the return value and 'arm64_rmi_is_available', users are
able to know the value of 'arm64_rmi_is_available' through kernel parameter 'initcall_debug'.
With the kernel parameter, the initcalls including arm64_init_rmi() are traced and its
return value is outputted in the traced messages, seeing do_trace_initcall_start().

> Thanks,
> Steve
> 
>>> +
>>> +    if (WARN_ON(rmi_features(0, &rmm_feat_reg0)))
>>> +        return 0;
>>> +    if (WARN_ON(rmi_features(1, &rmm_feat_reg1)))
>>> +        return 0;
>>> +
>>> +    return 0;
>>> +}
>>> +subsys_initcall(arm64_init_rmi);
>>

Thanks,
Gavin