diff mbox series

[RESEND,v5,02/12] firmware: qcom: scm: enable the TZ mem allocator

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

Commit Message

Bartosz Golaszewski Nov. 20, 2023, 1:21 p.m. UTC 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Select the TrustZone memory allocator in Kconfig and create a pool of
memory shareable with the TrustZone when probing the SCM driver.

This will allow a gradual conversion of all relevant SCM calls to using
the dedicated allocator.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
---
 drivers/firmware/qcom/Kconfig    |  1 +
 drivers/firmware/qcom/qcom_scm.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

Comments

Prasad Sodagudi Nov. 21, 2023, 6:35 a.m. UTC | #1 
On 11/20/2023 5:21 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Select the TrustZone memory allocator in Kconfig and create a pool of
> memory shareable with the TrustZone when probing the SCM driver.
>
> This will allow a gradual conversion of all relevant SCM calls to using
> the dedicated allocator.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
> Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
> ---
>   drivers/firmware/qcom/Kconfig    |  1 +
>   drivers/firmware/qcom/qcom_scm.c | 16 ++++++++++++++++
>   2 files changed, 17 insertions(+)
>
> diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> index b80269a28224..237da40de832 100644
> --- a/drivers/firmware/qcom/Kconfig
> +++ b/drivers/firmware/qcom/Kconfig
> @@ -7,6 +7,7 @@
>   menu "Qualcomm firmware drivers"
>   
>   config QCOM_SCM
> +	select QCOM_TZMEM
>   	tristate
>   
>   config QCOM_TZMEM
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 520de9b5633a..0d4c028be0c1 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -8,8 +8,10 @@
>   #include <linux/completion.h>
>   #include <linux/cpumask.h>
>   #include <linux/dma-mapping.h>
> +#include <linux/err.h>
>   #include <linux/export.h>
>   #include <linux/firmware/qcom/qcom_scm.h>
> +#include <linux/firmware/qcom/qcom_tzmem.h>
>   #include <linux/init.h>
>   #include <linux/interconnect.h>
>   #include <linux/interrupt.h>
> @@ -20,9 +22,11 @@
>   #include <linux/of_platform.h>
>   #include <linux/platform_device.h>
>   #include <linux/reset-controller.h>
> +#include <linux/sizes.h>
>   #include <linux/types.h>
>   
>   #include "qcom_scm.h"
> +#include "qcom_tzmem.h"
>   
>   static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
>   module_param(download_mode, bool, 0);
> @@ -41,6 +45,8 @@ struct qcom_scm {
>   	int scm_vote_count;
>   
>   	u64 dload_mode_addr;
> +
> +	struct qcom_tzmem_pool *mempool;
>   };
>   
>   struct qcom_scm_current_perm_info {
> @@ -1887,6 +1893,16 @@ static int qcom_scm_probe(struct platform_device *pdev)
>   	if (of_property_read_bool(pdev->dev.of_node, "qcom,sdi-enabled"))
>   		qcom_scm_disable_sdi();
>   
> +	ret = qcom_tzmem_enable(__scm->dev);
> +	if (ret)
> +		return dev_err_probe(__scm->dev, ret,
> +				     "Failed to enable the TrustZone memory allocator\n");

Based on my knowledge shmbridge is supported from sm8250 SoC in the 
firmware.

sdm845 and couple of other targets may not have support shmbridge 
support and probe will be fail for these targets right?

> +
> +	__scm->mempool = devm_qcom_tzmem_pool_new(__scm->dev, SZ_256K);
> +	if (IS_ERR(__scm->mempool))
> +		return dev_err_probe(__scm->dev, PTR_ERR(__scm->mempool),
> +				     "Failed to create the SCM memory pool\n");
> +
>   	/*
>   	 * Initialize the QSEECOM interface.
>   	 *
Bartosz Golaszewski Nov. 21, 2023, 9:25 a.m. UTC | #2 
On Tue, 21 Nov 2023 at 07:35, Prasad Sodagudi <quic_psodagud@quicinc.com> wrote:
>
>
> On 11/20/2023 5:21 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Select the TrustZone memory allocator in Kconfig and create a pool of
> > memory shareable with the TrustZone when probing the SCM driver.
> >
> > This will allow a gradual conversion of all relevant SCM calls to using
> > the dedicated allocator.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
> > Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
> > ---
> >   drivers/firmware/qcom/Kconfig    |  1 +
> >   drivers/firmware/qcom/qcom_scm.c | 16 ++++++++++++++++
> >   2 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
> > index b80269a28224..237da40de832 100644
> > --- a/drivers/firmware/qcom/Kconfig
> > +++ b/drivers/firmware/qcom/Kconfig
> > @@ -7,6 +7,7 @@
> >   menu "Qualcomm firmware drivers"
> >
> >   config QCOM_SCM
> > +     select QCOM_TZMEM
> >       tristate
> >
> >   config QCOM_TZMEM
> > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> > index 520de9b5633a..0d4c028be0c1 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -8,8 +8,10 @@
> >   #include <linux/completion.h>
> >   #include <linux/cpumask.h>
> >   #include <linux/dma-mapping.h>
> > +#include <linux/err.h>
> >   #include <linux/export.h>
> >   #include <linux/firmware/qcom/qcom_scm.h>
> > +#include <linux/firmware/qcom/qcom_tzmem.h>
> >   #include <linux/init.h>
> >   #include <linux/interconnect.h>
> >   #include <linux/interrupt.h>
> > @@ -20,9 +22,11 @@
> >   #include <linux/of_platform.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/reset-controller.h>
> > +#include <linux/sizes.h>
> >   #include <linux/types.h>
> >
> >   #include "qcom_scm.h"
> > +#include "qcom_tzmem.h"
> >
> >   static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
> >   module_param(download_mode, bool, 0);
> > @@ -41,6 +45,8 @@ struct qcom_scm {
> >       int scm_vote_count;
> >
> >       u64 dload_mode_addr;
> > +
> > +     struct qcom_tzmem_pool *mempool;
> >   };
> >
> >   struct qcom_scm_current_perm_info {
> > @@ -1887,6 +1893,16 @@ static int qcom_scm_probe(struct platform_device *pdev)
> >       if (of_property_read_bool(pdev->dev.of_node, "qcom,sdi-enabled"))
> >               qcom_scm_disable_sdi();
> >
> > +     ret = qcom_tzmem_enable(__scm->dev);
> > +     if (ret)
> > +             return dev_err_probe(__scm->dev, ret,
> > +                                  "Failed to enable the TrustZone memory allocator\n");
>
> Based on my knowledge shmbridge is supported from sm8250 SoC in the
> firmware.
>
> sdm845 and couple of other targets may not have support shmbridge
> support and probe will be fail for these targets right?
>

We check the availability of the SHM Bridge calls on the platform at
run-time before enabling it. We'll simply fall-back to not using SHM
bridge in this case.

Bart

> > +
> > +     __scm->mempool = devm_qcom_tzmem_pool_new(__scm->dev, SZ_256K);
> > +     if (IS_ERR(__scm->mempool))
> > +             return dev_err_probe(__scm->dev, PTR_ERR(__scm->mempool),
> > +                                  "Failed to create the SCM memory pool\n");
> > +
> >       /*
> >        * Initialize the QSEECOM interface.
> >        *
kernel test robot Nov. 24, 2023, 3:23 p.m. UTC | #3 
Hi Bartosz,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.7-rc2 next-20231124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/firmware-qcom-add-a-dedicated-TrustZone-buffer-allocator/20231120-213154
base:   linus/master
patch link:    https://lore.kernel.org/r/20231120132118.30473-3-brgl%40bgdev.pl
patch subject: [RESEND PATCH v5 02/12] firmware: qcom: scm: enable the TZ mem allocator
config: arm-randconfig-r071-20231123 (https://download.01.org/0day-ci/archive/20231124/202311241607.v3tvaLvH-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231124/202311241607.v3tvaLvH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311241607.v3tvaLvH-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/firmware/qcom/qcom_tzmem.c: In function 'qcom_tzmem_pool_new':
>> drivers/firmware/qcom/qcom_tzmem.c:87:64: error: passing argument 3 of 'dma_alloc_coherent' from incompatible pointer type [-Werror=incompatible-pointer-types]
      87 |         pool->vbase = dma_alloc_coherent(qcom_tzmem_dev, size, &pool->pbase,
         |                                                                ^~~~~~~~~~~~
         |                                                                |
         |                                                                phys_addr_t * {aka unsigned int *}
   In file included from drivers/firmware/qcom/qcom_tzmem.c:10:
   include/linux/dma-mapping.h:429:29: note: expected 'dma_addr_t *' {aka 'long long unsigned int *'} but argument is of type 'phys_addr_t *' {aka 'unsigned int *'}
     429 |                 dma_addr_t *dma_handle, gfp_t gfp)
         |                 ~~~~~~~~~~~~^~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/dma_alloc_coherent +87 drivers/firmware/qcom/qcom_tzmem.c

757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   59  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   60  /**
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   61   * qcom_tzmem_pool_new() - Create a new TZ memory pool.
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   62   * @size: Size of the new pool in bytes.
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   63   *
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   64   * Create a new pool of memory suitable for sharing with the TrustZone.
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   65   *
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   66   * Must not be used in atomic context.
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   67   *
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   68   * Returns:
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   69   * New memory pool address or ERR_PTR() on error.
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   70   */
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   71  struct qcom_tzmem_pool *qcom_tzmem_pool_new(size_t size)
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   72  {
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   73  	struct qcom_tzmem_pool *pool;
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   74  	int ret = -ENOMEM;
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   75  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   76  	if (!size)
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   77  		return ERR_PTR(-EINVAL);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   78  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   79  	size = PAGE_ALIGN(size);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   80  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   81  	pool = kzalloc(sizeof(*pool), GFP_KERNEL);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   82  	if (!pool)
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   83  		return ERR_PTR(-ENOMEM);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   84  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   85  	pool->size = size;
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   86  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  @87  	pool->vbase = dma_alloc_coherent(qcom_tzmem_dev, size, &pool->pbase,
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   88  					 GFP_KERNEL);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   89  	if (!pool->vbase)
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   90  		goto err_kfree_pool;
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   91  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   92  	pool->pool = gen_pool_create(PAGE_SHIFT, -1);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   93  	if (!pool)
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   94  		goto err_dma_free;
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   95  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   96  	gen_pool_set_algo(pool->pool, gen_pool_best_fit, NULL);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   97  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   98  	ret = gen_pool_add_virt(pool->pool, (unsigned long)pool->vbase,
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20   99  				pool->pbase, size, -1);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  100  	if (ret)
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  101  		goto err_destroy_genpool;
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  102  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  103  	ret = qcom_tzmem_init_pool(pool);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  104  	if (ret)
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  105  		goto err_destroy_genpool;
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  106  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  107  	return pool;
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  108  
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  109  err_destroy_genpool:
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  110  	gen_pool_destroy(pool->pool);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  111  err_dma_free:
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  112  	dma_free_coherent(qcom_tzmem_dev, size, pool->vbase, pool->pbase);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  113  err_kfree_pool:
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  114  	kfree(pool);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  115  	return ERR_PTR(ret);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  116  }
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  117  EXPORT_SYMBOL_GPL(qcom_tzmem_pool_new);
757fc9ecf8f244 Bartosz Golaszewski 2023-11-20  118
diff mbox series

Patch

diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
index b80269a28224..237da40de832 100644
--- a/drivers/firmware/qcom/Kconfig
+++ b/drivers/firmware/qcom/Kconfig
@@ -7,6 +7,7 @@ 
 menu "Qualcomm firmware drivers"
 
 config QCOM_SCM
+	select QCOM_TZMEM
 	tristate
 
 config QCOM_TZMEM
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 520de9b5633a..0d4c028be0c1 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -8,8 +8,10 @@ 
 #include <linux/completion.h>
 #include <linux/cpumask.h>
 #include <linux/dma-mapping.h>
+#include <linux/err.h>
 #include <linux/export.h>
 #include <linux/firmware/qcom/qcom_scm.h>
+#include <linux/firmware/qcom/qcom_tzmem.h>
 #include <linux/init.h>
 #include <linux/interconnect.h>
 #include <linux/interrupt.h>
@@ -20,9 +22,11 @@ 
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/reset-controller.h>
+#include <linux/sizes.h>
 #include <linux/types.h>
 
 #include "qcom_scm.h"
+#include "qcom_tzmem.h"
 
 static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
 module_param(download_mode, bool, 0);
@@ -41,6 +45,8 @@  struct qcom_scm {
 	int scm_vote_count;
 
 	u64 dload_mode_addr;
+
+	struct qcom_tzmem_pool *mempool;
 };
 
 struct qcom_scm_current_perm_info {
@@ -1887,6 +1893,16 @@  static int qcom_scm_probe(struct platform_device *pdev)
 	if (of_property_read_bool(pdev->dev.of_node, "qcom,sdi-enabled"))
 		qcom_scm_disable_sdi();
 
+	ret = qcom_tzmem_enable(__scm->dev);
+	if (ret)
+		return dev_err_probe(__scm->dev, ret,
+				     "Failed to enable the TrustZone memory allocator\n");
+
+	__scm->mempool = devm_qcom_tzmem_pool_new(__scm->dev, SZ_256K);
+	if (IS_ERR(__scm->mempool))
+		return dev_err_probe(__scm->dev, PTR_ERR(__scm->mempool),
+				     "Failed to create the SCM memory pool\n");
+
 	/*
 	 * Initialize the QSEECOM interface.
 	 *