Re: [PATCH v3 4/6] ata: libata: Fix FUA handling in ata_build_rw_tf()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27.10.2022 09:50, Damien Le Moal wrote:
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 test if LBA 28 can be used.
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.
While at it, also early test if the request is a FUA read and fail these
requests for the NCQ-disabled case instead of relying on
ata_rwcmd_protocol() returning an error.

Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
---
  drivers/ata/libata-core.c | 17 +++++++++++++++--
  1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 81b20ffb1554..fea06f41f371 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -725,9 +725,21 @@ int ata_build_rw_tf(struct ata_queued_cmd *qc, u64 block, u32 n_block,
  		    class == IOPRIO_CLASS_RT)
  			tf->hob_nsect |= ATA_PRIO_HIGH << ATA_SHIFT_PRIO;
  	} else if (dev->flags & ATA_DFLAG_LBA) {
+		bool lba28_ok;
+
+		if (tf->flags & ATA_TFLAG_FUA) {
+			/* FUA reads are not defined */
+			if (!(tf->flags & ATA_TFLAG_WRITE))
+				return -EINVAL;
+			/* We need LBA48 / WRITE DMA FUA EXT for FUA writes */
+			lba28_ok = false;
+		} else {
+			lba28_ok = lba_28_ok(block, n_block);
+		}
+

If we are supporting FUA even in the non-NCQ case (at least for writes)
we do *not* need to limit the FUA support to NCQ-supporting drives in
ata_dev_config_fua() anymore.

Having support for ATA_DFLAG_LBA48 and ata_id_has_fua() is enough in
this case.

Limiting the FUA support to NCQ-supporting drives would make sense if NCQ
commands were always used to implement FUA (even with QD set to 1), but
this patch set version does not do that.

Thanks,
Maciej




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux