[PATCH v3 0/2] scsi: libsas: Support spinup notification for SAS devices

[PATCH v3 0/2] scsi: libsas: Support spinup notification for SAS devices

[Xingui Yang]

When a SAS device is in the Active_Wait or Idle_Wait power state, it
returns NOT_READY with ASC/ASCQ = 0x04/0x11 (notify (enable spinup)
required), indicating that a NOTIFY(ENABLE SPINUP) primitive is needed to
trigger media spinup.

Without handling this condition, the SCSI mid-layer will indefinitely retry
the command with ACTION_DELAYED_RETRY, resulting in the disk never spinning
up and becoming unusable. A typical manifestation is:

  sd 4:0:9:0: [sde] Spinning up disk...
  ...not responding...
  sd 4:0:9:0: [sde] Sense Key : Not Ready
  sd 4:0:9:0: [sde] Add. Sense: Logical unit not ready, notify (enable spinup) required

To resolve this, the SAS controller needs to send a NOTIFY(ENABLE SPINUP)
primitive to the target phy, which transitions the device out of the
waiting state and allows normal spinup to proceed.

This patch series addresses the issue entirely within the SAS transport
layer (libsas), reusing the existing phy event framework, without modifying
the generic SCSI mid-layer. This addresses the review feedback from
John Garry on v2.

Changes in v3 (addressing John Garry's review on v2):
- Move spinup notification from scsi_host_template to libsas. Reuse
  the existing phy event framework: add PHYE_NOTIFY_ENABLE_SPINUP
  and lldd_notify_enable_spinup callback.
- Sense detection in sas_ssp_task_response() covers all SAS LLDDs.

Changes in v2 (addressing Sashiko AI review on v1):
- Add softirq context documentation to spinup_notify in
  scsi_host.h
- Defer sl_notify_ssp() to ordered workqueue, fixing
  msleep-in-atomic bug, preventing RMW races on SL_CONTROL, and
  deduplicating concurrent callbacks via queue_work()

Xingui Yang (2):
  scsi: libsas: Add PHYE_NOTIFY_ENABLE_SPINUP phy event for
    ASC/ASCQ=0x04/0x11
  scsi: hisi_sas: Add lldd_notify_enable_spinup callback for SAS devices

 drivers/scsi/hisi_sas/hisi_sas_main.c | 11 +++++++
 drivers/scsi/libsas/sas_internal.h    |  2 ++
 drivers/scsi/libsas/sas_phy.c         | 12 +++++++
 drivers/scsi/libsas/sas_scsi_host.c   | 45 +++++++++++++++++++++++++++
 drivers/scsi/libsas/sas_task.c        |  2 ++
 include/scsi/libsas.h                 |  9 ++++++
 6 files changed, 81 insertions(+)

-- 
2.43.0

[PATCH v3 1/2] scsi: libsas: Add PHYE_NOTIFY_ENABLE_SPINUP phy event for ASC/ASCQ=0x04/0x11

[Xingui Yang]

When a SAS device is in the Active_Wait or Idle_Wait power state, it
returns NOT_READY with ASC/ASCQ = 0x04/0x11 (notify (enable spinup)
required), indicating that a NOTIFY(ENABLE SPINUP) primitive is needed to
trigger media spinup.

Add a PHYE_NOTIFY_ENABLE_SPINUP phy event and an optional
lldd_notify_enable_spinup callback to sas_domain_function_template. Sense
detection is done in sas_ssp_task_spinup_notify(), called from
sas_ssp_task_response() which is the common entry point for all SAS LLDDs.

Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
 drivers/scsi/libsas/sas_internal.h  |  2 ++
 drivers/scsi/libsas/sas_phy.c       | 12 ++++++++
 drivers/scsi/libsas/sas_scsi_host.c | 45 +++++++++++++++++++++++++++++
 drivers/scsi/libsas/sas_task.c      |  2 ++
 include/scsi/libsas.h               |  9 ++++++
 5 files changed, 70 insertions(+)

diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index 7dce0f587149..fa06f50b0bf0 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -107,6 +107,8 @@ extern const work_func_t sas_port_event_fns[PORT_NUM_EVENTS];
 
 void sas_task_internal_done(struct sas_task *task);
 void sas_task_internal_timedout(struct timer_list *t);
+void sas_ssp_task_spinup_notify(struct sas_task *task,
+				struct ssp_response_iu *iu);
 int sas_execute_tmf(struct domain_device *device, void *parameter,
 		    int para_len, int force_phy_id,
 		    struct sas_tmf_task *tmf);
diff --git a/drivers/scsi/libsas/sas_phy.c b/drivers/scsi/libsas/sas_phy.c
index 58f08dc2c187..897a5b46db78 100644
--- a/drivers/scsi/libsas/sas_phy.c
+++ b/drivers/scsi/libsas/sas_phy.c
@@ -111,6 +111,17 @@ static void sas_phye_shutdown(struct work_struct *work)
 	phy->in_shutdown = 0;
 }
 
