[Patch v2 0/3] SecurityPkg/Opal: Change BlockSid policy

[Patch v2 0/3] SecurityPkg/Opal: Change BlockSid policy

[Dong, Eric]

This patch serial includes:
1. Change BlockSID default policy, default enable BlockSid.
2. Change SendBlockSID command time from ReadyToBoot to EndOfDxe.
3. Fix "Enable Feature" Menu disappear issue.

V2 change:
1. Include the Bugz link.

Eric Dong (3):
  SecurityPkg/SecurityPkg.dec: Change default value.
  SecurityPkg/OpalPassword: Change send BlockSID policy.
  SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue.

 .../Include/Library/Tcg2PhysicalPresenceLib.h |   3 +-
 SecurityPkg/SecurityPkg.dec                   |   2 +-
 .../Tcg/Opal/OpalPassword/OpalDriver.c        | 115 +++++++++---------
 .../Tcg/Opal/OpalPassword/OpalDriver.h        |   1 +
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c   |  46 +++++--
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h   |  15 +++
 6 files changed, 112 insertions(+), 70 deletions(-)

-- 
2.21.0.windows.1

[Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.

[Dong, Eric]

https://bugzilla.tianocore.org/show_bug.cgi?id=1782

Change BlockSID default policy, default enable BlockSid.

Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
---
 SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
 SecurityPkg/SecurityPkg.dec                           | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
index d9eee7f3e8..8da3deaf86 100644
--- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
+++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
@@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 // Default value
 //
 #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE_BLOCK_SID | \
-                                                   TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BLOCK_SID)
+                                                   TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BLOCK_SID |\
+                                                   TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
 
 /**
   Check and execute the pending TPM request.
diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 6e4c4c3a02..3314f1854b 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -410,7 +410,7 @@
   # PCD can be configured for different settings in different scenarios
   # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
   # @Prompt Initial setting of TCG2 Persistent Firmware Management Flags
-  gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UINT32|0x0001001B
+  gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UINT32|0x0001001B
 
   ## Indicate current TPM2 Interrupt Number reported by _CRS control method.<BR><BR>
   # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
-- 
2.21.0.windows.1

[Patch v2 2/3] SecurityPkg/OpalPassword: Change send BlockSID policy.

[Dong, Eric]

https://bugzilla.tianocore.org/show_bug.cgi?id=1782

Change Send BlockSID command time from ReadyToBoot to
EndOfDxe.

Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
---
 .../Tcg/Opal/OpalPassword/OpalDriver.c        | 104 ++++++++----------
 1 file changed, 46 insertions(+), 58 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
index 42999c89f0..009a97f66f 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
@@ -424,6 +424,47 @@ BuildOpalDeviceInfo (
   FreePool (S3InitDevices);
 }
 
+/**
+
+  Send BlockSid command if needed.
+
+**/
+VOID
+SendBlockSidCommand (
+  VOID
+  )
+{
+  OPAL_DRIVER_DEVICE                         *Itr;
+  TCG_RESULT                                 Result;
+  OPAL_SESSION                               Session;
+  UINT32                                     PpStorageFlag;
+
+  PpStorageFlag = Tcg2PhysicalPresenceLibGetManagementFlags ();
+  if ((PpStorageFlag & TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID) != 0) {
+    //
+    // Send BlockSID command to each Opal disk
+    //
+    Itr = mOpalDriver.DeviceList;
+    while (Itr != NULL) {
+      if (Itr->OpalDisk.SupportedAttributes.BlockSid) {
+        ZeroMem(&Session, sizeof(Session));
+        Session.Sscp = Itr->OpalDisk.Sscp;
+        Session.MediaId = Itr->OpalDisk.MediaId;
+        Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
+
+        DEBUG ((DEBUG_INFO, "OpalPassword: EndOfDxe 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"));
+          break;
+        }
+      }
+
+      Itr = Itr->Next;
+    }
+  }
+}
+
 /**
   Notification function of EFI_END_OF_DXE_EVENT_GROUP_GUID event group.
 
@@ -475,6 +516,11 @@ OpalEndOfDxeEventNotify (
     TmpDev = TmpDev->Next;
   }
 
+  //
+  // Send BlockSid command if needed.
+  //
+  SendBlockSidCommand ();
+
   DEBUG ((DEBUG_INFO, "%a() - exit\n", __FUNCTION__));
 
   gBS->CloseEvent (Event);
@@ -2262,53 +2308,6 @@ OpalDriverGetDeviceList(
   return mOpalDriver.DeviceList;
 }
 
-/**
-  ReadyToBoot callback to send BlockSid command.
-
-  @param  Event   Pointer to this event
-  @param  Context Event handler private Data
-
-**/
-VOID
-EFIAPI
-ReadyToBootCallback (
-  IN EFI_EVENT        Event,
-  IN VOID             *Context
-  )
-{
-  OPAL_DRIVER_DEVICE                         *Itr;
-  TCG_RESULT                                 Result;
-  OPAL_SESSION                               Session;
-  UINT32                                     PpStorageFlag;
-
-  gBS->CloseEvent (Event);
-
-  PpStorageFlag = Tcg2PhysicalPresenceLibGetManagementFlags ();
-  if ((PpStorageFlag & TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID) != 0) {
-    //
-    // Send BlockSID command to each Opal disk
-    //
-    Itr = mOpalDriver.DeviceList;
-    while (Itr != NULL) {
-      if (Itr->OpalDisk.SupportedAttributes.BlockSid) {
-        ZeroMem(&Session, sizeof(Session));
-        Session.Sscp = Itr->OpalDisk.Sscp;
-        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"));
-          break;
-        }
-      }
-
-      Itr = Itr->Next;
-    }
-  }
-}
-
 /**
   Stop this Controller.
 
@@ -2571,7 +2570,6 @@ EfiDriverEntryPoint(
   )
 {
   EFI_STATUS                     Status;
-  EFI_EVENT                      ReadyToBootEvent;
   EFI_EVENT                      EndOfDxeEvent;
 
   Status = EfiLibInstallDriverBindingComponentName2 (
@@ -2604,16 +2602,6 @@ EfiDriverEntryPoint(
                   );
   ASSERT_EFI_ERROR (Status);
 
-  //
-  // register a ReadyToBoot event callback for sending BlockSid command
-  //
-  Status = EfiCreateEventReadyToBootEx (
-                  TPL_CALLBACK,
-                  ReadyToBootCallback,
-                  (VOID *) &ImageHandle,
-                  &ReadyToBootEvent
-                  );
-
   //
   // Install Hii packages.
   //
-- 
2.21.0.windows.1

[Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue.

[Dong, Eric]

https://bugzilla.tianocore.org/show_bug.cgi?id=1782

After change behavior to send BlockSid command at EndOfDxe point,
check device ownership command will return un-authority error, it
finally caused opal driver can't show "Enable Feature" menu.

Update the code logic to send detect device ownership command
before send BlockSID command.

Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
---
 .../Tcg/Opal/OpalPassword/OpalDriver.c        | 11 +++++
 .../Tcg/Opal/OpalPassword/OpalDriver.h        |  1 +
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c   | 46 +++++++++++++++----
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h   | 15 ++++++
 4 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
index 009a97f66f..965205c0b2 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
@@ -458,6 +458,11 @@ SendBlockSidCommand (
           DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
           break;
         }
+
+        //
+        // Record BlockSID command has been sent.
+        //
+        Itr->OpalDisk.SentBlockSID = TRUE;
       }
 
       Itr = Itr->Next;
@@ -2204,6 +2209,12 @@ ProcessOpalRequest (
         ProcessOpalRequestEnableFeature (Dev, L"Enable Feature:");
       }
 
+      //
+      // Update Device ownership.
+      // Later BlockSID command may block the update.
+      //
+      OpalDiskUpdateOwnerShip (&Dev->OpalDisk);
+
       break;
     }
 
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
index a056e06106..beeabb1c0a 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
@@ -143,6 +143,7 @@ typedef struct {
   UINT8                                           Password[OPAL_MAX_PASSWORD_SIZE];
 
   UINT32                                          EstimateTimeCost;
+  BOOLEAN                                         SentBlockSID;           // Check whether BlockSid command has been sent.
 } OPAL_DISK;
 
 //
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
index d0f3eda1e8..f101ca1c20 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
@@ -1215,6 +1215,40 @@ OpalDiskInitialize (
   return OpalDiskUpdateStatus (&Dev->OpalDisk);
 }
 
+/**
+  Update the device ownship
+
+  @param OpalDisk                The Opal device.
+
+  @retval EFI_SUCESS             Get ownership success.
+  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change ownership.
+  @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership info.
+
+**/
+EFI_STATUS
+OpalDiskUpdateOwnerShip (
+  OPAL_DISK        *OpalDisk
+  )
+{

+  OPAL_SESSION  Session;
+
+  if (OpalDisk->MsidLength == 0) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (OpalDisk->SentBlockSID) {
+    return EFI_ACCESS_DENIED;
+  }
+
+  ZeroMem(&Session, sizeof(Session));
+  Session.Sscp = OpalDisk->Sscp;
+  Session.MediaId = OpalDisk->MediaId;
+  Session.OpalBaseComId = OpalDisk->OpalBaseComId;
+
+  OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk->Msid, OpalDisk->MsidLength);

