I was tracking down a Platform C state bug and I noticed the platform code was using this definition [1] for PcdCpuApTargetCstate and wrote that actual value used for Mwait. But the MpInitLIb seems to assume that PcdCpuApTargetCstate is eax[7:4] and it shifts the data left. This ends up with the value being 0 which means C1 for all calculated values.
So my question is which definition is correct? As far as I can tell eax[3:0] are sub C states and thus being able to pass that value would be useful? There is other code in the MpLib [3] that is also doing the shift so it seems intestinal. But given the PCD definition I can see how the platform code set the wrong value. [1] PCD Definition https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/UefiCpuPkg.dec <https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/UefiCpuPkg.dec> ## Specifies the AP target C-state for Mwait during POST phase. # The default value 0 means C1 state. # The value is defined as below.<BR><BR> # @Prompt The specified AP target C-state for Mwait. gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate|0|UINT8|0x00000007 [2] UefiCpuPkg code https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm <https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm> MwaitLoop: cli mov eax, esp ; Set Monitor Address xor ecx, ecx ; ecx = 0 xor edx, edx ; edx = 0 monitor mov eax, ebx ; Mwait Cx, Target C-State per eax[7:4] shl eax, 4 mwait jmp MwaitLoop https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c <https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c> AsmRelocateApLoopFunc ( MwaitSupport, CpuMpData->ApTargetCState, CpuMpData->PmCodeSegment, mReservedTopOfApStack - ProcessorNumber * AP_SAFE_STACK_SIZE, (UINTN) &mNumberToFinish ); [3] ApTargetCState shifting https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpInitLib/MpLib.c <https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/MpInitLib/MpLib.c> // // Check AP start-up signal again. // If AP start-up signal is not set, place AP into // the specified C-state // AsmMwait (CpuMpData->ApTargetCState << 4, 0); Thanks, Andrew Fish -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#45325): https://edk2.groups.io/g/devel/message/45325 Mute This Topic: https://groups.io/mt/32835440/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
