Hi Jiewen
I did.
https://github.com/tianocore/edk2/pull/4264

I think you used the previous patch. I've attached the latest patch.
Please help to check this .

Thanks.
 

-----Original Message-----
From: Yao, Jiewen <jiewen....@intel.com> 
Sent: Friday, May 5, 2023 2:30 PM
To: devel@edk2.groups.io; Yao, Jiewen <jiewen....@intel.com>; Liu, Linus 
<linus....@intel.com>
Cc: FST-FIR-PRC <fst-fir-...@intel.com>; FST FIR Server 
<fst.fir.ser...@intel.com>; Chu, Maggie <maggie....@intel.com>
Subject: RE: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update 
HddPasswordDxeInit to use Variable Policy

It seems CI failure - https://github.com/tianocore/edk2/pull/4334

Have you run CI before?



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, 
> Jiewen
> Sent: Friday, May 5, 2023 7:50 AM
> To: Liu, Linus <linus....@intel.com>; devel@edk2.groups.io
> Cc: FST-FIR-PRC <fst-fir-...@intel.com>; FST FIR Server 
> <fst.fir.ser...@intel.com>; Chu, Maggie <maggie....@intel.com>
> Subject: Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update 
> HddPasswordDxeInit to use Variable Policy
> 
> Sounds good. Thank you very much!
> 
> Reviewed-by: Jiewen Yao <jiewen....@intel.com>
> 
> > -----Original Message-----
> > From: Liu, Linus <linus....@intel.com>
> > Sent: Thursday, May 4, 2023 11:51 AM
> > To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io
> > Cc: FST-FIR-PRC <fst-fir-...@intel.com>; FST FIR Server 
> > <fst.fir.ser...@intel.com>; Chu, Maggie <maggie....@intel.com>
> > Subject: RE: [PATCH] Securitypkg/hddpassword: Update
> HddPasswordDxeInit
> > to use Variable Policy
> >
> > Hi Jieewn
> > Please refer the below reply.
> >
> > Have you done any function test? For example:
> > 1) The HDD password feature still works?
> >  Linus : yes , HDD password feature still works.
> >
> > 2) The variable is really locked?
> >  Linus : I've tried using dmpstore command to write HDDPassword in 
> > UEFI Shell. Can't override it.
> >
> > Please refer to the below log.
> > [2023-05-04 11:42:11.046] FS1:\> dmpstore -guid 737cded7-448b-4801- 
> > b57d-b19483ec606F -s HDDHDDPwd.txt
> > [2023-05-04 11:42:18.835] Save variable to file: HDDPwd.txt.
> > [2023-05-04 11:42:18.909] Variable NV+BS '737CDED7-448B-4801-B57D- 
> > B19483EC606F:HddPassword' DataSize = 0x48
> > [2023-05-04 11:42:42.859] Load and set variables from file: HDDPwd.txt.
> > [2023-05-04 11:42:42.934] Variable NV+BS '737CDED7-448B-4801-B57D- 
> > B19483EC606F:HddPassword' DataSize = 0x48
> > [2023-05-04 11:42:43.082] dmpstore: Failed to set variable HddPassword:
> > Write Protected.
> >
> >
> > Thanks.
> >
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen....@intel.com>
> > Sent: Wednesday, May 3, 2023 9:21 AM
> > To: Liu, Linus <linus....@intel.com>; devel@edk2.groups.io
> > Cc: FST-FIR-PRC <fst-fir-...@intel.com>; FST FIR Server 
> > <fst.fir.ser...@intel.com>; Chu, Maggie <maggie....@intel.com>
> > Subject: RE: [PATCH] Securitypkg/hddpassword: Update
> HddPasswordDxeInit
> > to use Variable Policy
> >
> > That only proves that you did change the interface. But that cannot 
> > prove you change it right.
> >
> > Have you done any function test? For example:
> > 1) The HDD password feature still works?
> > 2) The variable is really locked?
> >
> >
> > > -----Original Message-----
> > > From: Liu, Linus <linus....@intel.com>
> > > Sent: Wednesday, May 3, 2023 8:40 AM
> > > To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io
> > > Cc: FST-FIR-PRC <fst-fir-...@intel.com>; FST FIR Server 
> > > <fst.fir.ser...@intel.com>; Chu, Maggie <maggie....@intel.com>
> > > Subject: RE: [PATCH] Securitypkg/hddpassword: Update 
> > > HddPasswordDxeInit to use Variable Policy
> > >
> > > Hi Jiewen
> > > I add this patch into MTLS platform and collect the log.
> > > The below is before adding patch and after adding patch. There is 
> > > no warring message.
> > >
> > >
> > > Before
> > >
> > > InstallProtocolInterface: 09576E91-6D3F-11D2-8E39-00A0C969723B
> > > 67E4C490
> > > InstallProtocolInterface: 330D4706-F2A0-4E4F-A369-B66FA8D54385
> > > 68180030
> > > !!! DEPRECATED INTERFACE !!! VariableLockRequestToLock() will go
> away
> > > soon!
> > > !!! DEPRECATED INTERFACE !!! Please move to use Variable Policy!
> > > !!! DEPRECATED INTERFACE !!! Variable: 737CDED7-448B-4801-B57D- 
> > > B19483EC606F HddPassword
> > > HddPasswordDxeInit(): Lock HddPassword variable (Success)
> > >
> > >
> > > After
> > >
> > > InstallProtocolInterface: 09576E91-6D3F-11D2-8E39-00A0C969723B
> > > 67EA1370
> > > InstallProtocolInterface: 330D4706-F2A0-4E4F-A369-B66FA8D54385
> > > 68153DB0
> > > HddPasswordDxeInit(): Lock HddPassword variable (Success)
> > >
> > >
> > > Thanks
> > >
> > >
> > >
> > > -----Original Message-----
> > > From: Yao, Jiewen <jiewen....@intel.com>
> > > Sent: Wednesday, May 3, 2023 12:11 AM
> > > To: Liu, Linus <linus....@intel.com>; devel@edk2.groups.io
> > > Cc: FST-FIR-PRC <fst-fir-...@intel.com>; FST FIR Server 
> > > <fst.fir.ser...@intel.com>; Chu, Maggie <maggie....@intel.com>
> > > Subject: RE: [PATCH] Securitypkg/hddpassword: Update 
> > > HddPasswordDxeInit to use Variable Policy
> > >
> > > Thanks. The patch loos good to me.
> > >
> > > Would you please share with us, how you validate the patch?
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Liu, Linus <linus....@intel.com>
> > > > Sent: Tuesday, April 11, 2023 5:55 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Yao, Jiewen <jiewen....@intel.com>; FST-FIR-PRC <fst-fir- 
> > > > p...@intel.com>; FST FIR Server <fst.fir.ser...@intel.com>; Chu, 
> > > > Maggie <maggie....@intel.com>
> > > > Subject: [PATCH] Securitypkg/hddpassword: Update
> HddPasswordDxeInit
> > > to
> > > > use Variable Policy
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4408
> > > >
> > > > Change-Id: I3c4b466ef318766d6d70c9f73e36b94b5f10832c
> > > > Cc: Jiewen Yao <jiewen....@intel.com>
> > > > Cc: FST-FIR-PRC <fst-fir-...@intel.com>
> > > > Cc: FST FIR Server C <fst.fir.ser...@intel.com>
> > > > Cc: Maggie Chu <maggie....@intel.com>
> > > > Signed-off-by: Linus Liu <linus....@intel.com>
> > > > ---
> > > >  SecurityPkg/HddPassword/HddPasswordDxe.c   | 16 +++++++++++-----
> > > >  SecurityPkg/HddPassword/HddPasswordDxe.h   |  1 -
> > > >  SecurityPkg/HddPassword/HddPasswordDxe.inf |  3 ++-
> > > >  SecurityPkg/SecurityPkg.dsc                |  1 +
> > > >  4 files changed, 14 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.c
> > > > b/SecurityPkg/HddPassword/HddPasswordDxe.c
> > > > index a1a63b67a4..c20fdbe83f 100644
> > > > --- a/SecurityPkg/HddPassword/HddPasswordDxe.c
> > > > +++ b/SecurityPkg/HddPassword/HddPasswordDxe.c
> > > > @@ -9,6 +9,7 @@
> > > >  **/
> > > >
> > > >
> > > >
> > > >  #include "HddPasswordDxe.h"
> > > >
> > > > +#include <Library/VariablePolicyHelperLib.h>
> > > >
> > > >
> > > >
> > > >  EFI_GUID    mHddPasswordVendorGuid          =
> > > > HDD_PASSWORD_CONFIG_GUID;
> > > >
> > > >  CHAR16      mHddPasswordVendorStorageName[] =
> > > > L"HDD_PASSWORD_CONFIG";
> > > >
> > > > @@ -2822,7 +2823,7 @@ HddPasswordDxeInit (
> > > >    HDD_PASSWORD_DXE_PRIVATE_DATA  *Private;
> > > >
> > > >    VOID                           *Registration;
> > > >
> > > >    EFI_EVENT                      EndOfDxeEvent;
> > > >
> > > > -  EDKII_VARIABLE_LOCK_PROTOCOL   *VariableLock;
> > > >
> > > > +  EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy;
> > > >
> > > >
> > > >
> > > >    Private = NULL;
> > > >
> > > >
> > > >
> > > > @@ -2858,12 +2859,17 @@ HddPasswordDxeInit (
> > > >    //
> > > >
> > > >    // Make HDD_PASSWORD_VARIABLE_NAME variable read-only.
> > > >
> > > >    //
> > > >
> > > > -  Status = gBS->LocateProtocol 
> > > > (&gEdkiiVariableLockProtocolGuid, NULL, (VOID **)&VariableLock);
> > > >
> > > > +  Status = gBS->LocateProtocol 
> > > > + (&gEdkiiVariablePolicyProtocolGuid,
> > > > + NULL,
> > > > (VOID **)&VariablePolicy);
> > > >
> > > >    if (!EFI_ERROR (Status)) {
> > > >
> > > > -    Status = VariableLock->RequestToLock (
> > > >
> > > > -                             VariableLock,
> > > >
> > > > +    Status = RegisterBasicVariablePolicy (
> > > >
> > > > +                             VariablePolicy,
> > > >
> > > > +                             &mHddPasswordVendorGuid,
> > > >
> > > >                               HDD_PASSWORD_VARIABLE_NAME,
> > > >
> > > > -                             &mHddPasswordVendorGuid
> > > >
> > > > +                             VARIABLE_POLICY_NO_MIN_SIZE,
> > > >
> > > > +                             VARIABLE_POLICY_NO_MAX_SIZE,
> > > >
> > > > +                             VARIABLE_POLICY_NO_MUST_ATTR,
> > > >
> > > > +                             VARIABLE_POLICY_NO_CANT_ATTR,
> > > >
> > > > +                             VARIABLE_POLICY_TYPE_LOCK_NOW
> > > >
> > > >                               );
> > > >
> > > >      DEBUG ((DEBUG_INFO, "%a(): Lock %s variable (%r)\n", 
> > > > __FUNCTION__, HDD_PASSWORD_VARIABLE_NAME, Status));
> > > >
> > > >      ASSERT_EFI_ERROR (Status);
> > > >
> > > > diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.h
> > > > b/SecurityPkg/HddPassword/HddPasswordDxe.h
> > > > index 231533e737..049a208794 100644
> > > > --- a/SecurityPkg/HddPassword/HddPasswordDxe.h
> > > > +++ b/SecurityPkg/HddPassword/HddPasswordDxe.h
> > > > @@ -17,7 +17,6 @@
> > > >  #include <Protocol/AtaPassThru.h>
> > > >
> > > >  #include <Protocol/PciIo.h>
> > > >
> > > >  #include <Protocol/HiiConfigAccess.h>
> > > >
> > > > -#include <Protocol/VariableLock.h>
> > > >
> > > >
> > > >
> > > >  #include <Guid/MdeModuleHii.h>
> > > >
> > > >  #include <Guid/EventGroup.h>
> > > >
> > > > diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.inf
> > > > b/SecurityPkg/HddPassword/HddPasswordDxe.inf
> > > > index 06e8755ffc..2c0ebbcc78 100644
> > > > --- a/SecurityPkg/HddPassword/HddPasswordDxe.inf
> > > > +++ b/SecurityPkg/HddPassword/HddPasswordDxe.inf
> > > > @@ -50,6 +50,7 @@
> > > >    PrintLib
> > > >
> > > >    UefiLib
> > > >
> > > >    LockBoxLib
> > > >
> > > > +  VariablePolicyHelperLib
> > > >
> > > >    S3BootScriptLib
> > > >
> > > >    PciLib
> > > >
> > > >    BaseCryptLib
> > > >
> > > > @@ -63,7 +64,7 @@
> > > >    gEfiHiiConfigAccessProtocolGuid               ## PRODUCES
> > > >
> > > >    gEfiAtaPassThruProtocolGuid                   ## CONSUMES
> > > >
> > > >    gEfiPciIoProtocolGuid                         ## CONSUMES
> > > >
> > > > -  gEdkiiVariableLockProtocolGuid                ## CONSUMES
> > > >
> > > > +  gEdkiiVariablePolicyProtocolGuid              ## CONSUMES
> > > >
> > > >
> > > >
> > > >  [Pcd]
> > > >
> > > >    gEfiSecurityPkgTokenSpaceGuid.PcdSkipHddPasswordPrompt  ##
> > > CONSUMES
> > > >
> > > > diff --git a/SecurityPkg/SecurityPkg.dsc 
> > > > b/SecurityPkg/SecurityPkg.dsc index 3bad5375c0..3c62205162 
> > > > 100644
> > > > --- a/SecurityPkg/SecurityPkg.dsc
> > > > +++ b/SecurityPkg/SecurityPkg.dsc
> > > > @@ -74,6 +74,7 @@
> > > >
> > > >
> PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibV
> > > > PlatformPKProtectionLib|ar
> > > > PlatformPKProtectionLib|Po
> > > > licy/PlatformPKProtectionLibVarPolicy.inf
> > > >
> > > >
> > > >
> SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariabl
> > > > SecureBootVariableProvisionLib|eP ro
> > > > visionLib/SecureBootVariableProvisionLib.inf
> > > >
> > > >    TdxLib|MdePkg/Library/TdxLib/TdxLib.inf
> > > >
> > > > +
> > > >
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib
> > > > VariablePolicyHelperLib|/V
> > > > VariablePolicyHelperLib|ar
> > > > iablePolicyHelperLib.inf
> > > >
> > > >
> > > >
> > > >  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > > >
> > > >    #
> > > >
> > > > --
> > > > 2.33.1.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104229): https://edk2.groups.io/g/devel/message/104229
Mute This Topic: https://groups.io/mt/98224856/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


--- Begin Message ---
From: Linus Liu <linus....@intel.com>

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

Cc: Jiewen Yao   <jiewen....@intel.com>
Cc: Maggie Chu   <maggie....@intel.com>
Cc: Kumar Rahul  <rahul.r.ku...@intel.com>
Signed-off-by: Linus Liu <linus....@intel.com>
---
 SecurityPkg/HddPassword/HddPasswordDxe.c   | 20 +++++++++++++-------
 SecurityPkg/HddPassword/HddPasswordDxe.h   |  1 -
 SecurityPkg/HddPassword/HddPasswordDxe.inf |  3 ++-
 SecurityPkg/SecurityPkg.dsc                |  1 +
 4 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.c 
b/SecurityPkg/HddPassword/HddPasswordDxe.c
index 55dfb25886..86c11c749f 100644
--- a/SecurityPkg/HddPassword/HddPasswordDxe.c
+++ b/SecurityPkg/HddPassword/HddPasswordDxe.c
@@ -9,6 +9,7 @@
 **/



 #include "HddPasswordDxe.h"

+#include <Library/VariablePolicyHelperLib.h>



 EFI_GUID    mHddPasswordVendorGuid          = HDD_PASSWORD_CONFIG_GUID;

 CHAR16      mHddPasswordVendorStorageName[] = L"HDD_PASSWORD_CONFIG";

@@ -2822,7 +2823,7 @@ HddPasswordDxeInit (
   HDD_PASSWORD_DXE_PRIVATE_DATA  *Private;

   VOID                           *Registration;

   EFI_EVENT                      EndOfDxeEvent;

-  EDKII_VARIABLE_LOCK_PROTOCOL   *VariableLock;

+  EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy;



   Private = NULL;



@@ -2858,13 +2859,18 @@ HddPasswordDxeInit (
   //

   // Make HDD_PASSWORD_VARIABLE_NAME variable read-only.

   //

-  Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (VOID 
**)&VariableLock);

+  Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NULL, (VOID 
**)&VariablePolicy);

   if (!EFI_ERROR (Status)) {

-    Status = VariableLock->RequestToLock (

-                             VariableLock,

-                             HDD_PASSWORD_VARIABLE_NAME,

-                             &mHddPasswordVendorGuid

-                             );

+    Status = RegisterBasicVariablePolicy (

+                 VariablePolicy,

+                 &mHddPasswordVendorGuid,

+                 HDD_PASSWORD_VARIABLE_NAME,

+                 VARIABLE_POLICY_NO_MIN_SIZE,

+                 VARIABLE_POLICY_NO_MAX_SIZE,

+                 VARIABLE_POLICY_NO_MUST_ATTR,

+                 VARIABLE_POLICY_NO_CANT_ATTR,

+                 VARIABLE_POLICY_TYPE_LOCK_NOW

+                 );

     DEBUG ((DEBUG_INFO, "%a(): Lock %s variable (%r)\n", __func__, 
HDD_PASSWORD_VARIABLE_NAME, Status));

     ASSERT_EFI_ERROR (Status);

   }

diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.h 
b/SecurityPkg/HddPassword/HddPasswordDxe.h
index 231533e737..049a208794 100644
--- a/SecurityPkg/HddPassword/HddPasswordDxe.h
+++ b/SecurityPkg/HddPassword/HddPasswordDxe.h
@@ -17,7 +17,6 @@
 #include <Protocol/AtaPassThru.h>

 #include <Protocol/PciIo.h>

 #include <Protocol/HiiConfigAccess.h>

-#include <Protocol/VariableLock.h>



 #include <Guid/MdeModuleHii.h>

 #include <Guid/EventGroup.h>

diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.inf 
b/SecurityPkg/HddPassword/HddPasswordDxe.inf
index 06e8755ffc..2c0ebbcc78 100644
--- a/SecurityPkg/HddPassword/HddPasswordDxe.inf
+++ b/SecurityPkg/HddPassword/HddPasswordDxe.inf
@@ -50,6 +50,7 @@
   PrintLib

   UefiLib

   LockBoxLib

+  VariablePolicyHelperLib

   S3BootScriptLib

   PciLib

   BaseCryptLib

@@ -63,7 +64,7 @@
   gEfiHiiConfigAccessProtocolGuid               ## PRODUCES

   gEfiAtaPassThruProtocolGuid                   ## CONSUMES

   gEfiPciIoProtocolGuid                         ## CONSUMES

-  gEdkiiVariableLockProtocolGuid                ## CONSUMES

+  gEdkiiVariablePolicyProtocolGuid              ## CONSUMES



 [Pcd]

   gEfiSecurityPkgTokenSpaceGuid.PcdSkipHddPasswordPrompt  ## CONSUMES

diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index 3bad5375c0..3c62205162 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -74,6 +74,7 @@
   
PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf

   
SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf

   TdxLib|MdePkg/Library/TdxLib/TdxLib.inf

+  
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf



 [LibraryClasses.ARM, LibraryClasses.AARCH64]

   #

--
2.39.2.windows.1


--- End Message ---

Reply via email to