+  return EFI_SUCCESS;
+}
+
 /**
   Update the device info.
 
@@ -1223,6 +1257,7 @@ OpalDiskInitialize (
   @retval EFI_SUCESS             Initialize the device success.
   @retval EFI_DEVICE_ERROR       Get info from device failed.
   @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership info.
+  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change ownership.
 
 **/
 EFI_STATUS
@@ -1243,15 +1278,6 @@ OpalDiskUpdateStatus (
     return EFI_DEVICE_ERROR;
   }
 
-  if (OpalDisk->MsidLength == 0) {
-    return EFI_INVALID_PARAMETER;
-  } else {
-    //
-    // Base on the Msid info to get the ownership, so Msid info must get first.
-    //
-    OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk->Msid, OpalDisk->MsidLength);
-  }
-
-  return EFI_SUCCESS;
+  return OpalDiskUpdateOwnerShip (OpalDisk);
 }
 
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
index d3e236e2fe..89c709df99 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
@@ -357,4 +357,19 @@ OpalDiskInitialize (
   IN OPAL_DRIVER_DEVICE          *Dev
   );
 
+/**
+  Update the device ownership
+
+  @param OpalDisk                The Opal device.
+
+  @retval EFI_SUCESS             Get ownership success.
+  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change ownership.
+  @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership info.
+
+**/
+EFI_STATUS
+OpalDiskUpdateOwnerShip (
+  OPAL_DISK        *OpalDisk
+  );
+
 #endif // _HII_H_
