wil6210: prevent device memory access while in reset or suspend
authorAhmad Masri <amasri@codeaurora.org>
Thu, 28 Feb 2019 09:34:44 +0000 (11:34 +0200)
committerKalle Valo <kvalo@codeaurora.org>
Wed, 3 Apr 2019 12:34:04 +0000 (15:34 +0300)
Accessing some of the memory of the device while the device is
resetting or suspending may cause unexpected error as the HW is still
not in a stable state. Prevent this access to guarantee successful
read/write memory operations.

Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
drivers/net/wireless/ath/wil6210/debugfs.c
drivers/net/wireless/ath/wil6210/main.c
drivers/net/wireless/ath/wil6210/pm.c
drivers/net/wireless/ath/wil6210/wil6210.h
drivers/net/wireless/ath/wil6210/wil_crash_dump.c

index 7ad4e53284390c0abbf1b070c58acb14ebae250c..27cb182ae423ebf130b5e1171d34874be020bab3 100644 (file)
@@ -258,6 +258,11 @@ static void wil_print_mbox_ring(struct seq_file *s, const char *prefix,
 
        wil_halp_vote(wil);
 
+       if (wil_mem_access_lock(wil)) {
+               wil_halp_unvote(wil);
+               return;
+       }
+
        wil_memcpy_fromio_32(&r, off, sizeof(r));
        wil_mbox_ring_le2cpus(&r);
        /*
@@ -323,6 +328,7 @@ static void wil_print_mbox_ring(struct seq_file *s, const char *prefix,
        }
  out:
        seq_puts(s, "}\n");
+       wil_mem_access_unlock(wil);
        wil_halp_unvote(wil);
 }
 
@@ -601,6 +607,12 @@ static int memread_show(struct seq_file *s, void *data)
        if (ret < 0)
                return ret;
 
+       ret = wil_mem_access_lock(wil);
+       if (ret) {
+               wil_pm_runtime_put(wil);
+               return ret;
+       }
+
        a = wmi_buffer(wil, cpu_to_le32(mem_addr));
 
        if (a)
@@ -608,6 +620,7 @@ static int memread_show(struct seq_file *s, void *data)
        else
                seq_printf(s, "[0x%08x] = INVALID\n", mem_addr);
 
+       wil_mem_access_unlock(wil);
        wil_pm_runtime_put(wil);
 
        return 0;
@@ -626,10 +639,6 @@ static ssize_t wil_read_file_ioblob(struct file *file, char __user *user_buf,
        size_t unaligned_bytes, aligned_count, ret;
        int rc;
 
-       if (test_bit(wil_status_suspending, wil_blob->wil->status) ||
-           test_bit(wil_status_suspended, wil_blob->wil->status))
-               return 0;
-
        if (pos < 0)
                return -EINVAL;
 
@@ -656,11 +665,19 @@ static ssize_t wil_read_file_ioblob(struct file *file, char __user *user_buf,
                return rc;
        }
 
+       rc = wil_mem_access_lock(wil);
+       if (rc) {
+               kfree(buf);
+               wil_pm_runtime_put(wil);
+               return rc;
+       }
+
        wil_memcpy_fromio_32(buf, (const void __iomem *)
                             wil_blob->blob.data + aligned_pos, aligned_count);
 
        ret = copy_to_user(user_buf, buf + unaligned_bytes, count);
 
+       wil_mem_access_unlock(wil);
        wil_pm_runtime_put(wil);
 
        kfree(buf);
index 277abfdf3322c5b8a59b08093a7a9219a9105514..6df19a2a6ce717fccef65c08515e326cdbaef939 100644 (file)
@@ -184,6 +184,28 @@ void wil_memcpy_toio_32(volatile void __iomem *dst, const void *src,
        }
 }
 
+/* Device memory access is prohibited while reset or suspend.
+ * wil_mem_access_lock protects accessing device memory in these cases
+ */
+int wil_mem_access_lock(struct wil6210_priv *wil)
+{
+       if (!down_read_trylock(&wil->mem_lock))
+               return -EBUSY;
+
+       if (test_bit(wil_status_suspending, wil->status) ||
+           test_bit(wil_status_suspended, wil->status)) {
+               up_read(&wil->mem_lock);
+               return -EBUSY;
+       }
+
+       return 0;
+}
+
+void wil_mem_access_unlock(struct wil6210_priv *wil)
+{
+       up_read(&wil->mem_lock);
+}
+
 static void wil_ring_fini_tx(struct wil6210_priv *wil, int id)
 {
        struct wil_ring *ring = &wil->ring_tx[id];
@@ -703,6 +725,7 @@ int wil_priv_init(struct wil6210_priv *wil)
        spin_lock_init(&wil->wmi_ev_lock);
        spin_lock_init(&wil->net_queue_lock);
        init_waitqueue_head(&wil->wq);
+       init_rwsem(&wil->mem_lock);
 
        wil->wmi_wq = create_singlethread_workqueue(WIL_NAME "_wmi");
        if (!wil->wmi_wq)
@@ -1599,15 +1622,6 @@ int wil_reset(struct wil6210_priv *wil, bool load_fw)
        }
 
        set_bit(wil_status_resetting, wil->status);
-       if (test_bit(wil_status_collecting_dumps, wil->status)) {
-               /* Device collects crash dump, cancel the reset.
-                * following crash dump collection, reset would take place.
-                */
-               wil_dbg_misc(wil, "reject reset while collecting crash dump\n");
-               rc = -EBUSY;
-               goto out;
-       }
-
        mutex_lock(&wil->vif_mutex);
        wil_abort_scan_all_vifs(wil, false);
        mutex_unlock(&wil->vif_mutex);
@@ -1782,7 +1796,9 @@ int __wil_up(struct wil6210_priv *wil)
 
        WARN_ON(!mutex_is_locked(&wil->mutex));
 
+       down_write(&wil->mem_lock);
        rc = wil_reset(wil, true);
+       up_write(&wil->mem_lock);
        if (rc)
                return rc;
 
@@ -1854,6 +1870,7 @@ int wil_up(struct wil6210_priv *wil)
 
 int __wil_down(struct wil6210_priv *wil)
 {
+       int rc;
        WARN_ON(!mutex_is_locked(&wil->mutex));
 
        set_bit(wil_status_resetting, wil->status);
@@ -1873,7 +1890,11 @@ int __wil_down(struct wil6210_priv *wil)
        wil_abort_scan_all_vifs(wil, false);
        mutex_unlock(&wil->vif_mutex);
 
-       return wil_reset(wil, false);
+       down_write(&wil->mem_lock);
+       rc = wil_reset(wil, false);
+       up_write(&wil->mem_lock);
+
+       return rc;
 }
 
 int wil_down(struct wil6210_priv *wil)
index 75fe9323547ca58d53475bd27bebf262be6e8e93..f307522fa26df0d02569f0780a99fa01fe64d90f 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2014,2017 Qualcomm Atheros, Inc.
- * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -195,14 +195,18 @@ static int wil_suspend_keep_radio_on(struct wil6210_priv *wil)
        wil_dbg_pm(wil, "suspend keep radio on\n");
 
        /* Prevent handling of new tx and wmi commands */
-       set_bit(wil_status_suspending, wil->status);
-       if (test_bit(wil_status_collecting_dumps, wil->status)) {
-               /* Device collects crash dump, cancel the suspend */
-               wil_dbg_pm(wil, "reject suspend while collecting crash dump\n");
-               clear_bit(wil_status_suspending, wil->status);
+       rc = down_write_trylock(&wil->mem_lock);
+       if (!rc) {
+               wil_err(wil,
+                       "device is busy. down_write_trylock failed, returned (0x%x)\n",
+                       rc);
                wil->suspend_stats.rejected_by_host++;
                return -EBUSY;
        }
+
+       set_bit(wil_status_suspending, wil->status);
+       up_write(&wil->mem_lock);
+
        wil_pm_stop_all_net_queues(wil);
 
        if (!wil_is_tx_idle(wil)) {
@@ -310,15 +314,18 @@ static int wil_suspend_radio_off(struct wil6210_priv *wil)
 
        wil_dbg_pm(wil, "suspend radio off\n");
 
-       set_bit(wil_status_suspending, wil->status);
-       if (test_bit(wil_status_collecting_dumps, wil->status)) {
-               /* Device collects crash dump, cancel the suspend */
-               wil_dbg_pm(wil, "reject suspend while collecting crash dump\n");
-               clear_bit(wil_status_suspending, wil->status);
+       rc = down_write_trylock(&wil->mem_lock);
+       if (!rc) {
+               wil_err(wil,
+                       "device is busy. down_write_trylock failed, returned (0x%x)\n",
+                       rc);
                wil->suspend_stats.rejected_by_host++;
                return -EBUSY;
        }
 
+       set_bit(wil_status_suspending, wil->status);
+       up_write(&wil->mem_lock);
+
        /* if netif up, hardware is alive, shut it down */
        mutex_lock(&wil->vif_mutex);
        active_ifaces = wil_has_active_ifaces(wil, true, false);
index e1b1039b13abd009297abf5e841d1c6f825e759b..c4176323aab5c11443a0402f6774eacd7beec8cf 100644 (file)
@@ -660,7 +660,6 @@ enum { /* for wil6210_priv.status */
        wil_status_suspending, /* suspend in progress */
        wil_status_suspended, /* suspend completed, device is suspended */
        wil_status_resuming, /* resume in progress */
-       wil_status_collecting_dumps, /* crashdump collection in progress */
        wil_status_last /* keep last */
 };
 
@@ -992,6 +991,8 @@ struct wil6210_priv {
        struct wil_txrx_ops txrx_ops;
 
        struct mutex mutex; /* for wil6210_priv access in wil_{up|down} */
+       /* for synchronizing device memory access while reset or suspend */
+       struct rw_semaphore mem_lock;
        /* statistics */
        atomic_t isr_count_rx, isr_count_tx;
        /* debugfs */
@@ -1176,6 +1177,8 @@ void wil_memcpy_fromio_32(void *dst, const volatile void __iomem *src,
                          size_t count);
 void wil_memcpy_toio_32(volatile void __iomem *dst, const void *src,
                        size_t count);
+int wil_mem_access_lock(struct wil6210_priv *wil);
+void wil_mem_access_unlock(struct wil6210_priv *wil);
 
 struct wil6210_vif *
 wil_vif_alloc(struct wil6210_priv *wil, const char *name,
index dc33a0b4c3fac55c67371aa7ef762a2c90320636..772cb00c20025fa50b5663cc3079edc84ee9a673 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2015,2017 Qualcomm Atheros, Inc.
- * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -57,7 +57,7 @@ static int wil_fw_get_crash_dump_bounds(struct wil6210_priv *wil,
 
 int wil_fw_copy_crash_dump(struct wil6210_priv *wil, void *dest, u32 size)
 {
-       int i;
+       int i, rc;
        const struct fw_map *map;
        void *data;
        u32 host_min, dump_size, offset, len;
@@ -73,14 +73,9 @@ int wil_fw_copy_crash_dump(struct wil6210_priv *wil, void *dest, u32 size)
                return -EINVAL;
        }
 
-       set_bit(wil_status_collecting_dumps, wil->status);
-       if (test_bit(wil_status_suspending, wil->status) ||
-           test_bit(wil_status_suspended, wil->status) ||
-           test_bit(wil_status_resetting, wil->status)) {
-               wil_err(wil, "cannot collect fw dump during suspend/reset\n");
-               clear_bit(wil_status_collecting_dumps, wil->status);
-               return -EINVAL;
-       }
+       rc = wil_mem_access_lock(wil);
+       if (rc)
+               return rc;
 
        /* copy to crash dump area */
        for (i = 0; i < ARRAY_SIZE(fw_mapping); i++) {
@@ -100,8 +95,7 @@ int wil_fw_copy_crash_dump(struct wil6210_priv *wil, void *dest, u32 size)
                wil_memcpy_fromio_32((void * __force)(dest + offset),
                                     (const void __iomem * __force)data, len);
        }
-
-       clear_bit(wil_status_collecting_dumps, wil->status);
+       wil_mem_access_unlock(wil);
 
        return 0;
 }