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

[sashiko-bot@kernel.org]

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.

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.

Since the RMM handles range commands sequentially, the IPAs prior to the
error were successfully unmapped. Discarding out_top hides this partial
progress from the caller. Can this lead to memory leaks or missing TLB
invalidations because KVM loses sync with the actual RTT state?

This same pattern appears to affect the other range command wrappers
introduced in this patch, such as rmi_rtt_init_ripas(), rmi_rtt_unprot_map(),
rmi_rtt_set_ripas(), and rmi_rtt_unprot_unmap().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001084555.1456543-1-suzuki.poulose@arm.com?part=8