Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SmmCpuPlatformHookLib IsCpuSyncAlwaysNeeded interface #6557

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

sweeaun
Copy link
Contributor

@sweeaun sweeaun commented Dec 18, 2024

Description

Determines whether all CPUs sync is needed.

This function determine if all CPUs needed to be synced unconditional during the 1st APs sync.

By default, it returns FALSE which indicates not all CPUs needed to be synced. In case of all CPUs
needed to be synced, platform specific implementation can return TRUE.

How This Was Tested

No test except compile the code successfully as new interface not being used by other code now. Test can be done once it called by other code later.

@sweeaun sweeaun changed the title UefiCpuPkg/SmmCpuPlatformHookLibNull: Add new interface Add SmmCpuPlatformHookLib IsPlatformLmceStatusCheck interface Dec 18, 2024
@jiaxinwu
Copy link
Member

please also refine the commit message according the new name: IsLmceStatusCheckNeeded

@jiaxinwu
Copy link
Member

use 2 commits in 1 PR for those 2 different packages changes

@sweeaun sweeaun changed the title Add SmmCpuPlatformHookLib IsPlatformLmceStatusCheck interface Add SmmCpuPlatformHookLib IsLmceStatusCheckNeeded interface Dec 19, 2024
@sweeaun sweeaun requested a review from jiaxinwu December 19, 2024 06:46
@kraxel
Copy link
Member

kraxel commented Jan 2, 2025

LMCE expands to "Local Machine Check Exception" I assume?

The commit messages describes what is changed, but not why this change is needed. Can you please add that information?

@sweeaun
Copy link
Contributor Author

sweeaun commented Jan 2, 2025

LMCE expands to "Local Machine Check Exception" I assume?

The commit messages describes what is changed, but not why this change is needed. Can you please add that information?

Hi @kraxel,
Yup. LMCE here stands for Local Machine Check Exception.

This new interface allow platform specific implementation to check whether the LMCE status check is required.
This new interface will be consumed by UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c SmmWaitForApArrival() later. However, the consumer part PR will be submitted later to avoid immediate downstream build error. This PR allow downstream to implement platform specific implementation, thus the transition will be smoother when downstream received the consumer part PR later.
I shall add this info into the commit message. Let me know if you have any more questions. Thanks.

@jiaxinwu jiaxinwu added the push Auto push patch series in PR if all checks pass label Jan 6, 2025
@jiaxinwu jiaxinwu closed this Jan 6, 2025
@jiaxinwu jiaxinwu reopened this Jan 6, 2025
@sweeaun sweeaun requested a review from jiaxinwu January 6, 2025 08:11
@jiaxinwu jiaxinwu self-requested a review January 6, 2025 08:12
@jiaxinwu jiaxinwu self-requested a review January 6, 2025 08:18
@jiaxinwu jiaxinwu removed the push Auto push patch series in PR if all checks pass label Jan 6, 2025
@jiaxinwu jiaxinwu added push Auto push patch series in PR if all checks pass and removed push Auto push patch series in PR if all checks pass labels Jan 6, 2025
@sweeaun sweeaun changed the title Add SmmCpuPlatformHookLib IsLmceStatusCheckNeeded interface Add SmmCpuPlatformHookLib IsCpuSyncAlwaysNeeded interface Jan 8, 2025
@sweeaun
Copy link
Contributor Author

sweeaun commented Jan 8, 2025

Based on input from @niruiyu, renamed the interface to IsCpuSyncAlwaysNeeded which is more relevant and generic to the code consumer in MP service later.

@sweeaun sweeaun requested a review from jiaxinwu January 8, 2025 13:35
@jiaxinwu jiaxinwu added the push Auto push patch series in PR if all checks pass label Jan 10, 2025
This patch adds the IsCpuSyncAlwaysNeeded interface to the SmmCpuPlatformHookLib.
This interface will determine whether the first CPU Synchronization should be
executed unconditionally when a SMI occurs.

If the function returns true, it indicates that there is no need to check the
system configuration and status, and the first CPU Synchronization should be
executed unconditionally.

If the function returns false, it indicates that the first CPU Synchronization is
not executed unconditionally, and the decision to synchronize should be based on
the system configuration and status.

Signed-off-by: Khor Swee Aun <[email protected]>
This patch is to implement default IsCpuSyncAlwaysNeeded definition
for SmmCpuPlatformHookLibQemu. This interface will determine whether the first
CPU Synchronization should be executed unconditionally when a SMI occurs.

If the function returns true, it indicates that there is no need to check the system
configuration and status, and the first CPU Synchronization should be executed
unconditionally.

If the function returns false, it indicates that the first CPU Synchronization is
not executed unconditionally, and the decision to synchronize should be based on
the system configuration and status.

Signed-off-by: Khor Swee Aun <[email protected]>
Copy link

mergify bot commented Jan 10, 2025

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

@mergify mergify bot merged commit c0533b7 into tianocore:master Jan 10, 2025
126 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
push Auto push patch series in PR if all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants