[PATCH v6 0/6] ata: libata-scsi: multi-LUN ATAPI device support

[PATCH v6 0/6] ata: libata-scsi: multi-LUN ATAPI device support

[Phil Pemberton]

Hi all,

This is v6, addressing review feedback from Hannes Reinecke, Damien Le
Moal, and the Sashiko AI code review on v5.

This series gives libata support for ATAPI devices with multiple LUNs,
such as the Panasonic PD-1 PD/CD combo drive.  This exposes both the
CD-ROM and rewritable PD optical interfaces: CD-ROM as LUN 0 and PD
as LUN 1.

libata has never supported multi-LUN ATAPI.  This series adds support
by fixing the following limitations:

  1. shost->max_lun is hardcoded to 1 in ata_scsi_add_hosts(), preventing
     the SCSI layer from probing any LUN beyond 0.

  2. __ata_scsi_find_dev() rejects all commands where scsidev->lun != 0,
     returning NULL and resulting in DID_BAD_TARGET.

  3. The SCSI-2 CDB LUN field (byte 1, bits 7:5) is never populated.
     ATAPI tunnels SCSI commands over the ATA PACKET interface, and the
     transport-layer LUN addressing used by SPC-3+ is not available.
     Older multi-LUN ATAPI devices rely on this CDB field to route
     commands to the correct LUN.

  4. ata_scsi_scan_host() only calls __scsi_add_device() for LUN 0,
     never probing additional LUNs even when the SCSI device info table
     would indicate the device supports them.

  5. dev->sdev is a single pointer, but multi-LUN ATAPI puts multiple
     sdevs behind one ata_device.  Every call to ata_scsi_dev_config()
     overwrote the pointer, and ata_scsi_sdev_destroy() tore down the
     entire ATA device whenever any sdev was destroyed -- so removing a
     spurious LUN result during scanning would kill the whole port, and
     the other users of dev->sdev (scsi_remove_device in
     ata_port_detach(), ACPI uevents, zpodd, media-change notify,
     suspend/resume rescan) could only ever see one LUN.

Changes from v5:

  - Drop dev->nr_luns entirely (Hannes Reinecke): nr_luns was an
    unnecessary counter tracking how many dev->sdev[] slots were valid.
    All loop bounds now use ATAPI_MAX_LUN directly, relying on NULL
    checks to skip empty slots.  ata_dev_scsi_device() uses ATAPI_MAX_LUN
    in its WARN_ON_ONCE bound.  The pre-scan assignment
    "dev->nr_luns = ap->scsi_host->max_lun" in ata_scsi_scan_host() is
    also removed (it had been needed to let atapi_xlat() accept INQUIRY
    CDBs during scsi_scan_target(); that hack is no longer necessary).

  - Restructure CDB LUN encoding in atapi_xlat() (Damien Le Moal):
    always clear CDB byte 1 bits 7:5; only set them for nonzero LUNs
    (with unlikely()).  Bounds check uses host->max_lun vs ATAPI_MAX_LUN.
    Also set scmd->result = DID_ERROR << 16 before returning non-zero so
    the error is not silently dropped by the ata_scsi_translate() done
    path (Sashiko AI review).

  - ata_scsi_scan_host: early-continue after LUN 0 failure (Damien Le
    Moal): clear dev->sdev[0] = NULL and continue to the next device
    rather than goto when __scsi_add_device() fails.

  - ata_scsi_dev_rescan: split get-all from resume/rescan (Damien Le
    Moal): reference collection, do_resume initialisation, and the
    resume/rescan loop are now three separate steps, with leftover refs
    released on EWOULDBLOCK and error paths.

  - pdt_1f_for_no_lun set before the PDT=0x1f check (Sashiko AI
    review): in scsi_probe_and_add_lun(), set starget->pdt_1f_for_no_lun
    from bflags before the PDT=0x1f early-return rather than inside
    scsi_add_lun() after it.

  - ata_port_detach: snapshot dev->sdev[lun] under the lock before
    releasing it, then use the snapshot for scsi_remove_device() (Sashiko
    AI review).  Mirrors the existing pattern in ata_scsi_remove_dev().

  - ata_scsi_sdev_destroy: set ATA_DFLAG_DETACH only when all
    ATAPI_MAX_LUN slots are NULL, regardless of LUN removal order
    (Sashiko AI review).

  - ata_scsi_scan_host: move ata_scsi_assign_ofnode() before
    scsi_device_put() to avoid a potential use-after-free (Sashiko AI
    review).

  - MODULE_PARM_DESC for atapi_max_lun clarified (Damien Le Moal).

  - Style: braces on the single-statement else branch in
    ata_acpi_uevent(); braces on the ata_scsi_media_change_notify() loop
    body; store ata_dev_scsi_device() result in a local before
    dereferencing in zpodd_wake_dev() (Damien Le Moal / Sashiko AI
    review).

The series is split as:

  1/6: libata-scsi: add libata.atapi_max_lun module parameter.

  2/6: libata-scsi: convert dev->sdev to a per-LUN array, add the
       ata_dev_scsi_device() helper, and update every caller.

  3/6: libata-scsi: relax __ata_scsi_find_dev() to accept non-zero LUN
       for ATAPI devices, and encode the LUN in CDB byte 1 bits 7:5.

  4/6: scsi: add a BLIST_NO_LUN_1F blacklist flag, which sets
       scsi_target.pdt_1f_for_no_lun for matching devices so that
       PDT 0x1f / PQ 0 INQUIRY responses are treated as "LUN not
       present" and silently skipped.

  5/6: libata-scsi: after adding LUN 0, trigger scsi_scan_target() for
       BLIST_FORCELUN ATAPI devices only.  Single-LUN devices are
       completely unaffected.

  6/6: scsi_devinfo: add the COMPAQ-branded variant of the PD-1 to the
       device info table with BLIST_FORCELUN | BLIST_SINGLELUN |
       BLIST_NO_LUN_1F.

Tested on a Panasonic LF-1195C PD/CD (Compaq branded) attached to an
ata_piix host on i686, kernel 7.0.0-rc7+, with libata.atapi_max_lun=7.
Both LUNs enumerate correctly: the CD-ROM as sr0 and the PD as sda.
Reads from each device succeed against the appropriate media.
Non-responding LUNs are silently skipped (no spurious "No Device"
entries in dmesg).  An iHAS124 DVD writer on the same machine
(single-LUN, no BLIST_FORCELUN entry) is unaffected: only LUN 0 is
scanned.

Two known limitations around media-change detection on multi-LUN
ATAPI devices with a shared physical media slot (e.g. PD/CD combos
flagged BLIST_SINGLELUN):

1. The block layer disables in-kernel polling by default
   (block.events_dfl_poll_msecs defaults to 0).  Without polling,
   sd_check_events never runs and media insertion on the PD LUN is
   not detected automatically.  sr_mod is unaffected because it
   re-reads the TOC on every open.

   Workaround -- either globally via kernel boot parameter:

       block.events_dfl_poll_msecs=2000

   or per-device via udev rule:

       ACTION=="add", KERNEL=="sd*", \
         ATTRS{vendor}=="COMPAQ  ", ATTRS{model}=="PD-1*", \
         ATTR{events_poll_msecs}="2000"

2. Media-change sense is not propagated across sibling LUNs.  When
   one LUN reports UNIT ATTENTION (ASC 0x28 or 0x3A), the other LUNs
   are not notified.  With polling enabled, sd_check_events detects
   the change independently on each LUN via TUR, so this is mainly a
   latency issue rather than a functional one.  A follow-up to
   propagate media-change events to sibling LUNs in
   atapi_qc_complete is straightforward but deferred to keep this
   series focused on the LUN-scanning core.

Suspend/resume with multi-LUN ATAPI attached has not yet been tested;
this is on the list.  ata_scsi_dev_rescan iterates all populated LUN
slots, and the SCSI layer's host-level suspend tracking already
serialises port quiesce, so no additional per-LUN suspend counting
is needed in libata.

v5 collected Reviewed-by from Damien Le Moal and Hannes Reinecke on
patches 4/6 and 6/6, and from Hannes Reinecke on 1/6.  The tags on
1/6 and 6/6 are preserved as those patches are unchanged.  The tags on
4/6 have been dropped as that patch gained a scsi_scan.c change
(moving the pdt_1f_for_no_lun assignment earlier).

Comments and suggestions welcome.

Phil Pemberton (6):
  ata: libata-scsi: add atapi_max_lun module parameter
  ata: libata-scsi: convert dev->sdev to per-LUN array
  ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI
  scsi: add BLIST_NO_LUN_1F blacklist flag
  ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices
  scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

 drivers/ata/libata-acpi.c   |   6 +-
 drivers/ata/libata-core.c   |  16 ++-
 drivers/ata/libata-scsi.c   | 208 +++++++++++++++++++++++-------------
 drivers/ata/libata-zpodd.c  |   6 +-
 drivers/ata/libata.h        |   1 +
 drivers/scsi/scsi_devinfo.c |   2 +
 drivers/scsi/scsi_scan.c    |   2 +
 include/linux/libata.h      |  12 ++-
 include/scsi/scsi_devinfo.h |   6 +-
 9 files changed, 171 insertions(+), 88 deletions(-)


base-commit: a3f75e5e6b023958c92ad03fa2e68e047b6169c4
--
2.43.0

[PATCH v6 1/6] ata: libata-scsi: add atapi_max_lun module parameter

[Phil Pemberton]

Until now libata has hard-coded shost->max_lun = 1 for every ATA host,
so the SCSI layer never scans past LUN 0.  This blocks support for
the small handful of multi-LUN ATAPI devices (Panasonic LF-1195C and
COMPAQ PD-1 PD/CD combos export CD on LUN 0 and PD on LUN 1; old
Nakamichi MJ-x.y CD changers expose one LUN per disc slot, up to 7).

Introduce a libata module parameter, atapi_max_lun, that controls the
upper bound of the per-host SCSI LUN scan.  Default is 1, preserving
current behaviour exactly: out-of-the-box only LUN 0 is scanned.
Range is clamped to 1..ATAPI_MAX_LUN (8, the SCSI-2 ceiling).

