Re: [PATCH v18 1/7] firmware: arm_rmm: Add SMC definitions for calling the RMM

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 21/09/2026 22:27, Jonathan Cameron wrote:
> On Mon, 21 Sep 2026 11:02:56 +0100
> Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> 
>> On 21/09/2026 10:27, Suzuki K Poulose wrote:
>>> On 19/09/2026 02:27, Jonathan Cameron wrote:
>>>>> The RMM (Realm Management Monitor) provides functionality that can be
>>>>> accessed by SMC calls from the host.
>>>>>
>>>>> The SMC definitions are based on DEN0137[1] version 2.0-bet3
>>>>>
>>>>> [1] https://developer.arm.com/documentation/den0137/2-0bet3/
>>>>>
>>>>> Signed-off-by: Steven Price <steven.price@arm.com>
>>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>>
>>>> With Gavin's nitpicks and the GENMASK_ULL() from sashiko, just a few
>>>> comments inline.  Mostly on subtle inconsistencies that really don't
>>>> matter that much.
>>>>   
>>>>>    include/linux/arm-smccc-rmi.h | 497 ++++++++++++++++++++++++++++++++++
>>>>>    1 file changed, 497 insertions(+)
>>>>>    create mode 100644 include/linux/arm-smccc-rmi.h
>>>>>
>>>>> diff --git a/include/linux/arm-smccc-rmi.h b/include/linux/arm-smccc-
>>>>> rmi.h
>>>>> new file mode 100644
>>>>> index 000000000000..214d6228dfc2
>>>>> --- /dev/null
>>>>> +++ b/include/linux/arm-smccc-rmi.h
>>>>> @@ -0,0 +1,497 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>>> +/*
>>>>> + * Copyright (C) 2023-2026 ARM Ltd.
>>>>> + *
>>>>> + * The values and structures in this file are from the Realm
>>>>> Management Monitor
>>>>> + * specification (DEN0137) version 2.0-bet3:
>>>>> + * https://developer.arm.com/documentation/den0137/2-0bet3/
>>>>> + */
>>>>> +
>>>>> +#ifndef __LINUX_ARM_SMCCC_RMI_H_
>>>>> +#define __LINUX_ARM_SMCCC_RMI_H_
>>>>> +
>>>>> +#include <linux/arm-smccc.h>
>>>>> +#include <linux/bitfield.h>
>>>>> +#include <linux/bits.h>
>>>>> +#include <linux/build_bug.h>
>>>>> +#include <linux/sizes.h>
>>>>> +
>>>>> +#include <asm/page.h>
>>>>> +
>>>>> +#define SMC_RMI_CALL(func)                \
>>>>> +    ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,        \
>>>>> +               ARM_SMCCC_SMC_64,        \
>>>>> +               ARM_SMCCC_OWNER_STANDARD,    \
>>>>> +               (func))
>>>>
>>>> Obviously it is v18 so probably a future thing but nothing about this
>>>> is RMI specific.  Could be used for ARM_SMCCC_TRNG_RND64 for instance.
>>>> I'm not entirely sure what we'd call such a macro
>>>>
>>>> ARM_SMCCC_CALL_VAL64_STD() maybe?
>>>
>>> ARM_SMCCC_STD_CALL64_VAL() ?
>>>
>>> But, I would leave it as a wider cleanup in the tree as a separate
>>> series.
>>>    
>>>>
>>>> I'm not just not keen on macros whose names to me hint at something
>>>> special. FWIW this also matches SMC_RSI_FID() and FFA_SMC64().
>>>> Isn't it nice when we have a predictable naming scheme :)
>>>>
>>>> FID (for Function IDentifier) is in the spec, so maybe?
>>>>
>>>> Anyhow, I don't really care that much.
>>>>   
>>>>> +
>>>>> +#define SMC_RMI_VERSION                SMC_RMI_CALL(0x0150)
>>>>> +
>>>>
>>>>   
>>>>> +#define RMI_ABI_MAJOR_VERSION    2
>>>>> +#define RMI_ABI_MINOR_VERSION    0
>>>>> +
>>>>> +#define RMI_ABI_VERSION_GET_MAJOR(version) ((version) >> 16)
>>>>
>>>> I'd mask it.  Mostly because that would shout that it is only 15 bits.
>>>>   
>>>
>>> Ack
>>>    
>>>>> +#define RMI_ABI_VERSION_GET_MINOR(version) ((version) & 0xFFFF)
>>>>> +#define RMI_ABI_VERSION(major, minor)      (((major) << 16) | (minor))
>>>>
>>>> I'd go all in on FIELD_PREP() / FIELD_GET() + GENMASK just for the
>>>> sake of consistency + not having to be careful that everything is
>>>> checked for fit to keep the LLM bots happy.
>>>>   
>>>>> +
>>>>> +#define RMI_RETURN_STATUS_MASK        GENMASK(7, 0)
>>>>> +#define RMI_RETURN_INDEX_MASK        GENMASK(15, 8)
>>>>> +#define RMI_RETURN_MEMREQ_MASK        GENMASK(9, 8)
>>>>> +#define RMI_RETURN_CAN_CANCEL_MASK    BIT(10)
>>>>> +
>>>>> +#define RMI_RETURN_STATUS(ret)
>>>>> FIELD_GET(RMI_RETURN_STATUS_MASK, ret)
>>>>> +#define RMI_RETURN_INDEX(ret)
>>>>> FIELD_GET(RMI_RETURN_INDEX_MASK, ret)
>>>>
>>>> What's this one?  I can't find anything in the spec that matches it
>>>> and as far as I can tell you don't use it in this series.
>>>
>>> This is coming from RmiResultDataLevel. See RmiResult type.
>>> This was renamed after we introduce the RmiResultDataIncomplete.
>>> It is used in the KVM code to find the "level" where a command
>>> failed/walked.
>>>
>>> I could rename it to RMI_RESULT_DATA_LEVEL() ?
>>> Similarly RMI_RESULT_STATUS instead of RMI_RETURN_*
>>>    
>>>>   
>>>>> +#define RMI_RETURN_MEMREQ(ret)
>>>>> FIELD_GET(RMI_RETURN_MEMREQ_MASK, ret)
>>>>> +#define RMI_RETURN_CAN_CANCEL(ret)
>>>>> FIELD_GET(RMI_RETURN_CAN_CANCEL_MASK, ret)
>>>    
>>>> These are obscure enough to find in the spec I'd give a comment just
>>>> to save the sanity of anyone looking for them.
>>>
>>> As above, they are really RMI_RESULT_DATA_INCOMPLETE_*
>>>    
>>>>   
>>>>> +/*
>>>>> + * Note many of these fields are smaller than u64 but all fields
>>>>> have u64
>>>>> + * alignment, so use u64 to ensure correct alignment.
>>>>
>>>> Obviously this is only going to run on arm64 so it's not critical, but
>>>> more generally u64s aren't always 64 bit aligned.  So if you 'really'
>>>> care aligned_u64 is there to ensure it.  Meh, arm64 so fine.
>>>
>>> Agreed, I am worried about the churn in the consumer code. Also, like
>>> you said, this is only for ARM64. So, I would pass it.
>>>    
>>>>   
>>>>> + */
>>>>> +struct rmm_config {
>>>>> +    union { /* 0x0 */
>>>>> +        struct {
>>>>> +            u64 tracking_region_size;
>>>>> +            u64 rmi_granule_size;
>>>>> +        };
>>>>> +        u8 sizer[SZ_4K];
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +static_assert(sizeof(struct rmm_config) == SZ_4K);
>>>>> +
>>>>> +#define RMI_REALM_PARAM_FLAG_SVE        BIT(1)
>>>>> +#define RMI_REALM_PARAM_FLAG_PMU        BIT(2)
>>>>> +#define RMI_REALM_PARAM_FLAG_DA            BIT(3)
>>>>> +#define RMI_REALM_PARAM_FLAG_LFA_POLICY        GENMASK(6, 5)
>>>>> +#define RMI_REALM_PARAM_FLAG_MEC_POLICY        GENMASK(8, 7)
>>>>
>>>> Tiny bit inconsistent. When do you decide _MASK is needed and when
>>>> not?  Seems a little too random for multibit fields.
>>>
>>> I will try to clean this up.
>>
>> Actually, this is used when we don't consume them from RMM. i.e.,
>> we never call FIELD_GET() on them. But thats not a reason for
>> not being consistent in naming. I can fix it
> 
> Too obscure for me!  So if not too painful cleaner to just fix it.

Sorry, forgot to add that, I fixed them to _MASK

Cheers
Suzuki

> 
> Jonathan
> 
>>
>> Cheers
>> Suzuki
>>
>>
>