Re: [PATCH v18 4/7] firmware: arm_rmm: Add support for SRO

[Gavin Shan]

On 9/14/26 6:19 PM, Suzuki K Poulose wrote:
> On 14/09/2026 07:22, Suzuki K Poulose wrote:
> > On 14/09/2026 06:04, Gavin Shan wrote:
> > > On 9/12/26 6:36 PM, Suzuki K Poulose wrote:
> > > > RMM v2.0 introduces the concept of "Stateful RMI Operations" (SRO). This
> > > > means that an SMC can return with an operation still in progress. The
> > > > host is expected to continue the operation until it reaches a conclusion
> > > > (either success or failure). During this process the RMM can request
> > > > additional memory ('donate') or hand memory back to the host
> > > > ('reclaim'). The host can request an in progress operation is cancelled,
> > > > but still continue the operation until it has completed (otherwise the
> > > > incomplete operation may cause future RMM operations to fail).
> > > >
> > > > The SRO is tracked using a struct rmi_sro_state object which keeps track
> > > > of any memory which has been allocated but not yet consumed by the RMM
> > > > or reclaimed from the RMM. This allows the memory to be reused in a
> > > > future request within the same operation. It will also permit an
> > > > operation to be done in a context where memory allocation may be
> > > > difficult (e.g. atomic context) with the option to abort the operation
> > > > and retry the memory allocation outside of the atomic context. The
> > > > memory stored in the struct rmi_sro_state object can then be reused on
> > > > the subsequent attempt.
> > > >
> > > > Wrappers for SRO RMI commands are also provided here because they depend
> > > > on the rmi_sro_execute() implementation added by this patch.
> > > > Delegate/undelegate handles are also added here because they now use the
> > > > SRO/stateful command infrastructure and are also used for the memory
> > > > DONATE/RECLAIM flows.
> > > >
> > > > Signed-off-by: Steven Price <steven.price@xxxxxxx>
> > > > Co-Developed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> > > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> >
> >
> >
> > > > ---
> > > >   drivers/firmware/arm_rmm/rmi.c | 586 +++++++++++++++++++++++++++++++++
> > > >   include/linux/arm-rmi-cmds.h   |  41 +++
> > > >   2 files changed, 627 insertions(+)
> > > >
> > > > diff --git a/drivers/firmware/arm_rmm/rmi.c b/drivers/firmware/ arm_rmm/rmi.c
> > > > index 5b0e342ce3d58..4f9898ece7547 100644
> > > > --- a/drivers/firmware/arm_rmm/rmi.c
> > > > +++ b/drivers/firmware/arm_rmm/rmi.c
> >
> > > I would drop rmi_granule_range_{delegate, undelegate}() by combining their logics to
> > > their only callers rmi_{delegate, undelegate}_range(). More details are provided for
> > > rmi_{delegate, undelegate}_range() in the below.
> >
> > Ack
>
> I have moved them closer, but kept the logic separate, since this is not
> a simple RMI call.

Yeah, it's fine by moving rmi_granule_range_{delegate, undelegate}() to their only
callers. However, the unnecessary if statements can be avoided in their only callers
rmi_{delegate, undelegate)_range(). Besides, rmi_delegate_range() would come before
rmi_undelegate_range().

	while (...) {
		ret = rmi_granule_range_undelegate(phys, top, &next);
		if (ret != RMI_SUCCESS)
			break;

		/* Buggy RMM ? Let the caller leak the pages */
		if (next <= phys) {
			ret = -ENXIO;
			break;
		}

		phys = next;
	}

The variable 'out_top' in rmi_{delegate, undelegate)_range() may be renamed to 'next',
indicating it's the next (starting) granule for the RMI calls.

Thanks,
Gavin