Re: [PATCH v7 3/4] remoteproc: qcom: Make secure world call for mem ownership switch

[Dwivedi, Avaneesh Kumar (avani)]

On 10/12/2017 11:48 PM, Bjorn Andersson wrote:
> On Fri 21 Jul 03:49 PDT 2017, Avaneesh Kumar Dwivedi wrote:
>
> > MSS proc on msm8996 can not access fw loaded region without stage
> > second translation of memory pages where mpss image are loaded.
> > This patch in order to enable mss boot on msm8996 invoke scm call
> > to switch or share ownership between apps and modem.
> >
> > Signed-off-by: Avaneesh Kumar Dwivedi <akdwived@xxxxxxxxxxxxxx>
> > ---
> >   drivers/remoteproc/qcom_q6v5_pil.c | 90 ++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 87 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
> [..]
> > +static int q6v5_xfer_mem_ownership(struct q6v5 *qproc, int *current_perm,
> > +				   bool remote_owner, phys_addr_t addr,
> > +				   size_t size)
> > +{
> > +	struct qcom_scm_vmperm next;
> > +	int ret;
> > +
> > +	if (!qproc->need_mem_protection)
> > +		return 0;
> > +	if (remote_owner && *current_perm == BIT(QCOM_SCM_VMID_MSS_MSA))
> > +		return 0;
> > +	if (!remote_owner && *current_perm == BIT(QCOM_SCM_VMID_HLOS))
> > +		return 0;
> > +
> > +	next.vmid = remote_owner ? QCOM_SCM_VMID_MSS_MSA : QCOM_SCM_VMID_HLOS;
> > +	next.perm = remote_owner ? QCOM_SCM_PERM_RW : QCOM_SCM_PERM_RWX;
> > +
> > +	ret = qcom_scm_assign_mem(addr, ALIGN(size, SZ_4K),
> > +				  *current_perm, &next, 1);
> > +	if (ret < 0) {
> > +		pr_err("Failed to assign memory access in range %p to %p to %s ret = %d\n",
> > +		       (void *)addr, (void *)(addr + size),
> > +		       remote_owner ? "mss" : "hlos", ret);
> qcom_scm_assign_mem() also prints an error when this happens, there's no
> need for that. I'm happy with you dropping the print from
> qcom_scm_assign_mem() and keeping this one.
>
> > +		return ret;
> > +	}
> > +
> > +	*current_perm = ret;
> > +	return 0;
> > +}
> > +
> >   static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
> >   {
> >   	struct q6v5 *qproc = rproc->priv;
> > @@ -450,6 +484,8 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
> >   {
> >   	unsigned long dma_attrs = DMA_ATTR_FORCE_CONTIGUOUS;
> >   	dma_addr_t phys;
> > +	int mdata_perm;
> > +	int xferop_ret;
> >   	void *ptr;
> >   	int ret;
> >   
> > @@ -461,6 +497,12 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
> >   
> >   	memcpy(ptr, fw->data, fw->size);
> >   
> > +	/* Hypervisor mapping to access metadata by modem */
> > +	mdata_perm = BIT(QCOM_SCM_VMID_HLOS);
> > +	ret = q6v5_xfer_mem_ownership(qproc, &mdata_perm,
> > +				      1, phys, fw->size);
> Please use "true" instead of "1".
OK Sure, will do it.
> > +	if (ret)
> > +		return -EAGAIN;
> Please keep a empty line here, to keep the "paragraphs" or "chunks" of
> code split up.
OK.
> >   	writel(phys, qproc->rmb_base + RMB_PMI_META_DATA_REG);
> >   	writel(RMB_CMD_META_DATA_READY, qproc->rmb_base + RMB_MBA_COMMAND_REG);
> >   
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.