[edk2-devel] [PATCH v1 2/3] UserAuthFeaturePkg: Included DXE versions are mutually exclusive

Tue Aug 16 21:26:06 UTC 2022

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.doron00 at gmail.com>
Sent: Tuesday, August 9, 2022 10:09 AM
To: Oram, Isaac W <isaac.w.oram at intel.com>
Cc: devel at edk2.groups.io; Chaganty, Rangasai V <rangasai.v.chaganty at intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Sinha, Ankit <ankit.sinha at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>; Bi, Dandan <dandan.bi at 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.oram at intel.com<mailto:isaac.w.oram at 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.doron00 at gmail.com<mailto:benjamin.doron00 at gmail.com>>
Sent: Monday, July 25, 2022 10:12 AM
To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Cc: Chaganty, Rangasai V <rangasai.v.chaganty at intel.com<mailto:rangasai.v.chaganty at intel.com>>; Oram, Isaac W <isaac.w.oram at intel.com<mailto:isaac.w.oram at intel.com>>; Desimone, Nathaniel L <nathaniel.l.desimone at intel.com<mailto:nathaniel.l.desimone at intel.com>>; Sinha, Ankit <ankit.sinha at intel.com<mailto:ankit.sinha at intel.com>>; Gao, Liming <gaoliming at byosoft.com.cn<mailto:gaoliming at byosoft.com.cn>>; Bi, Dandan <dandan.bi at intel.com<mailto:dandan.bi at 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.chaganty at intel.com<mailto:rangasai.v.chaganty at intel.com>>
Cc: Isaac Oram <isaac.w.oram at intel.com<mailto:isaac.w.oram at intel.com>>
Cc: Nate DeSimone <nathaniel.l.desimone at intel.com<mailto:nathaniel.l.desimone at intel.com>>
Cc: Ankit Sinha <ankit.sinha at intel.com<mailto:ankit.sinha at intel.com>>
Cc: Liming Gao <gaoliming at byosoft.com.cn<mailto:gaoliming at byosoft.com.cn>>
Cc: Dandan Bi <dandan.bi at intel.com<mailto:dandan.bi at intel.com>>
Signed-off-by: Benjamin Doron <benjamin.doron00 at gmail.com<mailto:benjamin.doron00 at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20220816/316cdbe3/attachment-0001.htm>
