From b101ff3a368379370e66fc28aad40f7817fa6210 Mon Sep 17 00:00:00 2001 From: Bhaumik Bhatt Date: Thu, 20 Feb 2020 15:03:38 -0800 Subject: [PATCH] mhi: core: block unbind from userspace till channel reset MHI clients can unbind driver from userspace while a channel is active with ongoing transfers. As we can only do host side clean-up of the channel without sending a reset channel command to the device, a kernel panic could occur due to incoming packets if we proceed. Wait until the client unprepares the channel or device goes to an error state. Change-Id: I400cbf799d96fb7bc05df4c568593e12e9dc41f1 Signed-off-by: Bhaumik Bhatt --- drivers/bus/mhi/core/mhi_init.c | 47 +++++++++++++++++++++++++++------ drivers/bus/mhi/core/mhi_main.c | 14 +++++++--- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/drivers/bus/mhi/core/mhi_init.c b/drivers/bus/mhi/core/mhi_init.c index 6f07329d19a5..8738f760cf25 100644 --- a/drivers/bus/mhi/core/mhi_init.c +++ b/drivers/bus/mhi/core/mhi_init.c @@ -1967,7 +1967,8 @@ static int mhi_driver_remove(struct device *dev) MHI_CH_STATE_DISABLED, MHI_CH_STATE_DISABLED }; - int dir; + int dir, ret; + bool interrupted = false; /* control device has no work to do */ if (mhi_dev->dev_type == MHI_CONTROLLER_TYPE) @@ -1975,11 +1976,11 @@ static int mhi_driver_remove(struct device *dev) MHI_LOG("Removing device for chan:%s\n", mhi_dev->chan_name); - /* reset both channels */ + /* move both channels to suspended state and disallow processing */ for (dir = 0; dir < 2; dir++) { mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan; - if (!mhi_chan) + if (!mhi_chan || mhi_chan->offload_ch) continue; /* wake all threads waiting for completion */ @@ -1988,15 +1989,45 @@ static int mhi_driver_remove(struct device *dev) complete_all(&mhi_chan->completion); write_unlock_irq(&mhi_chan->lock); - /* move channel state to disable, no more processing */ mutex_lock(&mhi_chan->mutex); + write_lock_irq(&mhi_chan->lock); + if (mhi_chan->ch_state != MHI_CH_STATE_DISABLED) { + ch_state[dir] = mhi_chan->ch_state; + mhi_chan->ch_state = MHI_CH_STATE_SUSPENDED; + } + write_unlock_irq(&mhi_chan->lock); + mutex_unlock(&mhi_chan->mutex); + } + + /* wait for each channel to close and reset both channels */ + for (dir = 0; dir < 2; dir++) { + mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan; + + if (!mhi_chan || mhi_chan->offload_ch) + continue; + + /* unbind request from userspace, wait for channel reset */ + if (!(mhi_cntrl->power_down || + MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) && + ch_state[dir] != MHI_CH_STATE_DISABLED && !interrupted) { + MHI_ERR("Channel %s busy, wait for it to be reset\n", + mhi_dev->chan_name); + ret = wait_event_interruptible(mhi_cntrl->state_event, + mhi_chan->ch_state == MHI_CH_STATE_DISABLED || + MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)); + if (unlikely(ret)) + interrupted = true; + } + + /* update channel state as an error can exit above wait */ + mutex_lock(&mhi_chan->mutex); + write_lock_irq(&mhi_chan->lock); ch_state[dir] = mhi_chan->ch_state; - mhi_chan->ch_state = MHI_CH_STATE_SUSPENDED; write_unlock_irq(&mhi_chan->lock); - /* reset the channel */ - if (!mhi_chan->offload_ch) + /* reset channel if it was left enabled */ + if (ch_state[dir] != MHI_CH_STATE_DISABLED) mhi_reset_chan(mhi_cntrl, mhi_chan); mutex_unlock(&mhi_chan->mutex); @@ -2014,7 +2045,7 @@ static int mhi_driver_remove(struct device *dev) mutex_lock(&mhi_chan->mutex); - if (ch_state[dir] == MHI_CH_STATE_ENABLED && + if (ch_state[dir] != MHI_CH_STATE_DISABLED && !mhi_chan->offload_ch) mhi_deinit_chan_ctxt(mhi_cntrl, mhi_chan); diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index b2c98d9890df..bdf9305c3ea2 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -2053,19 +2053,22 @@ static void __mhi_unprepare_channel(struct mhi_controller *mhi_cntrl, { int ret; bool in_mission_mode = false; + bool notify = false; MHI_LOG("Entered: unprepare channel:%d\n", mhi_chan->chan); /* no more processing events for this channel */ mutex_lock(&mhi_chan->mutex); write_lock_irq(&mhi_chan->lock); - if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED) { + if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED && + mhi_chan->ch_state != MHI_CH_STATE_SUSPENDED) { MHI_LOG("chan:%d is already disabled\n", mhi_chan->chan); write_unlock_irq(&mhi_chan->lock); mutex_unlock(&mhi_chan->mutex); return; } - + if (mhi_chan->ch_state == MHI_CH_STATE_SUSPENDED) + notify = true; mhi_chan->ch_state = MHI_CH_STATE_DISABLED; write_unlock_irq(&mhi_chan->lock); @@ -2106,6 +2109,10 @@ error_invalid_state: if (!mhi_chan->offload_ch) { mhi_reset_chan(mhi_cntrl, mhi_chan); mhi_deinit_chan_ctxt(mhi_cntrl, mhi_chan); + + /* notify waiters to proceed with unbinding channel */ + if (notify) + wake_up_all(&mhi_cntrl->state_event); } MHI_LOG("chan:%d successfully resetted\n", mhi_chan->chan); mutex_unlock(&mhi_chan->mutex); @@ -2376,7 +2383,8 @@ static int mhi_update_channel_state(struct mhi_controller *mhi_cntrl, mhi_chan->chan, cmd == MHI_CMD_START_CHAN ? "START" : "STOP"); /* if channel is not active state state do not allow to state change */ - if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED) { + if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED && + mhi_chan->ch_state != MHI_CH_STATE_SUSPENDED) { ret = -EINVAL; MHI_LOG("channel:%d is not in active state, ch_state%d\n", mhi_chan->chan, mhi_chan->ch_state);