Subsequent patches gate actual LUN>0 probing on BLIST_FORCELUN, so a
device must both be on the SCSI device list (or carry the appropriate
quirk) and run on a host whose atapi_max_lun has been raised before
any extra LUNs are scanned.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-core.c | 5 +++++
 drivers/ata/libata-scsi.c | 2 +-
 drivers/ata/libata.h      | 1 +
 include/linux/libata.h    | 1 +
 4 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e76d15411e2a..4408b1fb48c7 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -122,6 +122,11 @@ int atapi_passthru16 = 1;
 module_param(atapi_passthru16, int, 0444);
 MODULE_PARM_DESC(atapi_passthru16, "Enable ATA_16 passthru for ATAPI devices (0=off, 1=on [default])");
 
+int atapi_max_lun = 1;
+module_param(atapi_max_lun, int, 0444);
+MODULE_PARM_DESC(atapi_max_lun,
+	"Maximum LUN to scan on ATAPI devices flagged BLIST_FORCELUN (1 [default] .. 7)");
+
 int libata_fua = 0;
 module_param_named(fua, libata_fua, int, 0444);
 MODULE_PARM_DESC(fua, "FUA support (0=off [default], 1=on)");
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index f44612e269a4..32c6a0e497cf 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4627,7 +4627,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
 		shost->transportt = &ata_scsi_transportt;
 		shost->unique_id = ap->print_id;
 		shost->max_id = 16;
-		shost->max_lun = 1;
+		shost->max_lun = clamp(atapi_max_lun, 1, ATAPI_MAX_LUN);
 		shost->max_channel = 1;
 		shost->max_cmd_len = 32;
 
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index b5423b6e97de..96d804d02b99 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -33,6 +33,7 @@ enum {
 #define ATA_PORT_TYPE_NAME	"ata_port"
 
 extern int atapi_passthru16;
+extern int atapi_max_lun;
 extern int libata_fua;
 extern int libata_noacpi;
 extern int libata_allow_tpm;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5c085ef4eda7..3e33ee30628d 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -131,6 +131,7 @@ enum {
 	ATA_SHORT_PAUSE		= 16,
 
 	ATAPI_MAX_DRAIN		= 16 << 10,
+	ATAPI_MAX_LUN		= 8,	/* SCSI-2 cap (LUN values 0..7) */
 
 	ATA_ALL_DEVICES		= (1 << ATA_MAX_DEVICES) - 1,
 
-- 
2.43.0

[PATCH v6 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array

[Phil Pemberton]

Multi-LUN ATAPI devices (PD/CD combos, CD changers) share a single
ata_device but expose multiple scsi_devices.  The previous single
dev->sdev pointer could only track one LUN, making all other LUNs
invisible to code that operates on sdevs: port detach, suspend/resume,
ACPI uevent, ZPODD, media change notification, and EH teardown.

Replace the scalar struct scsi_device *sdev with a fixed-size array
dev->sdev[ATAPI_MAX_LUN] indexed by LUN number, where ATAPI_MAX_LUN
is 8 (the SCSI-2 ceiling, LUN values 0..7).  Add a companion field
dev->nr_luns recording the number of valid entries -- defaults to 1
during ata_dev_init() and is bumped during multi-LUN probe -- so the
common single-LUN case iterates one slot, not eight.

Add an inline helper ata_dev_scsi_device(dev, lun) that returns
dev->sdev[lun] guarded by a WARN_ON_ONCE(lun >= dev->nr_luns) bounds
check.  Use it for the hardcoded LUN-0 references in libata-acpi
(uevent kobj), libata-zpodd (disk events, wake notify), and the
door-lock and OF-node paths in libata-scsi.

Key changes per call site:
  - ata_scsi_dev_config:  assign sdev to dev->sdev[sdev->lun]
  - ata_scsi_sdev_destroy: clear dev->sdev[sdev->lun]; only trigger
    ATA-level detach when LUN 0 is destroyed, since removing a higher
    LUN should not tear down the underlying ATA device
  - ata_port_detach:  iterate dev->nr_luns slots (high->low)
  - ata_scsi_offline_dev:  iterate dev->nr_luns slots
  - ata_scsi_remove_dev:  snapshot and remove all LUN slots, then
    scsi_remove_device each one outside the lock
  - ata_scsi_media_change_notify:  send event to all populated LUNs
  - ata_scsi_dev_rescan:  resume and rescan each populated LUN
  - ACPI, ZPODD, ofnode, door-lock:  use ata_dev_scsi_device(dev, 0)

For single-LUN devices (the vast majority) only dev->sdev[0] is ever
populated and dev->nr_luns stays at 1, so existing call paths see no
change in behaviour.

Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-acpi.c  |   6 +-
 drivers/ata/libata-core.c  |  11 ++-
 drivers/ata/libata-scsi.c  | 151 +++++++++++++++++++------------------
 drivers/ata/libata-zpodd.c |   6 +-
 include/linux/libata.h     |  11 ++-
 5 files changed, 103 insertions(+), 82 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 4433f626246b..8af35d0b1053 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -153,8 +153,10 @@ static void ata_acpi_uevent(struct ata_port *ap, struct ata_device *dev,
 	char *envp[] = { event_string, NULL };
 
 	if (dev) {
-		if (dev->sdev)
-			kobj = &dev->sdev->sdev_gendev.kobj;
+		struct scsi_device *sdev = ata_dev_scsi_device(dev, 0);
+
+		if (sdev)
+			kobj = &sdev->sdev_gendev.kobj;
 	} else
 		kobj = &ap->dev->kobj;
 
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 4408b1fb48c7..1cb159d9dbc7 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5564,6 +5564,7 @@ void ata_dev_init(struct ata_device *dev)
 	dev->pio_mask = UINT_MAX;
 	dev->mwdma_mask = UINT_MAX;
 	dev->udma_mask = UINT_MAX;
+	dev->nr_luns = 1;
 }
 
 /**
@@ -6275,11 +6276,15 @@ static void ata_port_detach(struct ata_port *ap)
 	/* Remove scsi devices */
 	ata_for_each_link(link, ap, HOST_FIRST) {
 		ata_for_each_dev(dev, link, ALL) {
-			if (dev->sdev) {
+			int lun;
+
+			for (lun = dev->nr_luns - 1; lun >= 0; lun--) {
+				if (!dev->sdev[lun])
+					continue;
 				spin_unlock_irqrestore(ap->lock, flags);
-				scsi_remove_device(dev->sdev);
+				scsi_remove_device(dev->sdev[lun]);
 				spin_lock_irqsave(ap->lock, flags);
-				dev->sdev = NULL;
+				dev->sdev[lun] = NULL;
 			}
 		}
 	}
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 32c6a0e497cf..7c3d31dc49a1 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1131,7 +1131,7 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct queue_limits *lim,
 	if (dev->flags & ATA_DFLAG_TRUSTED)
 		sdev->security_supported = 1;
 
-	dev->sdev = sdev;
+	dev->sdev[sdev->lun] = sdev;
 	return 0;
 }
 
@@ -1202,10 +1202,10 @@ EXPORT_SYMBOL_GPL(ata_scsi_sdev_configure);
  *
  *	@sdev is about to be destroyed for hot/warm unplugging.  If
  *	this unplugging was initiated by libata as indicated by NULL
- *	dev->sdev, this function doesn't have to do anything.
+ *	dev->sdev[], this function doesn't have to do anything.
  *	Otherwise, SCSI layer initiated warm-unplug is in progress.
- *	Clear dev->sdev, schedule the device for ATA detach and invoke
- *	EH.
+ *	Clear the per-LUN slot; when the last LUN (LUN 0) is destroyed,
+ *	schedule ATA-level detach via EH.
  *
  *	LOCKING:
  *	Defined by SCSI layer.  We don't really care.
@@ -1220,11 +1220,12 @@ void ata_scsi_sdev_destroy(struct scsi_device *sdev)
 
 	spin_lock_irqsave(ap->lock, flags);
 	dev = __ata_scsi_find_dev(ap, sdev);
-	if (dev && dev->sdev) {
-		/* SCSI device already in CANCEL state, no need to offline it */
-		dev->sdev = NULL;
-		dev->flags |= ATA_DFLAG_DETACH;
-		ata_port_schedule_eh(ap);
+	if (dev && dev->sdev[sdev->lun] == sdev) {
+		dev->sdev[sdev->lun] = NULL;
+		if (sdev->lun == 0) {
+			dev->flags |= ATA_DFLAG_DETACH;
+			ata_port_schedule_eh(ap);
+		}
 	}
 	spin_unlock_irqrestore(ap->lock, flags);
 
@@ -2911,10 +2912,15 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
 		 * avoid this infinite loop.
 		 *
 		 * This may happen before SCSI scan is complete.  Make
-		 * sure qc->dev->sdev isn't NULL before dereferencing.
+		 * sure the LUN-0 sdev isn't NULL before dereferencing.
 		 */
-		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL && qc->dev->sdev)
-			qc->dev->sdev->locked = 0;
+		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL) {
+			struct scsi_device *sdev =
+				ata_dev_scsi_device(qc->dev, 0);
+
+			if (sdev)
+				sdev->locked = 0;
+		}
 
 		ata_scsi_qc_done(qc, true, SAM_STAT_CHECK_CONDITION);
 		return;
@@ -4658,7 +4664,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
 #ifdef CONFIG_OF
 static void ata_scsi_assign_ofnode(struct ata_device *dev, struct ata_port *ap)
 {
-	struct scsi_device *sdev = dev->sdev;
+	struct scsi_device *sdev = ata_dev_scsi_device(dev, 0);
 	struct device *d = ap->host->dev;
 	struct device_node *np = d->of_node;
 	struct device_node *child;
@@ -4696,7 +4702,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			struct scsi_device *sdev;
 			int channel = 0, id = 0;
 
-			if (dev->sdev)
+			if (dev->sdev[0])
 				continue;
 
 			if (ata_is_host_link(link))
@@ -4707,11 +4713,11 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
 						 NULL);
 			if (!IS_ERR(sdev)) {
-				dev->sdev = sdev;
+				dev->sdev[0] = sdev;
 				ata_scsi_assign_ofnode(dev, ap);
 				scsi_device_put(sdev);
 			} else {
-				dev->sdev = NULL;
+				dev->sdev[0] = NULL;
 			}
 		}
 	}
