On 10/26/15 17:41, Laszlo Ersek wrote:
> On 10/26/15 14:47, Jeff Fan wrote:
>> Add CPU AP hlt-loop code into startup code structure which is located in ACPI
>> NVS range and will be safe during OS booting.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jeff Fan <jeff....@intel.com>
>> CC: Michael Kinney <michael.d.kin...@intel.com>
>> CC: Laszlo Ersek <ler...@redhat.com>
>> ---
>> UefiCpuPkg/CpuDxe/ApStartup.c | 12 ++++++++++++
>> UefiCpuPkg/CpuDxe/CpuMp.h | 8 ++++++++
>> 2 files changed, 20 insertions(+)
>
> I think the word "run" should be dropped from the patch subject.
>
> Other than that it *appears* sane to me. I didn't verify the binary
> opcodes in mCpuApRunHltLoopCodeTemplate.
Actually, I did now. Are you sure the binary is correct?
$ hexdump -C hlt-loop
00000000 fa f4 eb fa |úôëú|
00000004
ndisasm decompiles this to:
00000000 FA cli
00000001 F4 hlt
00000002 EBFA jmp short 0xfffe
I don't think that's right. I tried:
> BITS 16
>
> loop:
> cli
> hlt
> jmp loop
and the output is:
00000000 FA cli
00000001 F4 hlt
00000002 EBFC jmp short 0x0
That is, the relative jump offset is encoded as 0xFC, not 0xFA. (This
seems to be the case independently of BITS 16 vs. BITS 32.)
Thanks!
Laszlo
> Also, it doesn't work (more
> about that in the next patch), but I don't know if that's a problem with
> this patch or the next one.
>
> The general logic is of course sane here.
>
> Thanks
> Laszlo
>
>>
>> diff --git a/UefiCpuPkg/CpuDxe/ApStartup.c b/UefiCpuPkg/CpuDxe/ApStartup.c
>> index 77476ae..7449889 100644
>> --- a/UefiCpuPkg/CpuDxe/ApStartup.c
>> +++ b/UefiCpuPkg/CpuDxe/ApStartup.c
>> @@ -36,6 +36,12 @@ ENABLE_EXECUTE_DISABLE_CODE
>> mEnableExecuteDisableCodeTemplate = {
>> #endif
>> };
>>
>> +CPU_AP_RUN_HLT_LOOP_CODE mCpuApRunHltLoopCodeTemplate = {
>> + 0xFA, // cli (Clear Interrupts)
>> + 0xF4, // hlt
>> + {0xEB, 0xFA} // jmp $-2
>> +};
>> +
>> /**
>> This .asm code used for translating processor from 16 bit real mode into
>> 64 bit long mode. which help to create the mStartupCodeTemplate value.
>> @@ -317,6 +323,12 @@ PrepareAPStartupCode (
>> StartupCode->EnableExecuteDisable.Cr3Value = (UINT32) AsmReadCr3 ();
>> #endif
>>
>> + CopyMem (
>> + (VOID*) &StartupCode->CpuApRunHltLoopCode,
>> + &mCpuApRunHltLoopCodeTemplate,
>> + sizeof (CPU_AP_RUN_HLT_LOOP_CODE)
>> + );
>> +
>> return EFI_SUCCESS;
>> }
>>
>> diff --git a/UefiCpuPkg/CpuDxe/CpuMp.h b/UefiCpuPkg/CpuDxe/CpuMp.h
>> index feca3be..907e993 100644
>> --- a/UefiCpuPkg/CpuDxe/CpuMp.h
>> +++ b/UefiCpuPkg/CpuDxe/CpuMp.h
>> @@ -46,6 +46,12 @@ typedef struct {
>> } ENABLE_EXECUTE_DISABLE_CODE;
>>
>> typedef struct {
>> + UINT8 Cli; // cli
>> + UINT8 Hlt; // hlt
>> + UINT8 JmpToHlt[2]; // jmp $-2
>> +} CPU_AP_RUN_HLT_LOOP_CODE;
>> +
>> +typedef struct {
>> UINT8 JmpToCli[2];
>>
>> UINT16 GdtLimit;
>> @@ -117,6 +123,8 @@ typedef struct {
>> #endif
>> UINT8 JmpToCpuDxeEntry[2];
>>
>> + CPU_AP_RUN_HLT_LOOP_CODE CpuApRunHltLoopCode;
>> +
>> } STARTUP_CODE;
>>
>> #pragma pack()
>>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
