* [PATCH v2 1/2] ata: libata-scsi: fix DSM TRIM for sector sizes larger than 2048 bytes
2026-07-02 10:04 [PATCH v2 0/2] ata: DSM TRIM fix and improvements Niklas Cassel
@ 2026-07-02 10:04 ` Niklas Cassel
2026-07-02 10:04 ` [PATCH v2 2/2] ata: libata-scsi: scale DSM TRIM payload by MAX PAGES PER DSM COMMAND Niklas Cassel
1 sibling, 0 replies; 5+ messages in thread
From: Niklas Cassel @ 2026-07-02 10:04 UTC (permalink / raw)
To: Damien Le Moal, Niklas Cassel, Shaun Tancheff, Tejun Heo
Cc: Hannes Reinecke, stable, linux-ide
ata_scsi_write_same_xlat() translates a SCSI WRITE SAME command with the
UNMAP bit set into an ATA DATA SET MANAGEMENT TRIM command. The TRIM
descriptor is built by ata_format_dsm_trim_descr() into the 2048-byte
ata_scsi_rbuf staging buffer, and the number of bytes copied is compared
against the logical sector size by the caller:
size = ata_format_dsm_trim_descr(scmd, trmax, block, n_block);
if (size != len) /* len == sdp->sector_size */
goto invalid_param_len;
ata_format_dsm_trim_descr() clamps the copy length to ATA_SCSI_RBUF_SIZE
(2048). On a device whose logical sector size exceeds that (e.g. a 4Kn
device, where sector_size == 4096) the function can never return more than
2048, while the caller expects it to return sector_size. The comparison
therefore always fails, so every TRIM is rejected with "Parameter list
length error" and WARN_ON() splats on each attempt. TRIM / discard is
thus completely broken on such devices.
The descriptor was incorrectly sized from the logical sector size. A DSM
TRIM payload is a list of 512-byte pages, each holding up to
ATA_MAX_TRIM_RNUM (64) LBA Range Entries, and is independent of the logical
sector size. The Block Limits VPD page already advertises a single such
page as the maximum WRITE SAME length (65535 * ATA_MAX_TRIM_RNUM logical
blocks), so the block layer never sends a request that needs more than one
page.
Emit exactly one 512-byte page, independent of the logical sector size,
and transfer only that page (COUNT == 1). For a 512-byte-sector device
this is unchanged; devices with larger logical sectors now work instead of
failing every TRIM.
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Fixes: ef2d7392c4ec ("libata: SCT Write Same / DSM Trim")
Cc: stable@vger.kernel.org
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
drivers/ata/libata-scsi.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index d54ec1631e9a..429b03a08071 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3447,17 +3447,13 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
u64 sector, u32 count)
{
- struct scsi_device *sdp = cmd->device;
- size_t len = sdp->sector_size;
+ size_t len = ATA_SECT_SIZE;
size_t r;
__le64 *buf;
u32 i = 0;
unsigned long flags;
- WARN_ON(len > ATA_SCSI_RBUF_SIZE);
-
- if (len > ATA_SCSI_RBUF_SIZE)
- len = ATA_SCSI_RBUF_SIZE;
+ BUILD_BUG_ON(ATA_SECT_SIZE > ATA_SCSI_RBUF_SIZE);
spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
buf = ((void *)ata_scsi_rbuf);
@@ -3492,13 +3488,11 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
{
struct ata_taskfile *tf = &qc->tf;
struct scsi_cmnd *scmd = qc->scsicmd;
- struct scsi_device *sdp = scmd->device;
- size_t len = sdp->sector_size;
struct ata_device *dev = qc->dev;
const u8 *cdb = scmd->cmnd;
u64 block;
u32 n_block;
- const u32 trmax = len >> 3;
+ const u32 trmax = ATA_MAX_TRIM_RNUM;
u32 size;
u16 fp;
u8 bp = 0xff;
@@ -3542,13 +3536,13 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
goto invalid_param_len;
/*
- * size must match sector size in bytes
- * For DATA SET MANAGEMENT TRIM in ACS-2 nsect (aka count)
- * is defined as number of 512 byte blocks to be transferred.
+ * The TRIM descriptor is a single 512-byte page, which is the maximum
+ * WRITE SAME length advertised in the Block Limits VPD page. For DATA
+ * SET MANAGEMENT TRIM the COUNT field (aka nsect) is the number of
+ * 512-byte blocks to be transferred.
*/
-
size = ata_format_dsm_trim_descr(scmd, trmax, block, n_block);
- if (size != len)
+ if (size != ATA_SECT_SIZE)
goto invalid_param_len;
if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) {
@@ -3574,6 +3568,12 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
ATA_TFLAG_WRITE;
ata_qc_set_pc_nbytes(qc);
+ /*
+ * The DSM TRIM payload is a single 512-byte page, which may be smaller
+ * than the WRITE SAME data-out buffer (one logical block); only
+ * transfer that page so the length matches the COUNT field.
+ */
+ qc->nbytes = size;
return 0;
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/2] ata: libata-scsi: scale DSM TRIM payload by MAX PAGES PER DSM COMMAND
2026-07-02 10:04 [PATCH v2 0/2] ata: DSM TRIM fix and improvements Niklas Cassel
2026-07-02 10:04 ` [PATCH v2 1/2] ata: libata-scsi: fix DSM TRIM for sector sizes larger than 2048 bytes Niklas Cassel
@ 2026-07-02 10:04 ` Niklas Cassel
2026-07-02 10:20 ` sashiko-bot
1 sibling, 1 reply; 5+ messages in thread
From: Niklas Cassel @ 2026-07-02 10:04 UTC (permalink / raw)
To: Damien Le Moal, Niklas Cassel; +Cc: Shaun Tancheff, Hannes Reinecke, linux-ide
DSM TRIM currently always emits a single 512-byte page of LBA Range Entries
(ATA_MAX_TRIM_RNUM == 64 ranges), regardless of how many pages the device
can accept in one DATA SET MANAGEMENT command.
The maximum is reported by MAX PAGES PER DSM COMMAND (IDENTIFY DEVICE word
105). Honour it: size the TRIM descriptor as a whole number of 512-byte
pages, bounded by that limit and by the logical sector size (the WRITE SAME
data-out buffer is a single logical block). Build and transfer only as
many pages as the request needs, and set the DSM COUNT field, qc->nbytes
and the maximum WRITE SAME length in the Block Limits VPD page accordingly.
Build the descriptor straight into the WRITE SAME data-out buffer using an
atomic sg_miter mapping, instead of staging it in the shared ata_scsi_rbuf
and copying it out. This removes the global ata_scsi_rbuf_lock and a
memcpy from the TRIM path.
While commit 9379e6b8e0f9 ("libata: Safely overwrite attached page in WRITE
SAME xlat") replaced direct access to the data-out buffer with an
intermediate step that writes the entries in the ata_scsi_rbuf buffer, this
solution writes to the data-out buffer using sg_miter, which maps each
segment with kmap_atomic (SG_MITER_ATOMIC), so it's highmem- and
multi-segment-safe, and it's usable from the non-sleeping
command-submission path (unlike the page_address() access that
ata_scsi_rbuf originally replaced).
A 512-byte-sector device still uses a single page, so its behaviour is
unchanged.
Add ata_id_dsm_max_pages() to read IDENTIFY DEVICE word 105.
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
drivers/ata/libata-scsi.c | 135 ++++++++++++++++++++++++++------------
include/linux/ata.h | 13 ++++
2 files changed, 107 insertions(+), 41 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 429b03a08071..82342351b1c9 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2198,6 +2198,39 @@ static unsigned int ata_scsiop_inq_89(struct ata_device *dev,
return get_unaligned_be16(&rbuf[2]) + 4;
}
+/**
+ * ata_dsm_trim_pages - maximum DSM TRIM payload for a device, in 512-byte pages
+ * @dev: ATA device the DATA SET MANAGEMENT TRIM command will be sent to
+ *
+ * A DATA SET MANAGEMENT TRIM payload is a list of 512-byte pages, each holding
+ * up to ATA_MAX_TRIM_RNUM (64) LBA Range Entries; the format is page-based and
+ * unrelated to the logical sector size.
+ *
+ * The logical sector size still bounds it, though: the descriptor is written
+ * directly into the WRITE SAME data-out buffer, which sd sizes to a single
+ * logical block, so it can hold at most sector_size / 512 pages.
+ *
+ * Return: the maximum number of 512-byte pages a single translated WRITE SAME
+ * command may send to @dev (never less than one), that is the smaller of:
+ * - MAX PAGES PER DSM COMMAND (IDENTIFY DEVICE word 105), when the device
+ * reports a non-zero limit; and
+ * - the logical sector size expressed in 512-byte pages (see above).
+ */
+static unsigned int ata_dsm_trim_pages(struct ata_device *dev)
+{
+ unsigned int sector_size = ata_id_logical_sector_size(dev->id);
+ unsigned int max_pages = ata_id_dsm_max_pages(dev->id);
+ unsigned int pages = sector_size / ATA_SECT_SIZE;
+
+ /* If the device does not specify a limit, assume only a single page. */
+ if (!max_pages)
+ max_pages = 1;
+
+ pages = min(pages, max_pages);
+
+ return pages;
+}
+
/**
* ata_scsiop_inq_b0 - Simulate INQUIRY VPD page B0, Block Limits
* @dev: Target device.
@@ -2237,7 +2270,8 @@ static unsigned int ata_scsiop_inq_b0(struct ata_device *dev,
* with the unmap bit set.
*/
if (ata_id_has_trim(dev->id)) {
- u64 max_blocks = 65535 * ATA_MAX_TRIM_RNUM;
+ unsigned int max_pages = ata_dsm_trim_pages(dev);
+ u64 max_blocks = max_pages * ATA_MAX_TRIM_RNUM * (u64)U16_MAX;
if (dev->quirks & ATA_QUIRK_MAX_TRIM_128M)
max_blocks = 128 << (20 - SECTOR_SHIFT);
@@ -3426,14 +3460,14 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
/**
* ata_format_dsm_trim_descr() - SATL Write Same to DSM Trim
* @cmd: SCSI command being translated
- * @trmax: Maximum number of entries that will fit in sector_size bytes.
+ * @size: DSM TRIM payload size in bytes (a multiple of 512)
* @sector: Starting sector
* @count: Total Range of request in logical sectors
*
* Rewrite the WRITE SAME descriptor to be a DSM TRIM little-endian formatted
* descriptor.
*
- * Upto 64 entries of the format:
+ * The payload is a list of @size / 8 entries of the format:
* 63:48 Range Length
* 47:0 LBA
*
@@ -3442,35 +3476,45 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
*
* NOTE: this is the same format as ADD LBA(S) TO NV CACHE PINNED SET
*
- * Return: Number of bytes copied into sglist.
+ * The descriptor is written straight into the WRITE SAME data-out buffer;
+ * ata_dsm_trim_pages() guarantees @size does not exceed that buffer (one
+ * logical block). An atomic sg_miter mapping is used so this works from the
+ * command submission path and, unlike page_address(), copes with a high
+ * memory payload.
+ *
+ * Return: Number of bytes written into the data-out buffer.
*/
-static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, u32 trmax,
+static size_t ata_format_dsm_trim_descr(struct scsi_cmnd *cmd, size_t size,
u64 sector, u32 count)
{
- size_t len = ATA_SECT_SIZE;
- size_t r;
- __le64 *buf;
- u32 i = 0;
- unsigned long flags;
+ struct sg_mapping_iter miter;
+ size_t offset = 0;
- BUILD_BUG_ON(ATA_SECT_SIZE > ATA_SCSI_RBUF_SIZE);
+ sg_miter_start(&miter, scsi_sglist(cmd), scsi_sg_count(cmd),
+ SG_MITER_TO_SG | SG_MITER_ATOMIC);
+ while (offset < size && sg_miter_next(&miter)) {
+ __le64 *buf = miter.addr;
+ size_t chunk = min_t(size_t, miter.length, size - offset);
+ unsigned int n = chunk / sizeof(__le64);
+ unsigned int i;
- spin_lock_irqsave(&ata_scsi_rbuf_lock, flags);
- buf = ((void *)ata_scsi_rbuf);
- memset(buf, 0, len);
- while (i < trmax) {
- u64 entry = sector |
- ((u64)(count > 0xffff ? 0xffff : count) << 48);
- buf[i++] = __cpu_to_le64(entry);
- if (count <= 0xffff)
- break;
- count -= 0xffff;
- sector += 0xffff;
+ for (i = 0; i < n; i++) {
+ u64 entry = 0;
+
+ if (count) {
+ u32 rlen = min_t(u32, count, 0xffff);
+
+ entry = sector | ((u64)rlen << 48);
+ sector += rlen;
+ count -= rlen;
+ }
+ buf[i] = cpu_to_le64(entry);
+ }
+ offset += n * sizeof(__le64);
}
- r = sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd), buf, len);
- spin_unlock_irqrestore(&ata_scsi_rbuf_lock, flags);
+ sg_miter_stop(&miter);
- return r;
+ return offset;
}
/**
@@ -3490,10 +3534,11 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
struct scsi_cmnd *scmd = qc->scsicmd;
struct ata_device *dev = qc->dev;
const u8 *cdb = scmd->cmnd;
+ unsigned int max_pages = ata_dsm_trim_pages(dev);
+ unsigned int n_pages;
+ size_t size;
u64 block;
u32 n_block;
- const u32 trmax = ATA_MAX_TRIM_RNUM;
- u32 size;
u16 fp;
u8 bp = 0xff;
u8 unmap = cdb[1] & 0x8;
@@ -3523,7 +3568,7 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
goto invalid_fld;
}
/* If the request is too large the cmd is invalid */
- if (n_block > 0xffff * trmax) {
+ if (n_block > max_pages * ATA_MAX_TRIM_RNUM * (u64)U16_MAX) {
fp = 2;
goto invalid_fld;
}
@@ -3536,31 +3581,39 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
goto invalid_param_len;
/*
- * The TRIM descriptor is a single 512-byte page, which is the maximum
- * WRITE SAME length advertised in the Block Limits VPD page. For DATA
- * SET MANAGEMENT TRIM the COUNT field (aka nsect) is the number of
- * 512-byte blocks to be transferred.
+ * The DATA SET MANAGEMENT TRIM payload is a whole number of 512-byte
+ * pages (each holding up to ATA_MAX_TRIM_RNUM LBA Range Entries),
+ * independent of the logical sector size. Only use as many pages as
+ * are needed to describe the request, capped at max_pages.
*/
- size = ata_format_dsm_trim_descr(scmd, trmax, block, n_block);
- if (size != ATA_SECT_SIZE)
+ n_pages = DIV_ROUND_UP(DIV_ROUND_UP(n_block, U16_MAX),
+ ATA_MAX_TRIM_RNUM);
+ n_pages = clamp(n_pages, 1U, max_pages);
+ size = (size_t)n_pages * ATA_SECT_SIZE;
+
+ if (ata_format_dsm_trim_descr(scmd, size, block, n_block) != size)
goto invalid_param_len;
+ /*
+ * For DATA SET MANAGEMENT TRIM the COUNT field (aka nsect) is the
+ * number of 512-byte pages to be transferred.
+ */
if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) {
/* Newer devices support queued TRIM commands */
tf->protocol = ATA_PROT_NCQ;
tf->command = ATA_CMD_FPDMA_SEND;
tf->hob_nsect = ATA_SUBCMD_FPDMA_SEND_DSM & 0x1f;
tf->nsect = qc->hw_tag << 3;
- tf->hob_feature = (size / 512) >> 8;
- tf->feature = size / 512;
+ tf->hob_feature = n_pages >> 8;
+ tf->feature = n_pages;
tf->auxiliary = 1;
} else {
tf->protocol = ATA_PROT_DMA;
tf->hob_feature = 0;
tf->feature = ATA_DSM_TRIM;
- tf->hob_nsect = (size / 512) >> 8;
- tf->nsect = size / 512;
+ tf->hob_nsect = n_pages >> 8;
+ tf->nsect = n_pages;
tf->command = ATA_CMD_DSM;
}
@@ -3569,9 +3622,9 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
ata_qc_set_pc_nbytes(qc);
/*
- * The DSM TRIM payload is a single 512-byte page, which may be smaller
- * than the WRITE SAME data-out buffer (one logical block); only
- * transfer that page so the length matches the COUNT field.
+ * The DSM TRIM payload (size) may be smaller than the WRITE SAME
+ * data-out buffer (one logical block); only transfer the pages that
+ * were actually built so the transfer length matches the COUNT field.
*/
qc->nbytes = size;
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 8fd48bcb2a46..ac5616a9668b 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -75,6 +75,7 @@ enum {
ATA_ID_HW_CONFIG = 93,
ATA_ID_SPG = 98,
ATA_ID_LBA_CAPACITY_2 = 100,
+ ATA_ID_MAX_PAGES_PER_DSM = 105,
ATA_ID_SECTOR_SIZE = 106,
ATA_ID_WWN = 108,
ATA_ID_LOGICAL_SECTOR_SIZE = 117, /* and 118 */
@@ -928,6 +929,18 @@ static inline bool ata_id_has_trim(const u16 *id)
return false;
}
+static inline u16 ata_id_dsm_max_pages(const u16 *id)
+{
+ /*
+ * IDENTIFY DEVICE word 105: MAX PAGES PER DSM COMMAND. Maximum number
+ * of 512-byte pages of LBA Range Entries the device accepts in a
+ * single DATA SET MANAGEMENT command. Zero means the device does not
+ * specify a limit. The field is reserved unless TRIM is supported, so
+ * callers must gate on ata_id_has_trim().
+ */
+ return id[ATA_ID_MAX_PAGES_PER_DSM];
+}
+
static inline bool ata_id_has_zero_after_trim(const u16 *id)
{
/* DSM supported, deterministic read, and read zero after trim set */
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread