[PATCH v2 1/1] UefiCpuPkg/PiSmmCpuDxeSmm:add Ap Rendezvous check in PerformRemainingTasks.

[PATCH v2 1/1] UefiCpuPkg/PiSmmCpuDxeSmm:add Ap Rendezvous check in PerformRemainingTasks.

[Li, Zhihao]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4424

In Relaxed-AP Sync Mode, BSP will not wait for all Aps arrive. However,
PerformRemainingTasks() needs to wait all Aps arrive before calling
SetMemMapAttributes and ConfigSmmCodeAccessCheck() when mSmmReadyToLock
is true. In SetMemMapAttributes(), SmmSetMemoryAttributesEx() will call
FlushTlbForAll() that need to start up the aps. So it need to let all
aps arrive. Same as SetMemMapAttributes(), ConfigSmmCodeAccessCheck()
also will start up the aps.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Zhihao Li <zhihao.li@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index 655175a2c6db..1e210beb0e06 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -1,7 +1,7 @@
 /** @file
 Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.
 
-Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -1441,6 +1441,19 @@ PerformRemainingTasks (
   )
 {
   if (mSmmReadyToLock) {
+    //
+    // Check if all Aps enter SMM. In Relaxed-AP Sync Mode, BSP will not wait for
+    // all Aps arrive. However,PerformRemainingTasks() needs to wait all Aps arrive before calling
+    // SetMemMapAttributes() and ConfigSmmCodeAccessCheck() when mSmmReadyToLock
+    // is true. In SetMemMapAttributes(), SmmSetMemoryAttributesEx() will call
+    // FlushTlbForAll() that need to start up the aps. So it need to let all
+    // aps arrive. Same as SetMemMapAttributes(), ConfigSmmCodeAccessCheck()
+    // also will start up the aps.
+    //
+    if (EFI_ERROR (SmmCpuRendezvous (NULL, TRUE))) {
+      DEBUG ((DEBUG_ERROR, "PerformRemainingTasks: fail to wait for all AP check in SMM!\n"));
+    }
+
     //
     // Start SMM Profile feature
     //
-- 
2.26.2.windows.1

Re: [PATCH v2 1/1] UefiCpuPkg/PiSmmCpuDxeSmm:add Ap Rendezvous check in PerformRemainingTasks.

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Li, Zhihao <zhihao.li@intel.com>
> Sent: Thursday, May 4, 2023 3:07 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v2 1/1] UefiCpuPkg/PiSmmCpuDxeSmm:add Ap Rendezvous
> check in PerformRemainingTasks.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4424
> 
> In Relaxed-AP Sync Mode, BSP will not wait for all Aps arrive. However,
> PerformRemainingTasks() needs to wait all Aps arrive before calling
> SetMemMapAttributes and ConfigSmmCodeAccessCheck() when
> mSmmReadyToLock
> is true. In SetMemMapAttributes(), SmmSetMemoryAttributesEx() will call
> FlushTlbForAll() that need to start up the aps. So it need to let all
> aps arrive. Same as SetMemMapAttributes(), ConfigSmmCodeAccessCheck()
> also will start up the aps.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> 
> Signed-off-by: Zhihao Li <zhihao.li@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index 655175a2c6db..1e210beb0e06 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -1,7 +1,7 @@
>  /** @file
> 
>  Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.
> 
> 
> 
> -Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
> 
>  Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> 
> 
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -1441,6 +1441,19 @@ PerformRemainingTasks (
>    )
> 
>  {
> 
>    if (mSmmReadyToLock) {
> 
> +    //
> 
> +    // Check if all Aps enter SMM. In Relaxed-AP Sync Mode, BSP will not wait for
> 
> +    // all Aps arrive. However,PerformRemainingTasks() needs to wait all Aps
> arrive before calling
> 
> +    // SetMemMapAttributes() and ConfigSmmCodeAccessCheck() when
> mSmmReadyToLock
> 
> +    // is true. In SetMemMapAttributes(), SmmSetMemoryAttributesEx() will call
> 
> +    // FlushTlbForAll() that need to start up the aps. So it need to let all
> 
> +    // aps arrive. Same as SetMemMapAttributes(),
> ConfigSmmCodeAccessCheck()
> 
> +    // also will start up the aps.
> 
> +    //
> 
> +    if (EFI_ERROR (SmmCpuRendezvous (NULL, TRUE))) {
> 
> +      DEBUG ((DEBUG_ERROR, "PerformRemainingTasks: fail to wait for all AP
> check in SMM!\n"));
> 
> +    }
> 
> +
> 
>      //
> 
>      // Start SMM Profile feature
> 
>      //
> 
> --
> 2.26.2.windows.1

Re: [edk2-devel] [PATCH v2 1/1] UefiCpuPkg/PiSmmCpuDxeSmm:add Ap Rendezvous check in PerformRemainingTasks.

[Wu, Jiaxin]

> 
> +    if (EFI_ERROR (SmmCpuRendezvous (NULL, TRUE))) {
> 
> +      DEBUG ((DEBUG_ERROR, "PerformRemainingTasks: fail to wait for all AP
> check in SMM!\n"));

If error happen, do we should return directly or continue the executing?

Thanks,
Jiaxin 

> 
> +    }
> 
> +
> 
>      //
> 
>      // Start SMM Profile feature
> 
>      //
> 
> --
> 2.26.2.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#103972):
> https://edk2.groups.io/g/devel/message/103972
> Mute This Topic: https://groups.io/mt/98679265/1787330
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiaxin.wu@intel.com]
> -=-=-=-=-=-=
>