[PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers

[Sudeep Holla]

On Fri, Mar 26, 2021 at 12:18:50PM +0000, Mark Rutland wrote:
> On Thu, Mar 25, 2021 at 02:41:13PM +0000, Mark Rutland wrote:
> > Hi Sudeep,
> >
> > On Thu, Mar 25, 2021 at 02:32:50PM +0000, Sudeep Holla wrote:
> > > SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
> > > to be used as result registers in SMC64/HVC64. Arm Firmware Framework
> > > for Armv8-A specification makes use of x0-x7 as parameter and result
> > > registers.
> > >
> > > Current SMCCC interface in the kernel just use x0-x7 as parameter and
> > > x0-x3 as result registers. Let us add new interface to support x0-x7
> > > as parameter and result registers. This can be extended to include
> > > x8-x17 when there are users for the same.
> >
> > Michael Kelley is also looking at using SMCCCv1.2, and on his HyperV
> > thread I'd suggested we should deal with the whole set of SMCCCv1.2
> > registers now to avoid future churn in this area (using struct both for
> > the arguments and return values).
> >
> > How painful would it be to extend this patch to do that?
>
> I *think* the major change with this would be making the interfaces:
>
> void arm_smccc_1_2_{hvc,smc}(struct arm_smccc_1_2_args *args,
> 			     struct arm_smccc_1_2_res *res);
>
> ... and callers manipulating the structs directly, with arm64 having
> more fields, e.g.
>
> // arm64
> struct arm_smccc_1_2_args {
> 	unsigned long a1;
> 	...
> 	unsigned long a17;
> }
>
> struct arm_smccc_1_2_res {
> 	unsigned long a0;
> 	...
> 	unsigned long a17;
> }
>
> // arm
> struct arm_smccc_1_2_args {
> 	unsigned long a1;
> 	...
> 	unsigned long a7;
> }
>
> struct arm_smccc_1_2_res {
> 	unsigned long a0;
> 	...
> 	unsigned long a7;
> }
>
> I think that can be hidden in the FF-A wrappers, so that doesn't need to
> be what FF-A drivers see. Does that sound plausible, or is that painful?
>

Sounds possible, will give it a try.

> > > +  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct arm_smccc_v1_2_res, a0));
>
> As a general nit, for consistency with the existing arm_smccc_1_1 code,
> could we please drop the 'V' in these names, and use `ARM_SMCCC_1_2` or
> `arm_smccc_1_2` ?
>

Sure, makes sense.

> FWIW, other than the above comments, this all looks good to me
>

Thanks.

--
Regards,
Sudeep