[PATCH 5.17 103/343] scsi: pm8001: Fix tag values handling

From: Greg Kroah-Hartman
Date: Tue Apr 12 2022 - 05:30:19 EST

Next message: Greg Kroah-Hartman: "[PATCH 5.17 022/343] drm/amd/display: Use PSR version selected during set_psr_caps"
Previous message: Greg Kroah-Hartman: "[PATCH 5.17 182/343] x86/Kconfig: Do not allow CONFIG_X86_X32_ABI=y with llvm-objcopy"
In reply to: Greg Kroah-Hartman: "[PATCH 5.17 182/343] x86/Kconfig: Do not allow CONFIG_X86_X32_ABI=y with llvm-objcopy"
Next in thread: Greg Kroah-Hartman: "[PATCH 5.17 022/343] drm/amd/display: Use PSR version selected during set_psr_caps"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

From: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>

[ Upstream commit 7fb23a785ba38dea907323a039123f231195b297 ]

The function pm8001_tag_alloc() determines free tags using the function
find_first_zero_bit() which can return 0 when the first bit of the bitmap
being inspected is 0. As such, tag 0 is a valid tag value that should not
be dismissed as invalid. Fix the functions pm8001_work_fn(),
mpi_sata_completion(), pm8001_mpi_task_abort_resp() and
pm8001_open_reject_retry() to not dismiss 0 tags as invalid.

The value 0xffffffff is used for invalid tags for unused ccb information
structures. Add the macro definition PM8001_INVALID_TAG to define this
value.

