* [PATCH v3 1/4] scsi: libsas: Fix out-of-bounds memcpy in sas_ssp_task_response()
2026-10-09 3:32 [PATCH v3 0/4] scsi: hisi_sas: Some misc fixes Xingui Yang
@ 2026-10-09 3:32 ` Xingui Yang
2026-10-09 3:32 ` [PATCH v3 2/4] scsi: hisi_sas: Fix incorrect delay values from magic-number cleanup Xingui Yang
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Xingui Yang @ 2026-10-09 3:32 UTC (permalink / raw)
To: mkp, jejb, john.garry, yanaijie
Cc: linux-scsi, linux-kernel, linuxarm, yangxingui, liuyonglong,
kangfenglong
The min_t(int, ...) clamp of the device-provided sense_data_len
turns negative for values >= 0x80000000 and is then used as the
memcpy() length, reading and writing out of bounds. Compare as u32
instead. Well-formed responses are unaffected.
Fixes: 366ca51f30de ("[SCSI] libsas: abstract STP task status into a function")
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
drivers/scsi/libsas/sas_task.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/libsas/sas_task.c b/drivers/scsi/libsas/sas_task.c
index e9d291007817..2080e5836179 100644
--- a/drivers/scsi/libsas/sas_task.c
+++ b/drivers/scsi/libsas/sas_task.c
@@ -25,7 +25,7 @@ void sas_ssp_task_response(struct device *dev, struct sas_task *task,
case SAS_DATAPRES_SENSE_DATA:
tstat->stat = SAS_SAM_STAT_CHECK_CONDITION;
tstat->buf_valid_size =
- min_t(int, SAS_STATUS_BUF_SIZE,
+ min_t(u32, SAS_STATUS_BUF_SIZE,
be32_to_cpu(iu->sense_data_len));
memcpy(tstat->buf, iu->sense_data, tstat->buf_valid_size);
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 2/4] scsi: hisi_sas: Fix incorrect delay values from magic-number cleanup
2026-10-09 3:32 [PATCH v3 0/4] scsi: hisi_sas: Some misc fixes Xingui Yang
2026-10-09 3:32 ` [PATCH v3 1/4] scsi: libsas: Fix out-of-bounds memcpy in sas_ssp_task_response() Xingui Yang
@ 2026-10-09 3:32 ` Xingui Yang
2026-10-09 3:32 ` [PATCH v3 3/4] scsi: hisi_sas: Clear PHY error counts on phyup Xingui Yang
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Xingui Yang @ 2026-10-09 3:32 UTC (permalink / raw)
To: mkp, jejb, john.garry, yanaijie
Cc: linux-scsi, linux-kernel, linuxarm, yangxingui, liuyonglong,
kangfenglong
Commit 4ca7fe99fc84 ("scsi: hisi_sas: Use macro instead of magic
number") replaced several delay constants with a single macro of
value 100. However, three sites originally used different values:
- reset_hw_v3_hw: udelay(50) -> udelay(100) [doubled]
- disable_phy_v3_hw: mdelay(50) -> mdelay(100) [doubled]
- disable_host_v3_hw: mdelay(10) -> mdelay(100) [10x longer]
The overly long delays on the PHY disable and host shutdown paths
increase boot and shutdown time for all local PHYs.
Add separate macros for each delay to restore the original values
while still avoiding magic numbers.
Fixes: 4ca7fe99fc84 ("scsi: hisi_sas: Use macro instead of magic number")
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 8a2500993e19..9c363e237353 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -547,6 +547,9 @@ struct hisi_sas_err_record_v3 {
#define IRQ_AXI_INDEX 11
#define DELAY_FOR_RESET_HW 100
+#define STOP_PHY_DELAY_US 50
+#define DISABLE_PHY_DELAY_MS 50
+#define DISABLE_HOST_PHY_DELAY_MS 10
#define HDR_SG_MOD 0x2
#define LUN_SIZE 8
#define ATTR_PRIO_REGION 9
@@ -983,7 +986,7 @@ static int reset_hw_v3_hw(struct hisi_hba *hisi_hba)
/* Disable all of the PHYs */
hisi_sas_stop_phys(hisi_hba);
- udelay(HISI_SAS_DELAY_FOR_PHY_DISABLE);
+ udelay(STOP_PHY_DELAY_US);
/* Ensure axi bus idle */
ret = hisi_sas_read32_poll_timeout(AXI_CFG, val, !val,
@@ -1072,7 +1075,7 @@ static void disable_phy_v3_hw(struct hisi_hba *hisi_hba, int phy_no)
cfg &= ~PHY_CFG_ENA_MSK;
hisi_sas_phy_write32(hisi_hba, phy_no, PHY_CFG, cfg);
- mdelay(HISI_SAS_DELAY_FOR_PHY_DISABLE);
+ mdelay(DISABLE_PHY_DELAY_MS);
state = hisi_sas_read32(hisi_hba, PHY_STATE);
if (state & BIT(phy_no)) {
@@ -2755,7 +2758,7 @@ static int disable_host_v3_hw(struct hisi_hba *hisi_hba)
hisi_sas_stop_phys(hisi_hba);
- mdelay(HISI_SAS_DELAY_FOR_PHY_DISABLE);
+ mdelay(DISABLE_HOST_PHY_DELAY_MS);
reg_val = hisi_sas_read32(hisi_hba, AXI_MASTER_CFG_BASE +
AM_CTRL_GLOBAL);
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 3/4] scsi: hisi_sas: Clear PHY error counts on phyup
2026-10-09 3:32 [PATCH v3 0/4] scsi: hisi_sas: Some misc fixes Xingui Yang
2026-10-09 3:32 ` [PATCH v3 1/4] scsi: libsas: Fix out-of-bounds memcpy in sas_ssp_task_response() Xingui Yang
2026-10-09 3:32 ` [PATCH v3 2/4] scsi: hisi_sas: Fix incorrect delay values from magic-number cleanup Xingui Yang
@ 2026-10-09 3:32 ` Xingui Yang
2026-10-09 3:32 ` [PATCH v3 4/4] scsi: hisi_sas: Fix spinup failure for SAS SSP devices in Active_Wait state Xingui Yang
2026-10-09 9:08 ` [PATCH v3 0/4] scsi: hisi_sas: Some misc fixes yangxingui
4 siblings, 0 replies; 6+ messages in thread
From: Xingui Yang @ 2026-10-09 3:32 UTC (permalink / raw)
To: mkp, jejb, john.garry, yanaijie
Cc: linux-scsi, linux-kernel, linuxarm, yangxingui, liuyonglong,
kangfenglong
Some disks generate link errors during link establishment but still
phy up successfully. These intermediate error counts are not cleared
after phyup, leaving stale data for error detection. Clear them on
phyup.
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 38 +++++++++++++++++---------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 9c363e237353..030f050731ba 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -280,6 +280,9 @@
#define CHL_INT2_RX_CODE_ERR_OFF 29
#define CHL_INT2_RX_INVLD_DW_OFF 30
#define CHL_INT2_STP_LINK_TIMEOUT_OFF 31
+#define CHL_INT2_RX_ERR_MSK (BIT(CHL_INT2_RX_DISP_ERR_OFF) | \
+ BIT(CHL_INT2_RX_CODE_ERR_OFF) | \
+ BIT(CHL_INT2_RX_INVLD_DW_OFF))
#define CHL_INT0_MSK (PORT_BASE + 0x1c0)
#define CHL_INT1_MSK (PORT_BASE + 0x1c4)
#define CHL_INT2_MSK (PORT_BASE + 0x1c8)
@@ -1061,16 +1064,31 @@ static void enable_phy_v3_hw(struct hisi_hba *hisi_hba, int phy_no)
hisi_sas_phy_write32(hisi_hba, phy_no, PHY_CFG, cfg);
}
+static void clear_phy_err_cnt_v3_hw(struct hisi_hba *hisi_hba, int phy_no)
+{
+ u32 irq_msk = hisi_sas_phy_read32(hisi_hba, phy_no, CHL_INT2_MSK);
+
+ hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT2_MSK,
+ CHL_INT2_RX_ERR_MSK | irq_msk);
+
+ hisi_sas_phy_read32(hisi_hba, phy_no, ERR_CNT_DWS_LOST);
+ hisi_sas_phy_read32(hisi_hba, phy_no, ERR_CNT_RESET_PROB);
+ hisi_sas_phy_read32(hisi_hba, phy_no, ERR_CNT_INVLD_DW);
+ hisi_sas_phy_read32(hisi_hba, phy_no, ERR_CNT_DISP_ERR);
+ hisi_sas_phy_read32(hisi_hba, phy_no, ERR_CNT_CODE_ERR);
+
+ hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT2, CHL_INT2_RX_ERR_MSK);
+ hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT2_MSK, irq_msk);
+}
+
static void disable_phy_v3_hw(struct hisi_hba *hisi_hba, int phy_no)
{
u32 cfg = hisi_sas_phy_read32(hisi_hba, phy_no, PHY_CFG);
u32 irq_msk = hisi_sas_phy_read32(hisi_hba, phy_no, CHL_INT2_MSK);
- static const u32 msk = BIT(CHL_INT2_RX_DISP_ERR_OFF) |
- BIT(CHL_INT2_RX_CODE_ERR_OFF) |
- BIT(CHL_INT2_RX_INVLD_DW_OFF);
u32 state;
- hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT2_MSK, msk | irq_msk);
+ hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT2_MSK,
+ CHL_INT2_RX_ERR_MSK | irq_msk);
cfg &= ~PHY_CFG_ENA_MSK;
hisi_sas_phy_write32(hisi_hba, phy_no, PHY_CFG, cfg);
@@ -1085,11 +1103,7 @@ static void disable_phy_v3_hw(struct hisi_hba *hisi_hba, int phy_no)
udelay(1);
- hisi_sas_phy_read32(hisi_hba, phy_no, ERR_CNT_INVLD_DW);
- hisi_sas_phy_read32(hisi_hba, phy_no, ERR_CNT_DISP_ERR);
- hisi_sas_phy_read32(hisi_hba, phy_no, ERR_CNT_CODE_ERR);
-
- hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT2, msk);
+ clear_phy_err_cnt_v3_hw(hisi_hba, phy_no);
hisi_sas_phy_write32(hisi_hba, phy_no, CHL_INT2_MSK, irq_msk);
}
@@ -1671,6 +1685,7 @@ static irqreturn_t phy_up_v3_hw(int phy_no, struct hisi_hba *hisi_hba)
/* Delete timer and set phy_attached atomically */
timer_delete(&phy->timer);
phy->phy_attached = 1;
+ clear_phy_err_cnt_v3_hw(hisi_hba, phy_no);
spin_unlock(&phy->lock);
/*
@@ -1894,9 +1909,6 @@ static void handle_chl_int2_v3_hw(struct hisi_hba *hisi_hba, int phy_no)
struct hisi_sas_phy *phy = &hisi_hba->phy[phy_no];
struct pci_dev *pci_dev = hisi_hba->pci_dev;
struct device *dev = hisi_hba->dev;
- static const u32 msk = BIT(CHL_INT2_RX_DISP_ERR_OFF) |
- BIT(CHL_INT2_RX_CODE_ERR_OFF) |
- BIT(CHL_INT2_RX_INVLD_DW_OFF);
irq_value &= ~irq_msk;
if (!irq_value) {
@@ -1920,7 +1932,7 @@ static void handle_chl_int2_v3_hw(struct hisi_hba *hisi_hba, int phy_no)
hisi_sas_notify_phy_event(phy, HISI_PHYE_LINK_RESET);
}
- if (pci_dev->revision > 0x20 && (irq_value & msk)) {
+ if (pci_dev->revision > 0x20 && (irq_value & CHL_INT2_RX_ERR_MSK)) {
struct asd_sas_phy *sas_phy = &phy->sas_phy;
struct sas_phy *sphy = sas_phy->phy;
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 4/4] scsi: hisi_sas: Fix spinup failure for SAS SSP devices in Active_Wait state
2026-10-09 3:32 [PATCH v3 0/4] scsi: hisi_sas: Some misc fixes Xingui Yang
` (2 preceding siblings ...)
2026-10-09 3:32 ` [PATCH v3 3/4] scsi: hisi_sas: Clear PHY error counts on phyup Xingui Yang
@ 2026-10-09 3:32 ` Xingui Yang
2026-10-09 9:08 ` [PATCH v3 0/4] scsi: hisi_sas: Some misc fixes yangxingui
4 siblings, 0 replies; 6+ messages in thread
From: Xingui Yang @ 2026-10-09 3:32 UTC (permalink / raw)
To: mkp, jejb, john.garry, yanaijie
Cc: linux-scsi, linux-kernel, linuxarm, yangxingui, liuyonglong,
kangfenglong
SAS HDDs powered up with RNOT=1 wait in Active_Wait state for a
NOTIFY(ENABLE SPINUP) primitive and keep returning NOT_READY with
ASC/ASCQ 0x04/0x11, causing endless mid-layer retries and spinup
failure.
Parse the sense data in the slot completion path and, when
LU_NOT_READY_NOTIFY_REQUIRED is returned by a directly-attached SSP
device, queue work that sends the primitive from process context
(sl_notify_ssp() contains msleep()). The work takes a runtime PM
reference with pm_runtime_get_if_active(), pairing with
pm_runtime_put_sync() in the work, so the controller stays resumed
until the primitive is sent.
Fixes: 60b4a5ee9034 ("scsi: hisi_sas: add v3 cq interrupt handler")
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
drivers/scsi/hisi_sas/hisi_sas.h | 3 ++
drivers/scsi/hisi_sas/hisi_sas_main.c | 43 ++++++++++++++++++++++++++
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 4 +++
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 4 +++
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 4 +++
5 files changed, 58 insertions(+)
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 1323ed8aa717..d48ba3747b81 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -163,6 +163,7 @@ enum hisi_sas_phy_event {
HISI_PHYE_PHY_UP = 0U,
HISI_PHYE_LINK_RESET,
HISI_PHYE_PHY_UP_PM,
+ HISI_PHYE_SPINUP_NOTIFY,
HISI_PHYES_NUM,
};
@@ -689,4 +690,6 @@ extern void hisi_sas_sync_cqs(struct hisi_hba *hisi_hba);
extern void hisi_sas_sync_poll_cqs(struct hisi_hba *hisi_hba);
extern void hisi_sas_controller_reset_prepare(struct hisi_hba *hisi_hba);
extern void hisi_sas_controller_reset_done(struct hisi_hba *hisi_hba);
+extern void hisi_sas_spinup_notify(struct hisi_hba *hisi_hba,
+ struct sas_task *task);
#endif
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 944ce19ae2fc..cf7aa0a699cd 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -996,10 +996,25 @@ static void hisi_sas_phyup_pm_work(struct work_struct *work)
pm_runtime_put_sync(dev);
}
+static void hisi_sas_spinup_notify_work(struct work_struct *work)
+{
+ struct hisi_sas_phy *phy =
+ container_of(work, typeof(*phy), works[HISI_PHYE_SPINUP_NOTIFY]);
+ struct hisi_hba *hisi_hba = phy->hisi_hba;
+ struct device *dev = hisi_hba->dev;
+ int phy_no = phy->sas_phy.id;
+
+ hisi_hba->hw->sl_notify_ssp(hisi_hba, phy_no);
+ dev_info(dev, "spinup notify primitive on phy%d\n", phy_no);
+
+ pm_runtime_put_sync(dev);
+}
+
static const work_func_t hisi_sas_phye_fns[HISI_PHYES_NUM] = {
[HISI_PHYE_PHY_UP] = hisi_sas_phyup_work,
[HISI_PHYE_LINK_RESET] = hisi_sas_linkreset_work,
[HISI_PHYE_PHY_UP_PM] = hisi_sas_phyup_pm_work,
+ [HISI_PHYE_SPINUP_NOTIFY] = hisi_sas_spinup_notify_work,
};
bool hisi_sas_notify_phy_event(struct hisi_sas_phy *phy,
@@ -1639,6 +1654,34 @@ void hisi_sas_controller_reset_done(struct hisi_hba *hisi_hba)
}
EXPORT_SYMBOL_GPL(hisi_sas_controller_reset_done);
+void hisi_sas_spinup_notify(struct hisi_hba *hisi_hba,
+ struct sas_task *task)
+{
+ struct task_status_struct *ts = &task->task_status;
+ struct domain_device *dev = task->dev;
+ struct scsi_sense_hdr sshdr;
+ struct sas_phy *local_phy;
+ struct hisi_sas_phy *phy;
+
+ if (!scsi_normalize_sense(ts->buf, ts->buf_valid_size, &sshdr))
+ return;
+
+ if (sshdr.sense_key != NOT_READY ||
+ sshdr.sense_code != LU_NOT_READY_NOTIFY_REQUIRED)
+ return;
+
+ if (!pm_runtime_get_if_active(hisi_hba->dev))
+ return;
+
+ local_phy = sas_get_local_phy(dev);
+ phy = &hisi_hba->phy[local_phy->number];
+ if (!hisi_sas_notify_phy_event(phy, HISI_PHYE_SPINUP_NOTIFY))
+ pm_runtime_put(hisi_hba->dev);
+
+ sas_put_local_phy(local_phy);
+}
+EXPORT_SYMBOL_GPL(hisi_sas_spinup_notify);
+
static int hisi_sas_controller_prereset(struct hisi_hba *hisi_hba)
{
if (!hisi_hba->hw->soft_reset)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index fa94d7110714..5fcb8d5002f5 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1272,6 +1272,10 @@ static void slot_complete_v1_hw(struct hisi_hba *hisi_hba,
&status_buffer->iu[0];
sas_ssp_task_response(dev, task, iu);
+ if (unlikely(ts->stat == SAS_SAM_STAT_CHECK_CONDITION &&
+ !dev_parent_is_expander(device)))
+ hisi_sas_spinup_notify(hisi_hba, task);
+
break;
}
case SAS_PROTOCOL_SMP:
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index f3516a0611dd..f16896e962fd 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -2427,6 +2427,10 @@ static void slot_complete_v2_hw(struct hisi_hba *hisi_hba,
&status_buffer->iu[0];
sas_ssp_task_response(dev, task, iu);
+ if (unlikely(ts->stat == SAS_SAM_STAT_CHECK_CONDITION &&
+ !dev_parent_is_expander(device)))
+ hisi_sas_spinup_notify(hisi_hba, task);
+
break;
}
case SAS_PROTOCOL_SMP:
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 030f050731ba..9391d031f5df 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2447,6 +2447,10 @@ static void slot_complete_v3_hw(struct hisi_hba *hisi_hba,
sizeof(struct hisi_sas_err_record);
sas_ssp_task_response(dev, task, iu);
+ if (unlikely(ts->stat == SAS_SAM_STAT_CHECK_CONDITION &&
+ !dev_parent_is_expander(device)))
+ hisi_sas_spinup_notify(hisi_hba, task);
+
break;
}
case SAS_PROTOCOL_SMP: {
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3 0/4] scsi: hisi_sas: Some misc fixes
2026-10-09 3:32 [PATCH v3 0/4] scsi: hisi_sas: Some misc fixes Xingui Yang
` (3 preceding siblings ...)
2026-10-09 3:32 ` [PATCH v3 4/4] scsi: hisi_sas: Fix spinup failure for SAS SSP devices in Active_Wait state Xingui Yang
@ 2026-10-09 9:08 ` yangxingui
4 siblings, 0 replies; 6+ messages in thread
From: yangxingui @ 2026-10-09 9:08 UTC (permalink / raw)
To: mkp, jejb, john.garry, yanaijie
Cc: linux-scsi, linux-kernel, linuxarm, liuyonglong, kangfenglong
Hi Martin, James,
We have gone through all of the Sashiko review comments on v2 and v3
one by one. Of the six v2 findings, two were fixed in v3 (clearing
all five error counters and taking the runtime PM reference with
pm_runtime_get_if_active()), and one was answered in the v3 commit
message (the software counters are cumulative statistics and are not
reset by design). The rest are pre-existing issues, summarized below
by trigger condition and impact:
- The delays in the phy disable and host shutdown paths predate this
series. The 50 ms wait for the PHY to go down is a hardware
requirement, and a poll-based rework is planned.
- The pm8001 and mvsas response-IU findings are pre-existing issues
in those drivers. The libsas patch leaves these cases unchanged
and strictly narrows the worst case, and fixing them is outside the
scope of this series.
- The suspend flush-ordering window from the v2 review also predates
this series.
- The suggested CHL_INT2 clear order would trade delayed reporting of
error counters for actual data loss.
- The negative-return concern of pm_runtime_get_if_active() cannot be
triggered on this driver, and the suggested check would disable the
fix on !CONFIG_PM builds.
In short, none of the remaining findings is a regression introduced
by this series.
One concern to raise: Sashiko reviews every posting from scratch, and
its findings change from round to round. Of the six v2 findings, one
was re-raised in v3 and three were not raised again, while six new
ones appeared (7 findings on all 4 patches for v3). Iterating on its
comments alone will not converge.
Could you let us know whether this series is fine to be merged as is,
and how you would prefer the pre-existing findings to be handled: in
a follow-up series, or folded into this one?
Thanks,
Xingui
On 2026/10/9 11:32, Xingui Yang wrote:
> This series contains three independent fixes for the hisi_sas driver,
> plus a libsas fix found during review.
>
> Patch 1 fixes an out-of-bounds memcpy in sas_ssp_task_response(): a
> device-provided sense_data_len >= 0x80000000 turns negative in the
> min_t(int, ...) clamp and becomes a huge memcpy length.
>
> Patch 2 fixes incorrect delay values introduced by a previous
> magic-number cleanup commit. Three delay sites were changed to use
> a single macro (value 100) which did not match their original values,
> causing increased boot and shutdown time.
>
> Patch 3 clears stale PHY error counts on phyup so that error detection
> is not misled by intermediate errors generated during link
> establishment.
>
> Patch 4 fixes spinup failures of directly-attached SAS HDDs that power
> up in the Active_Wait state and wait for a NOTIFY(ENABLE SPINUP)
> primitive before becoming ready: parse the sense data in the slot
> completion path and send the primitive from deferred work.
>
> Changes from v2:
> Addresses the Sashiko AI review of v2.
> - Clear all five error counter registers.
> - Use pm_runtime_get_if_active().
>
> Changes from v1:
> Addresses the Sashiko AI review of v1.
> - Add Patch 1, found in review of the sense parsing of patch 4.
> - Clear the counters under phy->lock.
> - Take a runtime PM reference for the deferred work
>
> The remaining review findings are pre-existing issues, left for
> separate fixes in the future.
>
> Xingui Yang (4):
> scsi: libsas: Fix out-of-bounds memcpy in sas_ssp_task_response()
> scsi: hisi_sas: Fix incorrect delay values from magic-number cleanup
> scsi: hisi_sas: Clear PHY error counts on phyup
> scsi: hisi_sas: Fix spinup failure for SAS SSP devices in Active_Wait
> state
>
> drivers/scsi/hisi_sas/hisi_sas.h | 3 ++
> drivers/scsi/hisi_sas/hisi_sas_main.c | 43 ++++++++++++++++++++++
> drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 4 ++
> drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 4 ++
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 51 ++++++++++++++++++--------
> drivers/scsi/libsas/sas_task.c | 2 +-
> 6 files changed, 90 insertions(+), 17 deletions(-)
>
^ permalink raw reply [flat|nested] 6+ messages in thread