* [PATCH v2 0/5] Improve ata_scsi_report_supported_opcodes()
@ 2026-07-02 6:34 Damien Le Moal
2026-07-02 6:34 ` [PATCH v2 1/5] ata: libata: rename ata_dev_is_zac() Damien Le Moal
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: Damien Le Moal @ 2026-07-02 6:34 UTC (permalink / raw)
To: linux-ide, Niklas Cassel
The implementation of ata_scsi_report_supported_opcodes() mostly satisfies
the internal use when an ATA device is probed from the SCSI layer, but it
is very incomplete. The code is also not very flexible for extending
support for new commands.
This patch series address these issues, cleaning up the implementation and
making REPORT SUPPORTED OPERATION CODES fully functional. E.g., with these
changes, sg_opcodes can be used for listing all SCSI commands that are
supported by libata-scsi and a device.
Example:
# sg_opcodes /dev/sda
ATA WDC WUH722626AL WZ41
Peripheral device type: disk
Opcode Service CDB RWCDLP, Name
(hex) action(h) size CDLP
-----------------------------------------------
00 6 0,0 Test Unit Ready
01 6 0,0 Rezero Unit
03 6 0,0 Request Sense
08 6 0,0 Read(6)
0a 6 0,0 Write(6)
0b 6 0,0 Seek(6)
12 6 0,0 Inquiry
15 6 0,0 Mode select(6)
1a 6 0,0 Mode sense(6)
1b 6 0,0 Start stop unit
1d 6 0,0 Send diagnostic
25 10 0,0 Read capacity(10)
28 10 0,0 Read(10)
2a 10 0,0 Write(10)
2b 10 0,0 Seek(10)
2f 10 0,0 Verify(10)
35 10 0,0 Synchronize cache(10)
55 10 0,0 Mode select(10)
5a 10 0,0 Mode sense(10)
7f 1ff0 32 0,0 ATA pass-through(32)
85 16 0,0 ATA pass-through(16)
88 16 1,1 Read(16)
8a 16 1,2 Write(16)
8f 16 0,0 Verify(16)
91 16 0,0 Synchronize cache(16)
93 16 0,0 Write same(16)
9e 10 16 0,0 Read capacity(16)
a0 12 0,0 Report luns
a1 12 0,0 ATA pass-through(12)
a3 c 12 0,0 Report supported operation codes
Changes from v1:
- Patch 2: Added an sa_valid field to the array of supported command to
correctly handle sa == 0 being a valid service action (report zones) in
patch 3
- Dropped v1 patch 4
- Patch 5: correctly set the servact field based on the sa_valid field of
the array of supported commands.
Damien Le Moal (5):
ata: libata: rename ata_dev_is_zac()
ata: libata-scsi: refactor ata_scsi_report_supported_opcodes()
ata: libata-scsi: improve service action support in
ata_scsi_report_supported_opcodes()
ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED
OPERATION CODES
ata: libata-scsi: support the all command format for reporting
supported commands
drivers/ata/libata-core.c | 6 +-
drivers/ata/libata-scsi.c | 242 +++++++++++++++++++++++++++++---------
drivers/ata/libata.h | 2 +-
3 files changed, 189 insertions(+), 61 deletions(-)
--
2.54.0
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH v2 1/5] ata: libata: rename ata_dev_is_zac() 2026-07-02 6:34 [PATCH v2 0/5] Improve ata_scsi_report_supported_opcodes() Damien Le Moal @ 2026-07-02 6:34 ` Damien Le Moal 2026-07-02 6:38 ` Hannes Reinecke 2026-07-02 7:16 ` sashiko-bot 2026-07-02 6:34 ` [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() Damien Le Moal ` (3 subsequent siblings) 4 siblings, 2 replies; 23+ messages in thread From: Damien Le Moal @ 2026-07-02 6:34 UTC (permalink / raw) To: linux-ide, Niklas Cassel The helper function ata_dev_is_zac() checks if a device is a ZAC class device (host managed zoned disk) or if it is a host aware zoned disk, that is, a regular ATA disk that supports the zoned capabilities. So the name of this helper function is confusing as it hints at the first case only. Rename this helper function to ata_dev_is_zoned() to avoid confusions and better reflect the two cases tested. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/ata/libata-core.c | 6 +++--- drivers/ata/libata-scsi.c | 7 +++---- drivers/ata/libata.h | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 3b6243f0f91e..6daa99508b72 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2488,7 +2488,7 @@ static void ata_dev_config_sense_reporting(struct ata_device *dev) } } -static void ata_dev_config_zac(struct ata_device *dev) +static void ata_dev_config_zoned(struct ata_device *dev) { unsigned int err_mask; u8 *identify_buf = dev->sector_buf; @@ -2497,7 +2497,7 @@ static void ata_dev_config_zac(struct ata_device *dev) dev->zac_zones_optimal_nonseq = U32_MAX; dev->zac_zones_max_open = U32_MAX; - if (!ata_dev_is_zac(dev)) + if (!ata_dev_is_zoned(dev)) return; if (!ata_identify_page_supported(dev, ATA_LOG_ZONED_INFORMATION)) { @@ -3093,7 +3093,7 @@ int ata_dev_configure(struct ata_device *dev) ata_dev_config_fua(dev); ata_dev_config_devslp(dev); ata_dev_config_sense_reporting(dev); - ata_dev_config_zac(dev); + ata_dev_config_zoned(dev); ata_dev_config_trusted(dev); ata_dev_config_cpr(dev); ata_dev_config_cdl(dev); diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index d54ec1631e9a..8811bfc37369 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -2060,7 +2060,7 @@ static unsigned int ata_scsiop_inq_00(struct ata_device *dev, }; for (i = 0; i < sizeof(pages); i++) { - if (pages[i] == 0xb6 && !ata_dev_is_zac(dev)) + if (pages[i] == 0xb6 && !ata_dev_is_zoned(dev)) continue; rbuf[num_pages + 4] = pages[i]; num_pages++; @@ -2317,7 +2317,7 @@ static unsigned int ata_scsiop_inq_b2(struct ata_device *dev, static unsigned int ata_scsiop_inq_b6(struct ata_device *dev, struct scsi_cmnd *cmd, u8 *rbuf) { - if (!ata_dev_is_zac(dev)) { + if (!ata_dev_is_zoned(dev)) { ata_scsi_set_invalid_field(dev, cmd, 2, 0xff); return 0; } @@ -3656,8 +3656,7 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, break; case ZBC_IN: case ZBC_OUT: - if (ata_id_zoned_cap(dev->id) || - dev->class == ATA_DEV_ZAC) + if (ata_dev_is_zoned(dev)) supported = 3; break; case SECURITY_PROTOCOL_IN: diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index 0dd735c2e5b5..f813b1c32461 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -44,7 +44,7 @@ static inline bool ata_sstatus_online(u32 sstatus) return (sstatus & 0xf) == 0x3; } -static inline bool ata_dev_is_zac(struct ata_device *dev) +static inline bool ata_dev_is_zoned(struct ata_device *dev) { /* Host managed device or host aware device */ return dev->class == ATA_DEV_ZAC || -- 2.54.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/5] ata: libata: rename ata_dev_is_zac() 2026-07-02 6:34 ` [PATCH v2 1/5] ata: libata: rename ata_dev_is_zac() Damien Le Moal @ 2026-07-02 6:38 ` Hannes Reinecke 2026-07-02 7:16 ` sashiko-bot 1 sibling, 0 replies; 23+ messages in thread From: Hannes Reinecke @ 2026-07-02 6:38 UTC (permalink / raw) To: Damien Le Moal, linux-ide, Niklas Cassel On 7/2/26 8:34 AM, Damien Le Moal wrote: > The helper function ata_dev_is_zac() checks if a device is a ZAC class > device (host managed zoned disk) or if it is a host aware zoned disk, that > is, a regular ATA disk that supports the zoned capabilities. So the name > of this helper function is confusing as it hints at the first case only. > > Rename this helper function to ata_dev_is_zoned() to avoid confusions and > better reflect the two cases tested. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-core.c | 6 +++--- > drivers/ata/libata-scsi.c | 7 +++---- > drivers/ata/libata.h | 2 +- > 3 files changed, 7 insertions(+), 8 deletions(-) > Sigh ... This was the first function I ever coded when starting SMR support. You could have shown a bit more respect ;-) 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/5] ata: libata: rename ata_dev_is_zac() 2026-07-02 6:34 ` [PATCH v2 1/5] ata: libata: rename ata_dev_is_zac() Damien Le Moal 2026-07-02 6:38 ` Hannes Reinecke @ 2026-07-02 7:16 ` sashiko-bot 1 sibling, 0 replies; 23+ messages in thread From: sashiko-bot @ 2026-07-02 7:16 UTC (permalink / raw) To: Damien Le Moal; +Cc: cassel, linux-ide, dlemoal Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Incomplete refactoring of ZBC checks leaves SCSI emulation in an inconsistent state for Device Managed drives. -- commit 8a69ac9e4f68da535d403caef5279dfdb19419f2 Author: Damien Le Moal <dlemoal@kernel.org> ata: libata: rename ata_dev_is_zac() The helper function ata_dev_is_zac() is renamed to ata_dev_is_zoned() to better reflect its checks. Existing callers and an open-coded check are updated to use the renamed helper. > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index d54ec1631e9a6..8811bfc373697 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c [ ... ] > @@ -3656,8 +3656,7 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, > break; > case ZBC_IN: > case ZBC_OUT: > - if (ata_id_zoned_cap(dev->id) || > - dev->class == ATA_DEV_ZAC) > + if (ata_dev_is_zoned(dev)) [Severity: High] Should this refactoring also be applied to ata_scsiop_inq_std() and ata_scsiop_read_cap() to prevent an inconsistent state for Device Managed drives? They still use the identical open-coded check: drivers/ata/libata-scsi.c:ata_scsiop_inq_std() { ... if (ata_id_zoned_cap(dev->id) || dev->class == ATA_DEV_ZAC) memcpy(rbuf + 58, versions_zbc, sizeof(versions_zbc)); ... } drivers/ata/libata-scsi.c:ata_scsiop_read_cap() { ... if (ata_id_zoned_cap(dev->id) || dev->class == ATA_DEV_ZAC) rbuf[12] = (1 << 4); /* RC_BASIS */ ... } By only replacing the check here in ata_scsi_report_supported_opcodes(), Device Managed zoned drives (ata_id_zoned_cap == 0x02) will correctly no longer report support for ZBC commands, but will still incorrectly claim ZBC capabilities in standard INQUIRY and set the ZBC-specific RC_BASIS bit in READ CAPACITY (16). > supported = 3; > break; > case SECURITY_PROTOCOL_IN: -- Sashiko AI review · https://sashiko.dev/#/patchset/20260702063412.1892584-1-dlemoal@kernel.org?part=1 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() 2026-07-02 6:34 [PATCH v2 0/5] Improve ata_scsi_report_supported_opcodes() Damien Le Moal 2026-07-02 6:34 ` [PATCH v2 1/5] ata: libata: rename ata_dev_is_zac() Damien Le Moal @ 2026-07-02 6:34 ` Damien Le Moal 2026-07-02 6:44 ` Hannes Reinecke 2026-07-02 6:53 ` sashiko-bot 2026-07-02 6:34 ` [PATCH v2 3/5] ata: libata-scsi: improve service action support in ata_scsi_report_supported_opcodes() Damien Le Moal ` (2 subsequent siblings) 4 siblings, 2 replies; 23+ messages in thread From: Damien Le Moal @ 2026-07-02 6:34 UTC (permalink / raw) To: linux-ide, Niklas Cassel ata_scsi_report_supported_opcodes() is very limited in functionality as it lacks support for the all command format and also does not handle correctly commands that have a service action. In preparation for adding these missing features, refactor how ata_scsi_report_supported_opcodes() operates to make modifications and extensions easier. To do so, introduce the array of supported commands ata_supported_cmds. This array entries are of type struct ata_scsi_cmd. This structure stores the operation code, CDB length, and the service action of a supported SCSI command that libata SAT can translate or emulate. Since some service actions (e.g. ZI_REPORT_ZONES) can have a value of 0, the field sa_valid of struct ata_scsi_cmd is used to indicate if the sa field is valid, or if it should be ignored. The helper function ata_scsi_get_supported_cmd() is implemented to search for a particular command by opcode in this array. This function is used in ata_scsi_cmd_is_supported() together with a struct ata_scsi_cmd_support to check based on the target device features if the specified command is supported. ata_scsi_cmd_is_supported() is used as the main function in ata_scsi_report_supported_opcodes() to determine if a particular command is supported and fill the command reply rbuf as needed. In the case of a command that is not supported, the support field is set to 1 as specified in SPC, indicating that the command is not supported. Of note is that the old ata_scsi_report_supported_opcodes() code did not handle the WRITE_SAME_16 and VARIABLE_LENGTH_CMD commands which are both supported and translated by libata-scsi. The ata_supported_cmds array includes these commands. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/ata/libata-scsi.c | 164 +++++++++++++++++++++++++------------- 1 file changed, 110 insertions(+), 54 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 8811bfc37369..4f0ae44f59dd 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3590,87 +3590,143 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) return 1; } -static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, - struct scsi_cmnd *cmd, - u8 *rbuf) +struct ata_scsi_cmd { + u8 op; + u8 cdb_len; + bool sa_valid; + u16 sa; +}; + +/* + * Array of commands supported with translation or emulation, sorted in + * ascending opcode and service action order. All of these commands are + * processed either in ata_xlat_func() or in ata_scsi_simulate(); + */ +static const struct ata_scsi_cmd ata_supported_cmds[] = { + { TEST_UNIT_READY, 6, false, 0 }, + { REZERO_UNIT, 6, false, 0 }, + { REQUEST_SENSE, 6, false, 0 }, + { READ_6, 6, false, 0 }, + { WRITE_6, 6, false, 0 }, + { SEEK_6, 6, false, 0 }, + { INQUIRY, 6, false, 0 }, + { MODE_SELECT, 6, false, 0 }, + { MODE_SENSE, 6, false, 0 }, + { START_STOP, 6, false, 0 }, + { SEND_DIAGNOSTIC, 6, false, 0 }, + { READ_CAPACITY, 10, false, 0 }, + { READ_10, 10, false, 0 }, + { WRITE_10, 10, false, 0 }, + { SEEK_10, 10, false, 0 }, + { VERIFY, 10, false, 0 }, + { SYNCHRONIZE_CACHE, 10, false, 0 }, + { MODE_SELECT_10, 10, false, 0 }, + { MODE_SENSE_10, 10, false, 0 }, + { VARIABLE_LENGTH_CMD, 32, true, ATA_32 }, + { ATA_16, 16, false, 0 }, + { READ_16, 16, false, 0 }, + { WRITE_16, 16, false, 0 }, + { VERIFY_16, 16, false, 0 }, + { SYNCHRONIZE_CACHE_16, 16, false, 0 }, + { WRITE_SAME_16, 16, false, 0 }, + { ZBC_OUT, 16, true, ZO_CLOSE_ZONE }, + { ZBC_OUT, 16, true, ZO_FINISH_ZONE }, + { ZBC_OUT, 16, true, ZO_OPEN_ZONE }, + { ZBC_OUT, 16, true, ZO_RESET_WRITE_POINTER }, + { ZBC_IN, 16, true, ZI_REPORT_ZONES }, + { SERVICE_ACTION_IN_16, 16, true, SAI_READ_CAPACITY_16 }, + { REPORT_LUNS, 12, false, 0 }, + { ATA_12, 12, false, 0 }, + { SECURITY_PROTOCOL_IN, 12, false, 0 }, + { MAINTENANCE_IN, 12, true, MI_REPORT_SUPPORTED_OPERATION_CODES }, + { SECURITY_PROTOCOL_OUT, 12, false, 0 }, +}; + +static const struct ata_scsi_cmd *ata_scsi_get_supported_cmd(u8 op) { - u8 *cdb = cmd->cmnd; - u8 supported = 0, cdlp = 0, rwcdlp = 0; + const struct ata_scsi_cmd *cmd; + int i; - if (cdb[2] != 1 && cdb[2] != 3) { - ata_dev_warn(dev, "invalid command format %d\n", cdb[2]); - ata_scsi_set_invalid_field(dev, cmd, 2, 0xff); - return 0; + for (i = 0; i < ARRAY_SIZE(ata_supported_cmds); i++) { + cmd = &ata_supported_cmds[i]; + if (cmd->op == op) + return cmd; } - switch (cdb[3]) { - case INQUIRY: - case MODE_SENSE: - case MODE_SENSE_10: - case READ_CAPACITY: - case SERVICE_ACTION_IN_16: - case REPORT_LUNS: - case REQUEST_SENSE: - case SYNCHRONIZE_CACHE: - case SYNCHRONIZE_CACHE_16: - case REZERO_UNIT: - case SEEK_6: - case SEEK_10: - case TEST_UNIT_READY: - case SEND_DIAGNOSTIC: - case MAINTENANCE_IN: - case READ_6: - case READ_10: - case WRITE_6: - case WRITE_10: - case ATA_12: - case ATA_16: - case VERIFY: - case VERIFY_16: - case MODE_SELECT: - case MODE_SELECT_10: - case START_STOP: - supported = 3; - break; + return NULL; +} + +struct ata_scsi_cmd_support { + u8 cdlp; + u8 rwcdlp; +}; + +static bool ata_scsi_cmd_is_supported(struct ata_device *dev, u8 op, + struct ata_scsi_cmd_support *sup) +{ + const struct ata_scsi_cmd *cmd; + + /* First, see if we support the command. */ + cmd = ata_scsi_get_supported_cmd(op); + if (!cmd) + return false; + + /* Now refine the support report depending on the device features. */ + memset(sup, 0, sizeof(*sup)); + switch (op) { case READ_16: - supported = 3; if (dev->flags & ATA_DFLAG_CDL) { /* * CDL read descriptors map to the T2A page, that is, * rwcdlp = 0x01 and cdlp = 0x01 */ - rwcdlp = 0x01; - cdlp = 0x01 << 3; + sup->rwcdlp = 0x01; + sup->cdlp = 0x01; } - break; + return true; case WRITE_16: - supported = 3; if (dev->flags & ATA_DFLAG_CDL) { /* * CDL write descriptors map to the T2B page, that is, * rwcdlp = 0x01 and cdlp = 0x02 */ - rwcdlp = 0x01; - cdlp = 0x02 << 3; + sup->rwcdlp = 0x01; + sup->cdlp = 0x02; } - break; + return true; case ZBC_IN: case ZBC_OUT: - if (ata_dev_is_zoned(dev)) - supported = 3; - break; + return ata_dev_is_zoned(dev); case SECURITY_PROTOCOL_IN: case SECURITY_PROTOCOL_OUT: - if (dev->flags & ATA_DFLAG_TRUSTED) - supported = 3; - break; + return dev->flags & ATA_DFLAG_TRUSTED; default: break; } + return true; +} + +static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, + struct scsi_cmnd *cmd, + u8 *rbuf) +{ + struct ata_scsi_cmd_support sup; + u8 *cdb = cmd->cmnd; + + if (cdb[2] != 1 && cdb[2] != 3) { + ata_dev_warn(dev, "invalid command format %d\n", cdb[2]); + ata_scsi_set_invalid_field(dev, cmd, 2, 0xff); + return 0; + } + /* One command format */ - rbuf[0] = rwcdlp; - rbuf[1] = cdlp | supported; + if (ata_scsi_cmd_is_supported(dev, cdb[3], &sup)) { + rbuf[0] = sup.rwcdlp; + rbuf[1] = (sup.cdlp << 3) | 0x03; + } else { + rbuf[1] = 0x01; + } return 4; } -- 2.54.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() 2026-07-02 6:34 ` [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() Damien Le Moal @ 2026-07-02 6:44 ` Hannes Reinecke 2026-07-02 7:01 ` Damien Le Moal 2026-07-02 6:53 ` sashiko-bot 1 sibling, 1 reply; 23+ messages in thread From: Hannes Reinecke @ 2026-07-02 6:44 UTC (permalink / raw) To: Damien Le Moal, linux-ide, Niklas Cassel On 7/2/26 8:34 AM, Damien Le Moal wrote: > ata_scsi_report_supported_opcodes() is very limited in functionality as it > lacks support for the all command format and also does not handle > correctly commands that have a service action. > > In preparation for adding these missing features, refactor how > ata_scsi_report_supported_opcodes() operates to make modifications and > extensions easier. To do so, introduce the array of supported commands > ata_supported_cmds. This array entries are of type struct ata_scsi_cmd. > This structure stores the operation code, CDB length, and the service > action of a supported SCSI command that libata SAT can translate or > emulate. Since some service actions (e.g. ZI_REPORT_ZONES) can have a > value of 0, the field sa_valid of struct ata_scsi_cmd is used to indicate > if the sa field is valid, or if it should be ignored. > > The helper function ata_scsi_get_supported_cmd() is implemented to search > for a particular command by opcode in this array. This function is used in > ata_scsi_cmd_is_supported() together with a struct ata_scsi_cmd_support to > check based on the target device features if the specified command is > supported. > > ata_scsi_cmd_is_supported() is used as the main function in > ata_scsi_report_supported_opcodes() to determine if a particular command > is supported and fill the command reply rbuf as needed. In the case of a > command that is not supported, the support field is set to 1 as specified > in SPC, indicating that the command is not supported. > > Of note is that the old ata_scsi_report_supported_opcodes() code did not > handle the WRITE_SAME_16 and VARIABLE_LENGTH_CMD commands which are both > supported and translated by libata-scsi. The ata_supported_cmds array > includes these commands. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-scsi.c | 164 +++++++++++++++++++++++++------------- > 1 file changed, 110 insertions(+), 54 deletions(-) > > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index 8811bfc37369..4f0ae44f59dd 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c > @@ -3590,87 +3590,143 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc) > return 1; > } > > -static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, > - struct scsi_cmnd *cmd, > - u8 *rbuf) > +struct ata_scsi_cmd { > + u8 op; > + u8 cdb_len; > + bool sa_valid; > + u16 sa; > +}; > + > +/* > + * Array of commands supported with translation or emulation, sorted in > + * ascending opcode and service action order. All of these commands are > + * processed either in ata_xlat_func() or in ata_scsi_simulate(); > + */ > +static const struct ata_scsi_cmd ata_supported_cmds[] = { > + { TEST_UNIT_READY, 6, false, 0 }, > + { REZERO_UNIT, 6, false, 0 }, > + { REQUEST_SENSE, 6, false, 0 }, > + { READ_6, 6, false, 0 }, > + { WRITE_6, 6, false, 0 }, > + { SEEK_6, 6, false, 0 }, > + { INQUIRY, 6, false, 0 }, > + { MODE_SELECT, 6, false, 0 }, > + { MODE_SENSE, 6, false, 0 }, > + { START_STOP, 6, false, 0 }, > + { SEND_DIAGNOSTIC, 6, false, 0 }, > + { READ_CAPACITY, 10, false, 0 }, > + { READ_10, 10, false, 0 }, > + { WRITE_10, 10, false, 0 }, > + { SEEK_10, 10, false, 0 }, > + { VERIFY, 10, false, 0 }, > + { SYNCHRONIZE_CACHE, 10, false, 0 }, > + { MODE_SELECT_10, 10, false, 0 }, > + { MODE_SENSE_10, 10, false, 0 }, > + { VARIABLE_LENGTH_CMD, 32, true, ATA_32 }, > + { ATA_16, 16, false, 0 }, > + { READ_16, 16, false, 0 }, > + { WRITE_16, 16, false, 0 }, > + { VERIFY_16, 16, false, 0 }, > + { SYNCHRONIZE_CACHE_16, 16, false, 0 }, > + { WRITE_SAME_16, 16, false, 0 }, > + { ZBC_OUT, 16, true, ZO_CLOSE_ZONE }, > + { ZBC_OUT, 16, true, ZO_FINISH_ZONE }, > + { ZBC_OUT, 16, true, ZO_OPEN_ZONE }, > + { ZBC_OUT, 16, true, ZO_RESET_WRITE_POINTER }, > + { ZBC_IN, 16, true, ZI_REPORT_ZONES }, > + { SERVICE_ACTION_IN_16, 16, true, SAI_READ_CAPACITY_16 }, > + { REPORT_LUNS, 12, false, 0 }, > + { ATA_12, 12, false, 0 }, > + { SECURITY_PROTOCOL_IN, 12, false, 0 }, > + { MAINTENANCE_IN, 12, true, MI_REPORT_SUPPORTED_OPERATION_CODES }, > + { SECURITY_PROTOCOL_OUT, 12, false, 0 }, > +}; This probably is a tough ask, but wouldn't it be better to move to named initializer? IE things like { .op = TEST_UNIT_READY, .cdb_len = 0, .sa_valid = false, .sa = 0 }, that will not only make it easier to read, but will also be more user friendly if someone needs to expand the structure. > + > +static const struct ata_scsi_cmd *ata_scsi_get_supported_cmd(u8 op) > { > - u8 *cdb = cmd->cmnd; > - u8 supported = 0, cdlp = 0, rwcdlp = 0; > + const struct ata_scsi_cmd *cmd; > + int i; > > - if (cdb[2] != 1 && cdb[2] != 3) { > - ata_dev_warn(dev, "invalid command format %d\n", cdb[2]); > - ata_scsi_set_invalid_field(dev, cmd, 2, 0xff); > - return 0; > + for (i = 0; i < ARRAY_SIZE(ata_supported_cmds); i++) { > + cmd = &ata_supported_cmds[i]; > + if (cmd->op == op) > + return cmd; > } > > - switch (cdb[3]) { > - case INQUIRY: > - case MODE_SENSE: > - case MODE_SENSE_10: > - case READ_CAPACITY: > - case SERVICE_ACTION_IN_16: > - case REPORT_LUNS: > - case REQUEST_SENSE: > - case SYNCHRONIZE_CACHE: > - case SYNCHRONIZE_CACHE_16: > - case REZERO_UNIT: > - case SEEK_6: > - case SEEK_10: > - case TEST_UNIT_READY: > - case SEND_DIAGNOSTIC: > - case MAINTENANCE_IN: > - case READ_6: > - case READ_10: > - case WRITE_6: > - case WRITE_10: > - case ATA_12: > - case ATA_16: > - case VERIFY: > - case VERIFY_16: > - case MODE_SELECT: > - case MODE_SELECT_10: > - case START_STOP: > - supported = 3; > - break; > + return NULL; > +} > + > +struct ata_scsi_cmd_support { > + u8 cdlp; > + u8 rwcdlp; > +}; > + > +static bool ata_scsi_cmd_is_supported(struct ata_device *dev, u8 op, > + struct ata_scsi_cmd_support *sup) > +{ > + const struct ata_scsi_cmd *cmd; > + > + /* First, see if we support the command. */ > + cmd = ata_scsi_get_supported_cmd(op); > + if (!cmd) > + return false; > + > + /* Now refine the support report depending on the device features. */ > + memset(sup, 0, sizeof(*sup)); > + switch (op) { > case READ_16: > - supported = 3; > if (dev->flags & ATA_DFLAG_CDL) { > /* > * CDL read descriptors map to the T2A page, that is, > * rwcdlp = 0x01 and cdlp = 0x01 > */ > - rwcdlp = 0x01; > - cdlp = 0x01 << 3; > + sup->rwcdlp = 0x01; > + sup->cdlp = 0x01; > } > - break; > + return true; > case WRITE_16: > - supported = 3; > if (dev->flags & ATA_DFLAG_CDL) { > /* > * CDL write descriptors map to the T2B page, that is, > * rwcdlp = 0x01 and cdlp = 0x02 > */ > - rwcdlp = 0x01; > - cdlp = 0x02 << 3; > + sup->rwcdlp = 0x01; > + sup->cdlp = 0x02; > } > - break; > + return true; > case ZBC_IN: > case ZBC_OUT: > - if (ata_dev_is_zoned(dev)) > - supported = 3; > - break; > + return ata_dev_is_zoned(dev); > case SECURITY_PROTOCOL_IN: > case SECURITY_PROTOCOL_OUT: > - if (dev->flags & ATA_DFLAG_TRUSTED) > - supported = 3; > - break; > + return dev->flags & ATA_DFLAG_TRUSTED; > default: > break; > } > > + return true; > +} > + > +static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, > + struct scsi_cmnd *cmd, > + u8 *rbuf) > +{ > + struct ata_scsi_cmd_support sup; > + u8 *cdb = cmd->cmnd; > + > + if (cdb[2] != 1 && cdb[2] != 3) { > + ata_dev_warn(dev, "invalid command format %d\n", cdb[2]); > + ata_scsi_set_invalid_field(dev, cmd, 2, 0xff); > + return 0; > + } > + > /* One command format */ > - rbuf[0] = rwcdlp; > - rbuf[1] = cdlp | supported; > + if (ata_scsi_cmd_is_supported(dev, cdb[3], &sup)) { > + rbuf[0] = sup.rwcdlp; > + rbuf[1] = (sup.cdlp << 3) | 0x03; > + } else { > + rbuf[1] = 0x01; > + } > > return 4; > } Otherwise looks good. 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() 2026-07-02 6:44 ` Hannes Reinecke @ 2026-07-02 7:01 ` Damien Le Moal 0 siblings, 0 replies; 23+ messages in thread From: Damien Le Moal @ 2026-07-02 7:01 UTC (permalink / raw) To: Hannes Reinecke, linux-ide, Niklas Cassel On 7/2/26 15:44, Hannes Reinecke wrote: >> + { REPORT_LUNS, 12, false, 0 }, >> + { ATA_12, 12, false, 0 }, >> + { SECURITY_PROTOCOL_IN, 12, false, 0 }, >> + { MAINTENANCE_IN, 12, true, MI_REPORT_SUPPORTED_OPERATION_CODES }, >> + { SECURITY_PROTOCOL_OUT, 12, false, 0 }, >> +}; > > This probably is a tough ask, but wouldn't it be better to > move to named initializer? > IE things like > > { .op = TEST_UNIT_READY, .cdb_len = 0, .sa_valid = false, .sa = 0 }, > > that will not only make it easier to read, but will also be > more user friendly if someone needs to expand the structure. Arg. OK. Will do. -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() 2026-07-02 6:34 ` [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() Damien Le Moal 2026-07-02 6:44 ` Hannes Reinecke @ 2026-07-02 6:53 ` sashiko-bot 2026-07-02 13:25 ` Niklas Cassel 1 sibling, 1 reply; 23+ messages in thread From: sashiko-bot @ 2026-07-02 6:53 UTC (permalink / raw) To: Damien Le Moal; +Cc: dlemoal, linux-ide, cassel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] WRITE_SAME_16 is unconditionally reported as supported via REPORT SUPPORTED OPERATION CODES, even for devices that do not support TRIM. -- commit b8e6e9f4e6e3177fe0e766704af6b504468c2de3 Author: Damien Le Moal <dlemoal@kernel.org> ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() This commit refactors ata_scsi_report_supported_opcodes() to use an array of supported commands for easier modifications and extensions. It also adds handling for WRITE_SAME_16 and VARIABLE_LENGTH_CMD to the list of commands which libata-scsi translates. > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index 8811bfc373697..4f0ae44f59dd2 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c [ ... ] > +static bool ata_scsi_cmd_is_supported(struct ata_device *dev, u8 op, > + struct ata_scsi_cmd_support *sup) > +{ > + const struct ata_scsi_cmd *cmd; > + > + /* First, see if we support the command. */ > + cmd = ata_scsi_get_supported_cmd(op); > + if (!cmd) > + return false; > + > + /* Now refine the support report depending on the device features. */ > + memset(sup, 0, sizeof(*sup)); > + switch (op) { > + case READ_16: [ ... ] > + case ZBC_IN: > + case ZBC_OUT: > + return ata_dev_is_zoned(dev); > + case SECURITY_PROTOCOL_IN: > + case SECURITY_PROTOCOL_OUT: > + return dev->flags & ATA_DFLAG_TRUSTED; > + default: > + break; > + } > + > + return true; > +} [Severity: Medium] Should WRITE_SAME_16 be refined here to check for TRIM support? The commit message mentions adding WRITE_SAME_16 to the supported array, so it falls through to the default case in this switch statement and unconditionally returns true. However, looking at ata_scsi_write_same_xlat(), the command is unconditionally rejected if the device lacks TRIM capabilities: ata_scsi_write_same_xlat() ... if (!unmap || (dev->quirks & ATA_QUIRK_NOTRIM) || !ata_id_has_trim(dev->id)) { fp = 1; bp = 3; goto invalid_fld; } Does this cause the report supported operation codes query to incorrectly inform the SCSI layer that WRITE_SAME_16 is supported on ATA devices that do not actually support TRIM? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260702063412.1892584-1-dlemoal@kernel.org?part=2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() 2026-07-02 6:53 ` sashiko-bot @ 2026-07-02 13:25 ` Niklas Cassel 2026-07-02 14:36 ` Niklas Cassel 0 siblings, 1 reply; 23+ messages in thread From: Niklas Cassel @ 2026-07-02 13:25 UTC (permalink / raw) To: sashiko-reviews; +Cc: Damien Le Moal, linux-ide On Thu, Jul 02, 2026 at 06:53:20AM +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] WRITE_SAME_16 is unconditionally reported as supported via REPORT SUPPORTED OPERATION CODES, even for devices that do not support TRIM. > -- > > commit b8e6e9f4e6e3177fe0e766704af6b504468c2de3 > Author: Damien Le Moal <dlemoal@kernel.org> > > ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() > > This commit refactors ata_scsi_report_supported_opcodes() to use an array > of supported commands for easier modifications and extensions. It also adds > handling for WRITE_SAME_16 and VARIABLE_LENGTH_CMD to the list of commands > which libata-scsi translates. > > > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > > index 8811bfc373697..4f0ae44f59dd2 100644 > > --- a/drivers/ata/libata-scsi.c > > +++ b/drivers/ata/libata-scsi.c > [ ... ] > > +static bool ata_scsi_cmd_is_supported(struct ata_device *dev, u8 op, > > + struct ata_scsi_cmd_support *sup) > > +{ > > + const struct ata_scsi_cmd *cmd; > > + > > + /* First, see if we support the command. */ > > + cmd = ata_scsi_get_supported_cmd(op); > > + if (!cmd) > > + return false; > > + > > + /* Now refine the support report depending on the device features. */ > > + memset(sup, 0, sizeof(*sup)); > > + switch (op) { > > + case READ_16: > [ ... ] > > + case ZBC_IN: > > + case ZBC_OUT: > > + return ata_dev_is_zoned(dev); > > + case SECURITY_PROTOCOL_IN: > > + case SECURITY_PROTOCOL_OUT: > > + return dev->flags & ATA_DFLAG_TRUSTED; > > + default: > > + break; > > + } > > + > > + return true; > > +} > > [Severity: Medium] > Should WRITE_SAME_16 be refined here to check for TRIM support? No, AFAICT, libata should not expose WRITE SAME (16) support, regardless if TRIM is supported or not. In ata_scsi_write_same_xlat(), we have: /* * We only allow sending this command through the block layer, * as it modifies the DATA OUT buffer, which would corrupt user * memory for SG_IO commands. */ if (unlikely(blk_rq_is_passthrough(scsi_cmd_to_rq(scmd)))) goto invalid_opcode; This means that we will always reject a WRITE SAME (16) passthrough command. Additionally, in ata_scsi_sdev_config(), we have sdev->no_write_same = 1; Thus the only way we can reach ata_scsi_write_same_xlat() is via the block layer. Block layer commands that can reach ata_scsi_write_same_xlat(): REQ_OP_WRITE_ZEROES: In sd.c, sd_setup_write_zeroes_cmnd() we have: if (sdp->no_write_same) return BLK_STS_TARGET; So REQ_OP_WRITE_ZEROES will not reach ata_scsi_write_same_xlat(). REQ_OP_DISCARD: In sd.c, REQ_OP_DISCARD builds a WRITE_SAME_16 with the UNMAP bit and never consults no_write_same. provisioning_mode == SD_LBP_WS16 is chosen by sd_discard_mode() purely from the LBP/thin-provisioning bits (lbpme, lbpws - both of which libata advertises for a TRIM device), again independent of no_write_same. So a libata TRIM device ends up with no_write_same = 1 and provisioning_mode = SD_LBP_WS16 simultaneously, and every discard is a WRITE SAME(16) UNMAP that lands in ata_scsi_write_same_xlat() -> DSM TRIM. That's exactly the intended arrangement: no_write_same turns off the "write a pattern" use while leaving discard working. This seems correct as ATA has no way to splat an arbitrary pattern across many blocks in one command. Kind regards, Niklas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() 2026-07-02 13:25 ` Niklas Cassel @ 2026-07-02 14:36 ` Niklas Cassel 2026-07-02 14:48 ` Christoph Hellwig 0 siblings, 1 reply; 23+ messages in thread From: Niklas Cassel @ 2026-07-02 14:36 UTC (permalink / raw) To: sashiko-reviews; +Cc: Damien Le Moal, linux-ide, Christoph Hellwig I guess we could theoretically report WRITE SAME (16) support in REPORTED SUPPORTED OPERATION CODES. A WRITE SAME (16) command can either have the unmap bit set or not. ata_scsi_write_same_xlat() already rejects a WRITE SAME (16) command that does not have the unmap bit set: if (!unmap || (dev->quirks & ATA_QUIRK_NOTRIM) || !ata_id_has_trim(dev->id)) { fp = 1; bp = 3; goto invalid_fld; } I don't see anything in the SBC / SPC that forbids a device from acting in this way. Some searching claims that some controllers do reject any WRITE SAME (16) that does not have the unmap bit set. To mark that we suport WRITE SAME (16) for deallocation, in the Logical Block Provisioning VPD Page (0xB2), we currently set: " LBPWS (Logical Block Provisioning Write Same 16) Bit: This bit must be set to 1 to indicate that the device explicitly supports setting the UNMAP bit in a WRITE SAME (16) command to deallocate blocks. " This is done in ata_scsiop_inq_b2(): rbuf[5] = 1 << 6; However, considering that we always reject a passthrough WRITE SAME (16), I am not sure if we really want to report WRITE SAME (16) support in REPORTED SUPPORTED OPERATION CODES. If feels weird to report support for something, but if the user actually tries to submit such a command via SG_IO, it would be rejected (even if the unmap bit is set). Perhaps it is best to just continue to using it internally as an intermediate command/representation for REQ_OP_DISCARD ? I did find this old series from Christoph, that adds a ATA_TRIM SCSI vendor specific command to translate to DSM TRIM: https://lore.kernel.org/all/20170320204319.12628-3-hch@lst.de/T/#u I do like the idea, as we would no longer be limited by the sector size (the SCSI Data-Out buffer), when issuing trims. The downside is that it adds ATA specific code to sd.c (see sd_setup_ata_trim_cmnd()), and I guess that is why the proposal was NAKed by SCSI maintainers. With the current solution, which is dependent on the sector size (i.e. the WRITE SAME (16) Data-out buffer size), for a device with a sector size of 512, we can issue TRIMs of size: 512 * 0xffff * 64 = 1 Gi + 1023 Mi + 992 Ki A trim of 2 GB, so perhaps it is not so important to increase/migrate away from the current solution (which is dependent on the sector size). One of the biggest complaints Christoph had was the use of the ata_scsi_rbuf and ata_scsi_rbuf_lock, and that concern seem to be addressed by my series: https://lore.kernel.org/linux-ide/20260702105956.2058733-4-cassel@kernel.org/T/#u Kind regards, Niklas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() 2026-07-02 14:36 ` Niklas Cassel @ 2026-07-02 14:48 ` Christoph Hellwig 2026-07-02 14:53 ` Niklas Cassel 0 siblings, 1 reply; 23+ messages in thread From: Christoph Hellwig @ 2026-07-02 14:48 UTC (permalink / raw) To: Niklas Cassel Cc: sashiko-reviews, Damien Le Moal, linux-ide, Christoph Hellwig Who still cares about TRIM on ATA these days to bother with any of this? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() 2026-07-02 14:48 ` Christoph Hellwig @ 2026-07-02 14:53 ` Niklas Cassel 0 siblings, 0 replies; 23+ messages in thread From: Niklas Cassel @ 2026-07-02 14:53 UTC (permalink / raw) To: Christoph Hellwig; +Cc: sashiko-reviews, Damien Le Moal, linux-ide On Thu, Jul 02, 2026 at 04:48:16PM +0200, Christoph Hellwig wrote: > Who still cares about TRIM on ATA these days to bother with any > of this? > Have you not heard, ATA SSDs are coming back: https://www.tomshardware.com/pc-components/ssds/sandisk-brings-back-affordable-storage-to-rescue-buyers-from-the-ssd-crisis-new-320-and-520-sata-ssds-are-ready-to-launch :) Seriously though, was mostly interested to hear you opinion wrt. if WRITE SAME (16) should be exposed in REPORT SUPPORTED OPERATION CODES. But with all the information I have, I still think it is better to just continue to using WRITE SAME (16) internally as an intermediate command for REQ_OP_DISCARD, and not expose support in REPORT SUPPORTED OPERATION CODES. Kind regards, Niklas ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 3/5] ata: libata-scsi: improve service action support in ata_scsi_report_supported_opcodes() 2026-07-02 6:34 [PATCH v2 0/5] Improve ata_scsi_report_supported_opcodes() Damien Le Moal 2026-07-02 6:34 ` [PATCH v2 1/5] ata: libata: rename ata_dev_is_zac() Damien Le Moal 2026-07-02 6:34 ` [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() Damien Le Moal @ 2026-07-02 6:34 ` Damien Le Moal 2026-07-02 6:45 ` Hannes Reinecke 2026-07-02 6:47 ` sashiko-bot 2026-07-02 6:34 ` [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES Damien Le Moal 2026-07-02 6:34 ` [PATCH v2 5/5] ata: libata-scsi: support the all command format for reporting supported commands Damien Le Moal 4 siblings, 2 replies; 23+ messages in thread From: Damien Le Moal @ 2026-07-02 6:34 UTC (permalink / raw) To: linux-ide, Niklas Cassel ata_scsi_report_supported_opcodes() is ignoring the service action specified in the SCSI command CDB, but the one command format must take this field into consideration. With the reporting options field set to 1, the REPORT SUPPORTED OPERATION CODES command must be failed if the specified opcode to check has service actions, while reporting option 3 must match supported opcodes together with the specified service action. Stop ignoring the service action by passing it to ata_scsi_cmd_is_supported() and searching for commands in the array of supported commands (ata_supported_cmds) using both the command opcode and service action. Introduce the helper function ata_scsi_supported_cmd_use_sa() to determine if a particular command has service actions and use this function to fail a REPORT SUPPORTED OPERATION CODES command if such command is specified with reporting options 1. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/ata/libata-scsi.c | 41 ++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 4f0ae44f59dd..86ac41a7d81a 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3642,32 +3642,46 @@ static const struct ata_scsi_cmd ata_supported_cmds[] = { { SECURITY_PROTOCOL_OUT, 12, false, 0 }, }; -static const struct ata_scsi_cmd *ata_scsi_get_supported_cmd(u8 op) +static const struct ata_scsi_cmd *ata_scsi_get_supported_cmd(u8 op, u16 sa) { const struct ata_scsi_cmd *cmd; int i; for (i = 0; i < ARRAY_SIZE(ata_supported_cmds); i++) { cmd = &ata_supported_cmds[i]; - if (cmd->op == op) + if (cmd->op == op && cmd->sa == sa) return cmd; } return NULL; } +static bool ata_scsi_supported_cmd_use_sa(u8 op) +{ + const struct ata_scsi_cmd *cmd; + int i; + + for (i = 0; i < ARRAY_SIZE(ata_supported_cmds); i++) { + cmd = &ata_supported_cmds[i]; + if (cmd->op == op) + return cmd->sa_valid; + } + + return false; +} + struct ata_scsi_cmd_support { u8 cdlp; u8 rwcdlp; }; -static bool ata_scsi_cmd_is_supported(struct ata_device *dev, u8 op, +static bool ata_scsi_cmd_is_supported(struct ata_device *dev, u8 op, u16 sa, struct ata_scsi_cmd_support *sup) { const struct ata_scsi_cmd *cmd; /* First, see if we support the command. */ - cmd = ata_scsi_get_supported_cmd(op); + cmd = ata_scsi_get_supported_cmd(op, sa); if (!cmd) return false; @@ -3713,15 +3727,28 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, { struct ata_scsi_cmd_support sup; u8 *cdb = cmd->cmnd; - - if (cdb[2] != 1 && cdb[2] != 3) { + u16 sa = 0; + + switch (cdb[2]) { + case 1: + /* One command format with command support data, ignore sa. */ + if (ata_scsi_supported_cmd_use_sa(cdb[3])) { + ata_scsi_set_invalid_field(dev, cmd, 3, 0xff); + return 0; + } + break; + case 3: + /* One command format */ + sa = get_unaligned_be16(&cdb[4]); + break; + default: ata_dev_warn(dev, "invalid command format %d\n", cdb[2]); ata_scsi_set_invalid_field(dev, cmd, 2, 0xff); return 0; } /* One command format */ - if (ata_scsi_cmd_is_supported(dev, cdb[3], &sup)) { + if (ata_scsi_cmd_is_supported(dev, cdb[3], sa, &sup)) { rbuf[0] = sup.rwcdlp; rbuf[1] = (sup.cdlp << 3) | 0x03; } else { -- 2.54.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/5] ata: libata-scsi: improve service action support in ata_scsi_report_supported_opcodes() 2026-07-02 6:34 ` [PATCH v2 3/5] ata: libata-scsi: improve service action support in ata_scsi_report_supported_opcodes() Damien Le Moal @ 2026-07-02 6:45 ` Hannes Reinecke 2026-07-02 6:47 ` sashiko-bot 1 sibling, 0 replies; 23+ messages in thread From: Hannes Reinecke @ 2026-07-02 6:45 UTC (permalink / raw) To: Damien Le Moal, linux-ide, Niklas Cassel On 7/2/26 8:34 AM, Damien Le Moal wrote: > ata_scsi_report_supported_opcodes() is ignoring the service action > specified in the SCSI command CDB, but the one command format must take > this field into consideration. With the reporting options field set to 1, > the REPORT SUPPORTED OPERATION CODES command must be failed if the > specified opcode to check has service actions, while reporting option 3 > must match supported opcodes together with the specified service action. > > Stop ignoring the service action by passing it to > ata_scsi_cmd_is_supported() and searching for commands in the > array of supported commands (ata_supported_cmds) using both the command > opcode and service action. > > Introduce the helper function ata_scsi_supported_cmd_use_sa() to determine > if a particular command has service actions and use this function to fail > a REPORT SUPPORTED OPERATION CODES command if such command is specified > with reporting options 1. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-scsi.c | 41 ++++++++++++++++++++++++++++++++------- > 1 file changed, 34 insertions(+), 7 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/5] ata: libata-scsi: improve service action support in ata_scsi_report_supported_opcodes() 2026-07-02 6:34 ` [PATCH v2 3/5] ata: libata-scsi: improve service action support in ata_scsi_report_supported_opcodes() Damien Le Moal 2026-07-02 6:45 ` Hannes Reinecke @ 2026-07-02 6:47 ` sashiko-bot 1 sibling, 0 replies; 23+ messages in thread From: sashiko-bot @ 2026-07-02 6:47 UTC (permalink / raw) To: Damien Le Moal; +Cc: dlemoal, linux-ide, cassel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Improper handling of the REQUESTED SERVICE ACTION field for reporting option 3 in REPORT SUPPORTED OPERATION CODES. -- commit b3caa191a7aab568905b6cb9a00aa04fd0f6cf62 Author: Damien Le Moal <dlemoal@kernel.org> ata: libata-scsi: improve service action support in ata_scsi_report_supported_opcodes() This commit updates ata_scsi_report_supported_opcodes() to consider the service action specified in the SCSI command CDB. It adds support for failing reporting options 1 if the opcode has service actions, and matches opcodes with their service actions for reporting option 3. > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index 4f0ae44f59dd2..86ac41a7d81ab 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c > @@ -3642,32 +3642,46 @@ static const struct ata_scsi_cmd ata_supported_cmds[] = { > { SECURITY_PROTOCOL_OUT, 12, false, 0 }, > }; > > -static const struct ata_scsi_cmd *ata_scsi_get_supported_cmd(u8 op) > +static const struct ata_scsi_cmd *ata_scsi_get_supported_cmd(u8 op, u16 sa) > { > const struct ata_scsi_cmd *cmd; > int i; > > for (i = 0; i < ARRAY_SIZE(ata_supported_cmds); i++) { > cmd = &ata_supported_cmds[i]; > - if (cmd->op == op) > + if (cmd->op == op && cmd->sa == sa) [Severity: Medium] If the queried command does not have a service action, its cmd->sa will be 0. If a non-zero service action was passed in through sa, this strict comparison will fail. Since SPC-5 states that the REQUESTED SERVICE ACTION field "shall be ignored" when the operation code does not have service actions, could this result in valid commands being incorrectly reported as unsupported? > return cmd; > } > > return NULL; > } [ ... ] > @@ -3713,15 +3727,28 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, > { > struct ata_scsi_cmd_support sup; > u8 *cdb = cmd->cmnd; > - > - if (cdb[2] != 1 && cdb[2] != 3) { > + u16 sa = 0; > + > + switch (cdb[2]) { > + case 1: > + /* One command format with command support data, ignore sa. */ > + if (ata_scsi_supported_cmd_use_sa(cdb[3])) { > + ata_scsi_set_invalid_field(dev, cmd, 3, 0xff); > + return 0; > + } > + break; > + case 3: > + /* One command format */ > + sa = get_unaligned_be16(&cdb[4]); > + break; [Severity: Medium] If the queried command (e.g., READ_10) doesn't use service actions, does extracting the service action unconditionally here pass a non-zero value to ata_scsi_get_supported_cmd() if the initiator provided one? Should sa remain 0 if ata_scsi_supported_cmd_use_sa(cdb[3]) is false? > + default: > ata_dev_warn(dev, "invalid command format %d\n", cdb[2]); > ata_scsi_set_invalid_field(dev, cmd, 2, 0xff); > return 0; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260702063412.1892584-1-dlemoal@kernel.org?part=3 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES 2026-07-02 6:34 [PATCH v2 0/5] Improve ata_scsi_report_supported_opcodes() Damien Le Moal ` (2 preceding siblings ...) 2026-07-02 6:34 ` [PATCH v2 3/5] ata: libata-scsi: improve service action support in ata_scsi_report_supported_opcodes() Damien Le Moal @ 2026-07-02 6:34 ` Damien Le Moal 2026-07-02 6:46 ` Hannes Reinecke ` (2 more replies) 2026-07-02 6:34 ` [PATCH v2 5/5] ata: libata-scsi: support the all command format for reporting supported commands Damien Le Moal 4 siblings, 3 replies; 23+ messages in thread From: Damien Le Moal @ 2026-07-02 6:34 UTC (permalink / raw) To: linux-ide, Niklas Cassel Add support for the reporting options value 2 of the in REPORT SUPPORTED OPERATION CODES command in ata_scsi_report_supported_opcodes(). With this reporting option, the specified opcode to check must have a service action. Use ata_scsi_supported_cmd_has_sa() to check for this and fill the command reply buffer using the one command format based on the command support. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/ata/libata-scsi.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 86ac41a7d81a..e75ed573e234 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3737,6 +3737,13 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, return 0; } break; + case 2: + /* One command format, must have sa. */ + if (!ata_scsi_supported_cmd_use_sa(cdb[3])) { + ata_scsi_set_invalid_field(dev, cmd, 3, 0xff); + return 0; + } + fallthrough; case 3: /* One command format */ sa = get_unaligned_be16(&cdb[4]); -- 2.54.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES 2026-07-02 6:34 ` [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES Damien Le Moal @ 2026-07-02 6:46 ` Hannes Reinecke 2026-07-02 6:52 ` sashiko-bot 2026-07-02 6:53 ` Hannes Reinecke 2 siblings, 0 replies; 23+ messages in thread From: Hannes Reinecke @ 2026-07-02 6:46 UTC (permalink / raw) To: Damien Le Moal, linux-ide, Niklas Cassel On 7/2/26 8:34 AM, Damien Le Moal wrote: > Add support for the reporting options value 2 of the in REPORT SUPPORTED > OPERATION CODES command in ata_scsi_report_supported_opcodes(). With this > reporting option, the specified opcode to check must have a service > action. Use ata_scsi_supported_cmd_has_sa() to check for this and fill the > command reply buffer using the one command format based on the command > support. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-scsi.c | 7 +++++++ > 1 file changed, 7 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES 2026-07-02 6:34 ` [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES Damien Le Moal 2026-07-02 6:46 ` Hannes Reinecke @ 2026-07-02 6:52 ` sashiko-bot 2026-07-02 7:00 ` Damien Le Moal 2026-07-02 6:53 ` Hannes Reinecke 2 siblings, 1 reply; 23+ messages in thread From: sashiko-bot @ 2026-07-02 6:52 UTC (permalink / raw) To: Damien Le Moal; +Cc: dlemoal, cassel, linux-ide Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Reporting options 1 and 2 incorrectly terminate with CHECK CONDITION (INVALID FIELD IN CDB) for valid queries, violating SCSI protocol. - [Medium] Reporting option 3 incorrectly evaluates the REQUESTED SERVICE ACTION field for commands that do not use service actions. -- commit 8b0fcbdec2fbb5690930a99f6f8a7faf9e8bd511 Author: Damien Le Moal <dlemoal@kernel.org> ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES Add support for the reporting options value 2 of the in REPORT SUPPORTED OPERATION CODES command in ata_scsi_report_supported_opcodes(). With this reporting option, the specified opcode to check must have a service action. Use ata_scsi_supported_cmd_has_sa() to check for this and fill the command reply buffer using the one command format based on the command support. > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index 86ac41a7d81ab..e75ed573e234b 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c > @@ -3737,6 +3737,13 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, > return 0; > } > break; > + case 2: > + /* One command format, must have sa. */ > + if (!ata_scsi_supported_cmd_use_sa(cdb[3])) { > + ata_scsi_set_invalid_field(dev, cmd, 3, 0xff); > + return 0; > + } [Severity: Medium] Does this validation check incorrectly abort execution with a SCSI error for option 2 commands that are unknown? According to SPC-5, option 2 queries for unsupported commands (and similarly, option 1 queries for commands with service actions) must return SUPPORT=001b. Instead, this implementation erroneously aborts the command by calling ata_scsi_set_invalid_field(), preventing the code from falling through and correctly populating the response buffer. > + fallthrough; > case 3: > /* One command format */ > sa = get_unaligned_be16(&cdb[4]); [Severity: Medium] If an initiator issues REPORT SUPPORTED OPERATION CODES with option 3 for a standard command that lacks service actions, while providing non-zero data in the requested service action field, wouldn't this unconditional extraction of sa cause a regression? SPC-5 dictates that if the requested opcode does not have service actions, the requested service action field must be ignored. By unconditionally extracting sa here, it will fail to match in ata_scsi_get_supported_cmd() since non-service-action commands expect sa == 0, improperly yielding SUPPORT=001b for a valid, supported command. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260702063412.1892584-1-dlemoal@kernel.org?part=4 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES 2026-07-02 6:52 ` sashiko-bot @ 2026-07-02 7:00 ` Damien Le Moal 2026-07-02 11:41 ` Niklas Cassel 0 siblings, 1 reply; 23+ messages in thread From: Damien Le Moal @ 2026-07-02 7:00 UTC (permalink / raw) To: sashiko-reviews; +Cc: cassel, linux-ide On 7/2/26 15:52, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > - [Medium] Reporting options 1 and 2 incorrectly terminate with CHECK CONDITION (INVALID FIELD IN CDB) for valid queries, violating SCSI protocol. > - [Medium] Reporting option 3 incorrectly evaluates the REQUESTED SERVICE ACTION field for commands that do not use service actions. > -- > > commit 8b0fcbdec2fbb5690930a99f6f8a7faf9e8bd511 > Author: Damien Le Moal <dlemoal@kernel.org> > > ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES > > Add support for the reporting options value 2 of the in REPORT SUPPORTED > OPERATION CODES command in ata_scsi_report_supported_opcodes(). With this > reporting option, the specified opcode to check must have a service > action. Use ata_scsi_supported_cmd_has_sa() to check for this and fill the > command reply buffer using the one command format based on the command > support. > >> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c >> index 86ac41a7d81ab..e75ed573e234b 100644 >> --- a/drivers/ata/libata-scsi.c >> +++ b/drivers/ata/libata-scsi.c >> @@ -3737,6 +3737,13 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, >> return 0; >> } >> break; >> + case 2: >> + /* One command format, must have sa. */ >> + if (!ata_scsi_supported_cmd_use_sa(cdb[3])) { >> + ata_scsi_set_invalid_field(dev, cmd, 3, 0xff); >> + return 0; >> + } > > [Severity: Medium] > Does this validation check incorrectly abort execution with a SCSI error for > option 2 commands that are unknown? > > According to SPC-5, option 2 queries for unsupported commands (and similarly, > option 1 queries for commands with service actions) must return SUPPORT=001b. > Instead, this implementation erroneously aborts the command by calling > ata_scsi_set_invalid_field(), preventing the code from falling through and > correctly populating the response buffer. Sashiko, please reread the SPC specifications. It says: If the REQUESTED OPERATION CODE field specifies an operation code for which the device server does not implement service actions a, then the device server shall terminate the command with CHECK CONDITION status, with the sense key set to ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN CDB. Which is exactly what we are testing here. > >> + fallthrough; >> case 3: >> /* One command format */ >> sa = get_unaligned_be16(&cdb[4]); > > [Severity: Medium] > If an initiator issues REPORT SUPPORTED OPERATION CODES with option 3 for a > standard command that lacks service actions, while providing non-zero data in > the requested service action field, wouldn't this unconditional extraction of > sa cause a regression? No. Service action is not ignored with reporting options 3. > SPC-5 dictates that if the requested opcode does not have service actions, the > requested service action field must be ignored. By unconditionally extracting > sa here, it will fail to match in ata_scsi_get_supported_cmd() since > non-service-action commands expect sa == 0, improperly yielding SUPPORT=001b > for a valid, supported command. No. SPC says: a) the operation code specified by the REQUESTED OPERATION CODE field specifies an operation code for which the device server does not implement service actions, the REQUESTED SERVICE ACTION field is set to 00h, and the command is supported; or ... then the command support data shall indicate that the command is supported (i.e., the SUPPORT field (see table 262) is set to 011b or 101b). Otherwise, the command support data shall indicate that the command is not supported (i.e., the SUPPORT field is set to 001b). Set for commands that do not support service actions, the command must have service action set to 0. -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES 2026-07-02 7:00 ` Damien Le Moal @ 2026-07-02 11:41 ` Niklas Cassel 0 siblings, 0 replies; 23+ messages in thread From: Niklas Cassel @ 2026-07-02 11:41 UTC (permalink / raw) To: Damien Le Moal; +Cc: sashiko-reviews, linux-ide On Thu, Jul 02, 2026 at 04:00:18PM +0900, Damien Le Moal wrote: > > According to SPC-5, option 2 queries for unsupported commands (and similarly, > > option 1 queries for commands with service actions) must return SUPPORT=001b. > > Instead, this implementation erroneously aborts the command by calling > > ata_scsi_set_invalid_field(), preventing the code from falling through and > > correctly populating the response buffer. > > Sashiko, please reread the SPC specifications. It says: > > If the REQUESTED OPERATION CODE field specifies an operation code for > which the device server does not implement service actions a, then the > device server shall terminate the command with CHECK CONDITION > status, with the sense key set to ILLEGAL REQUEST, and the additional > sense code set to INVALID FIELD IN CDB. > > Which is exactly what we are testing here. This is a bit funny :D It would be nice if Sashiko actually listened (like a real AI agent), re-read the spec, and updated posted an updated review comment :) Perhaps try to include the SPC spec excerpt in the commit message for V3, and see if that helps Sashiko parse the spec correctly ? :) Kind regards, Niklas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES 2026-07-02 6:34 ` [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES Damien Le Moal 2026-07-02 6:46 ` Hannes Reinecke 2026-07-02 6:52 ` sashiko-bot @ 2026-07-02 6:53 ` Hannes Reinecke 2 siblings, 0 replies; 23+ messages in thread From: Hannes Reinecke @ 2026-07-02 6:53 UTC (permalink / raw) To: Damien Le Moal, linux-ide, Niklas Cassel On 7/2/26 8:34 AM, Damien Le Moal wrote: > Add support for the reporting options value 2 of the in REPORT SUPPORTED > OPERATION CODES command in ata_scsi_report_supported_opcodes(). With this > reporting option, the specified opcode to check must have a service > action. Use ata_scsi_supported_cmd_has_sa() to check for this and fill the > command reply buffer using the one command format based on the command > support. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-scsi.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index 86ac41a7d81a..e75ed573e234 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c > @@ -3737,6 +3737,13 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, > return 0; > } > break; > + case 2: > + /* One command format, must have sa. */ > + if (!ata_scsi_supported_cmd_use_sa(cdb[3])) { > + ata_scsi_set_invalid_field(dev, cmd, 3, 0xff); > + return 0; > + } > + fallthrough; > case 3: > /* One command format */ > sa = get_unaligned_be16(&cdb[4]); 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 5/5] ata: libata-scsi: support the all command format for reporting supported commands 2026-07-02 6:34 [PATCH v2 0/5] Improve ata_scsi_report_supported_opcodes() Damien Le Moal ` (3 preceding siblings ...) 2026-07-02 6:34 ` [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES Damien Le Moal @ 2026-07-02 6:34 ` Damien Le Moal 2026-07-02 6:54 ` Hannes Reinecke 4 siblings, 1 reply; 23+ messages in thread From: Damien Le Moal @ 2026-07-02 6:34 UTC (permalink / raw) To: linux-ide, Niklas Cassel Add support to ata_scsi_report_supported_opcodes() for the all command format indicated with a reporting option of 0. The function ata_scsi_report_all_supported_opcodes() is introduced to implement this support. This function operates by testing all commands of the ata_supported_cmds array and testing them using ata_scsi_cmd_is_supported(), filling rbuf as it loops through all the commands that libata can emulate or translate. With this change, sg_opcodes /dev/sdX is able to list all commands supported by libata and a device. An example below is shown for a SATA disk also supporting CDL: # sg_opcodes /dev/sda ATA WDC WUH722626AL WZ41 Peripheral device type: disk Opcode Service CDB RWCDLP, Name (hex) action(h) size CDLP ----------------------------------------------- 00 6 0,0 Test Unit Ready 01 6 0,0 Rezero Unit 03 6 0,0 Request Sense 08 6 0,0 Read(6) 0a 6 0,0 Write(6) 0b 6 0,0 Seek(6) 12 6 0,0 Inquiry 15 6 0,0 Mode select(6) 1a 6 0,0 Mode sense(6) 1b 6 0,0 Start stop unit 1d 6 0,0 Send diagnostic 25 10 0,0 Read capacity(10) 28 10 0,0 Read(10) 2a 10 0,0 Write(10) 2b 10 0,0 Seek(10) 2f 10 0,0 Verify(10) 35 10 0,0 Synchronize cache(10) 55 10 0,0 Mode select(10) 5a 10 0,0 Mode sense(10) 7f 1ff0 32 0,0 ATA pass-through(32) 85 16 0,0 ATA pass-through(16) 88 16 1,1 Read(16) 8a 16 1,2 Write(16) 8f 16 0,0 Verify(16) 91 16 0,0 Synchronize cache(16) 93 16 0,0 Write same(16) 9e 10 16 0,0 Read capacity(16) a0 12 0,0 Report luns a1 12 0,0 ATA pass-through(12) a3 c 12 0,0 Report supported operation codes Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/ata/libata-scsi.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index e75ed573e234..b14fd30c0e38 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3721,6 +3721,42 @@ static bool ata_scsi_cmd_is_supported(struct ata_device *dev, u8 op, u16 sa, return true; } +static unsigned int +ata_scsi_report_all_supported_opcodes(struct ata_device *dev, u8 *rbuf) +{ + struct ata_scsi_cmd_support sup; + const struct ata_scsi_cmd *cmd; + unsigned int len = 4; + u8 *buf = &rbuf[len]; + int i; + + for (i = 0; i < ARRAY_SIZE(ata_supported_cmds); i++) { + if (len > ATA_SCSI_RBUF_SIZE - 8) + break; + + cmd = &ata_supported_cmds[i]; + + /* All command format */ + if (!ata_scsi_cmd_is_supported(dev, cmd->op, cmd->sa, &sup)) + continue; + + buf[0] = cmd->op; + put_unaligned_be16(cmd->sa, &buf[2]); + buf[5] = (sup.rwcdlp << 6) | (sup.cdlp << 2); + if (cmd->sa_valid) + buf[5] |= 0x01; + put_unaligned_be16(cmd->cdb_len, &buf[6]); + + /* CTDP == 0 */ + len += 8; + buf += 8; + } + + put_unaligned_be32(len - 4, &rbuf[0]); + + return len; +} + static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, struct scsi_cmnd *cmd, u8 *rbuf) @@ -3730,6 +3766,9 @@ static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev, u16 sa = 0; switch (cdb[2]) { + case 0: + /* All command format */ + return ata_scsi_report_all_supported_opcodes(dev, rbuf); case 1: /* One command format with command support data, ignore sa. */ if (ata_scsi_supported_cmd_use_sa(cdb[3])) { -- 2.54.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 5/5] ata: libata-scsi: support the all command format for reporting supported commands 2026-07-02 6:34 ` [PATCH v2 5/5] ata: libata-scsi: support the all command format for reporting supported commands Damien Le Moal @ 2026-07-02 6:54 ` Hannes Reinecke 0 siblings, 0 replies; 23+ messages in thread From: Hannes Reinecke @ 2026-07-02 6:54 UTC (permalink / raw) To: Damien Le Moal, linux-ide, Niklas Cassel On 7/2/26 8:34 AM, Damien Le Moal wrote: > Add support to ata_scsi_report_supported_opcodes() for the all command > format indicated with a reporting option of 0. The function > ata_scsi_report_all_supported_opcodes() is introduced to implement this > support. This function operates by testing all commands of the > ata_supported_cmds array and testing them using > ata_scsi_cmd_is_supported(), filling rbuf as it loops through all the > commands that libata can emulate or translate. > > With this change, sg_opcodes /dev/sdX is able to list all commands > supported by libata and a device. An example below is shown for a SATA > disk also supporting CDL: > > # sg_opcodes /dev/sda > ATA WDC WUH722626AL WZ41 > Peripheral device type: disk > > Opcode Service CDB RWCDLP, Name > (hex) action(h) size CDLP > ----------------------------------------------- > 00 6 0,0 Test Unit Ready > 01 6 0,0 Rezero Unit > 03 6 0,0 Request Sense > 08 6 0,0 Read(6) > 0a 6 0,0 Write(6) > 0b 6 0,0 Seek(6) > 12 6 0,0 Inquiry > 15 6 0,0 Mode select(6) > 1a 6 0,0 Mode sense(6) > 1b 6 0,0 Start stop unit > 1d 6 0,0 Send diagnostic > 25 10 0,0 Read capacity(10) > 28 10 0,0 Read(10) > 2a 10 0,0 Write(10) > 2b 10 0,0 Seek(10) > 2f 10 0,0 Verify(10) > 35 10 0,0 Synchronize cache(10) > 55 10 0,0 Mode select(10) > 5a 10 0,0 Mode sense(10) > 7f 1ff0 32 0,0 ATA pass-through(32) > 85 16 0,0 ATA pass-through(16) > 88 16 1,1 Read(16) > 8a 16 1,2 Write(16) > 8f 16 0,0 Verify(16) > 91 16 0,0 Synchronize cache(16) > 93 16 0,0 Write same(16) > 9e 10 16 0,0 Read capacity(16) > a0 12 0,0 Report luns > a1 12 0,0 ATA pass-through(12) > a3 c 12 0,0 Report supported operation codes > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-scsi.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2026-07-02 14:53 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-02 6:34 [PATCH v2 0/5] Improve ata_scsi_report_supported_opcodes() Damien Le Moal 2026-07-02 6:34 ` [PATCH v2 1/5] ata: libata: rename ata_dev_is_zac() Damien Le Moal 2026-07-02 6:38 ` Hannes Reinecke 2026-07-02 7:16 ` sashiko-bot 2026-07-02 6:34 ` [PATCH v2 2/5] ata: libata-scsi: refactor ata_scsi_report_supported_opcodes() Damien Le Moal 2026-07-02 6:44 ` Hannes Reinecke 2026-07-02 7:01 ` Damien Le Moal 2026-07-02 6:53 ` sashiko-bot 2026-07-02 13:25 ` Niklas Cassel 2026-07-02 14:36 ` Niklas Cassel 2026-07-02 14:48 ` Christoph Hellwig 2026-07-02 14:53 ` Niklas Cassel 2026-07-02 6:34 ` [PATCH v2 3/5] ata: libata-scsi: improve service action support in ata_scsi_report_supported_opcodes() Damien Le Moal 2026-07-02 6:45 ` Hannes Reinecke 2026-07-02 6:47 ` sashiko-bot 2026-07-02 6:34 ` [PATCH v2 4/5] ata: libata-scsi: support reporting options 2 in REPORT SUPPORTED OPERATION CODES Damien Le Moal 2026-07-02 6:46 ` Hannes Reinecke 2026-07-02 6:52 ` sashiko-bot 2026-07-02 7:00 ` Damien Le Moal 2026-07-02 11:41 ` Niklas Cassel 2026-07-02 6:53 ` Hannes Reinecke 2026-07-02 6:34 ` [PATCH v2 5/5] ata: libata-scsi: support the all command format for reporting supported commands Damien Le Moal 2026-07-02 6:54 ` Hannes Reinecke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox