Linux wireless drivers development
 help / color / mirror / Atom feed
From: Luca Coelho <luca@coelho.fi>
To: kvalo@codeaurora.org
Cc: linux-wireless@vger.kernel.org
Subject: [PATCH 09/19] iwlwifi: dbg_ini: verify debug TLVs at allocation phase
Date: Sat, 31 Aug 2019 12:48:49 +0300	[thread overview]
Message-ID: <20190831094859.6391-10-luca@coelho.fi> (raw)
In-Reply-To: <20190831094859.6391-1-luca@coelho.fi>

From: Shahar S Matityahu <shahar.s.matityahu@intel.com>

Reimplement debug TLV allocation flow. The driver will check the
validity of the debug TLVs prior allocating space for them.
Any malformed or unsupported TLV will be skipped.
The TLV specific checks will be added in later patches.

Signed-off-by: Shahar S Matityahu <shahar.s.matityahu@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/fw/dbg.c   |  15 +-
 .../net/wireless/intel/iwlwifi/iwl-dbg-tlv.c  | 151 +++++++++++++-----
 .../net/wireless/intel/iwlwifi/iwl-dbg-tlv.h  |   4 +-
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c  |   2 +-
 .../net/wireless/intel/iwlwifi/iwl-trans.h    |  24 ++-
 5 files changed, 139 insertions(+), 57 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