-- 
2.21.0.windows.1

Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.

[Wu, Hao A]

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Dong,
> Eric
> Sent: Wednesday, May 08, 2019 11:02 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A
> Subject: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change
> default value.

Just one minor comment, how about changing the title to:
SecurityPkg/SecurityPkg.dec: Change BlockSID default policy

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

Best Regards,
Hao Wu

> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
> 
> Change BlockSID default policy, default enable BlockSid.
> 
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> ---
>  SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
>  SecurityPkg/SecurityPkg.dec                           | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> index d9eee7f3e8..8da3deaf86 100644
> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  // Default value
>  //
>  #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE_BL
> OCK_SID | \
> -
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BL
> OCK_SID)
> +
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BL
> OCK_SID |\
> +
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
> 
>  /**
>    Check and execute the pending TPM request.
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 6e4c4c3a02..3314f1854b 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -410,7 +410,7 @@
>    # PCD can be configured for different settings in different scenarios
>    # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT |
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>    # @Prompt Initial setting of TCG2 Persistent Firmware Management Flags
> -
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UINT3
> 2|0x0001001B
> +
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UINT3
> 2|0x0001001B
> 
>    ## Indicate current TPM2 Interrupt Number reported by _CRS control
> method.<BR><BR>
>    # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
> --
> 2.21.0.windows.1
> 
> 
> 

Re: [edk2-devel] [Patch v2 2/3] SecurityPkg/OpalPassword: Change send BlockSID policy.

[Wu, Hao A]

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Dong,
> Eric
> Sent: Wednesday, May 08, 2019 11:02 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A
> Subject: [edk2-devel] [Patch v2 2/3] SecurityPkg/OpalPassword: Change send
> BlockSID policy.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
> 
> Change Send BlockSID command time from ReadyToBoot to
> EndOfDxe.

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

Best Regards,
Hao Wu

> 
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> ---
>  .../Tcg/Opal/OpalPassword/OpalDriver.c        | 104 ++++++++----------
>  1 file changed, 46 insertions(+), 58 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index 42999c89f0..009a97f66f 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -424,6 +424,47 @@ BuildOpalDeviceInfo (
>    FreePool (S3InitDevices);
>  }
> 
> +/**
> +
> +  Send BlockSid command if needed.
> +
> +**/
> +VOID
> +SendBlockSidCommand (
> +  VOID
> +  )
> +{
> +  OPAL_DRIVER_DEVICE                         *Itr;
> +  TCG_RESULT                                 Result;
> +  OPAL_SESSION                               Session;
> +  UINT32                                     PpStorageFlag;
> +
> +  PpStorageFlag = Tcg2PhysicalPresenceLibGetManagementFlags ();
> +  if ((PpStorageFlag &
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID) != 0) {
> +    //
> +    // Send BlockSID command to each Opal disk
> +    //
> +    Itr = mOpalDriver.DeviceList;
> +    while (Itr != NULL) {
> +      if (Itr->OpalDisk.SupportedAttributes.BlockSid) {
> +        ZeroMem(&Session, sizeof(Session));
> +        Session.Sscp = Itr->OpalDisk.Sscp;
> +        Session.MediaId = Itr->OpalDisk.MediaId;
> +        Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
> +
> +        DEBUG ((DEBUG_INFO, "OpalPassword: EndOfDxe 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"));
> +          break;
> +        }
> +      }
> +
> +      Itr = Itr->Next;
> +    }
> +  }
> +}
> +
>  /**
>    Notification function of EFI_END_OF_DXE_EVENT_GROUP_GUID event group.
> 
> @@ -475,6 +516,11 @@ OpalEndOfDxeEventNotify (
>      TmpDev = TmpDev->Next;
>    }
> 
> +  //
> +  // Send BlockSid command if needed.
> +  //
> +  SendBlockSidCommand ();
> +
>    DEBUG ((DEBUG_INFO, "%a() - exit\n", __FUNCTION__));
> 
>    gBS->CloseEvent (Event);
> @@ -2262,53 +2308,6 @@ OpalDriverGetDeviceList(
>    return mOpalDriver.DeviceList;
>  }
> 
> -/**
> -  ReadyToBoot callback to send BlockSid command.
> -
> -  @param  Event   Pointer to this event
> -  @param  Context Event handler private Data
> -
> -**/
> -VOID
> -EFIAPI
> -ReadyToBootCallback (
> -  IN EFI_EVENT        Event,
> -  IN VOID             *Context
> -  )
> -{
> -  OPAL_DRIVER_DEVICE                         *Itr;
> -  TCG_RESULT                                 Result;
> -  OPAL_SESSION                               Session;
> -  UINT32                                     PpStorageFlag;
> -
> -  gBS->CloseEvent (Event);
> -
> -  PpStorageFlag = Tcg2PhysicalPresenceLibGetManagementFlags ();
> -  if ((PpStorageFlag &
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID) != 0) {
> -    //
> -    // Send BlockSID command to each Opal disk
> -    //
> -    Itr = mOpalDriver.DeviceList;
> -    while (Itr != NULL) {
> -      if (Itr->OpalDisk.SupportedAttributes.BlockSid) {
> -        ZeroMem(&Session, sizeof(Session));
> -        Session.Sscp = Itr->OpalDisk.Sscp;
> -        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"));
> -          break;
> -        }
> -      }
> -
> -      Itr = Itr->Next;
> -    }
> -  }
> -}
> -
>  /**
>    Stop this Controller.
> 
> @@ -2571,7 +2570,6 @@ EfiDriverEntryPoint(
>    )
>  {
>    EFI_STATUS                     Status;
> -  EFI_EVENT                      ReadyToBootEvent;
>    EFI_EVENT                      EndOfDxeEvent;
> 
>    Status = EfiLibInstallDriverBindingComponentName2 (
> @@ -2604,16 +2602,6 @@ EfiDriverEntryPoint(
>                    );
>    ASSERT_EFI_ERROR (Status);
> 
> -  //
> -  // register a ReadyToBoot event callback for sending BlockSid command
> -  //
> -  Status = EfiCreateEventReadyToBootEx (
> -                  TPL_CALLBACK,
> -                  ReadyToBootCallback,
> -                  (VOID *) &ImageHandle,
> -                  &ReadyToBootEvent
> -                  );
> -
>    //
>    // Install Hii packages.
>    //
> --
> 2.21.0.windows.1
> 
> 
> 

Re: [edk2-devel] [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable Feature" Menu disappear issue.

[Wu, Hao A]

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Dong,
> Eric
> Sent: Wednesday, May 08, 2019 11:02 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A
> Subject: [edk2-devel] [Patch v2 3/3] SecurityPkg/OpalPassword: Fix "Enable
> Feature" Menu disappear issue.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
> 
> After change behavior to send BlockSid command at EndOfDxe point,
> check device ownership command will return un-authority error, it
> finally caused opal driver can't show "Enable Feature" menu.
> 
> Update the code logic to send detect device ownership command
> before send BlockSID command.

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

Best Regards,
Hao Wu

> 
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> ---
>  .../Tcg/Opal/OpalPassword/OpalDriver.c        | 11 +++++
>  .../Tcg/Opal/OpalPassword/OpalDriver.h        |  1 +
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c   | 46 +++++++++++++++----
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h   | 15 ++++++
>  4 files changed, 63 insertions(+), 10 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index 009a97f66f..965205c0b2 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -458,6 +458,11 @@ SendBlockSidCommand (
>            DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
>            break;
>          }
> +
> +        //
> +        // Record BlockSID command has been sent.
> +        //
> +        Itr->OpalDisk.SentBlockSID = TRUE;
>        }
> 
>        Itr = Itr->Next;
> @@ -2204,6 +2209,12 @@ ProcessOpalRequest (
>          ProcessOpalRequestEnableFeature (Dev, L"Enable Feature:");
>        }
> 
> +      //
> +      // Update Device ownership.
> +      // Later BlockSID command may block the update.
> +      //
> +      OpalDiskUpdateOwnerShip (&Dev->OpalDisk);
> +
>        break;
>      }
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> index a056e06106..beeabb1c0a 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h
> @@ -143,6 +143,7 @@ typedef struct {
>    UINT8                                           Password[OPAL_MAX_PASSWORD_SIZE];
> 
>    UINT32                                          EstimateTimeCost;
> +  BOOLEAN                                         SentBlockSID;           // Check whether
> BlockSid command has been sent.
>  } OPAL_DISK;
> 
>  //
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> index d0f3eda1e8..f101ca1c20 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c
> @@ -1215,6 +1215,40 @@ OpalDiskInitialize (
>    return OpalDiskUpdateStatus (&Dev->OpalDisk);
>  }
> 
> +/**
> +  Update the device ownship
> +
> +  @param OpalDisk                The Opal device.
> +
> +  @retval EFI_SUCESS             Get ownership success.
> +  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change
> ownership.
> +  @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership
> info.
> +
> +**/
> +EFI_STATUS
> +OpalDiskUpdateOwnerShip (
> +  OPAL_DISK        *OpalDisk
> +  )
> +{
> 
> +  OPAL_SESSION  Session;
> +
> +  if (OpalDisk->MsidLength == 0) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (OpalDisk->SentBlockSID) {
> +    return EFI_ACCESS_DENIED;
> +  }
> +
> +  ZeroMem(&Session, sizeof(Session));
> +  Session.Sscp = OpalDisk->Sscp;
> +  Session.MediaId = OpalDisk->MediaId;
> +  Session.OpalBaseComId = OpalDisk->OpalBaseComId;
> +
> +  OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk->Msid,
> OpalDisk->MsidLength);
> 
> +  return EFI_SUCCESS;
> +}
> +
>  /**
>    Update the device info.
> 
> @@ -1223,6 +1257,7 @@ OpalDiskInitialize (
>    @retval EFI_SUCESS             Initialize the device success.
>    @retval EFI_DEVICE_ERROR       Get info from device failed.
>    @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership
> info.
> +  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change
> ownership.
> 
>  **/
>  EFI_STATUS
> @@ -1243,15 +1278,6 @@ OpalDiskUpdateStatus (
>      return EFI_DEVICE_ERROR;
>    }
> 
> -  if (OpalDisk->MsidLength == 0) {
> -    return EFI_INVALID_PARAMETER;
> -  } else {
> -    //
> -    // Base on the Msid info to get the ownership, so Msid info must get first.
> -    //
> -    OpalDisk->Owner = OpalUtilDetermineOwnership(&Session, OpalDisk-
> >Msid, OpalDisk->MsidLength);
> -  }
> -
> -  return EFI_SUCCESS;
> +  return OpalDiskUpdateOwnerShip (OpalDisk);
>  }
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> index d3e236e2fe..89c709df99 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.h
> @@ -357,4 +357,19 @@ OpalDiskInitialize (
>    IN OPAL_DRIVER_DEVICE          *Dev
>    );
> 
> +/**
> +  Update the device ownership
> +
> +  @param OpalDisk                The Opal device.
> +
> +  @retval EFI_SUCESS             Get ownership success.
> +  @retval EFI_ACCESS_DENIED      Has send BlockSID command, can't change
> ownership.
> +  @retval EFI_INVALID_PARAMETER  Not get Msid info before get ownership
> info.
> +
> +**/
> +EFI_STATUS
> +OpalDiskUpdateOwnerShip (
> +  OPAL_DISK        *OpalDisk
> +  );
> +
>  #endif // _HII_H_
> --
> 2.21.0.windows.1
> 
> 
> 

Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.

