Re: [edk2-devel] [PATCH edk2-platforms 1/1] UserAuthFeaturePkg/UserAuthenticationSmm: Support Standalone MM.
Hi Wei, There is a bigger issue here that your patch series highlights. It is not possible to implement the user authentication feature in standalone MM without also having a DXE driver. But that is an issue that is beyond the scope of this patch series as it will require modifications to the variable driver. Please send a v2 patch with the following changes: - Please update copyright year on UserAuthFeature.dsc and UserAuthenticationSmm.* - Please rename UserAuthenticationVariableLockDxe so that it is clear that this driver is only used for the standalone MM case. Perhaps UserAuthenticationStandaloneMmDxe Thanks, Nate -Original Message- From: Xu, Wei6 Sent: Thursday, November 9, 2023 9:38 PM To: devel@edk2.groups.io Cc: Xu, Wei6 ; Bi, Dandan ; Desimone, Nathaniel L ; Gao, Liming Subject: [PATCH edk2-platforms 1/1] UserAuthFeaturePkg/UserAuthenticationSmm: Support Standalone MM. Refactor UserAuthenticationSmm to support Standalone MM. - Factor out variable lock code logic that references boot services. - UserAuthenticationVariableLockDxe is added to lock the variables. - UserAuthenticationStandaloneMm doesn't lock the variables, needs to reply on UserAuthenticationVariableLockDxe to do the lock. - UserAuthenticationSmm still locks the variables by itself, no need to include UserAuthenticationVariableLockDxe. - Register gEfiEventExitBootServicesGuid notify which is used by the StandaloneMmCore. Since gEdkiiVariableLockProtocolGuid is a deprecated interface, use gEdkiiVariablePolicyProtocolGuid to lock password variables instead. Cc: Dandan Bi Cc: Nate DeSimone Cc: Liming Gao Signed-off-by: Wei6 Xu --- .../Include/UserAuthFeature.dsc | 2 + .../UserAuthenticationSmm.c | 38 - .../UserAuthenticationSmm.h | 26 +++--- .../UserAuthenticationSmm.inf | 11 ++- .../UserAuthenticationStandaloneMm.c | 43 ++ .../UserAuthenticationStandaloneMm.inf| 58 + .../UserAuthenticationTraditionalMm.c | 28 +++ .../UserAuthenticationVariable.h | 36 .../UserAuthenticationVariableLock.c | 84 +++ .../UserAuthenticationVariableLockDxe.c | 31 +++ .../UserAuthenticationVariableLockDxe.inf | 42 ++ 11 files changed, 359 insertions(+), 40 deletions(-) create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationStandaloneMm.c create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationStandaloneMm.inf create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationTraditionalMm.c create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariable.h create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariableLock.c create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariableLockDxe.c create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariableLockDxe.inf diff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc b/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc index 2f39a5580caf..d772b213aaeb 100644 --- a/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc +++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFe +++ ature.dsc @@ -75,3 +75,5 @@ UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationDxe.inf UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthentication2Dxe.inf UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.inf + + UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationStandalo + neMm.inf + UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariable + LockDxe.inf diff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c index 16e3405a82ef..89515ea11e85 100644 --- a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c +++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthentication +++ DxeSmm/UserAuthenticationSmm.c @@ -642,7 +642,7 @@ UaExitBootServices ( { DEBUG ((DEBUG_INFO, "Unregister User Authentication Smi\n")); - gSmst->SmiHandlerUnRegister(mSmmHandle); + gMmst->MmiHandlerUnRegister(mSmmHandle); return EFI_SUCCESS; } @@ -657,54 +657,44 @@ UaExitBootServices ( **/ EFI_STATUS -EFIAPI PasswordSmmInit ( - IN EFI_HANDLE ImageHandle, - IN EFI_SYSTEM_TABLE *SystemTable + VOID ) {
[edk2-devel] [PATCH edk2-platforms 1/1] UserAuthFeaturePkg/UserAuthenticationSmm: Support Standalone MM.
Refactor UserAuthenticationSmm to support Standalone MM. - Factor out variable lock code logic that references boot services. - UserAuthenticationVariableLockDxe is added to lock the variables. - UserAuthenticationStandaloneMm doesn't lock the variables, needs to reply on UserAuthenticationVariableLockDxe to do the lock. - UserAuthenticationSmm still locks the variables by itself, no need to include UserAuthenticationVariableLockDxe. - Register gEfiEventExitBootServicesGuid notify which is used by the StandaloneMmCore. Since gEdkiiVariableLockProtocolGuid is a deprecated interface, use gEdkiiVariablePolicyProtocolGuid to lock password variables instead. Cc: Dandan Bi Cc: Nate DeSimone Cc: Liming Gao Signed-off-by: Wei6 Xu --- .../Include/UserAuthFeature.dsc | 2 + .../UserAuthenticationSmm.c | 38 - .../UserAuthenticationSmm.h | 26 +++--- .../UserAuthenticationSmm.inf | 11 ++- .../UserAuthenticationStandaloneMm.c | 43 ++ .../UserAuthenticationStandaloneMm.inf| 58 + .../UserAuthenticationTraditionalMm.c | 28 +++ .../UserAuthenticationVariable.h | 36 .../UserAuthenticationVariableLock.c | 84 +++ .../UserAuthenticationVariableLockDxe.c | 31 +++ .../UserAuthenticationVariableLockDxe.inf | 42 ++ 11 files changed, 359 insertions(+), 40 deletions(-) create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationStandaloneMm.c create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationStandaloneMm.inf create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationTraditionalMm.c create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariable.h create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariableLock.c create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariableLockDxe.c create mode 100644 Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariableLockDxe.inf diff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc b/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc index 2f39a5580caf..d772b213aaeb 100644 --- a/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc +++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/Include/UserAuthFeature.dsc @@ -75,3 +75,5 @@ UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationDxe.inf UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthentication2Dxe.inf UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.inf + UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationStandaloneMm.inf + UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationVariableLockDxe.inf diff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c index 16e3405a82ef..89515ea11e85 100644 --- a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c +++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c @@ -642,7 +642,7 @@ UaExitBootServices ( { DEBUG ((DEBUG_INFO, "Unregister User Authentication Smi\n")); - gSmst->SmiHandlerUnRegister(mSmmHandle); + gMmst->MmiHandlerUnRegister(mSmmHandle); return EFI_SUCCESS; } @@ -657,54 +657,44 @@ UaExitBootServices ( **/ EFI_STATUS -EFIAPI PasswordSmmInit ( - IN EFI_HANDLE ImageHandle, - IN EFI_SYSTEM_TABLE *SystemTable + VOID ) { EFI_STATUSStatus; - EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock; - CHAR16 PasswordHistoryName[sizeof(USER_AUTHENTICATION_VAR_NAME)/sizeof(CHAR16) + 5]; - UINTN Index; EFI_EVENT ExitBootServicesEvent; EFI_EVENT LegacyBootEvent; + EFI_EVENT SmmExitBootServicesEvent; ASSERT (PASSWORD_HASH_SIZE == SHA256_DIGEST_SIZE); ASSERT (PASSWORD_HISTORY_CHECK_COUNT < 0x); - Status = gSmst->SmmLocateProtocol (, NULL, (VOID**)); + Status = gMmst->MmLocateProtocol (, NULL, (VOID**)); ASSERT_EFI_ERROR (Status); // // Make password variables read-only for DXE driver for security concern. // - Status = gBS->LocateProtocol (, NULL, (VOID **) ); - if (!EFI_ERROR (Status)) { -Status = VariableLock->RequestToLock