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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to