Re: [PATCH v21 8/9] firmware: arm_rmm: Add wrappers for Realm related RMI commands

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

On 01/10/2026 13:58, Catalin Marinas wrote:
> On Thu, Oct 01, 2026 at 09:45:54AM +0100, Suzuki K Poulose wrote:
>> diff --git a/include/linux/arm-rmi-cmds.h b/include/linux/arm-rmi-cmds.h
>> index 5d005054f3c6c..e3503aba646e2 100644
>> --- a/include/linux/arm-rmi-cmds.h
>> +++ b/include/linux/arm-rmi-cmds.h
>> @@ -53,6 +53,17 @@ static inline void rmi_smccc_invoke(struct arm_smccc_1_2_regs *regs)
>>   	}
>>   }
>>   
>> +/*
>> + * 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);
>> +}
> 
> Nit: I don't think we need to copy to args since there's no retry here.
> 
> That said, the SMCCC call clobbers regs->a0. We should write a comment

Ack. I will add the comment, similar to what we have for rmi_smccc_invoke().

> in case the caller thinks it can just retry with the same regs.
> Alternatively, we could pass args and res structures separately.

I thought about it, but settled in for copying the arguments.

> 
>> +/**
>> + * rmi_rec_destroy() - Destroy a REC
>> + * @rec: PA of the target REC
>> + * @sro: Allocated SRO context to be used
>> + *
>> + * Destroys a REC. The REC must not be running.
>> + *
>> + * Return: 0 on success, positive RMI result code or negative Linux error code
>> + */
>> +static inline long rmi_rec_destroy(unsigned long rec,
>> +				   struct rmi_sro_state *sro)
>> +{
>> +	return rmi_sro_memxfer_cmd(sro, GFP_KERNEL, SMC_RMI_REC_DESTROY, rec);
>> +}
>> +
>> +/**
>> + * 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)
> 
> Nit: should probably be __always_inline. rmi_rec_destroy() and
> rmi_smccc_invoke_once() as well.

Ack.

> 
> Not for this series but something claude spotted in the integration
> branch - kvm_rec_enter() is noinstr. On this path, we end up in
> arm_smccc_1_1_get_conduit() which is not noinstr (again, that's for the
> other series; here just __always_inline should suffice).

Good point. I will address this in the other series. May be we can
always do SMC. We don't support CCA over pKVM yet. Even there, may
be SMC counduit is available ?

i.e., arm_smccc_1_2_smc() instead of arm_smccc_1_2_invoke()

> 
>> +{
>> +	struct arm_smccc_1_2_regs regs = {
>> +		SMC_RMI_REC_ENTER, rec, run_ptr,
>> +	};
>> +
>> +	rmi_smccc_invoke_once(&regs);
>> +	return regs.a0;
> 
> Should this use RMI_RESULT_STATUS(regs.a0)?

No. We should always return the full result.

> 
> The rest looks fine to me.
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks Catalin

Suzuki