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

["Dwivedi, Avaneesh Kumar (avani)" <akdwived@codeaurora.org>]

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@codeaurora.org>
>> ---
>>   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@vger.kernel.org
> 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.