Hi Jiewen I've resent the patch and verify the patch. https://github.com/tianocore/edk2/pull/4354 Thanks. -----Original Message----- From: Yao, Jiewen Sent: Monday, May 8, 2023 9:30 AM To: Liu, Linus ; devel@edk2.groups.io Cc: FST-FIR-PRC ; FST FIR Server ; Chu, Maggie Subject: RE: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy https://github.com/tianocore/edk2/pull/4264 is from last month and it is out of date. https://github.com/tianocore/edk2/pull/4334 failed in latest branch. Please try it again. Also, I see you always use [V1] in the patch title. That is very confusing. Please use V2, V3, etc whenever you send a new patch. Thank you Yao, Jiewen > -----Original Message----- > From: Liu, Linus > Sent: Monday, May 8, 2023 9:09 AM > To: Yao, Jiewen ; devel@edk2.groups.io > Cc: FST-FIR-PRC ; FST FIR Server > ; Chu, Maggie > Subject: RE: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > HddPasswordDxeInit to use Variable Policy > > 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 > Sent: Friday, May 5, 2023 2:30 PM > To: devel@edk2.groups.io; Yao, Jiewen ; Liu, > Linus > Cc: FST-FIR-PRC ; FST FIR Server > ; Chu, Maggie > 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 On Behalf Of Yao, > > Jiewen > > Sent: Friday, May 5, 2023 7:50 AM > > To: Liu, Linus ; devel@edk2.groups.io > > Cc: FST-FIR-PRC ; FST FIR Server > > ; Chu, Maggie > > Subject: Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > > HddPasswordDxeInit to use Variable Policy > > > > Sounds good. Thank you very much! > > > > Reviewed-by: Jiewen Yao > > > > > -----Original Message----- > > > From: Liu, Linus > > > Sent: Thursday, May 4, 2023 11:51 AM > > > To: Yao, Jiewen ; devel@edk2.groups.io > > > Cc: FST-FIR-PRC ; FST FIR Server > > > ; Chu, Maggie > > > 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 > > > Sent: Wednesday, May 3, 2023 9:21 AM > > > To: Liu, Linus ; devel@edk2.groups.io > > > Cc: FST-FIR-PRC ; FST FIR Server > > > ; Chu, Maggie > > > 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 > > > > Sent: Wednesday, May 3, 2023 8:40 AM > > > > To: Yao, Jiewen ; devel@edk2.groups.io > > > > Cc: FST-FIR-PRC ; FST FIR Server > > > > ; Chu, Maggie > > > > 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 > > > > Sent: Wednesday, May 3, 2023 12:11 AM > > > > To: Liu, Linus ; devel@edk2.groups.io > > > > Cc: FST-FIR-PRC ; FST FIR Server > > > > ; Chu, Maggie > > > > 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 > > > > > Sent: Tuesday, April 11, 2023 5:55 PM > > > > > To: devel@edk2.groups.io > > > > > Cc: Yao, Jiewen ; FST-FIR-PRC > > > > prc@intel.com>; FST FIR Server ; > > > > > Chu, Maggie > > > > > 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 > > > > > Cc: FST-FIR-PRC > > > > > Cc: FST FIR Server C > > > > > Cc: Maggie Chu > > > > > Signed-off-by: Linus Liu > > > > > --- > > > > > 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 > > > > > > > > > > > > > > > > > > > > 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 > > > > > > > > > > #include > > > > > > > > > > #include > > > > > > > > > > -#include > > > > > > > > > > > > > > > > > > > > #include > > > > > > > > > > #include > > > > > > > > > > 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 > > > > > > > > > >