Re: [edk2-devel] [PATCH v3 3/6] UefiCpuPkg: Implements SmmCpuSyncLib library instance

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

BTW, for SmmCpuSyncGetArrivedCpuCount ():

we can't check the CpuCount (Original is named as Counter Sem) is locked or not, then decide return from the *Context->CpuCount or locked value for the arrived CPU in SMI. Just like:

if (*Context->CpuCount == MAX_UINT32) {        ------> does not meet this condition, means unlocked!
	Return real CpuCount from the SmmCpuSyncLockDoor(). 
} 
                ----> lock operation is here!!!! *Context->CpuCount change to MAX_UINT32
Return *Context->CpuCount;   --> return wrong value since MAX_UINT32 is return.

Because if we found it's not locked during the check, but it suddenly locked before return, then -1 will be returned. this is not atomic operation. The behavior is not expected. If we add the atomic operation here, I believe it will surely impact the existing performance.

And the real usage case is that we only need this api before the lock. I don't want make it complex.

So, based on this, we add the comment in the function:
  The caller shall not call this function for the number of arrived CPU after look door
  in SMI since the value has been returned in the parameter of LockDoor().
 
See below: 

/**
  Get current number of arrived CPU in SMI.

  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.

  The caller shall not call this function for the number of arrived CPU after look door
  in SMI since the value has been returned in the parameter of LockDoor().

  If Context is NULL, then ASSERT().

  @param[in]      Context     Pointer to the SMM CPU Sync context object.

  @retval    Current number of arrived CPU in SMI.

**/
UINTN
EFIAPI
SmmCpuSyncGetArrivedCpuCount (
  IN  SMM_CPU_SYNC_CONTEXT  *Context
  )
{
  ASSERT (Context != NULL);

  return *Context->CpuCount;
}

Thanks,
Jiaxin 
















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