Link: https://lore.kernel.org/r/20220220031810.738362-20-damien.lemoal@xxxxxxxxxxxxxxxxxx
Reviewed-by: Jack Wang <jinpu.wang@xxxxxxxxx>
Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
drivers/scsi/pm8001/pm8001_hwi.c | 52 +++++++++++--------------------
drivers/scsi/pm8001/pm8001_init.c | 3 +-
drivers/scsi/pm8001/pm8001_sas.c | 13 ++++----
drivers/scsi/pm8001/pm8001_sas.h | 2 ++
drivers/scsi/pm8001/pm80xx_hwi.c | 5 ---
5 files changed, 28 insertions(+), 47 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 4f4a9dcb6a1e..d244231e62f9 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1522,7 +1522,6 @@ void pm8001_work_fn(struct work_struct *work)
case IO_XFER_ERROR_BREAK:
{ /* This one stashes the sas_task instead */
struct sas_task *t = (struct sas_task *)pm8001_dev;
- u32 tag;
struct pm8001_ccb_info *ccb;
struct pm8001_hba_info *pm8001_ha = pw->pm8001_ha;
unsigned long flags, flags1;
@@ -1544,8 +1543,8 @@ void pm8001_work_fn(struct work_struct *work)
/* Search for a possible ccb that matches the task */
for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {
ccb = &pm8001_ha->ccb_info[i];
- tag = ccb->ccb_tag;
- if ((tag != 0xFFFFFFFF) && (ccb->task == t))
+ if ((ccb->ccb_tag != PM8001_INVALID_TAG) &&
+ (ccb->task == t))
break;
}
if (!ccb) {
@@ -1567,11 +1566,11 @@ void pm8001_work_fn(struct work_struct *work)
spin_unlock_irqrestore(&t->task_state_lock, flags1);
pm8001_dbg(pm8001_ha, FAIL, "task 0x%p done with event 0x%x resp 0x%x stat 0x%x but aborted by upper layer!\n",
t, pw->handler, ts->resp, ts->stat);
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, t, ccb, ccb->ccb_tag);
spin_unlock_irqrestore(&pm8001_ha->lock, flags);
} else {
spin_unlock_irqrestore(&t->task_state_lock, flags1);
- pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, t, ccb, ccb->ccb_tag);
mb();/* in order to force CPU ordering */
spin_unlock_irqrestore(&pm8001_ha->lock, flags);
t->task_done(t);
@@ -1580,7 +1579,6 @@ void pm8001_work_fn(struct work_struct *work)
case IO_XFER_OPEN_RETRY_TIMEOUT:
{ /* This one stashes the sas_task instead */
struct sas_task *t = (struct sas_task *)pm8001_dev;
- u32 tag;
struct pm8001_ccb_info *ccb;
struct pm8001_hba_info *pm8001_ha = pw->pm8001_ha;
unsigned long flags, flags1;
@@ -1614,8 +1612,8 @@ void pm8001_work_fn(struct work_struct *work)
/* Search for a possible ccb that matches the task */
for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {
ccb = &pm8001_ha->ccb_info[i];
- tag = ccb->ccb_tag;
- if ((tag != 0xFFFFFFFF) && (ccb->task == t))
+ if ((ccb->ccb_tag != PM8001_INVALID_TAG) &&
+ (ccb->task == t))
break;
}
if (!ccb) {
@@ -1686,19 +1684,13 @@ void pm8001_work_fn(struct work_struct *work)
struct task_status_struct *ts;
struct sas_task *task;
int i;
- u32 tag, device_id;
+ u32 device_id;

for (i = 0; ccb = NULL, i < PM8001_MAX_CCB; i++) {
ccb = &pm8001_ha->ccb_info[i];
task = ccb->task;
ts = &task->task_status;
- tag = ccb->ccb_tag;
- /* check if tag is NULL */
- if (!tag) {
- pm8001_dbg(pm8001_ha, FAIL,
- "tag Null\n");
- continue;
- }
+
if (task != NULL) {
dev = task->dev;
if (!dev) {
@@ -1707,10 +1699,11 @@ void pm8001_work_fn(struct work_struct *work)
continue;
}
/*complete sas task and update to top layer */
- pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, task, ccb,
+ ccb->ccb_tag);
ts->resp = SAS_TASK_COMPLETE;
task->task_done(task);
- } else if (tag != 0xFFFFFFFF) {
+ } else if (ccb->ccb_tag != PM8001_INVALID_TAG) {
/* complete the internal commands/non-sas task */
pm8001_dev = ccb->device;
if (pm8001_dev->dcompletion) {
@@ -1718,7 +1711,7 @@ void pm8001_work_fn(struct work_struct *work)
pm8001_dev->dcompletion = NULL;
}
complete(pm8001_ha->nvmd_completion);
- pm8001_tag_free(pm8001_ha, tag);
+ pm8001_tag_free(pm8001_ha, ccb->ccb_tag);
}
}
/* Deregister all the device ids */
@@ -2316,11 +2309,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
param = le32_to_cpu(psataPayload->param);
tag = le32_to_cpu(psataPayload->tag);

- if (!tag) {
- pm8001_dbg(pm8001_ha, FAIL, "tag null\n");
- return;
- }
-
ccb = &pm8001_ha->ccb_info[tag];
t = ccb->task;
pm8001_dev = ccb->device;
@@ -3056,7 +3044,7 @@ void pm8001_mpi_set_dev_state_resp(struct pm8001_hba_info *pm8001_ha,
device_id, pds, nds, status);
complete(pm8001_dev->setds_completion);
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
}

@@ -3074,7 +3062,7 @@ void pm8001_mpi_set_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
dlen_status);
}
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
}

@@ -3101,7 +3089,7 @@ pm8001_mpi_get_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
* freed by requesting path anywhere.
*/
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
return;
}
@@ -3147,7 +3135,7 @@ pm8001_mpi_get_nvmd_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
complete(pm8001_ha->nvmd_completion);
pm8001_dbg(pm8001_ha, MSG, "Get nvmd data complete!\n");
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
}

@@ -3560,7 +3548,7 @@ int pm8001_mpi_reg_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
complete(pm8001_dev->dcompletion);
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, htag);
return 0;
}
@@ -3632,7 +3620,7 @@ int pm8001_mpi_fw_flash_update_resp(struct pm8001_hba_info *pm8001_ha,
}
kfree(ccb->fw_control_context);
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
complete(pm8001_ha->nvmd_completion);
return 0;
@@ -3668,10 +3656,6 @@ int pm8001_mpi_task_abort_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)

status = le32_to_cpu(pPayload->status);
tag = le32_to_cpu(pPayload->tag);
- if (!tag) {
- pm8001_dbg(pm8001_ha, FAIL, " TAG NULL. RETURNING !!!\n");
- return -1;
- }

