linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Elliot Berman <eberman@codeaurora.org>
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	Brian Masney <masneyb@onstation.org>,
	Stephan Gerhold <stephan@gerhold.net>,
	Jeffrey Hugo <jhugo@codeaurora.org>,
	Douglas Anderson <dianders@chromium.org>
Subject: Re: [PATCH v2] firmware: qcom_scm: Only compile legacy calls on ARM
Date: Thu, 01 Apr 2021 23:58:48 -0700	[thread overview]
Message-ID: <161734672825.2260335.8472441215895199196@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <6ec0ca8d-85c7-53d6-acf2-22c4ac13e805@codeaurora.org>

Quoting Elliot Berman (2021-04-01 18:12:14)
> 
> It might be a good idea to wrap these lines from qcom_scm_call with #if 
> IS_ENABLED(CONFIG_ARM), and the corresponding ones in qcom_scm_call_atomic:
> 
>    case SMC_CONVENTION_LEGACY:
>        return scm_legacy_call(dev, desc, res);
> 
> If something is wrong with loaded firmware and LEGACY convention is 
> incorrectly selected, you would get a better hint about the problem: 
> "Unknown current SCM calling convention." You would still get the hint 
> earlier from __get_convention, but that may not be obvious to someone 
> unfamiliar with the SCM driver.
> 
> I'll defer to your/Bjorn's preference:
> 
> Acked-by: Elliot Berman <eberman@codeaurora.org>
> 
> with or without modifying qcom_scm_call{_atomic}.
> 

Maybe it would be better to catch that problem at the source and force
arm64 calling convention to be safe? I'm thinking of this patch, but it
could be even more fine tuned and probably the sc7180 check could be
removed in the process if the rest of this email makes sense.

If I understand correctly CONFIG_ARM64=y should never use legacy
convention (and never the 32-bit one either?), so we can figure that out
pretty easily and just force it to use 64-bit if the architecture is
arm64. If only the 64-bit convention is supported on arm64 then we
really don't even need to call into the firmware to figure it out on
arm64. We can do this convention detection stuff on arm32 (CONFIG_ARM)
and always assume 64-bit convention on CONFIG_ARM64. Is that right?

---8<---
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 747808a8ddf4..22187ebc1678 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -140,7 +140,13 @@ static enum qcom_scm_convention __get_convention(void)
 	if (!ret && res.result[0] == 1)
 		goto found;
 
-	probed_convention = SMC_CONVENTION_LEGACY;
+	if (IS_ENABLED(CONFIG_ARM)) {
+		probed_convention = SMC_CONVENTION_LEGACY;
+	} else {
+		probed_convention = SMC_CONVENTION_ARM_64;
+		forced = true;
+		WARN(1, "qcom_scm: Detected legacy convention on arm64; firmware is broken\n");
+	}
 found:
 	spin_lock_irqsave(&scm_query_lock, flags);
 	if (probed_convention != qcom_scm_convention) {

  reply	other threads:[~2021-04-02  6:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-23 22:43 [PATCH v2] firmware: qcom_scm: Only compile legacy calls on ARM Stephen Boyd
2021-04-01  1:26 ` Stephen Boyd
2021-04-01  1:52 ` Bjorn Andersson
2021-04-02  1:12 ` Elliot Berman
2021-04-02  6:58   ` Stephen Boyd [this message]
2021-04-02 10:18     ` Stephan Gerhold
2021-04-02 17:21       ` Stephen Boyd
2021-04-05 12:50         ` Stephan Gerhold
2021-04-08  0:12           ` Stephen Boyd
2021-04-08  7:19             ` Stephan Gerhold
2021-04-08 21:18               ` Stephen Boyd

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=161734672825.2260335.8472441215895199196@swboyd.mtv.corp.google.com \
    --to=swboyd@chromium.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=dianders@chromium.org \
    --cc=eberman@codeaurora.org \
    --cc=jhugo@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masneyb@onstation.org \
    --cc=stephan@gerhold.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).