diff mbox series

[v2,03/11] firmware: qcom: scm: switch to using the SCM allocator

Message ID 20230928092040.9420-4-brgl@bgdev.pl (mailing list archive)
State Superseded
Headers show
Series arm64: qcom: add and enable SHM Bridge support | expand

Commit Message

Bartosz Golaszewski Sept. 28, 2023, 9:20 a.m. UTC 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.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 SCM
allocator for that memory and shrink the code in process.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/firmware/qcom/qcom_scm-smc.c | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

Comments

Elliot Berman Sept. 28, 2023, 7:11 p.m. UTC | #1 
On 9/28/2023 2:20 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.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 SCM
> allocator for that memory and shrink the code in process.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>

> ---
>  drivers/firmware/qcom/qcom_scm-smc.c | 21 ++++-----------------
>  1 file changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm-smc.c b/drivers/firmware/qcom/qcom_scm-smc.c
> index 16cf88acfa8e..0d5554df1321 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>
> @@ -152,8 +153,7 @@ int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
>  {
>  	int arglen = desc->arginfo & 0xf;
>  	int i, ret;
> -	dma_addr_t args_phys = 0;
> -	void *args_virt = NULL;
> +	void *args_virt __free(qcom_scm_mem) = NULL;
>  	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;
> @@ -173,7 +173,7 @@ int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
>  
>  	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_scm_mem_alloc(PAGE_ALIGN(alloc_len), flag);
>  
>  		if (!args_virt)
>  			return -ENOMEM;
> @@ -192,25 +192,12 @@ 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_scm_mem_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;
>
Bartosz Golaszewski Sept. 29, 2023, 8:15 a.m. UTC | #2 
On Thu, Sep 28, 2023 at 11:21 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.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 SCM
> allocator for that memory and shrink the code in process.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/firmware/qcom/qcom_scm-smc.c | 21 ++++-----------------
>  1 file changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/firmware/qcom/qcom_scm-smc.c b/drivers/firmware/qcom/qcom_scm-smc.c
> index 16cf88acfa8e..0d5554df1321 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>
> @@ -152,8 +153,7 @@ int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
>  {
>         int arglen = desc->arginfo & 0xf;
>         int i, ret;
> -       dma_addr_t args_phys = 0;
> -       void *args_virt = NULL;
> +       void *args_virt __free(qcom_scm_mem) = NULL;
>         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;
> @@ -173,7 +173,7 @@ int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
>
>         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_scm_mem_alloc(PAGE_ALIGN(alloc_len), flag);

Ah, I realized the PAGE_ALIGN() here can be dropped when switching to
the new allocator.

There'll be a v3 anyway so I'll drop it then.

Bart

>
>                 if (!args_virt)
>                         return -ENOMEM;
> @@ -192,25 +192,12 @@ 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_scm_mem_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;
>
> --
> 2.39.2
>
diff mbox series

Patch

diff --git a/drivers/firmware/qcom/qcom_scm-smc.c b/drivers/firmware/qcom/qcom_scm-smc.c
index 16cf88acfa8e..0d5554df1321 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>
@@ -152,8 +153,7 @@  int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
 {
 	int arglen = desc->arginfo & 0xf;
 	int i, ret;
-	dma_addr_t args_phys = 0;
-	void *args_virt = NULL;
+	void *args_virt __free(qcom_scm_mem) = NULL;
 	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;
@@ -173,7 +173,7 @@  int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
 
 	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_scm_mem_alloc(PAGE_ALIGN(alloc_len), flag);
 
 		if (!args_virt)
 			return -ENOMEM;
@@ -192,25 +192,12 @@  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_scm_mem_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;