[PATCH 0/3] ata: libata-scsi: fix ATA PASS-THROUGH(32) handling

[PATCH 0/3] ata: libata-scsi: fix ATA PASS-THROUGH(32) handling

[shao.mingyin]

ATA PASS-THROUGH(32) uses the variable length CDB opcode 7Fh with
service action 1FF0h.  Commit b1ffbf854e08 added support for it, but
three paths still use the offsets of the 12- and 16-byte CDBs, or do
not recognize the 32-byte form at all.

Patch 1 fixes the PIO multi-sector sanity check and the multi_count
computation, which read cdb[1] instead of cdb[1 + cdb_offset], and
reports the field pointer for the same byte.

Patch 2 makes an unsupported service action fail with INVALID FIELD IN
CDB.  It currently returns 1 without setting sense data, so the command
completes with SAM_STAT_GOOD even though no ATA command was issued.

Patch 3 teaches the completion path to recognize ATA PASS-THROUGH(32)
and to read CK_COND from byte 11.  Without it, CK_COND is ignored and
the ATA Status Return sense descriptor is not built.

The three patches are independent and can be applied in any order.  All
were tested with a 32-byte CDB sent through SG_IO to a QEMU IDE disk;
the per-patch test descriptions are included after the --- line.

Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c

[PATCH 1/3] ata: libata-scsi: fix the multi_count check for 32-byte passthru

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

ata_scsi_pass_thru() translates 12, 16 and 32-byte ATA pass-through CDBs.
For a 32-byte CDB (VARIABLE_LENGTH_CMD with the ATA_32 service action) the
ATA command descriptor starts at byte offset 9, so the function adds
cdb_offset when reading the protocol (cdb[1 + cdb_offset]) and T_LENGTH
(cdb[2 + cdb_offset]) fields.

The PIO multi-sector sanity check and the multi_count computation,
however, still read cdb[1] without adding cdb_offset:

	if ((cdb[1] & 0xe0) && !is_multi_taskfile(tf))
	...
	unsigned int multi_count = 1 << (cdb[1] >> 5);

For a 32-byte CDB, cdb[1] is the variable length CDB control byte and
its bits 7:5 do not carry MULTIPLE_COUNT.  As a result, the sanity check
that rejects a multi-sector count on a non-multi taskfile is silently
skipped, and multi_count is always computed as 1 << 0 = 1, so the
comparison against dev->multi_count either emits a spurious
"invalid multi_count 1 ignored" warning for devices configured with a
different multi-sector size or misses a real mismatch.

Read the count field from cdb[1 + cdb_offset], matching the rest of the
function, and report the field pointer relative to the same byte so that
a rejected multi-sector count points at the MULTIPLE_COUNT field instead
of byte 1.  For 12 and 16-byte CDBs cdb_offset is zero, so their
behavior is unchanged.

Fixes: b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.")
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with an ATA disk
(ata_piix / QEMU IDE) at /dev/sda.  A 32-byte ATA PASS-THROUGH(32)
CDB for a non-multi command (READ SECTORS) with the multi-sector
count field set (cdb[10] bits 7:5) is sent via SG_IO; a control CDB
with the count field clear is accepted on both kernels.

  unpatched                            patched
  command accepted,                    rejected with
  read completes (status GOOD)         CHECK CONDITION,
                                       ILLEGAL_REQUEST,
                                       INVALID FIELD IN CDB (0x5/0x24)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a61fb0331279..927e75b7498d 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3574,13 +3574,13 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	}
 
 	/* sanity check for pio multi commands */
