* [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.
@ 2022-04-11 7:06 Li, Zhihao
2022-04-12 0:59 ` 回复: [edk2-devel] " gaoliming
0 siblings, 1 reply; 7+ messages in thread
From: Li, Zhihao @ 2022-04-11 7:06 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Liming Gao, Siyuan Fu, Ni Ray, Michael D Kinney
REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3854
In UefiCpuPkg, there are a new Protocol with the new service
SmmWaitForAllProcessor(), which can be used by SMI handler
to optionally wait for other APs to complete SMM rendezvous in
relaxed AP mode.
This patch use the new service to let VariableSmm driver work
normally in relaxed AP mode.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Ni Ray <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Zhihao Li <zhihao.li@intel.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 10 +++++++-
MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h | 27 ++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 5 +++-
MdeModulePkg/MdeModulePkg.dsc | 4 ++-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 3 ++-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf | 3 ++-
6 files changed, 47 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 517cae7b00f8..52a9b0e6b202 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -14,7 +14,7 @@
VariableServiceSetVariable(), VariableServiceQueryVariableInfo(), ReclaimForOS(),
SmmVariableGetStatistics() should also do validation based on its own knowledge.
-Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2022, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/MmServicesTableLib.h>
#include <Library/VariablePolicyLib.h>
+#include <Library/SmmCpuRendezvousLib.h>
#include <Guid/SmmVariableCommon.h>
#include "Variable.h"
@@ -656,6 +657,13 @@ SmmVariableHandler (
goto EXIT;
}
+ if ((SmmVariableHeader->Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
+ if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
+ DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check in SMM!\n"));
+ goto EXIT;
+ }
+ }
+
Status = VariableServiceSetVariable (
SmmVariableHeader->Name,
&SmmVariableHeader->Guid,
diff --git a/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
new file mode 100644
index 000000000000..82e459e9106e
--- /dev/null
+++ b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
@@ -0,0 +1,27 @@
+/** @file
+ SMM CPU Rendezvous library header file.
+
+ Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SMM_CPU_RENDEZVOUS_H_
+#define SMM_CPU_RENDEZVOUS_H_
+
+/**
+ This routine wait for all AP processors to arrive in SMM.
+
+ @param[in] BlockingMode Blocking mode or non-blocking mode.
+
+ @retval EFI_SUCCESS All processors checked in to SMM.
+ @retval EFI_TIMEOUT Wait for all APs until timeout.
+
+**/
+EFI_STATUS
+EFIAPI
+SmmWaitForAllProcessor (
+ IN BOOLEAN BlockingMode
+ );
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 463e889e9a68..06ada41b7344 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -4,7 +4,7 @@
# and libraries instances, which are used for those modules.
#
# Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
-# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
# (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development LP<BR>
# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
@@ -154,6 +154,9 @@
#
VariablePolicyHelperLib|Include/Library/VariablePolicyHelperLib.h
+ ## @libraryclass Provides function for SMM CPU Rendezvous Library.
+ SmmCpuRendezvousLib|Include/Library/SmmCpuRendezvousLib.h
+
[Guids]
## MdeModule package token space guid
# Include/Guid/MdeModulePkgTokenSpace.h
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index b1d83461865e..a15dd5d7b23d 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -2,7 +2,7 @@
# EFI/PI Reference Module Package for All Architectures
#
# (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>
# Copyright (c) Microsoft Corporation.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -152,6 +152,7 @@
SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf
LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf
SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
+ SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.inf
[LibraryClasses.common.UEFI_DRIVER]
HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
@@ -172,6 +173,7 @@
MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandaloneMmLib.inf
MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
+ SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.inf
[LibraryClasses.ARM, LibraryClasses.AARCH64]
ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
index eaa97a01c6e5..0bebd92b1626 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -18,7 +18,7 @@
# may not be modified without authorization. If platform fails to protect these resources,
# the authentication service provided in this driver will be broken, and the behavior is undefined.
#
-# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2022, Intel Corporation. All rights reserved.<BR>
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -82,6 +82,7 @@
UefiBootServicesTableLib
VariablePolicyLib
VariablePolicyHelperLib
+ SmmCpuRendezvousLib
[Protocols]
gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
index d8c4f77e7f1f..595baaf70164 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
@@ -18,7 +18,7 @@
# may not be modified without authorization. If platform fails to protect these resources,
# the authentication service provided in this driver will be broken, and the behavior is undefined.
#
-# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 2022, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -78,6 +78,7 @@
VarCheckLib
VariablePolicyLib
VariablePolicyHelperLib
+ SmmCpuRendezvousLib
[Protocols]
gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
--
2.26.2.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.
2022-04-11 7:06 [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver Li, Zhihao
@ 2022-04-12 0:59 ` gaoliming
2022-04-12 18:13 ` Li, Zhihao
0 siblings, 1 reply; 7+ messages in thread
From: gaoliming @ 2022-04-12 0:59 UTC (permalink / raw)
To: devel, zhihao.li
Cc: 'Jian J Wang', 'Siyuan Fu', 'Ni Ray',
'Michael D Kinney'
Zhihao:
This patch breaks two things. One is to let MdeModulePkg depend on UefiCpuPkg, another is to let VariableStandaloneMm depend on UefiCpuPkg SmmCpuRendezvousLib. Please provide your proposal to resolve these two dependency first.
Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li, Zhihao
> 发送时间: 2022年4月11日 15:07
> 收件人: devel@edk2.groups.io
> 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Siyuan Fu <siyuan.fu@intel.com>; Ni Ray
> <ray.ni@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> 主题: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> SmmWaitForAllProcessor() in VariableSmm driver.
>
> REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3854
>
> In UefiCpuPkg, there are a new Protocol with the new service
> SmmWaitForAllProcessor(), which can be used by SMI handler
> to optionally wait for other APs to complete SMM rendezvous in
> relaxed AP mode.
>
> This patch use the new service to let VariableSmm driver work
> normally in relaxed AP mode.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Ni Ray <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>
> Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> ---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> | 10 +++++++-
> MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> | 27 ++++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec
> | 5 +++-
> MdeModulePkg/MdeModulePkg.dsc
> | 4 ++-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> | 3 ++-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> | 3 ++-
> 6 files changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index 517cae7b00f8..52a9b0e6b202 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> @@ -14,7 +14,7 @@
> VariableServiceSetVariable(), VariableServiceQueryVariableInfo(),
> ReclaimForOS(),
>
> SmmVariableGetStatistics() should also do validation based on its own
> knowledge.
>
>
>
> -Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
>
> +Copyright (c) 2010 - 2022, Intel Corporation. All rights reserved.<BR>
>
> Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
> @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
> #include <Library/MmServicesTableLib.h>
>
> #include <Library/VariablePolicyLib.h>
>
> +#include <Library/SmmCpuRendezvousLib.h>
>
>
>
> #include <Guid/SmmVariableCommon.h>
>
> #include "Variable.h"
>
> @@ -656,6 +657,13 @@ SmmVariableHandler (
> goto EXIT;
>
> }
>
>
>
> + if ((SmmVariableHeader->Attributes &
> EFI_VARIABLE_NON_VOLATILE) != 0) {
>
> + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
>
> + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP
> check in SMM!\n"));
>
> + goto EXIT;
>
> + }
>
> + }
>
> +
>
> Status = VariableServiceSetVariable (
>
> SmmVariableHeader->Name,
>
> &SmmVariableHeader->Guid,
>
> diff --git a/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> new file mode 100644
> index 000000000000..82e459e9106e
> --- /dev/null
> +++ b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> @@ -0,0 +1,27 @@
> +/** @file
>
> + SMM CPU Rendezvous library header file.
>
> +
>
> + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +#ifndef SMM_CPU_RENDEZVOUS_H_
>
> +#define SMM_CPU_RENDEZVOUS_H_
>
> +
>
> +/**
>
> + This routine wait for all AP processors to arrive in SMM.
>
> +
>
> + @param[in] BlockingMode Blocking mode or non-blocking mode.
>
> +
>
> + @retval EFI_SUCCESS All processors checked in to SMM.
>
> + @retval EFI_TIMEOUT Wait for all APs until timeout.
>
> +
>
> +**/
>
> +EFI_STATUS
>
> +EFIAPI
>
> +SmmWaitForAllProcessor (
>
> + IN BOOLEAN BlockingMode
>
> + );
>
> +
>
> +#endif
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 463e889e9a68..06ada41b7344 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -4,7 +4,7 @@
> # and libraries instances, which are used for those modules.
>
> #
>
> # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
>
> -# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
>
> +# Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>
>
> # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
>
> # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development
> LP<BR>
>
> # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>
> @@ -154,6 +154,9 @@
> #
>
> VariablePolicyHelperLib|Include/Library/VariablePolicyHelperLib.h
>
>
>
> + ## @libraryclass Provides function for SMM CPU Rendezvous Library.
>
> + SmmCpuRendezvousLib|Include/Library/SmmCpuRendezvousLib.h
>
> +
>
> [Guids]
>
> ## MdeModule package token space guid
>
> # Include/Guid/MdeModulePkgTokenSpace.h
>
> diff --git a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc
> index b1d83461865e..a15dd5d7b23d 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -2,7 +2,7 @@
> # EFI/PI Reference Module Package for All Architectures
>
> #
>
> # (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
>
> -# Copyright (c) 2007 - 2021, Intel Corporation. All rights reserved.<BR>
>
> +# Copyright (c) 2007 - 2022, Intel Corporation. All rights reserved.<BR>
>
> # Copyright (c) Microsoft Corporation.
>
> #
>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -152,6 +152,7 @@
>
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTa
> bleLib.inf
>
>
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.i
> nf
>
> SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
>
> +
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> uRendezvousLib.inf
>
>
>
> [LibraryClasses.common.UEFI_DRIVER]
>
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>
> @@ -172,6 +173,7 @@
>
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
> aloneMmServicesTableLib.inf
>
>
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalon
> eMmLib.inf
>
>
> MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
> MemLib.inf
>
> +
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> uRendezvousLib.inf
>
>
>
> [LibraryClasses.ARM, LibraryClasses.AARCH64]
>
> ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index eaa97a01c6e5..0bebd92b1626 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> @@ -18,7 +18,7 @@
> # may not be modified without authorization. If platform fails to protect
> these resources,
>
> # the authentication service provided in this driver will be broken, and the
> behavior is undefined.
>
> #
>
> -# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
>
> +# Copyright (c) 2010 - 2022, Intel Corporation. All rights reserved.<BR>
>
> # Copyright (c) Microsoft Corporation.
>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #
>
> @@ -82,6 +82,7 @@
> UefiBootServicesTableLib
>
> VariablePolicyLib
>
> VariablePolicyHelperLib
>
> + SmmCpuRendezvousLib
>
>
>
> [Protocols]
>
> gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
>
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> index d8c4f77e7f1f..595baaf70164 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> @@ -18,7 +18,7 @@
> # may not be modified without authorization. If platform fails to protect
> these resources,
>
> # the authentication service provided in this driver will be broken, and the
> behavior is undefined.
>
> #
>
> -# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
>
> +# Copyright (c) 2010 - 2022, Intel Corporation. All rights reserved.<BR>
>
> # Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
>
> # Copyright (c) Microsoft Corporation.
>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -78,6 +78,7 @@
> VarCheckLib
>
> VariablePolicyLib
>
> VariablePolicyHelperLib
>
> + SmmCpuRendezvousLib
>
>
>
> [Protocols]
>
> gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
>
> --
> 2.26.2.windows.1
>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.
2022-04-12 0:59 ` 回复: [edk2-devel] " gaoliming
@ 2022-04-12 18:13 ` Li, Zhihao
2022-04-13 1:30 ` 回复: " gaoliming
0 siblings, 1 reply; 7+ messages in thread
From: Li, Zhihao @ 2022-04-12 18:13 UTC (permalink / raw)
To: Gao, Liming, devel@edk2.groups.io
Cc: Wang, Jian J, Fu, Siyuan, Ni, Ray, Kinney, Michael D
1. Although SmmCpuRendezvousLib in UefiCpuPkg, add SmmRendezvousLib.h into MdeModulePkg/Include/Library folder bypass the check.
2. The SmmRendezvousLib is a standalone MM library and doesn’t have any DXE service dependency. It can be used by SMM variable module and MM variable module.
As the patch following, it have passed the Edk2 CI test. The code can run successfully in practice, but I'm not sure if this is acceptable in terms of the standard.
-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn>
Sent: Tuesday, April 12, 2022 9:00 AM
To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.
Zhihao:
This patch breaks two things. One is to let MdeModulePkg depend on UefiCpuPkg, another is to let VariableStandaloneMm depend on UefiCpuPkg SmmCpuRendezvousLib. Please provide your proposal to resolve these two dependency first.
Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li, Zhihao
> 发送时间: 2022年4月11日 15:07
> 收件人: devel@edk2.groups.io
> 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Siyuan Fu <siyuan.fu@intel.com>; Ni Ray
> <ray.ni@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> 主题: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> SmmWaitForAllProcessor() in VariableSmm driver.
>
> REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3854
>
> In UefiCpuPkg, there are a new Protocol with the new service
> SmmWaitForAllProcessor(), which can be used by SMI handler to
> optionally wait for other APs to complete SMM rendezvous in relaxed AP
> mode.
>
> This patch use the new service to let VariableSmm driver work normally
> in relaxed AP mode.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Ni Ray <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>
> Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> ---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> | 10 +++++++-
> MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> | 27 ++++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec
> | 5 +++-
> MdeModulePkg/MdeModulePkg.dsc
> | 4 ++-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> | 3 ++-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> | 3 ++-
> 6 files changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index 517cae7b00f8..52a9b0e6b202 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> @@ -14,7 +14,7 @@
> VariableServiceSetVariable(), VariableServiceQueryVariableInfo(),
> ReclaimForOS(),
>
> SmmVariableGetStatistics() should also do validation based on its
> own knowledge.
>
>
>
> -Copyright (c) 2010 - 2019, Intel Corporation. All rights
> reserved.<BR>
>
> +Copyright (c) 2010 - 2022, Intel Corporation. All rights
> +reserved.<BR>
>
> Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
> @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
> #include <Library/MmServicesTableLib.h>
>
> #include <Library/VariablePolicyLib.h>
>
> +#include <Library/SmmCpuRendezvousLib.h>
>
>
>
> #include <Guid/SmmVariableCommon.h>
>
> #include "Variable.h"
>
> @@ -656,6 +657,13 @@ SmmVariableHandler (
> goto EXIT;
>
> }
>
>
>
> + if ((SmmVariableHeader->Attributes &
> EFI_VARIABLE_NON_VOLATILE) != 0) {
>
> + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
>
> + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP
> check in SMM!\n"));
>
> + goto EXIT;
>
> + }
>
> + }
>
> +
>
> Status = VariableServiceSetVariable (
>
> SmmVariableHeader->Name,
>
> &SmmVariableHeader->Guid,
>
> diff --git a/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> new file mode 100644
> index 000000000000..82e459e9106e
> --- /dev/null
> +++ b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> @@ -0,0 +1,27 @@
> +/** @file
>
> + SMM CPU Rendezvous library header file.
>
> +
>
> + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +
>
> +**/
>
> +
>
> +#ifndef SMM_CPU_RENDEZVOUS_H_
>
> +#define SMM_CPU_RENDEZVOUS_H_
>
> +
>
> +/**
>
> + This routine wait for all AP processors to arrive in SMM.
>
> +
>
> + @param[in] BlockingMode Blocking mode or non-blocking mode.
>
> +
>
> + @retval EFI_SUCCESS All processors checked in to SMM.
>
> + @retval EFI_TIMEOUT Wait for all APs until timeout.
>
> +
>
> +**/
>
> +EFI_STATUS
>
> +EFIAPI
>
> +SmmWaitForAllProcessor (
>
> + IN BOOLEAN BlockingMode
>
> + );
>
> +
>
> +#endif
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index 463e889e9a68..06ada41b7344
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -4,7 +4,7 @@
> # and libraries instances, which are used for those modules.
>
> #
>
> # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
>
> -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> reserved.<BR>
>
> +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> +reserved.<BR>
>
> # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
>
> # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development
> LP<BR>
>
> # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
>
> @@ -154,6 +154,9 @@
> #
>
> VariablePolicyHelperLib|Include/Library/VariablePolicyHelperLib.h
>
>
>
> + ## @libraryclass Provides function for SMM CPU Rendezvous Library.
>
> + SmmCpuRendezvousLib|Include/Library/SmmCpuRendezvousLib.h
>
> +
>
> [Guids]
>
> ## MdeModule package token space guid
>
> # Include/Guid/MdeModulePkgTokenSpace.h
>
> diff --git a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc index b1d83461865e..a15dd5d7b23d
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -2,7 +2,7 @@
> # EFI/PI Reference Module Package for All Architectures
>
> #
>
> # (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
>
> -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> reserved.<BR>
>
> +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> +reserved.<BR>
>
> # Copyright (c) Microsoft Corporation.
>
> #
>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -152,6 +152,7 @@
>
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTa
> bleLib.inf
>
>
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.i
> nf
>
> SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
>
> +
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> uRendezvousLib.inf
>
>
>
> [LibraryClasses.common.UEFI_DRIVER]
>
> HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>
> @@ -172,6 +173,7 @@
>
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
> aloneMmServicesTableLib.inf
>
>
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalon
> eMmLib.inf
>
>
> MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
> MemLib.inf
>
> +
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> uRendezvousLib.inf
>
>
>
> [LibraryClasses.ARM, LibraryClasses.AARCH64]
>
> ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
>
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index eaa97a01c6e5..0bebd92b1626 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> @@ -18,7 +18,7 @@
> # may not be modified without authorization. If platform fails to
> protect these resources,
>
> # the authentication service provided in this driver will be broken,
> and the behavior is undefined.
>
> #
>
> -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> reserved.<BR>
>
> +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> +reserved.<BR>
>
> # Copyright (c) Microsoft Corporation.
>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #
>
> @@ -82,6 +82,7 @@
> UefiBootServicesTableLib
>
> VariablePolicyLib
>
> VariablePolicyHelperLib
>
> + SmmCpuRendezvousLib
>
>
>
> [Protocols]
>
> gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
>
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> index d8c4f77e7f1f..595baaf70164 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> @@ -18,7 +18,7 @@
> # may not be modified without authorization. If platform fails to
> protect these resources,
>
> # the authentication service provided in this driver will be broken,
> and the behavior is undefined.
>
> #
>
> -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> reserved.<BR>
>
> +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> +reserved.<BR>
>
> # Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
>
> # Copyright (c) Microsoft Corporation.
>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -78,6 +78,7 @@
> VarCheckLib
>
> VariablePolicyLib
>
> VariablePolicyHelperLib
>
> + SmmCpuRendezvousLib
>
>
>
> [Protocols]
>
> gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
>
> --
> 2.26.2.windows.1
>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.
2022-04-12 18:13 ` Li, Zhihao
@ 2022-04-13 1:30 ` gaoliming
2022-04-15 9:12 ` Li, Zhihao
0 siblings, 1 reply; 7+ messages in thread
From: gaoliming @ 2022-04-13 1:30 UTC (permalink / raw)
To: 'Li, Zhihao', devel, 'Michael Kubacki',
'Ard Biesheuvel', 'Leif Lindholm',
'Sean Brogan'
Cc: 'Wang, Jian J', 'Fu, Siyuan', 'Ni, Ray',
'Kinney, Michael D', 'Jiewen Yao'
Zhihao:
I remember CI has the check for the package dependency. If this patch passes CI, seemly this checker doesn't do. You can see DependencyCheck in MdeModulePkg\MdeModulePkg.ci.yaml.
And, this patch introduces new dependency in VariableStandaloneMm. It has been used in edk2 platform ARM platform. This change will break these platform build. Please notify the platform owners.
Sean and Michael:
This patch adds UefiCpuPkg library instance SmmCpuRendezvousLib into MdeModulePkg.dsc. But, CI can pass. Is this the expected behavior?
Thanks
Liming
> -----邮件原件-----
> 发件人: Li, Zhihao <zhihao.li@intel.com>
> 发送时间: 2022年4月13日 2:14
> 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> 抄送: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> 主题: RE: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> SmmWaitForAllProcessor() in VariableSmm driver.
>
> 1. Although SmmCpuRendezvousLib in UefiCpuPkg, add SmmRendezvousLib.h
> into MdeModulePkg/Include/Library folder bypass the check.
> 2. The SmmRendezvousLib is a standalone MM library and doesn’t have any
> DXE service dependency. It can be used by SMM variable module and MM
> variable module.
>
> As the patch following, it have passed the Edk2 CI test. The code can run
> successfully in practice, but I'm not sure if this is acceptable in terms of the
> standard.
>
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Tuesday, April 12, 2022 9:00 AM
> To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> SmmWaitForAllProcessor() in VariableSmm driver.
>
> Zhihao:
> This patch breaks two things. One is to let MdeModulePkg depend on
> UefiCpuPkg, another is to let VariableStandaloneMm depend on UefiCpuPkg
> SmmCpuRendezvousLib. Please provide your proposal to resolve these two
> dependency first.
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li, Zhihao
> > 发送时间: 2022年4月11日 15:07
> > 收件人: devel@edk2.groups.io
> > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Siyuan Fu <siyuan.fu@intel.com>; Ni Ray
> > <ray.ni@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> > 主题: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > SmmWaitForAllProcessor() in VariableSmm driver.
> >
> > REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3854
> >
> > In UefiCpuPkg, there are a new Protocol with the new service
> > SmmWaitForAllProcessor(), which can be used by SMI handler to
> > optionally wait for other APs to complete SMM rendezvous in relaxed AP
> > mode.
> >
> > This patch use the new service to let VariableSmm driver work normally
> > in relaxed AP mode.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > Cc: Ni Ray <ray.ni@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >
> > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > ---
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > | 10 +++++++-
> > MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > | 27 ++++++++++++++++++++
> > MdeModulePkg/MdeModulePkg.dec
> > | 5 +++-
> > MdeModulePkg/MdeModulePkg.dsc
> > | 4 ++-
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > | 3 ++-
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > | 3 ++-
> > 6 files changed, 47 insertions(+), 5 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > index 517cae7b00f8..52a9b0e6b202 100644
> > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > @@ -14,7 +14,7 @@
> > VariableServiceSetVariable(), VariableServiceQueryVariableInfo(),
> > ReclaimForOS(),
> >
> > SmmVariableGetStatistics() should also do validation based on its
> > own knowledge.
> >
> >
> >
> > -Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > reserved.<BR>
> >
> > +Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > +reserved.<BR>
> >
> > Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> >
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >
> >
> > @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >
> > #include <Library/MmServicesTableLib.h>
> >
> > #include <Library/VariablePolicyLib.h>
> >
> > +#include <Library/SmmCpuRendezvousLib.h>
> >
> >
> >
> > #include <Guid/SmmVariableCommon.h>
> >
> > #include "Variable.h"
> >
> > @@ -656,6 +657,13 @@ SmmVariableHandler (
> > goto EXIT;
> >
> > }
> >
> >
> >
> > + if ((SmmVariableHeader->Attributes &
> > EFI_VARIABLE_NON_VOLATILE) != 0) {
> >
> > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> >
> > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP
> > check in SMM!\n"));
> >
> > + goto EXIT;
> >
> > + }
> >
> > + }
> >
> > +
> >
> > Status = VariableServiceSetVariable (
> >
> > SmmVariableHeader->Name,
> >
> > &SmmVariableHeader->Guid,
> >
> > diff --git a/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > new file mode 100644
> > index 000000000000..82e459e9106e
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > @@ -0,0 +1,27 @@
> > +/** @file
> >
> > + SMM CPU Rendezvous library header file.
> >
> > +
> >
> > + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> >
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > +
> >
> > +**/
> >
> > +
> >
> > +#ifndef SMM_CPU_RENDEZVOUS_H_
> >
> > +#define SMM_CPU_RENDEZVOUS_H_
> >
> > +
> >
> > +/**
> >
> > + This routine wait for all AP processors to arrive in SMM.
> >
> > +
> >
> > + @param[in] BlockingMode Blocking mode or non-blocking mode.
> >
> > +
> >
> > + @retval EFI_SUCCESS All processors checked in to SMM.
> >
> > + @retval EFI_TIMEOUT Wait for all APs until timeout.
> >
> > +
> >
> > +**/
> >
> > +EFI_STATUS
> >
> > +EFIAPI
> >
> > +SmmWaitForAllProcessor (
> >
> > + IN BOOLEAN BlockingMode
> >
> > + );
> >
> > +
> >
> > +#endif
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > b/MdeModulePkg/MdeModulePkg.dec index
> 463e889e9a68..06ada41b7344
> > 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -4,7 +4,7 @@
> > # and libraries instances, which are used for those modules.
> >
> > #
> >
> > # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> >
> > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > reserved.<BR>
> >
> > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > +reserved.<BR>
> >
> > # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> >
> > # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development
> > LP<BR>
> >
> > # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> >
> > @@ -154,6 +154,9 @@
> > #
> >
> > VariablePolicyHelperLib|Include/Library/VariablePolicyHelperLib.h
> >
> >
> >
> > + ## @libraryclass Provides function for SMM CPU Rendezvous Library.
> >
> > + SmmCpuRendezvousLib|Include/Library/SmmCpuRendezvousLib.h
> >
> > +
> >
> > [Guids]
> >
> > ## MdeModule package token space guid
> >
> > # Include/Guid/MdeModulePkgTokenSpace.h
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > b/MdeModulePkg/MdeModulePkg.dsc index
> b1d83461865e..a15dd5d7b23d
> > 100644
> > --- a/MdeModulePkg/MdeModulePkg.dsc
> > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > @@ -2,7 +2,7 @@
> > # EFI/PI Reference Module Package for All Architectures
> >
> > #
> >
> > # (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> >
> > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > reserved.<BR>
> >
> > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > +reserved.<BR>
> >
> > # Copyright (c) Microsoft Corporation.
> >
> > #
> >
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -152,6 +152,7 @@
> >
> >
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTa
> > bleLib.inf
> >
> >
> >
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.i
> > nf
> >
> > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
> >
> > +
> >
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > uRendezvousLib.inf
> >
> >
> >
> > [LibraryClasses.common.UEFI_DRIVER]
> >
> > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> >
> > @@ -172,6 +173,7 @@
> >
> >
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
> > aloneMmServicesTableLib.inf
> >
> >
> >
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalon
> > eMmLib.inf
> >
> >
> >
> MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
> > MemLib.inf
> >
> > +
> >
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > uRendezvousLib.inf
> >
> >
> >
> > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> >
> > ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> >
> > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > index eaa97a01c6e5..0bebd92b1626 100644
> > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > @@ -18,7 +18,7 @@
> > # may not be modified without authorization. If platform fails to
> > protect these resources,
> >
> > # the authentication service provided in this driver will be broken,
> > and the behavior is undefined.
> >
> > #
> >
> > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > reserved.<BR>
> >
> > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > +reserved.<BR>
> >
> > # Copyright (c) Microsoft Corporation.
> >
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > #
> >
> > @@ -82,6 +82,7 @@
> > UefiBootServicesTableLib
> >
> > VariablePolicyLib
> >
> > VariablePolicyHelperLib
> >
> > + SmmCpuRendezvousLib
> >
> >
> >
> > [Protocols]
> >
> > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > f
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > f
> > index d8c4f77e7f1f..595baaf70164 100644
> > ---
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > f
> > +++
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > f
> > @@ -18,7 +18,7 @@
> > # may not be modified without authorization. If platform fails to
> > protect these resources,
> >
> > # the authentication service provided in this driver will be broken,
> > and the behavior is undefined.
> >
> > #
> >
> > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > reserved.<BR>
> >
> > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > +reserved.<BR>
> >
> > # Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> >
> > # Copyright (c) Microsoft Corporation.
> >
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -78,6 +78,7 @@
> > VarCheckLib
> >
> > VariablePolicyLib
> >
> > VariablePolicyHelperLib
> >
> > + SmmCpuRendezvousLib
> >
> >
> >
> > [Protocols]
> >
> > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> >
> > --
> > 2.26.2.windows.1
> >
> >
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.
2022-04-13 1:30 ` 回复: " gaoliming
@ 2022-04-15 9:12 ` Li, Zhihao
2022-04-17 3:27 ` 回复: " gaoliming
0 siblings, 1 reply; 7+ messages in thread
From: Li, Zhihao @ 2022-04-15 9:12 UTC (permalink / raw)
To: Gao, Liming, devel@edk2.groups.io, 'Michael Kubacki',
'Ard Biesheuvel', 'Leif Lindholm',
'Sean Brogan'
Cc: Wang, Jian J, Fu, Siyuan, Ni, Ray, Kinney, Michael D, Yao, Jiewen
I see the configuration in MdeModulePkg\MdeModulePkg.ci.yaml. I add SmmCpuRendezvousLib.h in
MdeModulePkg/Include/Library folder so that it doesn't need add UefiCpuPkg.dec in [Packages] and bypass the check.
For the second point, due to the patch pass the CI test, it also pass the PlatformCI_ArmVirtPkg. I don't realize that any problem in
Arm platform build.
Leif and Ard
Does the patch has any influence on arm platform build?
Liming:
If the solution is not acceptable, how about I create NULL version of SmmCpuRendezvousLib in MdeModulePkg and use it in MdeModulePkg.dsc? Is that a acceptable solution for you?
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Wednesday, April 13, 2022 9:30 AM
> To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; 'Michael Kubacki'
> <mikuback@linux.microsoft.com>; 'Ard Biesheuvel'
> <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>; 'Sean
> Brogan' <sean.brogan@microsoft.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>
> Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> SmmWaitForAllProcessor() in VariableSmm driver.
>
> Zhihao:
> I remember CI has the check for the package dependency. If this patch passes
> CI, seemly this checker doesn't do. You can see DependencyCheck in
> MdeModulePkg\MdeModulePkg.ci.yaml.
> And, this patch introduces new dependency in VariableStandaloneMm. It has
> been used in edk2 platform ARM platform. This change will break these platform
> build. Please notify the platform owners.
>
> Sean and Michael:
> This patch adds UefiCpuPkg library instance SmmCpuRendezvousLib into
> MdeModulePkg.dsc. But, CI can pass. Is this the expected behavior?
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Li, Zhihao <zhihao.li@intel.com>
> > 发送时间: 2022年4月13日 2:14
> > 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > 主题: RE: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > SmmWaitForAllProcessor() in VariableSmm driver.
> >
> > 1. Although SmmCpuRendezvousLib in UefiCpuPkg, add SmmRendezvousLib.h
> > into MdeModulePkg/Include/Library folder bypass the check.
> > 2. The SmmRendezvousLib is a standalone MM library and doesn’t have
> > any DXE service dependency. It can be used by SMM variable module and
> > MM variable module.
> >
> > As the patch following, it have passed the Edk2 CI test. The code can
> > run successfully in practice, but I'm not sure if this is acceptable
> > in terms of the standard.
> >
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Tuesday, April 12, 2022 9:00 AM
> > To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > SmmWaitForAllProcessor() in VariableSmm driver.
> >
> > Zhihao:
> > This patch breaks two things. One is to let MdeModulePkg depend on
> > UefiCpuPkg, another is to let VariableStandaloneMm depend on
> > UefiCpuPkg SmmCpuRendezvousLib. Please provide your proposal to
> > resolve these two dependency first.
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li, Zhihao
> > > 发送时间: 2022年4月11日 15:07
> > > 收件人: devel@edk2.groups.io
> > > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > > <gaoliming@byosoft.com.cn>; Siyuan Fu <siyuan.fu@intel.com>; Ni Ray
> > > <ray.ni@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> > > 主题: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > > SmmWaitForAllProcessor() in VariableSmm driver.
> > >
> > > REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3854
> > >
> > > In UefiCpuPkg, there are a new Protocol with the new service
> > > SmmWaitForAllProcessor(), which can be used by SMI handler to
> > > optionally wait for other APs to complete SMM rendezvous in relaxed
> > > AP mode.
> > >
> > > This patch use the new service to let VariableSmm driver work
> > > normally in relaxed AP mode.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > Cc: Ni Ray <ray.ni@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > >
> > > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > > ---
> > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > | 10 +++++++-
> > > MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > | 27 ++++++++++++++++++++
> > > MdeModulePkg/MdeModulePkg.dec
> > > | 5 +++-
> > > MdeModulePkg/MdeModulePkg.dsc
> > > | 4 ++-
> > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > | 3 ++-
> > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > > | 3 ++-
> > > 6 files changed, 47 insertions(+), 5 deletions(-)
> > >
> > > diff --git
> > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > index 517cae7b00f8..52a9b0e6b202 100644
> > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > @@ -14,7 +14,7 @@
> > > VariableServiceSetVariable(), VariableServiceQueryVariableInfo(),
> > > ReclaimForOS(),
> > >
> > > SmmVariableGetStatistics() should also do validation based on its
> > > own knowledge.
> > >
> > >
> > >
> > > -Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > > Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > >
> > > SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >
> > >
> > > @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >
> > > #include <Library/MmServicesTableLib.h>
> > >
> > > #include <Library/VariablePolicyLib.h>
> > >
> > > +#include <Library/SmmCpuRendezvousLib.h>
> > >
> > >
> > >
> > > #include <Guid/SmmVariableCommon.h>
> > >
> > > #include "Variable.h"
> > >
> > > @@ -656,6 +657,13 @@ SmmVariableHandler (
> > > goto EXIT;
> > >
> > > }
> > >
> > >
> > >
> > > + if ((SmmVariableHeader->Attributes &
> > > EFI_VARIABLE_NON_VOLATILE) != 0) {
> > >
> > > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> > >
> > > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all
> > > + AP
> > > check in SMM!\n"));
> > >
> > > + goto EXIT;
> > >
> > > + }
> > >
> > > + }
> > >
> > > +
> > >
> > > Status = VariableServiceSetVariable (
> > >
> > > SmmVariableHeader->Name,
> > >
> > > &SmmVariableHeader->Guid,
> > >
> > > diff --git a/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > new file mode 100644
> > > index 000000000000..82e459e9106e
> > > --- /dev/null
> > > +++ b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > @@ -0,0 +1,27 @@
> > > +/** @file
> > >
> > > + SMM CPU Rendezvous library header file.
> > >
> > > +
> > >
> > > + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > >
> > > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > +
> > >
> > > +**/
> > >
> > > +
> > >
> > > +#ifndef SMM_CPU_RENDEZVOUS_H_
> > >
> > > +#define SMM_CPU_RENDEZVOUS_H_
> > >
> > > +
> > >
> > > +/**
> > >
> > > + This routine wait for all AP processors to arrive in SMM.
> > >
> > > +
> > >
> > > + @param[in] BlockingMode Blocking mode or non-blocking mode.
> > >
> > > +
> > >
> > > + @retval EFI_SUCCESS All processors checked in to SMM.
> > >
> > > + @retval EFI_TIMEOUT Wait for all APs until timeout.
> > >
> > > +
> > >
> > > +**/
> > >
> > > +EFI_STATUS
> > >
> > > +EFIAPI
> > >
> > > +SmmWaitForAllProcessor (
> > >
> > > + IN BOOLEAN BlockingMode
> > >
> > > + );
> > >
> > > +
> > >
> > > +#endif
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > > b/MdeModulePkg/MdeModulePkg.dec index
> > 463e889e9a68..06ada41b7344
> > > 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -4,7 +4,7 @@
> > > # and libraries instances, which are used for those modules.
> > >
> > > #
> > >
> > > # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> > >
> > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > > # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> > >
> > > # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development
> > > LP<BR>
> > >
> > > # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> > >
> > > @@ -154,6 +154,9 @@
> > > #
> > >
> > > VariablePolicyHelperLib|Include/Library/VariablePolicyHelperLib.h
> > >
> > >
> > >
> > > + ## @libraryclass Provides function for SMM CPU Rendezvous Library.
> > >
> > > + SmmCpuRendezvousLib|Include/Library/SmmCpuRendezvousLib.h
> > >
> > > +
> > >
> > > [Guids]
> > >
> > > ## MdeModule package token space guid
> > >
> > > # Include/Guid/MdeModulePkgTokenSpace.h
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > > b/MdeModulePkg/MdeModulePkg.dsc index
> > b1d83461865e..a15dd5d7b23d
> > > 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dsc
> > > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > > @@ -2,7 +2,7 @@
> > > # EFI/PI Reference Module Package for All Architectures
> > >
> > > #
> > >
> > > # (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> > >
> > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > > # Copyright (c) Microsoft Corporation.
> > >
> > > #
> > >
> > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -152,6 +152,7 @@
> > >
> > >
> > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTa
> > > bleLib.inf
> > >
> > >
> > >
> > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.i
> > > nf
> > >
> > > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
> > >
> > > +
> > >
> > SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > > uRendezvousLib.inf
> > >
> > >
> > >
> > > [LibraryClasses.common.UEFI_DRIVER]
> > >
> > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> > >
> > > @@ -172,6 +173,7 @@
> > >
> > >
> > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
> > > aloneMmServicesTableLib.inf
> > >
> > >
> > >
> > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalon
> > > eMmLib.inf
> > >
> > >
> > >
> > MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
> > > MemLib.inf
> > >
> > > +
> > >
> > SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > > uRendezvousLib.inf
> > >
> > >
> > >
> > > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > >
> > > ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > >
> > > diff --git
> > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > index eaa97a01c6e5..0bebd92b1626 100644
> > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > @@ -18,7 +18,7 @@
> > > # may not be modified without authorization. If platform fails to
> > > protect these resources,
> > >
> > > # the authentication service provided in this driver will be
> > > broken, and the behavior is undefined.
> > >
> > > #
> > >
> > > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > > # Copyright (c) Microsoft Corporation.
> > >
> > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > #
> > >
> > > @@ -82,6 +82,7 @@
> > > UefiBootServicesTableLib
> > >
> > > VariablePolicyLib
> > >
> > > VariablePolicyHelperLib
> > >
> > > + SmmCpuRendezvousLib
> > >
> > >
> > >
> > > [Protocols]
> > >
> > > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> > >
> > > diff --git
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > f
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > f
> > > index d8c4f77e7f1f..595baaf70164 100644
> > > ---
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > f
> > > +++
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > f
> > > @@ -18,7 +18,7 @@
> > > # may not be modified without authorization. If platform fails to
> > > protect these resources,
> > >
> > > # the authentication service provided in this driver will be
> > > broken, and the behavior is undefined.
> > >
> > > #
> > >
> > > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > > # Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > >
> > > # Copyright (c) Microsoft Corporation.
> > >
> > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -78,6 +78,7 @@
> > > VarCheckLib
> > >
> > > VariablePolicyLib
> > >
> > > VariablePolicyHelperLib
> > >
> > > + SmmCpuRendezvousLib
> > >
> > >
> > >
> > > [Protocols]
> > >
> > > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> > >
> > > --
> > > 2.26.2.windows.1
> > >
> > >
> > >
> > >
> > >
> >
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.
2022-04-15 9:12 ` Li, Zhihao
@ 2022-04-17 3:27 ` gaoliming
2022-04-20 15:35 ` Li, Zhihao
0 siblings, 1 reply; 7+ messages in thread
From: gaoliming @ 2022-04-17 3:27 UTC (permalink / raw)
To: devel, zhihao.li, 'Michael Kubacki',
'Ard Biesheuvel', 'Leif Lindholm',
'Sean Brogan'
Cc: 'Wang, Jian J', 'Fu, Siyuan', 'Ni, Ray',
'Kinney, Michael D', 'Yao, Jiewen'
Zhihao:
I see three platforms in edk2-platforms to consume VariableStandaloneMm module. So, I think this change will impact them. Can you confirm this change with those platform owners?
Platform\ARM\SgiPkg\PlatformStandaloneMm.fdf
Platform\Socionext\DeveloperBox\DeveloperBoxMm.fdf
Platform\StandaloneMm\PlatformStandaloneMmPkg\PlatformStandaloneMmRpmb.fdf
Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li, Zhihao
> 发送时间: 2022年4月15日 17:12
> 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
> 'Michael Kubacki' <mikuback@linux.microsoft.com>; 'Ard Biesheuvel'
> <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>; 'Sean
> Brogan' <sean.brogan@microsoft.com>
> 抄送: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> 主题: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> SmmWaitForAllProcessor() in VariableSmm driver.
>
> I see the configuration in MdeModulePkg\MdeModulePkg.ci.yaml. I add
> SmmCpuRendezvousLib.h in
> MdeModulePkg/Include/Library folder so that it doesn't need add
> UefiCpuPkg.dec in [Packages] and bypass the check.
>
> For the second point, due to the patch pass the CI test, it also pass the
> PlatformCI_ArmVirtPkg. I don't realize that any problem in
> Arm platform build.
> Leif and Ard
> Does the patch has any influence on arm platform build?
>
> Liming:
> If the solution is not acceptable, how about I create NULL version of
> SmmCpuRendezvousLib in MdeModulePkg and use it in MdeModulePkg.dsc?
> Is that a acceptable solution for you?
>
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Wednesday, April 13, 2022 9:30 AM
> > To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; 'Michael
> Kubacki'
> > <mikuback@linux.microsoft.com>; 'Ard Biesheuvel'
> > <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>; 'Sean
> > Brogan' <sean.brogan@microsoft.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> Ni,
> > Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao,
> > Jiewen <jiewen.yao@intel.com>
> > Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > SmmWaitForAllProcessor() in VariableSmm driver.
> >
> > Zhihao:
> > I remember CI has the check for the package dependency. If this patch
> passes
> > CI, seemly this checker doesn't do. You can see DependencyCheck in
> > MdeModulePkg\MdeModulePkg.ci.yaml.
> > And, this patch introduces new dependency in VariableStandaloneMm. It
> has
> > been used in edk2 platform ARM platform. This change will break these
> platform
> > build. Please notify the platform owners.
> >
> > Sean and Michael:
> > This patch adds UefiCpuPkg library instance SmmCpuRendezvousLib into
> > MdeModulePkg.dsc. But, CI can pass. Is this the expected behavior?
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: Li, Zhihao <zhihao.li@intel.com>
> > > 发送时间: 2022年4月13日 2:14
> > > 收件人: Gao, Liming <gaoliming@byosoft.com.cn>;
> devel@edk2.groups.io
> > > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > > <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > 主题: RE: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > > SmmWaitForAllProcessor() in VariableSmm driver.
> > >
> > > 1. Although SmmCpuRendezvousLib in UefiCpuPkg, add
> SmmRendezvousLib.h
> > > into MdeModulePkg/Include/Library folder bypass the check.
> > > 2. The SmmRendezvousLib is a standalone MM library and doesn’t have
> > > any DXE service dependency. It can be used by SMM variable module and
> > > MM variable module.
> > >
> > > As the patch following, it have passed the Edk2 CI test. The code can
> > > run successfully in practice, but I'm not sure if this is acceptable
> > > in terms of the standard.
> > >
> > > -----Original Message-----
> > > From: gaoliming <gaoliming@byosoft.com.cn>
> > > Sent: Tuesday, April 12, 2022 9:00 AM
> > > To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > > <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > > SmmWaitForAllProcessor() in VariableSmm driver.
> > >
> > > Zhihao:
> > > This patch breaks two things. One is to let MdeModulePkg depend on
> > > UefiCpuPkg, another is to let VariableStandaloneMm depend on
> > > UefiCpuPkg SmmCpuRendezvousLib. Please provide your proposal to
> > > resolve these two dependency first.
> > >
> > > Thanks
> > > Liming
> > > > -----邮件原件-----
> > > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li,
> Zhihao
> > > > 发送时间: 2022年4月11日 15:07
> > > > 收件人: devel@edk2.groups.io
> > > > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > > > <gaoliming@byosoft.com.cn>; Siyuan Fu <siyuan.fu@intel.com>; Ni Ray
> > > > <ray.ni@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> > > > 主题: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > > > SmmWaitForAllProcessor() in VariableSmm driver.
> > > >
> > > > REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3854
> > > >
> > > > In UefiCpuPkg, there are a new Protocol with the new service
> > > > SmmWaitForAllProcessor(), which can be used by SMI handler to
> > > > optionally wait for other APs to complete SMM rendezvous in relaxed
> > > > AP mode.
> > > >
> > > > This patch use the new service to let VariableSmm driver work
> > > > normally in relaxed AP mode.
> > > >
> > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > > Cc: Ni Ray <ray.ni@intel.com>
> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > >
> > > > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > > > ---
> > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > | 10 +++++++-
> > > > MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > > | 27 ++++++++++++++++++++
> > > > MdeModulePkg/MdeModulePkg.dec
> > > > | 5 +++-
> > > > MdeModulePkg/MdeModulePkg.dsc
> > > > | 4 ++-
> > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > | 3 ++-
> > > >
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > > > | 3 ++-
> > > > 6 files changed, 47 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git
> > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > index 517cae7b00f8..52a9b0e6b202 100644
> > > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > @@ -14,7 +14,7 @@
> > > > VariableServiceSetVariable(), VariableServiceQueryVariableInfo(),
> > > > ReclaimForOS(),
> > > >
> > > > SmmVariableGetStatistics() should also do validation based on its
> > > > own knowledge.
> > > >
> > > >
> > > >
> > > > -Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > > reserved.<BR>
> > > >
> > > > +Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > > +reserved.<BR>
> > > >
> > > > Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > > >
> > > > SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >
> > > >
> > > > @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >
> > > > #include <Library/MmServicesTableLib.h>
> > > >
> > > > #include <Library/VariablePolicyLib.h>
> > > >
> > > > +#include <Library/SmmCpuRendezvousLib.h>
> > > >
> > > >
> > > >
> > > > #include <Guid/SmmVariableCommon.h>
> > > >
> > > > #include "Variable.h"
> > > >
> > > > @@ -656,6 +657,13 @@ SmmVariableHandler (
> > > > goto EXIT;
> > > >
> > > > }
> > > >
> > > >
> > > >
> > > > + if ((SmmVariableHeader->Attributes &
> > > > EFI_VARIABLE_NON_VOLATILE) != 0) {
> > > >
> > > > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> > > >
> > > > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all
> > > > + AP
> > > > check in SMM!\n"));
> > > >
> > > > + goto EXIT;
> > > >
> > > > + }
> > > >
> > > > + }
> > > >
> > > > +
> > > >
> > > > Status = VariableServiceSetVariable (
> > > >
> > > > SmmVariableHeader->Name,
> > > >
> > > > &SmmVariableHeader->Guid,
> > > >
> > > > diff --git a/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > > b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > > new file mode 100644
> > > > index 000000000000..82e459e9106e
> > > > --- /dev/null
> > > > +++ b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > > @@ -0,0 +1,27 @@
> > > > +/** @file
> > > >
> > > > + SMM CPU Rendezvous library header file.
> > > >
> > > > +
> > > >
> > > > + Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> > > >
> > > > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > > +
> > > >
> > > > +**/
> > > >
> > > > +
> > > >
> > > > +#ifndef SMM_CPU_RENDEZVOUS_H_
> > > >
> > > > +#define SMM_CPU_RENDEZVOUS_H_
> > > >
> > > > +
> > > >
> > > > +/**
> > > >
> > > > + This routine wait for all AP processors to arrive in SMM.
> > > >
> > > > +
> > > >
> > > > + @param[in] BlockingMode Blocking mode or non-blocking
> mode.
> > > >
> > > > +
> > > >
> > > > + @retval EFI_SUCCESS All processors checked in to SMM.
> > > >
> > > > + @retval EFI_TIMEOUT Wait for all APs until timeout.
> > > >
> > > > +
> > > >
> > > > +**/
> > > >
> > > > +EFI_STATUS
> > > >
> > > > +EFIAPI
> > > >
> > > > +SmmWaitForAllProcessor (
> > > >
> > > > + IN BOOLEAN BlockingMode
> > > >
> > > > + );
> > > >
> > > > +
> > > >
> > > > +#endif
> > > >
> > > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > > > b/MdeModulePkg/MdeModulePkg.dec index
> > > 463e889e9a68..06ada41b7344
> > > > 100644
> > > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > > @@ -4,7 +4,7 @@
> > > > # and libraries instances, which are used for those modules.
> > > >
> > > > #
> > > >
> > > > # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> > > >
> > > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > > > reserved.<BR>
> > > >
> > > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > > > +reserved.<BR>
> > > >
> > > > # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> > > >
> > > > # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development
> > > > LP<BR>
> > > >
> > > > # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> > > >
> > > > @@ -154,6 +154,9 @@
> > > > #
> > > >
> > > > VariablePolicyHelperLib|Include/Library/VariablePolicyHelperLib.h
> > > >
> > > >
> > > >
> > > > + ## @libraryclass Provides function for SMM CPU Rendezvous
> Library.
> > > >
> > > > + SmmCpuRendezvousLib|Include/Library/SmmCpuRendezvousLib.h
> > > >
> > > > +
> > > >
> > > > [Guids]
> > > >
> > > > ## MdeModule package token space guid
> > > >
> > > > # Include/Guid/MdeModulePkgTokenSpace.h
> > > >
> > > > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > > > b/MdeModulePkg/MdeModulePkg.dsc index
> > > b1d83461865e..a15dd5d7b23d
> > > > 100644
> > > > --- a/MdeModulePkg/MdeModulePkg.dsc
> > > > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > > > @@ -2,7 +2,7 @@
> > > > # EFI/PI Reference Module Package for All Architectures
> > > >
> > > > #
> > > >
> > > > # (C) Copyright 2014 Hewlett-Packard Development Company,
> L.P.<BR>
> > > >
> > > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > > > reserved.<BR>
> > > >
> > > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > > > +reserved.<BR>
> > > >
> > > > # Copyright (c) Microsoft Corporation.
> > > >
> > > > #
> > > >
> > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > > @@ -152,6 +152,7 @@
> > > >
> > > >
> > >
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTa
> > > > bleLib.inf
> > > >
> > > >
> > > >
> > >
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.i
> > > > nf
> > > >
> > > > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
> > > >
> > > > +
> > > >
> > >
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > > > uRendezvousLib.inf
> > > >
> > > >
> > > >
> > > > [LibraryClasses.common.UEFI_DRIVER]
> > > >
> > > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> > > >
> > > > @@ -172,6 +173,7 @@
> > > >
> > > >
> > >
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
> > > > aloneMmServicesTableLib.inf
> > > >
> > > >
> > > >
> > >
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalon
> > > > eMmLib.inf
> > > >
> > > >
> > > >
> > >
> MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
> > > > MemLib.inf
> > > >
> > > > +
> > > >
> > >
> SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > > > uRendezvousLib.inf
> > > >
> > > >
> > > >
> > > > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > > >
> > > > ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > > >
> > > > diff --git
> > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > index eaa97a01c6e5..0bebd92b1626 100644
> > > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > @@ -18,7 +18,7 @@
> > > > # may not be modified without authorization. If platform fails to
> > > > protect these resources,
> > > >
> > > > # the authentication service provided in this driver will be
> > > > broken, and the behavior is undefined.
> > > >
> > > > #
> > > >
> > > > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > > reserved.<BR>
> > > >
> > > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > > +reserved.<BR>
> > > >
> > > > # Copyright (c) Microsoft Corporation.
> > > >
> > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > > #
> > > >
> > > > @@ -82,6 +82,7 @@
> > > > UefiBootServicesTableLib
> > > >
> > > > VariablePolicyLib
> > > >
> > > > VariablePolicyHelperLib
> > > >
> > > > + SmmCpuRendezvousLib
> > > >
> > > >
> > > >
> > > > [Protocols]
> > > >
> > > > gEfiSmmFirmwareVolumeBlockProtocolGuid ##
> CONSUMES
> > > >
> > > > diff --git
> > > >
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > > f
> > > >
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > > f
> > > > index d8c4f77e7f1f..595baaf70164 100644
> > > > ---
> > > >
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > > f
> > > > +++
> > > >
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > > f
> > > > @@ -18,7 +18,7 @@
> > > > # may not be modified without authorization. If platform fails to
> > > > protect these resources,
> > > >
> > > > # the authentication service provided in this driver will be
> > > > broken, and the behavior is undefined.
> > > >
> > > > #
> > > >
> > > > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > > reserved.<BR>
> > > >
> > > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > > +reserved.<BR>
> > > >
> > > > # Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > > >
> > > > # Copyright (c) Microsoft Corporation.
> > > >
> > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > > @@ -78,6 +78,7 @@
> > > > VarCheckLib
> > > >
> > > > VariablePolicyLib
> > > >
> > > > VariablePolicyHelperLib
> > > >
> > > > + SmmCpuRendezvousLib
> > > >
> > > >
> > > >
> > > > [Protocols]
> > > >
> > > > gEfiSmmFirmwareVolumeBlockProtocolGuid ##
> CONSUMES
> > > >
> > > > --
> > > > 2.26.2.windows.1
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> >
> >
>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver.
2022-04-17 3:27 ` 回复: " gaoliming
@ 2022-04-20 15:35 ` Li, Zhihao
0 siblings, 0 replies; 7+ messages in thread
From: Li, Zhihao @ 2022-04-20 15:35 UTC (permalink / raw)
To: devel@edk2.groups.io, sami.mujawar@arm.com,
ardb+tianocore@kernel.org, quic_llindhol@quicinc.com,
ilias.apalodimas@linaro.org
Cc: Wang, Jian J, Fu, Siyuan, Ni, Ray, Gao, Liming
Hi
As Liming's mail below, PlatformStandaloneMm.fdf, DeveloperBoxMm.fdf and PlatformStandaloneMmRpmb.fdf consume
VariableStandaloneMm module.
I send the patch that made VariableStandaloneMm driver use SmmWaitForAllProcessor function and depend on SmmCpuRendezvousLib. In MdeModulePkg, it will be a NullLib version. The patch_V2 using nulllib version will send later and will cc you.
If any problem in it, please give me some comments.
Thanks a lot
Best Regard
Zhihao
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Sunday, April 17, 2022 11:28 AM
> To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>; 'Michael Kubacki'
> <mikuback@linux.microsoft.com>; 'Ard Biesheuvel'
> <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>; 'Sean
> Brogan' <sean.brogan@microsoft.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>
> Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> SmmWaitForAllProcessor() in VariableSmm driver.
>
> Zhihao:
> I see three platforms in edk2-platforms to consume VariableStandaloneMm
> module. So, I think this change will impact them. Can you confirm this change
> with those platform owners?
>
> Platform\ARM\SgiPkg\PlatformStandaloneMm.fdf
> Platform\Socionext\DeveloperBox\DeveloperBoxMm.fdf
> Platform\StandaloneMm\PlatformStandaloneMmPkg\PlatformStandaloneMmR
> pmb.fdf
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li, Zhihao
> > 发送时间: 2022年4月15日 17:12
> > 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
> > 'Michael Kubacki' <mikuback@linux.microsoft.com>; 'Ard Biesheuvel'
> > <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>;
> > 'Sean Brogan' <sean.brogan@microsoft.com>
> > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> > 主题: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > SmmWaitForAllProcessor() in VariableSmm driver.
> >
> > I see the configuration in MdeModulePkg\MdeModulePkg.ci.yaml. I add
> > SmmCpuRendezvousLib.h in MdeModulePkg/Include/Library folder so that
> > it doesn't need add UefiCpuPkg.dec in [Packages] and bypass the check.
> >
> > For the second point, due to the patch pass the CI test, it also pass
> > the PlatformCI_ArmVirtPkg. I don't realize that any problem in Arm
> > platform build.
> > Leif and Ard
> > Does the patch has any influence on arm platform build?
> >
> > Liming:
> > If the solution is not acceptable, how about I create NULL version of
> > SmmCpuRendezvousLib in MdeModulePkg and use it in MdeModulePkg.dsc?
> > Is that a acceptable solution for you?
> >
> > > -----Original Message-----
> > > From: gaoliming <gaoliming@byosoft.com.cn>
> > > Sent: Wednesday, April 13, 2022 9:30 AM
> > > To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; 'Michael
> > Kubacki'
> > > <mikuback@linux.microsoft.com>; 'Ard Biesheuvel'
> > > <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>;
> > > 'Sean Brogan' <sean.brogan@microsoft.com>
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > > <siyuan.fu@intel.com>;
> > Ni,
> > > Ray <ray.ni@intel.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>;
> > Yao,
> > > Jiewen <jiewen.yao@intel.com>
> > > Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > > SmmWaitForAllProcessor() in VariableSmm driver.
> > >
> > > Zhihao:
> > > I remember CI has the check for the package dependency. If this
> > > patch
> > passes
> > > CI, seemly this checker doesn't do. You can see DependencyCheck in
> > > MdeModulePkg\MdeModulePkg.ci.yaml.
> > > And, this patch introduces new dependency in VariableStandaloneMm.
> > > It
> > has
> > > been used in edk2 platform ARM platform. This change will break
> > > these
> > platform
> > > build. Please notify the platform owners.
> > >
> > > Sean and Michael:
> > > This patch adds UefiCpuPkg library instance SmmCpuRendezvousLib
> > > into MdeModulePkg.dsc. But, CI can pass. Is this the expected behavior?
> > >
> > > Thanks
> > > Liming
> > > > -----邮件原件-----
> > > > 发件人: Li, Zhihao <zhihao.li@intel.com>
> > > > 发送时间: 2022年4月13日 2:14
> > > > 收件人: Gao, Liming <gaoliming@byosoft.com.cn>;
> > devel@edk2.groups.io
> > > > 抄送: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > > > <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael
> > > > D <michael.d.kinney@intel.com>
> > > > 主题: RE: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > > > SmmWaitForAllProcessor() in VariableSmm driver.
> > > >
> > > > 1. Although SmmCpuRendezvousLib in UefiCpuPkg, add
> > SmmRendezvousLib.h
> > > > into MdeModulePkg/Include/Library folder bypass the check.
> > > > 2. The SmmRendezvousLib is a standalone MM library and doesn’t
> > > > have any DXE service dependency. It can be used by SMM variable
> > > > module and MM variable module.
> > > >
> > > > As the patch following, it have passed the Edk2 CI test. The code
> > > > can run successfully in practice, but I'm not sure if this is
> > > > acceptable in terms of the standard.
> > > >
> > > > -----Original Message-----
> > > > From: gaoliming <gaoliming@byosoft.com.cn>
> > > > Sent: Tuesday, April 12, 2022 9:00 AM
> > > > To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Fu, Siyuan
> > > > <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael
> > > > D <michael.d.kinney@intel.com>
> > > > Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > > > SmmWaitForAllProcessor() in VariableSmm driver.
> > > >
> > > > Zhihao:
> > > > This patch breaks two things. One is to let MdeModulePkg depend
> > > > on UefiCpuPkg, another is to let VariableStandaloneMm depend on
> > > > UefiCpuPkg SmmCpuRendezvousLib. Please provide your proposal to
> > > > resolve these two dependency first.
> > > >
> > > > Thanks
> > > > Liming
> > > > > -----邮件原件-----
> > > > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Li,
> > Zhihao
> > > > > 发送时间: 2022年4月11日 15:07
> > > > > 收件人: devel@edk2.groups.io
> > > > > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > > > > <gaoliming@byosoft.com.cn>; Siyuan Fu <siyuan.fu@intel.com>; Ni
> > > > > Ray <ray.ni@intel.com>; Michael D Kinney
> > > > > <michael.d.kinney@intel.com>
> > > > > 主题: [edk2-devel] [PATCH v1 1/1] MdeModulePkg: Use
> > > > > SmmWaitForAllProcessor() in VariableSmm driver.
> > > > >
> > > > > REF： https://bugzilla.tianocore.org/show_bug.cgi?id=3854
> > > > >
> > > > > In UefiCpuPkg, there are a new Protocol with the new service
> > > > > SmmWaitForAllProcessor(), which can be used by SMI handler to
> > > > > optionally wait for other APs to complete SMM rendezvous in
> > > > > relaxed AP mode.
> > > > >
> > > > > This patch use the new service to let VariableSmm driver work
> > > > > normally in relaxed AP mode.
> > > > >
> > > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > > > Cc: Ni Ray <ray.ni@intel.com>
> > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > >
> > > > > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > > > > ---
> > > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > > | 10 +++++++-
> > > > > MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > > > | 27 ++++++++++++++++++++
> > > > > MdeModulePkg/MdeModulePkg.dec
> > > > > | 5 +++-
> > > > > MdeModulePkg/MdeModulePkg.dsc
> > > > > | 4 ++-
> > > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > > | 3 ++-
> > > > >
> > > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > > > > | 3 ++-
> > > > > 6 files changed, 47 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git
> > > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > > index 517cae7b00f8..52a9b0e6b202 100644
> > > > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > > @@ -14,7 +14,7 @@
> > > > > VariableServiceSetVariable(),
> > > > > VariableServiceQueryVariableInfo(),
> > > > > ReclaimForOS(),
> > > > >
> > > > > SmmVariableGetStatistics() should also do validation based on
> > > > > its own knowledge.
> > > > >
> > > > >
> > > > >
> > > > > -Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > > > reserved.<BR>
> > > > >
> > > > > +Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > > > +reserved.<BR>
> > > > >
> > > > > Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > > > >
> > > > > SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > >
> > > > >
> > > > > @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > >
> > > > > #include <Library/MmServicesTableLib.h>
> > > > >
> > > > > #include <Library/VariablePolicyLib.h>
> > > > >
> > > > > +#include <Library/SmmCpuRendezvousLib.h>
> > > > >
> > > > >
> > > > >
> > > > > #include <Guid/SmmVariableCommon.h>
> > > > >
> > > > > #include "Variable.h"
> > > > >
> > > > > @@ -656,6 +657,13 @@ SmmVariableHandler (
> > > > > goto EXIT;
> > > > >
> > > > > }
> > > > >
> > > > >
> > > > >
> > > > > + if ((SmmVariableHeader->Attributes &
> > > > > EFI_VARIABLE_NON_VOLATILE) != 0) {
> > > > >
> > > > > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> > > > >
> > > > > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for
> > > > > + all AP
> > > > > check in SMM!\n"));
> > > > >
> > > > > + goto EXIT;
> > > > >
> > > > > + }
> > > > >
> > > > > + }
> > > > >
> > > > > +
> > > > >
> > > > > Status = VariableServiceSetVariable (
> > > > >
> > > > > SmmVariableHeader->Name,
> > > > >
> > > > > &SmmVariableHeader->Guid,
> > > > >
> > > > > diff --git a/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > > > b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > > > new file mode 100644
> > > > > index 000000000000..82e459e9106e
> > > > > --- /dev/null
> > > > > +++ b/MdeModulePkg/Include/Library/SmmCpuRendezvousLib.h
> > > > > @@ -0,0 +1,27 @@
> > > > > +/** @file
> > > > >
> > > > > + SMM CPU Rendezvous library header file.
> > > > >
> > > > > +
> > > > >
> > > > > + Copyright (c) 2022, Intel Corporation. All rights
> > > > > + reserved.<BR>
> > > > >
> > > > > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > > +
> > > > >
> > > > > +**/
> > > > >
> > > > > +
> > > > >
> > > > > +#ifndef SMM_CPU_RENDEZVOUS_H_
> > > > >
> > > > > +#define SMM_CPU_RENDEZVOUS_H_
> > > > >
> > > > > +
> > > > >
> > > > > +/**
> > > > >
> > > > > + This routine wait for all AP processors to arrive in SMM.
> > > > >
> > > > > +
> > > > >
> > > > > + @param[in] BlockingMode Blocking mode or non-blocking
> > mode.
> > > > >
> > > > > +
> > > > >
> > > > > + @retval EFI_SUCCESS All processors checked in to SMM.
> > > > >
> > > > > + @retval EFI_TIMEOUT Wait for all APs until timeout.
> > > > >
> > > > > +
> > > > >
> > > > > +**/
> > > > >
> > > > > +EFI_STATUS
> > > > >
> > > > > +EFIAPI
> > > > >
> > > > > +SmmWaitForAllProcessor (
> > > > >
> > > > > + IN BOOLEAN BlockingMode
> > > > >
> > > > > + );
> > > > >
> > > > > +
> > > > >
> > > > > +#endif
> > > > >
> > > > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > > > > b/MdeModulePkg/MdeModulePkg.dec index
> > > > 463e889e9a68..06ada41b7344
> > > > > 100644
> > > > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > > > @@ -4,7 +4,7 @@
> > > > > # and libraries instances, which are used for those modules.
> > > > >
> > > > > #
> > > > >
> > > > > # Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> > > > >
> > > > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > > > > reserved.<BR>
> > > > >
> > > > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > > > > +reserved.<BR>
> > > > >
> > > > > # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
> > > > >
> > > > > # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise
> > > > > Development LP<BR>
> > > > >
> > > > > # Copyright (c) 2017, AMD Incorporated. All rights
> > > > > reserved.<BR>
> > > > >
> > > > > @@ -154,6 +154,9 @@
> > > > > #
> > > > >
> > > > >
> > > > > VariablePolicyHelperLib|Include/Library/VariablePolicyHelperLib.
> > > > > h
> > > > >
> > > > >
> > > > >
> > > > > + ## @libraryclass Provides function for SMM CPU Rendezvous
> > Library.
> > > > >
> > > > > + SmmCpuRendezvousLib|Include/Library/SmmCpuRendezvousLib.h
> > > > >
> > > > > +
> > > > >
> > > > > [Guids]
> > > > >
> > > > > ## MdeModule package token space guid
> > > > >
> > > > > # Include/Guid/MdeModulePkgTokenSpace.h
> > > > >
> > > > > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > > > > b/MdeModulePkg/MdeModulePkg.dsc index
> > > > b1d83461865e..a15dd5d7b23d
> > > > > 100644
> > > > > --- a/MdeModulePkg/MdeModulePkg.dsc
> > > > > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > > > > @@ -2,7 +2,7 @@
> > > > > # EFI/PI Reference Module Package for All Architectures
> > > > >
> > > > > #
> > > > >
> > > > > # (C) Copyright 2014 Hewlett-Packard Development Company,
> > L.P.<BR>
> > > > >
> > > > > -# Copyright (c) 2007 - 2021, Intel Corporation. All rights
> > > > > reserved.<BR>
> > > > >
> > > > > +# Copyright (c) 2007 - 2022, Intel Corporation. All rights
> > > > > +reserved.<BR>
> > > > >
> > > > > # Copyright (c) Microsoft Corporation.
> > > > >
> > > > > #
> > > > >
> > > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > > @@ -152,6 +152,7 @@
> > > > >
> > > > >
> > > >
> > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTa
> > > > > bleLib.inf
> > > > >
> > > > >
> > > > >
> > > >
> > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.i
> > > > > nf
> > > > >
> > > > > SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
> > > > >
> > > > > +
> > > > >
> > > >
> > SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > > > > uRendezvousLib.inf
> > > > >
> > > > >
> > > > >
> > > > > [LibraryClasses.common.UEFI_DRIVER]
> > > > >
> > > > > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> > > > >
> > > > > @@ -172,6 +173,7 @@
> > > > >
> > > > >
> > > >
> > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stand
> > > > > aloneMmServicesTableLib.inf
> > > > >
> > > > >
> > > > >
> > > >
> > LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxStandalon
> > > > > eMmLib.inf
> > > > >
> > > > >
> > > > >
> > > >
> > MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMm
> > > > > MemLib.inf
> > > > >
> > > > > +
> > > > >
> > > >
> > SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCp
> > > > > uRendezvousLib.inf
> > > > >
> > > > >
> > > > >
> > > > > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > > > >
> > > > > ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> > > > >
> > > > > diff --git
> > > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > > index eaa97a01c6e5..0bebd92b1626 100644
> > > > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > > @@ -18,7 +18,7 @@
> > > > > # may not be modified without authorization. If platform fails
> > > > > to protect these resources,
> > > > >
> > > > > # the authentication service provided in this driver will be
> > > > > broken, and the behavior is undefined.
> > > > >
> > > > > #
> > > > >
> > > > > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > > > reserved.<BR>
> > > > >
> > > > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > > > +reserved.<BR>
> > > > >
> > > > > # Copyright (c) Microsoft Corporation.
> > > > >
> > > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > > #
> > > > >
> > > > > @@ -82,6 +82,7 @@
> > > > > UefiBootServicesTableLib
> > > > >
> > > > > VariablePolicyLib
> > > > >
> > > > > VariablePolicyHelperLib
> > > > >
> > > > > + SmmCpuRendezvousLib
> > > > >
> > > > >
> > > > >
> > > > > [Protocols]
> > > > >
> > > > > gEfiSmmFirmwareVolumeBlockProtocolGuid ##
> > CONSUMES
> > > > >
> > > > > diff --git
> > > > >
> > > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > > > f
> > > > >
> > > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > > > f
> > > > > index d8c4f77e7f1f..595baaf70164 100644
> > > > > ---
> > > > >
> > > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > > > f
> > > > > +++
> > > > >
> > > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > > > > f
> > > > > @@ -18,7 +18,7 @@
> > > > > # may not be modified without authorization. If platform fails
> > > > > to protect these resources,
> > > > >
> > > > > # the authentication service provided in this driver will be
> > > > > broken, and the behavior is undefined.
> > > > >
> > > > > #
> > > > >
> > > > > -# Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > > > > reserved.<BR>
> > > > >
> > > > > +# Copyright (c) 2010 - 2022, Intel Corporation. All rights
> > > > > +reserved.<BR>
> > > > >
> > > > > # Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> > > > >
> > > > > # Copyright (c) Microsoft Corporation.
> > > > >
> > > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > > @@ -78,6 +78,7 @@
> > > > > VarCheckLib
> > > > >
> > > > > VariablePolicyLib
> > > > >
> > > > > VariablePolicyHelperLib
> > > > >
> > > > > + SmmCpuRendezvousLib
> > > > >
> > > > >
> > > > >
> > > > > [Protocols]
> > > > >
> > > > > gEfiSmmFirmwareVolumeBlockProtocolGuid ##
> > CONSUMES
> > > > >
> > > > > --
> > > > > 2.26.2.windows.1
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> > >
> >
> >
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-04-20 15:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-11 7:06 [PATCH v1 1/1] MdeModulePkg: Use SmmWaitForAllProcessor() in VariableSmm driver Li, Zhihao
2022-04-12 0:59 ` 回复: [edk2-devel] " gaoliming
2022-04-12 18:13 ` Li, Zhihao
2022-04-13 1:30 ` 回复: " gaoliming
2022-04-15 9:12 ` Li, Zhihao
2022-04-17 3:27 ` 回复: " gaoliming
2022-04-20 15:35 ` Li, Zhihao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox