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

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

On 12/09/2026 09:45, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Usage of GENMASK and BIT macros for 64-bit masks causes Undefined Behavior on 32-bit architectures

RMM is only supported for ARM64. But, given this is a header file, I 
could fix them to ULL

Cheers
Suzuki

> --
> 
> commit cfdd0e3f2fb70671ab2dbb64f06ca56946732456
> Author: Steven Price <steven.price@arm.com>
> 
> firmware: arm_rmm: Add SMC definitions for calling the RMM
> 
> This commit introduces SMC definitions for calling the Realm Management Monitor
> (RMM) from the host. The definitions are based on the ARM RMM specification
> version 2.0-bet3.
> 
> Link: https://developer.arm.com/documentation/den0137/2-0bet3/
> 
>> diff --git a/include/linux/arm-smccc-rmi.h b/include/linux/arm-smccc-rmi.h
>> new file mode 100644
>> index 0000000000000..214d6228dfc22
>> --- /dev/null
>> +++ b/include/linux/arm-smccc-rmi.h
> 
> [ ... ]
> 
>> +#define RMI_ADDR_TYPE_NONE		0
>> +#define RMI_ADDR_TYPE_SINGLE		1
>> +#define RMI_ADDR_TYPE_LIST		2
>> +
>> +#define RMI_ADDR_RANGE_STATE_MASK	GENMASK(63, 62)
>> +#define RMI_ADDR_RANGE_ADDR_MASK	GENMASK(51, PAGE_SHIFT)
> 
> [Severity: Low]
> Can the use of GENMASK for bit positions >= 32 cause undefined behavior on
> 32-bit architectures?
> 
> The GENMASK macro evaluates to unsigned long, which is 32 bits wide on 32-bit
> architectures. Shifting by 32 or more results in truncation or an
> out-of-bounds shift.
> 
> Would it be better to use GENMASK_ULL here for these 64-bit masks?
> 
>> +#define RMI_ADDR_RANGE_COUNT_MASK	GENMASK(PAGE_SHIFT - 1, 2)
>> +#define RMI_ADDR_RANGE_BLOCK_SIZE_MASK	GENMASK(1, 0)
> 
> [ ... ]
> 
>> +enum rmi_ripas {
>> +	RMI_EMPTY = 0,
>> +	RMI_RAM = 1,
>> +	RMI_DESTROYED = 2,
>> +	RMI_DEV = 3,
>> +};
>> +
>> +#define RMI_NO_MEASURE_CONTENT	0
>> +#define RMI_MEASURE_CONTENT	1
>> +
>> +#define RMI_FEATURE_REGISTER_0_S2OASZ			GENMASK(40, 33)
>> +#define RMI_FEATURE_REGISTER_0_L0GPT_BLOCK_DELEGATE	BIT(32)
> 
> [Severity: Low]
> Similarly, will this GENMASK and BIT usage trigger the same undefined behavior
> when evaluated as unsigned long? Using GENMASK_ULL and BIT_ULL might avoid this.
> 
>> +#define RMI_FEATURE_REGISTER_0_PMU_NUM_CTRS		GENMASK(31, 27)
>> +#define RMI_FEATURE_REGISTER_0_PMU			BIT(26)
> 
> [ ... ]
> 
>> +#define RMI_FEATURE_REGISTER_3_RTT_S2AP_INDIRECT	BIT(6)
>> +#define RMI_FEATURE_REGISTER_3_RTT_PLANE		GENMASK(5, 4)
>> +#define RMI_FEATURE_REGISTER_3_MAX_NUM_AUX_PLANES	GENMASK(3, 0)
>> +
>> +#define RMI_FEATURE_REGISTER_4_MEC_COUNT		GENMASK(63, 0)
> 
> [Severity: Low]
> Could GENMASK_ULL(63, 0) be used here as well?
> 
>> +
>> +#define RMI_MEM_CATEGORY_CONVENTIONAL		0
>> +#define RMI_MEM_CATEGORY_DEV_NCOH		1
>