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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Sudeep Holla <sudeep.holla@xxxxxxx> Sent: Friday, March 26, 2021 5:51 AM
> 
> 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

Sudeep -- let me know when you have a new version available.  I can
test it for use with Hyper-V.

Michael





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux