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

[Suzuki K Poulose]

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@xxxxxxx>

Thanks Catalin

Suzuki