public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ray" <ray.ni@intel.com>
To: "Wu, Jiaxin" <jiaxin.wu@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Laszlo Ersek <lersek@redhat.com>,
	"Dong, Eric" <eric.dong@intel.com>,
	"Zeng, Star" <star.zeng@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"Kumar, Rahul R" <rahul.r.kumar@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 2/6] UefiCpuPkg: Adds SmmCpuSyncLib library class
Date: Wed, 6 Dec 2023 01:08:53 +0000	[thread overview]
Message-ID: <MN6PR11MB82443506C154BF7220C2AAD18C84A@MN6PR11MB8244.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20231130063139.7472-3-jiaxin.wu@intel.com>

Patch is good to me.
Only one comment: The file header comments should have 2 space chars in the beginning of each line.


Thanks,
Ray
> -----Original Message-----
> From: Wu, Jiaxin <jiaxin.wu@intel.com>
> Sent: Thursday, November 30, 2023 2:32 PM
> To: devel@edk2.groups.io
> Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
> Subject: [PATCH v2 2/6] UefiCpuPkg: Adds SmmCpuSyncLib library class
> 
> Intel is planning to provide different SMM CPU Sync implementation
> along with some specific registers to improve the SMI performance,
> hence need SmmCpuSyncLib Library for Intel.
> 
> This patch is to:
> 1.Adds SmmCpuSyncLib Library class in UefiCpuPkg.dec.
> 2.Adds SmmCpuSyncLib.h function declaration header file.
> 
> For the new SmmCpuSyncLib, it provides 3 sets of APIs:
> 
> 1. ContextInit/ContextDeinit/ContextReset:
> ContextInit() is called in driver's entrypoint to allocate and
> initialize the SMM CPU Sync context. ContextDeinit() is called in
> driver's unload function to deinitialize SMM CPU Sync context.
> ContextReset() is called before CPU exist SMI, which allows CPU to
> check into the next SMI from this point.
> 
> 2. GetArrivedCpuCount/CheckInCpu/CheckOutCpu/LockDoor:
> When SMI happens, all processors including BSP enter to SMM mode by
> calling CheckInCpu(). The elected BSP calls LockDoor() so that
> CheckInCpu() will return the error code after that. CheckOutCpu() can
> be called in error handling flow for the CPU who calls CheckInCpu()
> earlier. GetArrivedCpuCount() returns the number of checked-in CPUs.
> 
> 3. WaitForAPs/ReleaseOneAp/WaitForBsp/ReleaseBsp
> WaitForAPs() & ReleaseOneAp() are called from BSP to wait the number
> of APs and release one specific AP. WaitForBsp() & ReleaseBsp() are
> called from APs to wait and release BSP. The 4 APIs are used to
> synchronize the running flow among BSP and APs. BSP and AP Sync flow
> can be easy understand as below:
> BSP: ReleaseOneAp  -->  AP: WaitForBsp
> BSP: WaitForAPs    <--  AP: ReleaseBsp
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zeng Star <star.zeng@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  UefiCpuPkg/Include/Library/SmmCpuSyncLib.h | 278
> +++++++++++++++++++++++++++++
>  UefiCpuPkg/UefiCpuPkg.dec                  |   3 +
>  2 files changed, 281 insertions(+)
>  create mode 100644 UefiCpuPkg/Include/Library/SmmCpuSyncLib.h
> 
> diff --git a/UefiCpuPkg/Include/Library/SmmCpuSyncLib.h
> b/UefiCpuPkg/Include/Library/SmmCpuSyncLib.h
> new file mode 100644
> index 0000000000..22935cc006
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Library/SmmCpuSyncLib.h
> @@ -0,0 +1,278 @@
> +/** @file
> +Library that provides SMM CPU Sync related operations.
> +The lib provides 3 sets of APIs:
> +1. ContextInit/ContextDeinit/ContextReset:
> +ContextInit() is called in driver's entrypoint to allocate and initialize the SMM
> CPU Sync context.
> +ContextDeinit() is called in driver's unload function to deinitialize the SMM
> CPU Sync context.
> +ContextReset() is called before CPU exist SMI, which allows CPU to check
> into the next SMI from this point.
> +
> +2. GetArrivedCpuCount/CheckInCpu/CheckOutCpu/LockDoor:
> +When SMI happens, all processors including BSP enter to SMM mode by
> calling CheckInCpu().
> +The elected BSP calls LockDoor() so that CheckInCpu() will return the error
> code after that.
> +CheckOutCpu() can be called in error handling flow for the CPU who calls
> CheckInCpu() earlier.
> +GetArrivedCpuCount() returns the number of checked-in CPUs.
> +
> +3. WaitForAPs/ReleaseOneAp/WaitForBsp/ReleaseBsp
> +WaitForAPs() & ReleaseOneAp() are called from BSP to wait the number of
> APs and release one specific AP.
> +WaitForBsp() & ReleaseBsp() are called from APs to wait and release BSP.
> +The 4 APIs are used to synchronize the running flow among BSP and APs. BSP
> and AP Sync flow can be
> +easy understand as below:
> +BSP: ReleaseOneAp  -->  AP: WaitForBsp
> +BSP: WaitForAPs    <--  AP: ReleaseBsp
> +
> +Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef SMM_CPU_SYNC_LIB_H_
> +#define SMM_CPU_SYNC_LIB_H_
> +
> +#include <Uefi/UefiBaseType.h>
> +
> +//
> +// Opaque structure for SMM CPU Sync context.
> +//
> +typedef struct SMM_CPU_SYNC_CTX SMM_CPU_SYNC_CTX;
> +
> +/**
> +  Create and initialize the SMM CPU Sync context.
> +
> +  SmmCpuSyncContextInit() function is to allocate and initialize the SMM
> CPU Sync context.
> +
> +  @param[in]  NumberOfCpus          The number of Logical
> Processors in the system.
> +  @param[out] SmmCpuSyncCtx         Pointer to the new created and
> initialized SMM CPU Sync context object.
> +                                    NULL will be returned if any
> error happen during init.
> +
> +  @retval RETURN_SUCCESS            The SMM CPU Sync context
> was successful created and initialized.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx is NULL.
> +  @retval RETURN_BUFFER_TOO_SMALL   Overflow happen
> +  @retval RETURN_OUT_OF_RESOURCES   There are not enough
> resources available to create and initialize SMM CPU Sync context.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncContextInit (
> +  IN   UINTN             NumberOfCpus,
> +  OUT  SMM_CPU_SYNC_CTX  **SmmCpuSyncCtx
> +  );
> +
> +/**
> +  Deinit an allocated SMM CPU Sync context.
> +
> +  SmmCpuSyncContextDeinit() function is to deinitialize SMM CPU Sync
> context, the resources allocated in
> +  SmmCpuSyncContextInit() will be freed.
> +
> +  Note: This function only can be called after SmmCpuSyncContextInit()
> return success.
> +
> +  @param[in,out]  SmmCpuSyncCtx     Pointer to the SMM CPU Sync
> context object to be deinitialized.
> +
> +  @retval RETURN_SUCCESS            The SMM CPU Sync context
> was successful deinitialized.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx is NULL.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncContextDeinit (
> +  IN OUT SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx
> +  );
> +
> +/**
> +  Reset SMM CPU Sync context.
> +
> +  SmmCpuSyncContextReset() function is to reset SMM CPU Sync context to
> the initialized state.
> +
> +  @param[in,out]  SmmCpuSyncCtx     Pointer to the SMM CPU Sync
> context object to be reset.
> +
> +  @retval RETURN_SUCCESS            The SMM CPU Sync context
> was successful reset.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx is NULL.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncContextReset (
> +  IN OUT SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx
> +  );
> +
> +/**
> +  Get current number of arrived CPU in SMI.
> +
> +  For traditional CPU synchronization method, BSP might need to know the
> current number of arrived CPU in
> +  SMI to make sure all APs in SMI. This API can be for that purpose.
> +
> +  @param[in]      SmmCpuSyncCtx     Pointer to the SMM CPU Sync
> context object.
> +  @param[in,out]  CpuCount          Current count of arrived CPU in
> SMI.
> +
> +  @retval RETURN_SUCCESS            Get current number of arrived
> CPU in SMI successfully.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx or CpuCount
> is NULL.
> +  @retval RETURN_ABORTED            Function Aborted due to the
> door has been locked by SmmCpuSyncLockDoor() function.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncGetArrivedCpuCount (
> +  IN     SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx,
> +  IN OUT UINTN             *CpuCount
> +  );
> +
> +/**
> +  Performs an atomic operation to check in CPU.
> +
> +  When SMI happens, all processors including BSP enter to SMM mode by
> calling SmmCpuSyncCheckInCpu().
> +
> +  @param[in,out]  SmmCpuSyncCtx     Pointer to the SMM CPU Sync
> context object.
> +  @param[in]      CpuIndex          Check in CPU index.
> +
> +  @retval RETURN_SUCCESS            Check in CPU (CpuIndex)
> successfully.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx is NULL.
> +  @retval RETURN_ABORTED            Check in CPU failed due to
> SmmCpuSyncLockDoor() has been called by one elected CPU.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncCheckInCpu (
> +  IN OUT SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx,
> +  IN     UINTN             CpuIndex
> +  );
> +
> +/**
> +  Performs an atomic operation to check out CPU.
> +
> +  CheckOutCpu() can be called in error handling flow for the CPU who calls
> CheckInCpu() earlier.
> +
> +  @param[in,out]  SmmCpuSyncCtx     Pointer to the SMM CPU Sync
> context object.
> +  @param[in]      CpuIndex          Check out CPU index.
> +
> +  @retval RETURN_SUCCESS            Check out CPU (CpuIndex)
> successfully.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx is NULL.
> +  @retval RETURN_NOT_READY          The CPU is not checked-in.
> +  @retval RETURN_ABORTED            Check out CPU failed due to
> SmmCpuSyncLockDoor() has been called by one elected CPU.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncCheckOutCpu (
> +  IN OUT SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx,
> +  IN     UINTN             CpuIndex
> +  );
> +
> +/**
> +  Performs an atomic operation lock door for CPU checkin or checkout.
> +
> +  After this function:
> +  CPU can not check in via SmmCpuSyncCheckInCpu().
> +  CPU can not check out via SmmCpuSyncCheckOutCpu().
> +  CPU can not get number of arrived CPU in SMI via
> SmmCpuSyncGetArrivedCpuCount(). The number of
> +  arrived CPU in SMI will be returned in CpuCount.
> +
> +  The CPU specified by CpuIndex is elected to lock door.
> +
> +  @param[in,out]  SmmCpuSyncCtx     Pointer to the SMM CPU Sync
> context object.
> +  @param[in]      CpuIndex          Indicate which CPU to lock door.
> +  @param[in,out]  CpuCount          Number of arrived CPU in SMI
> after look door.
> +
> +  @retval RETURN_SUCCESS            Lock door for CPU successfully.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx or CpuCount
> is NULL.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncLockDoor (
> +  IN OUT SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx,
> +  IN     UINTN             CpuIndex,
> +  IN OUT UINTN             *CpuCount
> +  );
> +
> +/**
> +  Used by the BSP to wait for APs.
> +
> +  The number of APs need to be waited is specified by NumberOfAPs. The
> BSP is specified by BspIndex.
> +
> +  Note: This function is blocking mode, and it will return only after the
> number of APs released by
> +  calling SmmCpuSyncReleaseBsp():
> +  BSP: WaitForAPs    <--  AP: ReleaseBsp
> +
> +  @param[in,out]  SmmCpuSyncCtx     Pointer to the SMM CPU Sync
> context object.
> +  @param[in]      NumberOfAPs       Number of APs need to be
> waited by BSP.
> +  @param[in]      BspIndex          The BSP Index to wait for APs.
> +
> +  @retval RETURN_SUCCESS            BSP to wait for APs
> successfully.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx is NULL or
> NumberOfAPs > total number of processors in system.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncWaitForAPs (
> +  IN OUT SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx,
> +  IN     UINTN             NumberOfAPs,
> +  IN     UINTN             BspIndex
> +  );
> +
> +/**
> +  Used by the BSP to release one AP.
> +
> +  The AP is specified by CpuIndex. The BSP is specified by BspIndex.
> +
> +  @param[in,out]  SmmCpuSyncCtx     Pointer to the SMM CPU Sync
> context object.
> +  @param[in]      CpuIndex          Indicate which AP need to be
> released.
> +  @param[in]      BspIndex          The BSP Index to release AP.
> +
> +  @retval RETURN_SUCCESS            BSP to release one AP
> successfully.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx is NULL or
> CpuIndex is same as BspIndex.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncReleaseOneAp   (
> +  IN OUT SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx,
> +  IN     UINTN             CpuIndex,
> +  IN     UINTN             BspIndex
> +  );
> +
> +/**
> +  Used by the AP to wait BSP.
> +
> +  The AP is specified by CpuIndex. The BSP is specified by BspIndex.
> +
> +  Note: This function is blocking mode, and it will return only after the AP
> released by
> +  calling SmmCpuSyncReleaseOneAp():
> +  BSP: ReleaseOneAp  -->  AP: WaitForBsp
> +
> +  @param[in,out]  SmmCpuSyncCtx    Pointer to the SMM CPU Sync
> context object.
> +  @param[in]      CpuIndex         Indicate which AP wait BSP.
> +  @param[in]      BspIndex         The BSP Index to be waited.
> +
> +  @retval RETURN_SUCCESS            AP to wait BSP successfully.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx is NULL or
> CpuIndex is same as BspIndex.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncWaitForBsp (
> +  IN OUT SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx,
> +  IN     UINTN             CpuIndex,
> +  IN     UINTN             BspIndex
> +  );
> +
> +/**
> +  Used by the AP to release BSP.
> +
> +  The AP is specified by CpuIndex. The BSP is specified by BspIndex.
> +
> +  @param[in,out]  SmmCpuSyncCtx     Pointer to the SMM CPU Sync
> context object.
> +  @param[in]      CpuIndex          Indicate which AP release BSP.
> +  @param[in]      BspIndex          The BSP Index to be released.
> +
> +  @retval RETURN_SUCCESS            AP to release BSP successfully.
> +  @retval RETURN_INVALID_PARAMETER  SmmCpuSyncCtx is NULL or
> CpuIndex is same as BspIndex.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +SmmCpuSyncReleaseBsp (
> +  IN OUT SMM_CPU_SYNC_CTX  *SmmCpuSyncCtx,
> +  IN     UINTN             CpuIndex,
> +  IN     UINTN             BspIndex
> +  );
> +
> +#endif
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index 0b5431dbf7..20ab079219 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -62,10 +62,13 @@
>    CpuPageTableLib|Include/Library/CpuPageTableLib.h
> 
>    ## @libraryclass   Provides functions for manipulating smram savestate
> registers.
>    MmSaveStateLib|Include/Library/MmSaveStateLib.h
> 
> +  ## @libraryclass   Provides functions for SMM CPU Sync Operation.
> +  SmmCpuSyncLib|Include/Library/SmmCpuSyncLib.h
> +
>  [LibraryClasses.RISCV64]
>    ##  @libraryclass  Provides functions to manage MMU features on
> RISCV64 CPUs.
>    ##
>    RiscVMmuLib|Include/Library/BaseRiscVMmuLib.h
> 
> --
> 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112086): https://edk2.groups.io/g/devel/message/112086
Mute This Topic: https://groups.io/mt/102889292/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2023-12-06  1:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-30  6:31 [edk2-devel] [PATCH v2 0/6] Refine SMM CPU Sync flow and abstract SmmCpuSyncLib Wu, Jiaxin
2023-11-30  6:31 ` [edk2-devel] [PATCH v2 1/6] UefiCpuPkg/PiSmmCpuDxeSmm: Optimize Semaphore Sync between BSP and AP Wu, Jiaxin
2023-11-30  6:31 ` [edk2-devel] [PATCH v2 2/6] UefiCpuPkg: Adds SmmCpuSyncLib library class Wu, Jiaxin
2023-12-06  1:08   ` Ni, Ray [this message]
2023-11-30  6:31 ` [edk2-devel] [PATCH v2 3/6] UefiCpuPkg: Implements SmmCpuSyncLib library instance Wu, Jiaxin
2023-12-06  3:18   ` Ni, Ray
2023-12-06  4:23     ` Wu, Jiaxin
2023-11-30  6:31 ` [edk2-devel] [PATCH v2 4/6] OvmfPkg: Specifies SmmCpuSyncLib instance Wu, Jiaxin
2023-11-30  6:31 ` [edk2-devel] [PATCH v2 5/6] UefiPayloadPkg: " Wu, Jiaxin
2023-12-06  0:09   ` Guo, Gua
2023-11-30  6:31 ` [edk2-devel] [PATCH v2 6/6] UefiCpuPkg/PiSmmCpuDxeSmm: Consume SmmCpuSyncLib Wu, Jiaxin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MN6PR11MB82443506C154BF7220C2AAD18C84A@MN6PR11MB8244.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox