From: "Liming Gao" <liming.gao@intel.com>
To: "Wu, Hao A" <hao.a.wu@intel.com>,
"Zurcher, Christopher J" <christopher.j.zurcher@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"Kinney, Michael D" <michael.d.kinney@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
Date: Fri, 30 Aug 2019 09:14:48 +0000 [thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E4E1373@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <B80AF82E9BFB8E4FBD8C89DA810C6A093C92324F@SHSMSX104.ccr.corp.intel.com>
UefiScsiLib is designed for the convenient usage with SCSI commands. They should try to align to UEFI definition.
If you check current SCSI APIs, their interface matches EFI_SCSI_IO_SCSI_REQUEST_PACKET strut.
So, new added APIs had better match EFI_STORAGE_SECURITY_COMMAND_PROTOCOL.
For the change in MdePkg\Include\Protocol\ScsiIo.h, where is new definition EFI_SCSI_IO_TYPE_WLUN from?
Thanks
Liming
>-----Original Message-----
>From: Wu, Hao A
>Sent: Friday, August 30, 2019 1:18 PM
>To: Zurcher, Christopher J <christopher.j.zurcher@intel.com>;
>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
>
>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 9:14 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
2019-08-30 9:14 ` Liming Gao [this message]
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=4A89E2EF3DFEDB4C8BFDE51014F606A14E4E1373@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