diff mbox series

[11/14] wifi: iwlwifi: dbg-tlv: fix DRAM data init

Message ID 20230613155501.87cf5528f4bc.I26ac907a4162297808b33467fc7f5d8177474a34@changeid (mailing list archive)
State Accepted
Delegated to: Johannes Berg
Headers show
Series wifi: iwlwifi: updates intended for v6.5 2023-06-13 | expand

Commit Message

Greenman, Gregory June 13, 2023, 12:57 p.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

Given the existing code in iwl_dbg_tlv_update_drams(), the
following can happen and cause firmware asserts, and even
the device to become unusable:

 * We set the magic so the firmware will use the data;
 * we try to fill multiple allocation IDs, with at least
   one successful, but - crucially - one failing and thus
   not touching the data;
 * we don't clear the data since there was one success.

This doesn't seem like much of a problem just yet, however,
what happens now is that the allocation ID(s) that failed
are not initialized.

There are two additional things to know:
 * we never free these allocations across FW restart or
   interface down/up etc., in fact we never free them until
   the driver is unbound from the device (e.g. unloaded)
 * the firmware uses the DRAM info structure for real debug
   data when it has used it completely

Given that, and the fact that we never initialize the data
on restart, we can be unlucky and end up with an allocation
that looks for the most part valid (valid ID, valid number
of buffers, etc.) but has bad sizes - causing the firmware
to throw an assert we can never recover from.

Fixing the code to have the entire buffers cleared (which
we should do so old debug data isn't sticking around) is a
bit more complex, so as a first step make the actual code
that fills the information more robust by clearing the
structure first, and filling the magic values only if it
actually succeeded for one, rather than doing it the other
way around.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Gregory Greenman <gregory.greenman@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
diff mbox series

Patch

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
index fb0277bd12cf..3f9d4670f6c6 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
@@ -1,6 +1,6 @@ 
 // SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
 /*
- * Copyright (C) 2018-2022 Intel Corporation
+ * Copyright (C) 2018-2023 Intel Corporation
  */
 #include <linux/firmware.h>
 #include "iwl-drv.h"
@@ -795,8 +795,7 @@  static void iwl_dbg_tlv_update_drams(struct iwl_fw_runtime *fwrt)
 			 IWL_UCODE_TLV_CAPA_DRAM_FRAG_SUPPORT))
 		return;
 
-	dram_info->first_word = cpu_to_le32(DRAM_INFO_FIRST_MAGIC_WORD);
-	dram_info->second_word = cpu_to_le32(DRAM_INFO_SECOND_MAGIC_WORD);
+	memset(dram_info, 0, sizeof(*dram_info));
 
 	for (i = IWL_FW_INI_ALLOCATION_ID_DBGC1;
 	     i < IWL_FW_INI_ALLOCATION_NUM; i++) {
@@ -813,11 +812,10 @@  static void iwl_dbg_tlv_update_drams(struct iwl_fw_runtime *fwrt)
 				 i, ret);
 	}
 
-	if (dram_alloc)
-		IWL_DEBUG_FW(fwrt, "block data after  %08x\n",
-			     dram_info->first_word);
-	else
-		memset(frags->block, 0, sizeof(*dram_info));
+	if (dram_alloc) {
+		dram_info->first_word = cpu_to_le32(DRAM_INFO_FIRST_MAGIC_WORD);
+		dram_info->second_word = cpu_to_le32(DRAM_INFO_SECOND_MAGIC_WORD);
+	}
 }
 
 static void iwl_dbg_tlv_send_hcmds(struct iwl_fw_runtime *fwrt,