I provided 4 comments in below, starting with "[Ray".

Sorry for the poor new Outlook that I am not able to put ">" in the original email.

Thanks,
Ray

________________________________

(1) I agree that the internals of the context should be hidden, but
(VOID*) is not the right way. Instead, please use an incomplete
structure type.

That is, in the library header class file, do the following:

  typedef struct SMM_CPU_SYNC_CTX SMM_CPU_SYNC_CTX;

and then make all these functions return and take

  SMM_CPU_SYNC_CTX *

The library users will still not know what is inside SMM_CPU_SYNC_CTX,
just like with (VOID*), but the compiler will be able to perform much
stronger type checking than with (VOID*).

And then in the library *instance(s)*, you can complete the incomplete type:

  struct SMM_CPU_SYNC_CTX {
    ...
  };

[Ray.1] Good suggestion. I still remember you taught me this technique when I
wrote the FrameBufferBltLib.

(3) By definition, in a function that resets the internals of an object,
you cannot specify "IN" for that function. It must be OUT.

[Ray.2] I have a different opinion about IN/OUT.  I think we should use "IN OUT".


Please add a large comment to the top of the library class header that
explains the operation / concepts of the context. What operations and
behaviors are defined for this data type?

[Ray.3] Good suggestions.
The lib provides 3 sets of APIs:
1. Init/Deinit
Init() is called in driver's entrypoint for allocating the storage and initialize the content of sync object.
Deinit() is called in driver's unload function for undoing what has been done in Init().

2. CheckInCpu/CheckOutCpu/LockDoor/GetArrivedCpuCount
When SMI happens, all processors including BSP enter to SMM mode by calling CheckInCpu().
The elected BSP calls LockDoor() so that CheckInCpu() after that returns failure code.
CheckOutCpu() is called in error handling flow for the CPU who calls CheckInCpu() earlier.
GetArrivedCpuCount() returns the number of checked-in CPUs.

3. WaitForAllAPs/ReleaseOneAp/WaitForBsp/ReleaseBsp
First 2 APIs are called from BSP.
Latter 2 APIs are called from APs.
The 4 APIs are used to synchronize the running flow among BSP and APs.

> +
> +  @return     CPU arrival count number.

(14) why is it necessary for outputting the arrived number when locking?
[Ray.4] As I explained above, when BSP locks the door, it might need to know how many CPUs are in the SMM "room".
Maybe today the number is not cared by BSP.



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