diff mbox series

[v3,08/15] firmware: qcom: scm: make qcom_scm_ice_set_key() use the TZ allocator

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

Commit Message

Bartosz Golaszewski Oct. 9, 2023, 3:34 p.m. UTC
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Let's use the new TZ memory allocator to obtain a buffer for this call
instead of using dma_alloc_coherent().

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

Comments

Andrew Halaney Oct. 10, 2023, 10:25 p.m. UTC | #1
On Mon, Oct 09, 2023 at 05:34:20PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Let's use the new TZ memory allocator to obtain a buffer for this call
> instead of using dma_alloc_coherent().
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 754f6056b99f..31071a714cf1 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1197,32 +1197,21 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
>  		.args[4] = data_unit_size,
>  		.owner = ARM_SMCCC_OWNER_SIP,
>  	};
> -	void *keybuf;
> -	dma_addr_t key_phys;
> +
>  	int ret;
>  
> -	/*
> -	 * 'key' may point to vmalloc()'ed memory, but we need to pass a
> -	 * physical address that's been properly flushed.  The sanctioned way to
> -	 * do this is by using the DMA API.  But as is best practice for crypto
> -	 * keys, we also must wipe the key after use.  This makes kmemdup() +
> -	 * dma_map_single() not clearly correct, since the DMA API can use
> -	 * bounce buffers.  Instead, just use dma_alloc_coherent().  Programming
> -	 * keys is normally rare and thus not performance-critical.
> -	 */
> -
> -	keybuf = dma_alloc_coherent(__scm->dev, key_size, &key_phys,
> -				    GFP_KERNEL);
> +	void *keybuf __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool,
> +							   key_size,
> +							   GFP_KERNEL);

At the risk of sounding like a broken record, the same nit about
declaration being moved, I'll just mention that one last time here in
the series and then accept the outcome of that discussion across the
series :) Also a bummer to lose that comment, but I guess oh well.

Reviewed-by: Andrew Halaney <ahalaney@redhat.com>

>  	if (!keybuf)
>  		return -ENOMEM;
>  	memcpy(keybuf, key, key_size);
> -	desc.args[1] = key_phys;
> +	desc.args[1] = qcom_tzmem_to_phys(keybuf);
>  
>  	ret = qcom_scm_call(__scm->dev, &desc, NULL);
>  
>  	memzero_explicit(keybuf, key_size);
>  
> -	dma_free_coherent(__scm->dev, key_size, keybuf, key_phys);
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key);
> -- 
> 2.39.2
>
diff mbox series

Patch

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 754f6056b99f..31071a714cf1 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1197,32 +1197,21 @@  int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
 		.args[4] = data_unit_size,
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
-	void *keybuf;
-	dma_addr_t key_phys;
+
 	int ret;
 
-	/*
-	 * 'key' may point to vmalloc()'ed memory, but we need to pass a
-	 * physical address that's been properly flushed.  The sanctioned way to
-	 * do this is by using the DMA API.  But as is best practice for crypto
-	 * keys, we also must wipe the key after use.  This makes kmemdup() +
-	 * dma_map_single() not clearly correct, since the DMA API can use
-	 * bounce buffers.  Instead, just use dma_alloc_coherent().  Programming
-	 * keys is normally rare and thus not performance-critical.
-	 */
-
-	keybuf = dma_alloc_coherent(__scm->dev, key_size, &key_phys,
-				    GFP_KERNEL);
+	void *keybuf __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool,
+							   key_size,
+							   GFP_KERNEL);
 	if (!keybuf)
 		return -ENOMEM;
 	memcpy(keybuf, key, key_size);
-	desc.args[1] = key_phys;
+	desc.args[1] = qcom_tzmem_to_phys(keybuf);
 
 	ret = qcom_scm_call(__scm->dev, &desc, NULL);
 
 	memzero_explicit(keybuf, key_size);
 
-	dma_free_coherent(__scm->dev, key_size, keybuf, key_phys);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key);