Messages in this thread | | | Date | Wed, 24 May 2023 13:17:45 +0200 | Subject | Re: [PATCH 01/13] firmware: qcom: scm: Introduce pas_metadata context | From | Robert Marko <> |
| |
On 28. 01. 2022. 03:55, Bjorn Andersson wrote: > Starting with Qualcomm SM8450, some new security enhancements has been > done in the secure world, which results in the requirement to keep the > metadata segment accessible by the secure world from init_image() until > auth_and_reset(). > > Introduce a "PAS metadata context" object that can be passed to > init_image() for tracking the mapped memory and a related release > function for client drivers to release the mapping once either > auth_and_reset() has been invoked or in error handling paths on the way > there. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Hi Bjorn, I know a lot of time has passed since this patch, but this patch breaks qcom_scm_pas_auth_and_reset SCM call. I have had MDT loader and SCM changes from this series reverted for a long time but was unable to find the exact culprit nor how to fix it. I even ported the TZ log driver to see if anything is obvious there but nope, calling qcom_scm_pas_auth_and_reset after this patch is just failling which is then causing Q6 WCSS on IPQ8074 to fail probing so WLAN doesnt work.
Since I am out of ideas, I was hoping that you maybe have an idea what could be the issue, I can provide the TZ log with all of the SCM calls when it works and when not if required.
Regards, Robert
> --- > drivers/firmware/qcom_scm.c | 39 ++++++++++++++++++++++++++++++----- > drivers/soc/qcom/mdt_loader.c | 2 +- > include/linux/qcom_scm.h | 10 ++++++++- > 3 files changed, 44 insertions(+), 7 deletions(-) > > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index 7db8066b19fd..3218d13cbf83 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -435,10 +435,16 @@ static void qcom_scm_set_download_mode(bool enable) > * and optional blob of data used for authenticating the metadata > * and the rest of the firmware > * @size: size of the metadata > + * @ctx: optional metadata context > * > - * Returns 0 on success. > + * Return: 0 on success. > + * > + * Upon successful return, the PAS metadata context (@ctx) will be used to > + * track the metadata allocation, this needs to be released by invoking > + * qcom_scm_pas_metadata_release() by the caller. > */ > -int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size) > +int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size, > + struct qcom_scm_pas_metadata *ctx) > { > dma_addr_t mdata_phys; > void *mdata_buf; > @@ -467,7 +473,7 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size) > > ret = qcom_scm_clk_enable(); > if (ret) > - goto free_metadata; > + goto out; > > desc.args[1] = mdata_phys; > > @@ -475,13 +481,36 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size) > > qcom_scm_clk_disable(); > > -free_metadata: > - dma_free_coherent(__scm->dev, size, mdata_buf, mdata_phys); > +out: > + if (ret < 0 || !ctx) { > + dma_free_coherent(__scm->dev, size, mdata_buf, mdata_phys); > + } else if (ctx) { > + ctx->ptr = mdata_buf; > + ctx->phys = mdata_phys; > + ctx->size = size; > + } > > return ret ? : res.result[0]; > } > EXPORT_SYMBOL(qcom_scm_pas_init_image); > > +/** > + * qcom_scm_pas_metadata_release() - release metadata context > + * @ctx: metadata context > + */ > +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); > + > + ctx->ptr = NULL; > + ctx->phys = 0; > + ctx->size = 0; > +} > +EXPORT_SYMBOL(qcom_scm_pas_metadata_release); > + > /** > * qcom_scm_pas_mem_setup() - Prepare the memory related to a given peripheral > * for firmware loading > diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c > index 72fc2b539213..b00586db5391 100644 > --- a/drivers/soc/qcom/mdt_loader.c > +++ b/drivers/soc/qcom/mdt_loader.c > @@ -171,7 +171,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, > goto out; > } > > - ret = qcom_scm_pas_init_image(pas_id, metadata, metadata_len); > + ret = qcom_scm_pas_init_image(pas_id, metadata, metadata_len, NULL); > > kfree(metadata); > if (ret) { > diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h > index 81cad9e1e412..4d8371410b05 100644 > --- a/include/linux/qcom_scm.h > +++ b/include/linux/qcom_scm.h > @@ -68,8 +68,16 @@ extern int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus); > extern void qcom_scm_cpu_power_down(u32 flags); > extern int qcom_scm_set_remote_state(u32 state, u32 id); > > +struct qcom_scm_pas_metadata { > + void *ptr; > + dma_addr_t phys; > + ssize_t size; > +}; > + > extern int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, > - size_t size); > + size_t size, > + struct qcom_scm_pas_metadata *ctx); > +void qcom_scm_pas_metadata_release(struct qcom_scm_pas_metadata *ctx); > extern int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, > phys_addr_t size); > extern int qcom_scm_pas_auth_and_reset(u32 peripheral);
| |