+static void sas_phye_notify_enable_spinup(struct work_struct *work)
+{
+	struct asd_sas_event *ev = to_asd_sas_event(work);
+	struct asd_sas_phy *phy = ev->phy;
+	struct sas_ha_struct *sas_ha = phy->ha;
+	struct sas_internal *i =
+		to_sas_internal(sas_ha->shost->transportt);
+
+	i->dft->lldd_notify_enable_spinup(phy);
+}
+
 /* ---------- Phy class registration ---------- */
 
 int sas_register_phys(struct sas_ha_struct *sas_ha)
@@ -186,4 +197,5 @@ const work_func_t sas_phy_event_fns[PHY_NUM_EVENTS] = {
 	[PHYE_SPINUP_HOLD] = sas_phye_spinup_hold,
 	[PHYE_RESUME_TIMEOUT] = sas_phye_resume_timeout,
 	[PHYE_SHUTDOWN] = sas_phye_shutdown,
+	[PHYE_NOTIFY_ENABLE_SPINUP] = sas_phye_notify_enable_spinup,
 };
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index c83282733ec4..bb1dfc16d8d1 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -34,6 +34,51 @@
 #include <linux/scatterlist.h>
 #include <linux/libata.h>
 
+/*
+ * If the SSP response carries NOT_READY sense with ASC/ASCQ = 0x04/0x11
+ * ("notify (enable spinup) required"), queue a PHYE_NOTIFY_ENABLE_SPINUP
+ * phy event so the LLDD can send a NOTIFY(ENABLE SPINUP) primitive.
+ */
+void sas_ssp_task_spinup_notify(struct sas_task *task,
+				struct ssp_response_iu *iu)
+{
+	struct domain_device *dev = task->dev;
+	struct sas_ha_struct *ha = dev->port->ha;
+	struct sas_internal *i = to_sas_internal(ha->shost->transportt);
+	struct scsi_sense_hdr sshdr;
+	struct sas_phy *local_phy;
+	struct asd_sas_phy *phy;
+	u32 sense_len;
+
+	/*
+	 * NOTIFY(ENABLE SPINUP) must be sent on the local phy directly
+	 * attached to the target. Skip expander-attached devices.
+	 */
+	if (dev->parent && dev_is_expander(dev->parent->dev_type))
+		return;
+
+	if (!i->dft->lldd_notify_enable_spinup)
+		return;
+
+	if (iu->status != SAM_STAT_CHECK_CONDITION)
+		return;
+
+	sense_len = min_t(u32, be32_to_cpu(iu->sense_data_len),
+			  SAS_STATUS_BUF_SIZE);
+	if (!scsi_normalize_sense(iu->sense_data, sense_len, &sshdr))
+		return;
+
+	if (sshdr.sense_key != NOT_READY ||
+	    sshdr.asc != 0x04 || sshdr.ascq != 0x11)
+		return;
+
+	local_phy = sas_get_local_phy(dev);
+	phy = ha->sas_phy[local_phy->number];
+	sas_put_local_phy(local_phy);
+
+	sas_notify_phy_event(phy, PHYE_NOTIFY_ENABLE_SPINUP, GFP_ATOMIC);
+}
+
 /* record final status and free the task */
 static void sas_end_task(struct scsi_cmnd *sc, struct sas_task *task)
 {
diff --git a/drivers/scsi/libsas/sas_task.c b/drivers/scsi/libsas/sas_task.c
index e9d291007817..d1eb6ce90626 100644
--- a/drivers/scsi/libsas/sas_task.c
+++ b/drivers/scsi/libsas/sas_task.c
@@ -29,6 +29,8 @@ void sas_ssp_task_response(struct device *dev, struct sas_task *task,
 			      be32_to_cpu(iu->sense_data_len));
 		memcpy(tstat->buf, iu->sense_data, tstat->buf_valid_size);
 
+		sas_ssp_task_spinup_notify(task, iu);
+
 		if (iu->status != SAM_STAT_CHECK_CONDITION)
 			dev_warn(dev, "dev %016llx sent sense data, but stat(0x%x) is not CHECK CONDITION\n",
 				 SAS_ADDR(task->dev->sas_addr), iu->status);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 163f23c92b41..945b7cfe0224 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -49,6 +49,7 @@ enum phy_event {
 	PHYE_SPINUP_HOLD,             /* hot plug SATA, no COMWAKE sent */
 	PHYE_RESUME_TIMEOUT,
 	PHYE_SHUTDOWN,
+	PHYE_NOTIFY_ENABLE_SPINUP,    /* NOTIFY(ENABLE SPINUP) primitive */
 	PHY_NUM_EVENTS,
 };
 
@@ -674,6 +675,14 @@ struct sas_domain_function_template {
 	/* GPIO support */
 	int (*lldd_write_gpio)(struct sas_ha_struct *, u8 reg_type,
 			       u8 reg_index, u8 reg_count, u8 *write_data);
+
+	/*
+	 * Optional callback invoked when an SSP target returns NOT_READY
+	 * with ASC/ASCQ = 0x04/0x11 ("notify (enable spinup) required"),
+	 * indicating the device is in Active_Wait/Idle_Wait state and
+	 * needs a NOTIFY(ENABLE SPINUP) primitive to proceed.
+	 */
+	void (*lldd_notify_enable_spinup)(struct asd_sas_phy *phy);
 };
 
 extern int sas_register_ha(struct sas_ha_struct *);
-- 
2.43.0

[PATCH v3 2/2] scsi: hisi_sas: Add lldd_notify_enable_spinup callback for SAS devices

[Xingui Yang]

Register the lldd_notify_enable_spinup callback. The callback sends a
NOTIFY(ENABLE SPINUP) primitive via sl_notify_ssp() to trigger media spinup
on SAS devices in Active_Wait or Idle_Wait state.

Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 944ce19ae2fc..a726d6680386 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -2081,6 +2081,16 @@ static int hisi_sas_write_gpio(struct sas_ha_struct *sha, u8 reg_type,
 				reg_index, reg_count, write_data);
 }
 
