Closed
Conversation
sunceping
force-pushed
the
updateMaskinApRunLoop.v1
branch
from
b296ba0 to
b5a6345
Compare
Refer to intel-tdx-module-api spec section 5.5.21, GPR mask (TDVMCALL_EXPOSE_REGS_MASK) is a bitmap that controls which part of the guest TD GPR and XMM state is passed as-is to the VMM and back. - A bit value of 0 indicates that the corresponding register is saved by the Intel TDX module and not passed as-is to Host VMM. - A bit value of 1 indicates that the corresponding register is passed as-is to the host VMM. Currently, RBX is used as the mailbox address in ApRunLoop.nasm, the corresponding bit value of RBX in MASK(Bit 3) is set as 1 which means the value is passed to Host VMM as-is and it can be changed by Host VMM. So the bitmask shall be set as 0 to avoid this situation. Reference: [TDX-API]: intel-tdx-module-abi-spec https://cdrdv2.intel.com/v1/dl/getContent/733579 Cc: Erdem Aktas <erdemaktas@google.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Hunter Adrian <adrian.hunter@intel.com> Signed-off-by: Ceping Sun <cepingx.sun@intel.com>
sunceping
force-pushed
the
updateMaskinApRunLoop.v1
branch
from
b5a6345 to
10b9439
Compare
sunceping
pushed a commit
that referenced
this pull request
Jul 1, 2025
…eEntry This patch introduces a synchronization point between the BSP and APs to ensure all APs have entered their SMM wait-loop (while (TRUE) in APHandler ()) before the BSP calls into the SMI handler logic via gSmmCpuPrivate ->SmmCoreEntry(). Previously, the BSP would invoke ReleaseAllAPs() and immediately proceed to SmmCoreEntry() without confirming whether APs had reached the stable waiting state. If SmmStartupThisAp() was called inside the SMI handler shortly after ReleaseAllAPs(), it might lead to a race condition: APs are issued two consecutive wait signals (SmmCpuSyncWaitForBsp()). BSP sends two consecutive releases (ReleaseAllAPs() + SmmStartupThisAp()) If an AP has not yet responded to the first release, the second release may overwrite the semaphore state, and the AP might miss the notification, causing it to hang or behave unpredictably. To address this: A SmmCpuSyncWaitForAPs() is added in BSP after mmCpuPlatformHookBeforeMmiHandler() and before entering SmmCoreEntry(). A matching SmmCpuSyncReleaseBsp() is added in AP immediately after its own SmmCpuPlatformHookBeforeMmiHandler() This ensures that BSP does not enter SMI handler logic or dispatch any AP-related requests before all APs are confirmed to be idle and ready. Debug sync point markers (e.g., /// #6, #7) are updated accordingly. This change eliminates a subtle but critical race condition in multi-processor/multi-socket systems during SMM entry and improves overall synchronization safety. Signed-off-by: Wei6 Xu <wei6.xu@intel.com>
mergify bot
pushed a commit
that referenced
this pull request
Jul 2, 2025
…eEntry This patch introduces a synchronization point between the BSP and APs to ensure all APs have entered their SMM wait-loop (while (TRUE) in APHandler ()) before the BSP calls into the SMI handler logic via gSmmCpuPrivate ->SmmCoreEntry(). Previously, the BSP would invoke ReleaseAllAPs() and immediately proceed to SmmCoreEntry() without confirming whether APs had reached the stable waiting state. If SmmStartupThisAp() was called inside the SMI handler shortly after ReleaseAllAPs(), it might lead to a race condition: APs are issued two consecutive wait signals (SmmCpuSyncWaitForBsp()). BSP sends two consecutive releases (ReleaseAllAPs() + SmmStartupThisAp()) If an AP has not yet responded to the first release, the second release may overwrite the semaphore state, and the AP might miss the notification, causing it to hang or behave unpredictably. To address this: A SmmCpuSyncWaitForAPs() is added in BSP after mmCpuPlatformHookBeforeMmiHandler() and before entering SmmCoreEntry(). A matching SmmCpuSyncReleaseBsp() is added in AP immediately after its own SmmCpuPlatformHookBeforeMmiHandler() This ensures that BSP does not enter SMI handler logic or dispatch any AP-related requests before all APs are confirmed to be idle and ready. Debug sync point markers (e.g., /// #6, #7) are updated accordingly. This change eliminates a subtle but critical race condition in multi-processor/multi-socket systems during SMM entry and improves overall synchronization safety. Signed-off-by: Wei6 Xu <wei6.xu@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refer to intel-tdx-module-api spec section 5.5.21, GPR mask
(TDVMCALL_EXPOSE_REGS_MASK) is a bitmap that controls which
part of the guest TD GPR and XMM state is passed as-is to
the VMM and back.
is saved by the Intel TDX module and not passed as-is to
Host VMM.
is passed as-is to the host VMM.
Currently, RBX is used as the mailbox address in ApRunLoop.nasm,
the corresponding bit value of RBX in MASK(Bit 3) is set as 1 which
means the value is passed to Host VMM as-is and it can be changed by
Host VMM.
So the bitmask shall be set as 0 to avoid this situation.
Reference:
[TDX-API]: intel-tdx-module-abi-spec
https://cdrdv2.intel.com/v1/dl/getContent/733579
Cc: Erdem Aktas erdemaktas@google.com
Cc: Jiewen Yao jiewen.yao@intel.com
Cc: Min Xu min.m.xu@intel.com
Cc: Gerd Hoffmann kraxel@redhat.com
Cc: Elena Reshetova elena.reshetova@intel.com
Description
<Include a description of the change and why this change was made.>
<For each item, place an "x" in between
[and]if true. Example:[x](you can also check items in GitHub UI)><Create the PR as a Draft PR if it is only created to run CI checks.>
<Delete lines in <> tags before creating the PR.>
How This Was Tested
Tested on intel platform without problem.
Integration Instructions
<Describe how these changes should be integrated. Use N/A if nothing is required.>