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...@intel.com>; Gao, Liming > <gaolim...@byosoft.com.cn>; Ni, Ray <ray...@intel.com>; kra...@redhat.com > Cc: Wang, Jian J <jian.j.w...@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 <gaolim...@byosoft.com.cn>; devel@edk2.groups.io; Ni, > > Ray <ray...@intel.com>; kra...@redhat.com > > Cc: Wang, Jian J <jian.j.w...@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 <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.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 > > > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105518): https://edk2.groups.io/g/devel/message/105518 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] -=-=-=-=-=-=-=-=-=-=-=-