public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Oram, Isaac W" <isaac.w.oram@intel.com>
To: Benjamin Doron <benjamin.doron00@gmail.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>,
	"Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>,
	"Sinha, Ankit" <ankit.sinha@intel.com>,
	"Gao, Liming" <gaoliming@byosoft.com.cn>,
	"Bi, Dandan" <dandan.bi@intel.com>
Subject: Re: [PATCH v1 2/3] UserAuthFeaturePkg: Included DXE versions are mutually exclusive
Date: Mon, 8 Aug 2022 22:51:42 +0000	[thread overview]
Message-ID: <SA1PR11MB5801D476841BB51BC5FC745FD0639@SA1PR11MB5801.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20220725171207.3154408-2-benjamin.doron00@gmail.com>

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@gmail.com> 
Sent: Monday, July 25, 2022 10:12 AM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Bi, Dandan <dandan.bi@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@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ankit Sinha <ankit.sinha@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Dandan Bi <dandan.bi@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@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


  reply	other threads:[~2022-08-08 22:51 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-25 17:12 [PATCH v1 1/3] AcpiDebugFeaturePkg: Included modules are mutually exclusive Benjamin Doron
2022-07-25 17:12 ` [PATCH v1 2/3] UserAuthFeaturePkg: Included DXE versions " Benjamin Doron
2022-08-08 22:51   ` Oram, Isaac W [this message]
2022-08-09 17:09     ` Benjamin Doron
2022-08-16 21:26       ` Oram, Isaac W
2022-07-25 17:12 ` [PATCH v1 3/3] LogoFeaturePkg,BoardModulePkg: Set existing logo as BGRT Benjamin Doron
2022-08-09 17:51   ` Oram, Isaac W
2022-08-08 19:21 ` [edk2-devel] [PATCH v1 1/3] AcpiDebugFeaturePkg: Included modules are mutually exclusive Oram, Isaac W
     [not found] ` <170974F020AA914C.7247@groups.io>
2022-08-08 19:26   ` Oram, Isaac W

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SA1PR11MB5801D476841BB51BC5FC745FD0639@SA1PR11MB5801.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox