The "SCSI Block Commands - 2" (SBC-2) standard defines bits [7:5] of the
CDB byte 1 as Reserved, for the READ, WRITE, and VERIFY commands.

The updated "SCSI Block Commands - 3" (SBC-3) standard defined the same
bit-fields as RDPROTECT, WRPROTECT, and VRPROTECT, respectively.

Historically hardware has existed that was based on SBC-2, but required
driver software to encode the Logical Unit Number (LUN) in the above -- at
that time Reserved -- bit-fields. (Thereby forcing the driver to violate
even SBC-2).

Such a LUN encoding creates nonsensical RDPROTECT, WRPROTECT, and
VRPROTECT values for devices that conform to the SBC-3. In particular, if
the encoded value is nonzero (corresponding to a nonzero LUN), and the
recipient device does not support protection information, then it is
*required* to reject the command with CHECK CONDITION, ILLEGAL REQUEST,
INVALID FIELD IN CDB:

  ScsiDiskRead10: Check Condition happened!
  ScsiDisk: Sense Key = 0x5 ASC = 0x24!
  ScsiDiskRead10: Check Condition happened!
  ScsiDisk: Sense Key = 0x5 ASC = 0x24!
  ScsiDiskRead10: Check Condition happened!
  ScsiDisk: Sense Key = 0x5 ASC = 0x24!
  ScsiDiskRead10: Check Condition happened!
  ScsiDisk: Sense Key = 0x5 ASC = 0x24!
  FatOpenDevice: read of part_lba failed Device Error

UefiScsiLib has always encoded the LUN in the CDB for these commands. SVN
r8331 (git commit 676e2a32) disabled it briefly, but SVN r8334 (git commit
6b3ecf5c) reenabled it just a few hours later.

Unfortunately, the current code breaks on devices that follow the above
SBC-3 requirement; for example on SCSI disks with nonzero LUNs emulated by
QEMU (after QEMU commit 96bdbbab, part of v1.2.0).

Turn the 'LUN encoding in CDB' feature into a Feature PCD, so that edk2
platforms can control it explicitly. The default value preserves the
present behavior.

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1166971
Reported-by: Cole Robinson <crobi...@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
 MdePkg/Library/UefiScsiLib/UefiScsiLib.inf |  2 ++
 MdePkg/Library/UefiScsiLib/UefiScsiLib.c   | 16 ++++++++++++----
 MdePkg/MdePkg.dec                          |  9 +++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Library/UefiScsiLib/UefiScsiLib.inf 
b/MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
index 1eb9076..a2e8618 100644
--- a/MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
+++ b/MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
@@ -43,3 +43,5 @@
   DebugLib
   BaseLib
 
+[FeaturePcd]
+  gEfiMdePkgTokenSpaceGuid.PcdScsiReadWriteVerifyCdbLunEncoding ## CONSUMES
diff --git a/MdePkg/Library/UefiScsiLib/UefiScsiLib.c 
b/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
index bd838c4..f6a1146 100644
--- a/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
+++ b/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
@@ -930,7 +930,9 @@ ScsiRead10Command (
   ScsiIo->GetDeviceLocation (ScsiIo, &Target, &Lun);
 
   Cdb[0]                        = EFI_SCSI_OP_READ10;
-  Cdb[1]                        = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
+  if (FeaturePcdGet (PcdScsiReadWriteVerifyCdbLunEncoding)) {
+    Cdb[1]                      = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
+  }
   WriteUnaligned32 ((UINT32 *)&Cdb[2], SwapBytes32 (StartLba));
   WriteUnaligned16 ((UINT16 *)&Cdb[7], SwapBytes16 ((UINT16) SectorSize));
 
@@ -1028,7 +1030,9 @@ ScsiWrite10Command (
   ScsiIo->GetDeviceLocation (ScsiIo, &Target, &Lun);
 
   Cdb[0]                        = EFI_SCSI_OP_WRITE10;
-  Cdb[1]                        = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
+  if (FeaturePcdGet (PcdScsiReadWriteVerifyCdbLunEncoding)) {
+    Cdb[1]                      = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
+  }
   WriteUnaligned32 ((UINT32 *)&Cdb[2], SwapBytes32 (StartLba));
   WriteUnaligned16 ((UINT16 *)&Cdb[7], SwapBytes16 ((UINT16) SectorSize));
 
@@ -1126,7 +1130,9 @@ ScsiRead16Command (
   ScsiIo->GetDeviceLocation (ScsiIo, &Target, &Lun);
 
   Cdb[0]                        = EFI_SCSI_OP_READ16;
-  Cdb[1]                        = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
+  if (FeaturePcdGet (PcdScsiReadWriteVerifyCdbLunEncoding)) {
+    Cdb[1]                      = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
+  }
   WriteUnaligned64 ((UINT64 *)&Cdb[2], SwapBytes64 (StartLba));
   WriteUnaligned32 ((UINT32 *)&Cdb[10], SwapBytes32 (SectorSize));
 
@@ -1224,7 +1230,9 @@ ScsiWrite16Command (
   ScsiIo->GetDeviceLocation (ScsiIo, &Target, &Lun);
 
   Cdb[0]                        = EFI_SCSI_OP_WRITE16;
-  Cdb[1]                        = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
+  if (FeaturePcdGet (PcdScsiReadWriteVerifyCdbLunEncoding)) {
+    Cdb[1]                      = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
+  }
   WriteUnaligned64 ((UINT64 *)&Cdb[2], SwapBytes64 (StartLba));
   WriteUnaligned32 ((UINT32 *)&Cdb[10], SwapBytes32 (SectorSize));
 
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 6dc696e..5359b59 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1428,6 +1428,15 @@
   # @Prompt Validate ORDERED_COLLECTION structure
   
gEfiMdePkgTokenSpaceGuid.PcdValidateOrderedCollection|FALSE|BOOLEAN|0x0000002a
 
+  ## Determines if UefiScsiLib encodes the Logical Unit Number (LUN) in bits
+  #  [7:5] in byte 1 of the Command Descriptor Block (CDB) for the the READ,
+  #  WRITE, VERIFY commands.<BR><BR>
+  #   TRUE  - Encode the LUN in bits [7:5] of Cdb[1] for READ, WRITE, 
VERIFY.<BR>
+  #   FALSE - Do not encode the LUN in bits [7:5] of Cdb[1] for READ, WRITE,
+  #           VERIFY.<BR>
+  # @Prompt Encode LUN in CDB for READ, WRITE, VERIFY.
+  
gEfiMdePkgTokenSpaceGuid.PcdScsiReadWriteVerifyCdbLunEncoding|TRUE|BOOLEAN|0x0000002b
+
 [PcdsFixedAtBuild]
   ## Status code value for indicating a watchdog timer has expired.
   # EFI_COMPUTING_UNIT_HOST_PROCESSOR | EFI_CU_HP_EC_TIMER_EXPIRED
-- 
1.8.3.1



------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to