+static void hisi_sas_notify_enable_spinup(struct asd_sas_phy *phy)
+{
+	struct hisi_sas_phy *hisi_phy =
+		container_of(phy, struct hisi_sas_phy, sas_phy);
+	struct hisi_hba *hisi_hba = hisi_phy->hisi_hba;
+
+	hisi_hba->hw->sl_notify_ssp(hisi_hba, phy->id);
+	dev_info(hisi_hba->dev, "phy%d notify enable spinup\n", phy->id);
+}
+
 static void hisi_sas_phy_disconnected(struct hisi_sas_phy *phy)
 {
 	struct asd_sas_phy *sas_phy = &phy->sas_phy;
@@ -2180,6 +2190,7 @@ static struct sas_domain_function_template hisi_sas_transport_ops = {
 	.lldd_write_gpio	= hisi_sas_write_gpio,
 	.lldd_tmf_aborted	= hisi_sas_tmf_aborted,
 	.lldd_abort_timeout	= hisi_sas_internal_abort_timeout,
+	.lldd_notify_enable_spinup = hisi_sas_notify_enable_spinup,
 };
 
 void hisi_sas_init_mem(struct hisi_hba *hisi_hba)
-- 
2.43.0

Re: [PATCH v3 2/2] scsi: hisi_sas: Add lldd_notify_enable_spinup callback for SAS devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing check for controller reset state before accessing hardware registers in the `hisi_sas_notify_enable_spinup` callback.

Pre-existing issues:
- [High] Race condition on SL_CONTROL register read-modify-write (RMW) operations between process and interrupt contexts without locking.
--

--- Patch [2]: [PATCH v3 2/2] scsi: hisi_sas: Add lldd_notify_enable_spinup callback for SAS devices ---
commit 1a996ce0b10b1f8ffbc88dbdf06f98375a589a5c
Author: Xingui Yang <yangxingui@huawei.com>

scsi: hisi_sas: Add lldd_notify_enable_spinup callback for SAS devices
    
This commit registers the lldd_notify_enable_spinup callback for hisi_sas.
The callback sends a NOTIFY(ENABLE SPINUP) primitive to trigger media spinup
on SAS devices in Active_Wait or Idle_Wait state.

> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index 944ce19ae2fc1..a726d66803865 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -2081,6 +2081,16 @@ static int hisi_sas_write_gpio(struct sas_ha_struct *sha, u8 reg_type,
>  				reg_index, reg_count, write_data);
>  }
>  
> +static void hisi_sas_notify_enable_spinup(struct asd_sas_phy *phy)
> +{
> +	struct hisi_sas_phy *hisi_phy =
> +		container_of(phy, struct hisi_sas_phy, sas_phy);
> +	struct hisi_hba *hisi_hba = hisi_phy->hisi_hba;
> +
> +	hisi_hba->hw->sl_notify_ssp(hisi_hba, phy->id);

[Severity: High]
Should this code check the controller reset state before accessing the
hardware registers?

If a controller reset is triggered concurrently (e.g., via PCIe FLR) and
sets HISI_SAS_RESETTING_BIT, calling sl_notify_ssp() performs MMIO writes
while the hardware is resetting. This could trigger a Synchronous External
Abort (SEA) and kernel panic on ARM platforms.

Other callbacks like hisi_sas_phy_bcast() use
test_bit(HISI_SAS_RESETTING_BIT, &hisi_hba->flags) to prevent this. Would
it be appropriate to add a similar check here?


[Severity: High]
This is a pre-existing issue, but does calling sl_notify_ssp() here
expose an unprotected read-modify-write race on the SL_CONTROL register?

The hardware-specific callback sl_notify_ssp_v3_hw() runs in process
context and modifies the register:

drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:sl_notify_ssp_v3_hw() {
    ...
    sl_control = hisi_sas_phy_read32(hisi_hba, phy_no, SL_CONTROL);
    sl_control |= SL_CONTROL_NOTIFY_EN_MSK;
    hisi_sas_phy_write32(hisi_hba, phy_no, SL_CONTROL, sl_control);
}

However, when a PHY goes down, phy_down_v3_hw() modifies the same register
in hardirq context:

drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:phy_down_v3_hw() {
    ...
    sl_ctrl = hisi_sas_phy_read32(hisi_hba, phy_no, SL_CONTROL);
    hisi_sas_phy_write32(hisi_hba, phy_no, SL_CONTROL,
                         sl_ctrl&(~SL_CTA_MSK));
}

If the hardirq preempts the process-context worker exactly between the read
and write in sl_notify_ssp_v3_hw(), the worker will subsequently write back
its stale read value, undoing the interrupt's modification. Can this corrupt
the PHY state machine?

> +	dev_info(hisi_hba->dev, "phy%d notify enable spinup\n", phy->id);
> +}
> +

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803020536.546544-1-yangxingui@huawei.com?part=2

