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]
-=-=-=-=-=-=-=-=-=-=-=-
