Re: [PATCH v18 3/7] firmware: arm_rmm: Configure the RMM with the host's page size

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

On 14/09/2026 02:21, Gavin Shan wrote:
> On 9/12/26 6:36 PM, Suzuki K Poulose wrote:
>> From: Steven Price <steven.price@arm.com>
>>
>> RMM v2.0 brings the ability to set the RMM's granule size. Check the
>> feature registers and configure the RMM so that it matches the host's
>> page size. This means that operations can be done with a granularity
>> equal to PAGE_SIZE.
>>
>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   Changes since v17:
>>    * Move rmi_config_set() out of the header file.
>>    * Print the error message for rmi_config_set if it fails
>>   Changes since v15:
>>    * Actually check the feature register for the host's page-size 
>> support.
>>   Changes since v14:
>>    * Move the implementation into drivers/firmware/arm_rmm.
>>   Changes since v13:
>>    * Moved out of KVM.
>> ---
>>   drivers/firmware/arm_rmm/rmi.c | 79 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 79 insertions(+)
>>
>> diff --git a/drivers/firmware/arm_rmm/rmi.c b/drivers/firmware/ 
>> arm_rmm/rmi.c
>> index 2fd538c937dca..5b0e342ce3d58 100644
>> --- a/drivers/firmware/arm_rmm/rmi.c
>> +++ b/drivers/firmware/arm_rmm/rmi.c
>> @@ -35,6 +35,25 @@ static int rmi_features(unsigned long index, 
>> unsigned long *out)
>>       return args.a0;
>>   }
>> +/**
>> + * rmi_rmm_config_set() - Configure the RMM
>> + * @cfg_ptr: PA of a struct rmm_config
>> + *
>> + * Sets configuration options on the RMM.
>> + *
>> + * Return: RMI return code
>> + */
>> +static int rmi_rmm_config_set(unsigned long cfg_ptr)
>> +{
>> +    struct arm_smccc_1_2_regs regs = {
>> +        SMC_RMI_RMM_CONFIG_SET, cfg_ptr,
>> +    };
>> +
>> +    rmi_smccc_invoke(&regs);
>> +
>> +    return regs.a0;
>> +}
>> +
> 
> The comments for rmi_rmm_config_set() can be dropped since its logic is 
> simply enough and
> the code is self-explainning. Besides, I would move this right before 
> its only caller
> rmi_configure().

Ack

> 
> I would suggest drop this function by combining its logics into the only 
> caller
> rmi_configure(), seeing below for more details.

That looks a bit odd in the middle of a function, given the argument 
setting. I have moved it closer to the configure().

Cheers
Suzuki