* [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.
@ 2023-05-10 10:56 Li, Zhihao
2023-05-18 9:37 ` 回复: " gaoliming
0 siblings, 1 reply; 8+ messages in thread
From: Li, Zhihao @ 2023-05-10 10:56 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Liming Gao
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4429
For Ap-Relaxed sync mode, SmmVariableSetVariable() need to let all Aps
arrive to smm before it set the variable. If not, it would return EFI_ACCESS_DENIED.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Zhihao Li <zhihao.li@intel.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 10 +++++++++-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 3 ++-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf | 3 ++-
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 5253c328dcd9..4944903e64d4 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 - 2023, 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"
@@ -87,6 +88,13 @@ SmmVariableSetVariable (
{
EFI_STATUS Status;
+ //
+ // Need to wait for all Aps to arrive in Relaxed-AP Sync Mode
+ //
+ if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
+ DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check in SMM!\n"));
+ }
+
//
// Disable write protection when the calling SetVariable() through EFI_SMM_VARIABLE_PROTOCOL.
//
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
index 8c552b87e080..1cf0d051e6c9 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 - 2023, Intel Corporation. All rights reserved.<BR>
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -84,6 +84,7 @@
VariablePolicyLib
VariablePolicyHelperLib
SafeIntLib
+ SmmCpuRendezvousLib
[Protocols]
gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
index f09bed40cf51..89187456ca25 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 - 2023, 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
@@ -80,6 +80,7 @@
VariableFlashInfoLib
VariablePolicyLib
VariablePolicyHelperLib
+ SmmCpuRendezvousLib
[Protocols]
gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
--
2.26.2.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* 回复: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.
2023-05-10 10:56 [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable Li, Zhihao
@ 2023-05-18 9:37 ` gaoliming
2023-05-19 8:11 ` Li, Zhihao
0 siblings, 1 reply; 8+ messages in thread
From: gaoliming @ 2023-05-18 9:37 UTC (permalink / raw)
To: 'Zhihao Li', devel, 'Ni, Ray', kraxel
Cc: 'Jian J Wang'
Zhihao:
Have you root cause this issue that SmmVariableSetVariable may return
EFI_ACCESS_DENIED?
I am not sure whether this fix is proper. I also add UefiCpuPkg
maintainers Ray and Gerd in the mail loop for this discussion.
Thanks
Liming
> -----邮件原件-----
> 发件人: Zhihao Li <zhihao.li@intel.com>
> 发送时间: 2023年5月10日 18:57
> 收件人: devel@edk2.groups.io
> 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>
> 主题: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous
> check before SmmSetVariable.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4429
>
> For Ap-Relaxed sync mode, SmmVariableSetVariable() need to let all Aps
> arrive to smm before it set the variable. If not, it would return
> EFI_ACCESS_DENIED.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>
> Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> ---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> | 10 +++++++++-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> | 3 ++-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> | 3 ++-
> 3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index 5253c328dcd9..4944903e64d4 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 - 2023, 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"
>
> @@ -87,6 +88,13 @@ SmmVariableSetVariable (
> {
>
> EFI_STATUS Status;
>
>
>
> + //
>
> + // Need to wait for all Aps to arrive in Relaxed-AP Sync Mode
>
> + //
>
> + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
>
> + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check in
> SMM!\n"));
>
> + }
>
> +
>
> //
>
> // Disable write protection when the calling SetVariable() through
> EFI_SMM_VARIABLE_PROTOCOL.
>
> //
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index 8c552b87e080..1cf0d051e6c9 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 - 2023, Intel Corporation. All rights reserved.<BR>
>
> # Copyright (c) Microsoft Corporation.
>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #
>
> @@ -84,6 +84,7 @@
> VariablePolicyLib
>
> VariablePolicyHelperLib
>
> SafeIntLib
>
> + SmmCpuRendezvousLib
>
>
>
> [Protocols]
>
> gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
>
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> index f09bed40cf51..89187456ca25 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 - 2023, 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
>
> @@ -80,6 +80,7 @@
> VariableFlashInfoLib
>
> VariablePolicyLib
>
> VariablePolicyHelperLib
>
> + SmmCpuRendezvousLib
>
>
>
> [Protocols]
>
> gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
>
> --
> 2.26.2.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.
2023-05-18 9:37 ` 回复: " gaoliming
@ 2023-05-19 8:11 ` Li, Zhihao
2023-05-19 9:00 ` 回复: " gaoliming
2023-06-01 1:02 ` [edk2-devel] " Wu, Jiaxin
0 siblings, 2 replies; 8+ messages in thread
From: Li, Zhihao @ 2023-05-19 8:11 UTC (permalink / raw)
To: Gao, Liming, devel@edk2.groups.io, Ni, Ray, kraxel@redhat.com
Cc: Wang, Jian J
Hi Liming
In Ap-Relaxed mode, Bsp will not wait for all ap arrive and call the SMI handlers. But some SMI handlers need all Aps arrive in smm mode such as SmmSetVariable. As the design, SetVariable need to let all aps arrive because it will write flash. Half year ago, I send the patch that calling SmmCpuRendezvous() before SmmSetVariable. It was reviewed but hasn't merged. SmmCpuRendezvous() will return immediately in traditional-AP mode.
I'm not sure what returns EFI_ACCESS_DENIED. Calling SmmCpuRendezvous() before SmmSetVariable is our original design but haven't implemented.
-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn>
Sent: Thursday, May 18, 2023 5:38 PM
To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; kraxel@redhat.com
Cc: Wang, Jian J <jian.j.wang@intel.com>
Subject: 回复: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.
Zhihao:
Have you root cause this issue that SmmVariableSetVariable may return EFI_ACCESS_DENIED?
I am not sure whether this fix is proper. I also add UefiCpuPkg maintainers Ray and Gerd in the mail loop for this discussion.
Thanks
Liming
> -----邮件原件-----
> 发件人: Zhihao Li <zhihao.li@intel.com>
> 发送时间: 2023年5月10日 18:57
> 收件人: devel@edk2.groups.io
> 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>
> 主题: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check
> before SmmSetVariable.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4429
>
> For Ap-Relaxed sync mode, SmmVariableSetVariable() need to let all Aps
> arrive to smm before it set the variable. If not, it would return
> EFI_ACCESS_DENIED.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>
> Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> ---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> | 10 +++++++++-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> | 3 ++-
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> | 3 ++-
> 3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index 5253c328dcd9..4944903e64d4 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 - 2023, 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"
>
> @@ -87,6 +88,13 @@ SmmVariableSetVariable ( {
>
> EFI_STATUS Status;
>
>
>
> + //
>
> + // Need to wait for all Aps to arrive in Relaxed-AP Sync Mode
>
> + //
>
> + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
>
> + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check
> + in
> SMM!\n"));
>
> + }
>
> +
>
> //
>
> // Disable write protection when the calling SetVariable() through
> EFI_SMM_VARIABLE_PROTOCOL.
>
> //
>
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index 8c552b87e080..1cf0d051e6c9 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 - 2023, Intel Corporation. All rights
> +reserved.<BR>
>
> # Copyright (c) Microsoft Corporation.
>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #
>
> @@ -84,6 +84,7 @@
> VariablePolicyLib
>
> VariablePolicyHelperLib
>
> SafeIntLib
>
> + SmmCpuRendezvousLib
>
>
>
> [Protocols]
>
> gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
>
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> f
> index f09bed40cf51..89187456ca25 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 - 2023, 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
>
> @@ -80,6 +80,7 @@
> VariableFlashInfoLib
>
> VariablePolicyLib
>
> VariablePolicyHelperLib
>
> + SmmCpuRendezvousLib
>
>
>
> [Protocols]
>
> gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
>
> --
> 2.26.2.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* 回复: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.
2023-05-19 8:11 ` Li, Zhihao
@ 2023-05-19 9:00 ` gaoliming
2023-06-01 1:02 ` [edk2-devel] " Wu, Jiaxin
1 sibling, 0 replies; 8+ messages in thread
From: gaoliming @ 2023-05-19 9:00 UTC (permalink / raw)
To: 'Li, Zhihao', devel, 'Ni, Ray', kraxel
Cc: 'Wang, Jian J'
Ray and Gerd:
Can you give the comments for this change?
Thanks
Liming
> -----邮件原件-----
> 发件人: Li, Zhihao <zhihao.li@intel.com>
> 发送时间: 2023年5月19日 16:11
> 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; Ni,
> Ray <ray.ni@intel.com>; kraxel@redhat.com
> 抄送: Wang, Jian J <jian.j.wang@intel.com>
> 主题: RE: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous
> check before SmmSetVariable.
>
> Hi Liming
> In Ap-Relaxed mode, Bsp will not wait for all ap arrive and call the SMI
> handlers. But some SMI handlers need all Aps arrive in smm mode such as
> SmmSetVariable. As the design, SetVariable need to let all aps arrive because
> it will write flash. Half year ago, I send the patch that calling
> SmmCpuRendezvous() before SmmSetVariable. It was reviewed but hasn't
> merged. SmmCpuRendezvous() will return immediately in traditional-AP
> mode.
> I'm not sure what returns EFI_ACCESS_DENIED. Calling SmmCpuRendezvous()
> before SmmSetVariable is our original design but haven't implemented.
>
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Thursday, May 18, 2023 5:38 PM
> To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; Ni, Ray
> <ray.ni@intel.com>; kraxel@redhat.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>
> Subject: 回复: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap
> rendezvous check before SmmSetVariable.
>
> Zhihao:
> Have you root cause this issue that SmmVariableSetVariable may return
> EFI_ACCESS_DENIED?
>
> I am not sure whether this fix is proper. I also add UefiCpuPkg maintainers
> Ray and Gerd in the mail loop for this discussion.
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Zhihao Li <zhihao.li@intel.com>
> > 发送时间: 2023年5月10日 18:57
> > 收件人: devel@edk2.groups.io
> > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>
> > 主题: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous
> check
> > before SmmSetVariable.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4429
> >
> > For Ap-Relaxed sync mode, SmmVariableSetVariable() need to let all Aps
> > arrive to smm before it set the variable. If not, it would return
> > EFI_ACCESS_DENIED.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >
> > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > ---
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > | 10 +++++++++-
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > | 3 ++-
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > | 3 ++-
> > 3 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > index 5253c328dcd9..4944903e64d4 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 - 2023, 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"
> >
> > @@ -87,6 +88,13 @@ SmmVariableSetVariable ( {
> >
> > EFI_STATUS Status;
> >
> >
> >
> > + //
> >
> > + // Need to wait for all Aps to arrive in Relaxed-AP Sync Mode
> >
> > + //
> >
> > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> >
> > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check
> > + in
> > SMM!\n"));
> >
> > + }
> >
> > +
> >
> > //
> >
> > // Disable write protection when the calling SetVariable() through
> > EFI_SMM_VARIABLE_PROTOCOL.
> >
> > //
> >
> > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > index 8c552b87e080..1cf0d051e6c9 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 - 2023, Intel Corporation. All rights
> > +reserved.<BR>
> >
> > # Copyright (c) Microsoft Corporation.
> >
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > #
> >
> > @@ -84,6 +84,7 @@
> > VariablePolicyLib
> >
> > VariablePolicyHelperLib
> >
> > SafeIntLib
> >
> > + SmmCpuRendezvousLib
> >
> >
> >
> > [Protocols]
> >
> > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > f
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.in
> > f
> > index f09bed40cf51..89187456ca25 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 - 2023, 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
> >
> > @@ -80,6 +80,7 @@
> > VariableFlashInfoLib
> >
> > VariablePolicyLib
> >
> > VariablePolicyHelperLib
> >
> > + SmmCpuRendezvousLib
> >
> >
> >
> > [Protocols]
> >
> > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> >
> > --
> > 2.26.2.windows.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.
2023-05-19 8:11 ` Li, Zhihao
2023-05-19 9:00 ` 回复: " gaoliming
@ 2023-06-01 1:02 ` Wu, Jiaxin
2023-06-01 1:06 ` Yao, Jiewen
1 sibling, 1 reply; 8+ messages in thread
From: Wu, Jiaxin @ 2023-06-01 1:02 UTC (permalink / raw)
To: devel@edk2.groups.io, Li, Zhihao, Gao, Liming, Ni, Ray,
kraxel@redhat.com
Cc: Wang, Jian J
Hi All,
I think we need this patch:
There is a requirement that all CPU threads must in SMM for Non-Volatile variable. Because the SMM will disables the flash protection. Before that, we must guarantee all CPU threads are in SMM to avoid the non-smm mode cpus modify the flash.
Zhihao,
I think this is only needed for the Non-Volatile, I suggest as below check:
if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
DEBUG ((DEBUG_ERROR, " SmmVariableSetVariable: Fail to wait for all AP check in SMM!\n"));
Status = EFI_ABORTED;
goto EXIT;
}
}
Thanks,
Jiaxin
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Li,
> Zhihao
> Sent: Friday, May 19, 2023 4:11 PM
> To: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; Ni,
> Ray <ray.ni@intel.com>; kraxel@redhat.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c:
> add Ap rendezvous check before SmmSetVariable.
>
> Hi Liming
> In Ap-Relaxed mode, Bsp will not wait for all ap arrive and call the SMI
> handlers. But some SMI handlers need all Aps arrive in smm mode such as
> SmmSetVariable. As the design, SetVariable need to let all aps arrive because
> it will write flash. Half year ago, I send the patch that calling
> SmmCpuRendezvous() before SmmSetVariable. It was reviewed but hasn't
> merged. SmmCpuRendezvous() will return immediately in traditional-AP
> mode.
> I'm not sure what returns EFI_ACCESS_DENIED. Calling SmmCpuRendezvous()
> before SmmSetVariable is our original design but haven't implemented.
>
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Thursday, May 18, 2023 5:38 PM
> To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; Ni, Ray
> <ray.ni@intel.com>; kraxel@redhat.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>
> Subject: 回复: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap
> rendezvous check before SmmSetVariable.
>
> Zhihao:
> Have you root cause this issue that SmmVariableSetVariable may return
> EFI_ACCESS_DENIED?
>
> I am not sure whether this fix is proper. I also add UefiCpuPkg maintainers
> Ray and Gerd in the mail loop for this discussion.
>
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Zhihao Li <zhihao.li@intel.com>
> > 发送时间: 2023年5月10日 18:57
> > 收件人: devel@edk2.groups.io
> > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>
> > 主题: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous
> check
> > before SmmSetVariable.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4429
> >
> > For Ap-Relaxed sync mode, SmmVariableSetVariable() need to let all Aps
> > arrive to smm before it set the variable. If not, it would return
> > EFI_ACCESS_DENIED.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >
> > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > ---
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > | 10 +++++++++-
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > | 3 ++-
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > | 3 ++-
> > 3 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > index 5253c328dcd9..4944903e64d4 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 - 2023, 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"
> >
> > @@ -87,6 +88,13 @@ SmmVariableSetVariable ( {
> >
> > EFI_STATUS Status;
> >
> >
> >
> > + //
> >
> > + // Need to wait for all Aps to arrive in Relaxed-AP Sync Mode
> >
> > + //
> >
> > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> >
> > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check
> > + in
> > SMM!\n"));
> >
> > + }
> >
> > +
> >
> > //
> >
> > // Disable write protection when the calling SetVariable() through
> > EFI_SMM_VARIABLE_PROTOCOL.
> >
> > //
> >
> > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > index 8c552b87e080..1cf0d051e6c9 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 - 2023, Intel Corporation. All rights
> > +reserved.<BR>
> >
> > # Copyright (c) Microsoft Corporation.
> >
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > #
> >
> > @@ -84,6 +84,7 @@
> > VariablePolicyLib
> >
> > VariablePolicyHelperLib
> >
> > SafeIntLib
> >
> > + SmmCpuRendezvousLib
> >
> >
> >
> > [Protocols]
> >
> > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> n
> > f
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> in
> > f
> > index f09bed40cf51..89187456ca25 100644
> > ---
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> n
> > 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 - 2023, 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
> >
> > @@ -80,6 +80,7 @@
> > VariableFlashInfoLib
> >
> > VariablePolicyLib
> >
> > VariablePolicyHelperLib
> >
> > + SmmCpuRendezvousLib
> >
> >
> >
> > [Protocols]
> >
> > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> >
> > --
> > 2.26.2.windows.1
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.
2023-06-01 1:02 ` [edk2-devel] " Wu, Jiaxin
@ 2023-06-01 1:06 ` Yao, Jiewen
2023-06-01 1:07 ` Ni, Ray
0 siblings, 1 reply; 8+ messages in thread
From: Yao, Jiewen @ 2023-06-01 1:06 UTC (permalink / raw)
To: devel@edk2.groups.io, Wu, Jiaxin, Li, Zhihao, Gao, Liming,
Ni, Ray, kraxel@redhat.com
Cc: Wang, Jian J
Disabling flash is a silicon behavior.
Can we do SmmWaitForAllProcessor() in FVB driver, or SPI driver ?
Thank you
Yao, Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Jiaxin
> Sent: Thursday, June 1, 2023 9:03 AM
> To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; kraxel@redhat.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add
> Ap rendezvous check before SmmSetVariable.
>
> Hi All,
>
> I think we need this patch:
>
> There is a requirement that all CPU threads must in SMM for Non-Volatile
> variable. Because the SMM will disables the flash protection. Before that, we
> must guarantee all CPU threads are in SMM to avoid the non-smm mode cpus
> modify the flash.
>
>
> Zhihao,
>
> I think this is only needed for the Non-Volatile, I suggest as below check:
>
> if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
> if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> DEBUG ((DEBUG_ERROR, " SmmVariableSetVariable: Fail to wait for all AP
> check in SMM!\n"));
> Status = EFI_ABORTED;
> goto EXIT;
> }
> }
>
> Thanks,
> Jiaxin
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Li,
> > Zhihao
> > Sent: Friday, May 19, 2023 4:11 PM
> > To: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; Ni,
> > Ray <ray.ni@intel.com>; kraxel@redhat.com
> > Cc: Wang, Jian J <jian.j.wang@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c:
> > add Ap rendezvous check before SmmSetVariable.
> >
> > Hi Liming
> > In Ap-Relaxed mode, Bsp will not wait for all ap arrive and call the SMI
> > handlers. But some SMI handlers need all Aps arrive in smm mode such as
> > SmmSetVariable. As the design, SetVariable need to let all aps arrive because
> > it will write flash. Half year ago, I send the patch that calling
> > SmmCpuRendezvous() before SmmSetVariable. It was reviewed but hasn't
> > merged. SmmCpuRendezvous() will return immediately in traditional-AP
> > mode.
> > I'm not sure what returns EFI_ACCESS_DENIED. Calling SmmCpuRendezvous()
> > before SmmSetVariable is our original design but haven't implemented.
> >
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Thursday, May 18, 2023 5:38 PM
> > To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; Ni, Ray
> > <ray.ni@intel.com>; kraxel@redhat.com
> > Cc: Wang, Jian J <jian.j.wang@intel.com>
> > Subject: 回复: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap
> > rendezvous check before SmmSetVariable.
> >
> > Zhihao:
> > Have you root cause this issue that SmmVariableSetVariable may return
> > EFI_ACCESS_DENIED?
> >
> > I am not sure whether this fix is proper. I also add UefiCpuPkg maintainers
> > Ray and Gerd in the mail loop for this discussion.
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: Zhihao Li <zhihao.li@intel.com>
> > > 发送时间: 2023年5月10日 18:57
> > > 收件人: devel@edk2.groups.io
> > > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > > <gaoliming@byosoft.com.cn>
> > > 主题: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous
> > check
> > > before SmmSetVariable.
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4429
> > >
> > > For Ap-Relaxed sync mode, SmmVariableSetVariable() need to let all Aps
> > > arrive to smm before it set the variable. If not, it would return
> > > EFI_ACCESS_DENIED.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > >
> > > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > > ---
> > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > | 10 +++++++++-
> > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > | 3 ++-
> > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > > | 3 ++-
> > > 3 files changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > index 5253c328dcd9..4944903e64d4 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 - 2023, 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"
> > >
> > > @@ -87,6 +88,13 @@ SmmVariableSetVariable ( {
> > >
> > > EFI_STATUS Status;
> > >
> > >
> > >
> > > + //
> > >
> > > + // Need to wait for all Aps to arrive in Relaxed-AP Sync Mode
> > >
> > > + //
> > >
> > > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> > >
> > > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check
> > > + in
> > > SMM!\n"));
> > >
> > > + }
> > >
> > > +
> > >
> > > //
> > >
> > > // Disable write protection when the calling SetVariable() through
> > > EFI_SMM_VARIABLE_PROTOCOL.
> > >
> > > //
> > >
> > > diff --git
> > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > index 8c552b87e080..1cf0d051e6c9 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 - 2023, Intel Corporation. All rights
> > > +reserved.<BR>
> > >
> > > # Copyright (c) Microsoft Corporation.
> > >
> > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > #
> > >
> > > @@ -84,6 +84,7 @@
> > > VariablePolicyLib
> > >
> > > VariablePolicyHelperLib
> > >
> > > SafeIntLib
> > >
> > > + SmmCpuRendezvousLib
> > >
> > >
> > >
> > > [Protocols]
> > >
> > > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> > >
> > > diff --git
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> > n
> > > f
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> > in
> > > f
> > > index f09bed40cf51..89187456ca25 100644
> > > ---
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> > n
> > > 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 - 2023, 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
> > >
> > > @@ -80,6 +80,7 @@
> > > VariableFlashInfoLib
> > >
> > > VariablePolicyLib
> > >
> > > VariablePolicyHelperLib
> > >
> > > + SmmCpuRendezvousLib
> > >
> > >
> > >
> > > [Protocols]
> > >
> > > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> > >
> > > --
> > > 2.26.2.windows.1
> >
> >
> >
> >
> >
> >
> >
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.
2023-06-01 1:06 ` Yao, Jiewen
@ 2023-06-01 1:07 ` Ni, Ray
2023-06-01 1:09 ` Wu, Jiaxin
0 siblings, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2023-06-01 1:07 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io, Wu, Jiaxin, Li, Zhihao,
Gao, Liming, kraxel@redhat.com
Cc: Wang, Jian J
Jiewen,
Good suggestion😊.
This also resolve's Jiaxin's comments that check if the variable is non-volatile.
Thanks,
Ray
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Thursday, June 1, 2023 9:06 AM
> To: devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>; Li, Zhihao
> <zhihao.li@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray
> <ray.ni@intel.com>; kraxel@redhat.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>
> Subject: RE: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c:
> add Ap rendezvous check before SmmSetVariable.
>
> Disabling flash is a silicon behavior.
>
> Can we do SmmWaitForAllProcessor() in FVB driver, or SPI driver ?
>
> Thank you
> Yao, Jiewen
>
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
> Jiaxin
> > Sent: Thursday, June 1, 2023 9:03 AM
> > To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>;
> kraxel@redhat.com
> > Cc: Wang, Jian J <jian.j.wang@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c:
> add
> > Ap rendezvous check before SmmSetVariable.
> >
> > Hi All,
> >
> > I think we need this patch:
> >
> > There is a requirement that all CPU threads must in SMM for Non-Volatile
> > variable. Because the SMM will disables the flash protection. Before that, we
> > must guarantee all CPU threads are in SMM to avoid the non-smm mode
> cpus
> > modify the flash.
> >
> >
> > Zhihao,
> >
> > I think this is only needed for the Non-Volatile, I suggest as below check:
> >
> > if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
> > if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> > DEBUG ((DEBUG_ERROR, " SmmVariableSetVariable: Fail to wait for all
> AP
> > check in SMM!\n"));
> > Status = EFI_ABORTED;
> > goto EXIT;
> > }
> > }
> >
> > Thanks,
> > Jiaxin
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Li,
> > > Zhihao
> > > Sent: Friday, May 19, 2023 4:11 PM
> > > To: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; Ni,
> > > Ray <ray.ni@intel.com>; kraxel@redhat.com
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c:
> > > add Ap rendezvous check before SmmSetVariable.
> > >
> > > Hi Liming
> > > In Ap-Relaxed mode, Bsp will not wait for all ap arrive and call the SMI
> > > handlers. But some SMI handlers need all Aps arrive in smm mode such as
> > > SmmSetVariable. As the design, SetVariable need to let all aps arrive
> because
> > > it will write flash. Half year ago, I send the patch that calling
> > > SmmCpuRendezvous() before SmmSetVariable. It was reviewed but hasn't
> > > merged. SmmCpuRendezvous() will return immediately in traditional-AP
> > > mode.
> > > I'm not sure what returns EFI_ACCESS_DENIED. Calling
> SmmCpuRendezvous()
> > > before SmmSetVariable is our original design but haven't implemented.
> > >
> > > -----Original Message-----
> > > From: gaoliming <gaoliming@byosoft.com.cn>
> > > Sent: Thursday, May 18, 2023 5:38 PM
> > > To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; Ni, Ray
> > > <ray.ni@intel.com>; kraxel@redhat.com
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>
> > > Subject: 回复: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap
> > > rendezvous check before SmmSetVariable.
> > >
> > > Zhihao:
> > > Have you root cause this issue that SmmVariableSetVariable may return
> > > EFI_ACCESS_DENIED?
> > >
> > > I am not sure whether this fix is proper. I also add UefiCpuPkg maintainers
> > > Ray and Gerd in the mail loop for this discussion.
> > >
> > > Thanks
> > > Liming
> > > > -----邮件原件-----
> > > > 发件人: Zhihao Li <zhihao.li@intel.com>
> > > > 发送时间: 2023年5月10日 18:57
> > > > 收件人: devel@edk2.groups.io
> > > > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > > > <gaoliming@byosoft.com.cn>
> > > > 主题: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap
> rendezvous
> > > check
> > > > before SmmSetVariable.
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4429
> > > >
> > > > For Ap-Relaxed sync mode, SmmVariableSetVariable() need to let all Aps
> > > > arrive to smm before it set the variable. If not, it would return
> > > > EFI_ACCESS_DENIED.
> > > >
> > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > >
> > > > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > > > ---
> > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > | 10 +++++++++-
> > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > | 3 ++-
> > > >
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > > > | 3 ++-
> > > > 3 files changed, 13 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git
> > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > index 5253c328dcd9..4944903e64d4 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 - 2023, 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"
> > > >
> > > > @@ -87,6 +88,13 @@ SmmVariableSetVariable ( {
> > > >
> > > > EFI_STATUS Status;
> > > >
> > > >
> > > >
> > > > + //
> > > >
> > > > + // Need to wait for all Aps to arrive in Relaxed-AP Sync Mode
> > > >
> > > > + //
> > > >
> > > > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> > > >
> > > > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check
> > > > + in
> > > > SMM!\n"));
> > > >
> > > > + }
> > > >
> > > > +
> > > >
> > > > //
> > > >
> > > > // Disable write protection when the calling SetVariable() through
> > > > EFI_SMM_VARIABLE_PROTOCOL.
> > > >
> > > > //
> > > >
> > > > diff --git
> > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > index 8c552b87e080..1cf0d051e6c9 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 - 2023, Intel Corporation. All rights
> > > > +reserved.<BR>
> > > >
> > > > # Copyright (c) Microsoft Corporation.
> > > >
> > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > > #
> > > >
> > > > @@ -84,6 +84,7 @@
> > > > VariablePolicyLib
> > > >
> > > > VariablePolicyHelperLib
> > > >
> > > > SafeIntLib
> > > >
> > > > + SmmCpuRendezvousLib
> > > >
> > > >
> > > >
> > > > [Protocols]
> > > >
> > > > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> > > >
> > > > diff --git
> > > >
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> > > n
> > > > f
> > > >
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> > > in
> > > > f
> > > > index f09bed40cf51..89187456ca25 100644
> > > > ---
> > > >
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> > > n
> > > > 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 - 2023, 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
> > > >
> > > > @@ -80,6 +80,7 @@
> > > > VariableFlashInfoLib
> > > >
> > > > VariablePolicyLib
> > > >
> > > > VariablePolicyHelperLib
> > > >
> > > > + SmmCpuRendezvousLib
> > > >
> > > >
> > > >
> > > > [Protocols]
> > > >
> > > > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> > > >
> > > > --
> > > > 2.26.2.windows.1
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable.
2023-06-01 1:07 ` Ni, Ray
@ 2023-06-01 1:09 ` Wu, Jiaxin
0 siblings, 0 replies; 8+ messages in thread
From: Wu, Jiaxin @ 2023-06-01 1:09 UTC (permalink / raw)
To: Ni, Ray, Yao, Jiewen, devel@edk2.groups.io, Li, Zhihao,
Gao, Liming, kraxel@redhat.com
Cc: Wang, Jian J
Thanks Jiewen & Ray.
Zhihao, could you try the suggestion from Jiewen? You can sync with me for detail.
Thanks,
Jiaxin
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, June 1, 2023 9:07 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Wu, Jiaxin
> <jiaxin.wu@intel.com>; Li, Zhihao <zhihao.li@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; kraxel@redhat.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>
> Subject: RE: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c:
> add Ap rendezvous check before SmmSetVariable.
>
> Jiewen,
> Good suggestion😊.
> This also resolve's Jiaxin's comments that check if the variable is non-volatile.
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Thursday, June 1, 2023 9:06 AM
> > To: devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>; Li, Zhihao
> > <zhihao.li@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray
> > <ray.ni@intel.com>; kraxel@redhat.com
> > Cc: Wang, Jian J <jian.j.wang@intel.com>
> > Subject: RE: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c:
> > add Ap rendezvous check before SmmSetVariable.
> >
> > Disabling flash is a silicon behavior.
> >
> > Can we do SmmWaitForAllProcessor() in FVB driver, or SPI driver ?
> >
> > Thank you
> > Yao, Jiewen
> >
> >
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
> > Jiaxin
> > > Sent: Thursday, June 1, 2023 9:03 AM
> > > To: devel@edk2.groups.io; Li, Zhihao <zhihao.li@intel.com>; Gao, Liming
> > > <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>;
> > kraxel@redhat.com
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/VariableSmm.c:
> > add
> > > Ap rendezvous check before SmmSetVariable.
> > >
> > > Hi All,
> > >
> > > I think we need this patch:
> > >
> > > There is a requirement that all CPU threads must in SMM for Non-Volatile
> > > variable. Because the SMM will disables the flash protection. Before that,
> we
> > > must guarantee all CPU threads are in SMM to avoid the non-smm mode
> > cpus
> > > modify the flash.
> > >
> > >
> > > Zhihao,
> > >
> > > I think this is only needed for the Non-Volatile, I suggest as below check:
> > >
> > > if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
> > > if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> > > DEBUG ((DEBUG_ERROR, " SmmVariableSetVariable: Fail to wait for
> all
> > AP
> > > check in SMM!\n"));
> > > Status = EFI_ABORTED;
> > > goto EXIT;
> > > }
> > > }
> > >
> > > Thanks,
> > > Jiaxin
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Li,
> > > > Zhihao
> > > > Sent: Friday, May 19, 2023 4:11 PM
> > > > To: Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io;
> Ni,
> > > > Ray <ray.ni@intel.com>; kraxel@redhat.com
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>
> > > > Subject: Re: [edk2-devel] [PATCH v1 1/1]
> MdeModulePkg/VariableSmm.c:
> > > > add Ap rendezvous check before SmmSetVariable.
> > > >
> > > > Hi Liming
> > > > In Ap-Relaxed mode, Bsp will not wait for all ap arrive and call the SMI
> > > > handlers. But some SMI handlers need all Aps arrive in smm mode such
> as
> > > > SmmSetVariable. As the design, SetVariable need to let all aps arrive
> > because
> > > > it will write flash. Half year ago, I send the patch that calling
> > > > SmmCpuRendezvous() before SmmSetVariable. It was reviewed but
> hasn't
> > > > merged. SmmCpuRendezvous() will return immediately in traditional-
> AP
> > > > mode.
> > > > I'm not sure what returns EFI_ACCESS_DENIED. Calling
> > SmmCpuRendezvous()
> > > > before SmmSetVariable is our original design but haven't implemented.
> > > >
> > > > -----Original Message-----
> > > > From: gaoliming <gaoliming@byosoft.com.cn>
> > > > Sent: Thursday, May 18, 2023 5:38 PM
> > > > To: Li, Zhihao <zhihao.li@intel.com>; devel@edk2.groups.io; Ni, Ray
> > > > <ray.ni@intel.com>; kraxel@redhat.com
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>
> > > > Subject: 回复: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap
> > > > rendezvous check before SmmSetVariable.
> > > >
> > > > Zhihao:
> > > > Have you root cause this issue that SmmVariableSetVariable may
> return
> > > > EFI_ACCESS_DENIED?
> > > >
> > > > I am not sure whether this fix is proper. I also add UefiCpuPkg
> maintainers
> > > > Ray and Gerd in the mail loop for this discussion.
> > > >
> > > > Thanks
> > > > Liming
> > > > > -----邮件原件-----
> > > > > 发件人: Zhihao Li <zhihao.li@intel.com>
> > > > > 发送时间: 2023年5月10日 18:57
> > > > > 收件人: devel@edk2.groups.io
> > > > > 抄送: Jian J Wang <jian.j.wang@intel.com>; Liming Gao
> > > > > <gaoliming@byosoft.com.cn>
> > > > > 主题: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap
> > rendezvous
> > > > check
> > > > > before SmmSetVariable.
> > > > >
> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4429
> > > > >
> > > > > For Ap-Relaxed sync mode, SmmVariableSetVariable() need to let all
> Aps
> > > > > arrive to smm before it set the variable. If not, it would return
> > > > > EFI_ACCESS_DENIED.
> > > > >
> > > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > > >
> > > > > Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> > > > > ---
> > > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > > | 10 +++++++++-
> > > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > > | 3 ++-
> > > > >
> > > >
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> > > > > | 3 ++-
> > > > > 3 files changed, 13 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git
> > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > > > index 5253c328dcd9..4944903e64d4 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 - 2023, 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"
> > > > >
> > > > > @@ -87,6 +88,13 @@ SmmVariableSetVariable ( {
> > > > >
> > > > > EFI_STATUS Status;
> > > > >
> > > > >
> > > > >
> > > > > + //
> > > > >
> > > > > + // Need to wait for all Aps to arrive in Relaxed-AP Sync Mode
> > > > >
> > > > > + //
> > > > >
> > > > > + if (EFI_ERROR (SmmWaitForAllProcessor (TRUE))) {
> > > > >
> > > > > + DEBUG ((DEBUG_ERROR, "SetVariable: fail to wait for all AP check
> > > > > + in
> > > > > SMM!\n"));
> > > > >
> > > > > + }
> > > > >
> > > > > +
> > > > >
> > > > > //
> > > > >
> > > > > // Disable write protection when the calling SetVariable() through
> > > > > EFI_SMM_VARIABLE_PROTOCOL.
> > > > >
> > > > > //
> > > > >
> > > > > diff --git
> > > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> > > > > index 8c552b87e080..1cf0d051e6c9 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 - 2023, Intel Corporation. All rights
> > > > > +reserved.<BR>
> > > > >
> > > > > # Copyright (c) Microsoft Corporation.
> > > > >
> > > > > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > > #
> > > > >
> > > > > @@ -84,6 +84,7 @@
> > > > > VariablePolicyLib
> > > > >
> > > > > VariablePolicyHelperLib
> > > > >
> > > > > SafeIntLib
> > > > >
> > > > > + SmmCpuRendezvousLib
> > > > >
> > > > >
> > > > >
> > > > > [Protocols]
> > > > >
> > > > > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> > > > >
> > > > > diff --git
> > > > >
> > > >
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> > > > n
> > > > > f
> > > > >
> > > >
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> > > > in
> > > > > f
> > > > > index f09bed40cf51..89187456ca25 100644
> > > > > ---
> > > > >
> > > >
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> > > > n
> > > > > 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 - 2023, 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
> > > > >
> > > > > @@ -80,6 +80,7 @@
> > > > > VariableFlashInfoLib
> > > > >
> > > > > VariablePolicyLib
> > > > >
> > > > > VariablePolicyHelperLib
> > > > >
> > > > > + SmmCpuRendezvousLib
> > > > >
> > > > >
> > > > >
> > > > > [Protocols]
> > > > >
> > > > > gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES
> > > > >
> > > > > --
> > > > > 2.26.2.windows.1
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> > >
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-06-01 1:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-10 10:56 [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add Ap rendezvous check before SmmSetVariable Li, Zhihao
2023-05-18 9:37 ` 回复: " gaoliming
2023-05-19 8:11 ` Li, Zhihao
2023-05-19 9:00 ` 回复: " gaoliming
2023-06-01 1:02 ` [edk2-devel] " Wu, Jiaxin
2023-06-01 1:06 ` Yao, Jiewen
2023-06-01 1:07 ` Ni, Ray
2023-06-01 1:09 ` Wu, Jiaxin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox