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

[Suzuki K Poulose]

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@xxxxxxx>
> >
> > 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@xxxxxxx>
> > Signed-off-by: Steven Price <steven.price@xxxxxxx>
> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> > ---
> >   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