index 3679ba89ea9d..6e5a3289e04a 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
+++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
@@ -1739,7 +1739,7 @@ static void iwl_dump_ini_info(struct iwl_fw_runtime *fwrt,
 	dump->version = cpu_to_le32(IWL_INI_DUMP_VER);
 	dump->trigger_id = trigger->trigger_id;
 	dump->is_external_cfg =
-		cpu_to_le32(fwrt->trans->dbg.external_ini_loaded);
+		cpu_to_le32(fwrt->trans->dbg.external_ini_cfg);
 
 	dump->ver_type = cpu_to_le32(fwrt->dump.fw_ver.type);
 	dump->ver_subtype = cpu_to_le32(fwrt->dump.fw_ver.subtype);
@@ -2855,17 +2855,22 @@ static void iwl_fw_dbg_ini_reset_cfg(struct iwl_fw_runtime *fwrt)
 void iwl_fw_dbg_apply_point(struct iwl_fw_runtime *fwrt,
 			    enum iwl_fw_ini_apply_point apply_point)
 {
-	void *data = &fwrt->trans->dbg.apply_points[apply_point];
+	void *data;
 
 	IWL_DEBUG_FW(fwrt, "WRT: enabling apply point %d\n", apply_point);
 
 	if (apply_point == IWL_FW_INI_APPLY_EARLY)
 		iwl_fw_dbg_ini_reset_cfg(fwrt);
 
-	_iwl_fw_dbg_apply_point(fwrt, data, apply_point, false);
+	if (fwrt->trans->dbg.internal_ini_cfg != IWL_INI_CFG_STATE_NOT_LOADED) {
+		data = &fwrt->trans->dbg.apply_points[apply_point];
+		_iwl_fw_dbg_apply_point(fwrt, data, apply_point, false);
+	}
 
-	data = &fwrt->trans->dbg.apply_points_ext[apply_point];
-	_iwl_fw_dbg_apply_point(fwrt, data, apply_point, true);
+	if (fwrt->trans->dbg.external_ini_cfg != IWL_INI_CFG_STATE_NOT_LOADED) {
+		data = &fwrt->trans->dbg.apply_points_ext[apply_point];
+		_iwl_fw_dbg_apply_point(fwrt, data, apply_point, true);
+	}
 }
 IWL_EXPORT_SYMBOL(iwl_fw_dbg_apply_point);
 
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
index 7c1e76ee7ede..5b1644a70ace 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
@@ -63,46 +63,124 @@
 #include "iwl-trans.h"
 #include "iwl-dbg-tlv.h"
 
-void iwl_dbg_tlv_copy(struct iwl_trans *trans, struct iwl_ucode_tlv *tlv,
-		      bool ext)
+/**
+ * enum iwl_dbg_tlv_type - debug TLV types
+ * @IWL_DBG_TLV_TYPE_DEBUG_INFO: debug info TLV
+ * @IWL_DBG_TLV_TYPE_BUF_ALLOC: buffer allocation TLV
+ * @IWL_DBG_TLV_TYPE_HCMD: host command TLV
+ * @IWL_DBG_TLV_TYPE_REGION: region TLV
+ * @IWL_DBG_TLV_TYPE_TRIGGER: trigger TLV
+ * @IWL_DBG_TLV_TYPE_NUM: number of debug TLVs
+ */
+enum iwl_dbg_tlv_type {
+	IWL_DBG_TLV_TYPE_DEBUG_INFO =
+		IWL_UCODE_TLV_TYPE_DEBUG_INFO - IWL_UCODE_TLV_DEBUG_BASE,
+	IWL_DBG_TLV_TYPE_BUF_ALLOC,
+	IWL_DBG_TLV_TYPE_HCMD,
+	IWL_DBG_TLV_TYPE_REGION,
+	IWL_DBG_TLV_TYPE_TRIGGER,
+	IWL_DBG_TLV_TYPE_NUM,
+};
+
+/**
+ * struct iwl_dbg_tlv_ver_data -  debug TLV version struct
+ * @min_ver: min version supported
+ * @max_ver: max version supported
+ */
+struct iwl_dbg_tlv_ver_data {
+	int min_ver;
+	int max_ver;
+};
+
+static const struct iwl_dbg_tlv_ver_data
+dbg_ver_table[IWL_DBG_TLV_TYPE_NUM] = {
+	[IWL_DBG_TLV_TYPE_DEBUG_INFO]	= {.min_ver = 1, .max_ver = 1,},
+	[IWL_DBG_TLV_TYPE_BUF_ALLOC]	= {.min_ver = 1, .max_ver = 1,},
+	[IWL_DBG_TLV_TYPE_HCMD]		= {.min_ver = 1, .max_ver = 1,},
+	[IWL_DBG_TLV_TYPE_REGION]	= {.min_ver = 1, .max_ver = 1,},
+	[IWL_DBG_TLV_TYPE_TRIGGER]	= {.min_ver = 1, .max_ver = 1,},
+};
+
+static int iwl_dbg_tlv_copy(struct iwl_ucode_tlv *tlv, struct list_head *list)
 {
-	struct iwl_apply_point_data *dbg_cfg, *tlv_copy;
-	struct iwl_fw_ini_header *header = (void *)&tlv->data[0];
-	u32 tlv_type = le32_to_cpu(tlv->type);
+	struct iwl_apply_point_data *tlv_copy;
 	u32 len = le32_to_cpu(tlv->length);
-	u32 apply_point = le32_to_cpu(header->apply_point);
 
-	if (le32_to_cpu(header->tlv_version) != 1)
-		return;
+	tlv_copy = kzalloc(sizeof(*tlv_copy) + len, GFP_KERNEL);
+	if (!tlv_copy)
+		return -ENOMEM;
 
-	if (WARN_ONCE(apply_point >= IWL_FW_INI_APPLY_NUM,
-		      "Invalid apply point %d\n", apply_point))
-		return;
+	INIT_LIST_HEAD(&tlv_copy->list);
+	memcpy(&tlv_copy->tlv, tlv, sizeof(*tlv) + len);
+
+	if (!list->next)
+		INIT_LIST_HEAD(list);
+
+	list_add_tail(&tlv_copy->list, list);
+
+	return 0;
+}
+
+static bool iwl_dbg_tlv_ver_support(struct iwl_ucode_tlv *tlv)
+{
+	struct iwl_fw_ini_header *hdr = (void *)&tlv->data[0];
+	u32 type = le32_to_cpu(tlv->type);
+	u32 tlv_idx = type - IWL_UCODE_TLV_DEBUG_BASE;
+	u32 ver = le32_to_cpu(hdr->tlv_version);
+
+	if (ver < dbg_ver_table[tlv_idx].min_ver ||
+	    ver > dbg_ver_table[tlv_idx].max_ver)
+		return false;
+
+	return true;
+}
+
+void iwl_dbg_tlv_alloc(struct iwl_trans *trans, struct iwl_ucode_tlv *tlv,
+		       bool ext)
+{
+	struct iwl_fw_ini_header *hdr = (void *)&tlv->data[0];
+	u32 type = le32_to_cpu(tlv->type);
+	u32 pnt = le32_to_cpu(hdr->apply_point);
+	u32 tlv_idx = type - IWL_UCODE_TLV_DEBUG_BASE;
+	enum iwl_ini_cfg_state *cfg_state = ext ?
+		&trans->dbg.external_ini_cfg : &trans->dbg.internal_ini_cfg;
+	struct list_head *dbg_cfg_list = ext ?
+		&trans->dbg.apply_points_ext[pnt].list :
+		&trans->dbg.apply_points[pnt].list;
 
 	IWL_DEBUG_FW(trans, "WRT: read TLV 0x%x, apply point %d\n",
-		     tlv_type, apply_point);
+		     type, pnt);
 
-	if (ext)
-		dbg_cfg = &trans->dbg.apply_points_ext[apply_point];
-	else
-		dbg_cfg = &trans->dbg.apply_points[apply_point];
+	if (tlv_idx >= IWL_DBG_TLV_TYPE_NUM) {
+		IWL_ERR(trans, "WRT: Unsupported TLV 0x%x\n", type);
+		goto out_err;
+	}
 
-	tlv_copy = kzalloc(sizeof(*tlv_copy) + len, GFP_KERNEL);
-	if (!tlv_copy) {
-		IWL_ERR(trans, "WRT: No memory for TLV 0x%x, apply point %d\n",
-			tlv_type, apply_point);
-		return;
+	if (pnt >= IWL_FW_INI_APPLY_NUM) {
+		IWL_ERR(trans, "WRT: Invalid apply point %d\n", pnt);
+		goto out_err;
 	}
 
-	INIT_LIST_HEAD(&tlv_copy->list);
-	memcpy(&tlv_copy->tlv, tlv, sizeof(*tlv) + len);
+	if (!iwl_dbg_tlv_ver_support(tlv)) {
+		IWL_ERR(trans, "WRT: Unsupported TLV 0x%x version %u\n", type,
+			le32_to_cpu(hdr->tlv_version));
+		goto out_err;
+	}
 
-	if (!dbg_cfg->list.next)
-		INIT_LIST_HEAD(&dbg_cfg->list);
+	if (iwl_dbg_tlv_copy(tlv, dbg_cfg_list)) {
+		IWL_ERR(trans,
+			"WRT: Failed to allocate TLV 0x%x, apply point %d\n",
+			type, pnt);
+		goto out_err;
+	}
+
+	if (*cfg_state == IWL_INI_CFG_STATE_NOT_LOADED)
+		*cfg_state = IWL_INI_CFG_STATE_LOADED;
 
-	list_add_tail(&tlv_copy->list, &dbg_cfg->list);
+	return;
 
-	trans->dbg.ini_valid = true;
+out_err:
+	*cfg_state = IWL_INI_CFG_STATE_CORRUPTED;
 }
 
 static void iwl_dbg_tlv_free_list(struct list_head *list)
@@ -138,7 +216,6 @@ static int iwl_dbg_tlv_parse_bin(struct iwl_trans *trans, const u8 *data,
 				 size_t len)
 {
 	struct iwl_ucode_tlv *tlv;
-	enum iwl_ucode_tlv_type tlv_type;
 	u32 tlv_len;
 
 	while (len >= sizeof(*tlv)) {
@@ -146,7 +223,6 @@ static int iwl_dbg_tlv_parse_bin(struct iwl_trans *trans, const u8 *data,
 		tlv = (void *)data;
 
 		tlv_len = le32_to_cpu(tlv->length);
-		tlv_type = le32_to_cpu(tlv->type);
 
 		if (len < tlv_len) {
 			IWL_ERR(trans, "invalid TLV len: %zd/%u\n",
@@ -156,19 +232,7 @@ static int iwl_dbg_tlv_parse_bin(struct iwl_trans *trans, const u8 *data,
 		len -= ALIGN(tlv_len, 4);
 		data += sizeof(*tlv) + ALIGN(tlv_len, 4);
 
-		switch (tlv_type) {
-		case IWL_UCODE_TLV_TYPE_DEBUG_INFO:
-		case IWL_UCODE_TLV_TYPE_BUFFER_ALLOCATION:
-		case IWL_UCODE_TLV_TYPE_HCMD:
-		case IWL_UCODE_TLV_TYPE_REGIONS:
-		case IWL_UCODE_TLV_TYPE_TRIGGERS:
-		case IWL_UCODE_TLV_TYPE_DEBUG_FLOW:
-			iwl_dbg_tlv_copy(trans, tlv, true);
-			break;
-		default:
-			WARN_ONCE(1, "Invalid TLV %x\n", tlv_type);
-			break;
-		}
+		iwl_dbg_tlv_alloc(trans, tlv, true);
 	}
 
 	return 0;
@@ -179,7 +243,7 @@ void iwl_dbg_tlv_load_bin(struct device *dev, struct iwl_trans *trans)
 	const struct firmware *fw;
 	int res;
 
-	if (trans->dbg.external_ini_loaded || !iwlwifi_mod_params.enable_ini)
+	if (!iwlwifi_mod_params.enable_ini)
 		return;
 
 	res = request_firmware(&fw, "iwl-dbg-tlv.ini", dev);
@@ -188,6 +252,5 @@ void iwl_dbg_tlv_load_bin(struct device *dev, struct iwl_trans *trans)
 
 	iwl_dbg_tlv_parse_bin(trans, fw->data, fw->size);
 
-	trans->dbg.external_ini_loaded = true;
 	release_firmware(fw);
 }
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.h b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.h
index 72552de801d4..3a60590d274d 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.h
@@ -77,7 +77,7 @@ struct iwl_apply_point_data {
 struct iwl_trans;
 void iwl_dbg_tlv_load_bin(struct device *dev, struct iwl_trans *trans);
 void iwl_dbg_tlv_free(struct iwl_trans *trans);
-void iwl_dbg_tlv_copy(struct iwl_trans *trans, struct iwl_ucode_tlv *tlv,
-		      bool ext);
+void iwl_dbg_tlv_alloc(struct iwl_trans *trans, struct iwl_ucode_tlv *tlv,
+		       bool ext);
 
 #endif /* __iwl_dbg_tlv_h__*/
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 3792b421746e..1351f7fe5ae2 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1153,7 +1153,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
 		case IWL_UCODE_TLV_TYPE_TRIGGERS:
 		case IWL_UCODE_TLV_TYPE_DEBUG_FLOW:
 			if (iwlwifi_mod_params.enable_ini)
-				iwl_dbg_tlv_copy(drv->trans, tlv, false);
+				iwl_dbg_tlv_alloc(drv->trans, tlv, false);
 			break;
 		case IWL_UCODE_TLV_CMD_VERSIONS:
 			if (tlv_len % sizeof(struct iwl_fw_cmd_version)) {
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-trans.h b/drivers/net/wireless/intel/iwlwifi/iwl-trans.h
index 731ffb4a33af..09ed0dd163eb 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-trans.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-trans.h
@@ -650,6 +650,19 @@ enum iwl_plat_pm_mode {
 	IWL_PLAT_PM_MODE_D3,
 };
 
+/**
+ * enum iwl_ini_cfg_state
+ * @IWL_INI_CFG_STATE_NOT_LOADED: no debug cfg was given
+ * @IWL_INI_CFG_STATE_LOADED: debug cfg was found and loaded
+ * @IWL_INI_CFG_STATE_CORRUPTED: debug cfg was found and some of the TLVs
+ *	are corrupted. The rest of the debug TLVs will still be used
+ */
+enum iwl_ini_cfg_state {
+	IWL_INI_CFG_STATE_NOT_LOADED,
+	IWL_INI_CFG_STATE_LOADED,
+	IWL_INI_CFG_STATE_CORRUPTED,
+};
+
 /* Max time to wait for nmi interrupt */
 #define IWL_TRANS_NMI_TIMEOUT (HZ / 4)
 
@@ -691,8 +704,8 @@ struct iwl_self_init_dram {
  * @umac_error_event_table: addr of umac error table
  * @error_event_table_tlv_status: bitmap that indicates what error table
  *	pointers was recevied via TLV. uses enum &iwl_error_event_table_status
- * @external_ini_loaded: indicates if an external ini cfg was given
- * @ini_valid: indicates if debug ini mode is on
+ * @internal_ini_cfg: internal debug cfg state. Uses &enum iwl_ini_cfg_state
+ * @external_ini_cfg: external debug cfg state. Uses &enum iwl_ini_cfg_state
  * @num_blocks: number of blocks in fw_mon
  * @fw_mon: address of the buffers for firmware monitor
  * @is_alloc: bit i is set if buffer i was allocated
@@ -711,8 +724,8 @@ struct iwl_trans_debug {
 	u32 umac_error_event_table;
 	unsigned int error_event_table_tlv_status;
 
-	bool external_ini_loaded;
-	bool ini_valid;
+	enum iwl_ini_cfg_state internal_ini_cfg;
+	enum iwl_ini_cfg_state external_ini_cfg;
 
 	struct iwl_apply_point_data apply_points[IWL_FW_INI_APPLY_NUM];
 	struct iwl_apply_point_data apply_points_ext[IWL_FW_INI_APPLY_NUM];
@@ -1218,7 +1231,8 @@ static inline void iwl_trans_sync_nmi(struct iwl_trans *trans)
 
 static inline bool iwl_trans_dbg_ini_valid(struct iwl_trans *trans)
 {
-	return trans->dbg.ini_valid;
+	return trans->dbg.internal_ini_cfg != IWL_INI_CFG_STATE_NOT_LOADED ||
+		trans->dbg.external_ini_cfg != IWL_INI_CFG_STATE_NOT_LOADED;
 }
 
 /*****************************************************
-- 
2.23.0.rc1


  parent reply	other threads:[~2019-08-31  9:49 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-31  9:48 [PATCH 00/19] iwlwifi: updates intended for v5.4 2019-08-31 Luca Coelho
2019-08-31  9:48 ` [PATCH 01/19] iwlwifi: stop passing bogus gfp flags arguments to dma_alloc_coherent Luca Coelho
2019-08-31  9:48 ` [PATCH 02/19] iwlwifi: mvm: simplify the channel switch flow for newer firmware Luca Coelho
2019-08-31  9:48 ` [PATCH 03/19] iwlwifi: mvm: remove check for lq_sta in __iwl_mvm_rs_tx_status() Luca Coelho
2019-08-31  9:48 ` [PATCH 04/19] iwlwifi: support per-platform antenna gain Luca Coelho
2019-08-31  9:48 ` [PATCH 05/19] iwlwifi: api: fix FTM struct documentation Luca Coelho
2019-08-31  9:48 ` [PATCH 06/19] iwlwifi: bump FW API to 50 for 22000 series Luca Coelho
2019-08-31  9:48 ` [PATCH 07/19] iwlwifi: remove duplicate FW string definitions Luca Coelho
2019-08-31  9:48 ` [PATCH 08/19] iwlwifi: dbg_ini: use function to check if ini dbg mode is on Luca Coelho
2019-08-31  9:48 ` Luca Coelho [this message]
2019-08-31  9:48 ` [PATCH 10/19] iwlwifi: dbg_ini: remove debug flow TLV Luca Coelho
2019-08-31  9:48 ` [PATCH 11/19] iwlwifi: dbg: align wrt log prints to the same format Luca Coelho
2019-08-31  9:48 ` [PATCH 12/19] iwlwifi: remove unnecessary IWL_DEVICE_AX200_COMMON definition Luca Coelho
2019-08-31  9:48 ` [PATCH 13/19] iwlwifi: allocate bigger nvm data in case of UHB Luca Coelho
2019-08-31  9:48 ` [PATCH 14/19] iwlwifi: mvm: look for the first supported channel when add/remove phy ctxt Luca Coelho
2019-08-31  9:48 ` [PATCH 15/19] iwlwifi: dbg_ini: separate cfg and dump flows to different modules Luca Coelho
2019-08-31  9:48 ` [PATCH 16/19] iwlwifi: dbg_ini: use linked list for dump TLVs during dump creation Luca Coelho
2019-08-31  9:48 ` [PATCH 17/19] iwlwifi: dbg_ini: move tx fifo data into fw runtime Luca Coelho
2019-08-31  9:48 ` [PATCH 18/19] iwlwifi: dbg_ini: make a single ops struct for paging collect Luca Coelho
2019-08-31  9:48 ` [PATCH 19/19] iwlwifi: dbg_ini: use regions ops array instead of switch case in dump flow Luca Coelho

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190831094859.6391-10-luca@coelho.fi \
    --to=luca@coelho.fi \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox