diff mbox series

[v2] scsi: pm8001: Fix bogus FW crash for maxcpus=1

Message ID 1642508105-95432-1-git-send-email-john.garry@huawei.com
State New
Headers show
Series [v2] scsi: pm8001: Fix bogus FW crash for maxcpus=1 | expand

Commit Message

John Garry Jan. 18, 2022, 12:15 p.m. UTC
According to the comment in check_fw_ready() we should not check the
IOP1_READY field in register SCRATCH_PAD_1 for 8008 or 8009 controllers.

However we check this very field in process_oq() for processing the highest
index interrupt vector. The highest interrupt vector is checked as the FW
is programmed to signal fatal errors through this irq.

Change that function to not check IOP1_READY for those mentioned
controllers, but do check ILA_READY in both cases.

The reason I assume that this was not hit earlier was because we always
allocated 64 MSI(X), and just did not pass the vector index check in
process_oq(), i.e.  the handler never ran for vector index 63.

Signed-off-by: John Garry <john.garry@huawei.com>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
Changes from v1:
- Improve coding style (Damien)
- Add comment and commit message info about checking fatal vector (Damien)
- Change patch subject to mention "bogus"
- Add tags (thanks!)

Comments

Martin K. Petersen Jan. 19, 2022, 3:41 a.m. UTC | #1
John,

> According to the comment in check_fw_ready() we should not check the
> IOP1_READY field in register SCRATCH_PAD_1 for 8008 or 8009
> controllers.

Applied to 5.17/scsi-staging, thanks!
Martin K. Petersen Jan. 25, 2022, 5:40 a.m. UTC | #2
On Tue, 18 Jan 2022 20:15:05 +0800, John Garry wrote:

> According to the comment in check_fw_ready() we should not check the
> IOP1_READY field in register SCRATCH_PAD_1 for 8008 or 8009 controllers.
> 
> However we check this very field in process_oq() for processing the highest
> index interrupt vector. The highest interrupt vector is checked as the FW
> is programmed to signal fatal errors through this irq.
> 
> [...]

Applied to 5.17/scsi-fixes, thanks!

[1/1] scsi: pm8001: Fix bogus FW crash for maxcpus=1
      https://git.kernel.org/mkp/scsi/c/62afb379a0fe
diff mbox series

Patch

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 97750d0ebee9..8a0e5e30b23b 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -4148,10 +4148,22 @@  static int process_oq(struct pm8001_hba_info *pm8001_ha, u8 vec)
 	u32 ret = MPI_IO_STATUS_FAIL;
 	u32 regval;
 
+	/*
+	 * Fatal errors are programmed to be signalled in irq vector
+	 * pm8001_ha->max_q_num - 1 through pm8001_ha->main_cfg_tbl.pm80xx_tbl.
+	 * fatal_err_interrupt
+	 */
 	if (vec == (pm8001_ha->max_q_num - 1)) {
+		u32 mipsall_ready;
+
+		if (pm8001_ha->chip_id == chip_8008 ||
+		    pm8001_ha->chip_id == chip_8009)
+			mipsall_ready = SCRATCH_PAD_MIPSALL_READY_8PORT;
+		else
+			mipsall_ready = SCRATCH_PAD_MIPSALL_READY_16PORT;
+
 		regval = pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_1);
-		if ((regval & SCRATCH_PAD_MIPSALL_READY) !=
-					SCRATCH_PAD_MIPSALL_READY) {
+		if ((regval & mipsall_ready) != mipsall_ready) {
 			pm8001_ha->controller_fatal_error = true;
 			pm8001_dbg(pm8001_ha, FAIL,
 				   "Firmware Fatal error! Regval:0x%x\n",
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h b/drivers/scsi/pm8001/pm80xx_hwi.h
index c7e5d93bea92..c41ed039c92a 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.h
+++ b/drivers/scsi/pm8001/pm80xx_hwi.h
@@ -1405,8 +1405,12 @@  typedef struct SASProtocolTimerConfig SASProtocolTimerConfig_t;
 #define SCRATCH_PAD_BOOT_LOAD_SUCCESS	0x0
 #define SCRATCH_PAD_IOP0_READY		0xC00
 #define SCRATCH_PAD_IOP1_READY		0x3000
-#define SCRATCH_PAD_MIPSALL_READY	(SCRATCH_PAD_IOP1_READY | \
+#define SCRATCH_PAD_MIPSALL_READY_16PORT	(SCRATCH_PAD_IOP1_READY | \
 					SCRATCH_PAD_IOP0_READY | \
+					SCRATCH_PAD_ILA_READY | \
+					SCRATCH_PAD_RAAE_READY)
+#define SCRATCH_PAD_MIPSALL_READY_8PORT	(SCRATCH_PAD_IOP0_READY | \
+					SCRATCH_PAD_ILA_READY | \
 					SCRATCH_PAD_RAAE_READY)
 
 /* boot loader state */