Re: [PATCH 1/2] arm64: kernel: Add SMC Session ID to results

From: Stephen Boyd <sboyd@codeaurora.org>
To: Will Deacon <will.deacon@arm.com>
Cc: Andy Gross <andy.gross@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-arm-msm@vger.kernel.org,
	Catalin Marinas <catalin.marinas@arm.com>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	stanimir.varbanov@linaro.org, linux-kernel@vger.kernel.org,
	patches@linaro.org, Bjorn Andersson <bjorn.andersson@linaro.org>,
	lorenzo.pieralisi@arm.com, sudeep.holla@arm.com
Subject: Re: [PATCH 1/2] arm64: kernel: Add SMC Session ID to results
Date: Mon, 22 Aug 2016 17:38:31 -0700	[thread overview]
Message-ID: <20160823003831.GN6502@codeaurora.org> (raw)
In-Reply-To: <20160822145326.GK14680@arm.com>

On 08/22, Will Deacon wrote:
> On Mon, Aug 22, 2016 at 09:02:46AM -0500, Andy Gross wrote:
> > On Mon, Aug 22, 2016 at 02:43:14PM +0100, Will Deacon wrote:
> > > On Sat, Aug 20, 2016 at 12:51:13AM -0500, Andy Gross wrote:
> > > >  
> > > >  /**
> > > >   * arm_smccc_smc() - make SMC calls
> > > >   * @a0-a7: arguments passed in registers 0 to 7
> > > > - * @res: result values from registers 0 to 3
> > > > + * @res: result values from registers 0 to 3 and optional register 6
> > > 
> > > AFAICT from reading the SMCCC spec, parameter register 6 is "Unpredictable,
> > > Scratch registers" in return state, so I don't think this is correct.
> > > 
> > > What am I missing?
> > 
> > In the case of Qualcomm's implementation, they return a value in register 6 that
> > may or may not be used in subsequent calls.  If I want to leverage the arm_smccc
> > functions, then I need to extend them to include the optional return value.  The
> > downside to this is that everyone who uses this is exposed to it.
> 
> Yes, I'm not keen on forcing this behaviour for everybody, as you never
> know what other firmware might do with unexpected a6 values. Could we
> perhaps quirk it, along the lines of the completely untested patch below?
> 

How would the firmware know about the a6 values? It isn't passed
to the firmware unless the caller of arm_smccc_smc() uses it. Is
the concern that we would be exposing x6 as a return register for
all users of arm_smccc_smc()? Presumably callers of
arm_smccc_smc() would know if they want to use that extra
register or not, so doing all the quirk stuff seems like more
instructions over always saving away x6 on the return path, but
maybe there's some reasoning I missed.

This all comes about because the firmware generates a session id
for the SMC call and jams it in x6. The assembly on the
non-secure side is written with a tight loop around the smc
instruction so that when the return value indicates
"interrupted", x6 is kept intact and the non-secure OS can jump
back to the secure OS without register reloading. Perhaps
referring to x6 as result value is not correct because it's
really a session id that's irrelevant once the smc call
completes.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project