crypto: ccree - fix PM race condition
authorGilad Ben-Yossef <gilad@benyossef.com>
Thu, 16 Jan 2020 10:14:43 +0000 (12:14 +0200)
committerHerbert Xu <herbert@gondor.apana.org.au>
Wed, 22 Jan 2020 08:21:10 +0000 (16:21 +0800)
The PM code was racy, possibly causing the driver to submit
requests to a powered down device. Fix the race and while
at it simplify the PM code.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Fixes: 1358c13a48c4 ("crypto: ccree - fix resume race condition on init")
Cc: stable@kernel.org # v4.20
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
drivers/crypto/ccree/cc_driver.h
drivers/crypto/ccree/cc_pm.c
drivers/crypto/ccree/cc_request_mgr.c
drivers/crypto/ccree/cc_request_mgr.h

index 7b6b5d6..9d77cfd 100644 (file)
@@ -160,6 +160,7 @@ struct cc_drvdata {
        int std_bodies;
        bool sec_disabled;
        u32 comp_mask;
+       bool pm_on;
 };
 
 struct cc_crypto_alg {
index 79c6121..ee9e9cb 100644 (file)
@@ -22,14 +22,8 @@ const struct dev_pm_ops ccree_pm = {
 int cc_pm_suspend(struct device *dev)
 {
        struct cc_drvdata *drvdata = dev_get_drvdata(dev);
-       int rc;
 
        dev_dbg(dev, "set HOST_POWER_DOWN_EN\n");
-       rc = cc_suspend_req_queue(drvdata);
-       if (rc) {
-               dev_err(dev, "cc_suspend_req_queue (%x)\n", rc);
-               return rc;
-       }
        fini_cc_regs(drvdata);
        cc_iowrite(drvdata, CC_REG(HOST_POWER_DOWN_EN), POWER_DOWN_ENABLE);
        cc_clk_off(drvdata);
@@ -63,13 +57,6 @@ int cc_pm_resume(struct device *dev)
        /* check if tee fips error occurred during power down */
        cc_tee_handle_fips_error(drvdata);
 
-       rc = cc_resume_req_queue(drvdata);
-       if (rc) {
-               dev_err(dev, "cc_resume_req_queue (%x)\n", rc);
-               return rc;
-       }
-
-       /* must be after the queue resuming as it uses the HW queue*/
        cc_init_hash_sram(drvdata);
 
        return 0;
@@ -80,10 +67,8 @@ int cc_pm_get(struct device *dev)
        int rc = 0;
        struct cc_drvdata *drvdata = dev_get_drvdata(dev);
 
-       if (cc_req_queue_suspended(drvdata))
+       if (drvdata->pm_on)
                rc = pm_runtime_get_sync(dev);
-       else
-               pm_runtime_get_noresume(dev);
 
        return (rc == 1 ? 0 : rc);
 }
@@ -93,14 +78,11 @@ int cc_pm_put_suspend(struct device *dev)
        int rc = 0;
        struct cc_drvdata *drvdata = dev_get_drvdata(dev);
 
-       if (!cc_req_queue_suspended(drvdata)) {
+       if (drvdata->pm_on) {
                pm_runtime_mark_last_busy(dev);
                rc = pm_runtime_put_autosuspend(dev);
-       } else {
-               /* Something wrong happens*/
-               dev_err(dev, "request to suspend already suspended queue");
-               rc = -EBUSY;
        }
+
        return rc;
 }
 
@@ -117,7 +99,7 @@ int cc_pm_init(struct cc_drvdata *drvdata)
        /* must be before the enabling to avoid redundant suspending */
        pm_runtime_set_autosuspend_delay(dev, CC_SUSPEND_TIMEOUT);
        pm_runtime_use_autosuspend(dev);
-       /* activate the PM module */
+       /* set us as active - note we won't do PM ops until cc_pm_go()! */
        return pm_runtime_set_active(dev);
 }
 
@@ -125,9 +107,11 @@ int cc_pm_init(struct cc_drvdata *drvdata)
 void cc_pm_go(struct cc_drvdata *drvdata)
 {
        pm_runtime_enable(drvdata_to_dev(drvdata));
+       drvdata->pm_on = true;
 }
 
 void cc_pm_fini(struct cc_drvdata *drvdata)
 {
        pm_runtime_disable(drvdata_to_dev(drvdata));
+       drvdata->pm_on = false;
 }
index ce09c43..9d61e6f 100644 (file)
@@ -41,7 +41,6 @@ struct cc_req_mgr_handle {
 #else
        struct tasklet_struct comptask;
 #endif
-       bool is_runtime_suspended;
 };
 
 struct cc_bl_item {
@@ -664,52 +663,3 @@ static void comp_handler(unsigned long devarg)
        cc_proc_backlog(drvdata);
        dev_dbg(dev, "Comp. handler done.\n");
 }
-
-/*
- * resume the queue configuration - no need to take the lock as this happens
- * inside the spin lock protection
- */
-#if defined(CONFIG_PM)
-int cc_resume_req_queue(struct cc_drvdata *drvdata)
-{
-       struct cc_req_mgr_handle *request_mgr_handle =
-               drvdata->request_mgr_handle;
-
-       spin_lock_bh(&request_mgr_handle->hw_lock);
-       request_mgr_handle->is_runtime_suspended = false;
-       spin_unlock_bh(&request_mgr_handle->hw_lock);
-
-       return 0;
-}
-
-/*
- * suspend the queue configuration. Since it is used for the runtime suspend
- * only verify that the queue can be suspended.
- */
-int cc_suspend_req_queue(struct cc_drvdata *drvdata)
-{
-       struct cc_req_mgr_handle *request_mgr_handle =
-                                               drvdata->request_mgr_handle;
-
-       /* lock the send_request */
-       spin_lock_bh(&request_mgr_handle->hw_lock);
-       if (request_mgr_handle->req_queue_head !=
-           request_mgr_handle->req_queue_tail) {
-               spin_unlock_bh(&request_mgr_handle->hw_lock);
-               return -EBUSY;
-       }
-       request_mgr_handle->is_runtime_suspended = true;
-       spin_unlock_bh(&request_mgr_handle->hw_lock);
-
-       return 0;
-}
-
-bool cc_req_queue_suspended(struct cc_drvdata *drvdata)
-{
-       struct cc_req_mgr_handle *request_mgr_handle =
-                                               drvdata->request_mgr_handle;
-
-       return  request_mgr_handle->is_runtime_suspended;
-}
-
-#endif
index f46cf76..ff7746a 100644 (file)
@@ -40,12 +40,4 @@ void complete_request(struct cc_drvdata *drvdata);
 
 void cc_req_mgr_fini(struct cc_drvdata *drvdata);
 
-#if defined(CONFIG_PM)
-int cc_resume_req_queue(struct cc_drvdata *drvdata);
-
-int cc_suspend_req_queue(struct cc_drvdata *drvdata);
-
-bool cc_req_queue_suspended(struct cc_drvdata *drvdata);
-#endif
-
 #endif /*__REQUEST_MGR_H__*/