diff mbox series

[v3,14/15] firmware: qcom: scm: clarify the comment in qcom_scm_pas_init_image()

Message ID 20231009153427.20951-15-brgl@bgdev.pl
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>

The "memory protection" mechanism mentioned in the comment is the SHM
Bridge. This is also the reason why we do not convert this call to using
the TM mem allocator.

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

Comments

Andrew Halaney Oct. 11, 2023, 9:19 p.m. UTC | #1
On Mon, Oct 09, 2023 at 05:34:26PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> The "memory protection" mechanism mentioned in the comment is the SHM
> Bridge. This is also the reason why we do not convert this call to using
> the TM mem allocator.

s/TM/TZ/ ?

> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 839773270a21..8a2475ced10a 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -563,9 +563,13 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size,
>  	struct qcom_scm_res res;
>  
>  	/*
> -	 * During the scm call memory protection will be enabled for the meta
> -	 * data blob, so make sure it's physically contiguous, 4K aligned and
> -	 * non-cachable to avoid XPU violations.
> +	 * During the SCM call the TrustZone will make the buffer containing
> +	 * the program data into an SHM Bridge. This is why we exceptionally
> +	 * must not use the TrustZone memory allocator here as - depending on
> +	 * Kconfig - it may already use the SHM Bridge mechanism internally.
> +	 *
> +	 * If we pass a buffer that is already part of an SHM Bridge to this
> +	 * call, it will fail.

I can at least confirm this matches my testing results in v2, fwiw.

I guess you could use the Kconfig and conditionally use the TZ mem
allocator if !SHMBridge, but I don't know if its worth the if statements
or not.

Bummer, I can't think of a beautiful way to unify this outside of a
dedicated non SHM bridge pool for this...

>  	 */
>  	mdata_buf = dma_alloc_coherent(__scm->dev, size, &mdata_phys,
>  				       GFP_KERNEL);
> -- 
> 2.39.2
>
diff mbox series

Patch

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 839773270a21..8a2475ced10a 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -563,9 +563,13 @@  int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size,
 	struct qcom_scm_res res;
 
 	/*
-	 * During the scm call memory protection will be enabled for the meta
-	 * data blob, so make sure it's physically contiguous, 4K aligned and
-	 * non-cachable to avoid XPU violations.
+	 * During the SCM call the TrustZone will make the buffer containing
+	 * the program data into an SHM Bridge. This is why we exceptionally
+	 * must not use the TrustZone memory allocator here as - depending on
+	 * Kconfig - it may already use the SHM Bridge mechanism internally.
+	 *
+	 * If we pass a buffer that is already part of an SHM Bridge to this
+	 * call, it will fail.
 	 */
 	mdata_buf = dma_alloc_coherent(__scm->dev, size, &mdata_phys,
 				       GFP_KERNEL);