[Patch 0/3] Enable Pyrite 2.0 for opal driver.

[Patch 0/3] Enable Pyrite 2.0 for opal driver.

[Eric Dong]

Eanble the pyrite 2.0 devices for opal driver.

Eric Dong (3):
  MdePkg: Add Feature definitions add in pyrite 2.0 spec.
  SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0 spec.
  SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.

 MdePkg/Include/IndustryStandard/TcgStorageCore.h   |   2 +
 MdePkg/Include/IndustryStandard/TcgStorageOpal.h   |  54 +++
 SecurityPkg/Include/Library/TcgStorageOpalLib.h    |  41 ++
 .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 426 ++++++++++++++++++---
 .../TcgStorageOpalLib/TcgStorageOpalLib.inf        |   1 +
 .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  98 +++++
 .../Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 214 ++++++++++-
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c     |  60 ++-
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h     |   9 +
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c        |  84 ++++
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.c        |   1 +
 11 files changed, 934 insertions(+), 56 deletions(-)
 create mode 100644 SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h

-- 
2.15.0.windows.1

[Patch 1/3] MdePkg: Add Feature definitions add in pyrite 2.0 spec.

[Eric Dong]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 MdePkg/Include/IndustryStandard/TcgStorageCore.h |  2 +
 MdePkg/Include/IndustryStandard/TcgStorageOpal.h | 54 ++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/TcgStorageCore.h b/MdePkg/Include/IndustryStandard/TcgStorageCore.h