Re: [PATCH v3 1/2] scsi: libsas: Add PHYE_NOTIFY_ENABLE_SPINUP phy event for ASC/ASCQ=0x04/0x11

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but it is a critical vulnerability. In `sas_ssp_task_response()`, the `min_t` macro incorrectly casts `be32_to_cpu(iu->sense_data_len)` to a signed `int`. If a malicious or malfunctioning SAS device sends a very large `sense_data_len` (e.g., 0xFFFFFFFF), the cast to `int` results in a negative value (e.g., -1). `min_t` evaluates to this negative number, which is stored in `tstat->buf_valid_size`. When passed to `memcpy()`, the negative size is implicitly cast to `size_t`, becoming `SIZE_MAX`. This causes a massive heap buffer overflow when copying from `iu->sense_data`.
--

--- Patch [1]: [PATCH v3 1/2] scsi: libsas: Add PHYE_NOTIFY_ENABLE_SPINUP phy event for ASC/ASCQ=0x04/0x11 ---
commit 04d93ab7400ada2fff2a532b87fdf708e22b793a
Author: Xingui Yang <yangxingui@huawei.com>

scsi: libsas: Add PHYE_NOTIFY_ENABLE_SPINUP phy event for ASC/ASCQ=0x04/0x11

This patch adds a new PHY event to handle ASC/ASCQ 0x04/0x11 for SAS devices
requiring a NOTIFY(ENABLE SPINUP) primitive. It introduces sense detection in
the common task response path and an optional LLDD callback.

> diff --git a/drivers/scsi/libsas/sas_task.c b/drivers/scsi/libsas/sas_task.c
> index e9d291007817b..d1eb6ce90626e 100644
> --- a/drivers/scsi/libsas/sas_task.c
> +++ b/drivers/scsi/libsas/sas_task.c
> @@ -29,6 +29,8 @@ void sas_ssp_task_response(struct device *dev, struct sas_task *task,
>  			      be32_to_cpu(iu->sense_data_len));
>  		memcpy(tstat->buf, iu->sense_data, tstat->buf_valid_size);

[Severity: Critical]
This is a pre-existing issue, but does this min_t() cast to signed int expose
the kernel to a heap buffer overflow?

If a malicious or malfunctioning SAS device returns an SSP response IU with a
very large sense_data_len (e.g., 0xFFFFFFFF), casting it to 'int' results in a
negative value.

This negative value would be stored in tstat->buf_valid_size, and when passed
to memcpy(), it is implicitly cast to size_t, becoming SIZE_MAX.

>  
> +		sas_ssp_task_spinup_notify(task, iu);
> +
>  		if (iu->status != SAM_STAT_CHECK_CONDITION)
>  			dev_warn(dev, "dev %016llx sent sense data, but stat(0x%x) is not CHECK CONDITION\n",
>  				 SAS_ADDR(task->dev->sas_addr), iu->status);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803020536.546544-1-yangxingui@huawei.com?part=1