| Message ID | 20241004212359.2263502-6-quic_mojha@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Peripheral Image Loader support for Qualcomm SoCs | expand |
Hi Mukesh, kernel test robot noticed the following build errors: [auto build test ERROR on remoteproc/rproc-next] [also build test ERROR on robh/for-next linus/master v6.12-rc1 next-20241004] [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/Mukesh-Ojha/dt-bindings-remoteproc-qcom-pas-common-Introduce-iommus-and-qcom-devmem-property/20241005-052733 base: https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git rproc-next patch link: https://lore.kernel.org/r/20241004212359.2263502-6-quic_mojha%40quicinc.com patch subject: [PATCH 5/6] remoteproc: qcom: Add support of SHM bridge to enable memory protection config: arc-randconfig-001-20241006 (https://download.01.org/0day-ci/archive/20241006/202410060641.ZedzhoKd-lkp@intel.com/config) compiler: arceb-elf-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241006/202410060641.ZedzhoKd-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/202410060641.ZedzhoKd-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/firmware/qcom/qcom_tzmem.c:50:12: error: static declaration of 'qcom_tzmem_init_area' follows non-static declaration 50 | static int qcom_tzmem_init_area(struct qcom_tzmem_area *area) | ^~~~~~~~~~~~~~~~~~~~ In file included from drivers/firmware/qcom/qcom_tzmem.c:12: include/linux/firmware/qcom/qcom_tzmem.h:59:5: note: previous declaration of 'qcom_tzmem_init_area' with type 'int(struct qcom_tzmem_area *)' 59 | int qcom_tzmem_init_area(struct qcom_tzmem_area *area); | ^~~~~~~~~~~~~~~~~~~~ >> drivers/firmware/qcom/qcom_tzmem.c:55:13: error: static declaration of 'qcom_tzmem_cleanup_area' follows non-static declaration 55 | static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area) | ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/firmware/qcom/qcom_tzmem.h:60:6: note: previous declaration of 'qcom_tzmem_cleanup_area' with type 'void(struct qcom_tzmem_area *)' 60 | void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area); | ^~~~~~~~~~~~~~~~~~~~~~~ vim +/qcom_tzmem_init_area +50 drivers/firmware/qcom/qcom_tzmem.c 84f5a7b67b61bf Bartosz Golaszewski 2024-05-27 49 84f5a7b67b61bf Bartosz Golaszewski 2024-05-27 @50 static int qcom_tzmem_init_area(struct qcom_tzmem_area *area) 84f5a7b67b61bf Bartosz Golaszewski 2024-05-27 51 { 84f5a7b67b61bf Bartosz Golaszewski 2024-05-27 52 return 0; 84f5a7b67b61bf Bartosz Golaszewski 2024-05-27 53 } 84f5a7b67b61bf Bartosz Golaszewski 2024-05-27 54 84f5a7b67b61bf Bartosz Golaszewski 2024-05-27 @55 static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area) 84f5a7b67b61bf Bartosz Golaszewski 2024-05-27 56 { 84f5a7b67b61bf Bartosz Golaszewski 2024-05-27 57
On 4.10.2024 11:23 PM, Mukesh Ojha wrote: > Qualcomm SoCs running with the Qualcomm EL2 hypervisor(QHEE) have been > utilizing the Peripheral Authentication Service (PAS) from its TrustZone > (TZ) firmware to securely authenticate and reset via sequence of SMC > calls like qcom_scm_pas_init_image(), qcom_scm_pas_mem_setup(), and > qcom_scm_pas_auth_and_reset(). > > Memory protection need to be enabled for both meta data memory region and > remoteproc carveout memory region. Will TZ refuse to start the remoteprocs if this is not the case? > For memory passed to Qualcomm TrustZone, the memory should be part of > SHM bridge memory. However, when QHEE is present, PAS SMC calls are > getting trapped in QHEE, which create or gets memory from SHM bridge for > both meta data memory and for remoteproc carve out regions before it get > passed to TZ. However, in absence of QHEE hypervisor, Linux need to > create SHM bridge for both meta data in qcom_scm_pas_init_image() and > for remoteproc memory before the call being made to > qcom_scm_pas_auth_and_reset(). > > For qcom_scm_pas_init_image() call, metadata content need to be copied > to the buffer allocated from SHM bridge before making the SMC call. > > For qcom_scm_pas_auth_and_reset(), remoteproc memory region need to be > protected and for that SHM bridge need to be created. Make > qcom_tzmem_init_area() and qcom_tzmem_cleanup_area() exported symbol so > that it could be used to create SHM bridge for remoteproc region. > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > --- > drivers/firmware/qcom/qcom_scm.c | 29 +++++++++++----- > drivers/firmware/qcom/qcom_tzmem.c | 14 +++----- > drivers/remoteproc/qcom_q6v5_pas.c | 44 ++++++++++++++++++++++++ > include/linux/firmware/qcom/qcom_scm.h | 1 + > include/linux/firmware/qcom/qcom_tzmem.h | 10 ++++++ > 5 files changed, 80 insertions(+), 18 deletions(-) This changes files in two separate subsystems. That implies this patch should be split in two. Or three. > > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c > index 10986cb11ec0..dafc07dc181f 100644 > --- a/drivers/firmware/qcom/qcom_scm.c > +++ b/drivers/firmware/qcom/qcom_scm.c > @@ -591,15 +591,19 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, > * data blob, so make sure it's physically contiguous, 4K aligned and > * non-cachable to avoid XPU violations. > * > - * For PIL calls the hypervisor creates SHM Bridges for the blob > - * buffers on behalf of Linux so we must not do it ourselves hence > - * not using the TZMem allocator here. > + * For PIL calls the hypervisor like Gunyah or older QHEE creates SHM > + * Bridges for the blob buffers on behalf of Linux so we must not do it > + * ourselves hence use TZMem allocator only when these hypervisors are > + * not present. This is a bit hard to read.. How about: PIL calls require SHMBridge is set up for shared memory regions. Qualcomm hypervisors (Gunyah, QHEE) already take care of this. Only create new bridges if they're absent. [...] Konrad
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c index 10986cb11ec0..dafc07dc181f 100644 --- a/drivers/firmware/qcom/qcom_scm.c +++ b/drivers/firmware/qcom/qcom_scm.c @@ -591,15 +591,19 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, * data blob, so make sure it's physically contiguous, 4K aligned and * non-cachable to avoid XPU violations. * - * For PIL calls the hypervisor creates SHM Bridges for the blob - * buffers on behalf of Linux so we must not do it ourselves hence - * not using the TZMem allocator here. + * For PIL calls the hypervisor like Gunyah or older QHEE creates SHM + * Bridges for the blob buffers on behalf of Linux so we must not do it + * ourselves hence use TZMem allocator only when these hypervisors are + * not present. * * 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); + if (ctx && ctx->shm_bridge_needed) + mdata_buf = qcom_tzmem_alloc(__scm->mempool, size, GFP_KERNEL); + else + mdata_buf = dma_alloc_coherent(__scm->dev, size, &mdata_phys, GFP_KERNEL); + if (!mdata_buf) return -ENOMEM; @@ -613,7 +617,10 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, if (ret) goto disable_clk; - desc.args[1] = mdata_phys; + if (ctx && ctx->shm_bridge_needed) + desc.args[1] = qcom_tzmem_to_phys(mdata_buf); + else + desc.args[1] = mdata_phys; ret = qcom_scm_call(__scm->dev, &desc, &res); qcom_scm_bw_disable(); @@ -625,8 +632,11 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, if (ret < 0 || !ctx) { dma_free_coherent(__scm->dev, size, mdata_buf, mdata_phys); } else if (ctx) { + if (ctx->shm_bridge_needed) + ctx->phys = qcom_tzmem_to_phys(mdata_buf); + else + ctx->phys = mdata_phys; ctx->ptr = mdata_buf; - ctx->phys = mdata_phys; ctx->size = size; } @@ -643,7 +653,10 @@ void qcom_scm_pas_metadata_release(struct qcom_scm_pas_metadata *ctx) if (!ctx->ptr) return; - dma_free_coherent(__scm->dev, ctx->size, ctx->ptr, ctx->phys); + if (ctx->shm_bridge_needed) + qcom_tzmem_free(ctx->ptr); + else + dma_free_coherent(__scm->dev, ctx->size, ctx->ptr, ctx->phys); ctx->ptr = NULL; ctx->phys = 0; diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c index 92b365178235..66aba2fc979d 100644 --- a/drivers/firmware/qcom/qcom_tzmem.c +++ b/drivers/firmware/qcom/qcom_tzmem.c @@ -22,14 +22,6 @@ #include "qcom_tzmem.h" -struct qcom_tzmem_area { - struct list_head list; - void *vaddr; - dma_addr_t paddr; - size_t size; - void *priv; -}; - struct qcom_tzmem_pool { struct gen_pool *genpool; struct list_head areas; @@ -107,7 +99,7 @@ static int qcom_tzmem_init(void) return 0; } -static int qcom_tzmem_init_area(struct qcom_tzmem_area *area) +int qcom_tzmem_init_area(struct qcom_tzmem_area *area) { u64 pfn_and_ns_perm, ipfn_and_s_perm, size_and_flags; int ret; @@ -133,8 +125,9 @@ static int qcom_tzmem_init_area(struct qcom_tzmem_area *area) return 0; } +EXPORT_SYMBOL_GPL(qcom_tzmem_init_area); -static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area) +void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area) { u64 *handle = area->priv; @@ -144,6 +137,7 @@ static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area) qcom_scm_shm_bridge_delete(qcom_tzmem_dev, *handle); kfree(handle); } +EXPORT_SYMBOL_GPL(qcom_tzmem_cleanup_area); #endif /* CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE */ diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index bdb071ab5938..ac339145e072 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -20,6 +20,7 @@ #include <linux/pm_domain.h> #include <linux/pm_runtime.h> #include <linux/firmware/qcom/qcom_scm.h> +#include <linux/firmware/qcom/qcom_tzmem.h> #include <linux/regulator/consumer.h> #include <linux/remoteproc.h> #include <linux/soc/qcom/mdt_loader.h> @@ -120,6 +121,7 @@ struct qcom_adsp { struct qcom_scm_pas_metadata dtb_pas_metadata; struct qcom_devmem_table *devmem; + struct qcom_tzmem_area *tzmem; }; static void adsp_segment_dump(struct rproc *rproc, struct rproc_dump_segment *segment, @@ -262,6 +264,43 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw) return ret; } +static int adsp_create_shmbridge(struct qcom_adsp *adsp) +{ + struct qcom_tzmem_area *rproc_tzmem; + struct rproc *rproc = adsp->rproc; + int ret; + + if (!rproc->has_iommu) + return 0; + + rproc_tzmem = devm_kzalloc(adsp->dev, sizeof(*rproc_tzmem), GFP_KERNEL); + if (!rproc_tzmem) + return -ENOMEM; + + rproc_tzmem->size = PAGE_ALIGN(adsp->mem_size); + rproc_tzmem->paddr = adsp->mem_phys; + ret = qcom_tzmem_init_area(rproc_tzmem); + if (ret) { + dev_err(adsp->dev, + "failed to create shmbridge for carveout: %d\n", ret); + return ret; + } + + adsp->tzmem = rproc_tzmem; + + return ret; +} + +static void adsp_delete_shmbridge(struct qcom_adsp *adsp) +{ + struct rproc *rproc = adsp->rproc; + + if (!rproc->has_iommu) + return; + + qcom_tzmem_cleanup_area(adsp->tzmem); +} + static int adsp_start(struct rproc *rproc) { struct qcom_adsp *adsp = rproc->priv; @@ -317,6 +356,10 @@ static int adsp_start(struct rproc *rproc) qcom_pil_info_store(adsp->info_name, adsp->mem_phys, adsp->mem_size); + ret = adsp_create_shmbridge(adsp); + if (ret) + goto release_pas_metadata; + ret = qcom_scm_pas_auth_and_reset(adsp->pas_id); if (ret) { dev_err(adsp->dev, @@ -324,6 +367,7 @@ static int adsp_start(struct rproc *rproc) goto release_pas_metadata; } + adsp_delete_shmbridge(adsp); ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000)); if (ret == -ETIMEDOUT) { dev_err(adsp->dev, "start timed out\n"); diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h index 9f14976399ab..25243cd889bb 100644 --- a/include/linux/firmware/qcom/qcom_scm.h +++ b/include/linux/firmware/qcom/qcom_scm.h @@ -70,6 +70,7 @@ struct qcom_scm_pas_metadata { void *ptr; dma_addr_t phys; ssize_t size; + bool shm_bridge_needed; }; int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, diff --git a/include/linux/firmware/qcom/qcom_tzmem.h b/include/linux/firmware/qcom/qcom_tzmem.h index b83b63a0c049..e0a57cc8f74b 100644 --- a/include/linux/firmware/qcom/qcom_tzmem.h +++ b/include/linux/firmware/qcom/qcom_tzmem.h @@ -39,6 +39,14 @@ struct qcom_tzmem_pool_config { size_t max_size; }; +struct qcom_tzmem_area { + struct list_head list; + void *vaddr; + dma_addr_t paddr; + size_t size; + void *priv; +}; + struct qcom_tzmem_pool * qcom_tzmem_pool_new(const struct qcom_tzmem_pool_config *config); void qcom_tzmem_pool_free(struct qcom_tzmem_pool *pool); @@ -48,6 +56,8 @@ devm_qcom_tzmem_pool_new(struct device *dev, void *qcom_tzmem_alloc(struct qcom_tzmem_pool *pool, size_t size, gfp_t gfp); void qcom_tzmem_free(void *ptr); +int qcom_tzmem_init_area(struct qcom_tzmem_area *area); +void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area); DEFINE_FREE(qcom_tzmem, void *, if (_T) qcom_tzmem_free(_T))
Qualcomm SoCs running with the Qualcomm EL2 hypervisor(QHEE) have been utilizing the Peripheral Authentication Service (PAS) from its TrustZone (TZ) firmware to securely authenticate and reset via sequence of SMC calls like qcom_scm_pas_init_image(), qcom_scm_pas_mem_setup(), and qcom_scm_pas_auth_and_reset(). Memory protection need to be enabled for both meta data memory region and remoteproc carveout memory region. For memory passed to Qualcomm TrustZone, the memory should be part of SHM bridge memory. However, when QHEE is present, PAS SMC calls are getting trapped in QHEE, which create or gets memory from SHM bridge for both meta data memory and for remoteproc carve out regions before it get passed to TZ. However, in absence of QHEE hypervisor, Linux need to create SHM bridge for both meta data in qcom_scm_pas_init_image() and for remoteproc memory before the call being made to qcom_scm_pas_auth_and_reset(). For qcom_scm_pas_init_image() call, metadata content need to be copied to the buffer allocated from SHM bridge before making the SMC call. For qcom_scm_pas_auth_and_reset(), remoteproc memory region need to be protected and for that SHM bridge need to be created. Make qcom_tzmem_init_area() and qcom_tzmem_cleanup_area() exported symbol so that it could be used to create SHM bridge for remoteproc region. Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> --- drivers/firmware/qcom/qcom_scm.c | 29 +++++++++++----- drivers/firmware/qcom/qcom_tzmem.c | 14 +++----- drivers/remoteproc/qcom_q6v5_pas.c | 44 ++++++++++++++++++++++++ include/linux/firmware/qcom/qcom_scm.h | 1 + include/linux/firmware/qcom/qcom_tzmem.h | 10 ++++++ 5 files changed, 80 insertions(+), 18 deletions(-)