Message ID | 20230522044512.4787-1-quic_kathirav@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1213330vqo; Sun, 21 May 2023 22:10:27 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6uROnje7m//+BxY3DKaExca3PKFTefGoZdtbKcZ7TvqVh60JNe2pprg5vmyL+6XSdifGbw X-Received: by 2002:a05:6a00:1703:b0:63f:1eb3:824b with SMTP id h3-20020a056a00170300b0063f1eb3824bmr14818071pfc.17.1684732226937; Sun, 21 May 2023 22:10:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1684732226; cv=none; d=google.com; s=arc-20160816; b=rV+iEevFHD9rYTYsXDSr9X8hVr457nLYysowLPp6G4qgISKvOvehcrJ6ZJwVJ7iFxB 3DG2rbmJliadcR2FYRoHXkP1Banp8pgzy/qrNuHcuAuag6HxSaSVaM47bqqFJc8l7pSw 9PkfjYuqN8Cu9+hRuc0r08R3z/aXP3At9LK2Wq+1UsA3eseEhvI6vlLa64+9EcZvxSKj 2PL5aZfbqbP7UQaF8UW7z4OjLKmu0dZGfwL5oTKb8OfBwunBzwdFjSMjaQNrWjmS8QRu 2CWe79rTUKraIjLZpTNGVJJKnFT9IwXYcTupJZAOwjoV7PvNY6ztXfux+S78u/2h1xsU 87eA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:date:subject:cc:to:from :dkim-signature; bh=q5ychQUgvG+RwtfStD3Hjtu0PUnjz/wL+HYL6NgvXe4=; b=M/qYM/PacBZdCDG0L5OD/ja4v+Nh2Sl4+GKy6qAUlSIVNk+DYjp/yhLSi2GGyEGo7B oRR8uAW99Jnyahs8X0WOfCuDpIE5T6vD8ReYOCarAPodXB4DKl2LmJ7E7SAx5FA9hnB9 quzItI0HY9EkdWlfRyClt6elvMzTPx9DnUNRvzphmzPXu0Uea9VOybG3Eu7M+RD4Yzue dlSpdb22M6RXCHQYNVui9v2TNF1aNeG1PO+NIEMSB30jWHEj2m9u70tm2IMQ92SiSyAW pFCCq6LxxGfJQ7CA24TVXKWmvYGWAzLclXZsnv+hcj9iexu0X+MXY45oZS/QQqtRbK0k K+PA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=Op0d3AXt; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y16-20020aa79430000000b006436b625317si2540926pfo.122.2023.05.21.22.10.14; Sun, 21 May 2023 22:10:26 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=Op0d3AXt; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230095AbjEVEpk (ORCPT <rfc822;cscallsign@gmail.com> + 99 others); Mon, 22 May 2023 00:45:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40954 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229470AbjEVEpg (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 22 May 2023 00:45:36 -0400 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D159BE0; Sun, 21 May 2023 21:45:35 -0700 (PDT) Received: from pps.filterd (m0279863.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 34M4eNww005716; Mon, 22 May 2023 04:45:29 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : mime-version : content-type; s=qcppdkim1; bh=q5ychQUgvG+RwtfStD3Hjtu0PUnjz/wL+HYL6NgvXe4=; b=Op0d3AXt6Kpi7oe0VF3XOpj87ujcXTJh4xNynsOOOJ2UHJgOAFG7PHEJ0aDI134EWdWg HRNlCE1Mi9Oua+5Wv/cT+TzPeU8bNzNKBzAbeUYCsCUVPDNkAvSg3Y+s3zr7Rkb95/k0 g6aIAvrRyd85djbtlxNjxKAbSRkoD6vGpqa093GjhH0IkDs4X9pLncPEnK47Go4hS0Eq bom7Ok8nxaZeqHjQnJ+kl+YNdp/d1Ye+NviGA1y3A4++eljYHsU2ivNZDeeJFPZ6MWWH pFh3lGKyxG3CI020lmJSbUV23BL5lUOwI2zH1Gq+6PiGYJ5YQX8OBUn13MkeAVpNcjMT ZA== Received: from nalasppmta01.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3qppe9amty-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 22 May 2023 04:45:28 +0000 Received: from nalasex01a.na.qualcomm.com (nalasex01a.na.qualcomm.com [10.47.209.196]) by NALASPPMTA01.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 34M4jS6R021611 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 22 May 2023 04:45:28 GMT Received: from kathirav-linux.qualcomm.com (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.42; Sun, 21 May 2023 21:45:25 -0700 From: Kathiravan T <quic_kathirav@quicinc.com> To: Andy Gross <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org> CC: <quic_eberman@quicinc.com>, Kathiravan T <quic_kathirav@quicinc.com> Subject: [PATCH] firmware: qcom_scm: use the SCM_CONVENTION based on ARM / ARM64 Date: Mon, 22 May 2023 10:15:12 +0530 Message-ID: <20230522044512.4787-1-quic_kathirav@quicinc.com> X-Mailer: git-send-email 2.17.1 MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01a.na.qualcomm.com (10.47.209.196) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: noNhBxUQCLjT3E6P1jWeXZPwDOxoyXA9 X-Proofpoint-ORIG-GUID: noNhBxUQCLjT3E6P1jWeXZPwDOxoyXA9 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-05-22_01,2023-05-17_02,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0 suspectscore=0 spamscore=0 malwarescore=0 adultscore=0 clxscore=1015 mlxlogscore=999 priorityscore=1501 mlxscore=0 bulkscore=0 impostorscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000 definitions=main-2305220039 X-Spam-Status: No, score=-0.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SORTED_RECIPS, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1766569779521905690?= X-GMAIL-MSGID: =?utf-8?q?1766569779521905690?= |
Series |
firmware: qcom_scm: use the SCM_CONVENTION based on ARM / ARM64
|
|
Commit Message
Kathiravan Thirumoorthy
May 22, 2023, 4:45 a.m. UTC
During SCM probe, to identify the SCM convention, scm call is made with
SMC_CONVENTION_ARM_64 followed by SMC_CONVENTION_ARM_32. Based on the
result what convention to be used is decided.
IPQ chipsets starting from IPQ807x, supports both 32bit and 64bit kernel
variants, however TZ firmware runs in 64bit mode. When running on 32bit
kernel, scm call is made with SMC_CONVENTION_ARM_64 is causing the
system crash, due to the difference in the register sets between ARM and
AARCH64, which is accessed by the TZ.
To avoid this, use SMC_CONVENTION_ARM_64 only on ARM64 builds.
Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com>
---
drivers/firmware/qcom_scm.c | 2 ++
1 file changed, 2 insertions(+)
Comments
Hi, On Mon, 22 May 2023 at 10:15, Kathiravan T <quic_kathirav@quicinc.com> wrote: > > During SCM probe, to identify the SCM convention, scm call is made with > SMC_CONVENTION_ARM_64 followed by SMC_CONVENTION_ARM_32. Based on the > result what convention to be used is decided. > > IPQ chipsets starting from IPQ807x, supports both 32bit and 64bit kernel > variants, however TZ firmware runs in 64bit mode. When running on 32bit > kernel, scm call is made with SMC_CONVENTION_ARM_64 is causing the > system crash, due to the difference in the register sets between ARM and > AARCH64, which is accessed by the TZ. If a crash is being fixed, should we use a Fixes tag as well? > To avoid this, use SMC_CONVENTION_ARM_64 only on ARM64 builds. > > Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> > --- > drivers/firmware/qcom_scm.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index fde33acd46b7..db6754db48a0 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -171,6 +171,7 @@ static enum qcom_scm_convention __get_convention(void) > if (likely(qcom_scm_convention != SMC_CONVENTION_UNKNOWN)) > return qcom_scm_convention; > > +#if IS_ENABLED(CONFIG_ARM64) > /* > * Device isn't required as there is only one argument - no device > * needed to dma_map_single to secure world > @@ -191,6 +192,7 @@ static enum qcom_scm_convention __get_convention(void) > forced = true; > goto found; > } > +#endif If we are already inside a 'CONFIG_ARM64' define here ^^^, do we even need the following snippet now: /* * Some SC7180 firmwares didn't implement the * QCOM_SCM_INFO_IS_CALL_AVAIL call, so we fallback to forcing ARM_64 * calling conventions on these firmwares. Luckily we don't make any * early calls into the firmware on these SoCs so the device pointer * will be valid here to check if the compatible matches. */ if (of_device_is_compatible(__scm ? __scm->dev->of_node : NULL, "qcom,scm-sc7180")) { forced = true; goto found; } 'forced' will always be 'true' now that we are inside the CONFIG_ARM64 check above, right? So, maybe you can clean-up that path as well. Thanks, Bhupesh
On 5/22/2023 12:03 PM, Bhupesh Sharma wrote: > Hi, > > On Mon, 22 May 2023 at 10:15, Kathiravan T <quic_kathirav@quicinc.com> wrote: >> During SCM probe, to identify the SCM convention, scm call is made with >> SMC_CONVENTION_ARM_64 followed by SMC_CONVENTION_ARM_32. Based on the >> result what convention to be used is decided. >> >> IPQ chipsets starting from IPQ807x, supports both 32bit and 64bit kernel >> variants, however TZ firmware runs in 64bit mode. When running on 32bit >> kernel, scm call is made with SMC_CONVENTION_ARM_64 is causing the >> system crash, due to the difference in the register sets between ARM and >> AARCH64, which is accessed by the TZ. > If a crash is being fixed, should we use a Fixes tag as well? Ack. Will add it in next spin. > >> To avoid this, use SMC_CONVENTION_ARM_64 only on ARM64 builds. >> >> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> >> --- >> drivers/firmware/qcom_scm.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >> index fde33acd46b7..db6754db48a0 100644 >> --- a/drivers/firmware/qcom_scm.c >> +++ b/drivers/firmware/qcom_scm.c >> @@ -171,6 +171,7 @@ static enum qcom_scm_convention __get_convention(void) >> if (likely(qcom_scm_convention != SMC_CONVENTION_UNKNOWN)) >> return qcom_scm_convention; >> >> +#if IS_ENABLED(CONFIG_ARM64) >> /* >> * Device isn't required as there is only one argument - no device >> * needed to dma_map_single to secure world >> @@ -191,6 +192,7 @@ static enum qcom_scm_convention __get_convention(void) >> forced = true; >> goto found; >> } >> +#endif > If we are already inside a 'CONFIG_ARM64' define here ^^^, do we even > need the following snippet now: > > /* > * Some SC7180 firmwares didn't implement the > * QCOM_SCM_INFO_IS_CALL_AVAIL call, so we fallback to forcing ARM_64 > * calling conventions on these firmwares. Luckily we don't make any > * early calls into the firmware on these SoCs so the device pointer > * will be valid here to check if the compatible matches. > */ > if (of_device_is_compatible(__scm ? __scm->dev->of_node : NULL, > "qcom,scm-sc7180")) { > forced = true; > goto found; > } > > 'forced' will always be 'true' now that we are inside the CONFIG_ARM64 > check above, right? > So, maybe you can clean-up that path as well. You want to remove the of_device_is_compatible check and by default set the 'forced' to true, if the SCM call fails? Currently it clearly states why we are forcing the convention. If we remove it, we may not able to identify which target FW lacks this scm call support and why we are forcing it... Thanks, > > Thanks, > Bhupesh
On 5/22/2023 12:03 PM, Bhupesh Sharma wrote: > Hi, > > On Mon, 22 May 2023 at 10:15, Kathiravan T <quic_kathirav@quicinc.com> wrote: >> >> During SCM probe, to identify the SCM convention, scm call is made with >> SMC_CONVENTION_ARM_64 followed by SMC_CONVENTION_ARM_32. Based on the >> result what convention to be used is decided. >> >> IPQ chipsets starting from IPQ807x, supports both 32bit and 64bit kernel >> variants, however TZ firmware runs in 64bit mode. When running on 32bit >> kernel, scm call is made with SMC_CONVENTION_ARM_64 is causing the >> system crash, due to the difference in the register sets between ARM and >> AARCH64, which is accessed by the TZ. > > If a crash is being fixed, should we use a Fixes tag as well? > >> To avoid this, use SMC_CONVENTION_ARM_64 only on ARM64 builds. >> >> Signed-off-by: Kathiravan T <quic_kathirav@quicinc.com> >> --- >> drivers/firmware/qcom_scm.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >> index fde33acd46b7..db6754db48a0 100644 >> --- a/drivers/firmware/qcom_scm.c >> +++ b/drivers/firmware/qcom_scm.c >> @@ -171,6 +171,7 @@ static enum qcom_scm_convention __get_convention(void) >> if (likely(qcom_scm_convention != SMC_CONVENTION_UNKNOWN)) >> return qcom_scm_convention; >> >> +#if IS_ENABLED(CONFIG_ARM64) >> /* >> * Device isn't required as there is only one argument - no device >> * needed to dma_map_single to secure world >> @@ -191,6 +192,7 @@ static enum qcom_scm_convention __get_convention(void) >> forced = true; >> goto found; >> } >> +#endif > > If we are already inside a 'CONFIG_ARM64' define here ^^^, do we even > need the following snippet now: > > /* > * Some SC7180 firmwares didn't implement the > * QCOM_SCM_INFO_IS_CALL_AVAIL call, so we fallback to forcing ARM_64 > * calling conventions on these firmwares. Luckily we don't make any > * early calls into the firmware on these SoCs so the device pointer > * will be valid here to check if the compatible matches. > */ > if (of_device_is_compatible(__scm ? __scm->dev->of_node : NULL, > "qcom,scm-sc7180")) { > forced = true; > goto found; > } > > 'forced' will always be 'true' now that we are inside the CONFIG_ARM64 > check above, right? Well, i think you can't get rid off the variable here as main purpose of the variable seems to be print the mode is being forced here (kind of workaround for a firmware) here and not natural. is not that ? -- Mukesh > So, maybe you can clean-up that path as well. > > Thanks, > Bhupesh
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index fde33acd46b7..db6754db48a0 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -171,6 +171,7 @@ static enum qcom_scm_convention __get_convention(void) if (likely(qcom_scm_convention != SMC_CONVENTION_UNKNOWN)) return qcom_scm_convention; +#if IS_ENABLED(CONFIG_ARM64) /* * Device isn't required as there is only one argument - no device * needed to dma_map_single to secure world @@ -191,6 +192,7 @@ static enum qcom_scm_convention __get_convention(void) forced = true; goto found; } +#endif probed_convention = SMC_CONVENTION_ARM_32; ret = __scm_smc_call(NULL, &desc, probed_convention, &res, true);