From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from zrleap.intel-email.com (zrleap.intel-email.com [114.80.218.36]) by mx.groups.io with SMTP id smtpd.web10.21372.1684486829661731734 for ; Fri, 19 May 2023 02:00:31 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@byosoft.com.cn header.s=cloud-union header.b=cxmpRqnu; spf=pass (domain: byosoft.com.cn, ip: 114.80.218.36, mailfrom: gaoliming@byosoft.com.cn) Received: from zrleap.intel-email.com (localhost [127.0.0.1]) by zrleap.intel-email.com (Postfix) with ESMTP id 1B71AA32E32D for ; Fri, 19 May 2023 17:00:26 +0800 (CST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=byosoft.com.cn; s=cloud-union; t=1684486827; bh=5SmVRiAc1taaO5vzuu7Nd5HPP6kuzlyMbf2UNupb/pU=; h=From:To:Cc:References:In-Reply-To:Subject:Date; b=cxmpRqnuaR2Jo+/omInbcArF64lJRpzAW9TvLvUWBlCTusf9q0AgUy9fW+i3g2uQq BXVTsguTzQ8Fir/HdN2mezu1fHjQU8+KHQtsv4y/S9QMo23SBNBv9rjh543Sqwy677 X5x3aM9bahJlkBnerYiScKVVKqwBVFo5AuEFB/Uo= Received: from localhost (localhost [127.0.0.1]) by zrleap.intel-email.com (Postfix) with ESMTP id 3713CA32E2C8 for ; Fri, 19 May 2023 17:00:26 +0800 (CST) Received: from zrleap.intel-email.com (localhost [127.0.0.1]) by zrleap.intel-email.com (Postfix) with ESMTP id 096C3A32E31C for ; Fri, 19 May 2023 17:00:25 +0800 (CST) Authentication-Results: zrleap.intel-email.com; none Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by zrleap.intel-email.com (Postfix) with SMTP id AD3BCA32E301 for ; Fri, 19 May 2023 17:00:21 +0800 (CST) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Fri, 19 May 2023 17:00:13 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Li, Zhihao'" , , "'Ni, Ray'" , Cc: "'Wang, Jian J'" References: <20230510105653.635-1-zhihao.li@intel.com> <05b701d9896c$5df57160$19e05420$@byosoft.com.cn> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbUEFUQ0ggdjEgMS8xXSBNZGVNb2R1bGVQa2cvVmFyaWFibGVTbW0uYzogYWRkIEFwIHJlbmRlenZvdXMgY2hlY2sgYmVmb3JlIFNtbVNldFZhcmlhYmxlLg==?= Date: Fri, 19 May 2023 17:00:18 +0800 Message-ID: <084601d98a30$558a4a90$009edfb0$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQKukQTCmi9SQ+IV648arAIwUCTP+AKd6JjaAhKKhu6tkbq38A== Sender: "gaoliming" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Ray and Gerd: Can you give the comments for this change? Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Li, Zhihao > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: = 2023=E5=B9=B45=E6=9C=8819=E6=97=A5 16:11 > =E6=94=B6=E4=BB=B6=E4=BA=BA: Gao, Liming ; = devel@edk2.groups.io; Ni, > Ray ; kraxel@redhat.com > =E6=8A=84=E9=80=81: Wang, Jian J > =E4=B8=BB=E9=A2=98: RE: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add = Ap rendezvous > check before SmmSetVariable. >=20 > 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. >=20 > -----Original Message----- > From: gaoliming > Sent: Thursday, May 18, 2023 5:38 PM > To: Li, Zhihao ; devel@edk2.groups.io; Ni, Ray > ; kraxel@redhat.com > Cc: Wang, Jian J > Subject: =E5=9B=9E=E5=A4=8D: [PATCH v1 1/1] = MdeModulePkg/VariableSmm.c: add Ap > rendezvous check before SmmSetVariable. >=20 > Zhihao: > Have you root cause this issue that SmmVariableSetVariable may = return > EFI_ACCESS_DENIED? >=20 > I am not sure whether this fix is proper. I also add UefiCpuPkg = maintainers > Ray and Gerd in the mail loop for this discussion. >=20 > Thanks > Liming > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > =E5=8F=91=E4=BB=B6=E4=BA=BA: Zhihao Li > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: = 2023=E5=B9=B45=E6=9C=8810=E6=97=A5 18:57 > > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io > > =E6=8A=84=E9=80=81: Jian J Wang ; Liming Gao > > > > =E4=B8=BB=E9=A2=98: [PATCH v1 1/1] MdeModulePkg/VariableSmm.c: add = Ap rendezvous > check > > before SmmSetVariable. > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4429 > > > > 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 > > Cc: Liming Gao > > > > Signed-off-by: Zhihao Li > > --- > > 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.
> > > > +Copyright (c) 2010 - 2023, Intel Corporation. All rights > > +reserved.
> > > > Copyright (c) 2018, Linaro, Ltd. All rights reserved.
> > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > > > @@ -28,6 +28,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > #include > > > > #include > > > > +#include > > > > > > > > #include > > > > #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.
> > > > +# Copyright (c) 2010 - 2023, Intel Corporation. All rights > > +reserved.
> > > > # 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.
> > > > +# Copyright (c) 2010 - 2023, Intel Corporation. All rights > > +reserved.
> > > > # Copyright (c) 2018, Linaro, Ltd. All rights reserved.
> > > > # 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 >=20 >=20