From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Zurcher, Christopher J" <christopher.j.zurcher@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"Gao, Liming" <liming.gao@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>,
"Wang, Jian J" <jian.j.wang@intel.com>
Subject: Re: [edk2-devel] [PATCH v5 1/4] MdePkg: Implement SCSI commands for Security Protocol In/Out
Date: Fri, 30 Aug 2019 05:17:51 +0000 [thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C92324F@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <8EE4873E19344F4DA986A2AC15D512AE4A4802AF@CRSMSX103.amr.corp.intel.com>
Hello,
Sorry for top-posting.
I was thinking to make the parameters interface match between the UefiScsiLib
API and the EFI Storage Security Command Protocol service, since the
implementation of the SSC protocol will directly call the UefiScsiLib API.
More specifically, for UefiScsiLib API:
EFI_STATUS
EFIAPI
ScsiSecurityProtocolInCommand (
...
IN UINT32 TransferLength,
...
IN OUT UINT32 *DataLength
)
to match the SSC protocol service:
typedef
EFI_STATUS
(EFIAPI *EFI_STORAGE_SECURITY_RECEIVE_DATA)(
...
IN UINTN PayloadBufferSize,
...
OUT UINTN *PayloadTransferSize
)
and for UefiScsiLib API:
EFI_STATUS
EFIAPI
ScsiSecurityProtocolOutCommand (
...
IN UINT32 TransferLength,
...
)
to match the SSC protocol service:
typedef
EFI_STATUS
(EFIAPI *EFI_STORAGE_SECURITY_SEND_DATA) (
...
IN UINTN PayloadBufferSize,
...
)
I am okay with the cast from UINTN to UINT32, as long as we can ensure
truncation will not happen (which I think should be safe when dealing with
data transfer with actual devices).
But for casting from UINTN* to UINT32*, I am not sure if this is a recommended
coding style. Maybe within the BIOS perspective, little endian is always the
case where such cast should work well.
I will leave this open to MdePkg package maintainers for their inputs.
Best Regards,
Hao Wu
> -----Original Message-----
> From: Zurcher, Christopher J
> Sent: Friday, August 30, 2019 8:35 AM
> To: Wu, Hao A; devel@edk2.groups.io
> Cc: Yao, Jiewen; Wang, Jian J; Gao, Liming
> Subject: RE: [edk2-devel] [PATCH v5 1/4] MdePkg: Implement SCSI
> commands for Security Protocol In/Out
>
> I've implemented all the suggested changes except changing the arguments
> from UINT32 to UINTN. No other functions in UefiScsiLib take UINTN
> arguments, and since the library is directly packing the CDB, I think it makes
> sense to force the caller to provide the correct-size length value. That way
> there is no ambiguity on what is going to the device.
> If you agree I will send the updated patchset.
>
> Thanks,
> Christopher Zurcher
>
> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, August 26, 2019 20:03
> To: devel@edk2.groups.io; Zurcher, Christopher J
> <christopher.j.zurcher@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [edk2-devel] [PATCH v5 1/4] MdePkg: Implement SCSI
> commands for Security Protocol In/Out
>
> Hello,
>
> Please refer to the below inline comments:
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Zurcher, Christopher J
> > Sent: Friday, August 23, 2019 6:02 AM
> > To: devel@edk2.groups.io
> > Cc: Yao, Jiewen; Wang, Jian J; Gao, Liming
> > Subject: [edk2-devel] [PATCH v5 1/4] MdePkg: Implement SCSI commands
> > for Security Protocol In/Out
> >
> > This patch implements the Security Protocol In and Security Protocol Out
> > commands in UefiScsiLib to prepare support for the Storage Security
> > Command Protocol.
> >
> > Cc: Jiewen Yao <jiewen.yao@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/Include/IndustryStandard/Scsi.h | 48 +++--
> > MdePkg/Include/Library/UefiScsiLib.h | 126 +++++++++++-
> > MdePkg/Include/Protocol/ScsiIo.h | 9 +-
> > MdePkg/Library/UefiScsiLib/UefiScsiLib.c | 205 +++++++++++++++++++-
> > 4 files changed, 366 insertions(+), 22 deletions(-)
> >
> > diff --git a/MdePkg/Include/IndustryStandard/Scsi.h
> > b/MdePkg/Include/IndustryStandard/Scsi.h
> > index cbe5709fe5..10d7b49ba7 100644
> > --- a/MdePkg/Include/IndustryStandard/Scsi.h
> > +++ b/MdePkg/Include/IndustryStandard/Scsi.h
> > @@ -1,7 +1,7 @@
> > /** @file
> > Support for SCSI-2 standard
> >
> > - Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -163,6 +163,12 @@
> > #define EFI_SCSI_OP_SEND_MESSAGE10 0x2a
> > #define EFI_SCSI_OP_SEND_MESSAGE12 0xaa
> >
> > +//
> > +// Additional commands for Secure Transactions
> > +//
> > +#define EFI_SCSI_OP_SECURITY_PROTOCOL_IN 0xa2
> > +#define EFI_SCSI_OP_SECURITY_PROTOCOL_OUT 0xb5
> > +
> > //
> > // SCSI Data Transfer Direction
> > //
> > @@ -172,22 +178,30 @@
> > //
> > // Peripheral Device Type Definitions
> > //
> > -#define EFI_SCSI_TYPE_DISK 0x00 ///< Direct-access device (e.g.
> > magnetic disk)
> > -#define EFI_SCSI_TYPE_TAPE 0x01 ///< Sequential-access device (e.g.
> > magnetic tape)
> > -#define EFI_SCSI_TYPE_PRINTER 0x02 ///< Printer device
> > -#define EFI_SCSI_TYPE_PROCESSOR 0x03 ///< Processor device
> > -#define EFI_SCSI_TYPE_WORM 0x04 ///< Write-once device (e.g.
> some
> > optical disks)
> > -#define EFI_SCSI_TYPE_CDROM 0x05 ///< CD-ROM device
> > -#define EFI_SCSI_TYPE_SCANNER 0x06 ///< Scanner device
> > -#define EFI_SCSI_TYPE_OPTICAL 0x07 ///< Optical memory device (e.g.
> > some optical disks)
> > -#define EFI_SCSI_TYPE_MEDIUMCHANGER 0x08 ///< Medium changer
> > device (e.g. jukeboxes)
> > -#define EFI_SCSI_TYPE_COMMUNICATION 0x09 ///< Communications
> > device
> > -#define EFI_SCSI_TYPE_ASCIT8_1 0x0A ///< Defined by ASC IT8
> (Graphic
> > arts pre-press devices)
> > -#define EFI_SCSI_TYPE_ASCIT8_2 0x0B ///< Defined by ASC IT8
> (Graphic
> > arts pre-press devices)
>
>
> Could you help to address Liming's comment in the V4 series that to preserve
> the definition for EFI_SCSI_TYPE_ASCIT8_1 & EFI_SCSI_TYPE_ASCIT8_2 for
> compatibility consideration:
>
> https://edk2.groups.io/g/devel/message/42361?p=,,,20,0,0,0::Created,,scsi,
> 20,2,40,32048246
>
>
> > -//
> > -// 0Ch - 1Eh are reserved
> > -//
> > -#define EFI_SCSI_TYPE_UNKNOWN 0x1F ///< Unknown or no device
> > type
> > +#define EFI_SCSI_TYPE_DISK 0x00 ///< Direct-access device (e.g.
> > magnetic disk)
> > +#define EFI_SCSI_TYPE_TAPE 0x01 ///< Sequential-access device
> (e.g.
> > magnetic tape)
> > +#define EFI_SCSI_TYPE_PRINTER 0x02 ///< Printer device
> > +#define EFI_SCSI_TYPE_PROCESSOR 0x03 ///< Processor device
> > +#define EFI_SCSI_TYPE_WORM 0x04 ///< Write-once device (e.g.
> > some optical disks)
> > +#define EFI_SCSI_TYPE_CDROM 0x05 ///< CD/DVD device
> > +#define EFI_SCSI_TYPE_SCANNER 0x06 ///< Scanner device (obsolete)
> > +#define EFI_SCSI_TYPE_OPTICAL 0x07 ///< Optical memory device
> (e.g.
> > some optical disks)
> > +#define EFI_SCSI_TYPE_MEDIUMCHANGER 0x08 ///< Medium changer
> > device (e.g. jukeboxes)
> > +#define EFI_SCSI_TYPE_COMMUNICATION 0x09 ///< Communications
> > device (obsolete)
> > +#define EFI_SCSI_TYPE_A 0x0A ///< Obsolete
> > +#define EFI_SCSI_TYPE_B 0x0B ///< Obsolete
> > +#define EFI_SCSI_TYPE_RAID 0x0C ///< Storage array controller
> > device (e.g., RAID)
> > +#define EFI_SCSI_TYPE_SES 0x0D ///< Enclosure services device
> > +#define EFI_SCSI_TYPE_RBC 0x0E ///< Simplified direct-access
> device
> > (e.g., magnetic disk)
> > +#define EFI_SCSI_TYPE_OCRW 0x0F ///< Optical card reader/writer
> > device
> > +#define EFI_SCSI_TYPE_BRIDGE 0x10 ///< Bridge Controller
> Commands
> > +#define EFI_SCSI_TYPE_OSD 0x11 ///< Object-based Storage
> Device
> > +#define EFI_SCSI_TYPE_AUTOMATION 0x12 ///< Automation/Drive
> > Interface
> > +#define EFI_SCSI_TYPE_SECURITYMANAGER 0x13 ///< Security manager
> > device
> > +#define EFI_SCSI_TYPE_RESERVED_LOW 0x14 ///< Reserved (low)
> > +#define EFI_SCSI_TYPE_RESERVED_HIGH 0x1D ///< Reserved (high)
> > +#define EFI_SCSI_TYPE_WLUN 0x1E ///< Well known logical unit
> > +#define EFI_SCSI_TYPE_UNKNOWN 0x1F ///< Unknown or no device
> > type
> >
> > //
> > // Page Codes for INQUIRY command
> > diff --git a/MdePkg/Include/Library/UefiScsiLib.h
> > b/MdePkg/Include/Library/UefiScsiLib.h
> > index 10dd81902b..a0d99e703a 100644
> > --- a/MdePkg/Include/Library/UefiScsiLib.h
> > +++ b/MdePkg/Include/Library/UefiScsiLib.h
> > @@ -5,7 +5,7 @@
> > for hard drive, CD and DVD devices that are the most common SCSI boot
> > targets used by UEFI platforms.
> > This library class depends on SCSI I/O Protocol defined in UEFI
> Specification
> > and SCSI-2 industry standard.
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -813,6 +813,130 @@ ScsiWrite16Command (
> > );
> >
> >
> > +/**
> > + Execute Security Protocol In SCSI command on a specific SCSI target.
> > +
> > + Executes the SCSI Security Protocol In command on the SCSI target
> > specified by ScsiIo.
> > + If Timeout is zero, then this function waits indefinitely for the command
> to
> > complete.
> > + If Timeout is greater than zero, then the command is executed and will
> > timeout after
> > + Timeout 100 ns units. The StartLba and SectorSize parameters are used
> to
> > construct
>
>
> As mentioned in V4 series:
>
> There is no 'StartLba' & 'SectorSize' parameters for APIs:
>
> ScsiSecurityProtocolInCommand
> ScsiSecurityProtocolOutCommand
>
> Could you help to update the comments to address this?
> (Please help to update UefiScsiLib.c as well.)
>
>
> > + the CDB for this SCSI command.
> > + If ScsiIo is NULL, then ASSERT().
> > + If SenseDataLength is NULL, then ASSERT().
> > + If HostAdapterStatus is NULL, then ASSERT().
> > + If TargetStatus is NULL, then ASSERT().
> > + If DataLength is NULL, then ASSERT().
> > +
> > + If SenseDataLength is non-zero and SenseData is not NULL, SenseData
> > must meet buffer
> > + alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise
> > EFI_INVALID_PARAMETER
> > + gets returned.
> > +
> > + If DataLength is non-zero and DataBuffer is not NULL, DataBuffer must
> > meet buffer
> > + alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise
> > EFI_INVALID_PARAMETER
> > + gets returned.
> > +
> > + @param[in] ScsiIo SCSI IO Protocol to use.
> > + @param[in] Timeout The length of timeout period.
> > + @param[in, out] SenseData A pointer to output sense data.
> > + @param[in, out] SenseDataLength The length of output sense data.
> > + @param[out] HostAdapterStatus The status of Host Adapter.
> > + @param[out] TargetStatus The status of the target.
> > + @param[in] SecurityProtocol The Security Protocol to use.
> > + @param[in] SecurityProtocolSpecific The Security Protocol Specific
> data.
> > + @param[in] TransferLength The size in bytes of the data allocation.
> > + @param[in, out] DataBuffer A pointer to a data buffer.
> > + @param[in, out] DataLength The length of data buffer.
>
>
> As mentioned in V4 series:
>
> Referring to the implementation of the library (changes made in
> MdePkg/Library/UefiScsiLib/UefiScsiLib.c):
>
> 'TransferLength' (input) specifies the length of content in 'DataBuffer';
> 'DataLength' (input & output) reflects the actual number of bytes
> transferred.
>
> How about swapping their names and changing the description comments to:
> (Please help to update UefiScsiLib.c as well.)
>
> @param[in] DataLength The size in bytes of the data buffer.
> ...
> @param[out] TransferLength A pointer to a buffer to store the size
> in bytes of the data written to the data
> buffer.
>
>
> > +
> > + @retval EFI_SUCCESS Command is executed successfully.
> > + @retval EFI_BAD_BUFFER_SIZE The SCSI Request Packet was
> > executed, but the entire DataBuffer could
> > + not be transferred. The actual number of bytes
> > transferred is returned in DataLength.
> > + @retval EFI_NOT_READY The SCSI Request Packet could not be
> > sent because there are too many
> > + SCSI Command Packets already queued.
> > + @retval EFI_DEVICE_ERROR A device error occurred while
> > attempting to send SCSI Request Packet.
> > + @retval EFI_UNSUPPORTED The command described by the SCSI
> > Request Packet is not supported by
> > + the SCSI initiator(i.e., SCSI Host Controller)
> > + @retval EFI_TIMEOUT A timeout occurred while waiting for the
> > SCSI Request Packet to execute.
> > + @retval EFI_INVALID_PARAMETER The contents of the SCSI Request
> > Packet are invalid.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +ScsiSecurityProtocolInCommand (
> > + IN EFI_SCSI_IO_PROTOCOL *ScsiIo,
> > + IN UINT64 Timeout,
> > + IN OUT VOID *SenseData, OPTIONAL
> > + IN OUT UINT8 *SenseDataLength,
> > + OUT UINT8 *HostAdapterStatus,
> > + OUT UINT8 *TargetStatus,
> > + IN UINT8 SecurityProtocol,
> > + IN UINT16 SecurityProtocolSpecific,
> > + IN UINT32 TransferLength,
> > + IN OUT VOID *DataBuffer, OPTIONAL
> > + IN OUT UINT32 *DataLength
> > + );
>
>
> As mentioned in V4 series, could you help to add a new parameter "Inc512"
> for
> both new APIs:
> ScsiSecurityProtocolInCommand
> ScsiSecurityProtocolOutCommand
>
> Though UFS spec requires the INC_512 field of a CDB to be set to 0, but
> for other devices, setting this field to 1 may be a valid configuration.
>
>
> Also, I would suggest the below parameter type changes to match with the
> services
> definition of the EFI_STORAGE_SECURITY_COMMAND_PROTOCOL (including
> the
> name swap mentioned above):
>
> IN UINT32 TransferLength,
> to
> IN UINTN DataLength,
>
> IN OUT UINT32 *DataLength
> to
> OUT UINTN *TransferLength
>
>
> > +
> > +
> > +/**
> > + Execute Security Protocol Out SCSI command on a specific SCSI target.
> > +
> > + Executes the SCSI Security Protocol Out command on the SCSI target
> > specified by ScsiIo.
> > + If Timeout is zero, then this function waits indefinitely for the command
> to
> > complete.
> > + If Timeout is greater than zero, then the command is executed and will
> > timeout after
> > + Timeout 100 ns units. The StartLba and SectorSize parameters are used
> to
> > construct
>
>
> As mentioned in V4 series:
>
> There is no 'StartLba' & 'SectorSize' parameters for APIs:
>
> ScsiSecurityProtocolInCommand
> ScsiSecurityProtocolOutCommand
>
> Could you help to update the comments to address this?
> (Please help to update UefiScsiLib.c as well.)
>
>
> > + the CDB for this SCSI command.
> > + If ScsiIo is NULL, then ASSERT().
> > + If SenseDataLength is NULL, then ASSERT().
> > + If HostAdapterStatus is NULL, then ASSERT().
> > + If TargetStatus is NULL, then ASSERT().
> > + If DataLength is NULL, then ASSERT().
> > +
> > + If SenseDataLength is non-zero and SenseData is not NULL, SenseData
> > must meet buffer
> > + alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise
> > EFI_INVALID_PARAMETER
> > + gets returned.
> > +
> > + If DataLength is non-zero and DataBuffer is not NULL, DataBuffer must
> > meet buffer
> > + alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise
> > EFI_INVALID_PARAMETER
> > + gets returned.
> > +
> > + @param[in] ScsiIo SCSI IO Protocol to use.
> > + @param[in] Timeout The length of timeout period.
> > + @param[in, out] SenseData A pointer to output sense data.
> > + @param[in, out] SenseDataLength The length of output sense data.
> > + @param[out] HostAdapterStatus The status of Host Adapter.
> > + @param[out] TargetStatus The status of the target.
> > + @param[in] SecurityProtocol The Security Protocol to use.
> > + @param[in] SecurityProtocolSpecific The Security Protocol Specific
> data.
> > + @param[in] TransferLength The size in bytes of the transfer data.
> > + @param[in, out] DataBuffer A pointer to a data buffer.
>
>
> As mentioned in V4:
>
> Suggest to rename 'TransferLength' to 'DataLength' so that it may be a bit
> more clear for users to know 'DataLength' reflects the size of 'DataBuffer'.
>
>
> > +
> > + @retval EFI_SUCCESS Command is executed successfully.
> > + @retval EFI_BAD_BUFFER_SIZE The SCSI Request Packet was
> > executed, but the entire DataBuffer could
> > + not be transferred. The actual number of bytes
> > transferred is returned in DataLength.
> > + @retval EFI_NOT_READY The SCSI Request Packet could not be
> > sent because there are too many
> > + SCSI Command Packets already queued.
> > + @retval EFI_DEVICE_ERROR A device error occurred while
> > attempting to send SCSI Request Packet.
> > + @retval EFI_UNSUPPORTED The command described by the SCSI
> > Request Packet is not supported by
> > + the SCSI initiator(i.e., SCSI Host Controller)
> > + @retval EFI_TIMEOUT A timeout occurred while waiting for the
> > SCSI Request Packet to execute.
> > + @retval EFI_INVALID_PARAMETER The contents of the SCSI Request
> > Packet are invalid.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +ScsiSecurityProtocolOutCommand (
> > + IN EFI_SCSI_IO_PROTOCOL *ScsiIo,
> > + IN UINT64 Timeout,
> > + IN OUT VOID *SenseData, OPTIONAL
> > + IN OUT UINT8 *SenseDataLength,
> > + OUT UINT8 *HostAdapterStatus,
> > + OUT UINT8 *TargetStatus,
> > + IN UINT8 SecurityProtocol,
> > + IN UINT16 SecurityProtocolSpecific,
> > + IN UINT32 TransferLength,
> > + IN OUT VOID *DataBuffer OPTIONAL
> > + );
>
>
> As mentioned in V4 series:
>
> Suggest the below parameter type changes to match with the services
> definition of the EFI_STORAGE_SECURITY_COMMAND_PROTOCOL (including
> the
> name change mentioned above):
>
> IN UINT32 TransferLength
> to
> IN UINTN DataLength
>
> Best Regards,
> Hao Wu
>
>
> > +
> > +
> > /**
> > Execute blocking/non-blocking Read(10) SCSI command on a specific SCSI
> > target.
> > diff --git a/MdePkg/Include/Protocol/ScsiIo.h
> > b/MdePkg/Include/Protocol/ScsiIo.h
> > index 05e46bda9c..27c31fe7f9 100644
> > --- a/MdePkg/Include/Protocol/ScsiIo.h
> > +++ b/MdePkg/Include/Protocol/ScsiIo.h
> > @@ -4,7 +4,7 @@
> > services environment to access SCSI devices. In particular, functions for
> > managing devices on SCSI buses are defined here.
> >
> > - Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -43,8 +43,11 @@ typedef struct _EFI_SCSI_IO_PROTOCOL
> > EFI_SCSI_IO_PROTOCOL;
> > #define MFI_SCSI_IO_TYPE_OCRW 0x0F ///< Optical
> card
> > reader/writer device
> > #define MFI_SCSI_IO_TYPE_BRIDGE 0x10 ///< Bridge
> > Controller Commands
> > #define MFI_SCSI_IO_TYPE_OSD 0x11 ///< Object-
> based
> > Storage Device
> > -#define EFI_SCSI_IO_TYPE_RESERVED_LOW 0x12 ///<
> > Reserved (low)
> > -#define EFI_SCSI_IO_TYPE_RESERVED_HIGH 0x1E ///<
> > Reserved (high)
> > +#define MFI_SCSI_IO_TYPE_AUTOMATION 0x12 ///<
> > Automation/Drive Interface
> > +#define MFI_SCSI_IO_TYPE_SECURITYMANAGER 0x13 ///<
> > Security manager device
> > +#define EFI_SCSI_IO_TYPE_RESERVED_LOW 0x14 ///<
> > Reserved (low)
> > +#define EFI_SCSI_IO_TYPE_RESERVED_HIGH 0x1D ///<
> > Reserved (high)
> > +#define EFI_SCSI_IO_TYPE_WLUN 0x1E ///< Well
> known
> > logical unit
> > #define EFI_SCSI_IO_TYPE_UNKNOWN 0x1F ///<
> Unknown
> > no device type
> >
> > //
> > diff --git a/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
> > b/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
> > index c7491d1436..7584d717ad 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 - 2018, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -23,6 +23,7 @@
> > //
> > #define EFI_SCSI_OP_LENGTH_SIX 0x6
> > #define EFI_SCSI_OP_LENGTH_TEN 0xa
> > +#define EFI_SCSI_OP_LENGTH_TWELVE 0xc
> > #define EFI_SCSI_OP_LENGTH_SIXTEEN 0x10
> >
> > //
> > @@ -1280,6 +1281,208 @@ ScsiWrite16Command (
> > }
> >
> >
> > +/**
> > + Execute Security Protocol In SCSI command on a specific SCSI target.
> > +
> > + Executes the SCSI Security Protocol In command on the SCSI target
> > specified by ScsiIo.
> > + If Timeout is zero, then this function waits indefinitely for the command
> to
> > complete.
> > + If Timeout is greater than zero, then the command is executed and will
> > timeout after
> > + Timeout 100 ns units. The StartLba and SectorSize parameters are used
> to
> > construct
>
>
> As mentioned in V4 series:
>
> There is no 'StartLba' & 'SectorSize' parameters for APIs:
>
> ScsiSecurityProtocolInCommand
> ScsiSecurityProtocolOutCommand
>
> Could you help to update the comments to address this?
> (Please help to update UefiScsiLib.c as well.)
>
>
> > + the CDB for this SCSI command.
> > + If ScsiIo is NULL, then ASSERT().
> > + If SenseDataLength is NULL, then ASSERT().
> > + If HostAdapterStatus is NULL, then ASSERT().
> > + If TargetStatus is NULL, then ASSERT().
> > + If DataLength is NULL, then ASSERT().
> > +
> > + If SenseDataLength is non-zero and SenseData is not NULL, SenseData
> > must meet buffer
> > + alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise
> > EFI_INVALID_PARAMETER
> > + gets returned.
> > +
> > + If DataLength is non-zero and DataBuffer is not NULL, DataBuffer must
> > meet buffer
> > + alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise
> > EFI_INVALID_PARAMETER
> > + gets returned.
> > +
> > + @param[in] ScsiIo SCSI IO Protocol to use.
> > + @param[in] Timeout The length of timeout period.
> > + @param[in, out] SenseData A pointer to output sense data.
> > + @param[in, out] SenseDataLength The length of output sense data.
> > + @param[out] HostAdapterStatus The status of Host Adapter.
> > + @param[out] TargetStatus The status of the target.
> > + @param[in] SecurityProtocol The Security Protocol to use.
> > + @param[in] SecurityProtocolSpecific The Security Protocol Specific
> data.
> > + @param[in] TransferLength The size in bytes of the data allocation.
> > + @param[in, out] DataBuffer A pointer to a data buffer.
> > + @param[in, out] DataLength The length of data buffer.
> > +
> > + @retval EFI_SUCCESS Command is executed successfully.
> > + @retval EFI_BAD_BUFFER_SIZE The SCSI Request Packet was
> > executed, but the entire DataBuffer could
> > + not be transferred. The actual number of bytes
> > transferred is returned in DataLength.
> > + @retval EFI_NOT_READY The SCSI Request Packet could not be
> > sent because there are too many
> > + SCSI Command Packets already queued.
> > + @retval EFI_DEVICE_ERROR A device error occurred while
> > attempting to send SCSI Request Packet.
> > + @retval EFI_UNSUPPORTED The command described by the SCSI
> > Request Packet is not supported by
> > + the SCSI initiator(i.e., SCSI Host Controller)
> > + @retval EFI_TIMEOUT A timeout occurred while waiting for the
> > SCSI Request Packet to execute.
> > + @retval EFI_INVALID_PARAMETER The contents of the SCSI Request
> > Packet are invalid.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +ScsiSecurityProtocolInCommand (
> > + IN EFI_SCSI_IO_PROTOCOL *ScsiIo,
> > + IN UINT64 Timeout,
> > + IN OUT VOID *SenseData, OPTIONAL
> > + IN OUT UINT8 *SenseDataLength,
> > + OUT UINT8 *HostAdapterStatus,
> > + OUT UINT8 *TargetStatus,
> > + IN UINT8 SecurityProtocol,
> > + IN UINT16 SecurityProtocolSpecific,
> > + IN UINT32 TransferLength,
> > + IN OUT VOID *DataBuffer, OPTIONAL
> > + IN OUT UINT32 *DataLength
> > + )
> > +{
> > + EFI_SCSI_IO_SCSI_REQUEST_PACKET CommandPacket;
> > + EFI_STATUS Status;
> > + UINT8 Cdb[EFI_SCSI_OP_LENGTH_TWELVE];
> > +
> > + ASSERT (SenseDataLength != NULL);
> > + ASSERT (HostAdapterStatus != NULL);
> > + ASSERT (TargetStatus != NULL);
> > + ASSERT (DataLength != NULL);
> > + ASSERT (ScsiIo != NULL);
> > +
> > + ZeroMem (&CommandPacket, sizeof
> > (EFI_SCSI_IO_SCSI_REQUEST_PACKET));
> > + ZeroMem (Cdb, EFI_SCSI_OP_LENGTH_TWELVE);
> > +
> > + CommandPacket.Timeout = Timeout;
> > + CommandPacket.InDataBuffer = DataBuffer;
> > + CommandPacket.SenseData = SenseData;
> > + CommandPacket.InTransferLength = TransferLength;
> > + CommandPacket.Cdb = Cdb;
> > + //
> > + // Fill Cdb for Security Protocol In Command
> > + //
> > + Cdb[0] = EFI_SCSI_OP_SECURITY_PROTOCOL_IN;
> > + Cdb[1] = SecurityProtocol;
> > + WriteUnaligned16 ((UINT16 *)&Cdb[2], SwapBytes16
> > (SecurityProtocolSpecific));
> > + WriteUnaligned32 ((UINT32 *)&Cdb[6], SwapBytes32 (TransferLength));
> > +
> > + CommandPacket.CdbLength = EFI_SCSI_OP_LENGTH_TWELVE;
> > + CommandPacket.DataDirection = EFI_SCSI_DATA_IN;
> > + CommandPacket.SenseDataLength = *SenseDataLength;
> > +
> > + Status = ScsiIo->ExecuteScsiCommand (ScsiIo,
> > &CommandPacket, NULL);
> > +
> > + *HostAdapterStatus = CommandPacket.HostAdapterStatus;
> > + *TargetStatus = CommandPacket.TargetStatus;
> > + *SenseDataLength = CommandPacket.SenseDataLength;
> > + *DataLength = CommandPacket.InTransferLength;
> > +
> > + return Status;
> > +}
> > +
> > +
> > +/**
> > + Execute Security Protocol Out SCSI command on a specific SCSI target.
> > +
> > + Executes the SCSI Security Protocol Out command on the SCSI target
> > specified by ScsiIo.
> > + If Timeout is zero, then this function waits indefinitely for the command
> to
> > complete.
> > + If Timeout is greater than zero, then the command is executed and will
> > timeout after
> > + Timeout 100 ns units. The StartLba and SectorSize parameters are used
> to
> > construct
> > + the CDB for this SCSI command.
> > + If ScsiIo is NULL, then ASSERT().
> > + If SenseDataLength is NULL, then ASSERT().
> > + If HostAdapterStatus is NULL, then ASSERT().
> > + If TargetStatus is NULL, then ASSERT().
> > + If DataLength is NULL, then ASSERT().
> > +
> > + If SenseDataLength is non-zero and SenseData is not NULL, SenseData
> > must meet buffer
> > + alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise
> > EFI_INVALID_PARAMETER
> > + gets returned.
> > +
> > + If DataLength is non-zero and DataBuffer is not NULL, DataBuffer must
> > meet buffer
> > + alignment requirement defined in EFI_SCSI_IO_PROTOCOL. Otherwise
> > EFI_INVALID_PARAMETER
> > + gets returned.
> > +
> > + @param[in] ScsiIo SCSI IO Protocol to use.
> > + @param[in] Timeout The length of timeout period.
> > + @param[in, out] SenseData A pointer to output sense data.
> > + @param[in, out] SenseDataLength The length of output sense data.
> > + @param[out] HostAdapterStatus The status of Host Adapter.
> > + @param[out] TargetStatus The status of the target.
> > + @param[in] SecurityProtocol The Security Protocol to use.
> > + @param[in] SecurityProtocolSpecific The Security Protocol Specific
> data.
> > + @param[in] TransferLength The size in bytes of the transfer data.
> > + @param[in, out] DataBuffer A pointer to a data buffer.
> > +
> > + @retval EFI_SUCCESS Command is executed successfully.
> > + @retval EFI_BAD_BUFFER_SIZE The SCSI Request Packet was
> > executed, but the entire DataBuffer could
> > + not be transferred. The actual number of bytes
> > transferred is returned in DataLength.
> > + @retval EFI_NOT_READY The SCSI Request Packet could not be
> > sent because there are too many
> > + SCSI Command Packets already queued.
> > + @retval EFI_DEVICE_ERROR A device error occurred while
> > attempting to send SCSI Request Packet.
> > + @retval EFI_UNSUPPORTED The command described by the SCSI
> > Request Packet is not supported by
> > + the SCSI initiator(i.e., SCSI Host Controller)
> > + @retval EFI_TIMEOUT A timeout occurred while waiting for the
> > SCSI Request Packet to execute.
> > + @retval EFI_INVALID_PARAMETER The contents of the SCSI Request
> > Packet are invalid.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +ScsiSecurityProtocolOutCommand (
> > + IN EFI_SCSI_IO_PROTOCOL *ScsiIo,
> > + IN UINT64 Timeout,
> > + IN OUT VOID *SenseData, OPTIONAL
> > + IN OUT UINT8 *SenseDataLength,
> > + OUT UINT8 *HostAdapterStatus,
> > + OUT UINT8 *TargetStatus,
> > + IN UINT8 SecurityProtocol,
> > + IN UINT16 SecurityProtocolSpecific,
> > + IN UINT32 TransferLength,
> > + IN OUT VOID *DataBuffer OPTIONAL
> > + )
> > +{
> > + EFI_SCSI_IO_SCSI_REQUEST_PACKET CommandPacket;
> > + EFI_STATUS Status;
> > + UINT8 Cdb[EFI_SCSI_OP_LENGTH_TWELVE];
> > +
> > + ASSERT (SenseDataLength != NULL);
> > + ASSERT (HostAdapterStatus != NULL);
> > + ASSERT (TargetStatus != NULL);
> > + ASSERT (ScsiIo != NULL);
> > +
> > + ZeroMem (&CommandPacket, sizeof
> > (EFI_SCSI_IO_SCSI_REQUEST_PACKET));
> > + ZeroMem (Cdb, EFI_SCSI_OP_LENGTH_TWELVE);
> > +
> > + CommandPacket.Timeout = Timeout;
> > + CommandPacket.OutDataBuffer = DataBuffer;
> > + CommandPacket.SenseData = SenseData;
> > + CommandPacket.OutTransferLength = TransferLength;
> > + CommandPacket.Cdb = Cdb;
> > + //
> > + // Fill Cdb for Security Protocol Out Command
> > + //
> > + Cdb[0] = EFI_SCSI_OP_SECURITY_PROTOCOL_OUT;
> > + Cdb[1] = SecurityProtocol;
> > + WriteUnaligned16 ((UINT16 *)&Cdb[2], SwapBytes16
> > (SecurityProtocolSpecific));
> > + WriteUnaligned32 ((UINT32 *)&Cdb[6], SwapBytes32 (TransferLength));
> > +
> > + CommandPacket.CdbLength = EFI_SCSI_OP_LENGTH_TWELVE;
> > + CommandPacket.DataDirection = EFI_SCSI_DATA_OUT;
> > + CommandPacket.SenseDataLength = *SenseDataLength;
> > +
> > + Status = ScsiIo->ExecuteScsiCommand (ScsiIo,
> > &CommandPacket, NULL);
> > +
> > + *HostAdapterStatus = CommandPacket.HostAdapterStatus;
> > + *TargetStatus = CommandPacket.TargetStatus;
> > + *SenseDataLength = CommandPacket.SenseDataLength;
> > +
> > + return Status;
> > +}
> > +
> > +
> > /**
> > Internal helper notify function in which update the result of the
> > non-blocking SCSI Read/Write commands and signal caller event.
> > --
> > 2.16.2.windows.1
> >
> >
> >
next prev parent reply other threads:[~2019-08-30 5:18 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-22 22:02 [PATCH v5 0/4] Add SCSI Support for Storage Security Command Protocol Zurcher, Christopher J
2019-08-22 22:02 ` [PATCH v5 1/4] MdePkg: Implement SCSI commands for Security Protocol In/Out Zurcher, Christopher J
2019-08-27 3:03 ` [edk2-devel] " Wu, Hao A
2019-08-30 0:34 ` Zurcher, Christopher J
2019-08-30 5:17 ` Wu, Hao A [this message]
2019-08-30 9:14 ` Liming Gao
2019-09-02 1:10 ` Wu, Hao A
2019-09-02 1:46 ` Liming Gao
2019-09-02 2:01 ` Wu, Hao A
2019-08-22 22:02 ` [PATCH v5 2/4] MdeModulePkg/UfsPassThruDxe: Check for RPMB W-LUN (SecurityLun) Zurcher, Christopher J
2019-08-27 3:03 ` [edk2-devel] " Wu, Hao A
2019-08-22 22:02 ` [PATCH v5 3/4] MdeModulePkg/ScsiBusDxe: Clean up Peripheral Type check Zurcher, Christopher J
2019-08-27 3:03 ` [edk2-devel] " Wu, Hao A
2019-08-22 22:02 ` [PATCH v5 4/4] MdeModulePkg/ScsiDiskDxe: Support Storage Security Command Protocol Zurcher, Christopher J
2019-08-27 3:03 ` [edk2-devel] " Wu, Hao A
2019-08-23 2:54 ` [edk2-devel] [PATCH v5 0/4] Add SCSI Support for " Wu, Hao A
2019-08-27 3:02 ` Wu, Hao A
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-list 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=B80AF82E9BFB8E4FBD8C89DA810C6A093C92324F@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox