From 3c1c4cd3338c29d224ecca65b6aa364b91c89f92 Mon Sep 17 00:00:00 2001 From: Veera Sundaram Sankaran Date: Tue, 8 Dec 2020 15:34:17 -0800 Subject: [PATCH 1/3] disp: msm: sde: avoid sde debugfs register access based on HW ownership Add VM ownership checks before accessing the HW through the debugfs path in sde crtc/encoder/connector modules to avoid illegal access. Change-Id: I4ff8f29353835d263beb2091bdeec40125addbf8 Signed-off-by: Veera Sundaram Sankaran --- msm/sde/sde_connector.c | 18 +++++++++++++++++- msm/sde/sde_crtc.c | 12 ++++++++++++ msm/sde/sde_encoder.c | 11 +++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/msm/sde/sde_connector.c b/msm/sde/sde_connector.c index c31c9731bd13..fabefa0dbe44 100644 --- a/msm/sde/sde_connector.c +++ b/msm/sde/sde_connector.c @@ -1901,6 +1901,8 @@ static ssize_t _sde_debugfs_conn_cmd_tx_write(struct file *file, { struct drm_connector *connector = file->private_data; struct sde_connector *c_conn = NULL; + struct sde_vm_ops *vm_ops; + struct sde_kms *sde_kms; char *input, *token, *input_copy, *input_dup = NULL; const char *delim = " "; char buffer[MAX_CMD_PAYLOAD_SIZE] = {0}; @@ -1911,9 +1913,14 @@ static ssize_t _sde_debugfs_conn_cmd_tx_write(struct file *file, SDE_ERROR("invalid argument(s), conn %d\n", connector != NULL); return -EINVAL; } - c_conn = to_sde_connector(connector); + sde_kms = _sde_connector_get_kms(&c_conn->base); + if (!sde_kms) { + SDE_ERROR("invalid kms\n"); + return -EINVAL; + } + if (!c_conn->ops.cmd_transfer) { SDE_ERROR("no cmd transfer support for connector name %s\n", c_conn->name); @@ -1924,6 +1931,14 @@ static ssize_t _sde_debugfs_conn_cmd_tx_write(struct file *file, if (!input) return -ENOMEM; + vm_ops = sde_vm_get_ops(sde_kms); + sde_vm_lock(sde_kms); + if (vm_ops && vm_ops->vm_owns_hw && !vm_ops->vm_owns_hw(sde_kms)) { + SDE_DEBUG("op not supported due to HW unavailablity\n"); + rc = -EOPNOTSUPP; + goto end; + } + if (copy_from_user(input, p, count)) { SDE_ERROR("copy from user failed\n"); rc = -EFAULT; @@ -1973,6 +1988,7 @@ static ssize_t _sde_debugfs_conn_cmd_tx_write(struct file *file, end1: kfree(input_dup); end: + sde_vm_unlock(sde_kms); kfree(input); return rc; } diff --git a/msm/sde/sde_crtc.c b/msm/sde/sde_crtc.c index 45a11b19bfed..b217dbc70d51 100644 --- a/msm/sde/sde_crtc.c +++ b/msm/sde/sde_crtc.c @@ -40,6 +40,7 @@ #include "sde_power_handle.h" #include "sde_core_perf.h" #include "sde_trace.h" +#include "sde_vm.h" #define SDE_PSTATES_MAX (SDE_STAGE_MAX * 4) #define SDE_MULTIRECT_PLANE_MAX (SDE_STAGE_MAX * 2) @@ -6003,6 +6004,7 @@ static ssize_t _sde_crtc_misr_read(struct file *file, struct sde_crtc *sde_crtc; struct sde_kms *sde_kms; struct sde_crtc_mixer *m; + struct sde_vm_ops *vm_ops; int i = 0, rc; ssize_t len = 0; char buf[MISR_BUFF_SIZE + 1] = {'\0'}; @@ -6023,8 +6025,17 @@ static ssize_t _sde_crtc_misr_read(struct file *file, if (rc < 0) return rc; + vm_ops = sde_vm_get_ops(sde_kms); + sde_vm_lock(sde_kms); + if (vm_ops && vm_ops->vm_owns_hw && !vm_ops->vm_owns_hw(sde_kms)) { + SDE_DEBUG("op not supported due to HW unavailability\n"); + rc = -EOPNOTSUPP; + goto end; + } + if (sde_kms_is_secure_session_inprogress(sde_kms)) { SDE_DEBUG("crtc:%d misr read not allowed\n", DRMID(crtc)); + rc = -EOPNOTSUPP; goto end; } @@ -6074,6 +6085,7 @@ buff_check: *ppos += len; /* increase offset */ end: + sde_vm_unlock(sde_kms); pm_runtime_put_sync(crtc->dev->dev); return len; } diff --git a/msm/sde/sde_encoder.c b/msm/sde/sde_encoder.c index dcce678ba64d..0fca0a2910c0 100644 --- a/msm/sde/sde_encoder.c +++ b/msm/sde/sde_encoder.c @@ -41,6 +41,7 @@ #include "sde_hw_top.h" #include "sde_hw_qdss.h" #include "sde_encoder_dce.h" +#include "sde_vm.h" #define SDE_DEBUG_ENC(e, fmt, ...) SDE_DEBUG("enc%d " fmt,\ (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) @@ -4496,6 +4497,7 @@ static ssize_t _sde_encoder_misr_read(struct file *file, struct sde_encoder_virt *sde_enc; struct sde_kms *sde_kms = NULL; struct drm_encoder *drm_enc; + struct sde_vm_ops *vm_ops; int i = 0, len = 0; char buf[MISR_BUFF_SIZE + 1] = {'\0'}; int rc; @@ -4521,6 +4523,14 @@ static ssize_t _sde_encoder_misr_read(struct file *file, if (rc < 0) return rc; + vm_ops = sde_vm_get_ops(sde_kms); + sde_vm_lock(sde_kms); + if (vm_ops && vm_ops->vm_owns_hw && !vm_ops->vm_owns_hw(sde_kms)) { + SDE_DEBUG("op not supported due to HW unavailablity\n"); + rc = -EOPNOTSUPP; + goto end; + } + if (!sde_enc->misr_enable) { len += scnprintf(buf + len, MISR_BUFF_SIZE - len, "disabled\n"); @@ -4568,6 +4578,7 @@ buff_check: *ppos += len; /* increase offset */ end: + sde_vm_unlock(sde_kms); pm_runtime_put_sync(drm_enc->dev->dev); return len; } From 4c29eaf3b09161963f26eacff7ebb801a0e63e0b Mon Sep 17 00:00:00 2001 From: Veera Sundaram Sankaran Date: Wed, 9 Dec 2020 16:48:40 -0800 Subject: [PATCH 2/3] disp: msm: avoid sde_dbg HW access based on HW ownership Add VM ownership checks before accessing the HW through the debugfs path in sde dbg module to avoid illegal access. Set the ownership status on bootup based on the VM. Change-Id: I30f2a1eb7e049740f1f7fe3ed03aa411264ba7b3 Signed-off-by: Veera Sundaram Sankaran --- msm/sde/sde_kms.c | 15 ++++++++-- msm/sde_dbg.c | 74 +++++++++++++++++++++++++++++++++++------------ msm/sde_dbg.h | 6 ++++ 3 files changed, 75 insertions(+), 20 deletions(-) diff --git a/msm/sde/sde_kms.c b/msm/sde/sde_kms.c index 5a0f4d6bb8bb..3e4a6e2c7f09 100644 --- a/msm/sde/sde_kms.c +++ b/msm/sde/sde_kms.c @@ -1073,6 +1073,8 @@ int sde_kms_vm_primary_prepare_commit(struct sde_kms *sde_kms, /* enable vblank events */ drm_crtc_vblank_on(crtc); + sde_dbg_set_hw_ownership_status(true); + /* handle non-SDE pre_acquire */ if (vm_ops->vm_client_post_acquire) rc = vm_ops->vm_client_post_acquire(sde_kms); @@ -1112,6 +1114,8 @@ int sde_kms_vm_trusted_prepare_commit(struct sde_kms *sde_kms, sde_hw_set_lutdma_sid(sde_kms->hw_sid, 1); + sde_dbg_set_hw_ownership_status(true); + return 0; } @@ -1317,6 +1321,8 @@ int sde_kms_vm_trusted_post_commit(struct sde_kms *sde_kms, sde_hw_set_lutdma_sid(sde_kms->hw_sid, 0); + sde_dbg_set_hw_ownership_status(false); + sde_vm_lock(sde_kms); if (vm_ops->vm_release) @@ -1370,6 +1376,8 @@ int sde_kms_vm_pre_release(struct sde_kms *sde_kms, /* reset sw state */ sde_crtc_reset_sw_state(crtc); + sde_dbg_set_hw_ownership_status(false); + return rc; } @@ -4536,10 +4544,13 @@ static int sde_kms_hw_init(struct msm_kms *kms) SDE_DEBUG("Registering for notification of irq_num: %d\n", irq_num); irq_set_affinity_notifier(irq_num, &sde_kms->affinity_notify); - if (sde_in_trusted_vm(sde_kms)) + if (sde_in_trusted_vm(sde_kms)) { rc = sde_vm_trusted_init(sde_kms); - else + sde_dbg_set_hw_ownership_status(false); + } else { rc = sde_vm_primary_init(sde_kms); + sde_dbg_set_hw_ownership_status(true); + } if (rc) { SDE_ERROR("failed to initialize VM ops, rc: %d\n", rc); goto error; diff --git a/msm/sde_dbg.c b/msm/sde_dbg.c index 06b06ccf3ff7..cc997057eed5 100644 --- a/msm/sde_dbg.c +++ b/msm/sde_dbg.c @@ -253,6 +253,7 @@ struct sde_dbg_regbuf { * @cur_evt_index: index used for tracking event logs dump in hw recovery * @dbgbus_dump_idx: index used for tracking dbg-bus dump in hw recovery * @vbif_dbgbus_dump_idx: index for tracking vbif dumps in hw recovery + * @hw_ownership: indicates if the VM owns the HW resources */ static struct sde_dbg_base { struct sde_dbg_evtlog *evtlog; @@ -278,6 +279,7 @@ static struct sde_dbg_base { struct sde_dbg_regbuf regbuf; u32 cur_evt_index; enum sde_dbg_dump_context dump_mode; + bool hw_ownership; } sde_dbg_base; static LIST_HEAD(sde_dbg_dsi_list); @@ -5609,6 +5611,12 @@ static ssize_t sde_recovery_regdump_read(struct file *file, char __user *ubuf, struct sde_dbg_regbuf *rbuf = &dbg_base->regbuf; mutex_lock(&sde_dbg_base.mutex); + if (!sde_dbg_base.hw_ownership) { + pr_debug("op not supported due to HW unavailablity\n"); + len = -EOPNOTSUPP; + goto err; + } + if (!rbuf->dump_done && !rbuf->cur_blk) { if (!rbuf->buf) rbuf->buf = kzalloc(DUMP_BUF_SIZE, GFP_KERNEL); @@ -5689,6 +5697,12 @@ static ssize_t sde_recovery_dbgbus_dump_read(struct file *file, memset(log_buf, 0, sizeof(log_buf)); mutex_lock(&sde_dbg_base.mutex); + if (!sde_dbg_base.hw_ownership) { + pr_debug("op not supported due to HW unavailablity\n"); + len = -EOPNOTSUPP; + goto dump_done; + } + if (!cmn->dumped_content || !cmn->entries_size) goto dump_done; @@ -6011,32 +6025,41 @@ static ssize_t sde_dbg_reg_base_reg_write(struct file *file, return -EFAULT; mutex_lock(&sde_dbg_base.mutex); + if (!sde_dbg_base.hw_ownership) { + pr_debug("op not supported due to hw unavailablity\n"); + count = -EOPNOTSUPP; + goto end; + } + if (off >= dbg->max_offset) { - mutex_unlock(&sde_dbg_base.mutex); - return -EFAULT; + count = -EFAULT; + goto end; } if (!list_empty(&dbg->sub_range_list)) { rc = sde_dbg_reg_base_is_valid_range(dbg, off, cnt); - if (!rc) - return -EINVAL; + if (!rc) { + count = -EINVAL; + goto end; + } } rc = pm_runtime_get_sync(sde_dbg_base.dev); if (rc < 0) { - mutex_unlock(&sde_dbg_base.mutex); pr_err("failed to enable power %d\n", rc); - return rc; + count = rc; + goto end; } writel_relaxed(data, dbg->base + off); pm_runtime_put_sync(sde_dbg_base.dev); - mutex_unlock(&sde_dbg_base.mutex); - pr_debug("addr=%zx data=%x\n", off, data); +end: + mutex_unlock(&sde_dbg_base.mutex); + return count; } #endif @@ -6068,6 +6091,12 @@ static ssize_t sde_dbg_reg_base_reg_read(struct file *file, return -EINVAL; mutex_lock(&sde_dbg_base.mutex); + if (!sde_dbg_base.hw_ownership) { + pr_debug("op not supported due to hw unavailablity\n"); + len = -EOPNOTSUPP; + goto end; + } + if (!dbg->buf) { char dump_buf[64]; char *ptr; @@ -6078,13 +6107,13 @@ static ssize_t sde_dbg_reg_base_reg_read(struct file *file, dbg->buf = kzalloc(dbg->buf_len, GFP_KERNEL); if (!dbg->buf) { - mutex_unlock(&sde_dbg_base.mutex); - return -ENOMEM; + len = -ENOMEM; + goto end; } if (dbg->off % sizeof(u32)) { - mutex_unlock(&sde_dbg_base.mutex); - return -EFAULT; + len = -EFAULT; + goto end; } ptr = dbg->base + dbg->off; @@ -6092,9 +6121,9 @@ static ssize_t sde_dbg_reg_base_reg_read(struct file *file, rc = pm_runtime_get_sync(sde_dbg_base.dev); if (rc < 0) { - mutex_unlock(&sde_dbg_base.mutex); pr_err("failed to enable power %d\n", rc); - return rc; + len = rc; + goto end; } for (cnt = dbg->cnt; cnt > 0; cnt -= ROW_BYTES) { @@ -6119,18 +6148,20 @@ static ssize_t sde_dbg_reg_base_reg_read(struct file *file, } if (*ppos >= dbg->buf_len) { - mutex_unlock(&sde_dbg_base.mutex); - return 0; /* done reading */ + len = 0; /* done reading */ + goto end; } len = min(count, dbg->buf_len - (size_t) *ppos); if (copy_to_user(user_buf, dbg->buf + *ppos, len)) { - mutex_unlock(&sde_dbg_base.mutex); pr_err("failed to copy to user\n"); - return -EFAULT; + len = -EFAULT; + goto end; } *ppos += len; /* increase offset */ + +end: mutex_unlock(&sde_dbg_base.mutex); return len; @@ -6563,6 +6594,13 @@ void sde_dbg_reg_register_dump_range(const char *base_name, range->offset.start, range->offset.end); } +void sde_dbg_set_hw_ownership_status(bool enable) +{ + mutex_lock(&sde_dbg_base.mutex); + sde_dbg_base.hw_ownership = enable; + mutex_unlock(&sde_dbg_base.mutex); +} + void sde_dbg_set_sde_top_offset(u32 blk_off) { sde_dbg_base.dbgbus_sde.top_blk_off = blk_off; diff --git a/msm/sde_dbg.h b/msm/sde_dbg.h index a5f095938614..dd3922f6f9a6 100644 --- a/msm/sde_dbg.h +++ b/msm/sde_dbg.h @@ -340,6 +340,12 @@ int sde_dbg_dsi_ctrl_register(void __iomem *base, const char *name); */ void sde_dbg_set_sde_top_offset(u32 blk_off); +/** + * sde_dbg_set_hw_ownership_status - set the VM HW ownership status + * @enable: flag to control HW ownership status + */ +void sde_dbg_set_hw_ownership_status(bool enable); + /** * sde_evtlog_set_filter - update evtlog filtering * @evtlog: pointer to evtlog From 8f957517273698353d5b63a5c68eab9c39bc76f4 Mon Sep 17 00:00:00 2001 From: Veera Sundaram Sankaran Date: Tue, 8 Dec 2020 15:37:29 -0800 Subject: [PATCH 3/3] disp: msm: dsi: avoid debugfs HW access based on HW ownership Add VM ownership checks before accessing the HW through the debugfs path in dsi display/ctrl modules to avoid illegal access. Change-Id: Ia6e2ab0ef60d0f11e5945a63885d939db2ef78b0 Signed-off-by: Veera Sundaram Sankaran --- msm/dsi/dsi_display.c | 52 ++++++++++++++++++++++++++++++++++++++++--- msm/dsi/dsi_display.h | 2 ++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/msm/dsi/dsi_display.c b/msm/dsi/dsi_display.c index 22c9c21ba296..af405275e11f 100644 --- a/msm/dsi/dsi_display.c +++ b/msm/dsi/dsi_display.c @@ -1383,6 +1383,14 @@ static ssize_t debugfs_misr_setup(struct file *file, display->misr_frame_count = frame_count; mutex_lock(&display->display_lock); + + if (!display->hw_ownership) { + DSI_DEBUG("[%s] op not supported due to HW unavailability\n", + display->name); + rc = -EOPNOTSUPP; + goto unlock; + } + rc = dsi_display_clk_ctrl(display->dsi_clk_handle, DSI_CORE_CLK, DSI_CLK_ON); if (rc) { @@ -1434,6 +1442,14 @@ static ssize_t debugfs_misr_read(struct file *file, return -ENOMEM; mutex_lock(&display->display_lock); + + if (!display->hw_ownership) { + DSI_DEBUG("[%s] op not supported due to HW unavailability\n", + display->name); + rc = -EOPNOTSUPP; + goto error; + } + rc = dsi_display_clk_ctrl(display->dsi_clk_handle, DSI_CORE_CLK, DSI_CLK_ON); if (rc) { @@ -1531,6 +1547,15 @@ static ssize_t debugfs_esd_trigger_check(struct file *file, display->esd_trigger = esd_trigger; + mutex_lock(&display->display_lock); + + if (!display->hw_ownership) { + DSI_DEBUG("[%s] op not supported due to HW unavailability\n", + display->name); + rc = -EOPNOTSUPP; + goto unlock; + } + if (display->esd_trigger) { DSI_INFO("ESD attack triggered by user\n"); rc = dsi_panel_trigger_esd_attack(display->panel, @@ -1542,6 +1567,8 @@ static ssize_t debugfs_esd_trigger_check(struct file *file, } rc = len; +unlock: + mutex_unlock(&display->display_lock); error: kfree(buf); return rc; @@ -4159,10 +4186,13 @@ static int dsi_display_res_init(struct dsi_display *display) * In trusted vm, the connectors will not be enabled * until the HW resources are assigned and accepted. */ - if (display->trusted_vm_env) + if (display->trusted_vm_env) { display->is_active = false; - else + display->hw_ownership = false; + } else { display->is_active = true; + display->hw_ownership = true; + } return 0; error_ctrl_put: @@ -5405,19 +5435,33 @@ end: static int dsi_display_pre_release(void *data) { + struct dsi_display *display; + if (!data) return -EINVAL; - dsi_display_ctrl_irq_update((struct dsi_display *)data, false); + display = (struct dsi_display *)data; + mutex_lock(&display->display_lock); + display->hw_ownership = false; + mutex_unlock(&display->display_lock); + + dsi_display_ctrl_irq_update(display, false); return 0; } static int dsi_display_pre_acquire(void *data) { + struct dsi_display *display; + if (!data) return -EINVAL; + display = (struct dsi_display *)data; + mutex_lock(&display->display_lock); + display->hw_ownership = true; + mutex_unlock(&display->display_lock); + dsi_display_ctrl_irq_update((struct dsi_display *)data, true); return 0; @@ -7651,6 +7695,7 @@ int dsi_display_prepare(struct dsi_display *display) SDE_EVT32(SDE_EVTLOG_FUNC_ENTRY); mutex_lock(&display->display_lock); + display->hw_ownership = true; mode = display->panel->cur_mode; dsi_display_set_ctrl_esd_check_flag(display, false); @@ -8508,6 +8553,7 @@ int dsi_display_unprepare(struct dsi_display *display) DSI_ERR("[%s] panel post-unprepare failed, rc=%d\n", display->name, rc); } + display->hw_ownership = false; mutex_unlock(&display->display_lock); diff --git a/msm/dsi/dsi_display.h b/msm/dsi/dsi_display.h index 844690ae506c..6c89273a0670 100644 --- a/msm/dsi/dsi_display.h +++ b/msm/dsi/dsi_display.h @@ -192,6 +192,7 @@ struct dsi_display_ext_bridge { * @is_active: status of the display * @trusted_vm_env: Set to true, it the executing VM is Trusted VM. * Set to false, otherwise. + * @hw_ownership: Indicates if VM owns the hardware resources. * @tx_cmd_buf_ndx: Index to the DSI debugfs TX CMD buffer. * @cmd_set: Debugfs TX cmd set. * @enabled: Boolean to indicate display enabled. @@ -290,6 +291,7 @@ struct dsi_display { bool is_active; bool trusted_vm_env; + bool hw_ownership; int tx_cmd_buf_ndx; struct dsi_panel_cmd_set cmd_set;