Re: [PATCH v21 8/9] firmware: arm_rmm: Add wrappers for Realm related RMI commands

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

On 01/10/2026 10:04, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] arm_rmm: dropped out_top on RMI_ERROR_RTT for range commands
> --
> 
> commit 68e63e1b38f1af07d69bbc2eee9306e4bedb8efa
> Author: Steven Price <steven.price@arm.com>
> 
> firmware: arm_rmm: Add wrappers for Realm related RMI commands
> 
> This commit introduces wrappers for the RMI calls used to create and
> manage realm guests. These inline functions map the standard KVM realm
> operations to their underlying SMCCC interfaces.
> 
>> diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi-cmds.h
>> index 5d005054f3c6c..e3503aba646e2 100644
>> --- a/include/linux/arm-rmi-cmds.h
>> +++ b/include/linux/arm-rmi-cmds.h
> [ ... ]
>> +static inline long rmi_rtt_data_map(unsigned long rd,
>> +				    unsigned long base,
>> +				    unsigned long top,
>> +				    unsigned long flags,
>> +				    unsigned long oaddr,
>> +				    unsigned long *out_top)
>> +{
>> +	struct arm_smccc_1_2_regs regs = {
>> +		SMC_RMI_RTT_DATA_MAP, rd, base, top, flags, oaddr
>> +	};
>> +	long ret;
>> +
>> +	ret = rmi_sro_execute(&regs);
>> +
>> +	if (ret == RMI_SUCCESS && out_top)
>> +		*out_top = regs.a1;
> 
> [Severity: High]
> If this operation encounters an RTT state conflict midway through processing
> the range, the RMM terminates early and returns RMI_ERROR_RTT.
> 

Nope, see below.

> When that happens, the RMM writes the failing IPA to out_top (register x1)
> so the caller knows which pages were successfully processed before the error.
> 
> By restricting the out_top assignment strictly to ret == RMI_SUCCESS, doesn't
> this silently discard the partial progress indicator on RMI_ERROR_RTT, causing
> KVM to lose track of the modified pages?
> 
>> +
>> +	return ret;
>> +}
> [ ... ]
>> +static inline long rmi_rtt_data_unmap(unsigned long rd,
>> +				      unsigned long base,
>> +				      unsigned long top,
>> +				      unsigned long flags,
>> +				      unsigned long oaddr,
>> +				      unsigned long *out_top,
>> +				      unsigned long *out_range,
>> +				      unsigned long *out_count)
>> +{
>> +	struct arm_smccc_1_2_regs regs = {
>> +		SMC_RMI_RTT_DATA_UNMAP, rd, base, top, flags, oaddr
>> +	};
>> +	long ret;
>> +
>> +	ret = rmi_sro_execute(&regs);
>> +
>> +	if (ret != RMI_SUCCESS)
>> +		return ret;
>> +
>> +	if (out_top)
>> +		*out_top = regs.a1;
> 
> [Severity: High]
> Similarly, if rmi_rtt_data_unmap() encounters an RMI_ERROR_RTT, it returns
> early here without populating out_top.

As explained earlier, out_top is invalid when result != RMI_SUCCESS for
all cases except RMI_RTT_DESTROY.

Suzuki