All of lore.kernel.org
 help / color / mirror / Atom feed
From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
To: linux-ide@vger.kernel.org, linux-block@vger.kernel.org,
	Jens Axboe <axboe@kernel.dk>
Cc: "Maciej S . Szmigiero" <mail@maciej.szmigiero.name>,
	Hannes Reinecke <hare@suse.de>
Subject: [PATCH v4 5/7] ata: libata: Fix FUA handling in ata_build_rw_tf()
Date: Mon, 31 Oct 2022 11:26:40 +0900	[thread overview]
Message-ID: <20221031022642.352794-6-damien.lemoal@opensource.wdc.com> (raw)
In-Reply-To: <20221031022642.352794-1-damien.lemoal@opensource.wdc.com>

If a user issues a write command with the FUA bit set for a device with
NCQ support disabled (that is, the device queue depth was set to 1), the
LBA 48 command WRITE DMA FUA EXT must be used. However,
ata_build_rw_tf() ignores this and first tests if LBA 28 can be used
based on the write command sector and number of blocks. That is, for
small FUA writes at low LBAs, ata_rwcmd_protocol() will cause the write
to fail.

Fix this by preventing the use of LBA 28 for any FUA write request.

Given that the WRITE MULTI FUA EXT command is marked as obsolete iin the
ATA specification since ACS-3 (published in 2013), remove the
ATA_CMD_WRITE_MULTI_FUA_EXT command from the ata_rw_cmds array.

Finally, since the block layer should never issue a FUA read
request, warn in ata_build_rw_tf() if we see such request.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/ata/libata-core.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 30adae16efff..83bea8591b08 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -552,7 +552,7 @@ static const u8 ata_rw_cmds[] = {
 	0,
 	0,
 	0,
-	ATA_CMD_WRITE_MULTI_FUA_EXT,
+	0,
 	/* pio */
 	ATA_CMD_PIO_READ,
 	ATA_CMD_PIO_WRITE,
@@ -693,6 +693,10 @@ int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block,
 	tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	tf->flags |= tf_flags;
 
+	/* We should never get a FUA read */
+	WARN_ON_ONCE((tf->flags & ATA_TFLAG_FUA) &&
+		     !(tf->flags & ATA_TFLAG_WRITE));
+
 	if (ata_ncq_enabled(dev)) {
 		/* yay, NCQ */
 		if (!lba_48_ok(block, n_block))
@@ -727,7 +731,8 @@ int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block,
 	} else if (dev->flags & ATA_DFLAG_LBA) {
 		tf->flags |= ATA_TFLAG_LBA;
 
-		if (lba_28_ok(block, n_block)) {
+		/* We need LBA48 for FUA writes */
+		if (!(tf->flags & ATA_TFLAG_FUA) && lba_28_ok(block, n_block)) {
 			/* use LBA28 */
 			tf->device |= (block >> 24) & 0xf;
 		} else if (lba_48_ok(block, n_block)) {
@@ -742,9 +747,10 @@ int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block,
 			tf->hob_lbah = (block >> 40) & 0xff;
 			tf->hob_lbam = (block >> 32) & 0xff;
 			tf->hob_lbal = (block >> 24) & 0xff;
-		} else
+		} else {
 			/* request too large even for LBA48 */
 			return -ERANGE;
+		}
 
 		if (unlikely(!ata_set_rwcmd_protocol(dev, tf)))
 			return -EINVAL;
-- 
2.38.1


  parent reply	other threads:[~2022-10-31  2:27 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-31  2:26 [PATCH v4 0/7] Improve libata support for FUA Damien Le Moal
2022-10-31  2:26 ` [PATCH v4 1/7] block: Prevent the use of REQ_FUA with read operations Damien Le Moal
2022-11-01 15:09   ` Christoph Hellwig
2022-11-01 22:05     ` Damien Le Moal
2022-11-02  7:09       ` Christoph Hellwig
2022-11-02  7:27         ` Damien Le Moal
2022-11-02 15:17         ` Jens Axboe
2022-11-02 22:28           ` Damien Le Moal
2022-11-03  7:49           ` Christoph Hellwig
2022-11-05 15:32             ` Jens Axboe
2022-10-31  2:26 ` [PATCH v4 2/7] ata: libata: Introduce ata_ncq_supported() Damien Le Moal
2022-10-31  2:26 ` [PATCH v4 3/7] ata: libata: Rename and cleanup ata_rwcmd_protocol() Damien Le Moal
2022-10-31  2:26 ` [PATCH v4 4/7] ata: libata: cleanup fua support detection Damien Le Moal
2022-10-31  2:26 ` Damien Le Moal [this message]
2022-10-31  2:26 ` [PATCH v4 6/7] ata: libata: blacklist FUA support for known buggy drives Damien Le Moal
2022-10-31  2:26 ` [PATCH v4 7/7] ata: libata: Enable fua support by default Damien Le Moal
  -- strict thread matches above, loose matches on Subject: below --
2022-10-31  1:53 [PATCH v4 0/7] Damien Le Moal
2022-10-31  1:53 ` [PATCH v4 5/7] ata: libata: Fix FUA handling in ata_build_rw_tf() Damien Le Moal
2022-11-02 14:25   ` Hannes Reinecke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221031022642.352794-6-damien.lemoal@opensource.wdc.com \
    --to=damien.lemoal@opensource.wdc.com \
    --cc=axboe@kernel.dk \
    --cc=hare@suse.de \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=mail@maciej.szmigiero.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.