* [PATCH 1/2] ata: libata-scsi: fix DSM TRIM for sector sizes larger than 2048 bytes
[not found] <20260701224638.1835123-4-cassel@kernel.org>
@ 2026-07-01 22:46 ` Niklas Cassel
2026-07-02 7:00 ` Hannes Reinecke
0 siblings, 1 reply; 2+ messages in thread
From: Niklas Cassel @ 2026-07-01 22:46 UTC (permalink / raw)
To: Damien Le Moal, Niklas Cassel, Shaun Tancheff, Tejun Heo
Cc: 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.
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.54.0
^ permalink raw reply related [flat|nested] 2+ messages in thread