Re: [PATCH v20 2/9] firmware: arm_rmm: Check for RMI support at init

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

On 01/10/2026 07:03, Suzuki K Poulose wrote:
> On 30/09/2026 12:02, Catalin Marinas wrote:
>> On Wed, Sep 30, 2026 at 09:18:48AM +0100, Suzuki K Poulose wrote:
>>> On 29/09/2026 23:28, sashiko-bot@kernel.org wrote:
>>>>> diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi- 
>>>>> cmds.h
>>>>> new file mode 100644
>>>>> index 0000000000000..911489b636522
>>>>> --- /dev/null
>>>>> +++ b/include/linux/arm-rmi-cmds.h
>>>> [ ... ]
>>>>> +static inline void rmi_smccc_invoke(struct arm_smccc_1_2_regs *regs)
>>>>> +{
>>>>> +    struct arm_smccc_1_2_regs args = *regs;
>>>>> +    long status;
>>>>> +    int i = 0;
>>>>> +
>>>>> +    while (i < RMM_BLOCKED_RETRY_COUNT) {
>>>>> +        arm_smccc_1_2_invoke(&args, regs);
>>>>> +
>>>>> +        status = RMI_RESULT_STATUS(regs->a0);
>>>>> +        if (status != RMI_BUSY && status != RMI_BLOCKED)
>>>>> +            break;
>>>>> +        if (status == RMI_BLOCKED)
>>>>> +            i++;
>>>>> +        cpu_relax();
>>>>> +    }
>>>>
>>>> [Severity: High]
>>>> Does this sequence create an unbounded spin loop if the firmware 
>>>> continuously
>>>> returns RMI_BUSY?
>>>>
>>>> If the status returned by arm_smccc_1_2_invoke() is RMI_BUSY, the loop
>>>> continues without incrementing the exit counter i. It issues 
>>>> cpu_relax()
>>>> but does not yield the CPU (e.g., via cond_resched()) or impose a 
>>>> timeout.
>>>> If the RMM firmware remains busy, this could cause the host kernel 
>>>> to lock
>>>> up indefinitely.
>>>
>>> This is like "spinlock contention". RMM returns EBUSY when a resource is
>>> locked temporarily. We should be able to make progress, unlike the
>>> RMI_BLOCKED, which is blocking due to a long running operation.
>>
>> I dug a bit into the RMI_BUSY description in the spec and it's not
>> always safe to spin forever. For RMI_{PDEV,VDEV}_COMMUNICATE, for
>> example, we need to return to the caller and retry later (for VDEV, the
>> spec suggests informing the realm). That's the TSM series, which doesn't
>> use the new API yet, but something to be aware of when it's updated.
> 
> Agree. There RMI_BUSY indicates the Pdev is busy with an ongoing
> communication which could take longer.
> 
>>
>> We may need the spec to bound this spin anyway, or at least give an
>> indication that it's not forever, otherwise it affects latency,
>> especially in an RT kernel.
>>
>> There's a generic RMI command return code table in B4.2 that lists
>> RMI_BUSY for an imp def reason for the command failing to make progress.
>> Does this apply to something like REC_ENTER? In the KVM series we call
>> that with IRQs masked.
> 
> REC_ENTER only tries to lock the REC object. The only other code that
> tries to lock is the PSCI completion processing to update the "rec"
> state to runnable. Hence we don't expect a the REC_ENTER to be waiting
> for the REC object lock and encounter an RMI_BUSY. Also, REC_ENTER
> locks the object to get a refcount on the granule and the lock is
> released. So even if the host issued REC_ENTER in parallel, the second
> one will encounter RMI_ERROR_REC due to the bumped refcount.
> 
> That said, I am happy to return to the caller on RMI_REC_ENTER in the 
> worst case and pretend there was an IRQ and that would allow the KVM
> to re-enter the guest after servicing the interrupts. I have added the 
> following changes to the patch where we introduce the REC_ENTER:
> 
> +/*
> + * rmi_smccc_invoke_once: Invoke the RMI call and return the results. 
> Do not
> + * retry the command. Let the caller deal with RMI_BUSY or RMI_BLOCKED.
> + */
> +static inline void rmi_smccc_invoke_once(struct arm_smccc_1_2_regs *regs)
> +{
> +       struct arm_smccc_1_2_regs args = *regs;
> +
> +       arm_smccc_1_2_invoke(&args, regs);
> +}
> +
> 
> ...
> 
> 
> +/**
> + * rmi_rec_enter() - Enter a REC
> + * @rec: PA of the target REC
> + * @run_ptr: PA of RecRun structure
> + *
> + * Starts (or continues) execution within a REC.
> + *
> + * Return: RMI return code
> + */
> +static inline long rmi_rec_enter(unsigned long rec, unsigned long run_ptr)
> +{
> +       struct arm_smccc_1_2_regs regs = {
> +               SMC_RMI_REC_ENTER, rec, run_ptr,
> +       };
> +
> +       rmi_smccc_invoke_once(&regs);
> +       return regs.a0;
> +}
> 
> 

And for the record, here is the change in the KVM CCA Driver for 
handling this case :

         if (status == RMI_ERROR_REALM) {
                 vcpu->run->exit_reason = KVM_EXIT_SHUTDOWN;
                 return ARM_EXCEPTION_EXIT;
         }

         /*
          * If a VCPU has been turned on, but the REC state hasn't been 
updated
          * we may experience RMI_ERROR_REC. Exit to the userspace with 
-EAGAIN
          * for a retry.
          */
         if (status == RMI_ERROR_REC)
                 return -EAGAIN;

+       /*
+        * If the RMI_REC_ENTER encounters RMI_BUSY, treat it as if it was
+        * an IRQ and go back to the run-loop. We don't sync the state back
+        * to the vcpu when status != RMI_SUCCESS.
+        */
+       if (status == RMI_BUSY)
+               return ARM_EXCEPTION_IRQ;
+
         if (rec_run_ret)
                 return rec_exit_fatal(vcpu, "Unexpected REC_ENTER status",
                                       rec_run_ret);

         switch (rec->run->exit.exit_reason) {
         case RMI_EXIT_SYNC:
                 /*
                  * HPFAR_EL2_NS is hijacked to indicate a valid HPFAR 
value,
                  * see __get_fault_info()
                  */
                 vcpu->arch.fault.hpfar_el2 = rec->run->exit.hpfar | 
HPFAR_EL2_NS;
                 rec_exit_sync(vcpu);
                 return ARM_EXCEPTION_TRAP;
         case RMI_EXIT_IRQ:
         case RMI_EXIT_FIQ:
                 return ARM_EXCEPTION_IRQ;
         case RMI_EXIT_SERROR:
                 return ARM_EXCEPTION_EL1_SERROR;
         case RMI_EXIT_PSCI:
                 rec_exit_hvc(vcpu);
                 /*
                  * Queue completion before dispatching the exit through the
                  * generic HVC handling path. The request will be 
processed after
                  * HVC handling has updated the vCPU state and before 
the next REC
                  * entry.
                  */
                 kvm_make_request(KVM_REQ_RMI, vcpu);
                 return ARM_EXCEPTION_TRAP;
         case RMI_EXIT_RIPAS_CHANGE:
                 return rec_exit_ripas_change(vcpu);

Cheers
Suzuki