[Laszlo Ersek]

On 05/08/19 05:01, Dong, Eric wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
> 
> Change BlockSID default policy, default enable BlockSid.
> 
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> ---
>  SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
>  SecurityPkg/SecurityPkg.dec                           | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Please change the subject line as follows:

----
SecurityPkg: enable BlockSID in PcdTcg2PhysicalPresenceFlags DEC default
----

no need to resubmit just for this, just pls update the patch before you
push it.

Thanks
Laszlo

> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> index d9eee7f3e8..8da3deaf86 100644
> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  // Default value
>  //
>  #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE_BLOCK_SID | \
> -                                                   TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BLOCK_SID)
> +                                                   TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BLOCK_SID |\
> +                                                   TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
>  
>  /**
>    Check and execute the pending TPM request.
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 6e4c4c3a02..3314f1854b 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -410,7 +410,7 @@
>    # PCD can be configured for different settings in different scenarios
>    # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>    # @Prompt Initial setting of TCG2 Persistent Firmware Management Flags
> -  gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UINT32|0x0001001B
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UINT32|0x0001001B
>  
>    ## Indicate current TPM2 Interrupt Number reported by _CRS control method.<BR><BR>
>    # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
> 

Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.

[Laszlo Ersek]

On 05/09/19 05:03, Wu, Hao A wrote:
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Dong,
>> Eric
>> Sent: Wednesday, May 08, 2019 11:02 AM
>> To: devel@edk2.groups.io
>> Cc: Wu, Hao A
>> Subject: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change
>> default value.
> 
> Just one minor comment, how about changing the title to:
> SecurityPkg/SecurityPkg.dec: Change BlockSID default policy

That's an improvement too, thanks.
Laszlo

> 
> Other than that, the patch is good to me:
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> 
> Best Regards,
> Hao Wu
> 
>>
>> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
>>
>> Change BlockSID default policy, default enable BlockSid.
>>
>> Signed-off-by: Eric Dong <eric.dong@intel.com>
>> Cc: Hao Wu <hao.a.wu@intel.com>
>> ---
>>  SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
>>  SecurityPkg/SecurityPkg.dec                           | 2 +-
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> index d9eee7f3e8..8da3deaf86 100644
>> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>  // Default value
>>  //
>>  #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE_BL
>> OCK_SID | \
>> -
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BL
>> OCK_SID)
>> +
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_BL
>> OCK_SID |\
>> +
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
>>
>>  /**
>>    Check and execute the pending TPM request.
>> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
>> index 6e4c4c3a02..3314f1854b 100644
>> --- a/SecurityPkg/SecurityPkg.dec
>> +++ b/SecurityPkg/SecurityPkg.dec
>> @@ -410,7 +410,7 @@
>>    # PCD can be configured for different settings in different scenarios
>>    # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT |
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>>    # @Prompt Initial setting of TCG2 Persistent Firmware Management Flags
>> -
>> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UINT3
>> 2|0x0001001B
>> +
>> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UINT3
>> 2|0x0001001B
>>
>>    ## Indicate current TPM2 Interrupt Number reported by _CRS control
>> method.<BR><BR>
>>    # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
>> --
>> 2.21.0.windows.1
>>
>>
>>
> 
> 
> 
> 

Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.

[Yao, Jiewen]

Hey
When I read https://bugzilla.tianocore.org/show_bug.cgi?id=1782, it says: "Current opal driver send blockSid command at ReadyToBoot event, it should been update to EndOfDxe point. Submit this bz to update the code."

But this patch is to update the default value.

I am very confused.

May I know what is the relationship between this patch and Bugzilla?
Why we need change the default value?


Thank you
Yao Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Thursday, May 9, 2019 2:53 AM
> To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec:
> Change default value.
> 
> On 05/08/19 05:01, Dong, Eric wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1782
> >
> > Change BlockSID default policy, default enable BlockSid.
> >
> > Signed-off-by: Eric Dong <eric.dong@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > ---
> >  SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
> >  SecurityPkg/SecurityPkg.dec                           | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> Please change the subject line as follows:
> 
> ----
> SecurityPkg: enable BlockSID in PcdTcg2PhysicalPresenceFlags DEC default
> ----
> 
> no need to resubmit just for this, just pls update the patch before you
> push it.
> 
> Thanks
> Laszlo
> 
> > diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> > index d9eee7f3e8..8da3deaf86 100644
> > --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> > +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> > @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >  // Default value
> >  //
> >  #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE
> _BLOCK_SID | \
> > -
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
> BLOCK_SID)
> > +
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
> BLOCK_SID |\
> > +
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
> >
> >  /**
> >    Check and execute the pending TPM request.
> > diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> > index 6e4c4c3a02..3314f1854b 100644
> > --- a/SecurityPkg/SecurityPkg.dec
> > +++ b/SecurityPkg/SecurityPkg.dec
> > @@ -410,7 +410,7 @@
> >    # PCD can be configured for different settings in different scenarios
> >    # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT
> | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> >    # @Prompt Initial setting of TCG2 Persistent Firmware Management
> Flags
> > -
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UI
> NT32|0x0001001B
> > +
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UI
> NT32|0x0001001B
> >
> >    ## Indicate current TPM2 Interrupt Number reported by _CRS control
> method.<BR><BR>
> >    # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
> >
> 
> 
> 

Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.

[Laszlo Ersek]

Hi Jiewen,

On 05/09/19 14:41, Yao, Jiewen wrote:
> Hey
> When I read https://bugzilla.tianocore.org/show_bug.cgi?id=1782, it says: "Current opal driver send blockSid command at ReadyToBoot event, it should been update to EndOfDxe point. Submit this bz to update the code."
> 
> But this patch is to update the default value.
> 
> I am very confused.
> 
> May I know what is the relationship between this patch and Bugzilla?
> Why we need change the default value?

I have absolutely no clue -- I only commented because I prefer a patch
subject to state *specifically* what a patch does. "Change default
value" was too vague. (When I commented I hadn't seen Hao Wu's similar
feedback just yet.)

Now, *why* this change is necessary, is totally over my head; I didn't
even begin to think about that.

Thanks
Laszlo

> 
> 
> Thank you
> Yao Jiewen
> 
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Laszlo Ersek
>> Sent: Thursday, May 9, 2019 2:53 AM
>> To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>
>> Cc: Wu, Hao A <hao.a.wu@intel.com>
>> Subject: Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec:
>> Change default value.
>>
>> On 05/08/19 05:01, Dong, Eric wrote:
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
>>>
>>> Change BlockSID default policy, default enable BlockSid.
>>>
>>> Signed-off-by: Eric Dong <eric.dong@intel.com>
>>> Cc: Hao Wu <hao.a.wu@intel.com>
>>> ---
>>>  SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
>>>  SecurityPkg/SecurityPkg.dec                           | 2 +-
>>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> Please change the subject line as follows:
>>
>> ----
>> SecurityPkg: enable BlockSID in PcdTcg2PhysicalPresenceFlags DEC default
>> ----
>>
>> no need to resubmit just for this, just pls update the patch before you
>> push it.
>>
>> Thanks
>> Laszlo
>>
>>> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>>> index d9eee7f3e8..8da3deaf86 100644
>>> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>>> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
>>> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>>>  // Default value
>>>  //
>>>  #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE
>> _BLOCK_SID | \
>>> -
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
>> BLOCK_SID)
>>> +
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
>> BLOCK_SID |\
>>> +
>> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
>>>
>>>  /**
>>>    Check and execute the pending TPM request.
>>> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
>>> index 6e4c4c3a02..3314f1854b 100644
>>> --- a/SecurityPkg/SecurityPkg.dec
>>> +++ b/SecurityPkg/SecurityPkg.dec
>>> @@ -410,7 +410,7 @@
>>>    # PCD can be configured for different settings in different scenarios
>>>    # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT
>> | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
>>>    # @Prompt Initial setting of TCG2 Persistent Firmware Management
>> Flags
>>> -
>> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UI
>> NT32|0x0001001B
>>> +
>> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UI
>> NT32|0x0001001B
>>>
>>>    ## Indicate current TPM2 Interrupt Number reported by _CRS control
>> method.<BR><BR>
>>>    # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
>>>
>>
>>
>> 
> 

Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec: Change default value.

[Yao, Jiewen]

Thanks Laszlo.
Sorry, I do not mean to ask you.
The question is for the original patch submitter.
I just reply the last email in my mail box.

I agree with you that the title should describe *what* code does.
At same time, I prefer to see the commit message on *why* we need do the change.

Thank you
Yao Jiewen


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, May 9, 2019 2:27 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Dong, Eric
> <eric.dong@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec:
> Change default value.
> 
> Hi Jiewen,
> 
> On 05/09/19 14:41, Yao, Jiewen wrote:
> > Hey
> > When I read https://bugzilla.tianocore.org/show_bug.cgi?id=1782, it says:
> "Current opal driver send blockSid command at ReadyToBoot event, it
> should been update to EndOfDxe point. Submit this bz to update the code."
> >
> > But this patch is to update the default value.
> >
> > I am very confused.
> >
> > May I know what is the relationship between this patch and Bugzilla?
> > Why we need change the default value?
> 
> I have absolutely no clue -- I only commented because I prefer a patch
> subject to state *specifically* what a patch does. "Change default
> value" was too vague. (When I commented I hadn't seen Hao Wu's similar
> feedback just yet.)
> 
> Now, *why* this change is necessary, is totally over my head; I didn't
> even begin to think about that.
> 
> Thanks
> Laszlo
> 
> >
> >
> > Thank you
> > Yao Jiewen
> >
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> Of
> >> Laszlo Ersek
> >> Sent: Thursday, May 9, 2019 2:53 AM
> >> To: devel@edk2.groups.io; Dong, Eric <eric.dong@intel.com>
> >> Cc: Wu, Hao A <hao.a.wu@intel.com>
> >> Subject: Re: [edk2-devel] [Patch v2 1/3] SecurityPkg/SecurityPkg.dec:
> >> Change default value.
> >>
> >> On 05/08/19 05:01, Dong, Eric wrote:
> >>> https://bugzilla.tianocore.org/show_bug.cgi?id=1782
> >>>
> >>> Change BlockSID default policy, default enable BlockSid.
> >>>
> >>> Signed-off-by: Eric Dong <eric.dong@intel.com>
> >>> Cc: Hao Wu <hao.a.wu@intel.com>
> >>> ---
> >>>  SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h | 3 ++-
> >>>  SecurityPkg/SecurityPkg.dec                           | 2 +-
> >>>  2 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> Please change the subject line as follows:
> >>
> >> ----
> >> SecurityPkg: enable BlockSID in PcdTcg2PhysicalPresenceFlags DEC
> default
> >> ----
> >>
> >> no need to resubmit just for this, just pls update the patch before you
> >> push it.
> >>
> >> Thanks
> >> Laszlo
> >>
> >>> diff --git a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> >> b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> >>> index d9eee7f3e8..8da3deaf86 100644
> >>> --- a/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> >>> +++ b/SecurityPkg/Include/Library/Tcg2PhysicalPresenceLib.h
> >>> @@ -51,7 +51,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >>>  // Default value
> >>>  //
> >>>  #define TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> >>
> (TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_ENABLE
> >> _BLOCK_SID | \
> >>> -
> >>
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
> >> BLOCK_SID)
> >>> +
> >>
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_PP_REQUIRED_FOR_DISABLE_
> >> BLOCK_SID |\
> >>> +
> >> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_ENABLE_BLOCK_SID)
> >>>
> >>>  /**
> >>>    Check and execute the pending TPM request.
> >>> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> >>> index 6e4c4c3a02..3314f1854b 100644
> >>> --- a/SecurityPkg/SecurityPkg.dec
> >>> +++ b/SecurityPkg/SecurityPkg.dec
> >>> @@ -410,7 +410,7 @@
> >>>    # PCD can be configured for different settings in different scenarios
> >>>    # Default setting is
> TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT
> >> | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> >>>    # @Prompt Initial setting of TCG2 Persistent Firmware Management
> >> Flags
> >>> -
> >>
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x300E2|UI
> >> NT32|0x0001001B
> >>> +
> >>
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags|0x700E2|UI
> >> NT32|0x0001001B
> >>>
> >>>    ## Indicate current TPM2 Interrupt Number reported by _CRS
> control
> >> method.<BR><BR>
> >>>    # TPM2 Interrupt feature is disabled If the pcd is set to 0.<BR>
> >>>
> >>
> >>
> >> 
> >