From patchwork Mon Jul 29 17:20:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Miri Korenblit X-Patchwork-Id: 815633 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4D206183098 for ; Mon, 29 Jul 2024 17:20:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.12 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722273646; cv=none; b=cM8aQEdriXTdj/nwf+pMQ2ifGfjT2r1nMbwv0bWkFt/O36aBGvRf0QFxY+Ijz0gscWzJqfmn3EYmFk4HJ+28pWg2SW1GwVDPzIUmBtEDDba6R/7Jljoc0KoIiQDAnxfNDS2LX/A8Ps29Z3Xzb19TfXLmvwP7rjpNMJ6cwM9X3QY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722273646; c=relaxed/simple; bh=9zOouA5pPojgBu5s6VMYGj4sd+DHsbsmo5Ucw3f8XGU=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=vC/iPyftvSFTcjppEHuRYdzXZdo3RtJIEN6+hiY/YvWAu0+KJ29tFSGuHvaxowbaJXP3xkjGabgim+9z6d9l8eOOrTow5fzw/NBpwYDSfz3gAhzPqy0L0lKAUA1HK/qr56iD3mVGuDKo/s8wtJ1yEkSS+61mbBCoHdw0f0OavPw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=D5BDRr+Y; arc=none smtp.client-ip=198.175.65.12 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="D5BDRr+Y" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1722273646; x=1753809646; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=9zOouA5pPojgBu5s6VMYGj4sd+DHsbsmo5Ucw3f8XGU=; b=D5BDRr+YyIsHoAyb/HYn7R+DWWxXZhVgVH8uQkDRCYlIualmScKUrr6n 1Hn/Lj/7NIhmOiacgs63VOPV5efq3gOl2YUzILxGE67ZrMwF3wwcZ3RMF CoSZkhkiig3Ql2agQYfK9xYrmN36+PoTizhbr5U7T5VDy3s8bPyaPTbzK 5tS0QpIgBgu/7FwYs1osduYSSjUC8a4wkQDtQZroHuTT85ep7k8sGJhh5 ln0T/F/HtPUbISZ5JAB+rwX/DZM0cxhFONgAzusf8ny9eHA88y1rpxxQ6 j5caL3gsvY8P2OO08/yHLF1peNlTGMi64kT4tkjk2pm8UWMo6Y+XtPvfS Q==; X-CSE-ConnectionGUID: /hXaxDC0RsycHBwBoC/KmQ== X-CSE-MsgGUID: T+fG37qAQniG/X6S7TrGaQ== X-IronPort-AV: E=McAfee;i="6700,10204,11148"; a="31445585" X-IronPort-AV: E=Sophos;i="6.09,246,1716274800"; d="scan'208";a="31445585" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by orvoesa104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Jul 2024 10:20:45 -0700 X-CSE-ConnectionGUID: NB4FqjaARoyUfEQd1z6xUw== X-CSE-MsgGUID: 7SLO6LPlTZS2Ecr7Po8FHA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,246,1716274800"; d="scan'208";a="54288311" Received: from weis0040.iil.intel.com ([10.12.217.108]) by orviesa006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Jul 2024 10:20:43 -0700 From: Miri Korenblit To: johannes@sipsolutions.net Cc: linux-wireless@vger.kernel.org, Ilan Peer , Johannes Berg Subject: [PATCH 09/17] wifi: iwlwifi: mvm: Fix a race in scan abort flow Date: Mon, 29 Jul 2024 20:20:10 +0300 Message-Id: <20240729201718.3a7acb9edce3.I3340556a222388504c6330b333360bf77d10f9e2@changeid> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240729172018.1993751-1-miriam.rachel.korenblit@intel.com> References: <20240729172018.1993751-1-miriam.rachel.korenblit@intel.com> Precedence: bulk X-Mailing-List: linux-wireless@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Organization: Intel Israel (74) Limited From: Ilan Peer When the upper layer requests to cancel an ongoing scan, a race is possible in which by the time the driver starts to handle the upper layers scan cancel flow, the FW already completed handling the scan request and the driver received the scan complete notification but still did not handle the notification. In such a case the FW will simply ignore the scan abort request coming from the driver, no notification would arrive from the FW and the entire abort flow would be considered a failure. To better handle this, check the status code returned by the FW for the scan abort command. In case the status indicates that no scan was aborted, complete the scan abort flow with success, i.e., the scan was aborted, as the flow is expected to consume the scan complete notification. Signed-off-by: Ilan Peer Reviewed-by: Johannes Berg Signed-off-by: Miri Korenblit --- .../net/wireless/intel/iwlwifi/fw/api/scan.h | 13 ++++++ drivers/net/wireless/intel/iwlwifi/mvm/scan.c | 42 +++++++++++++++---- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/scan.h b/drivers/net/wireless/intel/iwlwifi/fw/api/scan.h index 8598031567bb..0aefdf353b21 100644 --- a/drivers/net/wireless/intel/iwlwifi/fw/api/scan.h +++ b/drivers/net/wireless/intel/iwlwifi/fw/api/scan.h @@ -1132,6 +1132,19 @@ struct iwl_umac_scan_abort { __le32 flags; } __packed; /* SCAN_ABORT_CMD_UMAC_API_S_VER_1 */ +/** + * enum iwl_umac_scan_abort_status + * + * @IWL_UMAC_SCAN_ABORT_STATUS_SUCCESS: scan was successfully aborted + * @IWL_UMAC_SCAN_ABORT_STATUS_IN_PROGRESS: scan abort is in progress + * @IWL_UMAC_SCAN_ABORT_STATUS_NOT_FOUND: nothing to abort + */ +enum iwl_umac_scan_abort_status { + IWL_UMAC_SCAN_ABORT_STATUS_SUCCESS = 0, + IWL_UMAC_SCAN_ABORT_STATUS_IN_PROGRESS, + IWL_UMAC_SCAN_ABORT_STATUS_NOT_FOUND, +}; + /** * struct iwl_umac_scan_complete * @uid: scan id, &enum iwl_umac_scan_uid_offsets diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/scan.c b/drivers/net/wireless/intel/iwlwifi/mvm/scan.c index 8e0df31f1b3e..342182bcd566 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/scan.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/scan.c @@ -3301,13 +3301,23 @@ void iwl_mvm_rx_umac_scan_iter_complete_notif(struct iwl_mvm *mvm, mvm->scan_start); } -static int iwl_mvm_umac_scan_abort(struct iwl_mvm *mvm, int type) +static int iwl_mvm_umac_scan_abort(struct iwl_mvm *mvm, int type, bool *wait) { - struct iwl_umac_scan_abort cmd = {}; + struct iwl_umac_scan_abort abort_cmd = {}; + struct iwl_host_cmd cmd = { + .id = WIDE_ID(IWL_ALWAYS_LONG_GROUP, SCAN_ABORT_UMAC), + .len = { sizeof(abort_cmd), }, + .data = { &abort_cmd, }, + .flags = CMD_SEND_IN_RFKILL, + }; + int uid, ret; + u32 status; lockdep_assert_held(&mvm->mutex); + *wait = true; + /* We should always get a valid index here, because we already * checked that this type of scan was running in the generic * code. @@ -3316,17 +3326,28 @@ static int iwl_mvm_umac_scan_abort(struct iwl_mvm *mvm, int type) if (WARN_ON_ONCE(uid < 0)) return uid; - cmd.uid = cpu_to_le32(uid); + abort_cmd.uid = cpu_to_le32(uid); IWL_DEBUG_SCAN(mvm, "Sending scan abort, uid %u\n", uid); - ret = iwl_mvm_send_cmd_pdu(mvm, - WIDE_ID(IWL_ALWAYS_LONG_GROUP, SCAN_ABORT_UMAC), - CMD_SEND_IN_RFKILL, sizeof(cmd), &cmd); + ret = iwl_mvm_send_cmd_status(mvm, &cmd, &status); + + IWL_DEBUG_SCAN(mvm, "Scan abort: ret=%d, status=%u\n", ret, status); if (!ret) mvm->scan_uid_status[uid] = type << IWL_MVM_SCAN_STOPPING_SHIFT; - IWL_DEBUG_SCAN(mvm, "Scan abort: ret=%d\n", ret); + /* Handle the case that the FW is no longer familiar with the scan that + * is to be stopped. In such a case, it is expected that the scan + * complete notification was already received but not yet processed. + * In such a case, there is no need to wait for a scan complete + * notification and the flow should continue similar to the case that + * the scan was really aborted. + */ + if (status == IWL_UMAC_SCAN_ABORT_STATUS_NOT_FOUND) { + mvm->scan_uid_status[uid] = type << IWL_MVM_SCAN_STOPPING_SHIFT; + *wait = false; + } + return ret; } @@ -3336,6 +3357,7 @@ static int iwl_mvm_scan_stop_wait(struct iwl_mvm *mvm, int type) static const u16 scan_done_notif[] = { SCAN_COMPLETE_UMAC, SCAN_OFFLOAD_COMPLETE, }; int ret; + bool wait = true; lockdep_assert_held(&mvm->mutex); @@ -3347,7 +3369,7 @@ static int iwl_mvm_scan_stop_wait(struct iwl_mvm *mvm, int type) IWL_DEBUG_SCAN(mvm, "Preparing to stop scan, type %x\n", type); if (fw_has_capa(&mvm->fw->ucode_capa, IWL_UCODE_TLV_CAPA_UMAC_SCAN)) - ret = iwl_mvm_umac_scan_abort(mvm, type); + ret = iwl_mvm_umac_scan_abort(mvm, type, &wait); else ret = iwl_mvm_lmac_scan_abort(mvm); @@ -3355,6 +3377,10 @@ static int iwl_mvm_scan_stop_wait(struct iwl_mvm *mvm, int type) IWL_DEBUG_SCAN(mvm, "couldn't stop scan type %d\n", type); iwl_remove_notification(&mvm->notif_wait, &wait_scan_done); return ret; + } else if (!wait) { + IWL_DEBUG_SCAN(mvm, "no need to wait for scan type %d\n", type); + iwl_remove_notification(&mvm->notif_wait, &wait_scan_done); + return 0; } return iwl_wait_notification(&mvm->notif_wait, &wait_scan_done,