Hi Elliot, Thanks for the comments! On 7/29/20 8:15 PM, Elliot Berman wrote: > ++ > > On 7/24/2020 8:04 AM, Stanimir Varbanov wrote: >> Hi, >> >> Gentle ping for review. >> >> On 7/9/20 2:58 PM, Stanimir Varbanov wrote: >>> This adds a new SCM memprotect command to set virtual address ranges. >>> >>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> >>> --- >>> drivers/firmware/qcom_scm.c | 24 ++++++++++++++++++++++++ >>> drivers/firmware/qcom_scm.h | 1 + >>> include/linux/qcom_scm.h | 8 +++++++- >>> 3 files changed, 32 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >>> index 0e7233a20f34..a73870255c2e 100644 >>> --- a/drivers/firmware/qcom_scm.c >>> +++ b/drivers/firmware/qcom_scm.c >>> @@ -864,6 +864,30 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, >>> } >>> EXPORT_SYMBOL(qcom_scm_assign_mem); >>> >>> +int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size, >>> + u32 cp_nonpixel_start, >>> + u32 cp_nonpixel_size) >>> +{ >>> + int ret; >>> + struct qcom_scm_desc desc = { >>> + .svc = QCOM_SCM_SVC_MP, >>> + .cmd = QCOM_SCM_MP_VIDEO_VAR, >>> + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_VAL, QCOM_SCM_VAL, >>> + QCOM_SCM_VAL, QCOM_SCM_VAL), >>> + .args[0] = cp_start, >>> + .args[1] = cp_size, >>> + .args[2] = cp_nonpixel_start, >>> + .args[3] = cp_nonpixel_size, >>> + .owner = ARM_SMCCC_OWNER_SIP, >>> + }; >>> + struct qcom_scm_res res; >>> + >>> + ret = qcom_scm_call(__scm->dev, &desc, &res); >>> + >>> + return ret ? : res.result[0]; >>> +} >>> +EXPORT_SYMBOL(qcom_scm_mem_protect_video_var); >>> + > > Small nit, can you bump the function above assign_mem? It would keep order aligned with > the macros in qcom_scm.h Sure, I will do that. > >>> /** >>> * qcom_scm_ocmem_lock_available() - is OCMEM lock/unlock interface available >>> */ >>> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h >>> index d9ed670da222..14da834ac593 100644 >>> --- a/drivers/firmware/qcom_scm.h >>> +++ b/drivers/firmware/qcom_scm.h >>> @@ -97,6 +97,7 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc, >>> #define QCOM_SCM_MP_RESTORE_SEC_CFG 0x02 >>> #define QCOM_SCM_MP_IOMMU_SECURE_PTBL_SIZE 0x03 >>> #define QCOM_SCM_MP_IOMMU_SECURE_PTBL_INIT 0x04 >>> +#define QCOM_SCM_MP_VIDEO_VAR 0x08 >>> #define QCOM_SCM_MP_ASSIGN 0x16 >>> >>> #define QCOM_SCM_SVC_OCMEM 0x0f >>> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h >>> index 3d6a24697761..19b5188d17f4 100644 >>> --- a/include/linux/qcom_scm.h >>> +++ b/include/linux/qcom_scm.h >>> @@ -81,7 +81,9 @@ extern int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, >>> unsigned int *src, >>> const struct qcom_scm_vmperm *newvm, >>> unsigned int dest_cnt); >>> - >>> +extern int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size, >>> + u32 cp_nonpixel_start, >>> + u32 cp_nonpixel_size); > > Same here. > >>> extern bool qcom_scm_ocmem_lock_available(void); >>> extern int qcom_scm_ocmem_lock(enum qcom_scm_ocmem_client id, u32 offset, >>> u32 size, u32 mode); >>> @@ -131,6 +133,10 @@ static inline int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare) >>> static inline int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz, >>> unsigned int *src, const struct qcom_scm_vmperm *newvm, >>> unsigned int dest_cnt) { return -ENODEV; } >>> +extern inline int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size, >>> + u32 cp_nonpixel_start, >>> + u32 cp_nonpixel_size) >>> + { return -ENODEV; } > > Same here. > >>> >>> static inline bool qcom_scm_ocmem_lock_available(void) { return false; } >>> static inline int qcom_scm_ocmem_lock(enum qcom_scm_ocmem_client id, u32 offset, >>> >> > -- regards, Stan