-	if ((cdb[1] & 0xe0) && !is_multi_taskfile(tf)) {
+	if ((cdb[1 + cdb_offset] & 0xe0) && !is_multi_taskfile(tf)) {
-		fp = 1;
+		fp = 1 + cdb_offset;
 		goto invalid_fld;
 	}
 
 	if (is_multi_taskfile(tf)) {
-		unsigned int multi_count = 1 << (cdb[1] >> 5);
+		unsigned int multi_count = 1 << (cdb[1 + cdb_offset] >> 5);
 
 		if (!dev->multi_count) {
 			ata_scsi_set_sense(dev, scmd, ABORTED_COMMAND,

Re: [PATCH 1/3] ata: libata-scsi: fix the multi_count check for 32-byte passthru

[Damien Le Moal]

On 11/4/26 23:09, shao.mingyin@zte.com.cn wrote:
> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> ata_scsi_pass_thru() translates 12, 16 and 32-byte ATA pass-through CDBs.
> For a 32-byte CDB (VARIABLE_LENGTH_CMD with the ATA_32 service action) the
> ATA command descriptor starts at byte offset 9, so the function adds
> cdb_offset when reading the protocol (cdb[1 + cdb_offset]) and T_LENGTH
> (cdb[2 + cdb_offset]) fields.
> 
> The PIO multi-sector sanity check and the multi_count computation,
> however, still read cdb[1] without adding cdb_offset:
> 
> 	if ((cdb[1] & 0xe0) && !is_multi_taskfile(tf))
> 	...
> 	unsigned int multi_count = 1 << (cdb[1] >> 5);
> 
> For a 32-byte CDB, cdb[1] is the variable length CDB control byte and
> its bits 7:5 do not carry MULTIPLE_COUNT.  As a result, the sanity check
> that rejects a multi-sector count on a non-multi taskfile is silently
> skipped, and multi_count is always computed as 1 << 0 = 1, so the
> comparison against dev->multi_count either emits a spurious
> "invalid multi_count 1 ignored" warning for devices configured with a
> different multi-sector size or misses a real mismatch.
> 
> Read the count field from cdb[1 + cdb_offset], matching the rest of the
> function, and report the field pointer relative to the same byte so that
> a rejected multi-sector count points at the MULTIPLE_COUNT field instead
> of byte 1.  For 12 and 16-byte CDBs cdb_offset is zero, so their
> behavior is unchanged.
> 
> Fixes: b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.")
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

[PATCH 2/3] ata: libata-scsi: reject unsupported ATA-32 service actions

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

ata_scsi_var_len_cdb_xlat() handles a variable length CDB only when its
service action is ATA_32 (ATA PASS-THROUGH (32)).  For any other service
action it returns 1, which tells ata_scsi_translate() that the command
has been handled and that scmd->result and the sense buffer were set by
the translation function.

No sense data is set in that path, so ata_scsi_translate() frees the qc
and calls scsi_done() with scmd->result still zero.  The command thus
completes with SAM_STAT_GOOD even though no ATA command was issued and
no data was transferred.  Before ATA PASS-THROUGH (32) support was added,
such CDBs were not claimed by ata_get_xlat_func() and were rejected by
ata_scsi_simulate() with ILLEGAL REQUEST/INVALID COMMAND OPERATION CODE.

Fail unsupported service actions with ILLEGAL REQUEST/INVALID FIELD IN
CDB and point the field pointer at the SERVICE ACTION field.

Fixes: b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.")
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with an ATA disk
(ata_piix, /dev/sda); no special hardware is needed.

A 32-byte VARIABLE_LENGTH_CMD CDB with service action 0x1234 (not
ATA_32) and a 512-byte data-in buffer is sent with SG_IO.  A valid
ATA PASS-THROUGH(32) READ SECTORS CDB is the control and is accepted
on both kernels.

  unpatched                            patched
  status 0x00 (SAM_STAT_GOOD)          status 0x02 (CHECK CONDITION)
                                       sk 0x05, asc 0x24
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a61fb0331279..d29b54688b53 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -5027,7 +5027,13 @@ static unsigned int ata_scsi_var_len_cdb_xlat(struct ata_queued_cmd *qc)
 	if (sa == ATA_32)
 		return ata_scsi_pass_thru(qc);
 
-	/* unsupported service action */
+	/*
+	 * Unsupported service action.  Set the sense data explicitly, as
+	 * returning 1 without it makes ata_scsi_translate() complete the
+	 * command with the SCSI result still zero, that is, with
+	 * SAM_STAT_GOOD.
+	 */
+	ata_scsi_set_invalid_field(qc->dev, scmd, 8, 0xff);
 	return 1;
 }

Re: [PATCH 2/3] ata: libata-scsi: reject unsupported ATA-32 service actions

[Damien Le Moal]

On 11/4/26 23:11, shao.mingyin@zte.com.cn wrote:
> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> ata_scsi_var_len_cdb_xlat() handles a variable length CDB only when its
> service action is ATA_32 (ATA PASS-THROUGH (32)).  For any other service
> action it returns 1, which tells ata_scsi_translate() that the command
> has been handled and that scmd->result and the sense buffer were set by
> the translation function.
> 
> No sense data is set in that path, so ata_scsi_translate() frees the qc
> and calls scsi_done() with scmd->result still zero.  The command thus
> completes with SAM_STAT_GOOD even though no ATA command was issued and
> no data was transferred.  Before ATA PASS-THROUGH (32) support was added,
> such CDBs were not claimed by ata_get_xlat_func() and were rejected by
> ata_scsi_simulate() with ILLEGAL REQUEST/INVALID COMMAND OPERATION CODE.
> 
> Fail unsupported service actions with ILLEGAL REQUEST/INVALID FIELD IN
> CDB and point the field pointer at the SERVICE ACTION field.
> 
> Fixes: b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.")
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

[PATCH 3/3] ata: libata-scsi: generate SAT sense data for ATA PASS-THROUGH(32)

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

ata_scsi_qc_complete() detects ATA pass-through commands by the ATA_12
and ATA_16 opcodes only.  ATA PASS-THROUGH (32) uses the variable
length CDB opcode 7Fh with service action ATA_32, so for those commands
the function takes the ordinary completion path: the CK_COND bit is
never honored and the ATA Status Return sense descriptor is never
built.  A successful command with CK_COND set is reported as GOOD
instead of CHECK CONDITION, and a failed command gets the generic ATA
sense data without the ATA register values.

The byte that carries T_LENGTH and CK_COND is byte 2 for 12- and
16-byte CDBs, but byte 11 for a 32-byte CDB, whose byte 2 is part of the
variable length CDB header.  Detect ATA PASS-THROUGH (32) through its
service action and read CK_COND from the correct byte.

Fixes: b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with an ATA disk
(ata_piix, /dev/sda); no special hardware is needed.

A 32-byte ATA PASS-THROUGH(32) READ SECTORS CDB with CK_COND set in
cdb[11] and a 512-byte data-in buffer is sent with SG_IO.  The same
CDB with CK_COND clear is the control and completes GOOD on both.

  unpatched                            patched
  status 0x00, no sense data           status 0x02 (CHECK CONDITION)
                                       sk 0x01, asc/ascq 0x00/0x1d
                                       ATA descriptor 0x09 present
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a61fb0331279..97adeecc7736 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1888,9 +1888,12 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
 	struct ata_link *link = qc->dev->link;
 	struct scsi_cmnd *cmd = qc->scsicmd;
 	u8 *cdb = cmd->cmnd;
+	u16 cdb_offset = cdb[0] == VARIABLE_LENGTH_CMD ? 9 : 0;
 	bool have_sense = qc->flags & ATA_QCFLAG_SENSE_VALID;
-	bool is_ata_passthru = cdb[0] == ATA_16 || cdb[0] == ATA_12;
-	bool is_ck_cond_request = cdb[2] & 0x20;
+	bool is_ata_passthru = cdb[0] == ATA_16 || cdb[0] == ATA_12 ||
+		(cdb[0] == VARIABLE_LENGTH_CMD &&
+		 get_unaligned_be16(&cdb[8]) == ATA_32);
+	bool is_ck_cond_request = cdb[2 + cdb_offset] & 0x20;
 	bool is_error = qc->err_mask != 0;
 
 	/* For ATA pass thru (SAT) commands, generate a sense block if
@@ -3389,6 +3392,6 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
 	tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset]);
 	if (tf->protocol == ATA_PROT_UNKNOWN) {
-		fp = 1;
+		fp = 1 + cdb_offset;
 		goto invalid_fld;
 	}

Re: [PATCH 3/3] ata: libata-scsi: generate SAT sense data for ATA PASS-THROUGH(32)

[Damien Le Moal]

On 11/4/26 23:11, shao.mingyin@zte.com.cn wrote:
> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> ata_scsi_qc_complete() detects ATA pass-through commands by the ATA_12
> and ATA_16 opcodes only.  ATA PASS-THROUGH (32) uses the variable
> length CDB opcode 7Fh with service action ATA_32, so for those commands
> the function takes the ordinary completion path: the CK_COND bit is
> never honored and the ATA Status Return sense descriptor is never
> built.  A successful command with CK_COND set is reported as GOOD
> instead of CHECK CONDITION, and a failed command gets the generic ATA
> sense data without the ATA register values.
> 
> The byte that carries T_LENGTH and CK_COND is byte 2 for 12- and
> 16-byte CDBs, but byte 11 for a 32-byte CDB, whose byte 2 is part of the
> variable length CDB header.  Detect ATA PASS-THROUGH (32) through its
> service action and read CK_COND from the correct byte.
> 
> Fixes: b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 0/3] ata: libata-scsi: fix ATA PASS-THROUGH(32) handling

[Niklas Cassel]

On Wed, Nov 04, 2026 at 10:08:32PM +0800, shao.mingyin@zte.com.cn wrote:
> ATA PASS-THROUGH(32) uses the variable length CDB opcode 7Fh with
> service action 1FF0h.  Commit b1ffbf854e08 added support for it, but
> three paths still use the offsets of the 12- and 16-byte CDBs, or do
> not recognize the 32-byte form at all.
> 
> Patch 1 fixes the PIO multi-sector sanity check and the multi_count
> computation, which read cdb[1] instead of cdb[1 + cdb_offset], and
> reports the field pointer for the same byte.
> 
> Patch 2 makes an unsupported service action fail with INVALID FIELD IN
> CDB.  It currently returns 1 without setting sense data, so the command
> completes with SAM_STAT_GOOD even though no ATA command was issued.
> 
> Patch 3 teaches the completion path to recognize ATA PASS-THROUGH(32)
> and to read CK_COND from byte 11.  Without it, CK_COND is ignored and
> the ATA Status Return sense descriptor is not built.
> 
> The three patches are independent and can be applied in any order.  All
> were tested with a 32-byte CDB sent through SG_IO to a QEMU IDE disk;
> the per-patch test descriptions are included after the --- line.
> 
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> ---
> base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c

Hello Shao,


LLM found some review comments.

I looked at the SAT specs, and the comments appears to be valid:



commit d693212f2d5e4f09a56a4883db4172f7b5892673
Author: Shao Mingyin <shao.mingyin@zte.com.cn>

ata: libata-scsi: fix the multi_count check for 32-byte passthru

This patch makes the PIO multi-sector sanity check and the multi_count
computation in ata_scsi_pass_thru() read byte 1 + cdb_offset of the CDB
instead of byte 1, and reports the field pointer for the same byte.

> For a 32-byte CDB, cdb[1] is the variable length CDB control byte and
> its bits 7:5 do not carry MULTIPLE_COUNT.

[ ... ]

> Read the count field from cdb[1 + cdb_offset], matching the rest of the
> function, and report the field pointer relative to the same byte so that
> a rejected multi-sector count points at the MULTIPLE_COUNT field instead
> of byte 1.

Does the ATA PASS-THROUGH (32) CDB have a MULTIPLE_COUNT field at all?

The ATA PASS-THROUGH (32) CDB defined in SAT-4, SAT-5 and SAT-6 (table 210
in SAT-6 r02) has:

  10 | Reserved (7:5) | PROTOCOL (4:1) | EXTEND (0)

so bits 7:5 of byte 10 are reserved, not a count. The 32-byte CDB was added
in SAT-4, where bits 7:5 of byte 1 of the 12 and 16-byte CDBs, the former
MULTIPLE_COUNT field, are already obsolete.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index a61fb0331279..927e75b7498d 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3574,13 +3574,13 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
>  	}
>  
>  	/* sanity check for pio multi commands */
> -	if ((cdb[1] & 0xe0) && !is_multi_taskfile(tf)) {
> +	if ((cdb[1 + cdb_offset] & 0xe0) && !is_multi_taskfile(tf)) {
> -		fp = 1;
> +		fp = 1 + cdb_offset;

[Severity: Low]
Was this field pointer change part of the patch that was tested?

The index line of this patch records 927e75b7498d as the resulting
libata-scsi.c, but applying the posted diff to next-20261030 results in
20ba572c0e37. 927e75b7498d is what the diff gives without this change, so
it looks like the change was added after the patch was generated. That
would also explain the interleaved - and + lines above, which git does not
produce.

The test results below the --- line only list the sense key and the
additional sense code, not the field pointer.

>  		goto invalid_fld;
>  	}
>  
>  	if (is_multi_taskfile(tf)) {
> -		unsigned int multi_count = 1 << (cdb[1] >> 5);
> +		unsigned int multi_count = 1 << (cdb[1 + cdb_offset] >> 5);

[Severity: Low]
With a 32-byte CDB that leaves the reserved bits zero, doesn't this still
compute multi_count = 1 << 0 = 1 for READ MULTIPLE and WRITE MULTIPLE?

ata_scsi_pass_thru() {
    ...
	if (is_multi_taskfile(tf)) {
		unsigned int multi_count = 1 << (cdb[1 + cdb_offset] >> 5);
    ...
		if (multi_count != dev->multi_count)
			ata_dev_warn(dev, "invalid multi_count %u ignored\n",
				     multi_count);
	}
    ...
}

So the "invalid multi_count 1 ignored" warning that the commit message
describes is still emitted for devices with dev->multi_count != 1. Only an
initiator that sets the reserved bits gets a different multi_count.

The change that does take effect is that the CONTROL byte, whose bits 7:6
are vendor specific, is no longer interpreted as a multiple count.

Would it be more accurate to treat bits 7:5 of byte 10 as reserved, and to
skip the multi_count comparison for the 32-byte CDB, which cannot specify
a multiple count?


commit 5bd933578ed43952ff6544aadda2394a021acb83
Author: Shao Mingyin <shao.mingyin@zte.com.cn>

ata: libata-scsi: generate SAT sense data for ATA PASS-THROUGH(32)

This patch makes ata_scsi_qc_complete() recognize ATA PASS-THROUGH (32)
commands by their service action, and read CK_COND from byte 11 of the
32-byte CDB, so that CK_COND is honored and the ATA Status Return sense
descriptor is built for these commands.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index a61fb0331279..97adeecc7736 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c

[ ... ]

> @@ -3389,6 +3392,6 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
>  	tf->protocol = ata_scsi_map_proto(cdb[1 + cdb_offset]);
>  	if (tf->protocol == ATA_PROT_UNKNOWN) {
> -		fp = 1;
> +		fp = 1 + cdb_offset;
>  		goto invalid_fld;
>  	}

[Severity: Low]
This isn't a bug, but this hunk changes the field pointer that
ata_scsi_pass_thru() reports for an invalid PROTOCOL field, and the commit
message only describes ata_scsi_qc_complete().

Would this fit better in the first patch of the series, which changes the
field pointer for the same byte, or in a patch of its own?

[Severity: Low]
Was this hunk part of the patch that was tested?

As with the first patch, the index line records 97adeecc7736 as the
resulting libata-scsi.c, which is what this patch gives without this hunk.
Applying the posted diff to next-20261030 results in c23b110dae63, so it
looks like the hunk was added after the patch was generated.

[Severity: Low]
Should the remaining field pointers in ata_scsi_pass_thru() also take
cdb_offset into account? They still use the offsets of the 12 and 16-byte
CDBs:

ata_scsi_pass_thru() {
    ...
		if (tf->protocol != ATA_PROT_PIO || tf->nsect != 1) {
			fp = 1;
			goto invalid_fld;
		}
    ...
	if (tf->protocol == ATA_PROT_DMA && !ata_dma_enabled(dev)) {
		fp = 1;
		goto invalid_fld;
	}
    ...
	if (ata_is_ncq(tf->protocol) && !ata_ncq_enabled(dev)) {
		fp = 1;
		goto invalid_fld;
	}
    ...
	    tf->feature == SETFEATURES_XFER) {
		fp = (cdb[0] == ATA_16) ? 4 : 3;
    ...
	if (tf->command >= 0x5C && tf->command <= 0x5F && !libata_allow_tpm) {
		fp = (cdb[0] == ATA_16) ? 14 : 9;
    ...
}

In the 32-byte CDB, PROTOCOL is in byte 10, FEATURES (7:0) in byte 21 and
COMMAND in byte 25. The cover letter says "three paths still use the
offsets of the 12- and 16-byte CDBs", but these five still do after the
series.




Kind regards,
Niklas

Re: [PATCH 0/3] ata: libata-scsi: fix ATA PASS-THROUGH(32) handling

[shao.mingyin]

On Mon, Nov 09, 2026 at 10:32:42PM +0100, Niklas Cassel wrote:
> I looked at the SAT specs, and the comments appears to be valid:

They are, thanks for checking.  All three need action.

> Does the ATA PASS-THROUGH (32) CDB have a MULTIPLE_COUNT field at all?
> ...
> so bits 7:5 of byte 10 are reserved, not a count.

Right, and the commit message was wrong.  With those bits zero,
multi_count is always 1, so the warning it describes is still emitted.

v2 drops the MULTIPLE_COUNT handling for the 32-byte CDB instead of
moving the offset: the sanity check and the comparison are skipped for
VARIABLE_LENGTH_CMD.  Field pointer changes remain only for fields that
are actually validated.

> The index line of this patch records 927e75b7498d ... applying the
> posted diff results in 20ba572c0e37 ... That would also explain the
> interleaved - and + lines above, which git does not produce.

Confirmed, and sorry.  Those hunks were hand-added to the emails after
the patches were generated.  v2 comes from git format-patch on the
tested tree, and the test notes now include the field pointer.

> This isn't a bug, but this hunk changes the field pointer ... Would
> this fit better in the first patch ...
>
> Should the remaining field pointers ... also take cdb_offset into
> account? ... PROTOCOL is in byte 10, FEATURES (7:0) in byte 21 and
> COMMAND in byte 25.

Yes.  It moves into 1/4.  v2 is four patches: reworked 32-byte passthru
(no MULTIPLE_COUNT, PROTOCOL field pointer), the unchanged service
action patch, SAT sense data without the extra hunk, and a new patch for
the remaining field pointers (PROTOCOL/count, FEATURES, COMMAND) mapped
per CDB format.  The cover letter will say what is really left.

Kind regards,
Shao Mingyin