I am not aware of any preference between the drivers. Your plan makes sense to me. I will defer to Sai or Chasel if they know more history and use.
Also, please make sure that "build -a IA32 -a X64 -p UserInterface\UserAuthFeaturePkg\UserAuthFeaturePkg.dsc" and "build -a IA32 -a X64 -p AdvancedFeaturePkg\AdvancedFeaturePkg.dsc" complete successfully. Adding the following to UserAuthFeaturePkg.dsc seemed to work for me. [Packages] UserAuthFeaturePkg/UserAuthFeaturePkg.dec [PcdsFeatureFlag] gUserAuthFeaturePkgTokenSpaceGuid.PcdUseVersion1 |FALSE Regards, Isaac From: Benjamin Doron <benjamin.doro...@gmail.com> Sent: Tuesday, August 9, 2022 10:09 AM To: Oram, Isaac W <isaac.w.o...@intel.com> Cc: devel@edk2.groups.io; Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>; Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Sinha, Ankit <ankit.si...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>; Bi, Dandan <dandan...@intel.com> Subject: Re: [PATCH v1 2/3] UserAuthFeaturePkg: Included DXE versions are mutually exclusive Hi Isaac, From memory, the primary difference between the versions of this module is that v2 uses helper libraries to perform some of the functions in UserAuthenticationDxePassword.c and the HII helpers in UserAuthenticationDxe.c. There are other differences, at least including the handler to challenge for the admin password that should be ported or implemented for v2, but I think only one DXE module version is necessary to be in the tree. We can drop this commit and I can work on improving v2 and deprecating v1 - as was my plan, because v2 needs to challenge for the password - but not at the moment. Unless v1's implementation is preferred? Sorry about the subject prefix. I see that patch 1/3 is in. I think I'll work on 2/3 later and 3/3 separately. Best regards, Benjamin On Mon, 8 Aug 2022 at 18:51, Oram, Isaac W <isaac.w.o...@intel.com<mailto:isaac.w.o...@intel.com>> wrote: Benjamin, Can you update the readme with descriptions of the modules and their functionality? Can you comment the PcdUseVersion1 declaration in the DEC file? If you can think of a more descriptive name than "version 1", that would be better. Note that convention (maybe requirement) is to preface non-edk2 repo changes with the mailing list and repo info [edk2-devel][edk2-platforms] so that rules can filter more easily. And multi-commit reviews should have a cover-letter enabled as well so you can provide an overview of the patch series. Here is the command I use: git format-patch -M --thread origin/master --cover-letter --subject-prefix="edk2-devel][edk2-platforms][PATCH V1" Regards, Isaac -----Original Message----- From: Benjamin Doron <benjamin.doro...@gmail.com<mailto:benjamin.doro...@gmail.com>> Sent: Monday, July 25, 2022 10:12 AM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Chaganty, Rangasai V <rangasai.v.chaga...@intel.com<mailto:rangasai.v.chaga...@intel.com>>; Oram, Isaac W <isaac.w.o...@intel.com<mailto:isaac.w.o...@intel.com>>; Desimone, Nathaniel L <nathaniel.l.desim...@intel.com<mailto:nathaniel.l.desim...@intel.com>>; Sinha, Ankit <ankit.si...@intel.com<mailto:ankit.si...@intel.com>>; Gao, Liming <gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>; Bi, Dandan <dandan...@intel.com<mailto:dandan...@intel.com>> Subject: [PATCH v1 2/3] UserAuthFeaturePkg: Included DXE versions are mutually exclusive Versions of the DXE module perform nearly identical functions, and are therefore mutually exclusive. Including both modules results in at least a duplicate HII package and an "already started" assert. Therefore, add a new PCD to determine which module will be included. However, now a second bug can be seen: version 2 does not challenge for the password. An RSC handler to hook UiApp entry is probably needed, as in version 1. Cc: Sai Chaganty <rangasai.v.chaga...@intel.com<mailto:rangasai.v.chaga...@intel.com>> Cc: Isaac Oram <isaac.w.o...@intel.com<mailto:isaac.w.o...@intel.com>> Cc: Nate DeSimone <nathaniel.l.desim...@intel.com<mailto:nathaniel.l.desim...@intel.com>> Cc: Ankit Sinha <ankit.si...@intel.com<mailto:ankit.si...@intel.com>> Cc: Liming Gao <gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>> Cc: Dandan Bi <dandan...@intel.com<mailto:dandan...@intel.com>> Signed-off-by: Benjamin Doron <benjamin.doro...@gmail.com<mailto:benjamin.doro...@gmail.com>> --- Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc | 1 + Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc | 1 + Features/Intel/UserInterface/UserAuthFeaturePkg/Include/PostMemory.fdf | 9 ++++++--- Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc | 3 +++ Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg.dec | 3 ++- 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc index 908fe4d1f7a9..8924dadcaa18 100644 --- a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc +++ b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc @@ -79,6 +79,7 @@ # gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable |TRUE gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable |TRUE+ gUserAuthFeaturePkgTokenSpaceGuid.PcdUseVersion1 |FALSE gVirtualKeyboardFeaturePkgTokenSpaceGuid.PcdVirtualKeyboardFeatureEnable |TRUE #diff --git a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc index d59e3e61b9b9..5365f0a1b844 100644 --- a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc +++ b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc @@ -75,6 +75,7 @@ gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable |FALSE gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable |FALSE+ gUserAuthFeaturePkgTokenSpaceGuid.PcdUseVersion1 |FALSE gVirtualKeyboardFeaturePkgTokenSpaceGuid.PcdVirtualKeyboardFeatureEnable |FALSE #diff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/PostMemory.fdf b/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/PostMemory.fdf index 030dcbe763d8..827deb08de7e 100644 --- a/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/PostMemory.fdf +++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/PostMemory +++ .fdf @@ -7,6 +7,9 @@ # ## -INF UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationDxe.inf-INF UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthentication2Dxe.inf-INF UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.inf+!if gUserAuthFeaturePkgTokenSpaceGuid.PcdUseVersion1 == TRUE+ INF UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationDxe.inf+!else+ INF UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthentication2Dxe.inf+!endif+ INF UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.infdiff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc b/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc index 2f39a5580caf..022678d5c852 100644 --- a/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc +++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFe +++ ature.dsc @@ -72,6 +72,9 @@ UserAuthFeaturePkg/Library/UserPasswordUiLib/UserPasswordUiLib.inf # Add components here that should be included in the package build.+!if gUserAuthFeaturePkgTokenSpaceGuid.PcdUseVersion1 == TRUE UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationDxe.inf+!else UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthentication2Dxe.inf+!endif UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.infdiff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg.dec b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg.dec index a9174edd5486..209a0ba6a6c7 100644 --- a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg.dec +++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg +++ .dec @@ -38,8 +38,9 @@ gUserAuthenticationGuid = { 0xee24a7f7, 0x606b, 0x4724, { 0xb3, 0xc9, 0xf5, 0xae, 0x4a, 0x3b, 0x81, 0x65}} [PcdsFeatureFlag]- ## This PCD specifies whether StatusCode is reported via USB3 Serial port.+ ## This PCD specifies whether user authentication feature is enabled. gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable|FALSE|BOOLEAN|0xA0000001+ gUserAuthFeaturePkgTokenSpaceGuid.PcdUseVersion1|FALSE|BOOLEAN|0xA0000002 [PcdsFixedAtBuild,PcdsPatchableInModule,PcdsDynamic,PcdsDynamicEx] ## Indicate whether the password is cleared.-- 2.36.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92490): https://edk2.groups.io/g/devel/message/92490 Mute This Topic: https://groups.io/mt/92609643/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
