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
