On Tue, Nov 08, 2022 at 02:55:42PM +0900, 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 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 in the
> ATA specification since ACS-3 (published in 2013), remove the
> ATA_CMD_WRITE_MULTI_FUA_EXT command from the ata_rw_cmds array.
>
> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/ata/libata-core.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 30adae16efff..2b66fe529d81 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,
> @@ -727,7 +727,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 +743,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
>
Reviewed-by: Niklas Cassel <niklas.cassel@xxxxxxx>
