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 <gaolim...@byosoft.com.cn> Sent: Thursday, May 18, 2023 5:38 PM To: Li, Zhihao <zhihao...@intel.com>; devel@edk2.groups.io; Ni, Ray <ray...@intel.com>; kra...@redhat.com Cc: Wang, Jian J <jian.j.w...@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...@intel.com> > 发送时间: 2023年5月10日 18:57 > 收件人: devel@edk2.groups.io > 抄送: Jian J Wang <jian.j.w...@intel.com>; Liming Gao > <gaolim...@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.w...@intel.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > > Signed-off-by: Zhihao Li <zhihao...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105071): https://edk2.groups.io/g/devel/message/105071 Mute This Topic: https://groups.io/mt/99008324/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
