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

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

[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.

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

Reported-by: Philip Pemberton <lists@philpem.me.uk>
Closes: https://lore.kernel.org/linux-ide/c6722ee8-5e21-4169-af59-cbbae9edc02f@philpem.me.uk/
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-eh.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 3b303d4ae37a..16cd676eae1f 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -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 {
-- 
2.48.1

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

[Damien Le Moal]

On 2/21/25 10:54, Niklas Cassel wrote:
> 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 <lists@philpem.me.uk>
> Closes: https://lore.kernel.org/linux-ide/c6722ee8-5e21-4169-af59-cbbae9edc02f@philpem.me.uk/
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Looks good.

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

-- 
Damien Le Moal
Western Digital Research

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

[Niklas Cassel]

On Fri, 21 Feb 2025 02:54:23 +0100, Niklas Cassel wrote:
> 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