From d751a8d90f11bf22f52a3e95c84863b4780cf508 Mon Sep 17 00:00:00 2001 From: Andrey Markovytch Date: Thu, 19 Jan 2017 19:53:03 +0200 Subject: [PATCH] ufs: fixed bugs in ice related to key syncronization 1. Added reference count for requests in HW queue for particular key 2. Fixed race between block/unblock requests with asynchronous job for key configuration in ice Change-Id: Iaefc25739b420b2e5feae1895c7c2495b4850539 Signed-off-by: Andrey Markovytch --- drivers/scsi/ufs/ufs-qcom-ice.c | 80 +++++++++++++++++++++++++++++---- drivers/scsi/ufs/ufs-qcom.c | 4 +- drivers/scsi/ufs/ufs-qcom.h | 3 +- drivers/scsi/ufs/ufshcd.c | 11 ++--- security/pfe/pfk_kc.c | 50 ++++++++++++++++++--- 5 files changed, 125 insertions(+), 23 deletions(-) diff --git a/drivers/scsi/ufs/ufs-qcom-ice.c b/drivers/scsi/ufs/ufs-qcom-ice.c index 070d27df6b49..85f82b2251c1 100644 --- a/drivers/scsi/ufs/ufs-qcom-ice.c +++ b/drivers/scsi/ufs/ufs-qcom-ice.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2016, The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2017, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -14,6 +14,7 @@ #include #include #include +#include #include #include "ufs-qcom-ice.h" @@ -168,6 +169,7 @@ out: static void ufs_qcom_ice_cfg_work(struct work_struct *work) { + unsigned long flags; struct ice_data_setting ice_set; struct ufs_qcom_host *qcom_host = container_of(work, struct ufs_qcom_host, ice_cfg_work); @@ -185,12 +187,17 @@ static void ufs_qcom_ice_cfg_work(struct work_struct *work) qcom_host->ice.vops->config_start(qcom_host->ice.pdev, qcom_host->req_pending, &ice_set, false); + spin_lock_irqsave(&qcom_host->ice_work_lock, flags); + qcom_host->req_pending = NULL; + spin_unlock_irqrestore(&qcom_host->ice_work_lock, flags); + /* * Resume with requests processing. We assume config_start has been * successful, but even if it wasn't we still must resume in order to * allow for the request to be retried. */ ufshcd_scsi_unblock_requests(qcom_host->hba); + } /** @@ -246,6 +253,7 @@ int ufs_qcom_ice_req_setup(struct ufs_qcom_host *qcom_host, struct ice_data_setting ice_set; char cmd_op = cmd->cmnd[0]; int err; + unsigned long flags; if (!qcom_host->ice.pdev || !qcom_host->ice.vops) { dev_dbg(qcom_host->hba->dev, "%s: ice device is not enabled\n", @@ -272,14 +280,36 @@ int ufs_qcom_ice_req_setup(struct ufs_qcom_host *qcom_host, dev_dbg(qcom_host->hba->dev, "%s: scheduling task for ice setup\n", __func__); - qcom_host->req_pending = cmd->request; - if (schedule_work(&qcom_host->ice_cfg_work)) + + spin_lock_irqsave( + &qcom_host->ice_work_lock, flags); + + if (!qcom_host->req_pending) { ufshcd_scsi_block_requests( qcom_host->hba); + qcom_host->req_pending = cmd->request; + if (!schedule_work( + &qcom_host->ice_cfg_work)) { + qcom_host->req_pending = NULL; + + spin_unlock_irqrestore( + &qcom_host->ice_work_lock, + flags); + + ufshcd_scsi_unblock_requests( + qcom_host->hba); + return err; + } + } + + spin_unlock_irqrestore( + &qcom_host->ice_work_lock, flags); + } else { - dev_err(qcom_host->hba->dev, - "%s: error in ice_vops->config %d\n", - __func__, err); + if (err != -EBUSY) + dev_err(qcom_host->hba->dev, + "%s: error in ice_vops->config %d\n", + __func__, err); } return err; @@ -320,6 +350,7 @@ int ufs_qcom_ice_cfg_start(struct ufs_qcom_host *qcom_host, unsigned int bypass = 0; struct request *req; char cmd_op; + unsigned long flags; if (!qcom_host->ice.pdev || !qcom_host->ice.vops) { dev_dbg(dev, "%s: ice device is not enabled\n", __func__); @@ -365,12 +396,43 @@ int ufs_qcom_ice_cfg_start(struct ufs_qcom_host *qcom_host, * request processing. */ if (err == -EAGAIN) { - qcom_host->req_pending = req; - if (schedule_work(&qcom_host->ice_cfg_work)) + + dev_dbg(qcom_host->hba->dev, + "%s: scheduling task for ice setup\n", + __func__); + + spin_lock_irqsave( + &qcom_host->ice_work_lock, flags); + + if (!qcom_host->req_pending) { ufshcd_scsi_block_requests( + qcom_host->hba); + qcom_host->req_pending = cmd->request; + if (!schedule_work( + &qcom_host->ice_cfg_work)) { + qcom_host->req_pending = NULL; + + spin_unlock_irqrestore( + &qcom_host->ice_work_lock, + flags); + + ufshcd_scsi_unblock_requests( qcom_host->hba); + return err; + } + } + + spin_unlock_irqrestore( + &qcom_host->ice_work_lock, flags); + + } else { + if (err != -EBUSY) + dev_err(qcom_host->hba->dev, + "%s: error in ice_vops->config %d\n", + __func__, err); } - goto out; + + return err; } } diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index 2b0731b8358c..03b222d8be93 100644 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2016, Linux Foundation. All rights reserved. + * Copyright (c) 2013-2017, Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -1981,6 +1981,8 @@ static int ufs_qcom_init(struct ufs_hba *hba) /* Make a two way bind between the qcom host and the hba */ host->hba = hba; + spin_lock_init(&host->ice_work_lock); + ufshcd_set_variant(hba, host); err = ufs_qcom_ice_get_dev(host); diff --git a/drivers/scsi/ufs/ufs-qcom.h b/drivers/scsi/ufs/ufs-qcom.h index 394de8302fd2..74d8a7a30ad6 100644 --- a/drivers/scsi/ufs/ufs-qcom.h +++ b/drivers/scsi/ufs/ufs-qcom.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2013-2016, The Linux Foundation. All rights reserved. +/* Copyright (c) 2013-2017, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -370,6 +370,7 @@ struct ufs_qcom_host { u32 dbg_print_en; struct ufs_qcom_testbus testbus; + spinlock_t ice_work_lock; struct work_struct ice_cfg_work; struct request *req_pending; }; diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 6e88e4b11273..ae94d44e9a36 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -48,6 +48,7 @@ #include "ufshci.h" #include "ufs_quirks.h" #include "ufs-debugfs.h" +#include "ufs-qcom.h" #define CREATE_TRACE_POINTS #include @@ -2877,11 +2878,11 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) "%s: failed to compose upiu %d\n", __func__, err); - lrbp->cmd = NULL; - clear_bit_unlock(tag, &hba->lrb_in_use); - ufshcd_release_all(hba); - ufshcd_vops_pm_qos_req_end(hba, cmd->request, true); - goto out; + lrbp->cmd = NULL; + clear_bit_unlock(tag, &hba->lrb_in_use); + ufshcd_release_all(hba); + ufshcd_vops_pm_qos_req_end(hba, cmd->request, true); + goto out; } err = ufshcd_map_sg(lrbp); diff --git a/security/pfe/pfk_kc.c b/security/pfe/pfk_kc.c index 0869a862f521..cd3f08b3959d 100644 --- a/security/pfe/pfk_kc.c +++ b/security/pfe/pfk_kc.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015-2016, The Linux Foundation. All rights reserved. + * Copyright (c) 2015-2017, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -100,6 +100,9 @@ struct kc_entry { struct task_struct *thread_pending; enum pfk_kc_entry_state state; + + /* ref count for the number of requests in the HW queue for this key */ + int loaded_ref_cnt; int scm_error; }; @@ -520,6 +523,10 @@ int pfk_kc_load_key_start(const unsigned char *key, size_t key_size, if (entry_exists) { kc_update_timestamp(entry); entry->state = ACTIVE_ICE_LOADED; + + if (async) + entry->loaded_ref_cnt++; + break; } case (FREE): @@ -529,8 +536,17 @@ int pfk_kc_load_key_start(const unsigned char *key, size_t key_size, entry->scm_error = ret; pr_err("%s: key load error (%d)\n", __func__, ret); } else { - entry->state = ACTIVE_ICE_LOADED; kc_update_timestamp(entry); + entry->state = ACTIVE_ICE_LOADED; + + /* + * only increase ref cnt for async calls, + * sync calls from within work thread do not pass + * requests further to HW + */ + if (async) + entry->loaded_ref_cnt++; + } break; case (ACTIVE_ICE_PRELOAD): @@ -539,6 +555,10 @@ int pfk_kc_load_key_start(const unsigned char *key, size_t key_size, break; case (ACTIVE_ICE_LOADED): kc_update_timestamp(entry); + + if (async) + entry->loaded_ref_cnt++; + break; case(SCM_ERROR): ret = entry->scm_error; @@ -572,6 +592,8 @@ void pfk_kc_load_key_end(const unsigned char *key, size_t key_size, const unsigned char *salt, size_t salt_size) { struct kc_entry *entry = NULL; + struct task_struct *tmp_pending = NULL; + int ref_cnt = 0; if (!kc_is_ready()) return; @@ -591,14 +613,28 @@ void pfk_kc_load_key_end(const unsigned char *key, size_t key_size, if (!entry) { kc_spin_unlock(); pr_err("internal error, there should an entry to unlock\n"); + return; } - entry->state = INACTIVE; + ref_cnt = --entry->loaded_ref_cnt; - /* wake-up invalidation if it's waiting for the entry to be released */ - if (entry->thread_pending) { - wake_up_process(entry->thread_pending); - entry->thread_pending = NULL; + if (ref_cnt < 0) + pr_err("internal error, ref count should never be negative\n"); + + if (!ref_cnt) { + entry->state = INACTIVE; + /* + * wake-up invalidation if it's waiting + * for the entry to be released + */ + if (entry->thread_pending) { + tmp_pending = entry->thread_pending; + entry->thread_pending = NULL; + + kc_spin_unlock(); + wake_up_process(tmp_pending); + return; + } } kc_spin_unlock();