| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9315c8f1-19b8-9b11-8eb4-32b498f91659@quicinc.com>
Date: Mon, 20 Nov 2023 22:38:21 -0800
From: Prasad Sodagudi <quic_psodagud@...cinc.com>
To: Bartosz Golaszewski <brgl@...ev.pl>,
Andy Gross <agross@...nel.org>,
"Bjorn Andersson" <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Elliot Berman <quic_eberman@...cinc.com>,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Guru Das Srinagesh <quic_gurus@...cinc.com>,
Andrew Halaney <ahalaney@...hat.com>,
"Maximilian Luz" <luzmaximilian@...il.com>,
Alex Elder <elder@...aro.org>,
"Srini Kandagatla" <srinivas.kandagatla@...aro.org>
CC: <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <kernel@...cinc.com>,
"Bartosz Golaszewski" <bartosz.golaszewski@...aro.org>
Subject: Re: [RESEND PATCH v5 03/12] firmware: qcom: scm: smc: switch to using
the SCM allocator
On 11/20/2023 5:21 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
>
> We need to allocate, map and pass a buffer to the trustzone if we have
> more than 4 arguments for a given SCM calls. Let's use the new TrustZone
> allocator for that memory and shrink the code in process.
>
> As this code lives in a different compilation unit than the rest of the
> SCM code, we need to provide a helper in the form of
> qcom_scm_get_tzmem_pool() that allows the SMC low-level routines to
> access the SCM memory pool.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> Reviewed-by: Andrew Halaney <ahalaney@...hat.com>
> Tested-by: Andrew Halaney <ahalaney@...hat.com> # sc8280xp-lenovo-thinkpad-x13s
> ---
> drivers/firmware/qcom/qcom_scm-smc.c | 30 ++++++++--------------------
> drivers/firmware/qcom/qcom_scm.c | 5 +++++
> drivers/firmware/qcom/qcom_scm.h | 3 +++
> 3 files changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/firmware/qcom/qcom_scm-smc.c b/drivers/firmware/qcom/qcom_scm-smc.c
> index 16cf88acfa8e..dca5f3f1883b 100644
> --- a/drivers/firmware/qcom/qcom_scm-smc.c
> +++ b/drivers/firmware/qcom/qcom_scm-smc.c
> @@ -2,6 +2,7 @@
> /* Copyright (c) 2015,2019 The Linux Foundation. All rights reserved.
> */
>
> +#include <linux/cleanup.h>
> #include <linux/io.h>
> #include <linux/errno.h>
> #include <linux/delay.h>
> @@ -9,6 +10,7 @@
> #include <linux/slab.h>
> #include <linux/types.h>
> #include <linux/firmware/qcom/qcom_scm.h>
> +#include <linux/firmware/qcom/qcom_tzmem.h>
> #include <linux/arm-smccc.h>
> #include <linux/dma-mapping.h>
>
> @@ -150,11 +152,10 @@ 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)
> {
> + struct qcom_tzmem_pool *mempool = qcom_scm_get_tzmem_pool();
> int arglen = desc->arginfo & 0xf;
> int i, ret;
> - dma_addr_t args_phys = 0;
> - void *args_virt = NULL;
> - size_t alloc_len;
> + void *args_virt __free(qcom_tzmem) = NULL;
> 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_convention == SMC_CONVENTION_ARM_32) ?
> @@ -172,9 +173,9 @@ int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
> smc.args[i + SCM_SMC_FIRST_REG_IDX] = desc->args[i];
>
> if (unlikely(arglen > SCM_SMC_N_REG_ARGS)) {
> - alloc_len = SCM_SMC_N_EXT_ARGS * sizeof(u64);
> - args_virt = kzalloc(PAGE_ALIGN(alloc_len), flag);
> -
> + args_virt = qcom_tzmem_alloc(mempool,
> + SCM_SMC_N_EXT_ARGS * sizeof(u64),
> + flag);
I remember seeing page alignment for this memory allocation in
downstream code too.
I think, after moving to qcom_tzmem_alloc page alignment is not
followed. Is this cross checked with firmware requirements?
> if (!args_virt)
> return -ENOMEM;
>
> @@ -192,25 +193,10 @@ int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
> SCM_SMC_FIRST_EXT_IDX]);
> }
>
> - args_phys = dma_map_single(dev, args_virt, alloc_len,
> - DMA_TO_DEVICE);
> -
> - if (dma_mapping_error(dev, args_phys)) {
> - kfree(args_virt);
> - return -ENOMEM;
> - }
> -
> - smc.args[SCM_SMC_LAST_REG_IDX] = args_phys;
> + smc.args[SCM_SMC_LAST_REG_IDX] = qcom_tzmem_to_phys(args_virt);
> }
>
> - /* ret error check follows after args_virt cleanup*/
> ret = __scm_smc_do(dev, &smc, &smc_res, atomic);
> -
> - if (args_virt) {
> - dma_unmap_single(dev, args_phys, alloc_len, DMA_TO_DEVICE);
> - kfree(args_virt);
> - }
> -
> if (ret)
> return ret;
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 0d4c028be0c1..71e98b666391 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -201,6 +201,11 @@ static void qcom_scm_bw_disable(void)
> enum qcom_scm_convention qcom_scm_convention = SMC_CONVENTION_UNKNOWN;
> static DEFINE_SPINLOCK(scm_query_lock);
>
> +struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void)
> +{
> + return __scm->mempool;
> +}
> +
> static enum qcom_scm_convention __get_convention(void)
> {
> unsigned long flags;
> diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
> index 4532907e8489..aa7d06939f8e 100644
> --- a/drivers/firmware/qcom/qcom_scm.h
> +++ b/drivers/firmware/qcom/qcom_scm.h
> @@ -5,6 +5,7 @@
> #define __QCOM_SCM_INT_H
>
> struct device;
> +struct qcom_tzmem_pool;
>
> enum qcom_scm_convention {
> SMC_CONVENTION_UNKNOWN,
> @@ -78,6 +79,8 @@ int scm_legacy_call_atomic(struct device *dev, const struct qcom_scm_desc *desc,
> int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
> struct qcom_scm_res *res);
>
> +struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void);
> +
> #define QCOM_SCM_SVC_BOOT 0x01
> #define QCOM_SCM_BOOT_SET_ADDR 0x01
> #define QCOM_SCM_BOOT_TERMINATE_PC 0x02
Powered by blists - more mailing lists