Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 2 Apr 2021 12:18:04 +0200
From:   Stephan Gerhold <stephan@gerhold.net>
To:     Stephen Boyd <swboyd@chromium.org>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Elliot Berman <eberman@codeaurora.org>,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        Brian Masney <masneyb@onstation.org>,
        Jeffrey Hugo <jhugo@codeaurora.org>,
        Douglas Anderson <dianders@chromium.org>
Subject: Re: [PATCH v2] firmware: qcom_scm: Only compile legacy calls on ARM
Message-ID: <YGbvXFrMg6X7q3qL@gerhold.net>
References: <20210323224336.1311783-1-swboyd@chromium.org>
 <6ec0ca8d-85c7-53d6-acf2-22c4ac13e805@codeaurora.org>
 <161734672825.2260335.8472441215895199196@swboyd.mtv.corp.google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <161734672825.2260335.8472441215895199196@swboyd.mtv.corp.google.com>
Precedence: bulk

On Thu, Apr 01, 2021 at 11:58:48PM -0700, Stephen Boyd wrote:
> 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?
> 

No, the detection is also needed on ARM64. On ARM32 there can be either
legacy or SMC32, but on ARM64 there can be either SMC32 or SMC64.
You cannot use SMC64 on 32-bit, but you can use SMC32 on ARM64 just
fine. SMC32 is used on MSM8916 for example.

Stephan