Add memory ownership switch support and enable mss rproc on msm8996
v6 cover Add memory ownership switch support and enable mss rproc on msm8996
This patch set does following
1- Adds new scm call which helps in stage two translation of a memory region
so that memory ownership sharing and switching can be achieved on armv8 and later.
2- Enable mss remoteproc on msm8996
Major changes since last patch:
- Reorganization of q6v5_xfer_mem_ownership function.
- Other indentation and small errors.
Avaneesh Kumar Dwivedi (4):
firmware: scm: Add new SCM call API for switching memory ownership
remoteproc: qcom: refactor mss fw image loading sequence
remoteproc: qcom: Make secure world call for mem ownership switch
remoteproc: qcom: Add support for mss remoteproc on msm8996
.../devicetree/bindings/remoteproc/qcom,q6v5.txt | 1 + drivers/firmware/qcom_scm-32.c | 6 + drivers/firmware/qcom_scm-64.c | 27 +++ drivers/firmware/qcom_scm.c | 92 +++++++ drivers/firmware/qcom_scm.h | 5 + drivers/remoteproc/qcom_q6v5_pil.c | 268 ++++++++++++++++++--- include/linux/qcom_scm.h | 16 ++ 7 files changed, 377 insertions(+), 38 deletions(-)
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH v6 1/4 firmware: scm: Add new SCM call API for switching memory ownership
Two different processors on a SOC need to switch memory ownership during load/unload. To enable this, second level memory map table need to be updated, which is done by secure layer.
This patch adds the interface for making secure monitor call for memory ownership switching request.
--- drivers/firmware/qcom_scm-32.c | 6 +++ drivers/firmware/qcom_scm-64.c | 27 +++++++++++++ drivers/firmware/qcom_scm.c | 92 ++++++++++++++++++++++++++++++++++++++++++ drivers/firmware/qcom_scm.h | 5 +++ include/linux/qcom_scm.h | 16 ++++++++ 5 files changed, 146 insertions(+)
drivers/firmware/qcom_scm-32.c +6 -0
@@ -596,3 +596,9 @@ int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size, {return -ENODEV ; } +int __qcom_scm_assign_mem (struct device *dev,phys_addr_t mem_region, +size_t mem_sz,phys_addr_t src,size_t src_sz, +phys_addr_t dest,size_t dest_sz) +{ +return -ENODEV ; +}
drivers/firmware/qcom_scm-64.c +27 -0
@@ -439,3 +439,30 @@ int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size,return ret; } + +int __qcom_scm_assign_mem (struct device *dev,phys_addr_t mem_region, +size_t mem_sz,phys_addr_t src,size_t src_sz, +phys_addr_t dest,size_t dest_sz) +{ +int ret; +struct qcom_scm_desc desc = {0 }; +struct arm_smccc_res res; + + desc.args[0 ] = mem_region; + desc.args[1 ] = mem_sz; + desc.args[2 ] = src; + desc.args[3 ] = src_sz; + desc.args[4 ] = dest; + desc.args[5 ] = dest_sz; + desc.args[6 ] =0 ; + + desc.arginfo =QCOM_SCM_ARGS (7 ,QCOM_SCM_RO ,QCOM_SCM_VAL , +QCOM_SCM_RO ,QCOM_SCM_VAL ,QCOM_SCM_RO , +QCOM_SCM_VAL ,QCOM_SCM_VAL ); + + ret =qcom_scm_call (dev,QCOM_SCM_SVC_MP , +QCOM_MEM_PROT_ASSIGN_ID , + &desc, &res); + +return ret ? : res.a1; +}
drivers/firmware/qcom_scm.c +92 -0
104 lines not on this page · raw message
drivers/firmware/qcom_scm.h +5 -0
@@ -95,5 +95,10 @@ extern int __qcom_scm_iommu_secure_ptbl_size(struct device *dev, u32 spare, size_t *size);extern int __qcom_scm_iommu_secure_ptbl_init (struct device *dev,u64 addr,u32 size,u32 spare); +#define QCOM_MEM_PROT_ASSIGN_ID 0x16 +extern int __qcom_scm_assign_mem (struct device *dev, +phys_addr_t mem_region,size_t mem_sz, +phys_addr_t src,size_t src_sz, +phys_addr_t dest,size_t dest_sz);#endif
include/linux/qcom_scm.h +16 -0
@@ -23,6 +23,19 @@ struct qcom_scm_hdcp_req {u32 val; }; +struct qcom_scm_vmperm { +int vmid; +int perm; +}; + +#define QCOM_SCM_VMID_HLOS 0x3 +#define QCOM_SCM_VMID_MSS_MSA 0xF +#define QCOM_SCM_PERM_READ 0x4 +#define QCOM_SCM_PERM_WRITE 0x2 +#define QCOM_SCM_PERM_EXEC 0x1 +#define QCOM_SCM_PERM_RW (QCOM_SCM_PERM_READ | QCOM_SCM_PERM_WRITE) +#define QCOM_SCM_PERM_RWX (QCOM_SCM_PERM_RW | QCOM_SCM_PERM_EXEC) +#if IS_ENABLED (CONFIG_QCOM_SCM )extern int qcom_scm_set_cold_boot_addr (void *entry,const cpumask_t *cpus);extern int qcom_scm_set_warm_boot_addr (void *entry,const cpumask_t *cpus);
@@ -37,6 +50,9 @@ 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);extern int qcom_scm_pas_shutdown (u32 peripheral); +extern int qcom_scm_assign_mem (phys_addr_t mem_addr,size_t mem_sz, +int src,struct qcom_scm_vmperm *newvm, +int dest_cnt);extern void qcom_scm_cpu_power_down (u32 flags);extern u32 qcom_scm_get_version (void );extern int qcom_scm_set_remote_state (u32 state,u32 id);
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v6 1/4] firmware: scm: Add new SCM call API for switching memory ownership
Quoted 14 lines · Avaneesh Kumar Dwivedi
drivers/firmware/qcom_scm-64.c +6
-0@@ -292,6 +304,86 @@ int qcom_scm_pas_shutdown(u32 peripheral) }EXPORT_SYMBOL (qcom_scm_pas_shutdown); +/** +* qcom_scm_assign_mem() - Make a secure call to reassign memory ownership +* +* @mem_addr: mem region whose ownership need to be reassigned +* @mem_sz: size of the region. +* @srcvm: vmid for current set of owners, each set bit in
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 322
Quoted 10 lines
drivers/firmware/qcom_scm-64.c +10
-0+* flag indicate a unique owner +* @newvm: array having new owners and corrsponding permission +* flags +* @dest_cnt: number of owners in next set. +* Return next set of owners on success. +*/ +int qcom_scm_assign_mem (phys_addr_t mem_addr,size_t mem_sz,int srcvm, +struct qcom_scm_vmperm *newvm,int dest_cnt) +{ +unsigned long dma_attrs =DMA_ATTR_FORCE_CONTIGUOUS ;
Why do we need this? Just curious if we can drop this.
Quoted 18 lines
drivers/firmware/qcom_scm-64.c +18
-0+struct qcom_scm_current_perm_info *destvm; +struct qcom_scm_mem_map_info *mem; +phys_addr_t memory_phys; +phys_addr_t dest_phys; +phys_addr_t src_phys; +size_t mem_all_sz; +size_t memory_sz; +size_t dest_sz; +size_t src_sz; +int next_vm; +__le32 *src; +void *ptr; +int ret; +int len; +int i; + + src_sz =hweight_long (srcvm) *sizeof (*src); + memory_sz =sizeof (*mem);
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 350
Quoted 10 lines
drivers/firmware/qcom_scm-64.c +10
-0+ dest_sz = dest_cnt*sizeof (*destvm); + mem_all_sz = src_sz + memory_sz + dest_sz; + + ptr =dma_alloc_attrs (__scm->dev,ALIGN (mem_all_sz,SZ_64 ), + &src_phys,GFP_KERNEL , dma_attrs); +if (!ptr) +return -ENOMEM ; + +/* Fill source vmid detail */ + src = (__le32 *)ptr;
Cast is necessary?
Quoted 22 lines
drivers/firmware/qcom_scm-64.c +22
-0+ len =hweight_long (srcvm); +for (i =0 ; i < len; i++) { + src[i] =cpu_to_le32 (ffs (srcvm) -1 ); + srcvm ^=1 << (ffs (srcvm) -1 ); + } + +/* Fill details of mem buff to map */ + mem = ptr +ALIGN (src_sz,SZ_64 ); + memory_phys = src_phys +ALIGN (src_sz,SZ_64 ); + mem[0 ].mem_addr =cpu_to_le64 (mem_addr); + mem[0 ].mem_size =cpu_to_le64 (mem_sz); + + next_vm =0 ; +/* Fill details of next vmid detail */ + destvm = ptr +ALIGN (memory_sz,SZ_64 ) +ALIGN (src_sz,SZ_64 ); + dest_phys = memory_phys +ALIGN (memory_sz,SZ_64 ); +for (i =0 ; i < dest_cnt; i++) { + destvm[i].vmid =cpu_to_le32 (newvm[i].vmid); + destvm[i].perm =cpu_to_le32 (newvm[i].perm); + destvm[i].ctx =0 ; + destvm[i].ctx_size =0 ; + next_vm |=BIT (newvm[i].vmid);
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 382
Quoted 10 lines
drivers/firmware/qcom_scm-64.c +10
-0+ } + + ret =__qcom_scm_assign_mem (__scm->dev, memory_phys, + memory_sz, src_phys, src_sz, dest_phys, dest_sz); +dma_free_attrs (__scm->dev,ALIGN (mem_all_sz,SZ_64 ), + ptr, src_phys, dma_attrs); +if (ret ==0 ) +return next_vm; +else if (ret >0 ) +return -ret;
This still confuses me. Do we really just pass whatever the firmware tells us the error code is up to the caller? Shouldn't we be remapping the scm errors we receive to normal linux errnos?
Quoted 7 lines
drivers/firmware/qcom_scm-64.c +4
-0+return ret; +} +EXPORT_SYMBOL (qcom_scm_assign_mem); +static int qcom_scm_pas_reset_assert (struct reset_controller_dev *rcdev,unsigned long idx) {
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v6 1/4] firmware: scm: Add new SCM call API for switching memory ownership
Quoted 2 lines · Stephen Boyd
Quoted 1 line · Avaneesh Kumar Dwivedi
drivers/firmware/qcom_scm-64.c +0
-0
[..]
Quoted 7 lines
drivers/firmware/qcom_scm-64.c
Quoted 4 lines
drivers/firmware/qcom_scm-64.c +4
-0+int qcom_scm_assign_mem (phys_addr_t mem_addr,size_t mem_sz,int srcvm, +struct qcom_scm_vmperm *newvm,int dest_cnt) +{ +unsigned long dma_attrs =DMA_ATTR_FORCE_CONTIGUOUS ;Why do we need this? Just curious if we can drop this.
As far as I can see this attribute has no affect on the allocation, unless you have attached an iommu to the device. So in the current setup it should be perfectly fine to get the memory from dma_alloc_coherent()
Quoted 59 lines
Quoted 18 lines
drivers/firmware/qcom_scm-64.c +18
-0+struct qcom_scm_current_perm_info *destvm; +struct qcom_scm_mem_map_info *mem; +phys_addr_t memory_phys; +phys_addr_t dest_phys; +phys_addr_t src_phys; +size_t mem_all_sz; +size_t memory_sz; +size_t dest_sz; +size_t src_sz; +int next_vm; +__le32 *src; +void *ptr; +int ret; +int len; +int i; + + src_sz =hweight_long (srcvm) *sizeof (*src); + memory_sz =sizeof (*mem);drivers/firmware/qcom_scm-64.c
Quoted 10 lines
drivers/firmware/qcom_scm-64.c +10
-0+ dest_sz = dest_cnt*sizeof (*destvm); + mem_all_sz = src_sz + memory_sz + dest_sz; + + ptr =dma_alloc_attrs (__scm->dev,ALIGN (mem_all_sz,SZ_64 ), + &src_phys,GFP_KERNEL , dma_attrs); +if (!ptr) +return -ENOMEM ; + +/* Fill source vmid detail */ + src = (__le32 *)ptr;Cast is necessary?
Quoted 28 lines
drivers/firmware/qcom_scm-64.c +28
-0+ len =hweight_long (srcvm); +for (i =0 ; i < len; i++) { + src[i] =cpu_to_le32 (ffs (srcvm) -1 ); + srcvm ^=1 << (ffs (srcvm) -1 ); + } + +/* Fill details of mem buff to map */ + mem = ptr +ALIGN (src_sz,SZ_64 ); + memory_phys = src_phys +ALIGN (src_sz,SZ_64 ); + mem[0 ].mem_addr =cpu_to_le64 (mem_addr); + mem[0 ].mem_size =cpu_to_le64 (mem_sz); + + next_vm =0 ; +/* Fill details of next vmid detail */ + destvm = ptr +ALIGN (memory_sz,SZ_64 ) +ALIGN (src_sz,SZ_64 ); + dest_phys = memory_phys +ALIGN (memory_sz,SZ_64 ); +for (i =0 ; i < dest_cnt; i++) { + destvm[i].vmid =cpu_to_le32 (newvm[i].vmid); + destvm[i].perm =cpu_to_le32 (newvm[i].perm); + destvm[i].ctx =0 ; + destvm[i].ctx_size =0 ; + next_vm |=BIT (newvm[i].vmid); + } + + ret =__qcom_scm_assign_mem (__scm->dev, memory_phys, + memory_sz, src_phys, src_sz, dest_phys, dest_sz); +dma_free_attrs (__scm->dev,ALIGN (mem_all_sz,SZ_64 ), + ptr, src_phys, dma_attrs);
Quoted 9 lines
drivers/firmware/qcom_scm-64.c
Quoted 4 lines
drivers/firmware/qcom_scm-64.c +4
-0+if (ret ==0 ) +return next_vm; +else if (ret >0 ) +return -ret;This still confuses me. Do we really just pass whatever the firmware tells us the error code is up to the caller? Shouldn't we be remapping the scm errors we receive to normal linux errnos?
I agree, the one case I've seen this not being 0 on "success" it was 15 and I consider it wrong to return ENOTBLK when this happens.
It would be better to just return -EINVAL in this case.
Quoted 2 lines
drivers/firmware/qcom_scm-64.c
Quoted 2 lines
drivers/firmware/qcom_scm-64.c +2
-0+return ret; +}
Regards,
Bjorn
Re: [PATCH v6 1/4] firmware: scm: Add new SCM call API for switching memory ownership
In reply to Stephen BoydQuoted 18 lines · Stephen Boyd
Quoted 17 lines · Avaneesh Kumar Dwivedi
drivers/firmware/qcom_scm-64.c +9
-0@@ -292,6 +304,86 @@ int qcom_scm_pas_shutdown(u32 peripheral) }EXPORT_SYMBOL (qcom_scm_pas_shutdown); +/** +* qcom_scm_assign_mem() - Make a secure call to reassign memory ownership +* +* @mem_addr: mem region whose ownership need to be reassigned +* @mem_sz: size of the region. +* @srcvm: vmid for current set of owners, each set bit in +* flag indicate a unique owner +* @newvm: array having new owners and corrsponding permission +* flags
Quoted 8 lines
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 322
Quoted 7 lines
drivers/firmware/qcom_scm-64.c +7
-0+* @dest_cnt: number of owners in next set. +* Return next set of owners on success. +*/ +int qcom_scm_assign_mem (phys_addr_t mem_addr,size_t mem_sz,int srcvm, +struct qcom_scm_vmperm *newvm,int dest_cnt) +{ +unsigned long dma_attrs =DMA_ATTR_FORCE_CONTIGUOUS ;Why do we need this? Just curious if we can drop this.
The force contiguous flag is required with dma_alloc_attrs() api to allocate memory from physically contiguous zone.
I am not sure, are you saying that api will work without the attribute or you mean i shall use some api which does not take explicit attribute?
Quoted 22 lines
Quoted 22 lines
drivers/firmware/qcom_scm-64.c +21
-0+struct qcom_scm_current_perm_info *destvm; +struct qcom_scm_mem_map_info *mem; +phys_addr_t memory_phys; +phys_addr_t dest_phys; +phys_addr_t src_phys; +size_t mem_all_sz; +size_t memory_sz; +size_t dest_sz; +size_t src_sz; +int next_vm; +__le32 *src; +void *ptr; +int ret; +int len; +int i; + + src_sz =hweight_long (srcvm) *sizeof (*src); + memory_sz =sizeof (*mem); + dest_sz = dest_cnt*sizeof (*destvm); + mem_all_sz = src_sz + memory_sz + dest_sz; +
Quoted 8 lines
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 350
Quoted 7 lines
drivers/firmware/qcom_scm-64.c +7
-0+ ptr =dma_alloc_attrs (__scm->dev,ALIGN (mem_all_sz,SZ_64 ), + &src_phys,GFP_KERNEL , dma_attrs); +if (!ptr) +return -ENOMEM ; + +/* Fill source vmid detail */ + src = (__le32 *)ptr;Cast is necessary?
i removed many type casting but few still lingering, will check and remove whatever unnecessary.
Quoted 28 lines
Quoted 28 lines
drivers/firmware/qcom_scm-64.c +27
-0+ len =hweight_long (srcvm); +for (i =0 ; i < len; i++) { + src[i] =cpu_to_le32 (ffs (srcvm) -1 ); + srcvm ^=1 << (ffs (srcvm) -1 ); + } + +/* Fill details of mem buff to map */ + mem = ptr +ALIGN (src_sz,SZ_64 ); + memory_phys = src_phys +ALIGN (src_sz,SZ_64 ); + mem[0 ].mem_addr =cpu_to_le64 (mem_addr); + mem[0 ].mem_size =cpu_to_le64 (mem_sz); + + next_vm =0 ; +/* Fill details of next vmid detail */ + destvm = ptr +ALIGN (memory_sz,SZ_64 ) +ALIGN (src_sz,SZ_64 ); + dest_phys = memory_phys +ALIGN (memory_sz,SZ_64 ); +for (i =0 ; i < dest_cnt; i++) { + destvm[i].vmid =cpu_to_le32 (newvm[i].vmid); + destvm[i].perm =cpu_to_le32 (newvm[i].perm); + destvm[i].ctx =0 ; + destvm[i].ctx_size =0 ; + next_vm |=BIT (newvm[i].vmid); + } + + ret =__qcom_scm_assign_mem (__scm->dev, memory_phys, + memory_sz, src_phys, src_sz, dest_phys, dest_sz); + dma_free_attrs(__scm->dev, ALIGN(mem_all_sz, SZ_64),
Quoted 8 lines
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 382
Quoted 5 lines
drivers/firmware/qcom_scm-64.c +5
-0+ ptr, src_phys, dma_attrs); +if (ret ==0 ) +return next_vm; +else if (ret >0 ) +return -ret;This still confuses me. Do we really just pass whatever the firmware tells us the error code is up to the caller? Shouldn't we be remapping the scm errors we receive to normal linux errnos?
because i do not know in advance what exactly will be the return error code, moreover there are number of error codes which are returned in case of failure so if i have to return linux error code, i can not do one to one mapping of error code and will have to return single error code for all failure. let me know your comments further on this.+ return ret;
Quoted 6 lines
Quoted 6 lines
drivers/firmware/qcom_scm-64.c +3
-0+} +EXPORT_SYMBOL (qcom_scm_assign_mem); +static int qcom_scm_pas_reset_assert (struct reset_controller_dev *rcdev,unsigned long idx) {
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v6 1/4] firmware: scm: Add new SCM call API for switching memory ownership
Quoted 100 lines · Avaneesh Kumar (avani)
Quoted 17 lines · Stephen Boyd
Quoted 16 lines · Avaneesh Kumar Dwivedi
drivers/firmware/qcom_scm-64.c +9
-0@@ -292,6 +304,86 @@ int qcom_scm_pas_shutdown(u32 peripheral) }EXPORT_SYMBOL (qcom_scm_pas_shutdown); +/** +* qcom_scm_assign_mem() - Make a secure call to reassign memory ownership +* +* @mem_addr: mem region whose ownership need to be reassigned +* @mem_sz: size of the region. +* @srcvm: vmid for current set of owners, each set bit in +* flag indicate a unique owner +* @newvm: array having new owners and corrsponding permission +* flags Quoted 8 lines
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 321
Quoted 7 lines
drivers/firmware/qcom_scm-64.c +7
-0+* @dest_cnt: number of owners in next set. +* Return next set of owners on success. +*/ +int qcom_scm_assign_mem (phys_addr_t mem_addr,size_t mem_sz,int srcvm, +struct qcom_scm_vmperm *newvm,int dest_cnt) +{ +unsigned long dma_attrs =DMA_ATTR_FORCE_CONTIGUOUS ;Why do we need this? Just curious if we can drop this.
The force contiguous flag is required with
dma_alloc_attrs()api to allocate memory from physically contiguous zone.
I am not sure, are you saying that api will work without the attribute or you mean i shall use some api which does not take explicit attribute?Quoted 22 lines
Quoted 22 lines
drivers/firmware/qcom_scm-64.c +21
-0+struct qcom_scm_current_perm_info *destvm; +struct qcom_scm_mem_map_info *mem; +phys_addr_t memory_phys; +phys_addr_t dest_phys; +phys_addr_t src_phys; +size_t mem_all_sz; +size_t memory_sz; +size_t dest_sz; +size_t src_sz; +int next_vm; +__le32 *src; +void *ptr; +int ret; +int len; +int i; + + src_sz =hweight_long (srcvm) *sizeof (*src); + memory_sz =sizeof (*mem); + dest_sz = dest_cnt*sizeof (*destvm); + mem_all_sz = src_sz + memory_sz + dest_sz; +Quoted 8 lines
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 349
Quoted 7 lines
drivers/firmware/qcom_scm-64.c +7
-0+ ptr =dma_alloc_attrs (__scm->dev,ALIGN (mem_all_sz,SZ_64 ), + &src_phys,GFP_KERNEL , dma_attrs); +if (!ptr) +return -ENOMEM ; + +/* Fill source vmid detail */ + src = (__le32 *)ptr;Cast is necessary?
i removed many type casting but few still lingering, will check and remove whatever unnecessary.
Quoted 28 lines
Quoted 28 lines
drivers/firmware/qcom_scm-64.c +27
-0+ len =hweight_long (srcvm); +for (i =0 ; i < len; i++) { + src[i] =cpu_to_le32 (ffs (srcvm) -1 ); + srcvm ^=1 << (ffs (srcvm) -1 ); + } + +/* Fill details of mem buff to map */ + mem = ptr +ALIGN (src_sz,SZ_64 ); + memory_phys = src_phys +ALIGN (src_sz,SZ_64 ); + mem[0 ].mem_addr =cpu_to_le64 (mem_addr); + mem[0 ].mem_size =cpu_to_le64 (mem_sz); + + next_vm =0 ; +/* Fill details of next vmid detail */ + destvm = ptr +ALIGN (memory_sz,SZ_64 ) +ALIGN (src_sz,SZ_64 ); + dest_phys = memory_phys +ALIGN (memory_sz,SZ_64 ); +for (i =0 ; i < dest_cnt; i++) { + destvm[i].vmid =cpu_to_le32 (newvm[i].vmid); + destvm[i].perm =cpu_to_le32 (newvm[i].perm); + destvm[i].ctx =0 ; + destvm[i].ctx_size =0 ; + next_vm |=BIT (newvm[i].vmid); + } + + ret =__qcom_scm_assign_mem (__scm->dev, memory_phys, + memory_sz, src_phys, src_sz, dest_phys, dest_sz); + dma_free_attrs(__scm->dev, ALIGN(mem_all_sz, SZ_64),Quoted 8 lines
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 381
Quoted 5 lines
drivers/firmware/qcom_scm-64.c +5
-0+ ptr, src_phys, dma_attrs); +if (ret ==0 ) +return next_vm; +else if (ret >0 ) +return -ret;This still confuses me. Do we really just pass whatever the firmware tells us the error code is up to the caller? Shouldn't we be remapping the scm errors we receive to normal linux errnos?
Quoted 6 lines
because i do not know in advance what exactly will be the return error code, moreover there are number of error codes which are returned in case of failure
so if i have to return linux error code, i can not do one to one mapping of error code and will have to return single error code for all failure. let me know your comments further on this.+ return ret;
Returning a single error code for all these cases (e.g. -EINVAL) is fine.
You could amend this by translating some of the results to other codes, if that makes sense. But I'm not aware of the kind of errors this interface can return, so it's hard to advice on this.
Regards,
Bjorn
Re: [PATCH v6 1/4] firmware: scm: Add new SCM call API for switching memory ownership
In reply to Dwivedi, Avaneesh Kumar (avani)Quoted 28 lines · Avaneesh Kumar (avani)
Quoted 17 lines · Stephen Boyd
Quoted 16 lines · Avaneesh Kumar Dwivedi
drivers/firmware/qcom_scm-64.c +9
-0@@ -292,6 +304,86 @@ int qcom_scm_pas_shutdown(u32 peripheral) }EXPORT_SYMBOL (qcom_scm_pas_shutdown); +/** +* qcom_scm_assign_mem() - Make a secure call to reassign memory ownership +* +* @mem_addr: mem region whose ownership need to be reassigned +* @mem_sz: size of the region. +* @srcvm: vmid for current set of owners, each set bit in +* flag indicate a unique owner +* @newvm: array having new owners and corrsponding permission +* flags Quoted 8 lines
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 321
Quoted 7 lines
drivers/firmware/qcom_scm-64.c +7
-0+* @dest_cnt: number of owners in next set. +* Return next set of owners on success. +*/ +int qcom_scm_assign_mem (phys_addr_t mem_addr,size_t mem_sz,int srcvm, +struct qcom_scm_vmperm *newvm,int dest_cnt) +{ +unsigned long dma_attrs =DMA_ATTR_FORCE_CONTIGUOUS ;Why do we need this? Just curious if we can drop this.
Quoted 5 lines
The force contiguous flag is required with
dma_alloc_attrs()api to allocate memory from physically contiguous zone.
I am not sure, are you saying that api will work without the attribute or you mean i shall use some api which does not take explicit attribute?
Does physically contiguous zone mean some CMA carveout? I wasn't aware of a carveout for scm devices. I'm still not following the reasoning here.
I'm saying that I don't understand why we need this flag. It feels like this sort of constraint would apply all over the scm driver if it was true, hence the confusion.
Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
drivers/firmware/qcom_scm-64.c +4
-0+ + ret =__qcom_scm_assign_mem (__scm->dev, memory_phys, + memory_sz, src_phys, src_sz, dest_phys, dest_sz); + dma_free_attrs(__scm->dev, ALIGN(mem_all_sz, SZ_64),Quoted 8 lines
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 330
Quoted 5 lines
drivers/firmware/qcom_scm-64.c +5
-0+ ptr, src_phys, dma_attrs); +if (ret ==0 ) +return next_vm; +else if (ret >0 ) +return -ret;This still confuses me. Do we really just pass whatever the firmware tells us the error code is up to the caller? Shouldn't we be remapping the scm errors we receive to normal linux errnos?
Quoted 7 lines
because i do not know in advance what exactly will be the return error code, moreover there are number of error codes which are returned in case of failure
so if i have to return linux error code, i can not do one to one mapping of error code and will have to return single error code for all failure.
let me know your comments further on this.+ return ret;
Yes, returning -EINVAL all the time is fine if we can't remap the error. In fact, we should probably do what we do downstream and print out the error value returned from the firmware to the kernel log and then return some sane errno up to the caller. That way the few people who know what the error codes mean can tell us why the scm call failed.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v6 1/4] firmware: scm: Add new SCM call API for switching memory ownership
Quoted 28 lines · Stephen Boyd
Quoted 27 lines · Avaneesh Kumar (avani)
Quoted 25 lines · Stephen Boyd
Quoted 13 lines · Avaneesh Kumar Dwivedi
drivers/firmware/qcom_scm-64.c +6
-0@@ -292,6 +304,86 @@ int qcom_scm_pas_shutdown(u32 peripheral) }EXPORT_SYMBOL (qcom_scm_pas_shutdown); +/** +* qcom_scm_assign_mem() - Make a secure call to reassign memory ownership +* +* @mem_addr: mem region whose ownership need to be reassigned +* @mem_sz: size of the region. +* @srcvm: vmid for current set of owners, each set bit in drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 321
Quoted 10 lines
drivers/firmware/qcom_scm-64.c +10
-0+* flag indicate a unique owner +* @newvm: array having new owners and corrsponding permission +* flags +* @dest_cnt: number of owners in next set. +* Return next set of owners on success. +*/ +int qcom_scm_assign_mem (phys_addr_t mem_addr,size_t mem_sz,int srcvm, +struct qcom_scm_vmperm *newvm,int dest_cnt) +{ +unsigned long dma_attrs =DMA_ATTR_FORCE_CONTIGUOUS ;Why do we need this? Just curious if we can drop this.
Quoted 8 lines
Quoted 5 lines
The force contiguous flag is required with
dma_alloc_attrs()api to allocate memory from physically contiguous zone.
I am not sure, are you saying that api will work without the attribute or you mean i shall use some api which does not take explicit attribute?Does physically contiguous zone mean some CMA carveout? I wasn't aware of a carveout for scm devices. I'm still not following the reasoning here.
the memory will be allocated from common carveout, there is no scm device specific carveout. i will use dma_alloc_coherent() and will drop off this flag.
we need physical contigious zone to fill and pass scm call parameters to TZ.
Quoted 24 lines
I'm saying that I don't understand why we need this flag. It feels like this sort of constraint would apply all over the scm driver if it was true, hence the confusion.
Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
drivers/firmware/qcom_scm-64.c +4
-0+ + ret =__qcom_scm_assign_mem (__scm->dev, memory_phys, + memory_sz, src_phys, src_sz, dest_phys, dest_sz); + dma_free_attrs(__scm->dev, ALIGN(mem_all_sz, SZ_64),Quoted 8 lines
drivers/firmware/qcom_scm-64.c int qcom_scm_pas_shutdown(u32 peripheral) · line 330
Quoted 5 lines
drivers/firmware/qcom_scm-64.c +5
-0+ ptr, src_phys, dma_attrs); +if (ret ==0 ) +return next_vm; +else if (ret >0 ) +return -ret;This still confuses me. Do we really just pass whatever the firmware tells us the error code is up to the caller? Shouldn't we be remapping the scm errors we receive to normal linux errnos?
Quoted 7 lines
because i do not know in advance what exactly will be the return error code, moreover there are number of error codes which are returned in case of failure
so if i have to return linux error code, i can not do one to one mapping of error code and will have to return single error code for all failure.
let me know your comments further on this.+ return ret;
Quoted 6 lines
Yes, returning
-EINVALall the time is fine if we can't remap the error. In fact, we should probably do what we do downstream and print out the error value returned from the firmware to the kernel log and then return some sane errno up to the caller. That way the few people who know what the error codes mean can tell us why the scm call failed.
OK, will do same.
just last thing to ask, should i resend all 4 patches together again or only one patch in v7 version. as chnage will be in only 1 out of 4 patches.
Quoted 1 line
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v6 1/4] firmware: scm: Add new SCM call API for switching memory ownership
Quoted 4 lines · Avaneesh Kumar (avani)
OK, will do same.
just last thing to ask, should i resend all 4 patches together again or only one patch in v7 version.
as chnage will be in only 1 out of 4 patches.
I would resend all of them. That's standard practice for patch series.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v6 1/4] firmware: scm: Add new SCM call API for switching memory ownership
In reply to Avaneesh Kumar DwivediQuoted 51 lines · Avaneesh Kumar Dwivedi
Two different processors on a SOC need to switch memory ownership during load/unload. To enable this, second level memory map table need to be updated, which is done by secure layer.
This patch adds the interface for making secure monitor call for memory ownership switching request.Signed-off-by Avaneesh Kumar Dwivedi <[email protected]>--- drivers/firmware/qcom_scm-32.c | 6 +++ drivers/firmware/qcom_scm-64.c | 27 +++++++++++++ drivers/firmware/qcom_scm.c | 92 ++++++++++++++++++++++++++++++++++++++++++ drivers/firmware/qcom_scm.h | 5 +++ include/linux/qcom_scm.h | 16 ++++++++ 5 files changed, 146 insertions(+)drivers/firmware/qcom_scm-32.c +6
-0@@ -596,3 +596,9 @@ int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size, {return -ENODEV ; } +int __qcom_scm_assign_mem (struct device *dev,phys_addr_t mem_region, +size_t mem_sz,phys_addr_t src,size_t src_sz, +phys_addr_t dest,size_t dest_sz) +{ +return -ENODEV ; +}drivers/firmware/qcom_scm-64.c +14
-0@@ -439,3 +439,30 @@ int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size,return ret; } + +int __qcom_scm_assign_mem (struct device *dev,phys_addr_t mem_region, +size_t mem_sz,phys_addr_t src,size_t src_sz, +phys_addr_t dest,size_t dest_sz) +{ +int ret; +struct qcom_scm_desc desc = {0 }; +struct arm_smccc_res res; + + desc.args[0 ] = mem_region; + desc.args[1 ] = mem_sz; + desc.args[2 ] = src; + desc.args[3 ] = src_sz; + desc.args[4 ] = dest;
drivers/firmware/qcom_scm-64.c int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size, · line 465
Quoted 10 lines
drivers/firmware/qcom_scm-64.c +10
-0+ desc.args[5 ] = dest_sz; + desc.args[6 ] =0 ; + + desc.arginfo =QCOM_SCM_ARGS (7 ,QCOM_SCM_RO ,QCOM_SCM_VAL , +QCOM_SCM_RO ,QCOM_SCM_VAL ,QCOM_SCM_RO , +QCOM_SCM_VAL ,QCOM_SCM_VAL ); + + ret =qcom_scm_call (dev,QCOM_SCM_SVC_MP , +QCOM_MEM_PROT_ASSIGN_ID , + &desc, &res);
Please indent broken lines by the start parenthesis, throughout the patch, this makes the code easier to read. You can run checkpatch.pl with the --strict flag to show a few other places below that has the same issue.
Please clean these up together with the dma allocation and the return value as pointed out by Stephen and I'm happy to pick the series up.
Regards,
Bjorn
Re: [PATCH v6 1/4] firmware: scm: Add new SCM call API for switching memory ownership
Quoted 69 lines · Bjorn Andersson
Quoted 51 lines · Avaneesh Kumar Dwivedi
Two different processors on a SOC need to switch memory ownership during load/unload. To enable this, second level memory map table need to be updated, which is done by secure layer.
This patch adds the interface for making secure monitor call for memory ownership switching request.Signed-off-by Avaneesh Kumar Dwivedi <[email protected]>--- drivers/firmware/qcom_scm-32.c | 6 +++ drivers/firmware/qcom_scm-64.c | 27 +++++++++++++ drivers/firmware/qcom_scm.c | 92 ++++++++++++++++++++++++++++++++++++++++++ drivers/firmware/qcom_scm.h | 5 +++ include/linux/qcom_scm.h | 16 ++++++++ 5 files changed, 146 insertions(+)drivers/firmware/qcom_scm-32.c +6
-0@@ -596,3 +596,9 @@ int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size, {return -ENODEV ; } +int __qcom_scm_assign_mem (struct device *dev,phys_addr_t mem_region, +size_t mem_sz,phys_addr_t src,size_t src_sz, +phys_addr_t dest,size_t dest_sz) +{ +return -ENODEV ; +}drivers/firmware/qcom_scm-64.c +14
-0@@ -439,3 +439,30 @@ int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size,return ret; } + +int __qcom_scm_assign_mem (struct device *dev,phys_addr_t mem_region, +size_t mem_sz,phys_addr_t src,size_t src_sz, +phys_addr_t dest,size_t dest_sz) +{ +int ret; +struct qcom_scm_desc desc = {0 }; +struct arm_smccc_res res; + + desc.args[0 ] = mem_region; + desc.args[1 ] = mem_sz; + desc.args[2 ] = src; + desc.args[3 ] = src_sz; + desc.args[4 ] = dest;drivers/firmware/qcom_scm-64.c int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr, u32 size, · line 465
Quoted 10 lines
drivers/firmware/qcom_scm-64.c +10
-0+ desc.args[5 ] = dest_sz; + desc.args[6 ] =0 ; + + desc.arginfo =QCOM_SCM_ARGS (7 ,QCOM_SCM_RO ,QCOM_SCM_VAL , +QCOM_SCM_RO ,QCOM_SCM_VAL ,QCOM_SCM_RO , +QCOM_SCM_VAL ,QCOM_SCM_VAL ); + + ret =qcom_scm_call (dev,QCOM_SCM_SVC_MP , +QCOM_MEM_PROT_ASSIGN_ID , + &desc, &res);Please indent broken lines by the start parenthesis, throughout the patch, this makes the code easier to read. You can run checkpatch.pl with the --strict flag to show a few other places below that has the same issue.
Quoted 2 lines
Please clean these up together with the dma allocation and the return value as pointed out by Stephen and I'm happy to pick the series up.
Sure, thank you, will do and send out by eod tomorrow.
Quoted 3 lines
Regards,
Bjorn
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH v6 2/4 remoteproc: qcom: refactor mss fw image loading sequence
In reply to Avaneesh Kumar DwivediThis patch refactor code to first load all firmware blobs and then update modem proc to authenticate and boot fw.
--- drivers/remoteproc/qcom_q6v5_pil.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
drivers/remoteproc/qcom_q6v5_pil.c +8 -11
@@ -503,7 +503,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)bool relocate = false;char seg_name[10 ];ssize_t offset; -size_t size; +size_t size =0 ;void *ptr;int ret;int i;
@@ -541,7 +541,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc) } mpss_reloc = relocate ? min_addr : qproc->mpss_phys; - +/* Load firmware segments */ for (i =0 ; i < ehdr->e_phnum; i++) { phdr = &phdrs[i];
@@ -574,18 +574,15 @@ static int q6v5_mpss_load(struct q6v5 *qproc)memset (ptr + phdr->p_filesz,0 , phdr->p_memsz - phdr->p_filesz); } - - size =readl (qproc->rmb_base +RMB_PMI_CODE_LENGTH_REG ); -if (!size) { - boot_addr = relocate ? qproc->mpss_phys : min_addr; -writel (boot_addr, qproc->rmb_base +RMB_PMI_CODE_START_REG ); -writel (RMB_CMD_LOAD_READY , qproc->rmb_base +RMB_MBA_COMMAND_REG ); - } - size += phdr->p_memsz; -writel (size, qproc->rmb_base +RMB_PMI_CODE_LENGTH_REG ); } +/* Transfer ownership of modem ddr region with q6*/ + boot_addr = relocate ? qproc->mpss_phys : min_addr; +writel (boot_addr, qproc->rmb_base +RMB_PMI_CODE_START_REG ); +writel (RMB_CMD_LOAD_READY , qproc->rmb_base +RMB_MBA_COMMAND_REG ); +writel (size, qproc->rmb_base +RMB_PMI_CODE_LENGTH_REG ); + ret =q6v5_rmb_mba_wait (qproc,RMB_MBA_AUTH_COMPLETE ,10000 );if (ret == -ETIMEDOUT )dev_err (qproc->dev,"MPSS authentication timed out\n" );
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH v6 3/4 remoteproc: qcom: Make secure world call for mem ownership switch
In reply to Avaneesh Kumar DwivediMSS 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.
--- drivers/remoteproc/qcom_q6v5_pil.c | 86 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 83 insertions(+), 3 deletions(-)
drivers/remoteproc/qcom_q6v5_pil.c +83 -3
@@ -110,6 +110,7 @@ struct rproc_hexagon_res {struct qcom_mss_reg_res *active_supply;char **proxy_clk_names;char **active_clk_names; +bool need_mem_protection; };struct q6v5 {
@@ -151,8 +152,11 @@ struct q6v5 {phys_addr_t mpss_reloc;void *mpss_region;size_t mpss_size; +bool need_mem_protection;struct qcom_rproc_subdev smd_subdev; +int mpss_perm; +int mba_perm; }; static int q6v5_regulator_init(struct device *dev, struct reg_info *regs,
@@ -288,6 +292,36 @@ static struct resource_table *q6v5_find_rsc_table(struct rproc *rproc,return &table; } +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); +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); +if (ret) +return -EAGAIN ;writel (phys, qproc->rmb_base +RMB_PMI_META_DATA_REG );writel (RMB_CMD_META_DATA_READY , qproc->rmb_base +RMB_MBA_COMMAND_REG );
@@ -470,6 +512,12 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)else if (ret <0 )dev_err (qproc->dev,"MPSS header authentication failed: %d\n" , ret); +/* Metadata authentication done, remove modem access */ + xferop_ret =q6v5_xfer_mem_ownership (qproc, + &mdata_perm,0 , phys, fw->size); +if (xferop_ret) +dev_warn (qproc->dev, +"mdt buffer not reclaimed system may become unstable\n" );dma_free_attrs (qproc->dev, fw->size, ptr, phys, dma_attrs);return ret <0 ? ret :0 ;
@@ -579,6 +627,10 @@ static int q6v5_mpss_load(struct q6v5 *qproc)/* Transfer ownership of modem ddr region with q6*/ boot_addr = relocate ? qproc->mpss_phys : min_addr; + ret =q6v5_xfer_mem_ownership (qproc, &qproc->mpss_perm,1 , + qproc->mpss_phys, qproc->mpss_size); +if (ret) +return -EAGAIN ;writel (boot_addr, qproc->rmb_base +RMB_PMI_CODE_START_REG );writel (RMB_CMD_LOAD_READY , qproc->rmb_base +RMB_MBA_COMMAND_REG );writel (size, qproc->rmb_base +RMB_PMI_CODE_LENGTH_REG );
@@ -598,6 +650,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)static int q6v5_start (struct rproc *rproc) {struct q6v5 *qproc = (struct q6v5 *)rproc->priv; +int xfermemop_ret;int ret; ret = q6v5_regulator_enable(qproc, qproc->proxy_regs,
@@ -633,6 +686,11 @@ static int q6v5_start(struct rproc *rproc) goto assert_reset; } +/* Assign MBA image access in DDR to q6 */ + xfermemop_ret =q6v5_xfer_mem_ownership (qproc, &qproc->mba_perm,1 , + qproc->mba_phys, qproc->mba_size); +if (xfermemop_ret) + goto assert_reset;writel (qproc->mba_phys, qproc->rmb_base +RMB_MBA_IMAGE_REG ); ret =q6v5proc_reset (qproc);
@@ -654,16 +712,21 @@ static int q6v5_start(struct rproc *rproc) ret =q6v5_mpss_load (qproc);if (ret) - goto halt_axi_ports; + goto reclaim_mem; ret =wait_for_completion_timeout (&qproc->start_done,msecs_to_jiffies (5000 ));if (ret ==0 ) {dev_err (qproc->dev,"start timed out\n" ); ret = -ETIMEDOUT ; - goto halt_axi_ports; + goto reclaim_mem; } + xfermemop_ret =q6v5_xfer_mem_ownership (qproc, &qproc->mba_perm,0 , + qproc->mba_phys, qproc->mba_size); +if (xfermemop_ret) +dev_err (qproc->dev, +"Failed to reclaim mba buffer system may become unstable\n" ); qproc->running = true; q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
@@ -673,12 +736,21 @@ static int q6v5_start(struct rproc *rproc)return 0 ; +reclaim_mem: + xfermemop_ret =q6v5_xfer_mem_ownership (qproc, &qproc->mpss_perm,0 , + qproc->mpss_phys, qproc->mpss_size); +WARN_ON (xfermemop_ret); halt_axi_ports:q6v5proc_halt_axi_port (qproc, qproc->halt_map, qproc->halt_q6);q6v5proc_halt_axi_port (qproc, qproc->halt_map, qproc->halt_modem);q6v5proc_halt_axi_port (qproc, qproc->halt_map, qproc->halt_nc);q6v5_clk_disable (qproc->dev, qproc->active_clks, qproc->active_clk_count); + xfermemop_ret =q6v5_xfer_mem_ownership (qproc, &qproc->mba_perm,0 , + qproc->mba_phys, qproc->mba_size); +if (xfermemop_ret) +dev_err (qproc->dev,"Failed to reclaim mba buffer, system may become unstable\n" ); + assert_reset:reset_control_assert (qproc->mss_restart); disable_vdd:
@@ -698,6 +770,7 @@ static int q6v5_stop(struct rproc *rproc) {struct q6v5 *qproc = (struct q6v5 *)rproc->priv;int ret; +u32 val; qproc->running = false;
@@ -715,6 +788,9 @@ static int q6v5_stop(struct rproc *rproc)q6v5proc_halt_axi_port (qproc, qproc->halt_map, qproc->halt_modem);q6v5proc_halt_axi_port (qproc, qproc->halt_map, qproc->halt_nc); + ret =q6v5_xfer_mem_ownership (qproc, &qproc->mpss_perm,0 , + qproc->mpss_phys, qproc->mpss_size); +WARN_ON (ret);reset_control_assert (qproc->mss_restart);q6v5_clk_disable (qproc->dev, qproc->active_clks, qproc->active_clk_count);
@@ -1012,6 +1088,7 @@ static int q6v5_probe(struct platform_device *pdev)if (ret) goto free_rproc; + qproc->need_mem_protection = desc->need_mem_protection; ret =q6v5_request_irq (qproc, pdev,"wdog" , q6v5_wdog_interrupt);if (ret <0 ) goto free_rproc;
@@ -1033,7 +1110,8 @@ static int q6v5_probe(struct platform_device *pdev) ret =PTR_ERR (qproc->state); goto free_rproc; } - + qproc->mpss_perm =BIT (QCOM_SCM_VMID_HLOS ); + qproc->mba_perm =BIT (QCOM_SCM_VMID_HLOS );qcom_add_smd_subdev (rproc, &qproc->smd_subdev); ret =rproc_add (rproc);
@@ -1087,6 +1165,7 @@ static int q6v5_remove(struct platform_device *pdev)"mem" ,NULL }, + .need_mem_protection = false, }; static const struct rproc_hexagon_res msm8974_mss = {
@@ -1124,6 +1203,7 @@ static int q6v5_remove(struct platform_device *pdev)"mem" ,NULL }, + .need_mem_protection = false, }; static const struct of_device_id q6v5_of_match[] = {
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH v6 4/4 remoteproc: qcom: Add support for mss remoteproc on msm8996
In reply to Avaneesh Kumar DwivediThis patch add support for mss boot on msm8996. Major changes include initializing mss rproc for msm8996, making appropriate change for executing mss reset sequence etc.
--- .../devicetree/bindings/remoteproc/qcom,q6v5.txt | 1 + drivers/remoteproc/qcom_q6v5_pil.c | 163 ++++++++++++++++++--- 2 files changed, 140 insertions(+), 24 deletions(-)
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +1 -0
@@ -11,6 +11,7 @@ on the Qualcomm Hexagon core. "qcom,msm8916-mss-pil", "qcom,msm8974-mss-pil" + "qcom,msm8996-mss-pil" - reg: Usage: required Value type: <prop-encoded-array>
drivers/remoteproc/qcom_q6v5_pil.c +139 -24
238 lines not on this page · raw message
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.