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

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

On 14/09/2026 15:02, Suzuki K Poulose wrote:
> On 14/09/2026 13:50, Sudeep Holla wrote:
>> On Sat, Sep 12, 2026 at 09:36:07AM +0100, 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@arm.com>
>>> Co-Developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>
>> [...]
>>
>>> +static int rmi_sro_donate_noncontig(struct rmi_sro_state *sro,
>>> +                    unsigned long sro_handle,
>>> +                    unsigned long donatereq,
>>> +                    struct arm_smccc_1_2_regs *out_regs,
>>> +                    gfp_t gfp)
>>> +{
>>> +    unsigned long block_size_fld = RMI_DONATE_BLOCK_SIZE(donatereq);
>>> +    unsigned long block_size = 
>>> rmi_addr_block_size_to_bytes(block_size_fld);
>>> +    unsigned long count = RMI_DONATE_COUNT(donatereq);
>>> +    unsigned long state = RMI_DONATE_STATE(donatereq);
>>> +    unsigned long found = 0;
>>> +    unsigned long donated_granules;
>>> +    unsigned long granules_per_block = block_size >> PAGE_SHIFT;
>>> +    unsigned long consumed_blocks;
>>> +    int addr_list_start = sro->addr_count;
>>> +
>>> +    int ret;
>>> +
>>> +    for (int i = 0; i < addr_list_start && found < count; i++) {
>>> +        unsigned long entry = sro->addr_list[i];
>>> +
>>> +        if (RMI_ADDR_RANGE_BLOCK_SIZE(entry) == block_size_fld &&
>>> +            RMI_ADDR_RANGE_COUNT(entry) == 1 &&
>>> +            RMI_ADDR_RANGE_STATE(entry) == state) {
>>> +            addr_list_start--;
>>> +            swap(sro->addr_list[addr_list_start],
>>> +                 sro->addr_list[i]);
>>> +            found++;
>>> +            i--;
>>> +        }
>>> +    }
>>> +
>>> +    ret = rmi_sro_ensure_capacity(sro, count - found);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    while (found < count) {
>>> +        unsigned long addr_range;
>>> +        void *virt = alloc_pages_exact(block_size, gfp);
>>> +        phys_addr_t phys;
>>> +
>>> +        if (!virt)
>>> +            return -ENOMEM;
>>> +
>>> +        phys = virt_to_phys(virt);
>>> +
>>> +        if (state == RMI_OP_MEM_DELEGATED) {
>>
>> Based on my understanding, rmi_sro_memxfer_execute() is an exported 
>> function
>> and can be invoked by any module. The donatereq argument appears to 
>> accept one
>> of three operations:
>>
>> RMI_OP_MEM_DELEGATED
>> RMI_OP_MEM_UNDELEGATED
>> RMI_OP_MEM_CONDITIONAL
> 
> Ack
> 
>>
>> Currently, the check confirming the state is RMI_OP_MEM_DELEGATED occurs
>> relatively late in the function execution. It seems this function is
>> explicitly designed to handle only RMI_OP_MEM_DELEGATED.
> 
> No, that is not correct. The function handles both OP_MEM_DELEGATED and
> OP_MEM_UNDELEGATED. In the former case, we explicitly "delegate" the
> pages before donating. The "UNDELEGATED" case doesn't need to do that
> extra step.
> 
>>
>> Given that this is an exported interface, would it make sense to
>> fail-fast by moving this validation to the very beginning of the 
>> function?
>> Even if RMI_OP_MEM_CONDITIONAL is intended for future use, it should
> 
> Yep, agree. We can rejec the CONDITIONAL ones.

For the record, the CONDITIONAL ones are required for self-describing
cases for L1_GPT_CREATE and TRACKING_GRANULE_SET, where the RMM could
accept DELEGATED granules for the objects (if not self describing) or
UNDELEGATED granules  (if they are self-describing).

For now, we don't support such RMMs, so will reject the type for now.

Cheers
Suzuki

> 
>> probably be rejected as invalid for now. Also, it is not clear why the
>> current check is inside the loop while the state itself doesn't get
>> modified.
> 
> As above, that check is additionally preparing the memory for RMM
> consumption.
> 
> 
>>
>> If this is a valid concern, the same architectural pattern should 
>> likely be
>> applied to rmi_sro_donate_contig().
>>
> 
> Ack, we can reject the CONDITIONAL ones.
> 
> Suzuki
>