From c427eba1c51034235eae31585d591eb9e55715c6 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 16 Dec 2019 18:01:41 -0800 Subject: [PATCH 1/7] mhi: core: Dump more logs when invalid cookie is received Dump event ring element, cookie value and bug_ring length when invalid cookie is received. Change-Id: I831f10f6e894326b17cb3a405b498cae8111b11d Signed-off-by: Hemant Kumar --- drivers/bus/mhi/core/mhi_main.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index 0cd40bf99786..1d3bedd6d577 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -1047,7 +1047,12 @@ static int parse_rsc_event(struct mhi_controller *mhi_cntrl, xfer_len = MHI_TRE_GET_EV_LEN(event); /* received out of bound cookie */ - MHI_ASSERT(cookie >= buf_ring->len, "Invalid Cookie\n"); + if (cookie >= buf_ring->len) { + MHI_ERR("cookie 0x%08x bufring_len %zu", cookie, buf_ring->len); + MHI_ERR("Processing Event:0x%llx 0x%08x 0x%08x\n", + event->ptr, event->dword[0], event->dword[1]); + panic("invalid cookie"); + } buf_info = buf_ring->base + cookie; From e25d6582a3182c1151025155afcef404ac761f9a Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 6 Jan 2020 13:48:46 -0800 Subject: [PATCH 2/7] mhi: core: Read transfer length from an event properly When MHI Driver receives an EOT event, it reads xfer_len from the event in the last TRE. The value is under control of the MHI device and never validated by Host MHI driver. The value should never be larger than the real size of the buffer but a malicious device can set the value 0xFFFF as maximum. This causes device to memory overflow (both read or write). Fix this issue by reading minimum of transfer length from event and the buffer length provided. Change-Id: I1ff21ed504acc901ec334b402362915ca2a7d4c4 Signed-off-by: Hemant Kumar --- drivers/bus/mhi/core/mhi_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index 1d3bedd6d577..775bd523cc9e 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2018-2019, The Linux Foundation. All rights reserved. */ +/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved. */ #include #include @@ -966,7 +966,8 @@ static int parse_xfer_event(struct mhi_controller *mhi_cntrl, mhi_cntrl->unmap_single(mhi_cntrl, buf_info); result.buf_addr = buf_info->cb_buf; - result.bytes_xferd = xfer_len; + result.bytes_xferd = min_t(u16, xfer_len, + buf_info->len); mhi_del_ring_element(mhi_cntrl, buf_ring); mhi_del_ring_element(mhi_cntrl, tre_ring); local_rp = tre_ring->rp; From 8edb589abf5c5221df0491fc7450ce0da89d0eb0 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Tue, 14 Jan 2020 17:18:55 -0800 Subject: [PATCH 3/7] mhi: core: Treat MHI_ASSERT as fatal error Currently MHI_ASSERT causes kernel panic in debug mode. In performance mode it just prints warning and continues. This can cause memory over-read, out of bound memory read. All the places driver is asserting need to result into a kernel panic to root cause the issue and prevent unwanted memory access. Change-Id: Ica8e1fec7be916621398cd4c5f8bfa6718c33d72 Signed-off-by: Hemant Kumar --- drivers/bus/mhi/core/mhi_internal.h | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/drivers/bus/mhi/core/mhi_internal.h b/drivers/bus/mhi/core/mhi_internal.h index da76271371c3..0c50be382507 100644 --- a/drivers/bus/mhi/core/mhi_internal.h +++ b/drivers/bus/mhi/core/mhi_internal.h @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -/* Copyright (c) 2018-2019, The Linux Foundation. All rights reserved. */ +/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved. */ #include @@ -919,22 +919,9 @@ irqreturn_t mhi_intvec_threaded_handlr(int irq_number, void *dev); irqreturn_t mhi_intvec_handlr(int irq_number, void *dev); void mhi_ev_task(unsigned long data); -#ifdef CONFIG_MHI_DEBUG - -#define MHI_ASSERT(cond, msg) do { \ +#define MHI_ASSERT(cond, fmt, ...) do { \ if (cond) \ - panic(msg); \ + panic(fmt); \ } while (0) -#else - -#define MHI_ASSERT(cond, msg) do { \ - if (cond) { \ - MHI_ERR(msg); \ - WARN_ON(cond); \ - } \ -} while (0) - -#endif - #endif /* _MHI_INT_H */ From 52aee5bca771eb6e48a255c5ae2626cc0146f525 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Fri, 20 Sep 2019 19:23:27 -0700 Subject: [PATCH 4/7] mhi: core: Add write_reg call back for mhi controller This allows to make a decision if different write call back needs to be called. Change-Id: I888da16e15e30ac1a7cb58d9272d6041b4d30ec7 Signed-off-by: Hemant Kumar --- drivers/bus/mhi/core/mhi_boot.c | 27 +++++++++++++++------------ drivers/bus/mhi/core/mhi_init.c | 8 +++++--- drivers/bus/mhi/core/mhi_main.c | 8 ++++---- drivers/bus/mhi/core/mhi_pm.c | 4 ++-- include/linux/mhi.h | 2 ++ 5 files changed, 28 insertions(+), 21 deletions(-) diff --git a/drivers/bus/mhi/core/mhi_boot.c b/drivers/bus/mhi/core/mhi_boot.c index 816f1d9d1aaf..4601ecb0c988 100644 --- a/drivers/bus/mhi/core/mhi_boot.c +++ b/drivers/bus/mhi/core/mhi_boot.c @@ -158,13 +158,14 @@ void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl, MHI_LOG("BHIe programming for RDDM\n"); - mhi_write_reg(mhi_cntrl, base, BHIE_RXVECADDR_HIGH_OFFS, + mhi_cntrl->write_reg(mhi_cntrl, base, BHIE_RXVECADDR_HIGH_OFFS, upper_32_bits(mhi_buf->dma_addr)); - mhi_write_reg(mhi_cntrl, base, BHIE_RXVECADDR_LOW_OFFS, + mhi_cntrl->write_reg(mhi_cntrl, base, BHIE_RXVECADDR_LOW_OFFS, lower_32_bits(mhi_buf->dma_addr)); - mhi_write_reg(mhi_cntrl, base, BHIE_RXVECSIZE_OFFS, mhi_buf->len); + mhi_cntrl->write_reg(mhi_cntrl, base, BHIE_RXVECSIZE_OFFS, + mhi_buf->len); sequence_id = prandom_u32() & BHIE_RXVECSTATUS_SEQNUM_BMSK; if (unlikely(!sequence_id)) @@ -234,7 +235,7 @@ static int __mhi_download_rddm_in_panic(struct mhi_controller *mhi_cntrl) /* Hardware reset; force device to enter rddm */ MHI_LOG( "Did not enter RDDM, do a host req. reset\n"); - mhi_write_reg(mhi_cntrl, mhi_cntrl->regs, + mhi_cntrl->write_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET, MHI_SOC_RESET_REQ); udelay(delayus); @@ -310,13 +311,14 @@ static int mhi_fw_load_amss(struct mhi_controller *mhi_cntrl, MHI_LOG("Starting BHIe Programming\n"); - mhi_write_reg(mhi_cntrl, base, BHIE_TXVECADDR_HIGH_OFFS, + mhi_cntrl->write_reg(mhi_cntrl, base, BHIE_TXVECADDR_HIGH_OFFS, upper_32_bits(mhi_buf->dma_addr)); - mhi_write_reg(mhi_cntrl, base, BHIE_TXVECADDR_LOW_OFFS, + mhi_cntrl->write_reg(mhi_cntrl, base, BHIE_TXVECADDR_LOW_OFFS, lower_32_bits(mhi_buf->dma_addr)); - mhi_write_reg(mhi_cntrl, base, BHIE_TXVECSIZE_OFFS, mhi_buf->len); + mhi_cntrl->write_reg(mhi_cntrl, base, BHIE_TXVECSIZE_OFFS, + mhi_buf->len); mhi_cntrl->sequence_id = prandom_u32() & BHIE_TXVECSTATUS_SEQNUM_BMSK; mhi_write_reg_field(mhi_cntrl, base, BHIE_TXVECDB_OFFS, @@ -374,14 +376,15 @@ static int mhi_fw_load_sbl(struct mhi_controller *mhi_cntrl, goto invalid_pm_state; } - mhi_write_reg(mhi_cntrl, base, BHI_STATUS, 0); - mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH, + mhi_cntrl->write_reg(mhi_cntrl, base, BHI_STATUS, 0); + mhi_cntrl->write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH, upper_32_bits(dma_addr)); - mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_LOW, + mhi_cntrl->write_reg(mhi_cntrl, base, BHI_IMGADDR_LOW, lower_32_bits(dma_addr)); - mhi_write_reg(mhi_cntrl, base, BHI_IMGSIZE, size); + mhi_cntrl->write_reg(mhi_cntrl, base, BHI_IMGSIZE, size); mhi_cntrl->session_id = prandom_u32() & BHI_TXDB_SEQNUM_BMSK; - mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, mhi_cntrl->session_id); + mhi_cntrl->write_reg(mhi_cntrl, base, BHI_IMGTXDB, + mhi_cntrl->session_id); read_unlock_bh(pm_lock); MHI_LOG("Waiting for image transfer completion\n"); diff --git a/drivers/bus/mhi/core/mhi_init.c b/drivers/bus/mhi/core/mhi_init.c index 8e3a5861f4ae..21a7043cef4b 100644 --- a/drivers/bus/mhi/core/mhi_init.c +++ b/drivers/bus/mhi/core/mhi_init.c @@ -684,7 +684,7 @@ static int mhi_init_bw_scale(struct mhi_controller *mhi_cntrl) MHI_LOG("BW_CFG OFFSET:0x%x\n", bw_cfg_offset); /* advertise host support */ - mhi_write_reg(mhi_cntrl, mhi_cntrl->regs, bw_cfg_offset, + mhi_cntrl->write_reg(mhi_cntrl, mhi_cntrl->regs, bw_cfg_offset, MHI_BW_SCALE_SETUP(er_index)); return 0; @@ -782,8 +782,8 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl) /* setup wake db */ mhi_cntrl->wake_db = base + val + (8 * MHI_DEV_WAKE_DB); - mhi_write_reg(mhi_cntrl, mhi_cntrl->wake_db, 4, 0); - mhi_write_reg(mhi_cntrl, mhi_cntrl->wake_db, 0, 0); + mhi_cntrl->write_reg(mhi_cntrl, mhi_cntrl->wake_db, 4, 0); + mhi_cntrl->write_reg(mhi_cntrl, mhi_cntrl->wake_db, 0, 0); mhi_cntrl->wake_set = false; /* setup bw scale db */ @@ -1405,6 +1405,8 @@ int of_register_mhi_controller(struct mhi_controller *mhi_cntrl) mhi_cntrl->unmap_single = mhi_unmap_single_no_bb; } + mhi_cntrl->write_reg = mhi_write_reg; + /* read the device info if possible */ if (mhi_cntrl->regs) { ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index 775bd523cc9e..cd2c2225bf8f 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -113,15 +113,15 @@ void mhi_write_reg_field(struct mhi_controller *mhi_cntrl, tmp &= ~mask; tmp |= (val << shift); - mhi_write_reg(mhi_cntrl, base, offset, tmp); + mhi_cntrl->write_reg(mhi_cntrl, base, offset, tmp); } void mhi_write_db(struct mhi_controller *mhi_cntrl, void __iomem *db_addr, dma_addr_t wp) { - mhi_write_reg(mhi_cntrl, db_addr, 4, upper_32_bits(wp)); - mhi_write_reg(mhi_cntrl, db_addr, 0, lower_32_bits(wp)); + mhi_cntrl->write_reg(mhi_cntrl, db_addr, 4, upper_32_bits(wp)); + mhi_cntrl->write_reg(mhi_cntrl, db_addr, 0, lower_32_bits(wp)); } void mhi_db_brstmode(struct mhi_controller *mhi_cntrl, @@ -1473,7 +1473,7 @@ int mhi_process_bw_scale_ev_ring(struct mhi_controller *mhi_cntrl, read_lock_bh(&mhi_cntrl->pm_lock); if (likely(MHI_DB_ACCESS_VALID(mhi_cntrl))) - mhi_write_reg(mhi_cntrl, mhi_cntrl->bw_scale_db, 0, + mhi_cntrl->write_reg(mhi_cntrl, mhi_cntrl->bw_scale_db, 0, MHI_BW_SCALE_RESULT(result, link_info.sequence_num)); diff --git a/drivers/bus/mhi/core/mhi_pm.c b/drivers/bus/mhi/core/mhi_pm.c index a7c297c8d0b6..7d1e33a4e469 100644 --- a/drivers/bus/mhi/core/mhi_pm.c +++ b/drivers/bus/mhi/core/mhi_pm.c @@ -609,7 +609,7 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl, * device cleares INTVEC as part of RESET processing, * re-program it */ - mhi_write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0); + mhi_cntrl->write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0); } MHI_LOG("Waiting for all pending event ring processing to complete\n"); @@ -932,7 +932,7 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl) mhi_cntrl->bhie = mhi_cntrl->regs + val; } - mhi_write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0); + mhi_cntrl->write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0); mhi_cntrl->pm_state = MHI_PM_POR; mhi_cntrl->ee = MHI_EE_MAX; current_ee = mhi_get_exec_env(mhi_cntrl); diff --git a/include/linux/mhi.h b/include/linux/mhi.h index 1537bdadf45c..a5abc090414c 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -352,6 +352,8 @@ struct mhi_controller { void (*tsync_log)(struct mhi_controller *mhi_cntrl, u64 remote_time); int (*bw_scale)(struct mhi_controller *mhi_cntrl, struct mhi_link_info *link_info); + void (*write_reg)(struct mhi_controller *mhi_cntrl, void __iomem *base, + u32 offset, u32 val); /* channel to control DTR messaging */ struct mhi_device *dtr_dev; From d167ac3bf61802e70f7294547e447414b606e0a5 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Tue, 24 Sep 2019 19:24:27 -0700 Subject: [PATCH 5/7] mhi: core: Add support to offload MHI register write to worker thread When PCIe endpoint enters L1SS sleep and mhi client on Host tries to queue a transfer request endpoint takes more than 6ms to come back to L0 state. This can cause CPU stall if MHI register write is followed by a write memory barrier. This can cause other tasks to get blocked. In order to prevent this add register write offload API mhi_write_reg_offload() which would queue the write and handle write request from worker thread in AMSS execution environment. Change-Id: I0a8b06d2ba96d9beb32fa31564aa0cbb26c885e6 Signed-off-by: Hemant Kumar --- drivers/bus/mhi/core/mhi_internal.h | 4 +++ drivers/bus/mhi/core/mhi_main.c | 39 +++++++++++++++++++++++++++++ drivers/bus/mhi/core/mhi_pm.c | 20 +++++++++++++-- include/linux/mhi.h | 22 ++++++++++++++++ 4 files changed, 83 insertions(+), 2 deletions(-) diff --git a/drivers/bus/mhi/core/mhi_internal.h b/drivers/bus/mhi/core/mhi_internal.h index 0c50be382507..abc5b1de1b0e 100644 --- a/drivers/bus/mhi/core/mhi_internal.h +++ b/drivers/bus/mhi/core/mhi_internal.h @@ -819,6 +819,8 @@ void mhi_destroy_timesync(struct mhi_controller *mhi_cntrl); int mhi_create_sysfs(struct mhi_controller *mhi_cntrl); void mhi_destroy_sysfs(struct mhi_controller *mhi_cntrl); int mhi_early_notify_device(struct device *dev, void *data); +void mhi_write_reg_offload(struct mhi_controller *mhi_cntrl, + void __iomem *base, u32 offset, u32 val); /* timesync log support */ static inline void mhi_timesync_log(struct mhi_controller *mhi_cntrl) @@ -912,6 +914,8 @@ void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl, struct image_info *img_info); int mhi_prepare_channel(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan); +void mhi_reset_reg_write_q(struct mhi_controller *mhi_cntrl); +void mhi_force_reg_write(struct mhi_controller *mhi_cntrl); /* isr handlers */ irqreturn_t mhi_msi_handlr(int irq_number, void *dev); diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index cd2c2225bf8f..cd792cff0a4a 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -89,6 +89,45 @@ int mhi_get_capability_offset(struct mhi_controller *mhi_cntrl, return -ENXIO; } +void mhi_force_reg_write(struct mhi_controller *mhi_cntrl) +{ + if (mhi_cntrl->offload_wq) + flush_work(&mhi_cntrl->reg_write_work); +} + +void mhi_reset_reg_write_q(struct mhi_controller *mhi_cntrl) +{ + cancel_work_sync(&mhi_cntrl->reg_write_work); + memset(mhi_cntrl->reg_write_q, 0, + sizeof(struct reg_write_info) * REG_WRITE_QUEUE_LEN); + mhi_cntrl->read_idx = 0; + atomic_set(&mhi_cntrl->write_idx, -1); +} + +static void mhi_reg_write_enqueue(struct mhi_controller *mhi_cntrl, + void __iomem *reg_addr, u32 val) +{ + u32 q_index = atomic_inc_return(&mhi_cntrl->write_idx); + + q_index = q_index & (REG_WRITE_QUEUE_LEN - 1); + + MHI_ASSERT(mhi_cntrl->reg_write_q[q_index].valid, "queue full idx %d", + q_index); + + mhi_cntrl->reg_write_q[q_index].reg_addr = reg_addr; + mhi_cntrl->reg_write_q[q_index].val = val; + mhi_cntrl->reg_write_q[q_index].valid = true; +} + +void mhi_write_reg_offload(struct mhi_controller *mhi_cntrl, + void __iomem *base, + u32 offset, + u32 val) +{ + mhi_reg_write_enqueue(mhi_cntrl, base + offset, val); + queue_work(mhi_cntrl->offload_wq, &mhi_cntrl->reg_write_work); +} + void mhi_write_reg(struct mhi_controller *mhi_cntrl, void __iomem *base, u32 offset, diff --git a/drivers/bus/mhi/core/mhi_pm.c b/drivers/bus/mhi/core/mhi_pm.c index 7d1e33a4e469..666f1d2b6bc2 100644 --- a/drivers/bus/mhi/core/mhi_pm.c +++ b/drivers/bus/mhi/core/mhi_pm.c @@ -167,8 +167,8 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl, mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL, MHICTRL_RESET_MASK, MHICTRL_RESET_SHIFT, 1); } else { - mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL, - MHICTRL_MHISTATE_MASK, MHICTRL_MHISTATE_SHIFT, state); + mhi_cntrl->write_reg(mhi_cntrl, mhi_cntrl->regs, MHICTRL, + (state << MHICTRL_MHISTATE_SHIFT)); } } @@ -481,6 +481,12 @@ static int mhi_pm_mission_mode_transition(struct mhi_controller *mhi_cntrl) wake_up_all(&mhi_cntrl->state_event); + /* offload register write if supported */ + if (mhi_cntrl->offload_wq) { + mhi_reset_reg_write_q(mhi_cntrl); + mhi_cntrl->write_reg = mhi_write_reg_offload; + } + /* force MHI to be in M0 state before continuing */ ret = __mhi_device_get_sync(mhi_cntrl); if (ret) @@ -556,6 +562,12 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl, TO_MHI_STATE_STR(mhi_cntrl->dev_state), to_mhi_pm_state_str(transition_state)); + /* restore async write call back */ + mhi_cntrl->write_reg = mhi_write_reg; + + if (mhi_cntrl->offload_wq) + mhi_reset_reg_write_q(mhi_cntrl); + /* We must notify MHI control driver so it can clean up first */ if (transition_state == MHI_PM_SYS_ERR_PROCESS) mhi_cntrl->status_cb(mhi_cntrl, mhi_cntrl->priv_data, @@ -1001,6 +1013,8 @@ void mhi_control_error(struct mhi_controller *mhi_cntrl) goto exit_control_error; } + mhi_cntrl->dev_state = MHI_STATE_SYS_ERR; + /* notify waiters to bail out early since MHI has entered ERROR state */ wake_up_all(&mhi_cntrl->state_event); @@ -1444,6 +1458,8 @@ int __mhi_device_get_sync(struct mhi_controller *mhi_cntrl) mhi_trigger_resume(mhi_cntrl); read_unlock_bh(&mhi_cntrl->pm_lock); + mhi_force_reg_write(mhi_cntrl); + ret = wait_event_timeout(mhi_cntrl->state_event, mhi_cntrl->pm_state == MHI_PM_M0 || MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), diff --git a/include/linux/mhi.h b/include/linux/mhi.h index a5abc090414c..b11704dd06e3 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -13,6 +13,8 @@ struct bhi_vec_entry; struct mhi_timesync; struct mhi_buf_info; +#define REG_WRITE_QUEUE_LEN 1024 + /** * enum MHI_CB - MHI callback * @MHI_CB_IDLE: MHI entered idle state @@ -185,6 +187,19 @@ struct file_info { u32 rem_seg_len; }; +/** + * struct reg_write_info - offload reg write info + * @reg_addr - register address + * @val - value to be written to register + * @chan - channel number + * @valid - entry is valid or not + */ +struct reg_write_info { + void __iomem *reg_addr; + u32 val; + bool valid; +}; + /** * struct mhi_controller - Master controller structure for external modem * @dev: Device associated with this controller @@ -380,6 +395,13 @@ struct mhi_controller { void *log_buf; struct dentry *dentry; struct dentry *parent; + + /* for reg write offload */ + struct workqueue_struct *offload_wq; + struct work_struct reg_write_work; + struct reg_write_info *reg_write_q; + atomic_t write_idx; + u32 read_idx; }; /** From 34c9ed3cc4f39e9a1c7474a64e76a00334ee4d30 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 6 Jan 2020 16:37:19 -0800 Subject: [PATCH 6/7] mhi: core: Finish pending reg writes before entering suspend It is possible that the write offload worker gets a chance to run after PCIe link is handed off to DRV subsystem. DRV subsystem can enter L1SS sleep while work handler is trying to access MHI register. This results into NOC error. Fix this issue by flushing work before handing off PCIe link to DRV subsystem or link enters D3 cold. Change-Id: I83e33b3753b62bf13a7f50af52c951b7a09a32e7 Signed-off-by: Hemant Kumar --- drivers/bus/mhi/core/mhi_pm.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/bus/mhi/core/mhi_pm.c b/drivers/bus/mhi/core/mhi_pm.c index 666f1d2b6bc2..ec393a5eaea4 100644 --- a/drivers/bus/mhi/core/mhi_pm.c +++ b/drivers/bus/mhi/core/mhi_pm.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2018-2019, The Linux Foundation. All rights reserved. */ +/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved. */ #include #include @@ -1160,6 +1160,9 @@ int mhi_pm_suspend(struct mhi_controller *mhi_cntrl) write_unlock_irq(&mhi_cntrl->pm_lock); MHI_LOG("Wait for M3 completion\n"); + /* finish reg writes before D3 cold */ + mhi_force_reg_write(mhi_cntrl); + ret = wait_event_timeout(mhi_cntrl->state_event, mhi_cntrl->dev_state == MHI_STATE_M3 || MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state), @@ -1274,6 +1277,9 @@ int mhi_pm_fast_suspend(struct mhi_controller *mhi_cntrl, bool notify_client) mhi_cntrl->M3_FAST++; write_unlock_irq(&mhi_cntrl->pm_lock); + /* finish reg writes before DRV hand-off to avoid noc err */ + mhi_force_reg_write(mhi_cntrl); + /* now safe to check ctrl event ring */ tasklet_enable(&mhi_cntrl->mhi_event->task); mhi_msi_handlr(0, mhi_cntrl->mhi_event); From b99432af45235a1387b460ba55c918377d31cc7d Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Tue, 14 Jan 2020 17:35:10 -0800 Subject: [PATCH 7/7] mhi: core: Add range check for channel id received in event ring The mhi_process_cmd_completion function reads cmd channel id from cmd_pkt using MHI_TRE_GET_CHID, the value is under the control of MHI devices and can be any value between 0 and 255. However the max channel is defined in device tree file and it is usually smaller than 255. This can cause out of bound access to the channel array. Fix this by checking the channel id received in cmd ring against the max channel allowed on target. Change-Id: Ib6faf67c7eae67186b3a44e6b1612deff6bf05fa Signed-off-by: Hemant Kumar --- drivers/bus/mhi/core/mhi_main.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index cd792cff0a4a..ee0b38aecf43 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -1160,6 +1160,10 @@ static void mhi_process_cmd_completion(struct mhi_controller *mhi_cntrl, complete(&mhi_tsync->completion); } else { chan = MHI_TRE_GET_CMD_CHID(cmd_pkt); + if (chan >= mhi_cntrl->max_chan) { + MHI_ERR("invalid channel id %u\n", chan); + goto del_ring_el; + } mhi_chan = &mhi_cntrl->mhi_chan[chan]; write_lock_bh(&mhi_chan->lock); mhi_chan->ccs = MHI_TRE_GET_EV_CODE(tre); @@ -1167,6 +1171,7 @@ static void mhi_process_cmd_completion(struct mhi_controller *mhi_cntrl, write_unlock_bh(&mhi_chan->lock); } +del_ring_el: mhi_del_ring_element(mhi_cntrl, mhi_ring); }