Skip to contentSkip to conversation

ata: libata-eh: Do not use ATAPI DMA for a device limited to PIO mode

PATCH Applied Niklas Cassel 2 participants 3 messages Feb 21, 2025 libata

lore
Share & tools
lore

Niklas Cassel

author maintainer
#0 lore raw

PATCH 1/1 ata: libata-eh: Do not use ATAPI DMA for a device limited to PIO mode

atapi_eh_request_sense() currently uses ATAPI DMA if the SATA controller has ATA_FLAG_PIO_DMA (PIO cmds via DMA) set.

However, ATA_FLAG_PIO_DMA is a flag that can be set by a low-level driver on a port at initialization time, before any devices are scanned.

If a controller detects a connected device that only supports PIO, we set the flag ATA_DFLAG_PIO.

Modify atapi_eh_request_sense() to not use ATAPI DMA if the connected device only supports PIO.

Reported-by Philip Pemberton <[email protected]>
Closes https://lore.kernel.org/linux-ide/[email protected]/ open here
Signed-off-by Niklas Cassel <[email protected]>
---
 drivers/ata/libata-eh.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/ata/libata-eh.c +9 -2
index 3b303d4ae37a..16cd676eae1f 100644
@@ -1542,8 +1542,15 @@ unsigned int atapi_eh_request_sense(struct ata_device *dev,
 	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	tf.command = ATA_CMD_PACKET;
 
-	/* is it pointless to prefer PIO for "safety reasons"? */
-	if (ap->flags & ATA_FLAG_PIO_DMA) {
+	/*
+	 * Do not use DMA if the connected device only supports PIO, even if the
+	 * port prefers PIO commands via DMA.
+	 *
+	 * Ideally, we should call atapi_check_dma() to check if it is safe for
+	 * the LLD to use DMA for REQUEST_SENSE, but we don't have a qc.
+	 * Since we can't check the command, perhaps we should only use pio?
+	 */
+	if ((ap->flags & ATA_FLAG_PIO_DMA) && !(dev->flags & ATA_DFLAG_PIO)) {
 		tf.protocol = ATAPI_PROT_DMA;
 		tf.feature |= ATAPI_PKT_DMA;
 	} else {
Signature · 1 line
2.48.1
maintainer Reviewed-by on v1 1/1
#1 lore raw
Quoted 9 lines · Niklas Cassel

atapi_eh_request_sense() currently uses ATAPI DMA if the SATA controller has ATA_FLAG_PIO_DMA (PIO cmds via DMA) set.

However, ATA_FLAG_PIO_DMA is a flag that can be set by a low-level driver on a port at initialization time, before any devices are scanned.

If a controller detects a connected device that only supports PIO, we set the flag ATA_DFLAG_PIO.

Quoted 6 lines

Modify atapi_eh_request_sense() to not use ATAPI DMA if the connected device only supports PIO.

Reported-by Philip Pemberton <[email protected]>
Closes https://lore.kernel.org/linux-ide/[email protected]/ open here
Signed-off-by Niklas Cassel <[email protected]>

Looks good.

Reviewed-by Damien Le Moal <[email protected]>
Signature · 2 lines
Damien Le Moal
Western Digital Research
author maintainer Applied
#2 lore raw
In reply to Niklas Cassel
Quoted 10 lines · Niklas Cassel

atapi_eh_request_sense() currently uses ATAPI DMA if the SATA controller has ATA_FLAG_PIO_DMA (PIO cmds via DMA) set.

However, ATA_FLAG_PIO_DMA is a flag that can be set by a low-level driver on a port at initialization time, before any devices are scanned.

If a controller detects a connected device that only supports PIO, we set the flag ATA_DFLAG_PIO.

[...]

Applied to libata/linux.git (for-6.15), thanks!

[1/1] ata: libata-eh: Do not use ATAPI DMA for a device limited to PIO mode

      https://git.kernel.org/libata/linux/c/91ec84f8

Kind regards,
Niklas