Re: [PATCH v19 6/7] firmware: arm_rmm: Ensure the RMM has GPT entries for memory

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

On 24/09/2026 22:38, Jonathan Cameron wrote:
> On Thu, 24 Sep 2026 14:52:00 +0100
> Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> 
>> From: Steven Price <steven.price@arm.com>
>>
>> The RMM maintains the state of all the granules in the system to make
>> sure that the host is abiding by the rules. This state can be maintained
>> at different granularity, per page (TRACKING_FINE) or per region
>> (TRACKING_COARSE or TRACKING_INTERMEDIATE). The region size depends on the

...

>>
>> If the firmware requires us to manage the tracking or GPT memory,
>> deactivate the RMM and reclaim any memory donated at RMM activation.
>>
>> Apply the same checks when hotplugged memory is brought online.
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> [ Switch to RMI_GPT_L1_INFO for checking GPTs and deactivate RMM ]
>> Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> A few trivial comments. Either way on those
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Thank you !
> 
>> ---
>>   drivers/firmware/arm_rmm/rmi.c | 218 ++++++++++++++++++++++++++++++++-
>>   include/linux/arm-rmi-cmds.h   |   2 +
>>   2 files changed, 219 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/firmware/arm_rmm/rmi.c b/drivers/firmware/arm_rmm/rmi.c
>> index 0859f256e192b..1a8f3debd844a 100644
>> --- a/drivers/firmware/arm_rmm/rmi.c
>> +++ b/drivers/firmware/arm_rmm/rmi.c
> 
> 
>> +
>> +/*
>> + * rmi_gpt_info - Query the GPT info for the given PAR.
>> + * @start: Base of the physical address region
>> + * @end: Top of the physical address region
>> + * @out_top: Top of the physical address region for which
>> + *		the GPT @out_gpt_par_state is valid
> 
> wrap to 80.
> 

Ack

>> + * @out_gpt_par_state: State of the GPT covered by [start, out_top)
>> + */
>> +static long rmi_gpt_info(unsigned long start, unsigned long end,
>> +			 unsigned long *out_top,
>> +			 unsigned long *out_gpt_par_state)
>> +{
>> +	struct arm_smccc_1_2_regs regs = {
>> +		SMC_RMI_GPT_INFO, start, end,
>> +	};
>> +
>> +	rmi_smccc_invoke(&regs);
>> +	if (regs.a0 != RMI_SUCCESS)
>> +		return regs.a0;
>> +
>> +	if (out_top)
>> +		*out_top = regs.a1;
>> +	if (out_gpt_par_state)
>> +		*out_gpt_par_state = regs.a2;
>> +
>> +	return RMI_SUCCESS;
>> +}
>> +
>> +/*
>> + * We do not support creating L1 GPTs yet. So, make sure that
>> + * all the regions are managed by the firmware.
> Wrap to 80 chars.
> 
> Not relevant to this patch, but when are you thinking we will support that?

Once we have a full firmware stack that implements the L1GPT (Firme in 
tf-a and tf-RMM) we will take it up. That won't come before the first
drop of KVM is merged.

>> +
>> +static int rmi_init_metadata(void)
>> +{
>> +	phys_addr_t start, end;
>> +	struct memblock_region *r;
>> +
>> +	for_each_mem_region(r) {
>> +		int ret;
>> +
>> +		/* Firmware-reserved NOMAP regions are not usable system RAM */
>> +		if (memblock_is_nomap(r))
>> +			continue;
>> +
>> +		start = PAGE_ALIGN(r->base);
>> +		end = PAGE_ALIGN_DOWN(r->base + r->size);
> 
> Add a comment on why rounding down.  Doe we expect that to ever be relevant?

Do we really need a comment here ? We always deal with PAGE_SIZE and if
in the odd case this region boundary is not PAGE aligned we stick to the
page that we can use from the region. I can stick in a comment if you
really think so.


...

>> @@ -843,9 +1049,19 @@ static int __init arm64_init_rmi(void)

...

>> -	return ret;
>> +	ret = rmi_init_memory();
>> +	if (ret) {
>> +		/* Deactivate the RMM */
>> +		WARN_ON(rmi_sro_memxfer_cmd(sro, GFP_KERNEL, SMC_RMI_RMM_DEACTIVATE));
>> +		return ret;
>> +	}
>> +
>> +	arm64_rmi_is_available = true;
>> +	pr_info("RMI configured\n");
> 
> Blank line nice for the eye to spot the return.

Ack

Thanks for your review !

Suzuki