@@ -4722,7 +4728,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 	 */
 	ata_for_each_link(link, ap, EDGE) {
 		ata_for_each_dev(dev, link, ENABLED) {
-			if (!dev->sdev)
+			if (!dev->sdev[0])
 				goto exit_loop;
 		}
 	}
@@ -4763,7 +4769,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
  *
  *	This function is called from ata_eh_detach_dev() and is responsible for
  *	taking the SCSI device attached to @dev offline.  This function is
- *	called with host lock which protects dev->sdev against clearing.
+ *	called with host lock which protects dev->sdev[] against clearing.
  *
  *	LOCKING:
  *	spin_lock_irqsave(host lock)
@@ -4773,11 +4779,16 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
  */
 bool ata_scsi_offline_dev(struct ata_device *dev)
 {
-	if (dev->sdev) {
-		scsi_device_set_state(dev->sdev, SDEV_OFFLINE);
-		return true;
+	bool found = false;
+	int lun;
+
+	for (lun = dev->nr_luns - 1; lun >= 0; lun--) {
+		if (dev->sdev[lun]) {
+			scsi_device_set_state(dev->sdev[lun], SDEV_OFFLINE);
+			found = true;
+		}
 	}
-	return false;
+	return found;
 }
 
 /**
@@ -4793,49 +4804,38 @@ bool ata_scsi_offline_dev(struct ata_device *dev)
 static void ata_scsi_remove_dev(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
-	struct scsi_device *sdev;
+	struct scsi_device *sdevs[ATAPI_MAX_LUN] = {};
 	unsigned long flags;
+	int lun;
 
-	/* Alas, we need to grab scan_mutex to ensure SCSI device
-	 * state doesn't change underneath us and thus
-	 * scsi_device_get() always succeeds.  The mutex locking can
-	 * be removed if there is __scsi_device_get() interface which
-	 * increments reference counts regardless of device state.
-	 */
 	mutex_lock(&ap->scsi_host->scan_mutex);
 	spin_lock_irqsave(ap->lock, flags);
 
-	/* clearing dev->sdev is protected by host lock */
-	sdev = dev->sdev;
-	dev->sdev = NULL;
+	for (lun = dev->nr_luns - 1; lun >= 0; lun--) {
+		struct scsi_device *sdev = dev->sdev[lun];
+
+		dev->sdev[lun] = NULL;
+		if (!sdev)
+			continue;
 
-	if (sdev) {
-		/* If user initiated unplug races with us, sdev can go
-		 * away underneath us after the host lock and
-		 * scan_mutex are released.  Hold onto it.
-		 */
 		if (scsi_device_get(sdev) == 0) {
-			/* The following ensures the attached sdev is
-			 * offline on return from ata_scsi_offline_dev()
-			 * regardless it wins or loses the race
-			 * against this function.
-			 */
 			scsi_device_set_state(sdev, SDEV_OFFLINE);
+			sdevs[lun] = sdev;
 		} else {
 			WARN_ON(1);
-			sdev = NULL;
 		}
 	}
 
 	spin_unlock_irqrestore(ap->lock, flags);
 	mutex_unlock(&ap->scsi_host->scan_mutex);
 
-	if (sdev) {
+	for (lun = dev->nr_luns - 1; lun >= 0; lun--) {
+		if (!sdevs[lun])
+			continue;
 		ata_dev_info(dev, "detaching (SCSI %s)\n",
-			     dev_name(&sdev->sdev_gendev));
-
-		scsi_remove_device(sdev);
-		scsi_device_put(sdev);
+			     dev_name(&sdevs[lun]->sdev_gendev));
+		scsi_remove_device(sdevs[lun]);
+		scsi_device_put(sdevs[lun]);
 	}
 }
 
@@ -4872,9 +4872,12 @@ static void ata_scsi_handle_link_detach(struct ata_link *link)
  */
 void ata_scsi_media_change_notify(struct ata_device *dev)
 {
-	if (dev->sdev)
-		sdev_evt_send_simple(dev->sdev, SDEV_EVT_MEDIA_CHANGE,
-				     GFP_ATOMIC);
+	int lun;
+
+	for (lun = 0; lun < dev->nr_luns; lun++)
+		if (dev->sdev[lun])
+			sdev_evt_send_simple(dev->sdev[lun],
+					     SDEV_EVT_MEDIA_CHANGE, GFP_ATOMIC);
 }
 
 /**
@@ -5007,37 +5010,39 @@ void ata_scsi_dev_rescan(struct work_struct *work)
 
 	ata_for_each_link(link, ap, EDGE) {
 		ata_for_each_dev(dev, link, ENABLED) {
-			struct scsi_device *sdev = dev->sdev;
+			int lun;
 
-			/*
-			 * If the port was suspended before this was scheduled,
-			 * bail out.
-			 */
 			if (ap->pflags & ATA_PFLAG_SUSPENDED)
 				goto unlock_ap;
 
-			if (!sdev)
-				continue;
-			if (scsi_device_get(sdev))
-				continue;
-
 			do_resume = dev->flags & ATA_DFLAG_RESUMING;
 
-			spin_unlock_irqrestore(ap->lock, flags);
-			if (do_resume) {
-				ret = scsi_resume_device(sdev);
-				if (ret == -EWOULDBLOCK) {
-					scsi_device_put(sdev);
-					goto unlock_scan;
+			for (lun = 0; lun < dev->nr_luns; lun++) {
+				struct scsi_device *sdev = dev->sdev[lun];
+
+				if (!sdev)
+					continue;
+				if (scsi_device_get(sdev))
+					continue;
+
+				spin_unlock_irqrestore(ap->lock, flags);
+				if (do_resume) {
+					ret = scsi_resume_device(sdev);
+					if (ret == -EWOULDBLOCK) {
+						scsi_device_put(sdev);
+						goto unlock_scan;
+					}
 				}
-				dev->flags &= ~ATA_DFLAG_RESUMING;
+				ret = scsi_rescan_device(sdev);
+				scsi_device_put(sdev);
+				spin_lock_irqsave(ap->lock, flags);
+
+				if (ret)
+					goto unlock_ap;
 			}
-			ret = scsi_rescan_device(sdev);
-			scsi_device_put(sdev);
-			spin_lock_irqsave(ap->lock, flags);
 
-			if (ret)
-				goto unlock_ap;
+			if (do_resume)
+				dev->flags &= ~ATA_DFLAG_RESUMING;
 		}
 	}
 
diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
index 414e7c63bd85..dca774d8ec05 100644
--- a/drivers/ata/libata-zpodd.c
+++ b/drivers/ata/libata-zpodd.c
@@ -185,7 +185,7 @@ void zpodd_enable_run_wake(struct ata_device *dev)
 {
 	struct zpodd *zpodd = dev->zpodd;
 
-	sdev_disable_disk_events(dev->sdev);
+	sdev_disable_disk_events(ata_dev_scsi_device(dev, 0));
 
 	zpodd->powered_off = true;
 	acpi_pm_set_device_wakeup(&dev->tdev, true);
@@ -233,14 +233,14 @@ void zpodd_post_poweron(struct ata_device *dev)
 	zpodd->zp_sampled = false;
 	zpodd->zp_ready = false;
 
-	sdev_enable_disk_events(dev->sdev);
+	sdev_enable_disk_events(ata_dev_scsi_device(dev, 0));
 }
 
 static void zpodd_wake_dev(acpi_handle handle, u32 event, void *context)
 {
 	struct ata_device *ata_dev = context;
 	struct zpodd *zpodd = ata_dev->zpodd;
-	struct device *dev = &ata_dev->sdev->sdev_gendev;
+	struct device *dev = &ata_dev_scsi_device(ata_dev, 0)->sdev_gendev;
 
 	if (event == ACPI_NOTIFY_DEVICE_WAKE && pm_runtime_suspended(dev)) {
 		zpodd->from_notify = true;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3e33ee30628d..5db8a2e3f051 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -722,7 +722,8 @@ struct ata_device {
 	unsigned int		devno;		/* 0 or 1 */
 	u64			quirks;		/* List of broken features */
 	unsigned long		flags;		/* ATA_DFLAG_xxx */
-	struct scsi_device	*sdev;		/* attached SCSI device */
+	struct scsi_device	*sdev[ATAPI_MAX_LUN];	/* per-LUN SCSI devices */
+	unsigned int		nr_luns;	/* valid entries in sdev[] */
 	void			*private_data;
 #ifdef CONFIG_ATA_ACPI
 	union acpi_object	*gtf_cache;
@@ -1715,6 +1716,14 @@ static inline unsigned int ata_dev_absent(const struct ata_device *dev)
 	return ata_class_absent(dev->class);
 }
 
+static inline struct scsi_device *
+ata_dev_scsi_device(struct ata_device *dev, unsigned int lun)
+{
+	if (WARN_ON_ONCE(lun >= dev->nr_luns))
+		return NULL;
+	return dev->sdev[lun];
+}
+
 /*
  * link helpers
  */
-- 
2.43.0

[PATCH v6 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI

[Phil Pemberton]

Two changes are required to route commands to ATAPI LUNs other than 0:

1. __ata_scsi_find_dev():  The existing code rejects any scsi_device
   with a non-zero LUN, returning NULL and dropping the command on
   the floor.  Hoist a non-zero LUN early-exit ahead of the original
   channel/id checks: when scsidev->lun is non-zero, allow it through
   only if the underlying ata_device is ATAPI class.  The original
   LUN-0 path is left structurally unchanged.

2. atapi_xlat():  Older ATAPI devices (SCSI-2 era) expect the LUN in
   CDB byte 1 bits 7:5 rather than relying on transport-level LUN
   addressing.  Encode scmd->device->lun into those bits, preserving
   the existing command-specific bits in 4:0.  This is required by
   both the Panasonic PD/CD combos and Nakamichi CD changers.

   The SCSI layer caps the LUN at shost->max_lun, so a value beyond
   the device's nr_luns should never reach this point; guard with
   WARN_ON_ONCE() and return AC_ERR_INVALID if it does, since the
   3-bit CDB field cannot represent it.

Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-scsi.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 7c3d31dc49a1..2d714efc855f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2953,6 +2953,15 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
 	memset(qc->cdb, 0, dev->cdb_len);
 	memcpy(qc->cdb, scmd->cmnd, scmd->cmd_len);
 
+	/*
+	 * SCSI-2 CDB LUN encoding: bits 7:5 of byte 1 (3-bit field).
+	 * The SCSI layer caps the LUN at shost->max_lun (<= ATAPI_MAX_LUN),
+	 * so this should never trip; warn and reject if it does.
+	 */
+	if (WARN_ON_ONCE(scmd->device->lun >= dev->nr_luns))
+		return AC_ERR_INVALID;
+	qc->cdb[1] = (qc->cdb[1] & 0x1f) | ((u8)scmd->device->lun << 5);
+
 	qc->complete_fn = atapi_qc_complete;
 
 	qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
@@ -3063,6 +3072,29 @@ static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
 {
 	int devno;
 
+	/*
+	 * Non-zero LUN is only legal for ATAPI devices, since they can
+	 * legitimately expose more than one LUN (PD/CD combos, CD changers).
+	 * Handle that case up front so the LUN-0 path below stays unchanged.
+	 */
+	if (unlikely(scsidev->lun)) {
+		struct ata_device *dev;
+
+		if (!sata_pmp_attached(ap)) {
+			if (unlikely(scsidev->channel))
+				return NULL;
+			devno = scsidev->id;
+		} else {
+			if (unlikely(scsidev->id))
+				return NULL;
+			devno = scsidev->channel;
+		}
+		dev = ata_find_dev(ap, devno);
+		if (!dev || dev->class != ATA_DEV_ATAPI)
+			return NULL;
+		return dev;
+	}
+
 	/* skip commands not addressed to targets we simulate */
 	if (!sata_pmp_attached(ap)) {
 		if (unlikely(scsidev->channel || scsidev->lun))
-- 
2.43.0

[PATCH v6 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag

[Phil Pemberton]

Some multi-LUN devices respond to INQUIRY on unpopulated LUNs with
PQ=0 / PDT=0x1f instead of the standard PQ=3.  The SCSI scan layer
normally adds such devices (PQ=0 means "connected"), producing
spurious "No Device" entries.

The scsi_target field pdt_1f_for_no_lun already exists to suppress
this, but was previously only set by the USB UFI driver.

Add BLIST_NO_LUN_1F so the flag can be set per-device from
scsi_devinfo, and wire it up in scsi_add_lun() to set
starget->pdt_1f_for_no_lun from the blacklist flags.  This runs
during LUN 0 processing, before the sequential LUN scan probes
higher LUNs.

Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/scsi/scsi_scan.c    | 2 ++
 include/scsi/scsi_devinfo.h | 6 +++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index ef22a4228b85..bfbbf9be05d2 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1069,6 +1069,8 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	transport_configure_device(&sdev->sdev_gendev);
 
 	sdev->sdev_bflags = *bflags;
+	if (sdev->sdev_bflags & BLIST_NO_LUN_1F)
+		sdev->sdev_target->pdt_1f_for_no_lun = 1;
 
 	if (scsi_device_is_pseudo_dev(sdev))
 		return SCSI_SCAN_LUN_PRESENT;
diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
index 1d79a3b536ce..6957b0705510 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -34,7 +34,8 @@
 #define BLIST_NOSTARTONADD	((__force blist_flags_t)(1ULL << 12))
 /* do not ask for VPD page size first on some broken targets */
 #define BLIST_NO_VPD_SIZE	((__force blist_flags_t)(1ULL << 13))
-#define __BLIST_UNUSED_14	((__force blist_flags_t)(1ULL << 14))
+/* PDT 0x1f with PQ 0 means no LUN present (e.g. some ATAPI multi-LUN) */
+#define BLIST_NO_LUN_1F		((__force blist_flags_t)(1ULL << 14))
 #define __BLIST_UNUSED_15	((__force blist_flags_t)(1ULL << 15))
 #define __BLIST_UNUSED_16	((__force blist_flags_t)(1ULL << 16))
 /* try REPORT_LUNS even for SCSI-2 devs (if HBA supports more than 8 LUNs) */
@@ -77,8 +78,7 @@
 #define __BLIST_HIGH_UNUSED (~(__BLIST_LAST_USED | \
 			       (__force blist_flags_t) \
 			       ((__force __u64)__BLIST_LAST_USED - 1ULL)))
-#define __BLIST_UNUSED_MASK (__BLIST_UNUSED_14 | \
-			     __BLIST_UNUSED_15 | \
+#define __BLIST_UNUSED_MASK (__BLIST_UNUSED_15 | \
 			     __BLIST_UNUSED_16 | \
 			     __BLIST_UNUSED_24 | \
 			     __BLIST_UNUSED_27 | \
-- 
2.43.0

[PATCH v6 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

[Phil Pemberton]

After LUN 0 is added for an ATAPI device, check its BLIST_FORCELUN
flag.  If set, bump dev->nr_luns to the host's max_lun so the LUN
routing in atapi_xlat() accepts the probe INQUIRYs, then call
scsi_scan_target() with SCAN_WILD_CARD to trigger the SCSI layer's
built-in sequential LUN scan for that target only.  This probes
LUNs 1..shost->max_lun, driven by the libata atapi_max_lun module
parameter.

Devices without BLIST_FORCELUN (the vast majority of ATAPI devices)
are left with only LUN 0 -- no sequential scan is triggered, so
single-LUN devices like the iHAS124 DVD writer are completely
unaffected.

Non-responding LUNs (PQ=0/PDT=0x1f) are silently skipped by
scsi_probe_and_add_lun() when BLIST_NO_LUN_1F is set on the device
via scsi_devinfo.

Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-scsi.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2d714efc855f..a6f5557014c7 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -26,6 +26,7 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_tcq.h>
 #include <scsi/scsi_transport.h>
+#include <scsi/scsi_devinfo.h>
 #include <linux/libata.h>
 #include <linux/hdreg.h>
 #include <linux/uaccess.h>
@@ -4745,12 +4746,30 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
 						 NULL);
 			if (!IS_ERR(sdev)) {
-				dev->sdev[0] = sdev;
-				ata_scsi_assign_ofnode(dev, ap);
+				/*
+				 * For multi-LUN ATAPI (BLIST_FORCELUN), bump
+				 * dev->nr_luns to the host max so the LUN
+				 * routing in atapi_xlat() accepts the probe
+				 * INQUIRYs to LUN > 0, then trigger the
+				 * sequential scan.  pdt_1f_for_no_lun, set
+				 * during LUN 0 configure, ensures
+				 * non-responding LUNs are silently skipped;
+				 * dev->sdev[] is populated by
+				 * ata_scsi_dev_config() during the scan.
+				 */
+				if (dev->class == ATA_DEV_ATAPI &&
+				    sdev->sdev_bflags & BLIST_FORCELUN) {
+					dev->nr_luns = ap->scsi_host->max_lun;
+					scsi_scan_target(
+						&ap->scsi_host->shost_gendev,
+						channel, id, SCAN_WILD_CARD,
+						SCSI_SCAN_RESCAN);
+				}
 				scsi_device_put(sdev);
-			} else {
-				dev->sdev[0] = NULL;
 			}
+
+			if (dev->sdev[0])
+				ata_scsi_assign_ofnode(dev, ap);
 		}
 	}
 
-- 
2.43.0

[PATCH v6 6/6] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

[Phil Pemberton]

The COMPAQ PD-1 (OEM Panasonic/Matsushita LF-1195C) is a PD/CD combo
drive that exposes two ATAPI LUNs: LUN 0 is a CD-ROM (TYPE_ROM),
LUN 1 is a 650 MB PD (TYPE_DISK).

Add it to the SCSI device list with:
  - BLIST_FORCELUN: tells the SCSI layer to scan past LUN 0
  - BLIST_SINGLELUN: serialises commands across the two LUNs, since
    the drive has a single transport and cannot handle concurrent
    operations on both
  - BLIST_NO_LUN_1F: the drive returns PQ=0/PDT=0x1f for unpopulated
    LUNs instead of PQ=3; this flag tells scsi_probe_and_add_lun()
    to silently skip them

The INQUIRY strings as reported by the device are:
  Vendor:  "COMPAQ  " (T10 format, space-padded)
  Product: "PD-1"

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/scsi/scsi_devinfo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 68a992494b12..bfc2cbd43897 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -150,6 +150,8 @@ static struct {
 	{"COMPAQ", "MSA1000", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
 	{"COMPAQ", "MSA1000 VOLUME", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
 	{"COMPAQ", "HSV110", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
+	{"COMPAQ", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN |
+				 BLIST_NO_LUN_1F},
 	{"DDN", "SAN DataDirector", "*", BLIST_SPARSELUN},
 	{"DEC", "HSG80", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
 	{"DELL", "PV660F", NULL, BLIST_SPARSELUN},
-- 
2.43.0

Re: [PATCH v6 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag

[sashiko-bot]

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

New issues:
- [Medium] The BLIST_NO_LUN_1F flag fails to suppress the initial unpopulated LUN (usually LUN 0) from being added as a spurious 'No Device' entry.

Pre-existing issues:
- [High] Concurrent read-modify-write on `struct scsi_target` bitfields causes data races and lost updates.
--

commit 10038c8d1287bfd2d8ba061ac2b0a14d56b4076e
Author: Phil Pemberton <philpem@philpem.me.uk>
scsi: add BLIST_NO_LUN_1F blacklist flag

This commit introduces the BLIST_NO_LUN_1F blacklist flag to suppress spurious
"No Device" entries on certain multi-LUN devices. It modifies scsi_add_lun()
to set the pdt_1f_for_no_lun flag during LUN scan based on the new blacklist
flag.

> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index ef22a4228b855..bfbbf9be05d21 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -1069,6 +1069,8 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
>  	transport_configure_device(&sdev->sdev_gendev);
>  
>  	sdev->sdev_bflags = *bflags;
> +	if (sdev->sdev_bflags & BLIST_NO_LUN_1F)
> +		sdev->sdev_target->pdt_1f_for_no_lun = 1;

[Severity: Medium]
Will this actually suppress the spurious entry for the first scanned LUN
(typically LUN 0)?

Looking at scsi_probe_and_add_lun(), the suppression check evaluates
starget->pdt_1f_for_no_lun before scsi_add_lun() is called:

scsi_probe_and_add_lun() {
    ...
    if (((result[0] >> 5) == 1 || starget->pdt_1f_for_no_lun) &&
        ...
        /* skip LUN */
        return SCSI_SCAN_TARGET_PRESENT;
    ...
    scsi_add_lun(...);
}

If LUN 0 is unpopulated, does it bypass the suppression check since the flag
is only set here in scsi_add_lun() afterward?

[Severity: High]
This is a pre-existing issue, but does adding another non-atomic bitfield
modification here expand a race window?

The fields pdt_1f_for_no_lun, single_lun, and expecting_lun_change are
adjacent bitfields in struct scsi_target that share the same memory word.
Modifying a bitfield performs a non-atomic read-modify-write.

Since scsi_add_lun() modifies pdt_1f_for_no_lun and single_lun from process
context (e.g., during a rescan), and an I/O completion on an already-active
LUN encountering a Unit Attention might call scsi_report_lun_change() which
modifies expecting_lun_change from softirq context, can this concurrent
read-modify-write cause updates to be lost?

If expecting_lun_change is lost, does it result in failing commands with
spurious I/O errors instead of retrying them?

If single_lun is lost, could it allow concurrent I/O to devices that do not
support it, potentially causing device hangs or data corruption?

>  
>  	if (scsi_device_is_pseudo_dev(sdev))
>  		return SCSI_SCAN_LUN_PRESENT;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608213443.2296614-1-philpem@philpem.me.uk?part=4

Re: [PATCH v6 1/6] ata: libata-scsi: add atapi_max_lun module parameter

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Logic and documentation mismatch in `atapi_max_lun` leading to an off-by-one configuration error.
--

commit 62d1cf766cb69ddd660120719652195586b409e8
Author: Phil Pemberton <philpem@philpem.me.uk>

ata: libata-scsi: add atapi_max_lun module parameter

This commit introduces a libata module parameter atapi_max_lun to control
the upper bound of the per-host SCSI LUN scan for ATAPI devices. The
default is 1 to preserve existing behavior, but can be clamped up to
ATAPI_MAX_LUN (8) to support multi-LUN ATAPI devices.

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index e76d15411e2ae..4408b1fb48c77 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c

[ ... ]

> @@ -122,6 +122,11 @@ int atapi_passthru16 = 1;
>  module_param(atapi_passthru16, int, 0444);
>  MODULE_PARM_DESC(atapi_passthru16, "Enable ATA_16 passthru for ATAPI devices (0=off, 1=on [default])");
>  
> +int atapi_max_lun = 1;
> +module_param(atapi_max_lun, int, 0444);
> +MODULE_PARM_DESC(atapi_max_lun,
> +	"Maximum LUN to scan on ATAPI devices flagged BLIST_FORCELUN (1 [default] .. 7)");

[Severity: Medium]
Is this documentation correct about the upper limit being 7?

The SCSI core treats shost->max_lun as an exclusive upper bound, meaning it
represents the total count of LUNs to scan. If a user sets atapi_max_lun to 7
based on this description, only LUNs 0 through 6 will be scanned, missing
LUN 7.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index f44612e269a4d..32c6a0e497cf9 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c

[ ... ]

> @@ -4627,7 +4627,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
>  		shost->transportt = &ata_scsi_transportt;
>  		shost->unique_id = ap->print_id;
>  		shost->max_id = 16;
> -		shost->max_lun = 1;
> +		shost->max_lun = clamp(atapi_max_lun, 1, ATAPI_MAX_LUN);

[Severity: Medium]
Since ATAPI_MAX_LUN is 8, the clamp allows a value of 8. To correctly scan up
to LUN 7, should the module parameter description be updated to indicate the
maximum value is 8? Or does atapi_max_lun require a +1 adjustment when
assigning it to shost->max_lun in ata_scsi_add_hosts()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608213443.2296614-1-philpem@philpem.me.uk?part=1

Re: [PATCH v6 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array

[sashiko-bot]

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

New issues:
- [High] Hardcoding LUN 0 in `atapi_qc_complete()` after a failed `ALLOW_MEDIUM_REMOVAL` command re-introduces an infinite SCSI EH loop for higher LUNs.
- [High] The `ATA_DFLAG_RESUMING` flag is permanently leaked if `scsi_rescan_device()` fails, blocking future power management operations.
- [High] `ata_scsi_sdev_destroy()` triggers a full ATA device detach simply when LUN 0 is destroyed, abruptly severing all other active LUNs.

Pre-existing issues:
- [High] In `ata_port_detach()`, `dev->sdev[lun]` is not cleared before calling `scsi_remove_device()`, leaving a window for Use-After-Free by lockless readers.
--

commit 21e5b1c2633a42382bfa70b2a8ecf4892badd416
Author: Phil Pemberton <philpem@philpem.me.uk>
Subject: ata: libata-scsi: convert dev->sdev to per-LUN array

This commit replaces the single scsi_device pointer in ata_device with an
array to support multi-LUN ATAPI devices. It also adds a dev->nr_luns field
and an ata_dev_scsi_device() helper to manage the per-LUN access.

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
[ ... ]
> @@ -6275,11 +6276,15 @@ static void ata_port_detach(struct ata_port *ap)
>  	/* Remove scsi devices */
>  	ata_for_each_link(link, ap, HOST_FIRST) {
>  		ata_for_each_dev(dev, link, ALL) {
> -			if (dev->sdev) {
> +			int lun;
> +
> +			for (lun = dev->nr_luns - 1; lun >= 0; lun--) {
> +				if (!dev->sdev[lun])
> +					continue;
>  				spin_unlock_irqrestore(ap->lock, flags);
> -				scsi_remove_device(dev->sdev);
> +				scsi_remove_device(dev->sdev[lun]);
>  				spin_lock_irqsave(ap->lock, flags);
> -				dev->sdev = NULL;
> +				dev->sdev[lun] = NULL;
>  			}
>  		}
>  	}

[Severity: High]
This is a pre-existing issue, but does this leave a window for a
use-after-free? By dropping the lock and calling scsi_remove_device() while
the pointer is still accessible in the array slot, concurrent lockless
readers might access the scsi_device during or after its destruction.
Would it be safer to snapshot the pointer and clear the slot under the lock
before calling scsi_remove_device(), similar to how it is handled in
ata_scsi_remove_dev()?

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
[ ... ]
> @@ -1220,11 +1220,12 @@ void ata_scsi_sdev_destroy(struct scsi_device *sdev)
>  
>  	spin_lock_irqsave(ap->lock, flags);
>  	dev = __ata_scsi_find_dev(ap, sdev);
> -	if (dev && dev->sdev) {
> -		/* SCSI device already in CANCEL state, no need to offline it */
> -		dev->sdev = NULL;
> -		dev->flags |= ATA_DFLAG_DETACH;
> -		ata_port_schedule_eh(ap);
> +	if (dev && dev->sdev[sdev->lun] == sdev) {
> +		dev->sdev[sdev->lun] = NULL;
> +		if (sdev->lun == 0) {
> +			dev->flags |= ATA_DFLAG_DETACH;
> +			ata_port_schedule_eh(ap);
> +		}
>  	}
>  	spin_unlock_irqrestore(ap->lock, flags);

[Severity: High]
Could this abruptly detach the physical device while other LUNs are still
active? If a user manually removes LUN 0 via sysfs while LUN 1 is still
running, the sdev->lun == 0 condition evaluates to true, triggering a
full ATA device teardown. Should the detach sequence instead be deferred
until all active LUN slots in the device array have been cleared?

[ ... ]
> @@ -2911,10 +2912,15 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
>  		 * avoid this infinite loop.
>  		 *
>  		 * This may happen before SCSI scan is complete.  Make
> -		 * sure qc->dev->sdev isn't NULL before dereferencing.
> +		 * sure the LUN-0 sdev isn't NULL before dereferencing.
>  		 */
> -		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL && qc->dev->sdev)
> -			qc->dev->sdev->locked = 0;
> +		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL) {
> +			struct scsi_device *sdev =
> +				ata_dev_scsi_device(qc->dev, 0);
> +
> +			if (sdev)
> +				sdev->locked = 0;
> +		}

[Severity: High]
Does hardcoding LUN 0 here re-introduce an infinite SCSI EH loop for higher
LUNs? If an ALLOW_MEDIUM_REMOVAL command fails on a LUN > 0, this code
clears the locked flag on LUN 0 instead of the target device. This leaves
the higher LUN's locked flag set, which causes SCSI EH to retry the lock
indefinitely. Should this use the actual device that the command targeted?

[ ... ]
> @@ -5007,37 +5010,39 @@ void ata_scsi_dev_rescan(struct work_struct *work)
[ ... ]
> -			spin_unlock_irqrestore(ap->lock, flags);
> -			if (do_resume) {
> -				ret = scsi_resume_device(sdev);
> -				if (ret == -EWOULDBLOCK) {
> -					scsi_device_put(sdev);
> -					goto unlock_scan;
> +			for (lun = 0; lun < dev->nr_luns; lun++) {
> +				struct scsi_device *sdev = dev->sdev[lun];
> +
> +				if (!sdev)
> +					continue;
> +				if (scsi_device_get(sdev))
> +					continue;
> +
> +				spin_unlock_irqrestore(ap->lock, flags);
> +				if (do_resume) {
> +					ret = scsi_resume_device(sdev);
> +					if (ret == -EWOULDBLOCK) {
> +						scsi_device_put(sdev);
> +						goto unlock_scan;
> +					}
>  				}
> -				dev->flags &= ~ATA_DFLAG_RESUMING;
> +				ret = scsi_rescan_device(sdev);
> +				scsi_device_put(sdev);
> +				spin_lock_irqsave(ap->lock, flags);
> +
> +				if (ret)
> +					goto unlock_ap;
>  			}
> -			ret = scsi_rescan_device(sdev);
> -			scsi_device_put(sdev);
> -			spin_lock_irqsave(ap->lock, flags);
>  
> -			if (ret)
> -				goto unlock_ap;
> +			if (do_resume)
> +				dev->flags &= ~ATA_DFLAG_RESUMING;
>  		}
>  	}

[Severity: High]
Can the ATA_DFLAG_RESUMING flag be permanently leaked here?
If scsi_rescan_device() returns an error for any LUN, the code breaks out
early using goto unlock_ap. This skips the block at the bottom that clears
the ATA_DFLAG_RESUMING flag, leaving the device stuck in a resuming state
and potentially blocking future power management operations.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608213443.2296614-1-philpem@philpem.me.uk?part=2

Re: [PATCH v6 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Returning AC_ERR_INVALID without setting scmd->result causes the SCSI layer to treat the aborted command as a success.
- [High] Unconditional modification of CDB byte 1 clears parameter bits for valid MMC and SCSI commands, corrupting them.
- [Medium] The __ata_scsi_find_dev() function accepts non-zero LUNs for all ATAPI devices, leading to WARN_ON_ONCE triggers and phantom devices during manual SCSI scans.
--

commit 23cd1ea8143d086cbe9cf8a4825c18b4ede2d943
Author: Phil Pemberton <philpem@philpem.me.uk>

ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI

This patch updates libata-scsi to route commands to non-zero LUNs for ATAPI
devices. It modifies __ata_scsi_find_dev() to allow non-zero LUNs for ATAPI
devices and updates atapi_xlat() to encode the LUN into bits 7:5 of CDB byte 1.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 7c3d31dc49a17..2d714efc855f0 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2953,6 +2953,15 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
>  	memset(qc->cdb, 0, dev->cdb_len);
>  	memcpy(qc->cdb, scmd->cmnd, scmd->cmd_len);
>  
> +	/*
> +	 * SCSI-2 CDB LUN encoding: bits 7:5 of byte 1 (3-bit field).
> +	 * The SCSI layer caps the LUN at shost->max_lun (<= ATAPI_MAX_LUN),
> +	 * so this should never trip; warn and reject if it does.
> +	 */
> +	if (WARN_ON_ONCE(scmd->device->lun >= dev->nr_luns))
> +		return AC_ERR_INVALID;

[Severity: High]
Does returning AC_ERR_INVALID here without setting scmd->result cause the
SCSI layer to treat the aborted command as a success?

The caller, ata_scsi_translate(), drops the command and calls scsi_done(cmd)
when xlat_func returns an error, but it doesn't set cmd->result to an error
status:

drivers/ata/libata-scsi.c:ata_scsi_translate() {
    ...
done:
    ata_qc_free(qc);
    scsi_done(cmd);
    return 0;
}

Since scmd->result defaults to SAM_STAT_GOOD (0), the SCSI midlayer will
believe the command succeeded.

This can cause the INQUIRY command to succeed, resulting in phantom LUNs.
For write operations, it can cause silent data corruption as the data is
dropped but reported as successfully written.

> +	qc->cdb[1] = (qc->cdb[1] & 0x1f) | ((u8)scmd->device->lun << 5);

[Severity: High]
Does this unconditionally overwrite CDB byte 1 and corrupt parameter bits for
valid MMC and SCSI commands?

Some modern SCSI commands use bits 7:5 of byte 1 for actual parameters, such
as RDPROTECT in READ (16) and WRITE (16), or MULTIPLE_COUNT in ATA
PASS-THROUGH (16).

If this overwrites those bits, the parameters will be silently corrupted. Is
it possible to rely on sdev->lun_in_cdb which the SCSI midlayer uses to
handle LUN injection safely for legacy devices?

[ ... ]

> @@ -3063,6 +3072,29 @@ static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
>  {
>  	int devno;
>  
> +	/*
> +	 * Non-zero LUN is only legal for ATAPI devices, since they can
> +	 * legitimately expose more than one LUN (PD/CD combos, CD changers).
> +	 * Handle that case up front so the LUN-0 path below stays unchanged.
> +	 */
> +	if (unlikely(scsidev->lun)) {
> +		struct ata_device *dev;
> +
> +		if (!sata_pmp_attached(ap)) {
> +			if (unlikely(scsidev->channel))
> +				return NULL;
> +			devno = scsidev->id;
> +		} else {
> +			if (unlikely(scsidev->id))
> +				return NULL;
> +			devno = scsidev->channel;
> +		}
> +		dev = ata_find_dev(ap, devno);
> +		if (!dev || dev->class != ATA_DEV_ATAPI)
> +			return NULL;
> +		return dev;

[Severity: Medium]
Does permitting non-zero LUNs for all ATAPI devices here allow users to
trigger the WARN_ON_ONCE() in atapi_xlat()?

Since dev->nr_luns remains 1 for standard single-LUN ATAPI devices that do
not have the BLIST_FORCELUN quirk, a manual SCSI scan (e.g., via sysfs) on a
non-zero LUN will reach atapi_xlat() and hit the WARN_ON_ONCE().

> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608213443.2296614-1-philpem@philpem.me.uk?part=3

Re: [PATCH v6 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Moving `ata_scsi_assign_ofnode()` after `scsi_device_put()` introduces a TOCTOU race leading to a NULL pointer dereference or Use-After-Free.
- [High] Hot-unplugging a multi-LUN ATAPI device fails to detach and free LUNs > 0, resulting in memory leaks and zombie SCSI devices.
--

commit 6a6b71770a262841f2823d6eb0dbe90bb0dcb456
Author: Phil Pemberton <philpem@philpem.me.uk>
ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

After LUN 0 is added for an ATAPI device, this commit checks the BLIST_FORCELUN
flag. If set, it bumps the LUN count to the host's maximum and triggers a
sequential SCSI scan for that target. Non-responding LUNs are silently skipped.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 2d714efc855f0..a6f5557014c70 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
[ ... ]
> @@ -4745,12 +4746,30 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>  			sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
>  						 NULL);
>  			if (!IS_ERR(sdev)) {
> -				dev->sdev[0] = sdev;
> -				ata_scsi_assign_ofnode(dev, ap);
> +				/*
> +				 * For multi-LUN ATAPI (BLIST_FORCELUN), bump
> +				 * dev->nr_luns to the host max so the LUN
> +				 * routing in atapi_xlat() accepts the probe
> +				 * INQUIRYs to LUN > 0, then trigger the
> +				 * sequential scan.  pdt_1f_for_no_lun, set
> +				 * during LUN 0 configure, ensures
> +				 * non-responding LUNs are silently skipped;
> +				 * dev->sdev[] is populated by
> +				 * ata_scsi_dev_config() during the scan.
> +				 */
> +				if (dev->class == ATA_DEV_ATAPI &&
> +				    sdev->sdev_bflags & BLIST_FORCELUN) {
> +					dev->nr_luns = ap->scsi_host->max_lun;

[Severity: High]
Does bumping dev->nr_luns here cause a memory leak during a device
hot-unplug or fatal error reset?

During those events, the libata error handler synchronously calls
ata_eh_detach_dev() followed by ata_dev_init(), which unconditionally resets
dev->nr_luns = 1 without clearing the dev->sdev[] array.

When the asynchronous SCSI hotplug task later calls ata_scsi_remove_dev(),
it bounds its cleanup loop using the newly reset count:

for (lun = dev->nr_luns - 1; lun >= 0; lun--) { ... }

Since dev->nr_luns was reset to 1, any active SCSI devices populated in
dev->sdev[1..7] appear to be skipped. Could this permanently leak the
scsi_device structures and leave unresponsive zombie block devices exposed
to userspace?

> +					scsi_scan_target(
> +						&ap->scsi_host->shost_gendev,
> +						channel, id, SCAN_WILD_CARD,
> +						SCSI_SCAN_RESCAN);
> +				}
>  				scsi_device_put(sdev);
> -			} else {
> -				dev->sdev[0] = NULL;
>  			}
> +
> +			if (dev->sdev[0])
> +				ata_scsi_assign_ofnode(dev, ap);

[Severity: Critical]
Is there a race condition here between dropping the local reference and
checking dev->sdev[0]?

By calling scsi_device_put(sdev) before checking dev->sdev[0] and executing
ata_scsi_assign_ofnode(dev, ap), the object's lifetime relies solely on sysfs
since shost->scan_mutex is not held here.

If a concurrent device removal happens (such as userspace writing to the
delete attribute), the concurrent path executes:

sdev_store_delete()
  scsi_remove_device()
    ata_scsi_sdev_destroy()

This path sets dev->sdev[0] = NULL and drops the sysfs reference.

If this concurrent sysfs removal happens exactly between the unprotected
if (dev->sdev[0]) check and its use inside ata_scsi_assign_ofnode(), could
it result in a NULL pointer dereference, or a Use-After-Free if the object
is freed before accessing sdev->sdev_gendev.of_node?

>  		}
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608213443.2296614-1-philpem@philpem.me.uk?part=5

Re: [PATCH v6 2/6] ata: libata-scsi: convert dev->sdev to per-LUN array

[Hannes Reinecke]

On 6/8/26 23:34, Phil Pemberton wrote:
> Multi-LUN ATAPI devices (PD/CD combos, CD changers) share a single
> ata_device but expose multiple scsi_devices.  The previous single
> dev->sdev pointer could only track one LUN, making all other LUNs
> invisible to code that operates on sdevs: port detach, suspend/resume,
> ACPI uevent, ZPODD, media change notification, and EH teardown.
> 
> Replace the scalar struct scsi_device *sdev with a fixed-size array
> dev->sdev[ATAPI_MAX_LUN] indexed by LUN number, where ATAPI_MAX_LUN
> is 8 (the SCSI-2 ceiling, LUN values 0..7).  Add a companion field
> dev->nr_luns recording the number of valid entries -- defaults to 1
> during ata_dev_init() and is bumped during multi-LUN probe -- so the
> common single-LUN case iterates one slot, not eight.
> 
> Add an inline helper ata_dev_scsi_device(dev, lun) that returns
> dev->sdev[lun] guarded by a WARN_ON_ONCE(lun >= dev->nr_luns) bounds
> check.  Use it for the hardcoded LUN-0 references in libata-acpi
> (uevent kobj), libata-zpodd (disk events, wake notify), and the
> door-lock and OF-node paths in libata-scsi.
> 
> Key changes per call site:
>    - ata_scsi_dev_config:  assign sdev to dev->sdev[sdev->lun]
>    - ata_scsi_sdev_destroy: clear dev->sdev[sdev->lun]; only trigger
>      ATA-level detach when LUN 0 is destroyed, since removing a higher
>      LUN should not tear down the underlying ATA device
>    - ata_port_detach:  iterate dev->nr_luns slots (high->low)
>    - ata_scsi_offline_dev:  iterate dev->nr_luns slots
>    - ata_scsi_remove_dev:  snapshot and remove all LUN slots, then
>      scsi_remove_device each one outside the lock
>    - ata_scsi_media_change_notify:  send event to all populated LUNs
>    - ata_scsi_dev_rescan:  resume and rescan each populated LUN
>    - ACPI, ZPODD, ofnode, door-lock:  use ata_dev_scsi_device(dev, 0)
> 
> For single-LUN devices (the vast majority) only dev->sdev[0] is ever
> populated and dev->nr_luns stays at 1, so existing call paths see no
> change in behaviour.
> 
> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>   drivers/ata/libata-acpi.c  |   6 +-
>   drivers/ata/libata-core.c  |  11 ++-
>   drivers/ata/libata-scsi.c  | 151 +++++++++++++++++++------------------
>   drivers/ata/libata-zpodd.c |   6 +-
>   include/linux/libata.h     |  11 ++-
>   5 files changed, 103 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> index 4433f626246b..8af35d0b1053 100644
> --- a/drivers/ata/libata-acpi.c
> +++ b/drivers/ata/libata-acpi.c
> @@ -153,8 +153,10 @@ static void ata_acpi_uevent(struct ata_port *ap, struct ata_device *dev,
>   	char *envp[] = { event_string, NULL };
>   
>   	if (dev) {
> -		if (dev->sdev)
> -			kobj = &dev->sdev->sdev_gendev.kobj;
> +		struct scsi_device *sdev = ata_dev_scsi_device(dev, 0);
> +
> +		if (sdev)
> +			kobj = &sdev->sdev_gendev.kobj;
>   	} else
>   		kobj = &ap->dev->kobj;
>   
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 4408b1fb48c7..1cb159d9dbc7 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5564,6 +5564,7 @@ void ata_dev_init(struct ata_device *dev)
>   	dev->pio_mask = UINT_MAX;
>   	dev->mwdma_mask = UINT_MAX;
>   	dev->udma_mask = UINT_MAX;
> +	dev->nr_luns = 1;
>   }
>   
>   /**
> @@ -6275,11 +6276,15 @@ static void ata_port_detach(struct ata_port *ap)
>   	/* Remove scsi devices */
>   	ata_for_each_link(link, ap, HOST_FIRST) {
>   		ata_for_each_dev(dev, link, ALL) {
> -			if (dev->sdev) {
> +			int lun;
> +
> +			for (lun = dev->nr_luns - 1; lun >= 0; lun--) {
> +				if (!dev->sdev[lun])
> +					continue;
>   				spin_unlock_irqrestore(ap->lock, flags);
> -				scsi_remove_device(dev->sdev);
> +				scsi_remove_device(dev->sdev[lun]);
>   				spin_lock_irqsave(ap->lock, flags);
> -				dev->sdev = NULL;
> +				dev->sdev[lun] = NULL;

As pointed out by sashiko, this is racy.
Please move 'dev->sdev[lun] = NULL' before unlock, and hold
'sdev' in a temporary variable.

Maybe even make this a separate patch, then this patch can be kept
as just the interface change.

>   			}
>   		}
>   	}
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 32c6a0e497cf..7c3d31dc49a1 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1131,7 +1131,7 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct queue_limits *lim,
>   	if (dev->flags & ATA_DFLAG_TRUSTED)
>   		sdev->security_supported = 1;
>   
> -	dev->sdev = sdev;
> +	dev->sdev[sdev->lun] = sdev;
>   	return 0;
>   }
>   
> @@ -1202,10 +1202,10 @@ EXPORT_SYMBOL_GPL(ata_scsi_sdev_configure);
>    *
>    *	@sdev is about to be destroyed for hot/warm unplugging.  If
>    *	this unplugging was initiated by libata as indicated by NULL
> - *	dev->sdev, this function doesn't have to do anything.
> + *	dev->sdev[], this function doesn't have to do anything.
>    *	Otherwise, SCSI layer initiated warm-unplug is in progress.
> - *	Clear dev->sdev, schedule the device for ATA detach and invoke
> - *	EH.
> + *	Clear the per-LUN slot; when the last LUN (LUN 0) is destroyed,
> + *	schedule ATA-level detach via EH.
>    *
>    *	LOCKING:
>    *	Defined by SCSI layer.  We don't really care.
> @@ -1220,11 +1220,12 @@ void ata_scsi_sdev_destroy(struct scsi_device *sdev)
>   
>   	spin_lock_irqsave(ap->lock, flags);
>   	dev = __ata_scsi_find_dev(ap, sdev);
> -	if (dev && dev->sdev) {
> -		/* SCSI device already in CANCEL state, no need to offline it */
> -		dev->sdev = NULL;
> -		dev->flags |= ATA_DFLAG_DETACH;
> -		ata_port_schedule_eh(ap);
> +	if (dev && dev->sdev[sdev->lun] == sdev) {
> +		dev->sdev[sdev->lun] = NULL;
> +		if (sdev->lun == 0) {
> +			dev->flags |= ATA_DFLAG_DETACH;
> +			ata_port_schedule_eh(ap);
> +		}
>   	}
>   	spin_unlock_irqrestore(ap->lock, flags);
>   
> @@ -2911,10 +2912,15 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
>   		 * avoid this infinite loop.
>   		 *
>   		 * This may happen before SCSI scan is complete.  Make
> -		 * sure qc->dev->sdev isn't NULL before dereferencing.
> +		 * sure the LUN-0 sdev isn't NULL before dereferencing.
>   		 */
> -		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL && qc->dev->sdev)
> -			qc->dev->sdev->locked = 0;
> +		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL) {
> +			struct scsi_device *sdev =
> +				ata_dev_scsi_device(qc->dev, 0);
> +
> +			if (sdev)
> +				sdev->locked = 0;
> +		}
>   
>   		ata_scsi_qc_done(qc, true, SAM_STAT_CHECK_CONDITION);
>   		return;
> @@ -4658,7 +4664,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
>   #ifdef CONFIG_OF
>   static void ata_scsi_assign_ofnode(struct ata_device *dev, struct ata_port *ap)
>   {
> -	struct scsi_device *sdev = dev->sdev;
> +	struct scsi_device *sdev = ata_dev_scsi_device(dev, 0);
>   	struct device *d = ap->host->dev;
>   	struct device_node *np = d->of_node;
>   	struct device_node *child;
> @@ -4696,7 +4702,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>   			struct scsi_device *sdev;
>   			int channel = 0, id = 0;
>   
> -			if (dev->sdev)
> +			if (dev->sdev[0])
>   				continue;
>   
>   			if (ata_is_host_link(link))
> @@ -4707,11 +4713,11 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>   			sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
>   						 NULL);
>   			if (!IS_ERR(sdev)) {
> -				dev->sdev = sdev;
> +				dev->sdev[0] = sdev;
>   				ata_scsi_assign_ofnode(dev, ap);
>   				scsi_device_put(sdev);
>   			} else {
> -				dev->sdev = NULL;
> +				dev->sdev[0] = NULL;
>   			}
>   		}
>   	}
> @@ -4722,7 +4728,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>   	 */
>   	ata_for_each_link(link, ap, EDGE) {
>   		ata_for_each_dev(dev, link, ENABLED) {
> -			if (!dev->sdev)
> +			if (!dev->sdev[0])
>   				goto exit_loop;
>   		}
>   	}
> @@ -4763,7 +4769,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>    *
>    *	This function is called from ata_eh_detach_dev() and is responsible for
>    *	taking the SCSI device attached to @dev offline.  This function is
> - *	called with host lock which protects dev->sdev against clearing.
> + *	called with host lock which protects dev->sdev[] against clearing.
>    *
>    *	LOCKING:
>    *	spin_lock_irqsave(host lock)
> @@ -4773,11 +4779,16 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>    */
>   bool ata_scsi_offline_dev(struct ata_device *dev)
>   {
> -	if (dev->sdev) {
> -		scsi_device_set_state(dev->sdev, SDEV_OFFLINE);
> -		return true;
> +	bool found = false;
> +	int lun;
> +
> +	for (lun = dev->nr_luns - 1; lun >= 0; lun--) {
> +		if (dev->sdev[lun]) {
> +			scsi_device_set_state(dev->sdev[lun], SDEV_OFFLINE);
> +			found = true;
> +		}
>   	}
> -	return false;
> +	return found;
>   }
>   
>   /**
> @@ -4793,49 +4804,38 @@ bool ata_scsi_offline_dev(struct ata_device *dev)
>   static void ata_scsi_remove_dev(struct ata_device *dev)
>   {
>   	struct ata_port *ap = dev->link->ap;
> -	struct scsi_device *sdev;
> +	struct scsi_device *sdevs[ATAPI_MAX_LUN] = {};
>   	unsigned long flags;
> +	int lun;
>   
> -	/* Alas, we need to grab scan_mutex to ensure SCSI device
> -	 * state doesn't change underneath us and thus
> -	 * scsi_device_get() always succeeds.  The mutex locking can
> -	 * be removed if there is __scsi_device_get() interface which
> -	 * increments reference counts regardless of device state.
> -	 */
>   	mutex_lock(&ap->scsi_host->scan_mutex);
>   	spin_lock_irqsave(ap->lock, flags);
>   
> -	/* clearing dev->sdev is protected by host lock */
> -	sdev = dev->sdev;
> -	dev->sdev = NULL;
> +	for (lun = dev->nr_luns - 1; lun >= 0; lun--) {
> +		struct scsi_device *sdev = dev->sdev[lun];
> +
> +		dev->sdev[lun] = NULL;
> +		if (!sdev)
> +			continue;
>   
> -	if (sdev) {
> -		/* If user initiated unplug races with us, sdev can go
> -		 * away underneath us after the host lock and
> -		 * scan_mutex are released.  Hold onto it.
> -		 */
>   		if (scsi_device_get(sdev) == 0) {
> -			/* The following ensures the attached sdev is
> -			 * offline on return from ata_scsi_offline_dev()
> -			 * regardless it wins or loses the race
> -			 * against this function.
> -			 */
>   			scsi_device_set_state(sdev, SDEV_OFFLINE);
> +			sdevs[lun] = sdev;
>   		} else {
>   			WARN_ON(1);
> -			sdev = NULL;
>   		}
>   	}
>   
>   	spin_unlock_irqrestore(ap->lock, flags);
>   	mutex_unlock(&ap->scsi_host->scan_mutex);
>   
> -	if (sdev) {
> +	for (lun = dev->nr_luns - 1; lun >= 0; lun--) {
> +		if (!sdevs[lun])
> +			continue;
>   		ata_dev_info(dev, "detaching (SCSI %s)\n",
> -			     dev_name(&sdev->sdev_gendev));
> -
> -		scsi_remove_device(sdev);
> -		scsi_device_put(sdev);
> +			     dev_name(&sdevs[lun]->sdev_gendev));
> +		scsi_remove_device(sdevs[lun]);
> +		scsi_device_put(sdevs[lun]);
 >   	}>   }
>   
Wouldn't it be simpler to have another mutex under 'dev' to protect
'dev->sdev[]' ?
That would get us out of this mess, and we could do away with the
temporary adev array.

> @@ -4872,9 +4872,12 @@ static void ata_scsi_handle_link_detach(struct ata_link *link)
>    */
>   void ata_scsi_media_change_notify(struct ata_device *dev)
>   {
> -	if (dev->sdev)
> -		sdev_evt_send_simple(dev->sdev, SDEV_EVT_MEDIA_CHANGE,
> -				     GFP_ATOMIC);
> +	int lun;
> +
> +	for (lun = 0; lun < dev->nr_luns; lun++)
> +		if (dev->sdev[lun])
> +			sdev_evt_send_simple(dev->sdev[lun],
> +					     SDEV_EVT_MEDIA_CHANGE, GFP_ATOMIC);
>   }

I guess the iteration need to be protected somehow, either by
taking 'ap->lock' or with the dedicated mutex from the above
comments.

>   
>   /**
> @@ -5007,37 +5010,39 @@ void ata_scsi_dev_rescan(struct work_struct *work)
>   
>   	ata_for_each_link(link, ap, EDGE) {
>   		ata_for_each_dev(dev, link, ENABLED) {
> -			struct scsi_device *sdev = dev->sdev;
> +			int lun;
>   
> -			/*
> -			 * If the port was suspended before this was scheduled,
> -			 * bail out.
> -			 */
>   			if (ap->pflags & ATA_PFLAG_SUSPENDED)
>   				goto unlock_ap;
>   
> -			if (!sdev)
> -				continue;
> -			if (scsi_device_get(sdev))
> -				continue;
> -
>   			do_resume = dev->flags & ATA_DFLAG_RESUMING;
>   
> -			spin_unlock_irqrestore(ap->lock, flags);
> -			if (do_resume) {
> -				ret = scsi_resume_device(sdev);
> -				if (ret == -EWOULDBLOCK) {
> -					scsi_device_put(sdev);
> -					goto unlock_scan;
> +			for (lun = 0; lun < dev->nr_luns; lun++) {
> +				struct scsi_device *sdev = dev->sdev[lun];
> +
> +				if (!sdev)
> +					continue;
> +				if (scsi_device_get(sdev))
> +					continue;
> +
> +				spin_unlock_irqrestore(ap->lock, flags);
> +				if (do_resume) {
> +					ret = scsi_resume_device(sdev);
> +					if (ret == -EWOULDBLOCK) {
> +						scsi_device_put(sdev);
> +						goto unlock_scan;
> +					}
 >   				}> -				dev->flags &= ~ATA_DFLAG_RESUMING;
> +				ret = scsi_rescan_device(sdev);
> +				scsi_device_put(sdev);
> +				spin_lock_irqsave(ap->lock, flags);
> +
> +				if (ret)
> +					goto unlock_ap;
>   			}
> -			ret = scsi_rescan_device(sdev);
> -			scsi_device_put(sdev);
> -			spin_lock_irqsave(ap->lock, flags);
>   
> -			if (ret)
> -				goto unlock_ap;
> +			if (do_resume)
> +				dev->flags &= ~ATA_DFLAG_RESUMING;
>   		}
>   	}
>   
> diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
> index 414e7c63bd85..dca774d8ec05 100644
> --- a/drivers/ata/libata-zpodd.c
> +++ b/drivers/ata/libata-zpodd.c
> @@ -185,7 +185,7 @@ void zpodd_enable_run_wake(struct ata_device *dev)
>   {
>   	struct zpodd *zpodd = dev->zpodd;
>   
> -	sdev_disable_disk_events(dev->sdev);
> +	sdev_disable_disk_events(ata_dev_scsi_device(dev, 0));

I _think_  we should call this for every LUN.

>   
>   	zpodd->powered_off = true;
>   	acpi_pm_set_device_wakeup(&dev->tdev, true);
> @@ -233,14 +233,14 @@ void zpodd_post_poweron(struct ata_device *dev)
>   	zpodd->zp_sampled = false;
>   	zpodd->zp_ready = false;
>   
> -	sdev_enable_disk_events(dev->sdev);
> +	sdev_enable_disk_events(ata_dev_scsi_device(dev, 0));

Same here.

>   }
>   
>   static void zpodd_wake_dev(acpi_handle handle, u32 event, void *context)
>   {
>   	struct ata_device *ata_dev = context;
>   	struct zpodd *zpodd = ata_dev->zpodd;
> -	struct device *dev = &ata_dev->sdev->sdev_gendev;
> +	struct device *dev = &ata_dev_scsi_device(ata_dev, 0)->sdev_gendev;

And here.

>   
>   	if (event == ACPI_NOTIFY_DEVICE_WAKE && pm_runtime_suspended(dev)) {
>   		zpodd->from_notify = true;
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 3e33ee30628d..5db8a2e3f051 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -722,7 +722,8 @@ struct ata_device {
>   	unsigned int		devno;		/* 0 or 1 */
>   	u64			quirks;		/* List of broken features */
>   	unsigned long		flags;		/* ATA_DFLAG_xxx */
> -	struct scsi_device	*sdev;		/* attached SCSI device */
> +	struct scsi_device	*sdev[ATAPI_MAX_LUN];	/* per-LUN SCSI devices */
> +	unsigned int		nr_luns;	/* valid entries in sdev[] */
>   	void			*private_data;
>   #ifdef CONFIG_ATA_ACPI
>   	union acpi_object	*gtf_cache;
> @@ -1715,6 +1716,14 @@ static inline unsigned int ata_dev_absent(const struct ata_device *dev)
>   	return ata_class_absent(dev->class);
>   }
>   
> +static inline struct scsi_device *
> +ata_dev_scsi_device(struct ata_device *dev, unsigned int lun)
> +{
> +	if (WARN_ON_ONCE(lun >= dev->nr_luns))
> +		return NULL;
> +	return dev->sdev[lun];
> +}
> +
>   /*
>    * link helpers
>    */

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v6 3/6] ata: libata-scsi: route non-zero LUN commands for multi-LUN ATAPI

[Hannes Reinecke]

On 6/8/26 23:34, Phil Pemberton wrote:
> Two changes are required to route commands to ATAPI LUNs other than 0:
> 
> 1. __ata_scsi_find_dev():  The existing code rejects any scsi_device
>     with a non-zero LUN, returning NULL and dropping the command on
>     the floor.  Hoist a non-zero LUN early-exit ahead of the original
>     channel/id checks: when scsidev->lun is non-zero, allow it through
>     only if the underlying ata_device is ATAPI class.  The original
>     LUN-0 path is left structurally unchanged.
> 
> 2. atapi_xlat():  Older ATAPI devices (SCSI-2 era) expect the LUN in
>     CDB byte 1 bits 7:5 rather than relying on transport-level LUN
>     addressing.  Encode scmd->device->lun into those bits, preserving
>     the existing command-specific bits in 4:0.  This is required by
>     both the Panasonic PD/CD combos and Nakamichi CD changers.
> 
>     The SCSI layer caps the LUN at shost->max_lun, so a value beyond
>     the device's nr_luns should never reach this point; guard with
>     WARN_ON_ONCE() and return AC_ERR_INVALID if it does, since the
>     3-bit CDB field cannot represent it.
> 
> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>   drivers/ata/libata-scsi.c | 32 ++++++++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v6 4/6] scsi: add BLIST_NO_LUN_1F blacklist flag

[Hannes Reinecke]

On 6/8/26 23:34, Phil Pemberton wrote:
> Some multi-LUN devices respond to INQUIRY on unpopulated LUNs with
> PQ=0 / PDT=0x1f instead of the standard PQ=3.  The SCSI scan layer
> normally adds such devices (PQ=0 means "connected"), producing
> spurious "No Device" entries.
> 
> The scsi_target field pdt_1f_for_no_lun already exists to suppress
> this, but was previously only set by the USB UFI driver.
> 
> Add BLIST_NO_LUN_1F so the flag can be set per-device from
> scsi_devinfo, and wire it up in scsi_add_lun() to set
> starget->pdt_1f_for_no_lun from the blacklist flags.  This runs
> during LUN 0 processing, before the sequential LUN scan probes
> higher LUNs.
> 
> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>   drivers/scsi/scsi_scan.c    | 2 ++
>   include/scsi/scsi_devinfo.h | 6 +++---
>   2 files changed, 5 insertions(+), 3 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v6 5/6] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

[Hannes Reinecke]

On 6/8/26 23:34, Phil Pemberton wrote:
> After LUN 0 is added for an ATAPI device, check its BLIST_FORCELUN
> flag.  If set, bump dev->nr_luns to the host's max_lun so the LUN
> routing in atapi_xlat() accepts the probe INQUIRYs, then call
> scsi_scan_target() with SCAN_WILD_CARD to trigger the SCSI layer's
> built-in sequential LUN scan for that target only.  This probes
> LUNs 1..shost->max_lun, driven by the libata atapi_max_lun module
> parameter.
> 
> Devices without BLIST_FORCELUN (the vast majority of ATAPI devices)
> are left with only LUN 0 -- no sequential scan is triggered, so
> single-LUN devices like the iHAS124 DVD writer are completely
> unaffected.
> 
> Non-responding LUNs (PQ=0/PDT=0x1f) are silently skipped by
> scsi_probe_and_add_lun() when BLIST_NO_LUN_1F is set on the device
> via scsi_devinfo.
> 
> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>   drivers/ata/libata-scsi.c | 27 +++++++++++++++++++++++----
>   1 file changed, 23 insertions(+), 4 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich