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

From: Michael Kelley <mikelley@microsoft.com>
To: Sudeep Holla <sudeep.holla@arm.com>, Mark Rutland <Mark.Rutland@arm.com>
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Trilok Soni <tsoni@codeaurora.org>,
	"arve@android.com" <arve@android.com>,
	Andrew Walbran <qwandor@google.com>,
	David Hartley <dhh@qti.qualcomm.com>,
	Achin Gupta <Achin.Gupta@arm.com>,
	Jens Wiklander <jens.wiklander@linaro.org>,
	Arunachalam Ganapathy <arunachalam.ganapathy@arm.com>,
	Marc Bonnici <marc.bonnici@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>
Subject: RE: [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
Date: Thu, 1 Apr 2021 15:50:06 +0000	[thread overview]
Message-ID: <MWHPR21MB15939FCE24BDF74B06616560D77B9@MWHPR21MB1593.namprd21.prod.outlook.com> (raw)
In-Reply-To: <20210326125124.yl234hz5ipr6yn2d@bogus>

From: Sudeep Holla <sudeep.holla@arm.com> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel