From 4f7c2e0d8765a0266b920c66ffc495fde44c1ec8 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Tue, 28 May 2019 18:56:20 +0300 Subject: thunderbolt: Make sure device runtime resume completes before taking domain lock When a device is authorized from userspace by writing to authorized attribute we first take the domain lock and then runtime resume the device in question. There are two issues with this. First is that the device connected notifications are blocked during this time which means we get them only after the authorization operation is complete. Because of this the authorization needed flag from the firmware notification is not reflecting the real authorization status anymore. So what happens is that the "authorized" keeps returning 0 even if the device was already authorized properly. Second issue is that each time the controller is runtime resumed the connection_id field of device connected notification may be different than in the previous resume. We need to use the latest connection_id otherwise the firmware rejects the authorization command. Fix these by moving runtime resume operations to happen before the domain lock is taken, and waiting for the updated device connected notification from the firmware before we allow runtime resume of a device to complete. While there add missing locking to tb_switch_nvm_read(). Fixes: 09f11b6c99fe ("thunderbolt: Take domain lock in switch sysfs attribute callbacks") Reported-by: Pengfei Xu Signed-off-by: Mika Westerberg --- drivers/thunderbolt/switch.c | 45 ++++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-) (limited to 'drivers/thunderbolt/switch.c') diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index c1b016574fb4..10b56c66fec3 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -239,7 +239,16 @@ static int tb_switch_nvm_read(void *priv, unsigned int offset, void *val, int ret; pm_runtime_get_sync(&sw->dev); + + if (!mutex_trylock(&sw->tb->lock)) { + ret = restart_syscall(); + goto out; + } + ret = dma_port_flash_read(sw->dma_port, offset, val, bytes); + mutex_unlock(&sw->tb->lock); + +out: pm_runtime_mark_last_busy(&sw->dev); pm_runtime_put_autosuspend(&sw->dev); @@ -1019,7 +1028,6 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val) * the new tunnel too early. */ pci_lock_rescan_remove(); - pm_runtime_get_sync(&sw->dev); switch (val) { /* Approve switch */ @@ -1040,8 +1048,6 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val) break; } - pm_runtime_mark_last_busy(&sw->dev); - pm_runtime_put_autosuspend(&sw->dev); pci_unlock_rescan_remove(); if (!ret) { @@ -1069,7 +1075,10 @@ static ssize_t authorized_store(struct device *dev, if (val > 2) return -EINVAL; + pm_runtime_get_sync(&sw->dev); ret = tb_switch_set_authorized(sw, val); + pm_runtime_mark_last_busy(&sw->dev); + pm_runtime_put_autosuspend(&sw->dev); return ret ? ret : count; } @@ -1195,8 +1204,12 @@ static ssize_t nvm_authenticate_store(struct device *dev, bool val; int ret; - if (!mutex_trylock(&sw->tb->lock)) - return restart_syscall(); + pm_runtime_get_sync(&sw->dev); + + if (!mutex_trylock(&sw->tb->lock)) { + ret = restart_syscall(); + goto exit_rpm; + } /* If NVMem devices are not yet added */ if (!sw->nvm) { @@ -1217,13 +1230,9 @@ static ssize_t nvm_authenticate_store(struct device *dev, goto exit_unlock; } - pm_runtime_get_sync(&sw->dev); ret = nvm_validate_and_write(sw); - if (ret) { - pm_runtime_mark_last_busy(&sw->dev); - pm_runtime_put_autosuspend(&sw->dev); + if (ret) goto exit_unlock; - } sw->nvm->authenticating = true; @@ -1239,12 +1248,13 @@ static ssize_t nvm_authenticate_store(struct device *dev, } else { ret = nvm_authenticate_device(sw); } - pm_runtime_mark_last_busy(&sw->dev); - pm_runtime_put_autosuspend(&sw->dev); } exit_unlock: mutex_unlock(&sw->tb->lock); +exit_rpm: + pm_runtime_mark_last_busy(&sw->dev); + pm_runtime_put_autosuspend(&sw->dev); if (ret) return ret; @@ -1380,11 +1390,22 @@ static void tb_switch_release(struct device *dev) */ static int __maybe_unused tb_switch_runtime_suspend(struct device *dev) { + struct tb_switch *sw = tb_to_switch(dev); + const struct tb_cm_ops *cm_ops = sw->tb->cm_ops; + + if (cm_ops->runtime_suspend_switch) + return cm_ops->runtime_suspend_switch(sw); + return 0; } static int __maybe_unused tb_switch_runtime_resume(struct device *dev) { + struct tb_switch *sw = tb_to_switch(dev); + const struct tb_cm_ops *cm_ops = sw->tb->cm_ops; + + if (cm_ops->runtime_resume_switch) + return cm_ops->runtime_resume_switch(sw); return 0; } -- cgit v1.2.3