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

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Mon, 21 Sep 2026 11:14:22 +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  
> 
> ..
> 
> >>> +
> >>> +struct rec_exit {
> >>> +    union { /* 0x000 */
> >>> +        u8 exit_reason;
> >>> +        u8 padding0[0x100];  
> 
> ...
> 
> >>  
> >>> +    };
> >>> +    union { /* 0x400 */
> >>> +        struct {
> >>> +            u64 cntp_ctl;
> >>> +            u64 cntp_cval;
> >>> +            u64 cntv_ctl;
> >>> +            u64 cntv_cval;
> >>> +        };
> >>> +        u8 padding4[0x100];
> >>> +    };
> >>> +    union { /* 0x500 */
> >>> +        struct {
> >>> +            u64 ripas_base;
> >>> +            u64 ripas_top;
> >>> +            u8 ripas_value;
> >>> +            u8 padding5[0xf];  
> >>
> >> In various other places you just use a u64 for a u8
> >> + padding. Why is this one special? And for that matter
> >> various other fields later in this particular structure?  
> > 
> > Please feel free to point out, if you spot any. I am happy
> > to fix them.  
> 
> I took a look and the padding is for extra "u64" bits in there.
> e.g, ripas_value offset = 0x510 and the s2ap_base is really at 0x520
> not 0x518. Same applies for the pmu_ovf_status and vsmmu.
> 
> So, I would leave them as they are. The rest are consistent.
> e.g, num_bps and num_wps. in realm_parameter
Ok. That's logical enough for me.  When no padding happens to follow
that goes beyond the u64 then use that for simplicity and if there
is padding anyway use a u8[] that include the rest of the u64 and
the next bit

Thanks for talking me through it!

Jonathan
> 
> Cheers
> Suzuki
> 
> 
> >