All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will.deacon@arm.com>
To: Andy Gross <andy.gross@linaro.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Stephen Boyd <sboyd@codeaurora.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>,
	sudeep.holla@arm.com
Subject: Re: [PATCH 1/2] arm64: kernel: Add SMC Session ID to results
Date: Wed, 31 Aug 2016 15:36:29 +0100	[thread overview]
Message-ID: <20160831143629.GD29505@arm.com> (raw)
In-Reply-To: <20160830201642.GD24683@hector.attlocal.net>

On Tue, Aug 30, 2016 at 03:16:42PM -0500, Andy Gross wrote:
> On Tue, Aug 23, 2016 at 11:38:41AM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Aug 22, 2016 at 05:38:31PM -0700, Stephen Boyd wrote:
> > 
> > [...]
> > 
> > > 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.
> > 
> > Sorry I missed this bit. The session id is _generated_ by secure
> > firmware (probably only when the value passed in x6 == 0 (?))
> > and actually returned to the caller so that subsequent (interrupted)
> > calls can re-issue the same value, is that correct ?
> > 
> > If that's the case the value in x6 is a result value from an SMCCC
> > perspective and your current FW is not SMCCC compliant.
> > 
> 
> So is Will's solution to this ok?  If so I will respin with the minor change to
> get it working and resend.  If not, do I roll my own smccc wrapper?

Obviously I'm biased, but I prefer to handle this as a quirk to make it
clear that it's a vendor-specific extension to the SMCCC, so if you
could post a patch based on the diff I sent, that would be great.

You'll also need to:

  (1) Make sure you don't break 32-bit ARM
  (2) Make sure that struct arm_smccc_res is always zero-initialised by
      its other users (to ensure that QUIRK_NONE is set). In fact, it
      might be nicer to pass the quirk structure as a separate argument,
      rather than embed it in arm_smccc_res.

Will

WARNING: multiple messages have this Message-ID (diff)
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] arm64: kernel: Add SMC Session ID to results
Date: Wed, 31 Aug 2016 15:36:29 +0100	[thread overview]
Message-ID: <20160831143629.GD29505@arm.com> (raw)
In-Reply-To: <20160830201642.GD24683@hector.attlocal.net>

On Tue, Aug 30, 2016 at 03:16:42PM -0500, Andy Gross wrote:
> On Tue, Aug 23, 2016 at 11:38:41AM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Aug 22, 2016 at 05:38:31PM -0700, Stephen Boyd wrote:
> > 
> > [...]
> > 
> > > 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.
> > 
> > Sorry I missed this bit. The session id is _generated_ by secure
> > firmware (probably only when the value passed in x6 == 0 (?))
> > and actually returned to the caller so that subsequent (interrupted)
> > calls can re-issue the same value, is that correct ?
> > 
> > If that's the case the value in x6 is a result value from an SMCCC
> > perspective and your current FW is not SMCCC compliant.
> > 
> 
> So is Will's solution to this ok?  If so I will respin with the minor change to
> get it working and resend.  If not, do I roll my own smccc wrapper?

Obviously I'm biased, but I prefer to handle this as a quirk to make it
clear that it's a vendor-specific extension to the SMCCC, so if you
could post a patch based on the diff I sent, that would be great.

You'll also need to:

  (1) Make sure you don't break 32-bit ARM
  (2) Make sure that struct arm_smccc_res is always zero-initialised by
      its other users (to ensure that QUIRK_NONE is set). In fact, it
      might be nicer to pass the quirk structure as a separate argument,
      rather than embed it in arm_smccc_res.

Will

  reply	other threads:[~2016-08-31 14:36 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-20  5:51 [PATCH 0/2] Qualcomm SMCCC Session ID Support Andy Gross
2016-08-20  5:51 ` Andy Gross
2016-08-20  5:51 ` [PATCH 1/2] arm64: kernel: Add SMC Session ID to results Andy Gross
2016-08-20  5:51   ` Andy Gross
2016-08-22 13:43   ` Will Deacon
2016-08-22 13:43     ` Will Deacon
2016-08-22 14:02     ` Andy Gross
2016-08-22 14:02       ` Andy Gross
2016-08-22 14:53       ` Will Deacon
2016-08-22 14:53         ` Will Deacon
2016-08-22 15:16         ` Andy Gross
2016-08-22 15:16           ` Andy Gross
2016-08-23 12:39           ` Andy Gross
2016-08-23 12:39             ` Andy Gross
2016-08-23 12:39             ` Andy Gross
2016-08-23  0:38         ` Stephen Boyd
2016-08-23  0:38           ` Stephen Boyd
2016-08-23  9:07           ` Lorenzo Pieralisi
2016-08-23  9:07             ` Lorenzo Pieralisi
2016-08-23  9:07             ` Lorenzo Pieralisi
2016-08-23 10:38           ` Lorenzo Pieralisi
2016-08-23 10:38             ` Lorenzo Pieralisi
2016-08-23 10:38             ` Lorenzo Pieralisi
2016-08-23 12:36             ` Andy Gross
2016-08-23 12:36               ` Andy Gross
2016-08-30 20:16             ` Andy Gross
2016-08-30 20:16               ` Andy Gross
2016-08-31 14:36               ` Will Deacon [this message]
2016-08-31 14:36                 ` Will Deacon
2016-08-24 18:24   ` Bjorn Andersson
2016-08-24 18:24     ` Bjorn Andersson
2016-08-20  5:51 ` [PATCH 2/2] firmware: qcom: scm: Fix interrupted SCM calls Andy Gross
2016-08-20  5:51   ` Andy Gross

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160831143629.GD29505@arm.com \
    --to=will.deacon@arm.com \
    --cc=andy.gross@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=patches@linaro.org \
    --cc=sboyd@codeaurora.org \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=stanimir.varbanov@linaro.org \
    --cc=sudeep.holla@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.