scp = le32_to_cpu(pPayload->scp);
ccb = &pm8001_ha->ccb_info[tag];
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index d8a2121cb8d9..d2a2593f669d 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -1216,10 +1216,11 @@ pm8001_init_ccb_tag(struct pm8001_hba_info *pm8001_ha, struct Scsi_Host *shost,
goto err_out;
}
pm8001_ha->ccb_info[i].task = NULL;
- pm8001_ha->ccb_info[i].ccb_tag = 0xffffffff;
+ pm8001_ha->ccb_info[i].ccb_tag = PM8001_INVALID_TAG;
pm8001_ha->ccb_info[i].device = NULL;
++pm8001_ha->tags_num;
}
+
return 0;

err_out_noccb:
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 32edda3e55c6..c1f871561b32 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -567,7 +567,7 @@ void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha,

task->lldd_task = NULL;
ccb->task = NULL;
- ccb->ccb_tag = 0xFFFFFFFF;
+ ccb->ccb_tag = PM8001_INVALID_TAG;
ccb->open_retry = 0;
pm8001_tag_free(pm8001_ha, ccb_idx);
}
@@ -952,9 +952,11 @@ void pm8001_open_reject_retry(
struct task_status_struct *ts;
struct pm8001_device *pm8001_dev;
unsigned long flags1;
- u32 tag;
struct pm8001_ccb_info *ccb = &pm8001_ha->ccb_info[i];

+ if (ccb->ccb_tag == PM8001_INVALID_TAG)
+ continue;
+
pm8001_dev = ccb->device;
if (!pm8001_dev || (pm8001_dev->dev_type == SAS_PHY_UNUSED))
continue;
@@ -966,9 +968,6 @@ void pm8001_open_reject_retry(
continue;
} else if (pm8001_dev != device_to_close)
continue;
- tag = ccb->ccb_tag;
- if (!tag || (tag == 0xFFFFFFFF))
- continue;
task = ccb->task;
if (!task || !task->task_done)
continue;
@@ -989,11 +988,11 @@ void pm8001_open_reject_retry(
& SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&task->task_state_lock,
flags1);
- pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb->ccb_tag);
} else {
spin_unlock_irqrestore(&task->task_state_lock,
flags1);
- pm8001_ccb_task_free(pm8001_ha, task, ccb, tag);
+ pm8001_ccb_task_free(pm8001_ha, task, ccb, ccb->ccb_tag);
mb();/* in order to force CPU ordering */
spin_unlock_irqrestore(&pm8001_ha->lock, flags);
task->task_done(task);
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index a17da1cebce1..1791cdf30276 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -738,6 +738,8 @@ void pm8001_free_dev(struct pm8001_device *pm8001_dev);
/* ctl shared API */
extern const struct attribute_group *pm8001_host_groups[];

+#define PM8001_INVALID_TAG ((u32)-1)
+
static inline void
pm8001_ccb_task_free_done(struct pm8001_hba_info *pm8001_ha,
struct sas_task *task, struct pm8001_ccb_info *ccb,
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 9a0d65ac0174..728190b26924 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -2404,11 +2404,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
param = le32_to_cpu(psataPayload->param);
tag = le32_to_cpu(psataPayload->tag);

- if (!tag) {
- pm8001_dbg(pm8001_ha, FAIL, "tag null\n");
- return;
- }
-
ccb = &pm8001_ha->ccb_info[tag];
t = ccb->task;
pm8001_dev = ccb->device;
--
2.35.1

Next message: Greg Kroah-Hartman: "[PATCH 5.17 022/343] drm/amd/display: Use PSR version selected during set_psr_caps"
Previous message: Greg Kroah-Hartman: "[PATCH 5.17 182/343] x86/Kconfig: Do not allow CONFIG_X86_X32_ABI=y with llvm-objcopy"
In reply to: Greg Kroah-Hartman: "[PATCH 5.17 182/343] x86/Kconfig: Do not allow CONFIG_X86_X32_ABI=y with llvm-objcopy"
Next in thread: Greg Kroah-Hartman: "[PATCH 5.17 022/343] drm/amd/display: Use PSR version selected during set_psr_caps"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]