From 99d52c872d34b9d75fccab1782474cde45426708 Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Sat, 27 Mar 2021 07:56:41 +0100 Subject: [PATCH 01/13] MAINTAINERS: add another entry for ARM/QUALCOMM SUPPORT The files in ./include/linux/soc/qcom/ are headers to the corresponding files in ./drivers/soc/qcom/, which are assigned to ARM/QUALCOMM SUPPORT. Possibly, the file pattern include/linux/*/qcom* intended to match this directory and its containing files, but unfortunately, it does not. Hence, add a file entry for this directory to ARM/QUALCOMM SUPPORT. Signed-off-by: Lukas Bulwahn Link: https://lore.kernel.org/r/20210327065642.11969-2-lukas.bulwahn@gmail.com Signed-off-by: Bjorn Andersson --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index d92f85ca831d..8c6846e6ebcb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2296,6 +2296,7 @@ F: drivers/tty/serial/msm_serial.c F: drivers/usb/dwc3/dwc3-qcom.c F: include/dt-bindings/*/qcom* F: include/linux/*/qcom* +F: include/linux/soc/qcom/ ARM/RADISYS ENP2611 MACHINE SUPPORT M: Lennert Buytenhek From 8058dfa05ab765153f20020fc4ea3b296e391a00 Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Sat, 27 Mar 2021 07:56:42 +0100 Subject: [PATCH 02/13] soc: qcom: address kernel-doc warnings The command: find ./include/linux/soc/qcom/ | xargs ./scripts/kernel-doc -none reports: ./include/linux/soc/qcom/qmi.h:26: warning: cannot understand function prototype: 'struct qmi_header ' ./include/linux/soc/qcom/qmi.h:101: warning: cannot understand function prototype: 'struct qmi_response_type_v01 ' ./include/linux/soc/qcom/irq.h:19: warning: expecting prototype for QCOM specific IRQ domain flags that distinguishes the handling of wakeup(). Prototype was for IRQ_DOMAIN_FLAG_QCOM_PDC_WAKEUP() instead ./include/linux/soc/qcom/apr.h:126: warning: Function parameter or member '__apr_driver' not described in 'module_apr_driver' ./include/linux/soc/qcom/apr.h:126: warning: Excess function parameter '__aprbus_driver' description in 'module_apr_driver' ./include/linux/soc/qcom/llcc-qcom.h:43: warning: cannot understand function prototype: 'struct llcc_slice_desc ' ./include/linux/soc/qcom/llcc-qcom.h:60: warning: cannot understand function prototype: 'struct llcc_edac_reg_data ' ./include/linux/soc/qcom/llcc-qcom.h:86: warning: cannot understand function prototype: 'struct llcc_drv_data ' Address all those warnings by: - prefixing kernel-doc descriptions for structs with the keyword 'struct', - turning a kernel-doc comment that does not follow the kernel-doc syntax into a normal comment, and - correcting a parameter name in a kernel-doc comment. Signed-off-by: Lukas Bulwahn Link: https://lore.kernel.org/r/20210327065642.11969-3-lukas.bulwahn@gmail.com Signed-off-by: Bjorn Andersson --- include/linux/soc/qcom/apr.h | 2 +- include/linux/soc/qcom/irq.h | 2 +- include/linux/soc/qcom/llcc-qcom.h | 6 +++--- include/linux/soc/qcom/qmi.h | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/linux/soc/qcom/apr.h b/include/linux/soc/qcom/apr.h index 7f0bc3cf4d61..137f9f2ac4c3 100644 --- a/include/linux/soc/qcom/apr.h +++ b/include/linux/soc/qcom/apr.h @@ -113,7 +113,7 @@ void apr_driver_unregister(struct apr_driver *drv); /** * module_apr_driver() - Helper macro for registering a aprbus driver - * @__aprbus_driver: aprbus_driver struct + * @__apr_driver: apr_driver struct * * Helper macro for aprbus drivers which do not do anything special in * module init/exit. This eliminates a lot of boilerplate. Each module diff --git a/include/linux/soc/qcom/irq.h b/include/linux/soc/qcom/irq.h index 9e1ece58e55b..72b9231e9fdd 100644 --- a/include/linux/soc/qcom/irq.h +++ b/include/linux/soc/qcom/irq.h @@ -7,7 +7,7 @@ #define GPIO_NO_WAKE_IRQ ~0U -/** +/* * QCOM specific IRQ domain flags that distinguishes the handling of wakeup * capable interrupts by different interrupt controllers. * diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h index 64fc582ae415..437c9df13229 100644 --- a/include/linux/soc/qcom/llcc-qcom.h +++ b/include/linux/soc/qcom/llcc-qcom.h @@ -35,7 +35,7 @@ #define LLCC_WRCACHE 31 /** - * llcc_slice_desc - Cache slice descriptor + * struct llcc_slice_desc - Cache slice descriptor * @slice_id: llcc slice id * @slice_size: Size allocated for the llcc slice */ @@ -45,7 +45,7 @@ struct llcc_slice_desc { }; /** - * llcc_edac_reg_data - llcc edac registers data for each error type + * struct llcc_edac_reg_data - llcc edac registers data for each error type * @name: Name of the error * @synd_reg: Syndrome register address * @count_status_reg: Status register address to read the error count @@ -69,7 +69,7 @@ struct llcc_edac_reg_data { }; /** - * llcc_drv_data - Data associated with the llcc driver + * struct llcc_drv_data - Data associated with the llcc driver * @regmap: regmap associated with the llcc device * @bcast_regmap: regmap associated with llcc broadcast offset * @cfg: pointer to the data structure for slice configuration diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h index e712f94b89fc..b1f80e756d2a 100644 --- a/include/linux/soc/qcom/qmi.h +++ b/include/linux/soc/qcom/qmi.h @@ -16,7 +16,7 @@ struct socket; /** - * qmi_header - wireformat header of QMI messages + * struct qmi_header - wireformat header of QMI messages * @type: type of message * @txn_id: transaction id * @msg_id: message id @@ -93,7 +93,7 @@ struct qmi_elem_info { #define QMI_ERR_NOT_SUPPORTED_V01 94 /** - * qmi_response_type_v01 - common response header (decoded) + * struct qmi_response_type_v01 - common response header (decoded) * @result: result of the transaction * @error: error value, when @result is QMI_RESULT_FAILURE_V01 */ From f553ba158109a4c5b2e0f704fe5b9316375a1a9a Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Thu, 11 Mar 2021 16:33:14 -0800 Subject: [PATCH 03/13] dt-bindings: soc: qcom: wcnss: Add firmware-name property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WCNSS needs firmware which differs between platforms, and possibly boards. Add a new property "firmware-name" to allow the DT to specify the platform/board specific path to this firmware file. Acked-by: Rob Herring Tested-by: Aníbal Limón Link: https://lore.kernel.org/r/20210312003318.3273536-2-bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson --- Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt index 042a2e4159bd..1382b64e1381 100644 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt @@ -24,6 +24,13 @@ block and a BT, WiFi and FM radio block, all using SMD as command channels. "qcom,riva", "qcom,pronto" +- firmware-name: + Usage: optional + Value type: + Definition: specifies the relative firmware image path for the WLAN NV + blob. Defaults to "wlan/prima/WCNSS_qcom_wlan_nv.bin" if + not specified. + = SUBNODES The subnodes of the wcnss node are optional and describe the individual blocks in the WCNSS. From ac3f2784200d490558882c83c25a2d38270b02a2 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Thu, 11 Mar 2021 16:33:16 -0800 Subject: [PATCH 04/13] soc: qcom: wcnss_ctrl: Introduce local variable "dev" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a local variable to carry the struct device *, to reduce the line lengths in the next patch. Tested-by: Aníbal Limón Link: https://lore.kernel.org/r/20210312003318.3273536-4-bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson --- drivers/soc/qcom/wcnss_ctrl.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/soc/qcom/wcnss_ctrl.c b/drivers/soc/qcom/wcnss_ctrl.c index 32bed249f90e..358526b9de06 100644 --- a/drivers/soc/qcom/wcnss_ctrl.c +++ b/drivers/soc/qcom/wcnss_ctrl.c @@ -199,6 +199,7 @@ static int wcnss_download_nv(struct wcnss_ctrl *wcnss, bool *expect_cbc) { struct wcnss_download_nv_req *req; const struct firmware *fw; + struct device *dev = wcnss->dev; const void *data; ssize_t left; int ret; @@ -207,9 +208,9 @@ static int wcnss_download_nv(struct wcnss_ctrl *wcnss, bool *expect_cbc) if (!req) return -ENOMEM; - ret = request_firmware(&fw, NVBIN_FILE, wcnss->dev); + ret = request_firmware(&fw, NVBIN_FILE, dev); if (ret < 0) { - dev_err(wcnss->dev, "Failed to load nv file %s: %d\n", + dev_err(dev, "Failed to load nv file %s: %d\n", NVBIN_FILE, ret); goto free_req; } @@ -235,7 +236,7 @@ static int wcnss_download_nv(struct wcnss_ctrl *wcnss, bool *expect_cbc) ret = rpmsg_send(wcnss->channel, req, req->hdr.len); if (ret < 0) { - dev_err(wcnss->dev, "failed to send smd packet\n"); + dev_err(dev, "failed to send smd packet\n"); goto release_fw; } @@ -248,7 +249,7 @@ static int wcnss_download_nv(struct wcnss_ctrl *wcnss, bool *expect_cbc) ret = wait_for_completion_timeout(&wcnss->ack, WCNSS_REQUEST_TIMEOUT); if (!ret) { - dev_err(wcnss->dev, "timeout waiting for nv upload ack\n"); + dev_err(dev, "timeout waiting for nv upload ack\n"); ret = -ETIMEDOUT; } else { *expect_cbc = wcnss->ack_status == WCNSS_ACK_COLD_BOOTING; From 82ec0c290d1a731a2b33b8a713c5bc96b03fb2d3 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Thu, 11 Mar 2021 16:33:17 -0800 Subject: [PATCH 05/13] soc: qcom: wcnss_ctrl: Allow reading firmware-name from DT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WLAN NV firmware blob differs between platforms, and possibly devices, so add support in the wcnss_ctrl driver for reading the path of this file from DT in order to allow these files to live in a generic file system (or linux-firmware). The new property is optional and the code falls back to the old filename if the property isn't specified. Tested-by: Bryan O'Donoghue Tested-by: Aníbal Limón Link: https://lore.kernel.org/r/20210312003318.3273536-5-bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson --- drivers/soc/qcom/wcnss_ctrl.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/soc/qcom/wcnss_ctrl.c b/drivers/soc/qcom/wcnss_ctrl.c index 358526b9de06..2a06d631e415 100644 --- a/drivers/soc/qcom/wcnss_ctrl.c +++ b/drivers/soc/qcom/wcnss_ctrl.c @@ -200,6 +200,7 @@ static int wcnss_download_nv(struct wcnss_ctrl *wcnss, bool *expect_cbc) struct wcnss_download_nv_req *req; const struct firmware *fw; struct device *dev = wcnss->dev; + const char *nvbin = NVBIN_FILE; const void *data; ssize_t left; int ret; @@ -208,10 +209,13 @@ static int wcnss_download_nv(struct wcnss_ctrl *wcnss, bool *expect_cbc) if (!req) return -ENOMEM; - ret = request_firmware(&fw, NVBIN_FILE, dev); + ret = of_property_read_string(dev->of_node, "firmware-name", &nvbin); + if (ret < 0 && ret != -EINVAL) + goto free_req; + + ret = request_firmware(&fw, nvbin, dev); if (ret < 0) { - dev_err(dev, "Failed to load nv file %s: %d\n", - NVBIN_FILE, ret); + dev_err(dev, "Failed to load nv file %s: %d\n", nvbin, ret); goto free_req; } From 9d11af8b06a811c5c4878625f51ce109e2af4e80 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:34 -0800 Subject: [PATCH 06/13] 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(-) 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); From f6ea568f0ddcdfad52807110ed8983e610f0e03b Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:35 -0800 Subject: [PATCH 07/13] 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 | 53 ++++++++++++++++----------------- drivers/firmware/qcom_scm.h | 7 +++-- 3 files changed, 38 insertions(+), 34 deletions(-) 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; + goto found; - 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]); -} + 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, From 257f2935cbbf14b16912c635fcd8ff43345c953b Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:36 -0800 Subject: [PATCH 08/13] 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(-) 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); From 87abf2ba3846d28e4b5f0e5f9cef873b4352a0a9 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:37 -0800 Subject: [PATCH 09/13] 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(+) 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, From e1cd92da0b33212de5a3da3e913767bd1666dc43 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Tue, 23 Feb 2021 13:45:38 -0800 Subject: [PATCH 10/13] 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(-) 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 From 769738fc49bb578e05d404b481a9241d18147d86 Mon Sep 17 00:00:00 2001 From: Qinglang Miao Date: Wed, 25 Nov 2020 14:50:34 +0800 Subject: [PATCH 11/13] soc: qcom: pdr: Fix error return code in pdr_register_listener Fix to return the error code -EREMOTEIO from pdr_register_listener rather than 0. Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers") Reported-by: Hulk Robot Signed-off-by: Qinglang Miao Link: https://lore.kernel.org/r/20201125065034.154217-1-miaoqinglang@huawei.com Signed-off-by: Bjorn Andersson --- drivers/soc/qcom/pdr_interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/qcom/pdr_interface.c b/drivers/soc/qcom/pdr_interface.c index 209dcdca923f..915d5bc3d46e 100644 --- a/drivers/soc/qcom/pdr_interface.c +++ b/drivers/soc/qcom/pdr_interface.c @@ -153,7 +153,7 @@ static int pdr_register_listener(struct pdr_handle *pdr, if (resp.resp.result != QMI_RESULT_SUCCESS_V01) { pr_err("PDR: %s register listener failed: 0x%x\n", pds->service_path, resp.resp.error); - return ret; + return -EREMOTEIO; } pds->state = resp.curr_state; From 84168d1b54e76a1bcb5192991adde5176abe02e3 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Thu, 7 Jan 2021 15:31:19 -0800 Subject: [PATCH 12/13] soc: qcom: mdt_loader: Validate that p_filesz < p_memsz The code validates that segments of p_memsz bytes of a segment will fit in the provided memory region, but does not validate that p_filesz bytes will, which means that an incorrectly crafted ELF header might write beyond the provided memory region. Fixes: 051fb70fd4ea ("remoteproc: qcom: Driver for the self-authenticating Hexagon v5") Reviewed-by: Sibi Sankar Link: https://lore.kernel.org/r/20210107233119.717173-1-bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson --- drivers/soc/qcom/mdt_loader.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c index 24cd193dec55..2ddaee5ef9cc 100644 --- a/drivers/soc/qcom/mdt_loader.c +++ b/drivers/soc/qcom/mdt_loader.c @@ -230,6 +230,14 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, break; } + if (phdr->p_filesz > phdr->p_memsz) { + dev_err(dev, + "refusing to load segment %d with p_filesz > p_memsz\n", + i); + ret = -EINVAL; + break; + } + ptr = mem_region + offset; if (phdr->p_filesz && phdr->p_offset < fw->size) { From 0648c55e3a21ccd816e99b6600d6199fbf39d23a Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Thu, 7 Jan 2021 15:25:26 -0800 Subject: [PATCH 13/13] soc: qcom: mdt_loader: Detect truncated read of segments Given that no validation of how much data the firmware loader read in for a given segment truncated segment files would best case result in a hash verification failure, without any indication of what went wrong. Improve this by validating that the firmware loader did return the amount of data requested. Fixes: 445c2410a449 ("soc: qcom: mdt_loader: Use request_firmware_into_buf()") Reviewed-by: Sibi Sankar Link: https://lore.kernel.org/r/20210107232526.716989-1-bjorn.andersson@linaro.org Signed-off-by: Bjorn Andersson --- drivers/soc/qcom/mdt_loader.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c index 2ddaee5ef9cc..eba7f76f9d61 100644 --- a/drivers/soc/qcom/mdt_loader.c +++ b/drivers/soc/qcom/mdt_loader.c @@ -261,6 +261,15 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, break; } + if (seg_fw->size != phdr->p_filesz) { + dev_err(dev, + "failed to load segment %d from truncated file %s\n", + i, fw_name); + release_firmware(seg_fw); + ret = -EINVAL; + break; + } + release_firmware(seg_fw); }