index 56ea92f2eb..6d80da2401 100644
--- a/MdePkg/Include/IndustryStandard/TcgStorageCore.h
+++ b/MdePkg/Include/IndustryStandard/TcgStorageCore.h
@@ -228,7 +228,9 @@ typedef enum {
 #define TCG_FEATURE_OPAL_SSC_V2_0_0     (UINT16)0x0203
 #define TCG_FEATURE_OPAL_SSC_LITE       (UINT16)0x0301
 #define TCG_FEATURE_PYRITE_SSC          (UINT16)0x0302
+#define TCG_FEATURE_PYRITE_SSC_V2_0_0   (UINT16)0x0303
 #define TCG_FEATURE_BLOCK_SID           (UINT16)0x0402
+#define TCG_FEATURE_DATA_REMOVAL        (UINT16)0x0404
 
 // ACE Expression values
 #define TCG_ACE_EXPRESSION_AND 0x0
diff --git a/MdePkg/Include/IndustryStandard/TcgStorageOpal.h b/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
index 91d5008c05..8ff36efe50 100644
--- a/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
+++ b/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
@@ -34,6 +34,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define OPAL_ADMIN_SP_ACTIVATE_METHOD       TCG_TO_UID(0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x02, 0x03)
 #define OPAL_ADMIN_SP_REVERT_METHOD         TCG_TO_UID(0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x02, 0x02)
 
+// ADMIN_SP
+// Data Removal mechanism
+#define OPAL_UID_ADMIN_SP_DATA_REMOVAL_MECHANISM  TCG_TO_UID(0x00, 0x00, 0x11, 0x01, 0x00, 0x00, 0x00, 0x01)
 
 // LOCKING SP
 // Authorities
@@ -93,6 +96,23 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define OPAL_LOCKING_SP_C_PIN_TRYLIMIT_COL 5
 #define OPAL_RANDOM_METHOD_MAX_COUNT_SIZE 32
 
+// Data Removal Mechanism column.
+#define OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL  1
+
+//
+// Supported Data Removal Mechanism.
+// Detail see Pyrite SSC v2 spec.
+//
+typedef enum {
+  OverwriteDataErase = 0,
+  BlockErase,
+  CryptoErase,
+  Unmap,
+  ResetWritePointers,
+  VendorSpecificErase,
+  ResearvedMechanism
+} SUPPORTED_DATA_REMOVAL_MECHANISM;
+
 #pragma pack(1)
 
 typedef struct _OPAL_GEOMETRY_REPORTING_FEATURE {
@@ -162,6 +182,38 @@ typedef struct _PYRITE_SSC_FEATURE_DESCRIPTOR {
   UINT8                                Future[5];
 } PYRITE_SSC_FEATURE_DESCRIPTOR;
 
+typedef struct _PYRITE_SSCV2_FEATURE_DESCRIPTOR {
+  TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER Header;
+  UINT16                               BaseComdIdBE;
+  UINT16                               NumComIdsBE;
+  UINT8                                Reserved[5];
+  UINT8                                InitialCPINSIDPIN;
+  UINT8                                CPINSIDPINRevertBehavior;
+  UINT8                                Future[5];
+} PYRITE_SSCV2_FEATURE_DESCRIPTOR;
+
+typedef struct _DATA_REMOVAL_FEATURE_DESCRIPTOR {
+  TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER Header;
+  UINT8                                Reserved;
+  UINT8                                OperationProcessing : 1;
+  UINT8                                Reserved2 : 7;
+  UINT8                                RemovalMechanism;
+  UINT8                                FormatBit0 : 1;   // Data Removal Time Format for Bit 0
+  UINT8                                FormatBit1 : 1;   // Data Removal Time Format for Bit 1
+  UINT8                                FormatBit2 : 1;   // Data Removal Time Format for Bit 2
+  UINT8                                FormatBit3 : 1;   // Data Removal Time Format for Bit 3
+  UINT8                                FormatBit4 : 1;   // Data Removal Time Format for Bit 4
+  UINT8                                FormatBit5 : 1;   // Data Removal Time Format for Bit 5
+  UINT8                                Reserved3 : 2;
+  UINT16                               TimeBit0;         // Data Removal Time for Supported Data Removal Mechanism Bit 0
+  UINT16                               TimeBit1;         // Data Removal Time for Supported Data Removal Mechanism Bit 1
+  UINT16                               TimeBit2;         // Data Removal Time for Supported Data Removal Mechanism Bit 2
+  UINT16                               TimeBit3;         // Data Removal Time for Supported Data Removal Mechanism Bit 3
+  UINT16                               TimeBit4;         // Data Removal Time for Supported Data Removal Mechanism Bit 4
+  UINT16                               TimeBit5;         // Data Removal Time for Supported Data Removal Mechanism Bit 5
+  UINT8                                Future[16];
+} DATA_REMOVAL_FEATURE_DESCRIPTOR;
+
 typedef union {
   TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER     CommonHeader;
   TCG_TPER_FEATURE_DESCRIPTOR              Tper;
@@ -173,7 +225,9 @@ typedef union {
   OPAL_SSCV2_FEATURE_DESCRIPTOR            OpalSscV2;
   OPAL_SSCLITE_FEATURE_DESCRIPTOR          OpalSscLite;
   PYRITE_SSC_FEATURE_DESCRIPTOR            PyriteSsc;
+  PYRITE_SSCV2_FEATURE_DESCRIPTOR          PyriteSscV2;
   TCG_BLOCK_SID_FEATURE_DESCRIPTOR         BlockSid;
+  DATA_REMOVAL_FEATURE_DESCRIPTOR          DataRemoval;
 } OPAL_LEVEL0_FEATURE_DESCRIPTOR;
 
 #pragma pack()
-- 
2.15.0.windows.1

Re: [Patch 1/3] MdePkg: Add Feature definitions add in pyrite 2.0 spec.

[Wu, Hao A]

Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> -----Original Message-----
> From: Dong, Eric
> Sent: Thursday, May 03, 2018 11:17 AM
> To: edk2-devel@lists.01.org; Wu, Hao A
> Subject: [Patch 1/3] MdePkg: Add Feature definitions add in pyrite 2.0 spec.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  MdePkg/Include/IndustryStandard/TcgStorageCore.h |  2 +
>  MdePkg/Include/IndustryStandard/TcgStorageOpal.h | 54
> ++++++++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/MdePkg/Include/IndustryStandard/TcgStorageCore.h
> b/MdePkg/Include/IndustryStandard/TcgStorageCore.h
> index 56ea92f2eb..6d80da2401 100644
> --- a/MdePkg/Include/IndustryStandard/TcgStorageCore.h
> +++ b/MdePkg/Include/IndustryStandard/TcgStorageCore.h
> @@ -228,7 +228,9 @@ typedef enum {
>  #define TCG_FEATURE_OPAL_SSC_V2_0_0     (UINT16)0x0203
>  #define TCG_FEATURE_OPAL_SSC_LITE       (UINT16)0x0301
>  #define TCG_FEATURE_PYRITE_SSC          (UINT16)0x0302
> +#define TCG_FEATURE_PYRITE_SSC_V2_0_0   (UINT16)0x0303
>  #define TCG_FEATURE_BLOCK_SID           (UINT16)0x0402
> +#define TCG_FEATURE_DATA_REMOVAL        (UINT16)0x0404
> 
>  // ACE Expression values
>  #define TCG_ACE_EXPRESSION_AND 0x0
> diff --git a/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
> b/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
> index 91d5008c05..8ff36efe50 100644
> --- a/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
> +++ b/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
> @@ -34,6 +34,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define OPAL_ADMIN_SP_ACTIVATE_METHOD       TCG_TO_UID(0x00, 0x00,
> 0x00, 0x06, 0x00, 0x00, 0x02, 0x03)
>  #define OPAL_ADMIN_SP_REVERT_METHOD         TCG_TO_UID(0x00, 0x00,
> 0x00, 0x06, 0x00, 0x00, 0x02, 0x02)
> 
> +// ADMIN_SP
> +// Data Removal mechanism
> +#define OPAL_UID_ADMIN_SP_DATA_REMOVAL_MECHANISM
> TCG_TO_UID(0x00, 0x00, 0x11, 0x01, 0x00, 0x00, 0x00, 0x01)
> 
>  // LOCKING SP
>  // Authorities
> @@ -93,6 +96,23 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define OPAL_LOCKING_SP_C_PIN_TRYLIMIT_COL 5
>  #define OPAL_RANDOM_METHOD_MAX_COUNT_SIZE 32
> 
> +// Data Removal Mechanism column.
> +#define OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL  1
> +
> +//
> +// Supported Data Removal Mechanism.
> +// Detail see Pyrite SSC v2 spec.
> +//
> +typedef enum {
> +  OverwriteDataErase = 0,
> +  BlockErase,
> +  CryptoErase,
> +  Unmap,
> +  ResetWritePointers,
> +  VendorSpecificErase,
> +  ResearvedMechanism
> +} SUPPORTED_DATA_REMOVAL_MECHANISM;
> +
>  #pragma pack(1)
> 
>  typedef struct _OPAL_GEOMETRY_REPORTING_FEATURE {
> @@ -162,6 +182,38 @@ typedef struct _PYRITE_SSC_FEATURE_DESCRIPTOR {
>    UINT8                                Future[5];
>  } PYRITE_SSC_FEATURE_DESCRIPTOR;
> 
> +typedef struct _PYRITE_SSCV2_FEATURE_DESCRIPTOR {
> +  TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER Header;
> +  UINT16                               BaseComdIdBE;
> +  UINT16                               NumComIdsBE;
> +  UINT8                                Reserved[5];
> +  UINT8                                InitialCPINSIDPIN;
> +  UINT8                                CPINSIDPINRevertBehavior;
> +  UINT8                                Future[5];
> +} PYRITE_SSCV2_FEATURE_DESCRIPTOR;
> +
> +typedef struct _DATA_REMOVAL_FEATURE_DESCRIPTOR {
> +  TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER Header;
> +  UINT8                                Reserved;
> +  UINT8                                OperationProcessing : 1;
> +  UINT8                                Reserved2 : 7;
> +  UINT8                                RemovalMechanism;
> +  UINT8                                FormatBit0 : 1;   // Data Removal Time Format for
> Bit 0
> +  UINT8                                FormatBit1 : 1;   // Data Removal Time Format for
> Bit 1
> +  UINT8                                FormatBit2 : 1;   // Data Removal Time Format for
> Bit 2
> +  UINT8                                FormatBit3 : 1;   // Data Removal Time Format for
> Bit 3
> +  UINT8                                FormatBit4 : 1;   // Data Removal Time Format for
> Bit 4
> +  UINT8                                FormatBit5 : 1;   // Data Removal Time Format for
> Bit 5
> +  UINT8                                Reserved3 : 2;
> +  UINT16                               TimeBit0;         // Data Removal Time for
> Supported Data Removal Mechanism Bit 0
> +  UINT16                               TimeBit1;         // Data Removal Time for
> Supported Data Removal Mechanism Bit 1
> +  UINT16                               TimeBit2;         // Data Removal Time for
> Supported Data Removal Mechanism Bit 2
> +  UINT16                               TimeBit3;         // Data Removal Time for
> Supported Data Removal Mechanism Bit 3
> +  UINT16                               TimeBit4;         // Data Removal Time for
> Supported Data Removal Mechanism Bit 4
> +  UINT16                               TimeBit5;         // Data Removal Time for
> Supported Data Removal Mechanism Bit 5
> +  UINT8                                Future[16];
> +} DATA_REMOVAL_FEATURE_DESCRIPTOR;
> +
>  typedef union {
>    TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER     CommonHeader;
>    TCG_TPER_FEATURE_DESCRIPTOR              Tper;
> @@ -173,7 +225,9 @@ typedef union {
>    OPAL_SSCV2_FEATURE_DESCRIPTOR            OpalSscV2;
>    OPAL_SSCLITE_FEATURE_DESCRIPTOR          OpalSscLite;
>    PYRITE_SSC_FEATURE_DESCRIPTOR            PyriteSsc;
> +  PYRITE_SSCV2_FEATURE_DESCRIPTOR          PyriteSscV2;
>    TCG_BLOCK_SID_FEATURE_DESCRIPTOR         BlockSid;
> +  DATA_REMOVAL_FEATURE_DESCRIPTOR          DataRemoval;
>  } OPAL_LEVEL0_FEATURE_DESCRIPTOR;
> 
>  #pragma pack()
> --
> 2.15.0.windows.1

[Patch 2/3] SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0 spec.

[Eric Dong]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 SecurityPkg/Include/Library/TcgStorageOpalLib.h    |  41 ++
 .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 426 ++++++++++++++++++---
 .../TcgStorageOpalLib/TcgStorageOpalLib.inf        |   1 +
 .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  98 +++++
 .../Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 214 ++++++++++-
 5 files changed, 731 insertions(+), 49 deletions(-)
 create mode 100644 SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h

diff --git a/SecurityPkg/Include/Library/TcgStorageOpalLib.h b/SecurityPkg/Include/Library/TcgStorageOpalLib.h
index 9b64a8e5cd..2947c0eaf1 100644
--- a/SecurityPkg/Include/Library/TcgStorageOpalLib.h
+++ b/SecurityPkg/Include/Library/TcgStorageOpalLib.h
@@ -82,6 +82,15 @@ typedef struct {
     //
     UINT32 BlockSid : 1;
 
+    //
+    // Pyrite SSC V2 support  (0 - not supported, 1 - supported)
+    //
+    UINT32 PyriteSscV2 : 1;
+
+    //
+    // Supported Data Removal Mechanism support  (0 - not supported, 1 - supported)
+    //
+    UINT32 DataRemoval : 1;
 } OPAL_DISK_SUPPORT_ATTRIBUTE;
 
 //
@@ -834,4 +843,36 @@ OpalUtilAdminPasswordExists(
   IN  TCG_LOCKING_FEATURE_DESCRIPTOR   *LockingFeature
   );
 
+/**
+  Get Active Data Removal Mechanism Value.
+
+  @param[in]      Session,                       The session info for one opal device.
+  @param[in]      GeneratedSid                   Generated SID of disk
+  @param[in]      SidLength                      Length of generatedSid in bytes
+  @param[out]     ActiveDataRemovalMechanism     Return the active data removal mechanism.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalUtilGetActiveDataRemovalMechanism (
+  OPAL_SESSION      *Session,
+  const VOID        *GeneratedSid,
+  UINT32            SidLength,
+  UINT8             *ActiveDataRemovalMechanism
+  );
+
+/**
+  Get the supported Data Removal Mechanism list.
+
+  @param[in]      Session,                       The session info for one opal device.
+  @param[out]     RemovalMechanismLists          Return the supported data removal mechanism lists.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalUtilGetDataRemovalMechanismLists (
+  IN  OPAL_SESSION      *Session,
+  OUT UINT32            *RemovalMechanismLists
+  );
+
 #endif // _OPAL_CORE_H_
diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
index 90cc51a24c..d031ebe798 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
@@ -1,7 +1,7 @@
 /** @file
   Public API for Opal Core library.
 
-Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/DebugLib.h>
 #include <Library/TcgStorageOpalLib.h>
 
+#include "TcgStorageOpalLibInternal.h"
+
 #pragma pack(1)
 typedef struct {
     UINT8 HardwareReset : 1;
@@ -89,6 +91,7 @@ OpalTrustedSend(
   @param[in]      SpSpecific            Security Protocol Specific
   @param[in]      Buffer                Address of Data to transfer
   @param[in]      BufferSize            Full Size of Buffer, including space that may be used for padding.
+  @param[in]      EstimateTimeCost      Estimate the time needed.
 
 **/
 TCG_RESULT
@@ -98,10 +101,10 @@ OpalTrustedRecv(
   UINT8                                  SecurityProtocol,
   UINT16                                 SpSpecific,
   VOID                                   *Buffer,
-  UINTN                                  BufferSize
+  UINTN                                  BufferSize,
+  UINT32                                 EstimateTimeCost
   )
 {
-
   UINTN           TransferLength512;
   UINT32          Tries;
   TCG_COM_PACKET  *ComPacket;
@@ -129,9 +132,15 @@ OpalTrustedRecv(
   // so we need to retry the IF-RECV to get the actual Data.
   // See TCG Core Spec v2 Table 45 IF-RECV ComPacket Field Values Summary
   // This is an arbitrary number of retries, not from the spec.
-  // have a max timeout of 10 seconds, 5000 tries * 2ms = 10s
   //
-  Tries = 5000;
+  // if user input estimate time cost(second level) value bigger than 10s, base on user input value to wait.
+  // Else, Use a max timeout of 10 seconds to wait, 5000 tries * 2ms = 10s
+  //
+  if (EstimateTimeCost > 10) {
+    Tries = EstimateTimeCost * 500; // 500 = 1000 * 1000 / 2000;
+  } else {
+    Tries = 5000;
+  }
   while ((Tries--) > 0) {
     ZeroMem( Buffer, BufferSize );
     TransferSize = 0;
@@ -146,7 +155,6 @@ OpalTrustedRecv(
                  Buffer,
                  &TransferSize
              );
-
     if (EFI_ERROR (Status)) {
       return TcgResultFailure;
     }
@@ -179,23 +187,24 @@ OpalTrustedRecv(
 /**
   The function performs send, recv, check comIDs, check method status action.
 
-  @param[in]      Session         OPAL_SESSION related to this method..
-  @param[in]      SendSize        Transfer Length of Buffer (in bytes) - always a multiple of 512
-  @param[in]      Buffer          Address of Data to transfer
-  @param[in]      BufferSize      Full Size of Buffer, including space that may be used for padding.
-  @param[in]      ParseStruct     Structure used to parse received TCG response.
-  @param[in]      MethodStatus    Method status of last action performed.  If action succeeded, it should be TCG_METHOD_STATUS_CODE_SUCCESS.
-
+  @param[in]      Session           OPAL_SESSION related to this method..
+  @param[in]      SendSize          Transfer Length of Buffer (in bytes) - always a multiple of 512
+  @param[in]      Buffer            Address of Data to transfer
+  @param[in]      BufferSize        Full Size of Buffer, including space that may be used for padding.
+  @param[in]      ParseStruct       Structure used to parse received TCG response.
+  @param[in]      MethodStatus      Method status of last action performed.  If action succeeded, it should be TCG_METHOD_STATUS_CODE_SUCCESS.
+  @param[in]      EstimateTimeCost  Estimate the time need to for the method.
 **/
 TCG_RESULT
 EFIAPI
-OpalPerformMethod(
+OpalPerformMethod (
   OPAL_SESSION     *Session,
   UINT32           SendSize,
   VOID             *Buffer,
   UINT32           BufferSize,
   TCG_PARSE_STRUCT *ParseStruct,
-  UINT8            *MethodStatus
+  UINT8            *MethodStatus,
+  UINT32           EstimateTimeCost
   )
 {
   NULL_CHECK(Session);
@@ -217,7 +226,8 @@ OpalPerformMethod(
                   TCG_OPAL_SECURITY_PROTOCOL_1,
                   Session->OpalBaseComId,
                   Buffer,
-                  BufferSize
+                  BufferSize,
+                  EstimateTimeCost
               ));
 
   ERROR_CHECK(TcgInitTcgParseStruct(ParseStruct, Buffer, BufferSize));
@@ -309,7 +319,60 @@ OpalPsidRevert(
   //
   // Send Revert Method Call
   //
-  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer, BUFFER_SIZE, &ParseStruct, &MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer, BUFFER_SIZE, &ParseStruct, &MethodStatus, 0));
+  METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
+
+  return TcgResultSuccess;
+}
+
+/**
+
+  Reverts device using Admin SP Revert method.
+
+  @param[in]  AdminSpSession      OPAL_SESSION with OPAL_UID_ADMIN_SP as OPAL_ADMIN_SP_PSID_AUTHORITY to perform PSID revert.
+  @param[in]  EstimateTimeCost    Estimate the time needed.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalPyrite2PsidRevert(
+  OPAL_SESSION              *AdminSpSession,
+  UINT32                    EstimateTimeCost
+  )
+{
+  //
+  // Now that base comid is known, start Session
+  // we'll attempt to start Session as PSID authority
+  // verify PSID Authority is defined in ADMIN SP authority table... is this possible?
+  //
+  TCG_CREATE_STRUCT  CreateStruct;
+  TCG_PARSE_STRUCT   ParseStruct;
+  UINT32             Size;
+  UINT8              Buffer[BUFFER_SIZE];
+  UINT8              MethodStatus;
+
+
+  NULL_CHECK(AdminSpSession);
+
+  //
+  // Send Revert action on Admin SP
+  //
+  ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buffer, BUFFER_SIZE));
+  ERROR_CHECK(TcgStartComPacket(&CreateStruct, AdminSpSession->OpalBaseComId, AdminSpSession->ComIdExtension));
+  ERROR_CHECK(TcgStartPacket(&CreateStruct, AdminSpSession->TperSessionId, AdminSpSession->HostSessionId, 0x0, 0x0, 0x0));
+  ERROR_CHECK(TcgStartSubPacket(&CreateStruct, 0x0));
+  ERROR_CHECK(TcgStartMethodCall(&CreateStruct, OPAL_UID_ADMIN_SP, OPAL_ADMIN_SP_REVERT_METHOD));
+  ERROR_CHECK(TcgStartParameters(&CreateStruct));
+  ERROR_CHECK(TcgEndParameters(&CreateStruct));
+  ERROR_CHECK(TcgEndMethodCall(&CreateStruct));
+  ERROR_CHECK(TcgEndSubPacket(&CreateStruct));
+  ERROR_CHECK(TcgEndPacket(&CreateStruct));
+  ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
+
+  //
+  // Send Revert Method Call
+  //
+  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer, BUFFER_SIZE, &ParseStruct, &MethodStatus, EstimateTimeCost));
   METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
 
   return TcgResultSuccess;
@@ -339,7 +402,8 @@ OpalRetrieveLevel0DiscoveryHeader(
               TCG_OPAL_SECURITY_PROTOCOL_1,   // SP
               TCG_SP_SPECIFIC_PROTOCOL_LEVEL0_DISCOVERY, // SP_Specific
               BuffAddress,
-              BufferSize
+              BufferSize,
+              0
             ));
 }
 
@@ -367,7 +431,8 @@ OpalRetrieveSupportedProtocolList(
               TCG_SECURITY_PROTOCOL_INFO, // SP
               TCG_SP_SPECIFIC_PROTOCOL_LIST, // SP_Specific
               BuffAddress,
-              BufferSize
+              BufferSize,
+              0
           ));
 }
 
@@ -430,7 +495,7 @@ OpalStartSession(
                     HostChallenge,
                     HostSigningAuthority
                 ));
-  ERROR_CHECK(OpalPerformMethod(Session, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(Session, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
   if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
     return TcgResultSuccess; // return early if method failed - user must check MethodStatus
   }
@@ -487,7 +552,8 @@ OpalEndSession(
                   TCG_OPAL_SECURITY_PROTOCOL_1,
                   Session->OpalBaseComId,
                   Buffer,
-                  sizeof(Buffer)
+                  sizeof(Buffer),
+                  0
               ));
 
   ERROR_CHECK(TcgInitTcgParseStruct(&ParseStruct, Buffer, sizeof(Buffer)));
@@ -558,7 +624,7 @@ OpalGetMsid(
   //
   // Send MSID Method Call
   //
-  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer, BUFFER_SIZE, &ParseStruct, &MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer, BUFFER_SIZE, &ParseStruct, &MethodStatus, 0));
   METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
 
   ERROR_CHECK(TcgGetNextStartList(&ParseStruct));
@@ -592,6 +658,86 @@ OpalGetMsid(
   return TcgResultSuccess;
 }
 
+/**
+
+  The function retrieves the MSID from the device specified
+
+  @param[in]  AdminSpSession              OPAL_SESSION with OPAL_UID_ADMIN_SP as OPAL_ADMIN_SP_ANYBODY_AUTHORITY
+  @param[out] ActiveDataRemovalMechanism  Active Data Removal Mechanism that the device will use for Revert/RevertSP calls.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalPyrite2GetActiveDataRemovalMechanism (
+  IN  OPAL_SESSION    *AdminSpSession,
+  OUT UINT8           *ActiveDataRemovalMechanism
+  )
+{
+  TCG_CREATE_STRUCT CreateStruct;
+  TCG_PARSE_STRUCT  ParseStruct;
+  UINT32            Size;
+  UINT8             MethodStatus;
+  UINT32            Col;
+  UINT8             RecvActiveDataRemovalMechanism;
+  UINT8             Buffer[BUFFER_SIZE];
+
+  NULL_CHECK(AdminSpSession);
+  NULL_CHECK(ActiveDataRemovalMechanism);
+
+  ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buffer, BUFFER_SIZE));
+  ERROR_CHECK(TcgStartComPacket(&CreateStruct, AdminSpSession->OpalBaseComId, AdminSpSession->ComIdExtension));
+  ERROR_CHECK(TcgStartPacket(&CreateStruct, AdminSpSession->TperSessionId, AdminSpSession->HostSessionId, 0x0, 0x0, 0x0));
+  ERROR_CHECK(TcgStartSubPacket(&CreateStruct, 0x0));
+  ERROR_CHECK(TcgStartMethodCall(&CreateStruct, OPAL_UID_ADMIN_SP_DATA_REMOVAL_MECHANISM, TCG_UID_METHOD_GET));
+  ERROR_CHECK(TcgStartParameters(&CreateStruct));
+  ERROR_CHECK(TcgAddStartList(&CreateStruct));
+  ERROR_CHECK(TcgAddStartName(&CreateStruct));
+  ERROR_CHECK(TcgAddUINT8(&CreateStruct, TCG_CELL_BLOCK_START_COLUMN_NAME));
+  ERROR_CHECK(TcgAddUINT8(&CreateStruct, OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL));
+  ERROR_CHECK(TcgAddEndName(&CreateStruct));
+  ERROR_CHECK(TcgAddStartName(&CreateStruct));
+  ERROR_CHECK(TcgAddUINT8(&CreateStruct, TCG_CELL_BLOCK_END_COLUMN_NAME));
+  ERROR_CHECK(TcgAddUINT8(&CreateStruct, OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL));
+  ERROR_CHECK(TcgAddEndName(&CreateStruct));
+  ERROR_CHECK(TcgAddEndList(&CreateStruct));
+  ERROR_CHECK(TcgEndParameters(&CreateStruct));
+  ERROR_CHECK(TcgEndMethodCall(&CreateStruct));
+  ERROR_CHECK(TcgEndSubPacket(&CreateStruct));
+  ERROR_CHECK(TcgEndPacket(&CreateStruct));
+  ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
+
+  //
+  // Send Get Active Data Removal Mechanism Method Call
+  //
+  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer, BUFFER_SIZE, &ParseStruct, &MethodStatus, 0));
+  METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
+
+  ERROR_CHECK(TcgGetNextStartList(&ParseStruct));
+  ERROR_CHECK(TcgGetNextStartList(&ParseStruct));
+  ERROR_CHECK(TcgGetNextStartName(&ParseStruct));
+  ERROR_CHECK(TcgGetNextUINT32(&ParseStruct, &Col));
+  ERROR_CHECK(TcgGetNextUINT8(&ParseStruct, &RecvActiveDataRemovalMechanism));
+  ERROR_CHECK(TcgGetNextEndName(&ParseStruct));
+  ERROR_CHECK(TcgGetNextEndList(&ParseStruct));
+  ERROR_CHECK(TcgGetNextEndList(&ParseStruct));
+  ERROR_CHECK(TcgGetNextEndOfData(&ParseStruct));
+
+  if (Col != OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL) {
+    DEBUG ((DEBUG_INFO, "ERROR: got col %u, expected %u\n", Col, OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL));
+    return TcgResultFailure;
+  }
+
+  if (RecvActiveDataRemovalMechanism >= ResearvedMechanism) {
+    return TcgResultFailure;
+  }
+
+  //
+  // Copy active data removal mechanism into Buffer
+  //
+  CopyMem(ActiveDataRemovalMechanism, &RecvActiveDataRemovalMechanism, sizeof(RecvActiveDataRemovalMechanism));
+  return TcgResultSuccess;
+}
+
 /**
 
   The function calls the Admin SP RevertSP method on the Locking SP.  If KeepUserData is True, then the optional parameter
@@ -666,7 +812,104 @@ OpalAdminRevert(
   //
   // Send RevertSP method call
   //
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
+
+  //
+  // Session is immediately ended by device after successful revertsp, so no need to end Session
+  //
+  if (*MethodStatus == TCG_METHOD_STATUS_CODE_SUCCESS) {
+    //
+    // Caller should take ownership again
+    //
+    return TcgResultSuccess;
+  } else {
+    //
+    // End Session
+    //
+    METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);     // exit with success on method failure - user must inspect MethodStatus
+  }
+
+  return TcgResultSuccess;
+}
+
+
+/**
+
+  The function calls the Admin SP RevertSP method on the Locking SP.  If KeepUserData is True, then the optional parameter
+  to keep the user Data is set to True, otherwise the optional parameter is not provided.
+
+  @param[in]      LockingSpSession    OPAL_SESSION with OPAL_UID_LOCKING_SP as OPAL_LOCKING_SP_ADMIN1_AUTHORITY to revertSP
+  @param[in]      KeepUserData        Specifies whether or not to keep user Data when performing RevertSP action. True = keeps user Data.
+  @param[in/out]  MethodStatus        Method status of last action performed.  If action succeeded, it should be TCG_METHOD_STATUS_CODE_SUCCESS.
+  @param[in]      EstimateTimeCost    Estimate the time needed.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalPyrite2AdminRevert(
+  OPAL_SESSION    *LockingSpSession,
+  BOOLEAN         KeepUserData,
+  UINT8           *MethodStatus,
+  UINT32          EstimateTimeCost
+  )
+{
+  UINT8             Buf[BUFFER_SIZE];
+  TCG_CREATE_STRUCT CreateStruct;
+  UINT32            Size;
+  TCG_PARSE_STRUCT  ParseStruct;
+  TCG_RESULT        Ret;
+
+  NULL_CHECK(LockingSpSession);
+  NULL_CHECK(MethodStatus);
+
+  //
+  // ReadLocked or WriteLocked must be False (per Opal spec) to guarantee revertSP can keep user Data
+  //
+  if (KeepUserData) {
+    //
+    // set readlocked and writelocked to false
+    //
+    Ret = OpalUpdateGlobalLockingRange(
+                        LockingSpSession,
+                        FALSE,
+                        FALSE,
+                        MethodStatus);
+
+    if (Ret != TcgResultSuccess || *MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
+      //
+      // bail out
+      //
+      return Ret;
+    }
+  }
+
+  ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buf, sizeof(Buf)));
+  ERROR_CHECK(TcgStartComPacket(&CreateStruct, LockingSpSession->OpalBaseComId, LockingSpSession->ComIdExtension));
+  ERROR_CHECK(TcgStartPacket(&CreateStruct, LockingSpSession->TperSessionId, LockingSpSession->HostSessionId, 0x0, 0x0, 0x0));
+  ERROR_CHECK(TcgStartSubPacket(&CreateStruct, 0x0));
+  ERROR_CHECK(TcgStartMethodCall(&CreateStruct, TCG_UID_THIS_SP, OPAL_LOCKING_SP_REVERTSP_METHOD));
+  ERROR_CHECK(TcgStartParameters(&CreateStruct));
+
+  if (KeepUserData) {
+    //
+    // optional parameter to keep Data after revert
+    //
+    ERROR_CHECK(TcgAddStartName(&CreateStruct));
+    ERROR_CHECK(TcgAddUINT32(&CreateStruct, 0x060000));      // weird Value but that's what spec says
+    ERROR_CHECK(TcgAddBOOLEAN(&CreateStruct, KeepUserData));
+    ERROR_CHECK(TcgAddEndName(&CreateStruct));
+  }
+
+  ERROR_CHECK(TcgEndParameters(&CreateStruct));
+  ERROR_CHECK(TcgEndMethodCall(&CreateStruct));
+  ERROR_CHECK(TcgEndSubPacket(&CreateStruct));
+  ERROR_CHECK(TcgEndPacket(&CreateStruct));
+  ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
+
+  //
+  // Send RevertSP method call
+  //
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, EstimateTimeCost));
 
   //
   // Session is immediately ended by device after successful revertsp, so no need to end Session
@@ -729,7 +972,7 @@ OpalActivateLockingSp(
   //
   // Send Activate method call
   //
-  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
   METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess); // exit with success on method failure - user must inspect MethodStatus
 
   return TcgResultSuccess;
@@ -778,7 +1021,7 @@ OpalSetPassword(
                          NewPinLength
                          ));
 
-  ERROR_CHECK(OpalPerformMethod(Session, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(Session, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
   // exit with success on method failure - user must inspect MethodStatus
   METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);
 
@@ -831,7 +1074,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
                   AuthorityUid,
                   TRUE));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
   if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
     DEBUG ((DEBUG_INFO, "Send Set Authority error\n"));
@@ -851,7 +1094,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
                   NewPin,
                   NewPinLength));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
   //
   // allow user1 to set global range to unlocked/locked by modifying ACE_Locking_GlobalRange_SetRdLocked/SetWrLocked
@@ -870,7 +1113,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
                   OPAL_LOCKING_SP_ADMINS_AUTHORITY
               ));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
   if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
     DEBUG ((DEBUG_INFO, "Update ACE for RDLOCKED failed\n"));
@@ -891,7 +1134,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
                   OPAL_LOCKING_SP_ADMINS_AUTHORITY
               ));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
   if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
     DEBUG ((DEBUG_INFO, "Update ACE for WRLOCKED failed\n"));
@@ -900,7 +1143,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
 
   ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buf, sizeof(Buf)));
   ERROR_CHECK(OpalCreateRetrieveGlobalLockingRangeActiveKey(LockingSpSession, &CreateStruct, &Size));
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
   //
   // For Pyrite type SSC, it not supports Active Key. 
@@ -922,7 +1165,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
                     OPAL_LOCKING_SP_ADMINS_AUTHORITY
                 ));
 
-    ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+    ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
     if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
       DEBUG ((DEBUG_INFO, "Update ACE for GLOBALRANGE_GENKEY failed\n"));
@@ -947,7 +1190,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
                   OPAL_LOCKING_SP_ADMINS_AUTHORITY
               ));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
   if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
     DEBUG ((DEBUG_INFO, "Update ACE for OPAL_LOCKING_SP_ACE_LOCKING_GLOBALRANGE_GET_ALL failed\n"));
@@ -991,7 +1234,7 @@ OpalDisableUser(
                   OPAL_LOCKING_SP_USER1_AUTHORITY,
                   FALSE));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
   return TcgResultSuccess;
 }
@@ -1026,7 +1269,7 @@ OpalGlobalLockingRangeGenKey(
   // retrieve the activekey in order to know which globalrange key to generate
   //
   ERROR_CHECK(OpalCreateRetrieveGlobalLockingRangeActiveKey(LockingSpSession, &CreateStruct, &Size));
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
   METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);
 
@@ -1047,7 +1290,7 @@ OpalGlobalLockingRangeGenKey(
   ERROR_CHECK(TcgEndPacket(&CreateStruct));
   ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
 
   return TcgResultSuccess;
 }
@@ -1113,7 +1356,7 @@ OpalUpdateGlobalLockingRange(
   ERROR_CHECK(TcgEndPacket(&CreateStruct));
   ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
   METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);
 
   return TcgResultSuccess;
@@ -1214,7 +1457,7 @@ OpalSetLockingRange(
   ERROR_CHECK(TcgEndPacket(&CreateStruct));
   ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, MethodStatus, 0));
   // Exit with success on method failure - user must inspect MethodStatus
   METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);
 
@@ -1362,7 +1605,7 @@ OpalGetTryLimit(
   ERROR_CHECK(TcgEndPacket(&CreateStruct));
   ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
 
-  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, &MethodStatus));
+  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf), &ParseStruct, &MethodStatus, 0));
   METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
 
   ERROR_CHECK(TcgGetNextStartList(&ParseStruct));
@@ -1404,7 +1647,9 @@ OpalGetSupportedAttributesInfo(
   TCG_SUPPORTED_SECURITY_PROTOCOLS   *SupportedProtocols;
   TCG_LEVEL0_DISCOVERY_HEADER        *DiscoveryHeader;
   OPAL_LEVEL0_FEATURE_DESCRIPTOR     *Feat;
+  OPAL_LEVEL0_FEATURE_DESCRIPTOR     *Feat2;
   UINTN                              Size;
+  UINTN                              Size2;
 
   NULL_CHECK(Session);
   NULL_CHECK(SupportedAttributes);
@@ -1491,19 +1736,38 @@ OpalGetSupportedAttributesInfo(
     }
   }
 
+  //
+  // For some pyrite 2.0 device, it contains both pyrite 1.0 and 2.0 feature data.
+  // so here try to get data from pyrite 2.0 feature data first.
+  //
   Size = 0;
   Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature (DiscoveryHeader, TCG_FEATURE_PYRITE_SSC, &Size);
-  SupportedAttributes->PyriteSsc = (Feat != NULL);
-  if (Feat != NULL && Size >= sizeof (PYRITE_SSC_FEATURE_DESCRIPTOR)) {
+  Size2 = 0;
+  Feat2 = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature (DiscoveryHeader, TCG_FEATURE_PYRITE_SSC_V2_0_0, &Size2);
+  if (Feat2 != NULL && Size2 >= sizeof (PYRITE_SSCV2_FEATURE_DESCRIPTOR)) {
+    SupportedAttributes->PyriteSscV2 = TRUE;
     if (*OpalBaseComId == TCG_RESERVED_COMID) {
-      *OpalBaseComId = SwapBytes16 (Feat->PyriteSsc.BaseComdIdBE);
-      SupportedAttributes->InitCpinIndicator = (Feat->PyriteSsc.InitialCPINSIDPIN == 0);
-      SupportedAttributes->CpinUponRevert = (Feat->PyriteSsc.CPINSIDPINRevertBehavior == 0);
-      DEBUG ((DEBUG_INFO, "Pyrite SSC InitCpinIndicator %d  CpinUponRevert %d \n",
+      *OpalBaseComId = SwapBytes16 (Feat2->PyriteSscV2.BaseComdIdBE);
+      SupportedAttributes->InitCpinIndicator = (Feat2->PyriteSscV2.InitialCPINSIDPIN == 0);
+      SupportedAttributes->CpinUponRevert = (Feat2->PyriteSscV2.CPINSIDPINRevertBehavior == 0);
+      DEBUG ((DEBUG_INFO, "Pyrite SSC V2 InitCpinIndicator %d  CpinUponRevert %d \n",
                SupportedAttributes->InitCpinIndicator,
                SupportedAttributes->CpinUponRevert
               ));
     }
+  } else {
+    SupportedAttributes->PyriteSsc = (Feat != NULL);
+    if (Feat != NULL && Size >= sizeof (PYRITE_SSC_FEATURE_DESCRIPTOR)) {
+      if (*OpalBaseComId == TCG_RESERVED_COMID) {
+        *OpalBaseComId = SwapBytes16 (Feat->PyriteSsc.BaseComdIdBE);
+        SupportedAttributes->InitCpinIndicator = (Feat->PyriteSsc.InitialCPINSIDPIN == 0);
+        SupportedAttributes->CpinUponRevert = (Feat->PyriteSsc.CPINSIDPINRevertBehavior == 0);
+        DEBUG ((DEBUG_INFO, "Pyrite SSC InitCpinIndicator %d  CpinUponRevert %d \n",
+                 SupportedAttributes->InitCpinIndicator,
+                 SupportedAttributes->CpinUponRevert
+                ));
+      }
+    }
   }
 
   Size = 0;
@@ -1519,16 +1783,33 @@ OpalGetSupportedAttributesInfo(
   Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature (DiscoveryHeader, TCG_FEATURE_LOCKING, &Size);
   if (Feat != NULL && Size >= sizeof (TCG_LOCKING_FEATURE_DESCRIPTOR)) {
     SupportedAttributes->MediaEncryption = Feat->Locking.MediaEncryption;
+    DEBUG ((DEBUG_INFO, "SupportedAttributes->MediaEncryption 0x%X \n", SupportedAttributes->MediaEncryption));
   }
 
   Size = 0;
   Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature (DiscoveryHeader, TCG_FEATURE_BLOCK_SID, &Size);
   if (Feat != NULL && Size >= sizeof (TCG_BLOCK_SID_FEATURE_DESCRIPTOR)) {
     SupportedAttributes->BlockSid = TRUE;
+    DEBUG ((DEBUG_INFO, "BlockSid Supported!!! Current Status is 0x%X \n", Feat->BlockSid.SIDBlockedState));
+  } else {
+    DEBUG ((DEBUG_INFO, "BlockSid Unsupported!!!"));
   }
 
-  DEBUG ((DEBUG_INFO, "Base COMID 0x%04X \n", *OpalBaseComId));
+  Size = 0;
+  Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature (DiscoveryHeader, TCG_FEATURE_DATA_REMOVAL, &Size);
+  if (Feat != NULL && Size >= sizeof (DATA_REMOVAL_FEATURE_DESCRIPTOR)) {
+    SupportedAttributes->DataRemoval = TRUE;
+    DEBUG ((DEBUG_INFO, "DataRemoval Feature Supported!\n"));
+    DEBUG ((DEBUG_INFO, "Operation Processing = 0x%x\n", Feat->DataRemoval.OperationProcessing));
+    DEBUG ((DEBUG_INFO, "RemovalMechanism = 0x%x\n", Feat->DataRemoval.RemovalMechanism));
+    DEBUG ((DEBUG_INFO, "BIT0 :: Format = 0x%x, Time = 0x%x\n", Feat->DataRemoval.FormatBit0, SwapBytes16 (Feat->DataRemoval.TimeBit0)));
+    DEBUG ((DEBUG_INFO, "BIT1 :: Format = 0x%x, Time = 0x%x\n", Feat->DataRemoval.FormatBit1, SwapBytes16 (Feat->DataRemoval.TimeBit1)));
+    DEBUG ((DEBUG_INFO, "BIT2 :: Format = 0x%x, Time = 0x%x\n", Feat->DataRemoval.FormatBit2, SwapBytes16 (Feat->DataRemoval.TimeBit2)));
+    DEBUG ((DEBUG_INFO, "BIT3 :: Format = 0x%x, Time = 0x%x\n", Feat->DataRemoval.FormatBit3, SwapBytes16 (Feat->DataRemoval.TimeBit3)));
+    DEBUG ((DEBUG_INFO, "BIT4 :: Format = 0x%x, Time = 0x%x\n", Feat->DataRemoval.FormatBit4, SwapBytes16 (Feat->DataRemoval.TimeBit4)));
+  }
 
+  DEBUG ((DEBUG_INFO, "Base COMID 0x%04X \n", *OpalBaseComId));
 
   return TcgResultSuccess;
 }
@@ -1574,6 +1855,58 @@ OpalGetLockingInfo(
   return TcgResultSuccess;
 }
 
+/**
+
+  Get the support attribute info.
+
+  @param[in]      Session             OPAL_SESSION with OPAL_UID_LOCKING_SP to retrieve info.
+  @param[in]      FeatureCode         The feature code user request.
+  @param[in, out] DataSize            The data size.
+  @param[out]     Data                The data buffer used to save the feature descriptor.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalGetFeatureDescriptor (
+  IN     OPAL_SESSION              *Session,
+  IN     UINT16                    FeatureCode,
+  IN OUT UINTN                     *DataSize,
+  OUT    VOID                      *Data
+  )
+{
+  UINT8                              Buffer[BUFFER_SIZE];
+  TCG_LEVEL0_DISCOVERY_HEADER        *DiscoveryHeader;
+  OPAL_LEVEL0_FEATURE_DESCRIPTOR     *Feat;
+  UINTN                              Size;
+
+  NULL_CHECK(Session);
+  NULL_CHECK(DataSize);
+  NULL_CHECK(Data);
+
+  ZeroMem(Buffer, BUFFER_SIZE);
+  ASSERT(sizeof(Buffer) >= sizeof(TCG_SUPPORTED_SECURITY_PROTOCOLS));
+
+  if (OpalRetrieveLevel0DiscoveryHeader (Session, BUFFER_SIZE, Buffer) == TcgResultFailure) {
+    DEBUG ((DEBUG_INFO, "OpalRetrieveLevel0DiscoveryHeader failed\n"));
+    return TcgResultFailure;
+  }
+  DiscoveryHeader = (TCG_LEVEL0_DISCOVERY_HEADER*) Buffer;
+
+  Size = 0;
+  Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature (DiscoveryHeader, FeatureCode, &Size);
+  if (Feat != NULL) {
+    if (Size > *DataSize) {
+      *DataSize = Size;
+      return TcgResultFailureBufferTooSmall;
+    }
+
+    *DataSize = Size;
+    CopyMem (Data, Feat, Size);
+  }
+
+  return TcgResultSuccess;
+}
+
 /**
 
   The function determines whether or not all of the requirements for the Opal Feature (not full specification)
@@ -1597,7 +1930,8 @@ OpalFeatureSupported(
   if (SupportedAttributes->OpalSscLite == 0 &&
       SupportedAttributes->OpalSsc1 == 0 &&
       SupportedAttributes->OpalSsc2 == 0 &&
-      SupportedAttributes->PyriteSsc == 0
+      SupportedAttributes->PyriteSsc == 0 &&
+      SupportedAttributes->PyriteSscV2 == 0
      ) {
     return FALSE;
   }
diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
index 78e47387a9..70f54f7f8a 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
@@ -29,6 +29,7 @@
 [Sources]
   TcgStorageOpalCore.c
   TcgStorageOpalUtil.c
+  TcgStorageOpalLibInternal.h
 
 [LibraryClasses]
   BaseLib
diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
new file mode 100644
index 0000000000..cd16c51c3b
--- /dev/null
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
@@ -0,0 +1,98 @@
+/** @file
+  Internal functions for Opal Core library.
+
+Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution.  The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef _OPAL_INTERNAL_H_
+#define _OPAL_INTERNAL_H_
+
+#include <Library/TcgStorageOpalLib.h>
+
+
+/**
+
+  The function retrieves the MSID from the device specified
+
+  @param[in]  AdminSpSession              OPAL_SESSION with OPAL_UID_ADMIN_SP as OPAL_ADMIN_SP_ANYBODY_AUTHORITY
+  @param[out] ActiveDataRemovalMechanism  Active Data Removal Mechanism that the device will use for Revert/RevertSP calls.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalPyrite2GetActiveDataRemovalMechanism (
+  OPAL_SESSION    *AdminSpSession,
+  UINT8           *ActiveDataRemovalMechanism
+  );
+
+/**
+
+  Get the support attribute info.
+
+  @param[in]      Session             OPAL_SESSION with OPAL_UID_LOCKING_SP to retrieve info.
+  @param[in]      FeatureCode         The feature code user request.
+  @param[in, out] DataSize            The data size.
+  @param[out]     Data                The data buffer used to save the feature descriptor.
+
+**/
+TCG_RESULT
+OpalGetFeatureDescriptor (
+  IN     OPAL_SESSION              *Session,
+  IN     UINT16                    FeatureCode,
+  IN OUT UINTN                     *DataSize,
+  OUT    VOID                      *Data
+  );
+
+/**
+  Get revert timeout value.
+
+  @param[in]      Session                       The session info for one opal device.
+
+**/
+UINT32
+GetRevertTimeOut (
+  IN OPAL_SESSION                *Session
+  );
+
+/**
+
+  Reverts device using Admin SP Revert method.
+
+  @param[in]  AdminSpSession      OPAL_SESSION with OPAL_UID_ADMIN_SP as OPAL_ADMIN_SP_PSID_AUTHORITY to perform PSID revert.
+  @param[in]  EstimateTimeCost    Input the timeout value.
+
+**/
+TCG_RESULT
+OpalPyrite2PsidRevert(
+  OPAL_SESSION              *AdminSpSession,
+  UINT32                    EstimateTimeCost
+  );
+
+/**
+
+  The function calls the Admin SP RevertSP method on the Locking SP.  If KeepUserData is True, then the optional parameter
+  to keep the user Data is set to True, otherwise the optional parameter is not provided.
+
+  @param[in]      LockingSpSession    OPAL_SESSION with OPAL_UID_LOCKING_SP as OPAL_LOCKING_SP_ADMIN1_AUTHORITY to revertSP
+  @param[in]      KeepUserData        Specifies whether or not to keep user Data when performing RevertSP action. True = keeps user Data.
+  @param[in/out]  MethodStatus        Method status of last action performed.  If action succeeded, it should be TCG_METHOD_STATUS_CODE_SUCCESS.
+  @param[in]      EstimateTimeCost    Input the timeout value.
+
+**/
+TCG_RESULT
+OpalPyrite2AdminRevert(
+  OPAL_SESSION    *LockingSpSession,
+  BOOLEAN         KeepUserData,
+  UINT8           *MethodStatus,
+  UINT32          EstimateTimeCost
+  );
+
+#endif // _OPAL_CORE_H_
diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
index f77fbe25c1..0a597a20be 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
@@ -1,7 +1,7 @@
 /** @file
   Public API for Opal Core library.
 
-Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 ~ 2018, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -15,7 +15,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
 #include <Library/TcgStorageOpalLib.h>
+#include <TcgStorageOpalLibInternal.h>
 
+#define OPAL_MSID_LENGHT        128
 
 /**
   Creates a session with OPAL_UID_ADMIN_SP as OPAL_ADMIN_SP_PSID_AUTHORITY, then reverts device using Admin SP Revert method.
@@ -35,10 +37,14 @@ OpalUtilPsidRevert(
 {
   UINT8        MethodStatus;
   TCG_RESULT   Ret;
+  UINT32       RemovalTimeOut;
 
   NULL_CHECK(Session);
   NULL_CHECK(Psid);
 
+  RemovalTimeOut = GetRevertTimeOut (Session);
+  DEBUG ((DEBUG_INFO, "OpalUtilPsidRevert: Timeout value = %d\n", RemovalTimeOut));
+
   Ret = OpalStartSession(
                       Session,
                       OPAL_UID_ADMIN_SP,
@@ -48,7 +54,7 @@ OpalUtilPsidRevert(
                       OPAL_ADMIN_SP_PSID_AUTHORITY,
                       &MethodStatus);
   if (Ret == TcgResultSuccess && MethodStatus == TCG_METHOD_STATUS_CODE_SUCCESS) {
-    Ret = OpalPsidRevert(Session);
+    Ret = OpalPyrite2PsidRevert(Session, RemovalTimeOut);
     if (Ret != TcgResultSuccess) {
       //
       // If revert was successful, session was already ended by TPer, so only end session on failure
@@ -599,12 +605,16 @@ OpalUtilRevert(
 {
   UINT8        MethodStatus;
   TCG_RESULT   Ret;
+  UINT32       RemovalTimeOut;
 
   NULL_CHECK(Session);
   NULL_CHECK(Msid);
   NULL_CHECK(Password);
   NULL_CHECK(PasswordFailed);
 
+  RemovalTimeOut = GetRevertTimeOut (Session);
+  DEBUG ((DEBUG_INFO, "OpalUtilRevert: Timeout value = %d\n", RemovalTimeOut));
+
   Ret = OpalStartSession(
                    Session,
                    OPAL_UID_LOCKING_SP,
@@ -625,7 +635,7 @@ OpalUtilRevert(
   //
   // Try to revert with admin1
   //
-  Ret = OpalAdminRevert(Session, KeepUserData, &MethodStatus);
+  Ret = OpalPyrite2AdminRevert(Session, KeepUserData, &MethodStatus, RemovalTimeOut);
   if (Ret != TcgResultSuccess || MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
     //
     // Device ends the session on successful revert, so only call OpalEndSession when fail.
@@ -912,3 +922,201 @@ OpalUtilAdminPasswordExists(
   return (OwnerShip == OpalOwnershipUnknown && LockingFeature->LockingEnabled);
 }
 
+/**
+  Get Active Data Removal Mechanism Value.
+
+  @param[in]      Session                        The session info for one opal device.
+  @param[in]      GeneratedSid                   Generated SID of disk
+  @param[in]      SidLength                      Length of generatedSid in bytes
+  @param[out]     ActiveDataRemovalMechanism     Return the active data removal mechanism.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalUtilGetActiveDataRemovalMechanism (
+  OPAL_SESSION      *Session,
+  const VOID        *GeneratedSid,
+  UINT32            SidLength,
+  UINT8             *ActiveDataRemovalMechanism
+  )
+{
+  TCG_RESULT   Ret;
+  UINT8        MethodStatus;
+
+  NULL_CHECK(Session);
+  NULL_CHECK(GeneratedSid);
+  NULL_CHECK(ActiveDataRemovalMechanism);
+
+  Ret = OpalStartSession(
+                    Session,
+                    OPAL_UID_ADMIN_SP,
+                    TRUE,
+                    SidLength,
+                    GeneratedSid,
+                    OPAL_ADMIN_SP_ANYBODY_AUTHORITY,
+                    &MethodStatus
+                    );
+  if (Ret != TcgResultSuccess || MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
+    DEBUG ((DEBUG_INFO, "Start session with admin SP as SID authority failed: Ret=%d MethodStatus=%u\n", Ret, MethodStatus));
+    if (MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
+      Ret = TcgResultFailure;
+    }
+    return Ret;
+  }
+
+  Ret = OpalPyrite2GetActiveDataRemovalMechanism (
+                    Session,
+                    ActiveDataRemovalMechanism
+                    );
+
+  if (Ret != TcgResultSuccess) {
+    DEBUG ((DEBUG_INFO, "Pyrite2 Get Active Data Removal Mechanism failed: Ret=%d\n", Ret));
+  }
+
+  OpalEndSession(Session);
+
+  return Ret;
+}
+
+/**
+  Calculate the estimated time.
+
+  @param[in]      IsMinite               Whether the input time value is minute type or second type.
+  @param[in]      Time                   The input time value.
+
+**/
+UINT32
+CalculateDataRemovalTime (
+  IN BOOLEAN               IsMinute,
+  IN UINT16                Time
+  )
+{
+  if (IsMinute) {
+    return Time * 2 * 60;
+  } else {
+    return Time * 2;
+  }
+}
+
+/**
+  Return the estimated time for specific type.
+
+  @param[in]      Index               The input data removal type.
+  @param[in]      Descriptor          DATA_REMOVAL_FEATURE_DESCRIPTOR
+
+**/
+UINT32
+GetDataRemovalTime (
+  IN  UINT8                            Index,
+  IN  DATA_REMOVAL_FEATURE_DESCRIPTOR  *Descriptor
+  )
+{
+  switch (Index) {
+  case OverwriteDataErase:
+    return CalculateDataRemovalTime (Descriptor->FormatBit0, SwapBytes16 (Descriptor->TimeBit0));
+
+  case BlockErase:
+    return CalculateDataRemovalTime (Descriptor->FormatBit1, SwapBytes16 (Descriptor->TimeBit1));
+
+  case CryptoErase:
+    return CalculateDataRemovalTime (Descriptor->FormatBit2, SwapBytes16 (Descriptor->TimeBit2));
+
+  case Unmap:
+    return CalculateDataRemovalTime (Descriptor->FormatBit3, SwapBytes16 (Descriptor->TimeBit3));
+
+  case ResetWritePointers:
+    return CalculateDataRemovalTime (Descriptor->FormatBit4, SwapBytes16 (Descriptor->TimeBit4));
+
+  case VendorSpecificErase:
+    return CalculateDataRemovalTime (Descriptor->FormatBit5, SwapBytes16 (Descriptor->TimeBit5));
+
+  default:
+    return 0;
+  }
+}
+
+/**
+  Get the supported Data Removal Mechanism list.
+
+  @param[in]      Session                        The session info for one opal device.
+  @param[out]     RemovalMechanismLists          Return the supported data removal mechanism lists.
+
+**/
+TCG_RESULT
+EFIAPI
+OpalUtilGetDataRemovalMechanismLists (
+  IN  OPAL_SESSION      *Session,
+  OUT UINT32            *RemovalMechanismLists
+  )
+{
+  TCG_RESULT                       Ret;
+  UINTN                            DataSize;
+  DATA_REMOVAL_FEATURE_DESCRIPTOR  Descriptor;
+  UINT8                            Index;
+  UINT8                            BitValue;
+
+  NULL_CHECK(Session);
+  NULL_CHECK(RemovalMechanismLists);
+
+  DataSize = sizeof (Descriptor);
+  Ret = OpalGetFeatureDescriptor (Session, TCG_FEATURE_DATA_REMOVAL, &DataSize, &Descriptor);
+  if (Ret != TcgResultSuccess) {
+    return TcgResultFailure;
+  }
+
+  ASSERT (Descriptor.RemovalMechanism != 0);
+
+  for (Index = 0; Index < ResearvedMechanism; Index ++) {
+    BitValue = (BOOLEAN) BitFieldRead8 (Descriptor.RemovalMechanism, Index, Index);
+
+    if (BitValue == 0) {
+      RemovalMechanismLists[Index] = 0;
+    } else {
+      RemovalMechanismLists[Index] = GetDataRemovalTime (Index, &Descriptor);
+    }
+  }
+
+  return TcgResultSuccess;
+}
+
+/**
+  Get revert timeout value.
+
+  @param[in]      Session                       The session info for one opal device.
+
+**/
+UINT32
+GetRevertTimeOut (
+  IN OPAL_SESSION                *Session
+  )
+{
+  TCG_RESULT                   TcgResult;
+  OPAL_DISK_SUPPORT_ATTRIBUTE  SupportedAttributes;
+  UINT16                       BaseComId;
+  UINT32                       MsidLength;
+  UINT8                        Msid[OPAL_MSID_LENGHT];
+  UINT32                       RemovalMechanishLists[ResearvedMechanism];
+  UINT8                        ActiveDataRemovalMechanism;
+
+  TcgResult = OpalGetSupportedAttributesInfo (Session, &SupportedAttributes, &BaseComId);
+  if (TcgResult != TcgResultSuccess || SupportedAttributes.DataRemoval == 0) {
+    return 0;
+  }
+
+  TcgResult = OpalUtilGetMsid (Session, Msid, OPAL_MSID_LENGHT, &MsidLength);
+  if (TcgResult != TcgResultSuccess) {
+    return 0;
+  }
+
+  TcgResult = OpalUtilGetDataRemovalMechanismLists (Session, RemovalMechanishLists);
+  if (TcgResult != TcgResultSuccess) {
+    return 0;
+  }
+
+  TcgResult = OpalUtilGetActiveDataRemovalMechanism (Session, Msid, MsidLength, &ActiveDataRemovalMechanism);
+  if (TcgResult != TcgResultSuccess) {
+    return 0;
+  }
+
+  return RemovalMechanishLists[ActiveDataRemovalMechanism];
+}
-- 
2.15.0.windows.1

Re: [Patch 2/3] SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0 spec.

[Wu, Hao A]

Some minor comments:
1. In file TcgStorageOpalCore.c, the function description comment for
OpalGetFeatureDescriptor() seems not matching the function itself. Please help
to update.

2. For file TcgStorageOpalUtil.c, the copyright year format seems not proper.
Also, maybe double quotes can be used instead of brackets for:
#include <TcgStorageOpalLibInternal.h>

Other than the above comments, the patch is good to me.
Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> -----Original Message-----
> From: Dong, Eric
> Sent: Thursday, May 03, 2018 11:17 AM
> To: edk2-devel@lists.01.org; Wu, Hao A
> Subject: [Patch 2/3] SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0
> spec.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  SecurityPkg/Include/Library/TcgStorageOpalLib.h    |  41 ++
>  .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 426
> ++++++++++++++++++---
>  .../TcgStorageOpalLib/TcgStorageOpalLib.inf        |   1 +
>  .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  98 +++++
>  .../Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 214 ++++++++++-
>  5 files changed, 731 insertions(+), 49 deletions(-)
>  create mode 100644
> SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
> 
> diff --git a/SecurityPkg/Include/Library/TcgStorageOpalLib.h
> b/SecurityPkg/Include/Library/TcgStorageOpalLib.h
> index 9b64a8e5cd..2947c0eaf1 100644
> --- a/SecurityPkg/Include/Library/TcgStorageOpalLib.h
> +++ b/SecurityPkg/Include/Library/TcgStorageOpalLib.h
> @@ -82,6 +82,15 @@ typedef struct {
>      //
>      UINT32 BlockSid : 1;
> 
> +    //
> +    // Pyrite SSC V2 support  (0 - not supported, 1 - supported)
> +    //
> +    UINT32 PyriteSscV2 : 1;
> +
> +    //
> +    // Supported Data Removal Mechanism support  (0 - not supported, 1 -
> supported)
> +    //
> +    UINT32 DataRemoval : 1;
>  } OPAL_DISK_SUPPORT_ATTRIBUTE;
> 
>  //
> @@ -834,4 +843,36 @@ OpalUtilAdminPasswordExists(
>    IN  TCG_LOCKING_FEATURE_DESCRIPTOR   *LockingFeature
>    );
> 
> +/**
> +  Get Active Data Removal Mechanism Value.
> +
> +  @param[in]      Session,                       The session info for one opal device.
> +  @param[in]      GeneratedSid                   Generated SID of disk
> +  @param[in]      SidLength                      Length of generatedSid in bytes
> +  @param[out]     ActiveDataRemovalMechanism     Return the active data
> removal mechanism.
> +
> +**/
> +TCG_RESULT
> +EFIAPI
> +OpalUtilGetActiveDataRemovalMechanism (
> +  OPAL_SESSION      *Session,
> +  const VOID        *GeneratedSid,
> +  UINT32            SidLength,
> +  UINT8             *ActiveDataRemovalMechanism
> +  );
> +
> +/**
> +  Get the supported Data Removal Mechanism list.
> +
> +  @param[in]      Session,                       The session info for one opal device.
> +  @param[out]     RemovalMechanismLists          Return the supported data
> removal mechanism lists.
> +
> +**/
> +TCG_RESULT
> +EFIAPI
> +OpalUtilGetDataRemovalMechanismLists (
> +  IN  OPAL_SESSION      *Session,
> +  OUT UINT32            *RemovalMechanismLists
> +  );
> +
>  #endif // _OPAL_CORE_H_
> diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
> b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
> index 90cc51a24c..d031ebe798 100644
> --- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
> +++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Public API for Opal Core library.
> 
> -Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Library/DebugLib.h>
>  #include <Library/TcgStorageOpalLib.h>
> 
> +#include "TcgStorageOpalLibInternal.h"
> +
>  #pragma pack(1)
>  typedef struct {
>      UINT8 HardwareReset : 1;
> @@ -89,6 +91,7 @@ OpalTrustedSend(
>    @param[in]      SpSpecific            Security Protocol Specific
>    @param[in]      Buffer                Address of Data to transfer
>    @param[in]      BufferSize            Full Size of Buffer, including space that may
> be used for padding.
> +  @param[in]      EstimateTimeCost      Estimate the time needed.
> 
>  **/
>  TCG_RESULT
> @@ -98,10 +101,10 @@ OpalTrustedRecv(
>    UINT8                                  SecurityProtocol,
>    UINT16                                 SpSpecific,
>    VOID                                   *Buffer,
> -  UINTN                                  BufferSize
> +  UINTN                                  BufferSize,
> +  UINT32                                 EstimateTimeCost
>    )
>  {
> -
>    UINTN           TransferLength512;
>    UINT32          Tries;
>    TCG_COM_PACKET  *ComPacket;
> @@ -129,9 +132,15 @@ OpalTrustedRecv(
>    // so we need to retry the IF-RECV to get the actual Data.
>    // See TCG Core Spec v2 Table 45 IF-RECV ComPacket Field Values Summary
>    // This is an arbitrary number of retries, not from the spec.
> -  // have a max timeout of 10 seconds, 5000 tries * 2ms = 10s
>    //
> -  Tries = 5000;
> +  // if user input estimate time cost(second level) value bigger than 10s, base
> on user input value to wait.
> +  // Else, Use a max timeout of 10 seconds to wait, 5000 tries * 2ms = 10s
> +  //
> +  if (EstimateTimeCost > 10) {
> +    Tries = EstimateTimeCost * 500; // 500 = 1000 * 1000 / 2000;
> +  } else {
> +    Tries = 5000;
> +  }
>    while ((Tries--) > 0) {
>      ZeroMem( Buffer, BufferSize );
>      TransferSize = 0;
> @@ -146,7 +155,6 @@ OpalTrustedRecv(
>                   Buffer,
>                   &TransferSize
>               );
> -
>      if (EFI_ERROR (Status)) {
>        return TcgResultFailure;
>      }
> @@ -179,23 +187,24 @@ OpalTrustedRecv(
>  /**
>    The function performs send, recv, check comIDs, check method status action.
> 
> -  @param[in]      Session         OPAL_SESSION related to this method..
> -  @param[in]      SendSize        Transfer Length of Buffer (in bytes) - always a
> multiple of 512
> -  @param[in]      Buffer          Address of Data to transfer
> -  @param[in]      BufferSize      Full Size of Buffer, including space that may be
> used for padding.
> -  @param[in]      ParseStruct     Structure used to parse received TCG response.
> -  @param[in]      MethodStatus    Method status of last action performed.  If
> action succeeded, it should be TCG_METHOD_STATUS_CODE_SUCCESS.
> -
> +  @param[in]      Session           OPAL_SESSION related to this method..
> +  @param[in]      SendSize          Transfer Length of Buffer (in bytes) - always a
> multiple of 512
> +  @param[in]      Buffer            Address of Data to transfer
> +  @param[in]      BufferSize        Full Size of Buffer, including space that may
> be used for padding.
> +  @param[in]      ParseStruct       Structure used to parse received TCG
> response.
> +  @param[in]      MethodStatus      Method status of last action performed.  If
> action succeeded, it should be TCG_METHOD_STATUS_CODE_SUCCESS.
> +  @param[in]      EstimateTimeCost  Estimate the time need to for the method.
>  **/
>  TCG_RESULT
>  EFIAPI
> -OpalPerformMethod(
> +OpalPerformMethod (
>    OPAL_SESSION     *Session,
>    UINT32           SendSize,
>    VOID             *Buffer,
>    UINT32           BufferSize,
>    TCG_PARSE_STRUCT *ParseStruct,
> -  UINT8            *MethodStatus
> +  UINT8            *MethodStatus,
> +  UINT32           EstimateTimeCost
>    )
>  {
>    NULL_CHECK(Session);
> @@ -217,7 +226,8 @@ OpalPerformMethod(
>                    TCG_OPAL_SECURITY_PROTOCOL_1,
>                    Session->OpalBaseComId,
>                    Buffer,
> -                  BufferSize
> +                  BufferSize,
> +                  EstimateTimeCost
>                ));
> 
>    ERROR_CHECK(TcgInitTcgParseStruct(ParseStruct, Buffer, BufferSize));
> @@ -309,7 +319,60 @@ OpalPsidRevert(
>    //
>    // Send Revert Method Call
>    //
> -  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer,
> BUFFER_SIZE, &ParseStruct, &MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer,
> BUFFER_SIZE, &ParseStruct, &MethodStatus, 0));
> +  METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
> +
> +  return TcgResultSuccess;
> +}
> +
> +/**
> +
> +  Reverts device using Admin SP Revert method.
> +
> +  @param[in]  AdminSpSession      OPAL_SESSION with OPAL_UID_ADMIN_SP
> as OPAL_ADMIN_SP_PSID_AUTHORITY to perform PSID revert.
> +  @param[in]  EstimateTimeCost    Estimate the time needed.
> +
> +**/
> +TCG_RESULT
> +EFIAPI
> +OpalPyrite2PsidRevert(
> +  OPAL_SESSION              *AdminSpSession,
> +  UINT32                    EstimateTimeCost
> +  )
> +{
> +  //
> +  // Now that base comid is known, start Session
> +  // we'll attempt to start Session as PSID authority
> +  // verify PSID Authority is defined in ADMIN SP authority table... is this
> possible?
> +  //
> +  TCG_CREATE_STRUCT  CreateStruct;
> +  TCG_PARSE_STRUCT   ParseStruct;
> +  UINT32             Size;
> +  UINT8              Buffer[BUFFER_SIZE];
> +  UINT8              MethodStatus;
> +
> +
> +  NULL_CHECK(AdminSpSession);
> +
> +  //
> +  // Send Revert action on Admin SP
> +  //
> +  ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buffer, BUFFER_SIZE));
> +  ERROR_CHECK(TcgStartComPacket(&CreateStruct, AdminSpSession-
> >OpalBaseComId, AdminSpSession->ComIdExtension));
> +  ERROR_CHECK(TcgStartPacket(&CreateStruct, AdminSpSession-
> >TperSessionId, AdminSpSession->HostSessionId, 0x0, 0x0, 0x0));
> +  ERROR_CHECK(TcgStartSubPacket(&CreateStruct, 0x0));
> +  ERROR_CHECK(TcgStartMethodCall(&CreateStruct, OPAL_UID_ADMIN_SP,
> OPAL_ADMIN_SP_REVERT_METHOD));
> +  ERROR_CHECK(TcgStartParameters(&CreateStruct));
> +  ERROR_CHECK(TcgEndParameters(&CreateStruct));
> +  ERROR_CHECK(TcgEndMethodCall(&CreateStruct));
> +  ERROR_CHECK(TcgEndSubPacket(&CreateStruct));
> +  ERROR_CHECK(TcgEndPacket(&CreateStruct));
> +  ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
> +
> +  //
> +  // Send Revert Method Call
> +  //
> +  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer,
> BUFFER_SIZE, &ParseStruct, &MethodStatus, EstimateTimeCost));
>    METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
> 
>    return TcgResultSuccess;
> @@ -339,7 +402,8 @@ OpalRetrieveLevel0DiscoveryHeader(
>                TCG_OPAL_SECURITY_PROTOCOL_1,   // SP
>                TCG_SP_SPECIFIC_PROTOCOL_LEVEL0_DISCOVERY, // SP_Specific
>                BuffAddress,
> -              BufferSize
> +              BufferSize,
> +              0
>              ));
>  }
> 
> @@ -367,7 +431,8 @@ OpalRetrieveSupportedProtocolList(
>                TCG_SECURITY_PROTOCOL_INFO, // SP
>                TCG_SP_SPECIFIC_PROTOCOL_LIST, // SP_Specific
>                BuffAddress,
> -              BufferSize
> +              BufferSize,
> +              0
>            ));
>  }
> 
> @@ -430,7 +495,7 @@ OpalStartSession(
>                      HostChallenge,
>                      HostSigningAuthority
>                  ));
> -  ERROR_CHECK(OpalPerformMethod(Session, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(Session, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
>    if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
>      return TcgResultSuccess; // return early if method failed - user must check
> MethodStatus
>    }
> @@ -487,7 +552,8 @@ OpalEndSession(
>                    TCG_OPAL_SECURITY_PROTOCOL_1,
>                    Session->OpalBaseComId,
>                    Buffer,
> -                  sizeof(Buffer)
> +                  sizeof(Buffer),
> +                  0
>                ));
> 
>    ERROR_CHECK(TcgInitTcgParseStruct(&ParseStruct, Buffer, sizeof(Buffer)));
> @@ -558,7 +624,7 @@ OpalGetMsid(
>    //
>    // Send MSID Method Call
>    //
> -  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer,
> BUFFER_SIZE, &ParseStruct, &MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer,
> BUFFER_SIZE, &ParseStruct, &MethodStatus, 0));
>    METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
> 
>    ERROR_CHECK(TcgGetNextStartList(&ParseStruct));
> @@ -592,6 +658,86 @@ OpalGetMsid(
>    return TcgResultSuccess;
>  }
> 
> +/**
> +
> +  The function retrieves the MSID from the device specified
> +
> +  @param[in]  AdminSpSession              OPAL_SESSION with
> OPAL_UID_ADMIN_SP as OPAL_ADMIN_SP_ANYBODY_AUTHORITY
> +  @param[out] ActiveDataRemovalMechanism  Active Data Removal
> Mechanism that the device will use for Revert/RevertSP calls.
> +
> +**/
> +TCG_RESULT
> +EFIAPI
> +OpalPyrite2GetActiveDataRemovalMechanism (
> +  IN  OPAL_SESSION    *AdminSpSession,
> +  OUT UINT8           *ActiveDataRemovalMechanism
> +  )
> +{
> +  TCG_CREATE_STRUCT CreateStruct;
> +  TCG_PARSE_STRUCT  ParseStruct;
> +  UINT32            Size;
> +  UINT8             MethodStatus;
> +  UINT32            Col;
> +  UINT8             RecvActiveDataRemovalMechanism;
> +  UINT8             Buffer[BUFFER_SIZE];
> +
> +  NULL_CHECK(AdminSpSession);
> +  NULL_CHECK(ActiveDataRemovalMechanism);
> +
> +  ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buffer, BUFFER_SIZE));
> +  ERROR_CHECK(TcgStartComPacket(&CreateStruct, AdminSpSession-
> >OpalBaseComId, AdminSpSession->ComIdExtension));
> +  ERROR_CHECK(TcgStartPacket(&CreateStruct, AdminSpSession-
> >TperSessionId, AdminSpSession->HostSessionId, 0x0, 0x0, 0x0));
> +  ERROR_CHECK(TcgStartSubPacket(&CreateStruct, 0x0));
> +  ERROR_CHECK(TcgStartMethodCall(&CreateStruct,
> OPAL_UID_ADMIN_SP_DATA_REMOVAL_MECHANISM,
> TCG_UID_METHOD_GET));
> +  ERROR_CHECK(TcgStartParameters(&CreateStruct));
> +  ERROR_CHECK(TcgAddStartList(&CreateStruct));
> +  ERROR_CHECK(TcgAddStartName(&CreateStruct));
> +  ERROR_CHECK(TcgAddUINT8(&CreateStruct,
> TCG_CELL_BLOCK_START_COLUMN_NAME));
> +  ERROR_CHECK(TcgAddUINT8(&CreateStruct,
> OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL));
> +  ERROR_CHECK(TcgAddEndName(&CreateStruct));
> +  ERROR_CHECK(TcgAddStartName(&CreateStruct));
> +  ERROR_CHECK(TcgAddUINT8(&CreateStruct,
> TCG_CELL_BLOCK_END_COLUMN_NAME));
> +  ERROR_CHECK(TcgAddUINT8(&CreateStruct,
> OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL));
> +  ERROR_CHECK(TcgAddEndName(&CreateStruct));
> +  ERROR_CHECK(TcgAddEndList(&CreateStruct));
> +  ERROR_CHECK(TcgEndParameters(&CreateStruct));
> +  ERROR_CHECK(TcgEndMethodCall(&CreateStruct));
> +  ERROR_CHECK(TcgEndSubPacket(&CreateStruct));
> +  ERROR_CHECK(TcgEndPacket(&CreateStruct));
> +  ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
> +
> +  //
> +  // Send Get Active Data Removal Mechanism Method Call
> +  //
> +  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buffer,
> BUFFER_SIZE, &ParseStruct, &MethodStatus, 0));
> +  METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
> +
> +  ERROR_CHECK(TcgGetNextStartList(&ParseStruct));
> +  ERROR_CHECK(TcgGetNextStartList(&ParseStruct));
> +  ERROR_CHECK(TcgGetNextStartName(&ParseStruct));
> +  ERROR_CHECK(TcgGetNextUINT32(&ParseStruct, &Col));
> +  ERROR_CHECK(TcgGetNextUINT8(&ParseStruct,
> &RecvActiveDataRemovalMechanism));
> +  ERROR_CHECK(TcgGetNextEndName(&ParseStruct));
> +  ERROR_CHECK(TcgGetNextEndList(&ParseStruct));
> +  ERROR_CHECK(TcgGetNextEndList(&ParseStruct));
> +  ERROR_CHECK(TcgGetNextEndOfData(&ParseStruct));
> +
> +  if (Col != OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL) {
> +    DEBUG ((DEBUG_INFO, "ERROR: got col %u, expected %u\n", Col,
> OPAL_ADMIN_SP_ACTIVE_DATA_REMOVAL_MECHANISM_COL));
> +    return TcgResultFailure;
> +  }
> +
> +  if (RecvActiveDataRemovalMechanism >= ResearvedMechanism) {
> +    return TcgResultFailure;
> +  }
> +
> +  //
> +  // Copy active data removal mechanism into Buffer
> +  //
> +  CopyMem(ActiveDataRemovalMechanism,
> &RecvActiveDataRemovalMechanism,
> sizeof(RecvActiveDataRemovalMechanism));
> +  return TcgResultSuccess;
> +}
> +
>  /**
> 
>    The function calls the Admin SP RevertSP method on the Locking SP.  If
> KeepUserData is True, then the optional parameter
> @@ -666,7 +812,104 @@ OpalAdminRevert(
>    //
>    // Send RevertSP method call
>    //
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> +
> +  //
> +  // Session is immediately ended by device after successful revertsp, so no
> need to end Session
> +  //
> +  if (*MethodStatus == TCG_METHOD_STATUS_CODE_SUCCESS) {
> +    //
> +    // Caller should take ownership again
> +    //
> +    return TcgResultSuccess;
> +  } else {
> +    //
> +    // End Session
> +    //
> +    METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);     //
> exit with success on method failure - user must inspect MethodStatus
> +  }
> +
> +  return TcgResultSuccess;
> +}
> +
> +
> +/**
> +
> +  The function calls the Admin SP RevertSP method on the Locking SP.  If
> KeepUserData is True, then the optional parameter
> +  to keep the user Data is set to True, otherwise the optional parameter is not
> provided.
> +
> +  @param[in]      LockingSpSession    OPAL_SESSION with
> OPAL_UID_LOCKING_SP as OPAL_LOCKING_SP_ADMIN1_AUTHORITY to
> revertSP
> +  @param[in]      KeepUserData        Specifies whether or not to keep user
> Data when performing RevertSP action. True = keeps user Data.
> +  @param[in/out]  MethodStatus        Method status of last action performed.
> If action succeeded, it should be TCG_METHOD_STATUS_CODE_SUCCESS.
> +  @param[in]      EstimateTimeCost    Estimate the time needed.
> +
> +**/
> +TCG_RESULT
> +EFIAPI
> +OpalPyrite2AdminRevert(
> +  OPAL_SESSION    *LockingSpSession,
> +  BOOLEAN         KeepUserData,
> +  UINT8           *MethodStatus,
> +  UINT32          EstimateTimeCost
> +  )
> +{
> +  UINT8             Buf[BUFFER_SIZE];
> +  TCG_CREATE_STRUCT CreateStruct;
> +  UINT32            Size;
> +  TCG_PARSE_STRUCT  ParseStruct;
> +  TCG_RESULT        Ret;
> +
> +  NULL_CHECK(LockingSpSession);
> +  NULL_CHECK(MethodStatus);
> +
> +  //
> +  // ReadLocked or WriteLocked must be False (per Opal spec) to guarantee
> revertSP can keep user Data
> +  //
> +  if (KeepUserData) {
> +    //
> +    // set readlocked and writelocked to false
> +    //
> +    Ret = OpalUpdateGlobalLockingRange(
> +                        LockingSpSession,
> +                        FALSE,
> +                        FALSE,
> +                        MethodStatus);
> +
> +    if (Ret != TcgResultSuccess || *MethodStatus !=
> TCG_METHOD_STATUS_CODE_SUCCESS) {
> +      //
> +      // bail out
> +      //
> +      return Ret;
> +    }
> +  }
> +
> +  ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buf, sizeof(Buf)));
> +  ERROR_CHECK(TcgStartComPacket(&CreateStruct, LockingSpSession-
> >OpalBaseComId, LockingSpSession->ComIdExtension));
> +  ERROR_CHECK(TcgStartPacket(&CreateStruct, LockingSpSession-
> >TperSessionId, LockingSpSession->HostSessionId, 0x0, 0x0, 0x0));
> +  ERROR_CHECK(TcgStartSubPacket(&CreateStruct, 0x0));
> +  ERROR_CHECK(TcgStartMethodCall(&CreateStruct, TCG_UID_THIS_SP,
> OPAL_LOCKING_SP_REVERTSP_METHOD));
> +  ERROR_CHECK(TcgStartParameters(&CreateStruct));
> +
> +  if (KeepUserData) {
> +    //
> +    // optional parameter to keep Data after revert
> +    //
> +    ERROR_CHECK(TcgAddStartName(&CreateStruct));
> +    ERROR_CHECK(TcgAddUINT32(&CreateStruct, 0x060000));      // weird
> Value but that's what spec says
> +    ERROR_CHECK(TcgAddBOOLEAN(&CreateStruct, KeepUserData));
> +    ERROR_CHECK(TcgAddEndName(&CreateStruct));
> +  }
> +
> +  ERROR_CHECK(TcgEndParameters(&CreateStruct));
> +  ERROR_CHECK(TcgEndMethodCall(&CreateStruct));
> +  ERROR_CHECK(TcgEndSubPacket(&CreateStruct));
> +  ERROR_CHECK(TcgEndPacket(&CreateStruct));
> +  ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
> +
> +  //
> +  // Send RevertSP method call
> +  //
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, EstimateTimeCost));
> 
>    //
>    // Session is immediately ended by device after successful revertsp, so no
> need to end Session
> @@ -729,7 +972,7 @@ OpalActivateLockingSp(
>    //
>    // Send Activate method call
>    //
> -  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(AdminSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
>    METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess); // exit
> with success on method failure - user must inspect MethodStatus
> 
>    return TcgResultSuccess;
> @@ -778,7 +1021,7 @@ OpalSetPassword(
>                           NewPinLength
>                           ));
> 
> -  ERROR_CHECK(OpalPerformMethod(Session, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(Session, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
>    // exit with success on method failure - user must inspect MethodStatus
>    METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);
> 
> @@ -831,7 +1074,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
>                    AuthorityUid,
>                    TRUE));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>    if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
>      DEBUG ((DEBUG_INFO, "Send Set Authority error\n"));
> @@ -851,7 +1094,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
>                    NewPin,
>                    NewPinLength));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>    //
>    // allow user1 to set global range to unlocked/locked by modifying
> ACE_Locking_GlobalRange_SetRdLocked/SetWrLocked
> @@ -870,7 +1113,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
>                    OPAL_LOCKING_SP_ADMINS_AUTHORITY
>                ));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>    if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
>      DEBUG ((DEBUG_INFO, "Update ACE for RDLOCKED failed\n"));
> @@ -891,7 +1134,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
>                    OPAL_LOCKING_SP_ADMINS_AUTHORITY
>                ));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>    if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
>      DEBUG ((DEBUG_INFO, "Update ACE for WRLOCKED failed\n"));
> @@ -900,7 +1143,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
> 
>    ERROR_CHECK(TcgInitTcgCreateStruct(&CreateStruct, Buf, sizeof(Buf)));
> 
> ERROR_CHECK(OpalCreateRetrieveGlobalLockingRangeActiveKey(LockingSpSes
> sion, &CreateStruct, &Size));
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>    //
>    // For Pyrite type SSC, it not supports Active Key.
> @@ -922,7 +1165,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
>                      OPAL_LOCKING_SP_ADMINS_AUTHORITY
>                  ));
> 
> -    ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +    ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>      if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
>        DEBUG ((DEBUG_INFO, "Update ACE for GLOBALRANGE_GENKEY
> failed\n"));
> @@ -947,7 +1190,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
>                    OPAL_LOCKING_SP_ADMINS_AUTHORITY
>                ));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>    if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
>      DEBUG ((DEBUG_INFO, "Update ACE for
> OPAL_LOCKING_SP_ACE_LOCKING_GLOBALRANGE_GET_ALL failed\n"));
> @@ -991,7 +1234,7 @@ OpalDisableUser(
>                    OPAL_LOCKING_SP_USER1_AUTHORITY,
>                    FALSE));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>    return TcgResultSuccess;
>  }
> @@ -1026,7 +1269,7 @@ OpalGlobalLockingRangeGenKey(
>    // retrieve the activekey in order to know which globalrange key to generate
>    //
> 
> ERROR_CHECK(OpalCreateRetrieveGlobalLockingRangeActiveKey(LockingSpSes
> sion, &CreateStruct, &Size));
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>    METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);
> 
> @@ -1047,7 +1290,7 @@ OpalGlobalLockingRangeGenKey(
>    ERROR_CHECK(TcgEndPacket(&CreateStruct));
>    ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
> 
>    return TcgResultSuccess;
>  }
> @@ -1113,7 +1356,7 @@ OpalUpdateGlobalLockingRange(
>    ERROR_CHECK(TcgEndPacket(&CreateStruct));
>    ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
>    METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);
> 
>    return TcgResultSuccess;
> @@ -1214,7 +1457,7 @@ OpalSetLockingRange(
>    ERROR_CHECK(TcgEndPacket(&CreateStruct));
>    ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, MethodStatus, 0));
>    // Exit with success on method failure - user must inspect MethodStatus
>    METHOD_STATUS_ERROR_CHECK(*MethodStatus, TcgResultSuccess);
> 
> @@ -1362,7 +1605,7 @@ OpalGetTryLimit(
>    ERROR_CHECK(TcgEndPacket(&CreateStruct));
>    ERROR_CHECK(TcgEndComPacket(&CreateStruct, &Size));
> 
> -  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, &MethodStatus));
> +  ERROR_CHECK(OpalPerformMethod(LockingSpSession, Size, Buf, sizeof(Buf),
> &ParseStruct, &MethodStatus, 0));
>    METHOD_STATUS_ERROR_CHECK(MethodStatus, TcgResultFailure);
> 
>    ERROR_CHECK(TcgGetNextStartList(&ParseStruct));
> @@ -1404,7 +1647,9 @@ OpalGetSupportedAttributesInfo(
>    TCG_SUPPORTED_SECURITY_PROTOCOLS   *SupportedProtocols;
>    TCG_LEVEL0_DISCOVERY_HEADER        *DiscoveryHeader;
>    OPAL_LEVEL0_FEATURE_DESCRIPTOR     *Feat;
> +  OPAL_LEVEL0_FEATURE_DESCRIPTOR     *Feat2;
>    UINTN                              Size;
> +  UINTN                              Size2;
> 
>    NULL_CHECK(Session);
>    NULL_CHECK(SupportedAttributes);
> @@ -1491,19 +1736,38 @@ OpalGetSupportedAttributesInfo(
>      }
>    }
> 
> +  //
> +  // For some pyrite 2.0 device, it contains both pyrite 1.0 and 2.0 feature data.
> +  // so here try to get data from pyrite 2.0 feature data first.
> +  //
>    Size = 0;
>    Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature
> (DiscoveryHeader, TCG_FEATURE_PYRITE_SSC, &Size);
> -  SupportedAttributes->PyriteSsc = (Feat != NULL);
> -  if (Feat != NULL && Size >= sizeof (PYRITE_SSC_FEATURE_DESCRIPTOR)) {
> +  Size2 = 0;
> +  Feat2 = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature
> (DiscoveryHeader, TCG_FEATURE_PYRITE_SSC_V2_0_0, &Size2);
> +  if (Feat2 != NULL && Size2 >= sizeof (PYRITE_SSCV2_FEATURE_DESCRIPTOR))
> {
> +    SupportedAttributes->PyriteSscV2 = TRUE;
>      if (*OpalBaseComId == TCG_RESERVED_COMID) {
> -      *OpalBaseComId = SwapBytes16 (Feat->PyriteSsc.BaseComdIdBE);
> -      SupportedAttributes->InitCpinIndicator = (Feat->PyriteSsc.InitialCPINSIDPIN
> == 0);
> -      SupportedAttributes->CpinUponRevert = (Feat-
> >PyriteSsc.CPINSIDPINRevertBehavior == 0);
> -      DEBUG ((DEBUG_INFO, "Pyrite SSC InitCpinIndicator %d
> CpinUponRevert %d \n",
> +      *OpalBaseComId = SwapBytes16 (Feat2->PyriteSscV2.BaseComdIdBE);
> +      SupportedAttributes->InitCpinIndicator = (Feat2-
> >PyriteSscV2.InitialCPINSIDPIN == 0);
> +      SupportedAttributes->CpinUponRevert = (Feat2-
> >PyriteSscV2.CPINSIDPINRevertBehavior == 0);
> +      DEBUG ((DEBUG_INFO, "Pyrite SSC V2 InitCpinIndicator %d
> CpinUponRevert %d \n",
>                 SupportedAttributes->InitCpinIndicator,
>                 SupportedAttributes->CpinUponRevert
>                ));
>      }
> +  } else {
> +    SupportedAttributes->PyriteSsc = (Feat != NULL);
> +    if (Feat != NULL && Size >= sizeof (PYRITE_SSC_FEATURE_DESCRIPTOR)) {
> +      if (*OpalBaseComId == TCG_RESERVED_COMID) {
> +        *OpalBaseComId = SwapBytes16 (Feat->PyriteSsc.BaseComdIdBE);
> +        SupportedAttributes->InitCpinIndicator = (Feat-
> >PyriteSsc.InitialCPINSIDPIN == 0);
> +        SupportedAttributes->CpinUponRevert = (Feat-
> >PyriteSsc.CPINSIDPINRevertBehavior == 0);
> +        DEBUG ((DEBUG_INFO, "Pyrite SSC InitCpinIndicator %d
> CpinUponRevert %d \n",
> +                 SupportedAttributes->InitCpinIndicator,
> +                 SupportedAttributes->CpinUponRevert
> +                ));
> +      }
> +    }
>    }
> 
>    Size = 0;
> @@ -1519,16 +1783,33 @@ OpalGetSupportedAttributesInfo(
>    Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature
> (DiscoveryHeader, TCG_FEATURE_LOCKING, &Size);
>    if (Feat != NULL && Size >= sizeof (TCG_LOCKING_FEATURE_DESCRIPTOR)) {
>      SupportedAttributes->MediaEncryption = Feat->Locking.MediaEncryption;
> +    DEBUG ((DEBUG_INFO, "SupportedAttributes->MediaEncryption 0x%X \n",
> SupportedAttributes->MediaEncryption));
>    }
> 
>    Size = 0;
>    Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature
> (DiscoveryHeader, TCG_FEATURE_BLOCK_SID, &Size);
>    if (Feat != NULL && Size >= sizeof (TCG_BLOCK_SID_FEATURE_DESCRIPTOR))
> {
>      SupportedAttributes->BlockSid = TRUE;
> +    DEBUG ((DEBUG_INFO, "BlockSid Supported!!! Current Status is 0x%X \n",
> Feat->BlockSid.SIDBlockedState));
> +  } else {
> +    DEBUG ((DEBUG_INFO, "BlockSid Unsupported!!!"));
>    }
> 
> -  DEBUG ((DEBUG_INFO, "Base COMID 0x%04X \n", *OpalBaseComId));
> +  Size = 0;
> +  Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature
> (DiscoveryHeader, TCG_FEATURE_DATA_REMOVAL, &Size);
> +  if (Feat != NULL && Size >= sizeof (DATA_REMOVAL_FEATURE_DESCRIPTOR))
> {
> +    SupportedAttributes->DataRemoval = TRUE;
> +    DEBUG ((DEBUG_INFO, "DataRemoval Feature Supported!\n"));
> +    DEBUG ((DEBUG_INFO, "Operation Processing = 0x%x\n", Feat-
> >DataRemoval.OperationProcessing));
> +    DEBUG ((DEBUG_INFO, "RemovalMechanism = 0x%x\n", Feat-
> >DataRemoval.RemovalMechanism));
> +    DEBUG ((DEBUG_INFO, "BIT0 :: Format = 0x%x, Time = 0x%x\n", Feat-
> >DataRemoval.FormatBit0, SwapBytes16 (Feat->DataRemoval.TimeBit0)));
> +    DEBUG ((DEBUG_INFO, "BIT1 :: Format = 0x%x, Time = 0x%x\n", Feat-
> >DataRemoval.FormatBit1, SwapBytes16 (Feat->DataRemoval.TimeBit1)));
> +    DEBUG ((DEBUG_INFO, "BIT2 :: Format = 0x%x, Time = 0x%x\n", Feat-
> >DataRemoval.FormatBit2, SwapBytes16 (Feat->DataRemoval.TimeBit2)));
> +    DEBUG ((DEBUG_INFO, "BIT3 :: Format = 0x%x, Time = 0x%x\n", Feat-
> >DataRemoval.FormatBit3, SwapBytes16 (Feat->DataRemoval.TimeBit3)));
> +    DEBUG ((DEBUG_INFO, "BIT4 :: Format = 0x%x, Time = 0x%x\n", Feat-
> >DataRemoval.FormatBit4, SwapBytes16 (Feat->DataRemoval.TimeBit4)));
> +  }
> 
> +  DEBUG ((DEBUG_INFO, "Base COMID 0x%04X \n", *OpalBaseComId));
> 
>    return TcgResultSuccess;
>  }
> @@ -1574,6 +1855,58 @@ OpalGetLockingInfo(
>    return TcgResultSuccess;
>  }
> 
> +/**
> +
> +  Get the support attribute info.
> +
> +  @param[in]      Session             OPAL_SESSION with OPAL_UID_LOCKING_SP
> to retrieve info.
> +  @param[in]      FeatureCode         The feature code user request.
> +  @param[in, out] DataSize            The data size.
> +  @param[out]     Data                The data buffer used to save the feature
> descriptor.
> +
> +**/
> +TCG_RESULT
> +EFIAPI
> +OpalGetFeatureDescriptor (
> +  IN     OPAL_SESSION              *Session,
> +  IN     UINT16                    FeatureCode,
> +  IN OUT UINTN                     *DataSize,
> +  OUT    VOID                      *Data
> +  )
> +{
> +  UINT8                              Buffer[BUFFER_SIZE];
> +  TCG_LEVEL0_DISCOVERY_HEADER        *DiscoveryHeader;
> +  OPAL_LEVEL0_FEATURE_DESCRIPTOR     *Feat;
> +  UINTN                              Size;
> +
> +  NULL_CHECK(Session);
> +  NULL_CHECK(DataSize);
> +  NULL_CHECK(Data);
> +
> +  ZeroMem(Buffer, BUFFER_SIZE);
> +  ASSERT(sizeof(Buffer) >= sizeof(TCG_SUPPORTED_SECURITY_PROTOCOLS));
> +
> +  if (OpalRetrieveLevel0DiscoveryHeader (Session, BUFFER_SIZE, Buffer) ==
> TcgResultFailure) {
> +    DEBUG ((DEBUG_INFO, "OpalRetrieveLevel0DiscoveryHeader failed\n"));
> +    return TcgResultFailure;
> +  }
> +  DiscoveryHeader = (TCG_LEVEL0_DISCOVERY_HEADER*) Buffer;
> +
> +  Size = 0;
> +  Feat = (OPAL_LEVEL0_FEATURE_DESCRIPTOR*) TcgGetFeature
> (DiscoveryHeader, FeatureCode, &Size);
> +  if (Feat != NULL) {
> +    if (Size > *DataSize) {
> +      *DataSize = Size;
> +      return TcgResultFailureBufferTooSmall;
> +    }
> +
> +    *DataSize = Size;
> +    CopyMem (Data, Feat, Size);
> +  }
> +
> +  return TcgResultSuccess;
> +}
> +
>  /**
> 
>    The function determines whether or not all of the requirements for the Opal
> Feature (not full specification)
> @@ -1597,7 +1930,8 @@ OpalFeatureSupported(
>    if (SupportedAttributes->OpalSscLite == 0 &&
>        SupportedAttributes->OpalSsc1 == 0 &&
>        SupportedAttributes->OpalSsc2 == 0 &&
> -      SupportedAttributes->PyriteSsc == 0
> +      SupportedAttributes->PyriteSsc == 0 &&
> +      SupportedAttributes->PyriteSscV2 == 0
>       ) {
>      return FALSE;
>    }
> diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
> b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
> index 78e47387a9..70f54f7f8a 100644
> --- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
> +++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
> @@ -29,6 +29,7 @@
>  [Sources]
>    TcgStorageOpalCore.c
>    TcgStorageOpalUtil.c
> +  TcgStorageOpalLibInternal.h
> 
>  [LibraryClasses]
>    BaseLib
> diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
> b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
> new file mode 100644
> index 0000000000..cd16c51c3b
> --- /dev/null
> +++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
> @@ -0,0 +1,98 @@
> +/** @file
> +  Internal functions for Opal Core library.
> +
> +Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD
> License
> +which accompanies this distribution.  The full text of the license may be found
> at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> +
> +**/
> +
> +#ifndef _OPAL_INTERNAL_H_
> +#define _OPAL_INTERNAL_H_
> +
> +#include <Library/TcgStorageOpalLib.h>
> +
> +
> +/**
> +
> +  The function retrieves the MSID from the device specified
> +
> +  @param[in]  AdminSpSession              OPAL_SESSION with
> OPAL_UID_ADMIN_SP as OPAL_ADMIN_SP_ANYBODY_AUTHORITY
> +  @param[out] ActiveDataRemovalMechanism  Active Data Removal
> Mechanism that the device will use for Revert/RevertSP calls.
> +
> +**/
> +TCG_RESULT
> +EFIAPI
> +OpalPyrite2GetActiveDataRemovalMechanism (
> +  OPAL_SESSION    *AdminSpSession,
> +  UINT8           *ActiveDataRemovalMechanism
> +  );
> +
> +/**
> +
> +  Get the support attribute info.
> +
> +  @param[in]      Session             OPAL_SESSION with OPAL_UID_LOCKING_SP
> to retrieve info.
> +  @param[in]      FeatureCode         The feature code user request.
> +  @param[in, out] DataSize            The data size.
> +  @param[out]     Data                The data buffer used to save the feature
> descriptor.
> +
> +**/
> +TCG_RESULT
> +OpalGetFeatureDescriptor (
> +  IN     OPAL_SESSION              *Session,
> +  IN     UINT16                    FeatureCode,
> +  IN OUT UINTN                     *DataSize,
> +  OUT    VOID                      *Data
> +  );
> +
> +/**
> +  Get revert timeout value.
> +
> +  @param[in]      Session                       The session info for one opal device.
> +
> +**/
> +UINT32
> +GetRevertTimeOut (
> +  IN OPAL_SESSION                *Session
> +  );
> +
> +/**
> +
> +  Reverts device using Admin SP Revert method.
> +
> +  @param[in]  AdminSpSession      OPAL_SESSION with OPAL_UID_ADMIN_SP
> as OPAL_ADMIN_SP_PSID_AUTHORITY to perform PSID revert.
> +  @param[in]  EstimateTimeCost    Input the timeout value.
> +
> +**/
> +TCG_RESULT
> +OpalPyrite2PsidRevert(
> +  OPAL_SESSION              *AdminSpSession,
> +  UINT32                    EstimateTimeCost
> +  );
> +
> +/**
> +
> +  The function calls the Admin SP RevertSP method on the Locking SP.  If
> KeepUserData is True, then the optional parameter
> +  to keep the user Data is set to True, otherwise the optional parameter is not
> provided.
> +
> +  @param[in]      LockingSpSession    OPAL_SESSION with
> OPAL_UID_LOCKING_SP as OPAL_LOCKING_SP_ADMIN1_AUTHORITY to
> revertSP
> +  @param[in]      KeepUserData        Specifies whether or not to keep user
> Data when performing RevertSP action. True = keeps user Data.
> +  @param[in/out]  MethodStatus        Method status of last action performed.
> If action succeeded, it should be TCG_METHOD_STATUS_CODE_SUCCESS.
> +  @param[in]      EstimateTimeCost    Input the timeout value.
> +
> +**/
> +TCG_RESULT
> +OpalPyrite2AdminRevert(
> +  OPAL_SESSION    *LockingSpSession,
> +  BOOLEAN         KeepUserData,
> +  UINT8           *MethodStatus,
> +  UINT32          EstimateTimeCost
> +  );
> +
> +#endif // _OPAL_CORE_H_
> diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
> b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
> index f77fbe25c1..0a597a20be 100644
> --- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
> +++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Public API for Opal Core library.
> 
> -Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2016 ~ 2018, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -15,7 +15,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/TcgStorageOpalLib.h>
> +#include <TcgStorageOpalLibInternal.h>
> 
> +#define OPAL_MSID_LENGHT        128
> 
>  /**
>    Creates a session with OPAL_UID_ADMIN_SP as
> OPAL_ADMIN_SP_PSID_AUTHORITY, then reverts device using Admin SP Revert
> method.
> @@ -35,10 +37,14 @@ OpalUtilPsidRevert(
>  {
>    UINT8        MethodStatus;
>    TCG_RESULT   Ret;
> +  UINT32       RemovalTimeOut;
> 
>    NULL_CHECK(Session);
>    NULL_CHECK(Psid);
> 
> +  RemovalTimeOut = GetRevertTimeOut (Session);
> +  DEBUG ((DEBUG_INFO, "OpalUtilPsidRevert: Timeout value = %d\n",
> RemovalTimeOut));
> +
>    Ret = OpalStartSession(
>                        Session,
>                        OPAL_UID_ADMIN_SP,
> @@ -48,7 +54,7 @@ OpalUtilPsidRevert(
>                        OPAL_ADMIN_SP_PSID_AUTHORITY,
>                        &MethodStatus);
>    if (Ret == TcgResultSuccess && MethodStatus ==
> TCG_METHOD_STATUS_CODE_SUCCESS) {
> -    Ret = OpalPsidRevert(Session);
> +    Ret = OpalPyrite2PsidRevert(Session, RemovalTimeOut);
>      if (Ret != TcgResultSuccess) {
>        //
>        // If revert was successful, session was already ended by TPer, so only end
> session on failure
> @@ -599,12 +605,16 @@ OpalUtilRevert(
>  {
>    UINT8        MethodStatus;
>    TCG_RESULT   Ret;
> +  UINT32       RemovalTimeOut;
> 
>    NULL_CHECK(Session);
>    NULL_CHECK(Msid);
>    NULL_CHECK(Password);
>    NULL_CHECK(PasswordFailed);
> 
> +  RemovalTimeOut = GetRevertTimeOut (Session);
> +  DEBUG ((DEBUG_INFO, "OpalUtilRevert: Timeout value = %d\n",
> RemovalTimeOut));
> +
>    Ret = OpalStartSession(
>                     Session,
>                     OPAL_UID_LOCKING_SP,
> @@ -625,7 +635,7 @@ OpalUtilRevert(
>    //
>    // Try to revert with admin1
>    //
> -  Ret = OpalAdminRevert(Session, KeepUserData, &MethodStatus);
> +  Ret = OpalPyrite2AdminRevert(Session, KeepUserData, &MethodStatus,
> RemovalTimeOut);
>    if (Ret != TcgResultSuccess || MethodStatus !=
> TCG_METHOD_STATUS_CODE_SUCCESS) {
>      //
>      // Device ends the session on successful revert, so only call OpalEndSession
> when fail.
> @@ -912,3 +922,201 @@ OpalUtilAdminPasswordExists(
>    return (OwnerShip == OpalOwnershipUnknown && LockingFeature-
> >LockingEnabled);
>  }
> 
> +/**
> +  Get Active Data Removal Mechanism Value.
> +
> +  @param[in]      Session                        The session info for one opal device.
> +  @param[in]      GeneratedSid                   Generated SID of disk
> +  @param[in]      SidLength                      Length of generatedSid in bytes
> +  @param[out]     ActiveDataRemovalMechanism     Return the active data
> removal mechanism.
> +
> +**/
> +TCG_RESULT
> +EFIAPI
> +OpalUtilGetActiveDataRemovalMechanism (
> +  OPAL_SESSION      *Session,
> +  const VOID        *GeneratedSid,
> +  UINT32            SidLength,
> +  UINT8             *ActiveDataRemovalMechanism
> +  )
> +{
> +  TCG_RESULT   Ret;
> +  UINT8        MethodStatus;
> +
> +  NULL_CHECK(Session);
> +  NULL_CHECK(GeneratedSid);
> +  NULL_CHECK(ActiveDataRemovalMechanism);
> +
> +  Ret = OpalStartSession(
> +                    Session,
> +                    OPAL_UID_ADMIN_SP,
> +                    TRUE,
> +                    SidLength,
> +                    GeneratedSid,
> +                    OPAL_ADMIN_SP_ANYBODY_AUTHORITY,
> +                    &MethodStatus
> +                    );
> +  if (Ret != TcgResultSuccess || MethodStatus !=
> TCG_METHOD_STATUS_CODE_SUCCESS) {
> +    DEBUG ((DEBUG_INFO, "Start session with admin SP as SID authority failed:
> Ret=%d MethodStatus=%u\n", Ret, MethodStatus));
> +    if (MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
> +      Ret = TcgResultFailure;
> +    }
> +    return Ret;
> +  }
> +
> +  Ret = OpalPyrite2GetActiveDataRemovalMechanism (
> +                    Session,
> +                    ActiveDataRemovalMechanism
> +                    );
> +
> +  if (Ret != TcgResultSuccess) {
> +    DEBUG ((DEBUG_INFO, "Pyrite2 Get Active Data Removal Mechanism
> failed: Ret=%d\n", Ret));
> +  }
> +
> +  OpalEndSession(Session);
> +
> +  return Ret;
> +}
> +
> +/**
> +  Calculate the estimated time.
> +
> +  @param[in]      IsMinite               Whether the input time value is minute
> type or second type.
> +  @param[in]      Time                   The input time value.
> +
> +**/
> +UINT32
> +CalculateDataRemovalTime (
> +  IN BOOLEAN               IsMinute,
> +  IN UINT16                Time
> +  )
> +{
> +  if (IsMinute) {
> +    return Time * 2 * 60;
> +  } else {
> +    return Time * 2;
> +  }
> +}
> +
> +/**
> +  Return the estimated time for specific type.
> +
> +  @param[in]      Index               The input data removal type.
> +  @param[in]      Descriptor          DATA_REMOVAL_FEATURE_DESCRIPTOR
> +
> +**/
> +UINT32
> +GetDataRemovalTime (
> +  IN  UINT8                            Index,
> +  IN  DATA_REMOVAL_FEATURE_DESCRIPTOR  *Descriptor
> +  )
> +{
> +  switch (Index) {
> +  case OverwriteDataErase:
> +    return CalculateDataRemovalTime (Descriptor->FormatBit0, SwapBytes16
> (Descriptor->TimeBit0));
> +
> +  case BlockErase:
> +    return CalculateDataRemovalTime (Descriptor->FormatBit1, SwapBytes16
> (Descriptor->TimeBit1));
> +
> +  case CryptoErase:
> +    return CalculateDataRemovalTime (Descriptor->FormatBit2, SwapBytes16
> (Descriptor->TimeBit2));
> +
> +  case Unmap:
> +    return CalculateDataRemovalTime (Descriptor->FormatBit3, SwapBytes16
> (Descriptor->TimeBit3));
> +
> +  case ResetWritePointers:
> +    return CalculateDataRemovalTime (Descriptor->FormatBit4, SwapBytes16
> (Descriptor->TimeBit4));
> +
> +  case VendorSpecificErase:
> +    return CalculateDataRemovalTime (Descriptor->FormatBit5, SwapBytes16
> (Descriptor->TimeBit5));
> +
> +  default:
> +    return 0;
> +  }
> +}
> +
> +/**
> +  Get the supported Data Removal Mechanism list.
> +
> +  @param[in]      Session                        The session info for one opal device.
> +  @param[out]     RemovalMechanismLists          Return the supported data
> removal mechanism lists.
> +
> +**/
> +TCG_RESULT
> +EFIAPI
> +OpalUtilGetDataRemovalMechanismLists (
> +  IN  OPAL_SESSION      *Session,
> +  OUT UINT32            *RemovalMechanismLists
> +  )
> +{
> +  TCG_RESULT                       Ret;
> +  UINTN                            DataSize;
> +  DATA_REMOVAL_FEATURE_DESCRIPTOR  Descriptor;
> +  UINT8                            Index;
> +  UINT8                            BitValue;
> +
> +  NULL_CHECK(Session);
> +  NULL_CHECK(RemovalMechanismLists);
> +
> +  DataSize = sizeof (Descriptor);
> +  Ret = OpalGetFeatureDescriptor (Session, TCG_FEATURE_DATA_REMOVAL,
> &DataSize, &Descriptor);
> +  if (Ret != TcgResultSuccess) {
> +    return TcgResultFailure;
> +  }
> +
> +  ASSERT (Descriptor.RemovalMechanism != 0);
> +
> +  for (Index = 0; Index < ResearvedMechanism; Index ++) {
> +    BitValue = (BOOLEAN) BitFieldRead8 (Descriptor.RemovalMechanism,
> Index, Index);
> +
> +    if (BitValue == 0) {
> +      RemovalMechanismLists[Index] = 0;
> +    } else {
> +      RemovalMechanismLists[Index] = GetDataRemovalTime (Index,
> &Descriptor);
> +    }
> +  }
> +
> +  return TcgResultSuccess;
> +}
> +
> +/**
> +  Get revert timeout value.
> +
> +  @param[in]      Session                       The session info for one opal device.
> +
> +**/
> +UINT32
> +GetRevertTimeOut (
> +  IN OPAL_SESSION                *Session
> +  )
> +{
> +  TCG_RESULT                   TcgResult;
> +  OPAL_DISK_SUPPORT_ATTRIBUTE  SupportedAttributes;
> +  UINT16                       BaseComId;
> +  UINT32                       MsidLength;
> +  UINT8                        Msid[OPAL_MSID_LENGHT];
> +  UINT32                       RemovalMechanishLists[ResearvedMechanism];
> +  UINT8                        ActiveDataRemovalMechanism;
> +
> +  TcgResult = OpalGetSupportedAttributesInfo (Session, &SupportedAttributes,
> &BaseComId);
> +  if (TcgResult != TcgResultSuccess || SupportedAttributes.DataRemoval == 0)
> {
> +    return 0;
> +  }
> +
> +  TcgResult = OpalUtilGetMsid (Session, Msid, OPAL_MSID_LENGHT,
> &MsidLength);
> +  if (TcgResult != TcgResultSuccess) {
> +    return 0;
> +  }
> +
> +  TcgResult = OpalUtilGetDataRemovalMechanismLists (Session,
> RemovalMechanishLists);
> +  if (TcgResult != TcgResultSuccess) {
> +    return 0;
> +  }
> +
> +  TcgResult = OpalUtilGetActiveDataRemovalMechanism (Session, Msid,
> MsidLength, &ActiveDataRemovalMechanism);
> +  if (TcgResult != TcgResultSuccess) {
> +    return 0;
> +  }
> +
> +  return RemovalMechanishLists[ActiveDataRemovalMechanism];
> +}
> --
> 2.15.0.windows.1

[Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.

[Eric Dong]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c     | 60 ++++++++++++++--
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h     |  9 +++
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c        | 84 ++++++++++++++++++++++
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.c        |  1 +
 4 files changed, 147 insertions(+), 7 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
index 4133e503e2..91ab372f0c 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
@@ -105,10 +105,9 @@ OpalSupportGetAvailableActions(
   }
 
   //
-  // Psid revert is available for any device with media encryption support
-  // Revert is allowed for any device with media encryption support, however it requires
+  // Psid revert is available for any device with media encryption support or pyrite 2.0 type support.
   //
-  if (SupportedAttributes->MediaEncryption) {
+  if (SupportedAttributes->PyriteSscV2 || SupportedAttributes->MediaEncryption) {
 
     //
     // Only allow psid revert if media encryption is enabled.
@@ -657,6 +656,8 @@ OpalEndOfDxeEventNotify (
   @param[in]  Dev           The device which need Psid to process Psid Revert
                             OPAL request.
   @param[in]  PopUpString   Pop up string.
+  @param[in]  PopUpString2  Pop up string in line 2.
+
   @param[out] PressEsc      Whether user escape function through Press ESC.
 
   @retval Psid string if success. NULL if failed.
@@ -666,6 +667,7 @@ CHAR8 *
 OpalDriverPopUpPsidInput (
   IN OPAL_DRIVER_DEVICE     *Dev,
   IN CHAR16                 *PopUpString,
+  IN CHAR16                 *PopUpString2,
   OUT BOOLEAN               *PressEsc
   )
 {
@@ -689,6 +691,7 @@ OpalDriverPopUpPsidInput (
       EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
       &InputKey,
       PopUpString,
+      PopUpString2,
       L"---------------------",
       Mask,
       NULL
@@ -1369,6 +1372,8 @@ ProcessOpalRequestPsidRevert (
   EFI_INPUT_KEY         Key;
   TCG_RESULT            Ret;
   CHAR16                *PopUpString;
+  CHAR16                *PopUpString2;
+  UINTN                 BufferSize;
 
   if (Dev == NULL) {
     return;
@@ -1378,6 +1383,20 @@ ProcessOpalRequestPsidRevert (
 
   PopUpString = OpalGetPopUpString (Dev, RequestString);
 
+  if (Dev->OpalDisk.EstimateTimeCost > MAX_ACCEPTABLE_REVERTING_TIME) {
+    BufferSize = StrSize (L"Warning: Revert action will take about ###### seconds, DO NOT power off system during the revert action!");
+    PopUpString2 = AllocateZeroPool (BufferSize);
+    ASSERT (PopUpString2 != NULL);
+    UnicodeSPrint (
+        PopUpString2,
+        BufferSize,
+        L"WARNING: Revert action will take about %d seconds, DO NOT power off system during the revert action!",
+        Dev->OpalDisk.EstimateTimeCost
+      );
+  } else {
+    PopUpString2 = NULL;
+  }
+
   Count = 0;
 
   ZeroMem(&Session, sizeof(Session));
@@ -1386,7 +1405,7 @@ ProcessOpalRequestPsidRevert (
   Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
 
   while (Count < MAX_PSID_TRY_COUNT) {
-    Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, &PressEsc);
+    Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, PopUpString2, &PressEsc);
     if (PressEsc) {
         do {
           CreatePopUp (
@@ -1400,7 +1419,7 @@ ProcessOpalRequestPsidRevert (
 
         if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
           gST->ConOut->ClearScreen(gST->ConOut);
-          return;
+          goto Done;
         } else {
           //
           // Let user input Psid again.
@@ -1456,6 +1475,11 @@ ProcessOpalRequestPsidRevert (
     } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);
     gST->ConOut->ClearScreen(gST->ConOut);
   }
+
+Done:
+  if (PopUpString2 != NULL) {
+    FreePool (PopUpString2);
+  }
 }
 
 /**
@@ -1482,6 +1506,8 @@ ProcessOpalRequestRevert (
   TCG_RESULT            Ret;
   BOOLEAN               PasswordFailed;
   CHAR16                *PopUpString;
+  CHAR16                *PopUpString2;
+  UINTN                 BufferSize;
 
   if (Dev == NULL) {
     return;
@@ -1491,6 +1517,20 @@ ProcessOpalRequestRevert (
 
   PopUpString = OpalGetPopUpString (Dev, RequestString);
 
+  if (Dev->OpalDisk.EstimateTimeCost > MAX_ACCEPTABLE_REVERTING_TIME) {
+    BufferSize = StrSize (L"Warning: Revert action will take about ###### seconds, DO NOT power off system during the revert action!");
+    PopUpString2 = AllocateZeroPool (BufferSize);
+    ASSERT (PopUpString2 != NULL);
+    UnicodeSPrint (
+        PopUpString2,
+        BufferSize,
+        L"WARNING: Revert action will take about %d seconds, DO NOT power off system during the revert action!",
+        Dev->OpalDisk.EstimateTimeCost
+      );
+  } else {
+    PopUpString2 = NULL;
+  }
+
   Count = 0;
 
   ZeroMem(&Session, sizeof(Session));
@@ -1499,7 +1539,7 @@ ProcessOpalRequestRevert (
   Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
 
   while (Count < MAX_PASSWORD_TRY_COUNT) {
-    Password = OpalDriverPopUpPasswordInput (Dev, PopUpString, NULL, &PressEsc);
+    Password = OpalDriverPopUpPasswordInput (Dev, PopUpString, PopUpString2, &PressEsc);
     if (PressEsc) {
         do {
           CreatePopUp (
@@ -1513,7 +1553,7 @@ ProcessOpalRequestRevert (
 
         if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
           gST->ConOut->ClearScreen(gST->ConOut);
-          return;
+          goto Done;
         } else {
           //
           // Let user input password again.
@@ -1596,6 +1636,11 @@ ProcessOpalRequestRevert (
     } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);
     gST->ConOut->ClearScreen(gST->ConOut);
   }
+
+Done:
+  if (PopUpString2 != NULL) {
+    FreePool (PopUpString2);
+  }
 }
 
 /**
@@ -2337,6 +2382,7 @@ ReadyToBootCallback (
         Session.MediaId = Itr->OpalDisk.MediaId;
         Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
 
+        DEBUG ((DEBUG_INFO, "OpalPassword: ReadyToBoot point, send BlockSid command to device!\n"));
         Result = OpalBlockSid (&Session, TRUE);  // HardwareReset must always be TRUE
         if (Result != TcgResultSuccess) {
           DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
index 2154523e93..2fe7ada29b 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
@@ -75,6 +75,13 @@ extern EFI_COMPONENT_NAME2_PROTOCOL  gOpalComponentName2;
 #define PSID_CHARACTER_LENGTH   0x20
 #define MAX_PSID_TRY_COUNT      5
 
+//
+// The max timeout value assume the user can wait for the revert action.
+// If the revert time value bigger than this one, driver needs to popup a
+// dialog to let user confirm the revert action.
+//
+#define MAX_ACCEPTABLE_REVERTING_TIME    10
+
 #pragma pack(1)
 
 //
@@ -140,6 +147,8 @@ typedef struct {
   TCG_LOCKING_FEATURE_DESCRIPTOR                  LockingFeature;         // Locking Feature Descriptor retrieved from performing a Level 0 Discovery
   UINT8                                           PasswordLength;
   UINT8                                           Password[OPAL_MAX_PASSWORD_SIZE];
+
+  UINT32                                          EstimateTimeCost;
 } OPAL_DISK;
 
 //
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
index e4972227b6..479f413c07 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
@@ -13,6 +13,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 **/
 
 #include "OpalHii.h"
+//
+// Character definitions
+//
+#define UPPER_LOWER_CASE_OFFSET 0x20
 
 //
 // This is the generated IFR binary Data for each formset defined in VFR.
@@ -520,6 +524,59 @@ GetDiskNameStringId(
   return 0;
 }
 
+/**
+  Confirm whether user truly want to do the revert action.
+
+  @param     OpalDisk            The device which need to do the revert action.
+
+  @retval  EFI_SUCCESS           Confirmed user want to do the revert action.
+**/
+EFI_STATUS
+HiiConfirmRevertAction (
+  IN OPAL_DISK                  *OpalDisk
+
+  )
+{
+  CHAR16                        Unicode[512];
+  EFI_INPUT_KEY                 Key;
+  CHAR16                        ApproveResponse;
+  CHAR16                        RejectResponse;
+
+  //
+  // When the estimate cost time bigger than MAX_ACCEPTABLE_REVERTING_TIME, pop up dialog to let user confirm
+  // the revert action.
+  //
+  if (OpalDisk->EstimateTimeCost < MAX_ACCEPTABLE_REVERTING_TIME) {
+    return EFI_SUCCESS;
+  }
+
+  ApproveResponse = L'Y';
+  RejectResponse  = L'N';
+
+  UnicodeSPrint(Unicode, StrSize(L"WARNING: Revert device needs about ###### seconds"), L"WARNING: Revert device needs about %d seconds", OpalDisk->EstimateTimeCost);
+
+  do {
+    CreatePopUp(
+        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
+        &Key,
+        Unicode,
+        L" System should not be powered off until revert completion ",
+        L" ",
+        L" Press 'Y/y' to continue, press 'N/n' to cancal ",
+        NULL
+    );
+  } while (
+      ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) != (ApproveResponse | UPPER_LOWER_CASE_OFFSET)) &&
+      ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) != (RejectResponse | UPPER_LOWER_CASE_OFFSET))
+    );
+
+  if ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) == (RejectResponse | UPPER_LOWER_CASE_OFFSET)) {
+    return EFI_ABORTED;
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   This function processes the results of changes in configuration.
 
@@ -588,6 +645,17 @@ DriverCallback(
     switch (HiiKeyId) {
       case HII_KEY_ID_GOTO_DISK_INFO:
         return HiiSelectDisk((UINT8)HiiKey.KeyBits.Index);
+
+      case HII_KEY_ID_REVERT:
+      case HII_KEY_ID_PSID_REVERT:
+        OpalDisk = HiiGetOpalDiskCB(gHiiConfiguration.SelectedDiskIndex);
+        if (OpalDisk != NULL) {
+          return HiiConfirmRevertAction (OpalDisk);
+        } else {
+          ASSERT (FALSE);
+          return EFI_SUCCESS;
+        }
+
     }
   } else if (Action == EFI_BROWSER_ACTION_CHANGED) {
     switch (HiiKeyId) {
@@ -1112,6 +1180,8 @@ OpalDiskInitialize (
 {
   TCG_RESULT                  TcgResult;
   OPAL_SESSION                Session;
+  UINT8                       ActiveDataRemovalMechanism;
+  UINT32                      RemovalMechanishLists[ResearvedMechanism];
 
   ZeroMem(&Dev->OpalDisk, sizeof(OPAL_DISK));
   Dev->OpalDisk.Sscp = Dev->Sscp;
@@ -1133,6 +1203,20 @@ OpalDiskInitialize (
     return EFI_DEVICE_ERROR;
   }
 
+  if (Dev->OpalDisk.SupportedAttributes.DataRemoval) {
+    TcgResult = OpalUtilGetDataRemovalMechanismLists (&Session, RemovalMechanishLists);
+    if (TcgResult != TcgResultSuccess) {
+      return EFI_DEVICE_ERROR;
+    }
+
+    TcgResult = OpalUtilGetActiveDataRemovalMechanism (&Session, Dev->OpalDisk.Msid, Dev->OpalDisk.MsidLength, &ActiveDataRemovalMechanism);
+    if (TcgResult != TcgResultSuccess) {
+      return EFI_DEVICE_ERROR;
+    }
+
+    Dev->OpalDisk.EstimateTimeCost = RemovalMechanishLists[ActiveDataRemovalMechanism];
+  }
+
   return OpalDiskUpdateStatus (&Dev->OpalDisk);
 }
 
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
index b4b2d4b3f0..edb47ca8bc 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
@@ -635,6 +635,7 @@ UnlockOpalPassword (
     BlockSIDEnabled = FALSE;
   }
   if (BlockSIDEnabled && BlockSidSupport) {
+    DEBUG ((DEBUG_INFO, "OpalPassword: S3 phase send BlockSid command to device!\n"));
     ZeroMem(&Session, sizeof (Session));
     Session.Sscp = &OpalDev->Sscp;
     Session.MediaId = 0;
-- 
2.15.0.windows.1

Re: [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.

[Wu, Hao A]

Some comments below:

> -----Original Message-----
> From: Dong, Eric
> Sent: Thursday, May 03, 2018 11:17 AM
> To: edk2-devel@lists.01.org; Wu, Hao A
> Subject: [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 2.0
> devices.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c     | 60 ++++++++++++++--
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h     |  9 +++
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c        | 84
> ++++++++++++++++++++++
>  .../Tcg/Opal/OpalPassword/OpalPasswordPei.c        |  1 +
>  4 files changed, 147 insertions(+), 7 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index 4133e503e2..91ab372f0c 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -105,10 +105,9 @@ OpalSupportGetAvailableActions(
>    }
> 
>    //
> -  // Psid revert is available for any device with media encryption support
> -  // Revert is allowed for any device with media encryption support, however it
> requires
> +  // Psid revert is available for any device with media encryption support or
> pyrite 2.0 type support.
>    //
> -  if (SupportedAttributes->MediaEncryption) {
> +  if (SupportedAttributes->PyriteSscV2 || SupportedAttributes-
> >MediaEncryption) {
> 
>      //
>      // Only allow psid revert if media encryption is enabled.

For the comments here:
    //
    // Only allow psid revert if media encryption is enabled.
    // Otherwise, someone who steals a disk can psid revert the disk and the user Data is still
    // intact and accessible
    //
I think we'd better update it as well.

> @@ -657,6 +656,8 @@ OpalEndOfDxeEventNotify (
>    @param[in]  Dev           The device which need Psid to process Psid Revert
>                              OPAL request.
>    @param[in]  PopUpString   Pop up string.
> +  @param[in]  PopUpString2  Pop up string in line 2.
> +
>    @param[out] PressEsc      Whether user escape function through Press ESC.
> 
>    @retval Psid string if success. NULL if failed.
> @@ -666,6 +667,7 @@ CHAR8 *
>  OpalDriverPopUpPsidInput (
>    IN OPAL_DRIVER_DEVICE     *Dev,
>    IN CHAR16                 *PopUpString,
> +  IN CHAR16                 *PopUpString2,
>    OUT BOOLEAN               *PressEsc
>    )
>  {
> @@ -689,6 +691,7 @@ OpalDriverPopUpPsidInput (
>        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
>        &InputKey,
>        PopUpString,
> +      PopUpString2,
>        L"---------------------",
>        Mask,
>        NULL
> @@ -1369,6 +1372,8 @@ ProcessOpalRequestPsidRevert (
>    EFI_INPUT_KEY         Key;
>    TCG_RESULT            Ret;
>    CHAR16                *PopUpString;
> +  CHAR16                *PopUpString2;
> +  UINTN                 BufferSize;
> 
>    if (Dev == NULL) {
>      return;
> @@ -1378,6 +1383,20 @@ ProcessOpalRequestPsidRevert (
> 
>    PopUpString = OpalGetPopUpString (Dev, RequestString);
> 
> +  if (Dev->OpalDisk.EstimateTimeCost > MAX_ACCEPTABLE_REVERTING_TIME)
> {
> +    BufferSize = StrSize (L"Warning: Revert action will take about ######
> seconds, DO NOT power off system during the revert action!");

The 'BufferSize' seems not big enough to me.
Per my understanding, the possible max timeout value can be:
65534 * 2 * 60 = 7864080 seconds

So using '######' to keep space for 6 digits maybe not enough. Some characters
might get truncated.

> +    PopUpString2 = AllocateZeroPool (BufferSize);
> +    ASSERT (PopUpString2 != NULL);
> +    UnicodeSPrint (
> +        PopUpString2,
> +        BufferSize,
> +        L"WARNING: Revert action will take about %d seconds, DO NOT power
> off system during the revert action!",
> +        Dev->OpalDisk.EstimateTimeCost
> +      );
> +  } else {
> +    PopUpString2 = NULL;
> +  }
> +
>    Count = 0;
> 
>    ZeroMem(&Session, sizeof(Session));
> @@ -1386,7 +1405,7 @@ ProcessOpalRequestPsidRevert (
>    Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
> 
>    while (Count < MAX_PSID_TRY_COUNT) {
> -    Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, &PressEsc);
> +    Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, PopUpString2,
> &PressEsc);
>      if (PressEsc) {
>          do {
>            CreatePopUp (
> @@ -1400,7 +1419,7 @@ ProcessOpalRequestPsidRevert (
> 
>          if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
>            gST->ConOut->ClearScreen(gST->ConOut);
> -          return;
> +          goto Done;
>          } else {
>            //
>            // Let user input Psid again.
> @@ -1456,6 +1475,11 @@ ProcessOpalRequestPsidRevert (
>      } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);
>      gST->ConOut->ClearScreen(gST->ConOut);
>    }
> +
> +Done:
> +  if (PopUpString2 != NULL) {
> +    FreePool (PopUpString2);
> +  }
>  }
> 
>  /**
> @@ -1482,6 +1506,8 @@ ProcessOpalRequestRevert (
>    TCG_RESULT            Ret;
>    BOOLEAN               PasswordFailed;
>    CHAR16                *PopUpString;
> +  CHAR16                *PopUpString2;
> +  UINTN                 BufferSize;
> 
>    if (Dev == NULL) {
>      return;
> @@ -1491,6 +1517,20 @@ ProcessOpalRequestRevert (
> 
>    PopUpString = OpalGetPopUpString (Dev, RequestString);
> 
> +  if (Dev->OpalDisk.EstimateTimeCost > MAX_ACCEPTABLE_REVERTING_TIME)
> {
> +    BufferSize = StrSize (L"Warning: Revert action will take about ######
> seconds, DO NOT power off system during the revert action!");

Similar comments as above.

Using '######' to keep space for 6 digits maybe not enough. Some characters
might get truncated.

> +    PopUpString2 = AllocateZeroPool (BufferSize);
> +    ASSERT (PopUpString2 != NULL);
> +    UnicodeSPrint (
> +        PopUpString2,
> +        BufferSize,
> +        L"WARNING: Revert action will take about %d seconds, DO NOT power
> off system during the revert action!",
> +        Dev->OpalDisk.EstimateTimeCost
> +      );
> +  } else {
> +    PopUpString2 = NULL;
> +  }
> +
>    Count = 0;
> 
>    ZeroMem(&Session, sizeof(Session));
> @@ -1499,7 +1539,7 @@ ProcessOpalRequestRevert (
>    Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
> 
>    while (Count < MAX_PASSWORD_TRY_COUNT) {
> -    Password = OpalDriverPopUpPasswordInput (Dev, PopUpString, NULL,
> &PressEsc);
> +    Password = OpalDriverPopUpPasswordInput (Dev, PopUpString,
> PopUpString2, &PressEsc);
>      if (PressEsc) {
>          do {
>            CreatePopUp (
> @@ -1513,7 +1553,7 @@ ProcessOpalRequestRevert (
> 
>          if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
>            gST->ConOut->ClearScreen(gST->ConOut);
> -          return;
> +          goto Done;
>          } else {
>            //
>            // Let user input password again.
> @@ -1596,6 +1636,11 @@ ProcessOpalRequestRevert (
>      } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);
>      gST->ConOut->ClearScreen(gST->ConOut);
>    }
> +
> +Done:
> +  if (PopUpString2 != NULL) {
> +    FreePool (PopUpString2);
> +  }
>  }
> 
>  /**
> @@ -2337,6 +2382,7 @@ ReadyToBootCallback (
>          Session.MediaId = Itr->OpalDisk.MediaId;
>          Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
> 
> +        DEBUG ((DEBUG_INFO, "OpalPassword: ReadyToBoot point, send
> BlockSid command to device!\n"));
>          Result = OpalBlockSid (&Session, TRUE);  // HardwareReset must always
> be TRUE
>          if (Result != TcgResultSuccess) {
>            DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> index 2154523e93..2fe7ada29b 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> @@ -75,6 +75,13 @@ extern EFI_COMPONENT_NAME2_PROTOCOL
> gOpalComponentName2;
>  #define PSID_CHARACTER_LENGTH   0x20
>  #define MAX_PSID_TRY_COUNT      5
> 
> +//
> +// The max timeout value assume the user can wait for the revert action.
> +// If the revert time value bigger than this one, driver needs to popup a
> +// dialog to let user confirm the revert action.
> +//
> +#define MAX_ACCEPTABLE_REVERTING_TIME    10

It would be better to state that the unit of this macro is second.

> +
>  #pragma pack(1)
> 
>  //
> @@ -140,6 +147,8 @@ typedef struct {
>    TCG_LOCKING_FEATURE_DESCRIPTOR                  LockingFeature;         //
> Locking Feature Descriptor retrieved from performing a Level 0 Discovery
>    UINT8                                           PasswordLength;
>    UINT8                                           Password[OPAL_MAX_PASSWORD_SIZE];
> +
> +  UINT32                                          EstimateTimeCost;
>  } OPAL_DISK;
> 
>  //
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> index e4972227b6..479f413c07 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> @@ -13,6 +13,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  **/
> 
>  #include "OpalHii.h"
> +//
> +// Character definitions
> +//
> +#define UPPER_LOWER_CASE_OFFSET 0x20
> 
>  //
>  // This is the generated IFR binary Data for each formset defined in VFR.
> @@ -520,6 +524,59 @@ GetDiskNameStringId(
>    return 0;
>  }
> 
> +/**
> +  Confirm whether user truly want to do the revert action.
> +
> +  @param     OpalDisk            The device which need to do the revert action.
> +
> +  @retval  EFI_SUCCESS           Confirmed user want to do the revert action.
> +**/
> +EFI_STATUS
> +HiiConfirmRevertAction (
> +  IN OPAL_DISK                  *OpalDisk
> +
> +  )
> +{
> +  CHAR16                        Unicode[512];
> +  EFI_INPUT_KEY                 Key;
> +  CHAR16                        ApproveResponse;
> +  CHAR16                        RejectResponse;
> +
> +  //
> +  // When the estimate cost time bigger than
> MAX_ACCEPTABLE_REVERTING_TIME, pop up dialog to let user confirm
> +  // the revert action.
> +  //
> +  if (OpalDisk->EstimateTimeCost < MAX_ACCEPTABLE_REVERTING_TIME) {
> +    return EFI_SUCCESS;
> +  }
> +
> +  ApproveResponse = L'Y';
> +  RejectResponse  = L'N';
> +
> +  UnicodeSPrint(Unicode, StrSize(L"WARNING: Revert device needs about
> ###### seconds"), L"WARNING: Revert device needs about %d seconds",
> OpalDisk->EstimateTimeCost);

Again, using '######' to keep space for 6 digits maybe not enough. Some
characters might get truncated.

Best Regards,
Hao Wu

> +
> +  do {
> +    CreatePopUp(
> +        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
> +        &Key,
> +        Unicode,
> +        L" System should not be powered off until revert completion ",
> +        L" ",
> +        L" Press 'Y/y' to continue, press 'N/n' to cancal ",
> +        NULL
> +    );
> +  } while (
> +      ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) != (ApproveResponse
> | UPPER_LOWER_CASE_OFFSET)) &&
> +      ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) != (RejectResponse |
> UPPER_LOWER_CASE_OFFSET))
> +    );
> +
> +  if ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) == (RejectResponse |
> UPPER_LOWER_CASE_OFFSET)) {
> +    return EFI_ABORTED;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    This function processes the results of changes in configuration.
> 
> @@ -588,6 +645,17 @@ DriverCallback(
>      switch (HiiKeyId) {
>        case HII_KEY_ID_GOTO_DISK_INFO:
>          return HiiSelectDisk((UINT8)HiiKey.KeyBits.Index);
> +
> +      case HII_KEY_ID_REVERT:
> +      case HII_KEY_ID_PSID_REVERT:
> +        OpalDisk = HiiGetOpalDiskCB(gHiiConfiguration.SelectedDiskIndex);
> +        if (OpalDisk != NULL) {
> +          return HiiConfirmRevertAction (OpalDisk);
> +        } else {
> +          ASSERT (FALSE);
> +          return EFI_SUCCESS;
> +        }
> +
>      }
>    } else if (Action == EFI_BROWSER_ACTION_CHANGED) {
>      switch (HiiKeyId) {
> @@ -1112,6 +1180,8 @@ OpalDiskInitialize (
>  {
>    TCG_RESULT                  TcgResult;
>    OPAL_SESSION                Session;
> +  UINT8                       ActiveDataRemovalMechanism;
> +  UINT32                      RemovalMechanishLists[ResearvedMechanism];
> 
>    ZeroMem(&Dev->OpalDisk, sizeof(OPAL_DISK));
>    Dev->OpalDisk.Sscp = Dev->Sscp;
> @@ -1133,6 +1203,20 @@ OpalDiskInitialize (
>      return EFI_DEVICE_ERROR;
>    }
> 
> +  if (Dev->OpalDisk.SupportedAttributes.DataRemoval) {
> +    TcgResult = OpalUtilGetDataRemovalMechanismLists (&Session,
> RemovalMechanishLists);
> +    if (TcgResult != TcgResultSuccess) {
> +      return EFI_DEVICE_ERROR;
> +    }
> +
> +    TcgResult = OpalUtilGetActiveDataRemovalMechanism (&Session, Dev-
> >OpalDisk.Msid, Dev->OpalDisk.MsidLength, &ActiveDataRemovalMechanism);
> +    if (TcgResult != TcgResultSuccess) {
> +      return EFI_DEVICE_ERROR;
> +    }
> +
> +    Dev->OpalDisk.EstimateTimeCost =
> RemovalMechanishLists[ActiveDataRemovalMechanism];
> +  }
> +
>    return OpalDiskUpdateStatus (&Dev->OpalDisk);
>  }
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> index b4b2d4b3f0..edb47ca8bc 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> @@ -635,6 +635,7 @@ UnlockOpalPassword (
>      BlockSIDEnabled = FALSE;
>    }
>    if (BlockSIDEnabled && BlockSidSupport) {
> +    DEBUG ((DEBUG_INFO, "OpalPassword: S3 phase send BlockSid command
> to device!\n"));
>      ZeroMem(&Session, sizeof (Session));
>      Session.Sscp = &OpalDev->Sscp;
>      Session.MediaId = 0;
> --
> 2.15.0.windows.1

Re: [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.

[Dong, Eric]

Hi Hao,

Thanks for your comments, I will update it when I check in the code.

Thanks,
Eric

-----Original Message-----
From: Wu, Hao A 
Sent: Monday, May 7, 2018 11:08 AM
To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org
Subject: RE: [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.

Some comments below:

> -----Original Message-----
> From: Dong, Eric
> Sent: Thursday, May 03, 2018 11:17 AM
> To: edk2-devel@lists.01.org; Wu, Hao A
> Subject: [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 
> 2.0 devices.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c     | 60 ++++++++++++++--
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h     |  9 +++
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c        | 84
> ++++++++++++++++++++++
>  .../Tcg/Opal/OpalPassword/OpalPasswordPei.c        |  1 +
>  4 files changed, 147 insertions(+), 7 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index 4133e503e2..91ab372f0c 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -105,10 +105,9 @@ OpalSupportGetAvailableActions(
>    }
> 
>    //
> -  // Psid revert is available for any device with media encryption 
> support
> -  // Revert is allowed for any device with media encryption support, 
> however it requires
> +  // Psid revert is available for any device with media encryption 
> + support or
> pyrite 2.0 type support.
>    //
> -  if (SupportedAttributes->MediaEncryption) {
> +  if (SupportedAttributes->PyriteSscV2 || SupportedAttributes-
> >MediaEncryption) {
> 
>      //
>      // Only allow psid revert if media encryption is enabled.

For the comments here:
    //
    // Only allow psid revert if media encryption is enabled.
    // Otherwise, someone who steals a disk can psid revert the disk and the user Data is still
    // intact and accessible
    //
I think we'd better update it as well.

> @@ -657,6 +656,8 @@ OpalEndOfDxeEventNotify (
>    @param[in]  Dev           The device which need Psid to process Psid Revert
>                              OPAL request.
>    @param[in]  PopUpString   Pop up string.
> +  @param[in]  PopUpString2  Pop up string in line 2.
> +
>    @param[out] PressEsc      Whether user escape function through Press ESC.
> 
>    @retval Psid string if success. NULL if failed.
> @@ -666,6 +667,7 @@ CHAR8 *
>  OpalDriverPopUpPsidInput (
>    IN OPAL_DRIVER_DEVICE     *Dev,
>    IN CHAR16                 *PopUpString,
> +  IN CHAR16                 *PopUpString2,
>    OUT BOOLEAN               *PressEsc
>    )
>  {
> @@ -689,6 +691,7 @@ OpalDriverPopUpPsidInput (
>        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
>        &InputKey,
>        PopUpString,
> +      PopUpString2,
>        L"---------------------",
>        Mask,
>        NULL
> @@ -1369,6 +1372,8 @@ ProcessOpalRequestPsidRevert (
>    EFI_INPUT_KEY         Key;
>    TCG_RESULT            Ret;
>    CHAR16                *PopUpString;
> +  CHAR16                *PopUpString2;
> +  UINTN                 BufferSize;
> 
>    if (Dev == NULL) {
>      return;
> @@ -1378,6 +1383,20 @@ ProcessOpalRequestPsidRevert (
> 
>    PopUpString = OpalGetPopUpString (Dev, RequestString);
> 
> +  if (Dev->OpalDisk.EstimateTimeCost > MAX_ACCEPTABLE_REVERTING_TIME)
> {
> +    BufferSize = StrSize (L"Warning: Revert action will take about 
> + ######
> seconds, DO NOT power off system during the revert action!");

The 'BufferSize' seems not big enough to me.
Per my understanding, the possible max timeout value can be:
65534 * 2 * 60 = 7864080 seconds

So using '######' to keep space for 6 digits maybe not enough. Some characters might get truncated.

> +    PopUpString2 = AllocateZeroPool (BufferSize);
> +    ASSERT (PopUpString2 != NULL);
> +    UnicodeSPrint (
> +        PopUpString2,
> +        BufferSize,
> +        L"WARNING: Revert action will take about %d seconds, DO NOT 
> + power
> off system during the revert action!",
> +        Dev->OpalDisk.EstimateTimeCost
> +      );
> +  } else {
> +    PopUpString2 = NULL;
> +  }
> +
>    Count = 0;
> 
>    ZeroMem(&Session, sizeof(Session)); @@ -1386,7 +1405,7 @@ 
> ProcessOpalRequestPsidRevert (
>    Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
> 
>    while (Count < MAX_PSID_TRY_COUNT) {
> -    Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, &PressEsc);
> +    Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, PopUpString2,
> &PressEsc);
>      if (PressEsc) {
>          do {
>            CreatePopUp (
> @@ -1400,7 +1419,7 @@ ProcessOpalRequestPsidRevert (
> 
>          if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
>            gST->ConOut->ClearScreen(gST->ConOut);
> -          return;
> +          goto Done;
>          } else {
>            //
>            // Let user input Psid again.
> @@ -1456,6 +1475,11 @@ ProcessOpalRequestPsidRevert (
>      } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);
>      gST->ConOut->ClearScreen(gST->ConOut);
>    }
> +
> +Done:
> +  if (PopUpString2 != NULL) {
> +    FreePool (PopUpString2);
> +  }
>  }
> 
>  /**
> @@ -1482,6 +1506,8 @@ ProcessOpalRequestRevert (
>    TCG_RESULT            Ret;
>    BOOLEAN               PasswordFailed;
>    CHAR16                *PopUpString;
> +  CHAR16                *PopUpString2;
> +  UINTN                 BufferSize;
> 
>    if (Dev == NULL) {
>      return;
> @@ -1491,6 +1517,20 @@ ProcessOpalRequestRevert (
> 
>    PopUpString = OpalGetPopUpString (Dev, RequestString);
> 
> +  if (Dev->OpalDisk.EstimateTimeCost > MAX_ACCEPTABLE_REVERTING_TIME)
> {
> +    BufferSize = StrSize (L"Warning: Revert action will take about 
> + ######
> seconds, DO NOT power off system during the revert action!");

Similar comments as above.

Using '######' to keep space for 6 digits maybe not enough. Some characters might get truncated.

> +    PopUpString2 = AllocateZeroPool (BufferSize);
> +    ASSERT (PopUpString2 != NULL);
> +    UnicodeSPrint (
> +        PopUpString2,
> +        BufferSize,
> +        L"WARNING: Revert action will take about %d seconds, DO NOT 
> + power
> off system during the revert action!",
> +        Dev->OpalDisk.EstimateTimeCost
> +      );
> +  } else {
> +    PopUpString2 = NULL;
> +  }
> +
>    Count = 0;
> 
>    ZeroMem(&Session, sizeof(Session)); @@ -1499,7 +1539,7 @@ 
> ProcessOpalRequestRevert (
>    Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
> 
>    while (Count < MAX_PASSWORD_TRY_COUNT) {
> -    Password = OpalDriverPopUpPasswordInput (Dev, PopUpString, NULL,
> &PressEsc);
> +    Password = OpalDriverPopUpPasswordInput (Dev, PopUpString,
> PopUpString2, &PressEsc);
>      if (PressEsc) {
>          do {
>            CreatePopUp (
> @@ -1513,7 +1553,7 @@ ProcessOpalRequestRevert (
> 
>          if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
>            gST->ConOut->ClearScreen(gST->ConOut);
> -          return;
> +          goto Done;
>          } else {
>            //
>            // Let user input password again.
> @@ -1596,6 +1636,11 @@ ProcessOpalRequestRevert (
>      } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);
>      gST->ConOut->ClearScreen(gST->ConOut);
>    }
> +
> +Done:
> +  if (PopUpString2 != NULL) {
> +    FreePool (PopUpString2);
> +  }
>  }
> 
>  /**
> @@ -2337,6 +2382,7 @@ ReadyToBootCallback (
>          Session.MediaId = Itr->OpalDisk.MediaId;
>          Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
> 
> +        DEBUG ((DEBUG_INFO, "OpalPassword: ReadyToBoot point, send
> BlockSid command to device!\n"));
>          Result = OpalBlockSid (&Session, TRUE);  // HardwareReset 
> must always be TRUE
>          if (Result != TcgResultSuccess) {
>            DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n")); diff --git 
> a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> index 2154523e93..2fe7ada29b 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> @@ -75,6 +75,13 @@ extern EFI_COMPONENT_NAME2_PROTOCOL 
> gOpalComponentName2;
>  #define PSID_CHARACTER_LENGTH   0x20
>  #define MAX_PSID_TRY_COUNT      5
> 
> +//
> +// The max timeout value assume the user can wait for the revert action.
> +// If the revert time value bigger than this one, driver needs to 
> +popup a // dialog to let user confirm the revert action.
> +//
> +#define MAX_ACCEPTABLE_REVERTING_TIME    10

It would be better to state that the unit of this macro is second.

> +
>  #pragma pack(1)
> 
>  //
> @@ -140,6 +147,8 @@ typedef struct {
>    TCG_LOCKING_FEATURE_DESCRIPTOR                  LockingFeature;         //
> Locking Feature Descriptor retrieved from performing a Level 0 Discovery
>    UINT8                                           PasswordLength;
>    UINT8                                           Password[OPAL_MAX_PASSWORD_SIZE];
> +
> +  UINT32                                          EstimateTimeCost;
>  } OPAL_DISK;
> 
>  //
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> index e4972227b6..479f413c07 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> @@ -13,6 +13,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
> EITHER EXPRESS OR IMPLIED.
>  **/
> 
>  #include "OpalHii.h"
> +//
> +// Character definitions
> +//
> +#define UPPER_LOWER_CASE_OFFSET 0x20
> 
>  //
>  // This is the generated IFR binary Data for each formset defined in VFR.
> @@ -520,6 +524,59 @@ GetDiskNameStringId(
>    return 0;
>  }
> 
> +/**
> +  Confirm whether user truly want to do the revert action.
> +
> +  @param     OpalDisk            The device which need to do the revert action.
> +
> +  @retval  EFI_SUCCESS           Confirmed user want to do the revert action.
> +**/
> +EFI_STATUS
> +HiiConfirmRevertAction (
> +  IN OPAL_DISK                  *OpalDisk
> +
> +  )
> +{
> +  CHAR16                        Unicode[512];
> +  EFI_INPUT_KEY                 Key;
> +  CHAR16                        ApproveResponse;
> +  CHAR16                        RejectResponse;
> +
> +  //
> +  // When the estimate cost time bigger than
> MAX_ACCEPTABLE_REVERTING_TIME, pop up dialog to let user confirm
> +  // the revert action.
> +  //
> +  if (OpalDisk->EstimateTimeCost < MAX_ACCEPTABLE_REVERTING_TIME) {
> +    return EFI_SUCCESS;
> +  }
> +
> +  ApproveResponse = L'Y';
> +  RejectResponse  = L'N';
> +
> +  UnicodeSPrint(Unicode, StrSize(L"WARNING: Revert device needs about
> ###### seconds"), L"WARNING: Revert device needs about %d seconds",
> OpalDisk->EstimateTimeCost);

Again, using '######' to keep space for 6 digits maybe not enough. Some characters might get truncated.

Best Regards,
Hao Wu

> +
> +  do {
> +    CreatePopUp(
> +        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
> +        &Key,
> +        Unicode,
> +        L" System should not be powered off until revert completion ",
> +        L" ",
> +        L" Press 'Y/y' to continue, press 'N/n' to cancal ",
> +        NULL
> +    );
> +  } while (
> +      ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) != 
> + (ApproveResponse
> | UPPER_LOWER_CASE_OFFSET)) &&
> +      ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) != (RejectResponse 
> + |
> UPPER_LOWER_CASE_OFFSET))
> +    );
> +
> +  if ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) == (RejectResponse 
> + |
> UPPER_LOWER_CASE_OFFSET)) {
> +    return EFI_ABORTED;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    This function processes the results of changes in configuration.
> 
> @@ -588,6 +645,17 @@ DriverCallback(
>      switch (HiiKeyId) {
>        case HII_KEY_ID_GOTO_DISK_INFO:
>          return HiiSelectDisk((UINT8)HiiKey.KeyBits.Index);
> +
> +      case HII_KEY_ID_REVERT:
> +      case HII_KEY_ID_PSID_REVERT:
> +        OpalDisk = HiiGetOpalDiskCB(gHiiConfiguration.SelectedDiskIndex);
> +        if (OpalDisk != NULL) {
> +          return HiiConfirmRevertAction (OpalDisk);
> +        } else {
> +          ASSERT (FALSE);
> +          return EFI_SUCCESS;
> +        }
> +
>      }
>    } else if (Action == EFI_BROWSER_ACTION_CHANGED) {
>      switch (HiiKeyId) {
> @@ -1112,6 +1180,8 @@ OpalDiskInitialize (  {
>    TCG_RESULT                  TcgResult;
>    OPAL_SESSION                Session;
> +  UINT8                       ActiveDataRemovalMechanism;
> +  UINT32                      RemovalMechanishLists[ResearvedMechanism];
> 
>    ZeroMem(&Dev->OpalDisk, sizeof(OPAL_DISK));
>    Dev->OpalDisk.Sscp = Dev->Sscp;
> @@ -1133,6 +1203,20 @@ OpalDiskInitialize (
>      return EFI_DEVICE_ERROR;
>    }
> 
> +  if (Dev->OpalDisk.SupportedAttributes.DataRemoval) {
> +    TcgResult = OpalUtilGetDataRemovalMechanismLists (&Session,
> RemovalMechanishLists);
> +    if (TcgResult != TcgResultSuccess) {
> +      return EFI_DEVICE_ERROR;
> +    }
> +
> +    TcgResult = OpalUtilGetActiveDataRemovalMechanism (&Session, Dev-
> >OpalDisk.Msid, Dev->OpalDisk.MsidLength, 
> >&ActiveDataRemovalMechanism);
> +    if (TcgResult != TcgResultSuccess) {
> +      return EFI_DEVICE_ERROR;
> +    }
> +
> +    Dev->OpalDisk.EstimateTimeCost =
> RemovalMechanishLists[ActiveDataRemovalMechanism];
> +  }
> +
>    return OpalDiskUpdateStatus (&Dev->OpalDisk);  }
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> index b4b2d4b3f0..edb47ca8bc 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> @@ -635,6 +635,7 @@ UnlockOpalPassword (
>      BlockSIDEnabled = FALSE;
>    }
>    if (BlockSIDEnabled && BlockSidSupport) {
> +    DEBUG ((DEBUG_INFO, "OpalPassword: S3 phase send BlockSid command
> to device!\n"));
>      ZeroMem(&Session, sizeof (Session));
>      Session.Sscp = &OpalDev->Sscp;
>      Session.MediaId = 0;
> --
> 2.15.0.windows.1

Re: [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.

[Wu, Hao A]

Thanks and with the changes:
Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> -----Original Message-----
> From: Dong, Eric
> Sent: Monday, May 07, 2018 1:33 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Subject: RE: [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 2.0
> devices.
> 
> Hi Hao,
> 
> Thanks for your comments, I will update it when I check in the code.
> 
> Thanks,
> Eric
> 
> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, May 7, 2018 11:08 AM
> To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org
> Subject: RE: [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite 2.0
> devices.
> 
> Some comments below:
> 
> > -----Original Message-----
> > From: Dong, Eric
> > Sent: Thursday, May 03, 2018 11:17 AM
> > To: edk2-devel@lists.01.org; Wu, Hao A
> > Subject: [Patch 3/3] SecurityPkg/OpalPassword: Add support for pyrite
> > 2.0 devices.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > ---
> >  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c     | 60 ++++++++++++++--
> >  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h     |  9 +++
> >  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c        | 84
> > ++++++++++++++++++++++
> >  .../Tcg/Opal/OpalPassword/OpalPasswordPei.c        |  1 +
> >  4 files changed, 147 insertions(+), 7 deletions(-)
> >
> > diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> > b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> > index 4133e503e2..91ab372f0c 100644
> > --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> > +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> > @@ -105,10 +105,9 @@ OpalSupportGetAvailableActions(
> >    }
> >
> >    //
> > -  // Psid revert is available for any device with media encryption
> > support
> > -  // Revert is allowed for any device with media encryption support,
> > however it requires
> > +  // Psid revert is available for any device with media encryption
> > + support or
> > pyrite 2.0 type support.
> >    //
> > -  if (SupportedAttributes->MediaEncryption) {
> > +  if (SupportedAttributes->PyriteSscV2 || SupportedAttributes-
> > >MediaEncryption) {
> >
> >      //
> >      // Only allow psid revert if media encryption is enabled.
> 
> For the comments here:
>     //
>     // Only allow psid revert if media encryption is enabled.
>     // Otherwise, someone who steals a disk can psid revert the disk and the user
> Data is still
>     // intact and accessible
>     //
> I think we'd better update it as well.
> 
> > @@ -657,6 +656,8 @@ OpalEndOfDxeEventNotify (
> >    @param[in]  Dev           The device which need Psid to process Psid Revert
> >                              OPAL request.
> >    @param[in]  PopUpString   Pop up string.
> > +  @param[in]  PopUpString2  Pop up string in line 2.
> > +
> >    @param[out] PressEsc      Whether user escape function through Press ESC.
> >
> >    @retval Psid string if success. NULL if failed.
> > @@ -666,6 +667,7 @@ CHAR8 *
> >  OpalDriverPopUpPsidInput (
> >    IN OPAL_DRIVER_DEVICE     *Dev,
> >    IN CHAR16                 *PopUpString,
> > +  IN CHAR16                 *PopUpString2,
> >    OUT BOOLEAN               *PressEsc
> >    )
> >  {
> > @@ -689,6 +691,7 @@ OpalDriverPopUpPsidInput (
> >        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
> >        &InputKey,
> >        PopUpString,
> > +      PopUpString2,
> >        L"---------------------",
> >        Mask,
> >        NULL
> > @@ -1369,6 +1372,8 @@ ProcessOpalRequestPsidRevert (
> >    EFI_INPUT_KEY         Key;
> >    TCG_RESULT            Ret;
> >    CHAR16                *PopUpString;
> > +  CHAR16                *PopUpString2;
> > +  UINTN                 BufferSize;
> >
> >    if (Dev == NULL) {
> >      return;
> > @@ -1378,6 +1383,20 @@ ProcessOpalRequestPsidRevert (
> >
> >    PopUpString = OpalGetPopUpString (Dev, RequestString);
> >
> > +  if (Dev->OpalDisk.EstimateTimeCost >
> MAX_ACCEPTABLE_REVERTING_TIME)
> > {
> > +    BufferSize = StrSize (L"Warning: Revert action will take about
> > + ######
> > seconds, DO NOT power off system during the revert action!");
> 
> The 'BufferSize' seems not big enough to me.
> Per my understanding, the possible max timeout value can be:
> 65534 * 2 * 60 = 7864080 seconds
> 
> So using '######' to keep space for 6 digits maybe not enough. Some
> characters might get truncated.
> 
> > +    PopUpString2 = AllocateZeroPool (BufferSize);
> > +    ASSERT (PopUpString2 != NULL);
> > +    UnicodeSPrint (
> > +        PopUpString2,
> > +        BufferSize,
> > +        L"WARNING: Revert action will take about %d seconds, DO NOT
> > + power
> > off system during the revert action!",
> > +        Dev->OpalDisk.EstimateTimeCost
> > +      );
> > +  } else {
> > +    PopUpString2 = NULL;
> > +  }
> > +
> >    Count = 0;
> >
> >    ZeroMem(&Session, sizeof(Session)); @@ -1386,7 +1405,7 @@
> > ProcessOpalRequestPsidRevert (
> >    Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
> >
> >    while (Count < MAX_PSID_TRY_COUNT) {
> > -    Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, &PressEsc);
> > +    Psid = OpalDriverPopUpPsidInput (Dev, PopUpString, PopUpString2,
> > &PressEsc);
> >      if (PressEsc) {
> >          do {
> >            CreatePopUp (
> > @@ -1400,7 +1419,7 @@ ProcessOpalRequestPsidRevert (
> >
> >          if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
> >            gST->ConOut->ClearScreen(gST->ConOut);
> > -          return;
> > +          goto Done;
> >          } else {
> >            //
> >            // Let user input Psid again.
> > @@ -1456,6 +1475,11 @@ ProcessOpalRequestPsidRevert (
> >      } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);
> >      gST->ConOut->ClearScreen(gST->ConOut);
> >    }
> > +
> > +Done:
> > +  if (PopUpString2 != NULL) {
> > +    FreePool (PopUpString2);
> > +  }
> >  }
> >
> >  /**
> > @@ -1482,6 +1506,8 @@ ProcessOpalRequestRevert (
> >    TCG_RESULT            Ret;
> >    BOOLEAN               PasswordFailed;
> >    CHAR16                *PopUpString;
> > +  CHAR16                *PopUpString2;
> > +  UINTN                 BufferSize;
> >
> >    if (Dev == NULL) {
> >      return;
> > @@ -1491,6 +1517,20 @@ ProcessOpalRequestRevert (
> >
> >    PopUpString = OpalGetPopUpString (Dev, RequestString);
> >
> > +  if (Dev->OpalDisk.EstimateTimeCost >
> MAX_ACCEPTABLE_REVERTING_TIME)
> > {
> > +    BufferSize = StrSize (L"Warning: Revert action will take about
> > + ######
> > seconds, DO NOT power off system during the revert action!");
> 
> Similar comments as above.
> 
> Using '######' to keep space for 6 digits maybe not enough. Some characters
> might get truncated.
> 
> > +    PopUpString2 = AllocateZeroPool (BufferSize);
> > +    ASSERT (PopUpString2 != NULL);
> > +    UnicodeSPrint (
> > +        PopUpString2,
> > +        BufferSize,
> > +        L"WARNING: Revert action will take about %d seconds, DO NOT
> > + power
> > off system during the revert action!",
> > +        Dev->OpalDisk.EstimateTimeCost
> > +      );
> > +  } else {
> > +    PopUpString2 = NULL;
> > +  }
> > +
> >    Count = 0;
> >
> >    ZeroMem(&Session, sizeof(Session)); @@ -1499,7 +1539,7 @@
> > ProcessOpalRequestRevert (
> >    Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
> >
> >    while (Count < MAX_PASSWORD_TRY_COUNT) {
> > -    Password = OpalDriverPopUpPasswordInput (Dev, PopUpString, NULL,
> > &PressEsc);
> > +    Password = OpalDriverPopUpPasswordInput (Dev, PopUpString,
> > PopUpString2, &PressEsc);
> >      if (PressEsc) {
> >          do {
> >            CreatePopUp (
> > @@ -1513,7 +1553,7 @@ ProcessOpalRequestRevert (
> >
> >          if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
> >            gST->ConOut->ClearScreen(gST->ConOut);
> > -          return;
> > +          goto Done;
> >          } else {
> >            //
> >            // Let user input password again.
> > @@ -1596,6 +1636,11 @@ ProcessOpalRequestRevert (
> >      } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);
> >      gST->ConOut->ClearScreen(gST->ConOut);
> >    }
> > +
> > +Done:
> > +  if (PopUpString2 != NULL) {
> > +    FreePool (PopUpString2);
> > +  }
> >  }
> >
> >  /**
> > @@ -2337,6 +2382,7 @@ ReadyToBootCallback (
> >          Session.MediaId = Itr->OpalDisk.MediaId;
> >          Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
> >
> > +        DEBUG ((DEBUG_INFO, "OpalPassword: ReadyToBoot point, send
> > BlockSid command to device!\n"));
> >          Result = OpalBlockSid (&Session, TRUE);  // HardwareReset
> > must always be TRUE
> >          if (Result != TcgResultSuccess) {
> >            DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n")); diff --git
> > a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> > b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> > index 2154523e93..2fe7ada29b 100644
> > --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> > +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> > @@ -75,6 +75,13 @@ extern EFI_COMPONENT_NAME2_PROTOCOL
> > gOpalComponentName2;
> >  #define PSID_CHARACTER_LENGTH   0x20
> >  #define MAX_PSID_TRY_COUNT      5
> >
> > +//
> > +// The max timeout value assume the user can wait for the revert action.
> > +// If the revert time value bigger than this one, driver needs to
> > +popup a // dialog to let user confirm the revert action.
> > +//
> > +#define MAX_ACCEPTABLE_REVERTING_TIME    10
> 
> It would be better to state that the unit of this macro is second.
> 
> > +
> >  #pragma pack(1)
> >
> >  //
> > @@ -140,6 +147,8 @@ typedef struct {
> >    TCG_LOCKING_FEATURE_DESCRIPTOR                  LockingFeature;         //
> > Locking Feature Descriptor retrieved from performing a Level 0 Discovery
> >    UINT8                                           PasswordLength;
> >    UINT8                                           Password[OPAL_MAX_PASSWORD_SIZE];
> > +
> > +  UINT32                                          EstimateTimeCost;
> >  } OPAL_DISK;
> >
> >  //
> > diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> > b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> > index e4972227b6..479f413c07 100644
> > --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> > +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> > @@ -13,6 +13,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND,
> > EITHER EXPRESS OR IMPLIED.
> >  **/
> >
> >  #include "OpalHii.h"
> > +//
> > +// Character definitions
> > +//
> > +#define UPPER_LOWER_CASE_OFFSET 0x20
> >
> >  //
> >  // This is the generated IFR binary Data for each formset defined in VFR.
> > @@ -520,6 +524,59 @@ GetDiskNameStringId(
> >    return 0;
> >  }
> >
> > +/**
> > +  Confirm whether user truly want to do the revert action.
> > +
> > +  @param     OpalDisk            The device which need to do the revert action.
> > +
> > +  @retval  EFI_SUCCESS           Confirmed user want to do the revert action.
> > +**/
> > +EFI_STATUS
> > +HiiConfirmRevertAction (
> > +  IN OPAL_DISK                  *OpalDisk
> > +
> > +  )
> > +{
> > +  CHAR16                        Unicode[512];
> > +  EFI_INPUT_KEY                 Key;
> > +  CHAR16                        ApproveResponse;
> > +  CHAR16                        RejectResponse;
> > +
> > +  //
> > +  // When the estimate cost time bigger than
> > MAX_ACCEPTABLE_REVERTING_TIME, pop up dialog to let user confirm
> > +  // the revert action.
> > +  //
> > +  if (OpalDisk->EstimateTimeCost < MAX_ACCEPTABLE_REVERTING_TIME) {
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  ApproveResponse = L'Y';
> > +  RejectResponse  = L'N';
> > +
> > +  UnicodeSPrint(Unicode, StrSize(L"WARNING: Revert device needs about
> > ###### seconds"), L"WARNING: Revert device needs about %d seconds",
> > OpalDisk->EstimateTimeCost);
> 
> Again, using '######' to keep space for 6 digits maybe not enough. Some
> characters might get truncated.
> 
> Best Regards,
> Hao Wu
> 
> > +
> > +  do {
> > +    CreatePopUp(
> > +        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
> > +        &Key,
> > +        Unicode,
> > +        L" System should not be powered off until revert completion ",
> > +        L" ",
> > +        L" Press 'Y/y' to continue, press 'N/n' to cancal ",
> > +        NULL
> > +    );
> > +  } while (
> > +      ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) !=
> > + (ApproveResponse
> > | UPPER_LOWER_CASE_OFFSET)) &&
> > +      ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) != (RejectResponse
> > + |
> > UPPER_LOWER_CASE_OFFSET))
> > +    );
> > +
> > +  if ((Key.UnicodeChar | UPPER_LOWER_CASE_OFFSET) == (RejectResponse
> > + |
> > UPPER_LOWER_CASE_OFFSET)) {
> > +    return EFI_ABORTED;
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> >  /**
> >    This function processes the results of changes in configuration.
> >
> > @@ -588,6 +645,17 @@ DriverCallback(
> >      switch (HiiKeyId) {
> >        case HII_KEY_ID_GOTO_DISK_INFO:
> >          return HiiSelectDisk((UINT8)HiiKey.KeyBits.Index);
> > +
> > +      case HII_KEY_ID_REVERT:
> > +      case HII_KEY_ID_PSID_REVERT:
> > +        OpalDisk = HiiGetOpalDiskCB(gHiiConfiguration.SelectedDiskIndex);
> > +        if (OpalDisk != NULL) {
> > +          return HiiConfirmRevertAction (OpalDisk);
> > +        } else {
> > +          ASSERT (FALSE);
> > +          return EFI_SUCCESS;
> > +        }
> > +
> >      }
> >    } else if (Action == EFI_BROWSER_ACTION_CHANGED) {
> >      switch (HiiKeyId) {
> > @@ -1112,6 +1180,8 @@ OpalDiskInitialize (  {
> >    TCG_RESULT                  TcgResult;
> >    OPAL_SESSION                Session;
> > +  UINT8                       ActiveDataRemovalMechanism;
> > +  UINT32                      RemovalMechanishLists[ResearvedMechanism];
> >
> >    ZeroMem(&Dev->OpalDisk, sizeof(OPAL_DISK));
> >    Dev->OpalDisk.Sscp = Dev->Sscp;
> > @@ -1133,6 +1203,20 @@ OpalDiskInitialize (
> >      return EFI_DEVICE_ERROR;
> >    }
> >
> > +  if (Dev->OpalDisk.SupportedAttributes.DataRemoval) {
> > +    TcgResult = OpalUtilGetDataRemovalMechanismLists (&Session,
> > RemovalMechanishLists);
> > +    if (TcgResult != TcgResultSuccess) {
> > +      return EFI_DEVICE_ERROR;
> > +    }
> > +
> > +    TcgResult = OpalUtilGetActiveDataRemovalMechanism (&Session, Dev-
> > >OpalDisk.Msid, Dev->OpalDisk.MsidLength,
> > >&ActiveDataRemovalMechanism);
> > +    if (TcgResult != TcgResultSuccess) {
> > +      return EFI_DEVICE_ERROR;
> > +    }
> > +
> > +    Dev->OpalDisk.EstimateTimeCost =
> > RemovalMechanishLists[ActiveDataRemovalMechanism];
> > +  }
> > +
> >    return OpalDiskUpdateStatus (&Dev->OpalDisk);  }
> >
> > diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> > b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> > index b4b2d4b3f0..edb47ca8bc 100644
> > --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> > +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordPei.c
> > @@ -635,6 +635,7 @@ UnlockOpalPassword (
> >      BlockSIDEnabled = FALSE;
> >    }
> >    if (BlockSIDEnabled && BlockSidSupport) {
> > +    DEBUG ((DEBUG_INFO, "OpalPassword: S3 phase send BlockSid command
> > to device!\n"));
> >      ZeroMem(&Session, sizeof (Session));
> >      Session.Sscp = &OpalDev->Sscp;
> >      Session.MediaId = 0;
> > --
> > 2.15.0.windows.1

Re: [Patch 0/3] Enable Pyrite 2.0 for opal driver.

[Yao, Jiewen]

Hi Eric
Would you please add more detail on what unit test has been done for this patch?

Thank you
Yao Jiewen

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Eric
> Dong
> Sent: Wednesday, May 2, 2018 8:17 PM
> To: edk2-devel@lists.01.org; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.
> 
> Eanble the pyrite 2.0 devices for opal driver.
> 
> Eric Dong (3):
>   MdePkg: Add Feature definitions add in pyrite 2.0 spec.
>   SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0 spec.
>   SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.
> 
>  MdePkg/Include/IndustryStandard/TcgStorageCore.h   |   2 +
>  MdePkg/Include/IndustryStandard/TcgStorageOpal.h   |  54 +++
>  SecurityPkg/Include/Library/TcgStorageOpalLib.h    |  41 ++
>  .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 426
> ++++++++++++++++++---
>  .../TcgStorageOpalLib/TcgStorageOpalLib.inf        |   1 +
>  .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  98 +++++
>  .../Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 214 ++++++++++-
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c     |  60 ++-
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h     |   9 +
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c        |  84 ++++
>  .../Tcg/Opal/OpalPassword/OpalPasswordPei.c        |   1 +
>  11 files changed, 934 insertions(+), 56 deletions(-)
>  create mode 100644
> SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
> 
> --
> 2.15.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [Patch 0/3] Enable Pyrite 2.0 for opal driver.

[Dong, Eric]

Hi Jiewen,

I do the basic functionality test base on the opal UI. Also I request the QA to do the full regression test and no issue found.

Also will include unit test info in later patches.  Thanks for your comments.

Thanks,
Eric

-----Original Message-----
From: Yao, Jiewen 
Sent: Tuesday, May 8, 2018 1:42 PM
To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Wu, Hao A <hao.a.wu@intel.com>
Subject: RE: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.

Hi Eric
Would you please add more detail on what unit test has been done for this patch?

Thank you
Yao Jiewen

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Eric Dong
> Sent: Wednesday, May 2, 2018 8:17 PM
> To: edk2-devel@lists.01.org; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.
> 
> Eanble the pyrite 2.0 devices for opal driver.
> 
> Eric Dong (3):
>   MdePkg: Add Feature definitions add in pyrite 2.0 spec.
>   SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0 spec.
>   SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.
> 
>  MdePkg/Include/IndustryStandard/TcgStorageCore.h   |   2 +
>  MdePkg/Include/IndustryStandard/TcgStorageOpal.h   |  54 +++
>  SecurityPkg/Include/Library/TcgStorageOpalLib.h    |  41 ++
>  .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 426
> ++++++++++++++++++---
>  .../TcgStorageOpalLib/TcgStorageOpalLib.inf        |   1 +
>  .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  98 +++++  
> .../Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 214 ++++++++++-
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c     |  60 ++-
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h     |   9 +
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c        |  84 ++++
>  .../Tcg/Opal/OpalPassword/OpalPasswordPei.c        |   1 +
>  11 files changed, 934 insertions(+), 56 deletions(-)  create mode 
> 100644 
> SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
> 
> --
> 2.15.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [Patch 0/3] Enable Pyrite 2.0 for opal driver.

[Yao, Jiewen]

Thanks!
Please also add what is "Add supports for pyrite 2.0 spec" as detail as possible. 

Thank you
Yao Jiewen

> -----Original Message-----
> From: Dong, Eric
> Sent: Monday, May 7, 2018 10:50 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org; Wu, Hao A
> <hao.a.wu@intel.com>
> Subject: RE: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.
> 
> Hi Jiewen,
> 
> I do the basic functionality test base on the opal UI. Also I request the QA to do
> the full regression test and no issue found.
> 
> Also will include unit test info in later patches.  Thanks for your comments.
> 
> Thanks,
> Eric
> 
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Tuesday, May 8, 2018 1:42 PM
> To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Wu, Hao A
> <hao.a.wu@intel.com>
> Subject: RE: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.
> 
> Hi Eric
> Would you please add more detail on what unit test has been done for this
> patch?
> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Eric Dong
> > Sent: Wednesday, May 2, 2018 8:17 PM
> > To: edk2-devel@lists.01.org; Wu, Hao A <hao.a.wu@intel.com>
> > Subject: [edk2] [Patch 0/3] Enable Pyrite 2.0 for opal driver.
> >
> > Eanble the pyrite 2.0 devices for opal driver.
> >
> > Eric Dong (3):
> >   MdePkg: Add Feature definitions add in pyrite 2.0 spec.
> >   SecurityPkg/TcgStorageOpalLib: Add supports for pyrite 2.0 spec.
> >   SecurityPkg/OpalPassword: Add support for pyrite 2.0 devices.
> >
> >  MdePkg/Include/IndustryStandard/TcgStorageCore.h   |   2 +
> >  MdePkg/Include/IndustryStandard/TcgStorageOpal.h   |  54 +++
> >  SecurityPkg/Include/Library/TcgStorageOpalLib.h    |  41 ++
> >  .../Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 426
> > ++++++++++++++++++---
> >  .../TcgStorageOpalLib/TcgStorageOpalLib.inf        |   1 +
> >  .../TcgStorageOpalLib/TcgStorageOpalLibInternal.h  |  98 +++++
> > .../Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 214 ++++++++++-
> >  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c     |  60 ++-
> >  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h     |   9 +
> >  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c        |  84 ++++
> >  .../Tcg/Opal/OpalPassword/OpalPasswordPei.c        |   1 +
> >  11 files changed, 934 insertions(+), 56 deletions(-)  create mode
> > 100644
> > SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLibInternal.h
> >
> > --
> > 2.15.0.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel