> -----Original Message----- > From: Gao, Liming > Sent: Monday, September 02, 2019 9:47 AM > To: Wu, Hao A; Zurcher, Christopher J; devel@edk2.groups.io; Kinney, > Michael D > Cc: Yao, Jiewen; Wang, Jian J > Subject: RE: [edk2-devel] [PATCH v5 1/4] MdePkg: Implement SCSI > commands for Security Protocol In/Out > > Hao: > I add my comments. > > Thanks > Liming > > -----Original Message----- > > From: Wu, Hao A > > Sent: Monday, September 2, 2019 9:11 AM > > To: Gao, Liming <liming....@intel.com>; Zurcher, Christopher J > <christopher.j.zurc...@intel.com>; devel@edk2.groups.io; Kinney, > > Michael D <michael.d.kin...@intel.com> > > Cc: Yao, Jiewen <jiewen....@intel.com>; Wang, Jian J > <jian.j.w...@intel.com> > > Subject: RE: [edk2-devel] [PATCH v5 1/4] MdePkg: Implement SCSI > commands for Security Protocol In/Out > > > > > -----Original Message----- > > > From: Gao, Liming > > > Sent: Friday, August 30, 2019 5:15 PM > > > To: Wu, Hao A; Zurcher, Christopher J; devel@edk2.groups.io; Kinney, > > > Michael D > > > 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 > > > > > > 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? > > > > > > Hello Liming, > > > > The macro "EFI_SCSI_IO_TYPE_WLUN" comes from the SCSI Primary > Commands standard > > (SPC), just like other existing definitions listed together in the header > > file. > I check UEFI spec. Other existing definitions are defined in UEFI spec. > So, I think new added one should be proposed to UEFI spec for ScsiIo.h. > I am OK to the change in IndustryStandard/Scsi.h. Can the consumer code > get > new definition from IndustryStandard/Scsi.h instead of Protocol/ScsiIo.h? > If so, this patch doesn't need to update Protocol/ScsiIo.h.
Yes. I think the one "EFI_SCSI_IO_TYPE_WLUN" added in ScsiIo.h is not consumed by the proposed patch. So I think removing it should not impact the functionality. (Actually, all the "EFI_SCSI_IO_TYPE_XXX" macros seem not being used within edk2/edk2-platforms repositories at this moment.) Hello Christopher Zurcher, Could you help to double confirm? If "EFI_SCSI_IO_TYPE_WLUN" is not used, could you help to remove all the changes made to file ScsiIo.h from the proposed patch? I think we can update it later when the UEFI spec got updated. Best Regards, Hao Wu > > Thanks > Liming > > > > Best Regards, > > Hao Wu > > > > > > > > > > Thanks > > > Liming > > > >-----Original Message----- > > > >From: Wu, Hao A > > > >Sent: Friday, August 30, 2019 1:18 PM > > > >To: Zurcher, Christopher J <christopher.j.zurc...@intel.com>; > > > >devel@edk2.groups.io; Gao, Liming <liming....@intel.com>; Kinney, > > > Michael > > > >D <michael.d.kin...@intel.com> > > > >Cc: Yao, Jiewen <jiewen....@intel.com>; Wang, Jian J > > > ><jian.j.w...@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.zurc...@intel.com> > > > >> Cc: Yao, Jiewen <jiewen....@intel.com>; Wang, Jian J > > > >> <jian.j.w...@intel.com>; Gao, Liming <liming....@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....@intel.com> > > > >> > Cc: Jian J Wang <jian.j.w...@intel.com> > > > >> > Cc: Liming Gao <liming....@intel.com> > > > >> > Signed-off-by: Christopher J Zurcher > <christopher.j.zurc...@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 > > > >> > > > > >> > > > > >> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46654): https://edk2.groups.io/g/devel/message/46654 Mute This Topic: https://groups.io/mt/32994942/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
