[PATCH 0/1] UefiScsiLib: Set FUA bit for synchronous SCSI Write operations

[PATCH 0/1] UefiScsiLib: Set FUA bit for synchronous SCSI Write operations

[Zurcher, Christopher J]

Some SCSI devices have very aggressive write caching implemented, causing
data to be lost if the system is shut down or rebooted soon after
receiving a successful write confirmation from the device. By setting the
FUA bit in the synchronous versions of the Write10/Write16 commands, the
write cache is skipped and data is forced directly to the disk.

The Asynchronous (WriteEx) commands will not have this bit set, allowing
performance-sensitive transactions to continue utilizing the write cache.

An alternative, more complicated solution would be to implement the
SYNCHRONIZE CACHE command and call it from ScsiDiskFlushBlocks, which
currently returns EFI_SUCCESS without flushing anything. Since the
SYNCHRONIZE CACHE command requires the caller to provide the specific
blocks to flush, and the BlockIO FlushBlocks function does not accept any
arguments, this would require keeping track of the write history inside the
ScsiDiskDxe driver.

I have not tested the asynchronous commands for this issue, so there is a
chance that resetting the system even after a call to FlushBlocksEx might
still result in lost data. Currently the ScsiDiskFlushBlocksEx command
waits for the BlockIo2 requests queue to empty, but does not ask the device
itself to flush anything. (EFI_SCSI_OP_SYNC_CACHE is defined in
IndustryStandard/Scsi.h but is not implemented anywhere.)

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <liming.gao@intel.com>

Christopher J Zurcher (1):
  MdePkg/UefiScsiLib: Set FUA bit for synchronous SCSI Write operations

 MdePkg/Library/UefiScsiLib/UefiScsiLib.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.16.2.windows.1

[PATCH 1/1] MdePkg/UefiScsiLib: Set FUA bit for synchronous SCSI Write operations

[Zurcher, Christopher J]

The FUA (Force Unit Access) bit forces data to be written directly to
disk instead of the write cache. This prevents data from being lost if a
shutdown or reset is requested immediately after a SCSI write operation.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Christopher J Zurcher <christopher.j.zurcher@intel.com>
---
 MdePkg/Library/UefiScsiLib/UefiScsiLib.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/MdePkg/Library/UefiScsiLib/UefiScsiLib.c b/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
index 13a2a1912c..cf78f131bd 100644
--- a/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
+++ b/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI SCSI Library implementation
 
-  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -1055,15 +1055,16 @@ ScsiWrite10Command (
   ZeroMem (&CommandPacket, sizeof (EFI_SCSI_IO_SCSI_REQUEST_PACKET));
   ZeroMem (Cdb, EFI_SCSI_OP_LENGTH_TEN);
 
-  CommandPacket.Timeout         = Timeout;
-  CommandPacket.OutDataBuffer    = DataBuffer;
-  CommandPacket.SenseData       = SenseData;
-  CommandPacket.OutTransferLength= *DataLength;
-  CommandPacket.Cdb             = Cdb;
+  CommandPacket.Timeout           = Timeout;
+  CommandPacket.OutDataBuffer     = DataBuffer;
+  CommandPacket.SenseData         = SenseData;
+  CommandPacket.OutTransferLength = *DataLength;
+  CommandPacket.Cdb               = Cdb;
   //
   // Fill Cdb for Write (10) Command
   //
   Cdb[0]                        = EFI_SCSI_OP_WRITE10;
+  Cdb[1]                        = BIT3; //FUA bit (Force Unit Access)
   WriteUnaligned32 ((UINT32 *)&Cdb[2], SwapBytes32 (StartLba));
   WriteUnaligned16 ((UINT16 *)&Cdb[7], SwapBytes16 ((UINT16) SectorSize));
 
@@ -1263,6 +1264,7 @@ ScsiWrite16Command (
   // Fill Cdb for Write (16) Command
   //
   Cdb[0]                        = EFI_SCSI_OP_WRITE16;
+  Cdb[1]                        = BIT3; //FUA bit (Force Unit Access)
   WriteUnaligned64 ((UINT64 *)&Cdb[2], SwapBytes64 (StartLba));
   WriteUnaligned32 ((UINT32 *)&Cdb[10], SwapBytes32 (SectorSize));
 
-- 
2.16.2.windows.1

Re: [edk2-devel] [PATCH 1/1] MdePkg/UefiScsiLib: Set FUA bit for synchronous SCSI Write operations

[Zhiguang Liu]

Hi Christopher

Could you please give more information about this code change?
For example, which Spec the code change is based on.

Thanks
Zhiguang

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Zurcher, Christopher J
> Sent: Saturday, February 22, 2020 10:12 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2-devel] [PATCH 1/1] MdePkg/UefiScsiLib: Set FUA bit for
> synchronous SCSI Write operations
> 
> The FUA (Force Unit Access) bit forces data to be written directly to disk
> instead of the write cache. This prevents data from being lost if a shutdown
> or reset is requested immediately after a SCSI write operation.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Christopher J Zurcher <christopher.j.zurcher@intel.com>
> ---
>  MdePkg/Library/UefiScsiLib/UefiScsiLib.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
> b/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
> index 13a2a1912c..cf78f131bd 100644
> --- a/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
> +++ b/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>    UEFI SCSI Library implementation
> 
> -  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2020, Intel Corporation. All rights
> + reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -1055,15 +1055,16 @@ ScsiWrite10Command (
>    ZeroMem (&CommandPacket, sizeof
> (EFI_SCSI_IO_SCSI_REQUEST_PACKET));
>    ZeroMem (Cdb, EFI_SCSI_OP_LENGTH_TEN);
> 
> -  CommandPacket.Timeout         = Timeout;
> -  CommandPacket.OutDataBuffer    = DataBuffer;
> -  CommandPacket.SenseData       = SenseData;
> -  CommandPacket.OutTransferLength= *DataLength;
> -  CommandPacket.Cdb             = Cdb;
> +  CommandPacket.Timeout           = Timeout;
> +  CommandPacket.OutDataBuffer     = DataBuffer;
> +  CommandPacket.SenseData         = SenseData;
> +  CommandPacket.OutTransferLength = *DataLength;
> +  CommandPacket.Cdb               = Cdb;
>    //
>    // Fill Cdb for Write (10) Command
>    //
>    Cdb[0]                        = EFI_SCSI_OP_WRITE10;
> +  Cdb[1]                        = BIT3; //FUA bit (Force Unit Access)
>    WriteUnaligned32 ((UINT32 *)&Cdb[2], SwapBytes32 (StartLba));
>    WriteUnaligned16 ((UINT16 *)&Cdb[7], SwapBytes16 ((UINT16)
> SectorSize));
> 
> @@ -1263,6 +1264,7 @@ ScsiWrite16Command (
>    // Fill Cdb for Write (16) Command
>    //
>    Cdb[0]                        = EFI_SCSI_OP_WRITE16;
> +  Cdb[1]                        = BIT3; //FUA bit (Force Unit Access)
>    WriteUnaligned64 ((UINT64 *)&Cdb[2], SwapBytes64 (StartLba));
>    WriteUnaligned32 ((UINT32 *)&Cdb[10], SwapBytes32 (SectorSize));
> 
> --
> 2.16.2.windows.1
> 
> 
>