From 9d11af8b06a811c5c4878625f51ce109e2af4e80 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:34 -0800 Subject: firmware: qcom_scm: Make __qcom_scm_is_call_available() return bool Make __qcom_scm_is_call_available() return bool instead of int. The function has "is" in the name, so it should return a bool to indicate the truth of the call being available. Unfortunately, it can return a number < 0 which also looks "true", but not all callers expect that and thus they think a call is available when really the check to see if the call is available failed to figure it out. Reviewed-by: Bjorn Andersson Cc: Elliot Berman Cc: Brian Masney Cc: Stephan Gerhold Cc: Jeffrey Hugo Cc: Douglas Anderson Fixes: 0f206514749b ("scsi: firmware: qcom_scm: Add support for programming inline crypto keys") Fixes: 0434a4061471 ("firmware: qcom: scm: add support to restore secure config to qcm_scm-32") Fixes: b0a1614fb1f5 ("firmware: qcom: scm: add OCMEM lock/unlock interface") Signed-off-by: Stephen Boyd Link: https://lore.kernel.org/r/20210223214539.1336155-2-swboyd@chromium.org Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom_scm.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index f57779fc7ee9..2be5573dce53 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -113,9 +113,6 @@ static void qcom_scm_clk_disable(void) clk_disable_unprepare(__scm->bus_clk); } -static int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, - u32 cmd_id); - enum qcom_scm_convention qcom_scm_convention; static bool has_queried __read_mostly; static DEFINE_SPINLOCK(query_lock); @@ -219,8 +216,8 @@ static int qcom_scm_call_atomic(struct device *dev, } } -static int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, - u32 cmd_id) +static bool __qcom_scm_is_call_available(struct device *dev, u32 svc_id, + u32 cmd_id) { int ret; struct qcom_scm_desc desc = { @@ -247,7 +244,7 @@ static int __qcom_scm_is_call_available(struct device *dev, u32 svc_id, ret = qcom_scm_call(dev, &desc, &res); - return ret ? : res.result[0]; + return ret ? false : !!res.result[0]; } /** @@ -585,9 +582,8 @@ bool qcom_scm_pas_supported(u32 peripheral) }; struct qcom_scm_res res; - ret = __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL, - QCOM_SCM_PIL_PAS_IS_SUPPORTED); - if (ret <= 0) + if (!__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_PIL, + QCOM_SCM_PIL_PAS_IS_SUPPORTED)) return false; ret = qcom_scm_call(__scm->dev, &desc, &res); @@ -1060,17 +1056,18 @@ EXPORT_SYMBOL(qcom_scm_ice_set_key); */ bool qcom_scm_hdcp_available(void) { + bool avail; int ret = qcom_scm_clk_enable(); if (ret) return ret; - ret = __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_HDCP, + avail = __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_HDCP, QCOM_SCM_HDCP_INVOKE); qcom_scm_clk_disable(); - return ret > 0; + return avail; } EXPORT_SYMBOL(qcom_scm_hdcp_available); -- cgit v1.2.3 From f6ea568f0ddcdfad52807110ed8983e610f0e03b Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:35 -0800 Subject: firmware: qcom_scm: Reduce locking section for __get_convention() We shouldn't need to hold this spinlock here around the entire SCM call into the firmware and back. Instead, we should be able to query the firmware, potentially in parallel with other CPUs making the same convention detection firmware call, and then grab the lock to update the calling convention detected. The convention doesn't change at runtime so calling into firmware more than once is possibly wasteful but simpler. Besides, this is the slow path, not the fast path where we've already detected the convention used. More importantly, this allows us to add more logic here to workaround the case where the firmware call to check for availability isn't implemented in the firmware at all. In that case we can check the firmware node compatible string and force a calling convention. Note that we remove the 'has_queried' logic that is repeated twice. That could lead to the calling convention being printed multiple times to the kernel logs if the bool is true but __query_convention() is running on multiple CPUs. We also shorten the time where the lock is held, but we keep the lock held around the printk because it doesn't seem hugely important to drop it for that. Cc: Elliot Berman Cc: Brian Masney Cc: Stephan Gerhold Cc: Jeffrey Hugo Cc: Douglas Anderson Fixes: 9a434cee773a ("firmware: qcom_scm: Dynamically support SMCCC and legacy conventions") Signed-off-by: Stephen Boyd Link: https://lore.kernel.org/r/20210223214539.1336155-3-swboyd@chromium.org Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom_scm-smc.c | 12 +++++---- drivers/firmware/qcom_scm.c | 55 ++++++++++++++++++++--------------------- drivers/firmware/qcom_scm.h | 7 ++++-- 3 files changed, 39 insertions(+), 35 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qcom_scm-smc.c b/drivers/firmware/qcom_scm-smc.c index 497c13ba98d6..d111833364ba 100644 --- a/drivers/firmware/qcom_scm-smc.c +++ b/drivers/firmware/qcom_scm-smc.c @@ -77,8 +77,10 @@ static void __scm_smc_do(const struct arm_smccc_args *smc, } while (res->a0 == QCOM_SCM_V2_EBUSY); } -int scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc, - struct qcom_scm_res *res, bool atomic) + +int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc, + enum qcom_scm_convention qcom_convention, + struct qcom_scm_res *res, bool atomic) { int arglen = desc->arginfo & 0xf; int i; @@ -87,9 +89,8 @@ int scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc, size_t alloc_len; gfp_t flag = atomic ? GFP_ATOMIC : GFP_KERNEL; u32 smccc_call_type = atomic ? ARM_SMCCC_FAST_CALL : ARM_SMCCC_STD_CALL; - u32 qcom_smccc_convention = - (qcom_scm_convention == SMC_CONVENTION_ARM_32) ? - ARM_SMCCC_SMC_32 : ARM_SMCCC_SMC_64; + u32 qcom_smccc_convention = (qcom_convention == SMC_CONVENTION_ARM_32) ? + ARM_SMCCC_SMC_32 : ARM_SMCCC_SMC_64; struct arm_smccc_res smc_res; struct arm_smccc_args smc = {0}; @@ -148,4 +149,5 @@ int scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc, } return (long)smc_res.a0 ? qcom_scm_remap_error(smc_res.a0) : 0; + } diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index 2be5573dce53..21e07a464bd9 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -113,11 +113,10 @@ static void qcom_scm_clk_disable(void) clk_disable_unprepare(__scm->bus_clk); } -enum qcom_scm_convention qcom_scm_convention; -static bool has_queried __read_mostly; -static DEFINE_SPINLOCK(query_lock); +enum qcom_scm_convention qcom_scm_convention = SMC_CONVENTION_UNKNOWN; +static DEFINE_SPINLOCK(scm_query_lock); -static void __query_convention(void) +static enum qcom_scm_convention __get_convention(void) { unsigned long flags; struct qcom_scm_desc desc = { @@ -130,36 +129,36 @@ static void __query_convention(void) .owner = ARM_SMCCC_OWNER_SIP, }; struct qcom_scm_res res; + enum qcom_scm_convention probed_convention; int ret; - spin_lock_irqsave(&query_lock, flags); - if (has_queried) - goto out; + if (likely(qcom_scm_convention != SMC_CONVENTION_UNKNOWN)) + return qcom_scm_convention; - qcom_scm_convention = SMC_CONVENTION_ARM_64; - // Device isn't required as there is only one argument - no device - // needed to dma_map_single to secure world - ret = scm_smc_call(NULL, &desc, &res, true); + /* + * Device isn't required as there is only one argument - no device + * needed to dma_map_single to secure world + */ + probed_convention = SMC_CONVENTION_ARM_64; + ret = __scm_smc_call(NULL, &desc, probed_convention, &res, true); if (!ret && res.result[0] == 1) - goto out; + goto found; - qcom_scm_convention = SMC_CONVENTION_ARM_32; - ret = scm_smc_call(NULL, &desc, &res, true); + probed_convention = SMC_CONVENTION_ARM_32; + ret = __scm_smc_call(NULL, &desc, probed_convention, &res, true); if (!ret && res.result[0] == 1) - goto out; - - qcom_scm_convention = SMC_CONVENTION_LEGACY; -out: - has_queried = true; - spin_unlock_irqrestore(&query_lock, flags); - pr_info("qcom_scm: convention: %s\n", - qcom_scm_convention_names[qcom_scm_convention]); -} + goto found; + + probed_convention = SMC_CONVENTION_LEGACY; +found: + spin_lock_irqsave(&scm_query_lock, flags); + if (probed_convention != qcom_scm_convention) { + qcom_scm_convention = probed_convention; + pr_info("qcom_scm: convention: %s\n", + qcom_scm_convention_names[qcom_scm_convention]); + } + spin_unlock_irqrestore(&scm_query_lock, flags); -static inline enum qcom_scm_convention __get_convention(void) -{ - if (unlikely(!has_queried)) - __query_convention(); return qcom_scm_convention; } @@ -1239,7 +1238,7 @@ static int qcom_scm_probe(struct platform_device *pdev) __scm = scm; __scm->dev = &pdev->dev; - __query_convention(); + __get_convention(); /* * If requested enable "download mode", from this point on warmboot diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h index 95cd1ac30ab0..632fe3142462 100644 --- a/drivers/firmware/qcom_scm.h +++ b/drivers/firmware/qcom_scm.h @@ -61,8 +61,11 @@ struct qcom_scm_res { }; #define SCM_SMC_FNID(s, c) ((((s) & 0xFF) << 8) | ((c) & 0xFF)) -extern int scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc, - struct qcom_scm_res *res, bool atomic); +extern int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc, + enum qcom_scm_convention qcom_convention, + struct qcom_scm_res *res, bool atomic); +#define scm_smc_call(dev, desc, res, atomic) \ + __scm_smc_call((dev), (desc), qcom_scm_convention, (res), (atomic)) #define SCM_LEGACY_FNID(s, c) (((s) << 10) | ((c) & 0x3ff)) extern int scm_legacy_call_atomic(struct device *dev, -- cgit v1.2.3 From 257f2935cbbf14b16912c635fcd8ff43345c953b Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:36 -0800 Subject: firmware: qcom_scm: Workaround lack of "is available" call on SC7180 Some SC7180 firmwares don't implement the QCOM_SCM_INFO_IS_CALL_AVAIL API, so we can't probe the calling convention. We detect the legacy calling convention on these firmwares, because the availability call always fails and legacy is the fallback. This leads to problems where the rmtfs driver fails to probe, because it tries to assign memory with a bad calling convention, which then leads to modem failing to load and all networking, even wifi, to fail. Ouch! Let's force the calling convention to be what it always is on this SoC, i.e. arm64. Of course, the calling convention is not the same thing as implementing the QCOM_SCM_INFO_IS_CALL_AVAIL API. The absence of the "is this call available" API from the firmware means that any call to __qcom_scm_is_call_available() fails. This is OK for now though because none of the calls that are checked for existence are implemented on firmware running on sc7180. If such a call needs to be checked for existence in the future, we presume that firmware will implement this API and then things will "just work". Cc: Elliot Berman Cc: Brian Masney Cc: Stephan Gerhold Cc: Jeffrey Hugo Cc: Douglas Anderson Fixes: 9a434cee773a ("firmware: qcom_scm: Dynamically support SMCCC and legacy conventions") Signed-off-by: Stephen Boyd Link: https://lore.kernel.org/r/20210223214539.1336155-4-swboyd@chromium.org Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom_scm.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index 21e07a464bd9..9ac84b5d6ce0 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -131,6 +131,7 @@ static enum qcom_scm_convention __get_convention(void) struct qcom_scm_res res; enum qcom_scm_convention probed_convention; int ret; + bool forced = false; if (likely(qcom_scm_convention != SMC_CONVENTION_UNKNOWN)) return qcom_scm_convention; @@ -144,6 +145,18 @@ static enum qcom_scm_convention __get_convention(void) if (!ret && res.result[0] == 1) goto found; + /* + * 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; + } + probed_convention = SMC_CONVENTION_ARM_32; ret = __scm_smc_call(NULL, &desc, probed_convention, &res, true); if (!ret && res.result[0] == 1) @@ -154,8 +167,9 @@ found: spin_lock_irqsave(&scm_query_lock, flags); if (probed_convention != qcom_scm_convention) { qcom_scm_convention = probed_convention; - pr_info("qcom_scm: convention: %s\n", - qcom_scm_convention_names[qcom_scm_convention]); + pr_info("qcom_scm: convention: %s%s\n", + qcom_scm_convention_names[qcom_scm_convention], + forced ? " (forced)" : ""); } spin_unlock_irqrestore(&scm_query_lock, flags); -- cgit v1.2.3 From 87abf2ba3846d28e4b5f0e5f9cef873b4352a0a9 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:37 -0800 Subject: firmware: qcom_scm: Suppress sysfs bind attributes We don't want userspace ejecting this driver at runtime. Various other drivers call into this code because it provides the mechanism to communicate with the secure world on qcom SoCs. It should probe once and be present forever after that. Cc: Elliot Berman Cc: Brian Masney Cc: Stephan Gerhold Cc: Jeffrey Hugo Cc: Douglas Anderson Signed-off-by: Stephen Boyd Link: https://lore.kernel.org/r/20210223214539.1336155-5-swboyd@chromium.org Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom_scm.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index 9ac84b5d6ce0..ee9cb545e73b 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -1301,6 +1301,7 @@ static struct platform_driver qcom_scm_driver = { .driver = { .name = "qcom_scm", .of_match_table = qcom_scm_dt_match, + .suppress_bind_attrs = true, }, .probe = qcom_scm_probe, .shutdown = qcom_scm_shutdown, -- cgit v1.2.3 From e1cd92da0b33212de5a3da3e913767bd1666dc43 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:38 -0800 Subject: firmware: qcom_scm: Fix kernel-doc function names to match These functions were renamed but the kernel doc didn't follow along. Fix it. Cc: Elliot Berman Cc: Brian Masney Cc: Stephan Gerhold Cc: Jeffrey Hugo Cc: Douglas Anderson Fixes: 9a434cee773a ("firmware: qcom_scm: Dynamically support SMCCC and legacy conventions") Signed-off-by: Stephen Boyd Link: https://lore.kernel.org/r/20210223214539.1336155-6-swboyd@chromium.org Signed-off-by: Bjorn Andersson --- drivers/firmware/qcom_scm-legacy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/qcom_scm-legacy.c b/drivers/firmware/qcom_scm-legacy.c index eba6b60bfb61..1829ba220576 100644 --- a/drivers/firmware/qcom_scm-legacy.c +++ b/drivers/firmware/qcom_scm-legacy.c @@ -118,7 +118,7 @@ static void __scm_legacy_do(const struct arm_smccc_args *smc, } /** - * qcom_scm_call() - Sends a command to the SCM and waits for the command to + * scm_legacy_call() - Sends a command to the SCM and waits for the command to * finish processing. * * A note on cache maintenance: @@ -209,7 +209,7 @@ out: (n & 0xf)) /** - * qcom_scm_call_atomic() - Send an atomic SCM command with up to 5 arguments + * scm_legacy_call_atomic() - Send an atomic SCM command with up to 5 arguments * and 3 return values * @desc: SCM call descriptor containing arguments * @res: SCM call return values -- cgit v1.2.3