On 11/13/17 19:25, Jordan Justen wrote:
> On 2017-11-10 07:49:06, Laszlo Ersek wrote:
>> diff --git a/OvmfPkg/Sec/Ia32/SecEntry.nasm b/OvmfPkg/Sec/Ia32/SecEntry.nasm
>> index 54d074e621f6..1d426fafa888 100644
>> --- a/OvmfPkg/Sec/Ia32/SecEntry.nasm
>> +++ b/OvmfPkg/Sec/Ia32/SecEntry.nasm
>> @@ -29,6 +29,7 @@ extern ASM_PFX(SecCoreStartupWithStack)
>> ; @param[in] EAX Initial value of the EAX register (BIST: Built-in Self
>> Test)
>> ; @param[in] DI 'BP': boot-strap processor, or 'AP': application
>> processor
>> ; @param[in] EBP Pointer to the start of the Boot Firmware Volume
>> +; @param[in] ES Set to LINEAR_SEL in TransitionFromReal16To32BitFlat
>
> Can you document all the segment registers, and also document them in
> UefiCpuPkg/ResetVector/Vtf0/Main.asm?
Do you mean the above format (i.e., @param[in]...), just repeated for
the other segment registers too?
Regarding "UefiCpuPkg/ResetVector/Vtf0/Main.asm", what format do you
suggest? The @param[in]... format wouldn't be right, because the segment
registers are set up in TransitionFromReal16To32BitFlat. Should I write
a free-form comment / list above
OneTimeCall TransitionFromReal16To32BitFlat
?
>
>> ;
>> ; @return None This routine does not return
>> ;
>> @@ -44,6 +45,18 @@ ASM_PFX(_ModuleEntryPoint):
>> mov esp, ebx
>> nop
>>
>> + ;
>> + ; Fill the temporary RAM with the initial stack value.
>> + ; The loop below will seed the heap as well, but that's harmless.
>> + ;
>> + mov eax, FixedPcdGet32 (PcdInitValueInTempStack) ; dword to store
>> + mov edi, FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) ; base address,
>> + ; relative to ES
>> + mov ecx, FixedPcdGet32 (PcdOvmfSecPeiTempRamSize) ; byte count
>> + shr ecx, 2 ; dword count
>
> I'm not sure, but I think NASM might let you do something like:
>
> mov ecx, FixedPcdGet32 (PcdOvmfSecPeiTempRamSize) / 4
OK, I can try that. I was worried about NASM arithmetic in general, but
I see the "info" page describes "Multiplication and Division", so the
semantics should be well-defined.
>
>> + cld ; store from base
>> up
>> + rep stosd
>
> I think if you move this above the code in patch 1, then patch 1 is
> not needed.
I think I agree (to be seen in practice :) )
> I also think it would be reasonable to merge 2 & 3, but
> separate is fine too.
If I remove the "shr" from both, then I might feel tempted to merge
them; I'm not sure yet. For testing at least I prefer to keep them separate.
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
