Hi Ray,
Please check V4 patch series.
Thanks for the feedbacks.
Yuanhao
> + OriginalValue = InterlockedCompareExchange32 (
> + (UINT32 *)ApStartupSignalBuffer,
> + MP_HAND_OFF_SIGNAL,
> + 0
> + );
> + if (OriginalValue == MP_HAND_OFF_SIGNAL) {
> + SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateReady);
> + }
1. I don't think InterlockedCompareExchange32() is needed. But it's consistent
with the following code. Ok to me.
A: I keep this in V4 patch series.
> +
> InterlockedCompareExchange32 (
> (UINT32 *)ApStartupSignalBuffer,
> WAKEUP_AP_SIGNAL,
> @@ -887,6 +897,32 @@ ApWakeupFunction (
> }
> }
>
> +DxeApEntryPoint (
> + CPU_MP_DATA *CpuMpData
> + )
> +{
> + UINTN ProcessorNumber;
> +
> + GetProcessorNumber (CpuMpData, &ProcessorNumber);
> + InterlockedIncrement ((UINT32 *)&CpuMpData->FinishedCount);
> + RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters,
> + FALSE); PlaceAPInMwaitLoopOrRunLoop (
> + CpuMpData->ApLoopMode,
> + CpuMpData->CpuData[ProcessorNumber].StartupApSignal,
> + CpuMpData->ApTargetCState
> + );
2. CpuMpData->ApLoopMode is set to PcdGet8 (PcdCpuApLoopMode) in DXE phase.
It's possible that when building the Dxe instance of the library,
PcdCpuApLoopMode is ApInHltLoop.
Then above code should not expect the CpuMpData->ApLoopMode is either
MwaitLoop or RunLoop.
But, if the CPU runs here, PcdCpuApLoopMode in PEI phase should not be
APInHltLoop.
So the question becomes: Shall MpInitLib support different
PcdCpuApLoopMode values?
I prefer no for keeping the configuration simple.
Then, can you please add an assertion before calling SwitchApContext()?
(see comments below)
A: I added assertion in V4 patch series.
> + if (MpHandOff->WaitLoopExecutionMode == sizeof (VOID *)) {
3. can you add "ASSERT (CpuMpData->ApLoopMode != APInHltLoop)" here?
> + //
> + // In scenarios where both the PEI and DXE phases run in the same
> + // execution mode (32bit or 64bit), the BSP triggers
> + // a start-up signal during the DXE phase to wake up the APs. This
> causes any
> + // APs that are currently in a loop on the memory prepared during the
> PEI
> + // phase to awaken and run the SwitchContextPerAp procedure.
> + This
> procedure
> + // enables the APs to switch to a different memory section and
> continue their
> + // looping process there.
> + //
> + CpuMpData->FinishedCount = 0;
> + CpuMpData->InitFlag = ApInitDone;
> + SaveCpuMpData (CpuMpData);
> + SwitchApContext (MpHandOff);
> + ASSERT (CpuMpData->FinishedCount == (CpuMpData->CpuCount - 1));
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106465): https://edk2.groups.io/g/devel/message/106465
Mute This Topic: https://groups.io/mt/99782484/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
