This allows the PEI core to report the maximum temporary SEC/PEI stack
usage on the DEBUG_INFO level, in the PeiCheckAndSwitchStack() function
[MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c]:
* Normal boot:
> Temp Stack : BaseAddress=0x814000 Length=0x4000
> Temp Heap : BaseAddress=0x810000 Length=0x4000
> Total temporary memory: 32768 bytes.
> temporary memory stack ever used: 3664 bytes. <----
> temporary memory heap used for HobList: 5904 bytes.
> temporary memory heap occupied by memory pages: 0 bytes.
* S3 resume (with PEI decompression / SMM):
> Temp Stack : BaseAddress=0x814000 Length=0x4000
> Temp Heap : BaseAddress=0x810000 Length=0x4000
> Total temporary memory: 32768 bytes.
> temporary memory stack ever used: 3428 bytes. <----
> temporary memory heap used for HobList: 4816 bytes.
> temporary memory heap occupied by memory pages: 0 bytes.
I unit-tested this change by transitorily adding an infinite loop right
after the "rep stosd", and dumping the guest's temp SEC/PEI RAM (32KB
currently) while the guest was stuck in the loop. The dump includes one
dword from before and after the temp SEC/PEI RAM:
> $ virsh qemu-monitor-command GUEST_NAME --hmp 'xp /8194wx 0x80FFFC'
>
> 000000000080fffc: 0x00000000 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5
> 000000000081000c: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5
> ...
> 0000000000817fec: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5
> 0000000000817ffc: 0x5aa55aa5 0x00000000
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=747
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/Sec/SecMain.inf | 1 +
OvmfPkg/Sec/Ia32/SecEntry.nasm | 13 +++++++++++++
2 files changed, 14 insertions(+)
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 711b59530907..6051cb3c6c4c 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -71,6 +71,7 @@ [Pcd]
gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
+ gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack
[FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
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
;
; @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
+ cld ; store from base up
+ rep stosd
+
;
; Setup parameters and call SecCoreStartupWithStack
; [esp] return address for call
--
2.14.1.3.gb7cf6e02401b
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 10 November 2017 at 15:49, Laszlo Ersek <lersek@redhat.com> wrote: > This allows the PEI core to report the maximum temporary SEC/PEI stack > usage on the DEBUG_INFO level, in the PeiCheckAndSwitchStack() function > [MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c]: > > * Normal boot: > >> Temp Stack : BaseAddress=0x814000 Length=0x4000 >> Temp Heap : BaseAddress=0x810000 Length=0x4000 >> Total temporary memory: 32768 bytes. >> temporary memory stack ever used: 3664 bytes. <---- >> temporary memory heap used for HobList: 5904 bytes. >> temporary memory heap occupied by memory pages: 0 bytes. > > * S3 resume (with PEI decompression / SMM): > >> Temp Stack : BaseAddress=0x814000 Length=0x4000 >> Temp Heap : BaseAddress=0x810000 Length=0x4000 >> Total temporary memory: 32768 bytes. >> temporary memory stack ever used: 3428 bytes. <---- >> temporary memory heap used for HobList: 4816 bytes. >> temporary memory heap occupied by memory pages: 0 bytes. > > I unit-tested this change by transitorily adding an infinite loop right > after the "rep stosd", and dumping the guest's temp SEC/PEI RAM (32KB > currently) while the guest was stuck in the loop. The dump includes one > dword from before and after the temp SEC/PEI RAM: > >> $ virsh qemu-monitor-command GUEST_NAME --hmp 'xp /8194wx 0x80FFFC' >> >> 000000000080fffc: 0x00000000 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 >> 000000000081000c: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 >> ... >> 0000000000817fec: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 >> 0000000000817ffc: 0x5aa55aa5 0x00000000 > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Ruiyu Ni <ruiyu.ni@intel.com> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=747 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > OvmfPkg/Sec/SecMain.inf | 1 + > OvmfPkg/Sec/Ia32/SecEntry.nasm | 13 +++++++++++++ > 2 files changed, 14 insertions(+) > > diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf > index 711b59530907..6051cb3c6c4c 100644 > --- a/OvmfPkg/Sec/SecMain.inf > +++ b/OvmfPkg/Sec/SecMain.inf > @@ -71,6 +71,7 @@ [Pcd] > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress > gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd > + gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack > > [FeaturePcd] > gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire > 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 What does this mean? Does it belong in this patch? (Knowing you, and noticing that the next patch adds it to the x86 version of this code as well, I am sure it probably does, but I just need you to explain it to me :-)) > ; > ; @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 > + cld ; store from base up > + rep stosd > + > ; > ; Setup parameters and call SecCoreStartupWithStack > ; [esp] return address for call > -- > 2.14.1.3.gb7cf6e02401b > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 11/10/17 16:56, Ard Biesheuvel wrote: > On 10 November 2017 at 15:49, Laszlo Ersek <lersek@redhat.com> wrote: >> This allows the PEI core to report the maximum temporary SEC/PEI stack >> usage on the DEBUG_INFO level, in the PeiCheckAndSwitchStack() function >> [MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c]: >> >> * Normal boot: >> >>> Temp Stack : BaseAddress=0x814000 Length=0x4000 >>> Temp Heap : BaseAddress=0x810000 Length=0x4000 >>> Total temporary memory: 32768 bytes. >>> temporary memory stack ever used: 3664 bytes. <---- >>> temporary memory heap used for HobList: 5904 bytes. >>> temporary memory heap occupied by memory pages: 0 bytes. >> >> * S3 resume (with PEI decompression / SMM): >> >>> Temp Stack : BaseAddress=0x814000 Length=0x4000 >>> Temp Heap : BaseAddress=0x810000 Length=0x4000 >>> Total temporary memory: 32768 bytes. >>> temporary memory stack ever used: 3428 bytes. <---- >>> temporary memory heap used for HobList: 4816 bytes. >>> temporary memory heap occupied by memory pages: 0 bytes. >> >> I unit-tested this change by transitorily adding an infinite loop right >> after the "rep stosd", and dumping the guest's temp SEC/PEI RAM (32KB >> currently) while the guest was stuck in the loop. The dump includes one >> dword from before and after the temp SEC/PEI RAM: >> >>> $ virsh qemu-monitor-command GUEST_NAME --hmp 'xp /8194wx 0x80FFFC' >>> >>> 000000000080fffc: 0x00000000 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 >>> 000000000081000c: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 >>> ... >>> 0000000000817fec: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 >>> 0000000000817ffc: 0x5aa55aa5 0x00000000 >> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=747 >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> OvmfPkg/Sec/SecMain.inf | 1 + >> OvmfPkg/Sec/Ia32/SecEntry.nasm | 13 +++++++++++++ >> 2 files changed, 14 insertions(+) >> >> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf >> index 711b59530907..6051cb3c6c4c 100644 >> --- a/OvmfPkg/Sec/SecMain.inf >> +++ b/OvmfPkg/Sec/SecMain.inf >> @@ -71,6 +71,7 @@ [Pcd] >> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress >> gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd >> + gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack >> >> [FeaturePcd] >> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire >> 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 > > What does this mean? Does it belong in this patch? (Knowing you, and > noticing that the next patch adds it to the x86 version of this code > as well, I am sure it probably does, but I just need you to explain it > to me :-)) See the STOSD instruction in the Intel SDM, or just on the web: http://x86.renejeschke.de/html/file_module_x86_id_306.html > Stores a byte, word, or doubleword from the AL, AX, or EAX register, > respectively, into the destination operand. The destination operand is > a memory location, the address of which is read from either the ES:EDI > or the ES:DI registers (depending on the address-size attribute of the > instruction, 32 or 16, respectively). The ES segment cannot be > overridden with a segment override prefix. It means that whatever we put in EDI, it will be relative to the segment "identified by" ES. (See the code comment near "mov edi": "base address, relative to ES".) Above I put "identified by" in quotes, because the definition of "identified by" depends on the mode the processor is in. (Instead of a botched attempt at writing up x86 segmentation (plus, optionally, paging) generally :) , I'll just leave these references here: * Intel SDM: 2.2 MODES OF OPERATION; Figure 2-3. Transitions Among the Processor's Operating Modes * https://en.wikipedia.org/wiki/Unreal_mode * https://en.wikipedia.org/wiki/X86_memory_segmentation#80286_protected_mode * https://en.wikipedia.org/wiki/X86_memory_segmentation#80386_protected_mode ) So using STOSD and ES:EDI as an example, here's a hugely inaccurate list of possible address calculations, dependent on processor mode: * real mode: physical_address = ES * 0x10 + DI E.g., A000:1234 means physical address A1234. * big real mode (segmentation enabled, paging disabled): physical_address = GDT[ES].base_addr + EDI; Basically, the value in ES is a "selector"; an offset into the GDT (Global Descriptor Table). The base address of the GDT has been loaded into the GDTR with the LGDT instruction. Then, at the offset pointed-to by ES into the GDT, we find a segment descriptor entry, which provides the base address of the segment (and some other characteristics). The physical address is relative to that base. * 80386 protected mode (segmentation and paging): virtual_address = GDT[ES].base_addr + EDI; physical_address = paging(virtual_address); Where in paging(), the CPU can use a cached translation from the TLB, or perform a page table walk. In the code at hand, we are in "big real mode" (also called unreal mode, 32-bit flat mode). Segmentation is enabled, paging is not. The segment selector in ES is an "implicit" parameter for the code being added, because the STOSD instruction relies on ES. We move from real mode to big real mode earlier in UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm > %define SEC_DEFAULT_CR0 0x40000023 <---- bit 31, "Paging Enabled", is clear > bit 0, "Protection Enable", is set, it will enable segmentation > %define SEC_DEFAULT_CR4 0x640 > > BITS 16 > > ; > ; Modified: EAX, EBX > ; > TransitionFromReal16To32BitFlat: > > debugShowPostCode POSTCODE_16BIT_MODE > > cli > > mov bx, 0xf000 > mov ds, bx > > mov bx, ADDR16_OF(gdtr) > > o32 lgdt [cs:bx] <---- loads the GDT's address into GDTR > > mov eax, SEC_DEFAULT_CR0 > mov cr0, eax > > jmp LINEAR_CODE_SEL:dword ADDR_OF(jumpTo32BitAndLandHere) <--- actual mode switch > BITS 32 > jumpTo32BitAndLandHere: > > mov eax, SEC_DEFAULT_CR4 > mov cr4, eax > > debugShowPostCode POSTCODE_32BIT_MODE > > mov ax, LINEAR_SEL > mov ds, ax > mov es, ax <---- this is where ES gets the LINEAR_SEL offset into the GDT > mov fs, ax > mov gs, ax > mov ss, ax > > OneTimeCallRet TransitionFromReal16To32BitFlat The LINEAR_SEL offset is defined lower down in the same file, where the actual contents of that segment descriptor are defined as well: > ; > ; The Global Descriptor Table (GDT) > ; > > GDT_BASE: > ; null descriptor > NULL_SEL equ $-GDT_BASE > DW 0 ; limit 15:0 > DW 0 ; base 15:0 > DB 0 ; base 23:16 > DB 0 ; sys flag, dpl, type > DB 0 ; limit 19:16, flags > DB 0 ; base 31:24 > > ; linear data segment descriptor > LINEAR_SEL equ $-GDT_BASE > DW 0xffff ; limit 15:0 > DW 0 ; base 15:0 > DB 0 ; base 23:16 > DB PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(DATA32_TYPE) > DB GRANULARITY_FLAG(1)|DEFAULT_SIZE32(1)|CODE64_FLAG(0)|UPPER_LIMIT(0xf) > DB 0 ; base 31:24 > > [...] The point is that this descriptor defines a segment that is based at address 0, and limited at address 4GB. (Because "limit", including UPPER_LIMIT(0xf), gives 0xf_ffff, and GRANULARITY_FLAG(1) means the limit is expressed in 4KB pages). So, I added the "@param[in] ES" comment to show that I was aware of: - being in unreal mode (in fact the file being patched itself states "Processor is in flat protected mode"), - STOSD using ES, - ES having the selector value LINEAR_SEL, - the segment descriptor pointed-to by LINEAR_SEL defining a 0-based 32-bit segment, meant for "read/write data". In other words, that I was allowed to use EDI as a plain, zero-based physical address. Thanks, Laszlo >> ; >> ; @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 >> + cld ; store from base up >> + rep stosd >> + >> ; >> ; Setup parameters and call SecCoreStartupWithStack >> ; [esp] return address for call >> -- >> 2.14.1.3.gb7cf6e02401b >> >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 11/10/17 19:11, Laszlo Ersek wrote:
> On 11/10/17 16:56, Ard Biesheuvel wrote:
>> On 10 November 2017 at 15:49, Laszlo Ersek <lersek@redhat.com> wrote:
>>> This allows the PEI core to report the maximum temporary SEC/PEI stack
>>> usage on the DEBUG_INFO level, in the PeiCheckAndSwitchStack() function
>>> [MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c]:
>>>
>>> * Normal boot:
>>>
>>>> Temp Stack : BaseAddress=0x814000 Length=0x4000
>>>> Temp Heap : BaseAddress=0x810000 Length=0x4000
>>>> Total temporary memory: 32768 bytes.
>>>> temporary memory stack ever used: 3664 bytes. <----
>>>> temporary memory heap used for HobList: 5904 bytes.
>>>> temporary memory heap occupied by memory pages: 0 bytes.
>>>
>>> * S3 resume (with PEI decompression / SMM):
>>>
>>>> Temp Stack : BaseAddress=0x814000 Length=0x4000
>>>> Temp Heap : BaseAddress=0x810000 Length=0x4000
>>>> Total temporary memory: 32768 bytes.
>>>> temporary memory stack ever used: 3428 bytes. <----
>>>> temporary memory heap used for HobList: 4816 bytes.
>>>> temporary memory heap occupied by memory pages: 0 bytes.
>>>
>>> I unit-tested this change by transitorily adding an infinite loop right
>>> after the "rep stosd", and dumping the guest's temp SEC/PEI RAM (32KB
>>> currently) while the guest was stuck in the loop. The dump includes one
>>> dword from before and after the temp SEC/PEI RAM:
>>>
>>>> $ virsh qemu-monitor-command GUEST_NAME --hmp 'xp /8194wx 0x80FFFC'
>>>>
>>>> 000000000080fffc: 0x00000000 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5
>>>> 000000000081000c: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5
>>>> ...
>>>> 0000000000817fec: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5
>>>> 0000000000817ffc: 0x5aa55aa5 0x00000000
>>>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=747
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>> OvmfPkg/Sec/SecMain.inf | 1 +
>>> OvmfPkg/Sec/Ia32/SecEntry.nasm | 13 +++++++++++++
>>> 2 files changed, 14 insertions(+)
>>>
>>> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
>>> index 711b59530907..6051cb3c6c4c 100644
>>> --- a/OvmfPkg/Sec/SecMain.inf
>>> +++ b/OvmfPkg/Sec/SecMain.inf
>>> @@ -71,6 +71,7 @@ [Pcd]
>>> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress
>>> gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack
>>>
>>> [FeaturePcd]
>>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
>>> 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
>>
>> What does this mean? Does it belong in this patch? (Knowing you, and
>> noticing that the next patch adds it to the x86 version of this code
>> as well, I am sure it probably does, but I just need you to explain it
>> to me :-))
>
> See the STOSD instruction in the Intel SDM, or just on the web:
>
> http://x86.renejeschke.de/html/file_module_x86_id_306.html
>
>> Stores a byte, word, or doubleword from the AL, AX, or EAX register,
>> respectively, into the destination operand. The destination operand is
>> a memory location, the address of which is read from either the ES:EDI
>> or the ES:DI registers (depending on the address-size attribute of the
>> instruction, 32 or 16, respectively). The ES segment cannot be
>> overridden with a segment override prefix.
>
> It means that whatever we put in EDI, it will be relative to the segment
> "identified by" ES. (See the code comment near "mov edi": "base address,
> relative to ES".)
>
> Above I put "identified by" in quotes, because the definition of
> "identified by" depends on the mode the processor is in.
>
> (Instead of a botched attempt at writing up x86 segmentation (plus,
> optionally, paging) generally :) , I'll just leave these references
> here:
>
> * Intel SDM: 2.2 MODES OF OPERATION; Figure 2-3. Transitions Among the
> Processor's Operating Modes
>
> * https://en.wikipedia.org/wiki/Unreal_mode
>
> * https://en.wikipedia.org/wiki/X86_memory_segmentation#80286_protected_mode
>
> * https://en.wikipedia.org/wiki/X86_memory_segmentation#80386_protected_mode
> )
>
> So using STOSD and ES:EDI as an example, here's a hugely inaccurate list
> of possible address calculations, dependent on processor mode:
>
> * real mode:
>
> physical_address = ES * 0x10 + DI
>
> E.g., A000:1234 means physical address A1234.
>
> * big real mode (segmentation enabled, paging disabled):
>
> physical_address = GDT[ES].base_addr + EDI;
>
> Basically, the value in ES is a "selector"; an offset into the GDT
> (Global Descriptor Table). The base address of the GDT has been loaded
> into the GDTR with the LGDT instruction. Then, at the offset
> pointed-to by ES into the GDT, we find a segment descriptor entry,
> which provides the base address of the segment (and some other
> characteristics). The physical address is relative to that base.
>
> * 80386 protected mode (segmentation and paging):
>
> virtual_address = GDT[ES].base_addr + EDI;
> physical_address = paging(virtual_address);
>
> Where in paging(), the CPU can use a cached translation from the TLB,
> or perform a page table walk.
>
> In the code at hand, we are in "big real mode" (also called unreal mode,
> 32-bit flat mode). Segmentation is enabled, paging is not. The segment
> selector in ES is an "implicit" parameter for the code being added,
> because the STOSD instruction relies on ES.
>
> We move from real mode to big real mode earlier in
>
> UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
>
>> %define SEC_DEFAULT_CR0 0x40000023 <---- bit 31, "Paging Enabled", is clear
>> bit 0, "Protection Enable", is set, it will enable segmentation
>> %define SEC_DEFAULT_CR4 0x640
>>
>> BITS 16
>>
>> ;
>> ; Modified: EAX, EBX
>> ;
>> TransitionFromReal16To32BitFlat:
>>
>> debugShowPostCode POSTCODE_16BIT_MODE
>>
>> cli
>>
>> mov bx, 0xf000
>> mov ds, bx
>>
>> mov bx, ADDR16_OF(gdtr)
>>
>> o32 lgdt [cs:bx] <---- loads the GDT's address into GDTR
>>
>> mov eax, SEC_DEFAULT_CR0
>> mov cr0, eax
>>
>> jmp LINEAR_CODE_SEL:dword ADDR_OF(jumpTo32BitAndLandHere) <--- actual mode switch
>> BITS 32
>> jumpTo32BitAndLandHere:
>>
>> mov eax, SEC_DEFAULT_CR4
>> mov cr4, eax
>>
>> debugShowPostCode POSTCODE_32BIT_MODE
>>
>> mov ax, LINEAR_SEL
>> mov ds, ax
>> mov es, ax <---- this is where ES gets the LINEAR_SEL offset into the GDT
>> mov fs, ax
>> mov gs, ax
>> mov ss, ax
>>
>> OneTimeCallRet TransitionFromReal16To32BitFlat
>
> The LINEAR_SEL offset is defined lower down in the same file, where the
> actual contents of that segment descriptor are defined as well:
>
>> ;
>> ; The Global Descriptor Table (GDT)
>> ;
>>
>> GDT_BASE:
>> ; null descriptor
>> NULL_SEL equ $-GDT_BASE
>> DW 0 ; limit 15:0
>> DW 0 ; base 15:0
>> DB 0 ; base 23:16
>> DB 0 ; sys flag, dpl, type
>> DB 0 ; limit 19:16, flags
>> DB 0 ; base 31:24
>>
>> ; linear data segment descriptor
>> LINEAR_SEL equ $-GDT_BASE
>> DW 0xffff ; limit 15:0
>> DW 0 ; base 15:0
>> DB 0 ; base 23:16
>> DB PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(DATA32_TYPE)
>> DB GRANULARITY_FLAG(1)|DEFAULT_SIZE32(1)|CODE64_FLAG(0)|UPPER_LIMIT(0xf)
>> DB 0 ; base 31:24
>>
>> [...]
>
> The point is that this descriptor defines a segment that is based at
> address 0, and limited at address 4GB. (Because "limit", including
> UPPER_LIMIT(0xf), gives 0xf_ffff, and GRANULARITY_FLAG(1) means the
> limit is expressed in 4KB pages).
>
> So, I added the "@param[in] ES" comment to show that I was aware of:
>
> - being in unreal mode (in fact the file being patched itself states
> "Processor is in flat protected mode"),
>
> - STOSD using ES,
>
> - ES having the selector value LINEAR_SEL,
>
> - the segment descriptor pointed-to by LINEAR_SEL defining a 0-based
> 32-bit segment, meant for "read/write data".
>
> In other words, that I was allowed to use EDI as a plain, zero-based
> physical address.
I should add that the X64 version is a bit different, as far as the
"environment" goes. In the X64 version, paging *is* enabled (we build
the page tables, and enter long (64-bit) mode, *between* the code quoted
above, and the code added in the X64 patch). However, the segment
described by ES is the same (it remains limited to 4GB). Furthermore,
the (identity-mapping) page tables are only built for the first 4GB of RAM.
Thus the address translation is more involved in the X64 variant, but
ultimately RDI can be used in the exact same sense ("plain, zero-based
physical address").
Thanks
Laszlo
>>> ;
>>> ; @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
>>> + cld ; store from base up
>>> + rep stosd
>>> +
>>> ;
>>> ; Setup parameters and call SecCoreStartupWithStack
>>> ; [esp] return address for call
>>> --
>>> 2.14.1.3.gb7cf6e02401b
>>>
>>>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 10 November 2017 at 18:11, Laszlo Ersek <lersek@redhat.com> wrote: > On 11/10/17 16:56, Ard Biesheuvel wrote: >> On 10 November 2017 at 15:49, Laszlo Ersek <lersek@redhat.com> wrote: >>> This allows the PEI core to report the maximum temporary SEC/PEI stack >>> usage on the DEBUG_INFO level, in the PeiCheckAndSwitchStack() function >>> [MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c]: >>> >>> * Normal boot: >>> >>>> Temp Stack : BaseAddress=0x814000 Length=0x4000 >>>> Temp Heap : BaseAddress=0x810000 Length=0x4000 >>>> Total temporary memory: 32768 bytes. >>>> temporary memory stack ever used: 3664 bytes. <---- >>>> temporary memory heap used for HobList: 5904 bytes. >>>> temporary memory heap occupied by memory pages: 0 bytes. >>> >>> * S3 resume (with PEI decompression / SMM): >>> >>>> Temp Stack : BaseAddress=0x814000 Length=0x4000 >>>> Temp Heap : BaseAddress=0x810000 Length=0x4000 >>>> Total temporary memory: 32768 bytes. >>>> temporary memory stack ever used: 3428 bytes. <---- >>>> temporary memory heap used for HobList: 4816 bytes. >>>> temporary memory heap occupied by memory pages: 0 bytes. >>> >>> I unit-tested this change by transitorily adding an infinite loop right >>> after the "rep stosd", and dumping the guest's temp SEC/PEI RAM (32KB >>> currently) while the guest was stuck in the loop. The dump includes one >>> dword from before and after the temp SEC/PEI RAM: >>> >>>> $ virsh qemu-monitor-command GUEST_NAME --hmp 'xp /8194wx 0x80FFFC' >>>> >>>> 000000000080fffc: 0x00000000 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 >>>> 000000000081000c: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 >>>> ... >>>> 0000000000817fec: 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 0x5aa55aa5 >>>> 0000000000817ffc: 0x5aa55aa5 0x00000000 >>> >>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>> Cc: Jordan Justen <jordan.l.justen@intel.com> >>> Cc: Ruiyu Ni <ruiyu.ni@intel.com> >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=747 >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >>> --- >>> OvmfPkg/Sec/SecMain.inf | 1 + >>> OvmfPkg/Sec/Ia32/SecEntry.nasm | 13 +++++++++++++ >>> 2 files changed, 14 insertions(+) >>> >>> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf >>> index 711b59530907..6051cb3c6c4c 100644 >>> --- a/OvmfPkg/Sec/SecMain.inf >>> +++ b/OvmfPkg/Sec/SecMain.inf >>> @@ -71,6 +71,7 @@ [Pcd] >>> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress >>> gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack >>> >>> [FeaturePcd] >>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire >>> 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 >> >> What does this mean? Does it belong in this patch? (Knowing you, and >> noticing that the next patch adds it to the x86 version of this code >> as well, I am sure it probably does, but I just need you to explain it >> to me :-)) > > See the STOSD instruction in the Intel SDM, or just on the web: > > http://x86.renejeschke.de/html/file_module_x86_id_306.html > >> Stores a byte, word, or doubleword from the AL, AX, or EAX register, >> respectively, into the destination operand. The destination operand is >> a memory location, the address of which is read from either the ES:EDI >> or the ES:DI registers (depending on the address-size attribute of the >> instruction, 32 or 16, respectively). The ES segment cannot be >> overridden with a segment override prefix. > > It means that whatever we put in EDI, it will be relative to the segment > "identified by" ES. (See the code comment near "mov edi": "base address, > relative to ES".) > > Above I put "identified by" in quotes, because the definition of > "identified by" depends on the mode the processor is in. > > (Instead of a botched attempt at writing up x86 segmentation (plus, > optionally, paging) generally :) , I'll just leave these references > here: > > * Intel SDM: 2.2 MODES OF OPERATION; Figure 2-3. Transitions Among the > Processor's Operating Modes > > * https://en.wikipedia.org/wiki/Unreal_mode > > * https://en.wikipedia.org/wiki/X86_memory_segmentation#80286_protected_mode > > * https://en.wikipedia.org/wiki/X86_memory_segmentation#80386_protected_mode > ) > > So using STOSD and ES:EDI as an example, here's a hugely inaccurate list > of possible address calculations, dependent on processor mode: > > * real mode: > > physical_address = ES * 0x10 + DI > > E.g., A000:1234 means physical address A1234. > > * big real mode (segmentation enabled, paging disabled): > > physical_address = GDT[ES].base_addr + EDI; > > Basically, the value in ES is a "selector"; an offset into the GDT > (Global Descriptor Table). The base address of the GDT has been loaded > into the GDTR with the LGDT instruction. Then, at the offset > pointed-to by ES into the GDT, we find a segment descriptor entry, > which provides the base address of the segment (and some other > characteristics). The physical address is relative to that base. > > * 80386 protected mode (segmentation and paging): > > virtual_address = GDT[ES].base_addr + EDI; > physical_address = paging(virtual_address); > > Where in paging(), the CPU can use a cached translation from the TLB, > or perform a page table walk. > > In the code at hand, we are in "big real mode" (also called unreal mode, > 32-bit flat mode). Segmentation is enabled, paging is not. The segment > selector in ES is an "implicit" parameter for the code being added, > because the STOSD instruction relies on ES. > > We move from real mode to big real mode earlier in > > UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm > >> %define SEC_DEFAULT_CR0 0x40000023 <---- bit 31, "Paging Enabled", is clear >> bit 0, "Protection Enable", is set, it will enable segmentation >> %define SEC_DEFAULT_CR4 0x640 >> >> BITS 16 >> >> ; >> ; Modified: EAX, EBX >> ; >> TransitionFromReal16To32BitFlat: >> >> debugShowPostCode POSTCODE_16BIT_MODE >> >> cli >> >> mov bx, 0xf000 >> mov ds, bx >> >> mov bx, ADDR16_OF(gdtr) >> >> o32 lgdt [cs:bx] <---- loads the GDT's address into GDTR >> >> mov eax, SEC_DEFAULT_CR0 >> mov cr0, eax >> >> jmp LINEAR_CODE_SEL:dword ADDR_OF(jumpTo32BitAndLandHere) <--- actual mode switch >> BITS 32 >> jumpTo32BitAndLandHere: >> >> mov eax, SEC_DEFAULT_CR4 >> mov cr4, eax >> >> debugShowPostCode POSTCODE_32BIT_MODE >> >> mov ax, LINEAR_SEL >> mov ds, ax >> mov es, ax <---- this is where ES gets the LINEAR_SEL offset into the GDT >> mov fs, ax >> mov gs, ax >> mov ss, ax >> >> OneTimeCallRet TransitionFromReal16To32BitFlat > > The LINEAR_SEL offset is defined lower down in the same file, where the > actual contents of that segment descriptor are defined as well: > >> ; >> ; The Global Descriptor Table (GDT) >> ; >> >> GDT_BASE: >> ; null descriptor >> NULL_SEL equ $-GDT_BASE >> DW 0 ; limit 15:0 >> DW 0 ; base 15:0 >> DB 0 ; base 23:16 >> DB 0 ; sys flag, dpl, type >> DB 0 ; limit 19:16, flags >> DB 0 ; base 31:24 >> >> ; linear data segment descriptor >> LINEAR_SEL equ $-GDT_BASE >> DW 0xffff ; limit 15:0 >> DW 0 ; base 15:0 >> DB 0 ; base 23:16 >> DB PRESENT_FLAG(1)|DPL(0)|SYSTEM_FLAG(1)|DESC_TYPE(DATA32_TYPE) >> DB GRANULARITY_FLAG(1)|DEFAULT_SIZE32(1)|CODE64_FLAG(0)|UPPER_LIMIT(0xf) >> DB 0 ; base 31:24 >> >> [...] > > The point is that this descriptor defines a segment that is based at > address 0, and limited at address 4GB. (Because "limit", including > UPPER_LIMIT(0xf), gives 0xf_ffff, and GRANULARITY_FLAG(1) means the > limit is expressed in 4KB pages). > > So, I added the "@param[in] ES" comment to show that I was aware of: > > - being in unreal mode (in fact the file being patched itself states > "Processor is in flat protected mode"), > > - STOSD using ES, > > - ES having the selector value LINEAR_SEL, > > - the segment descriptor pointed-to by LINEAR_SEL defining a 0-based > 32-bit segment, meant for "read/write data". > > In other words, that I was allowed to use EDI as a plain, zero-based > physical address. > Excellent clarification, thanks. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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?
> ;
> ; @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
> + 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 also think it would be reasonable to merge 2 & 3, but
separate is fine too.
-Jordan
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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
On 2017-11-13 10:36:45, Laszlo Ersek wrote: > 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 How does something like this sound? ; @param[out] DS Selector allowing flat access to all addresses It seems to cover 32/64 bit and get the point across. -Jordan _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 11/13/17 20:02, Jordan Justen wrote: > On 2017-11-13 10:36:45, Laszlo Ersek wrote: >> 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 > > How does something like this sound? > > ; @param[out] DS Selector allowing flat access to all addresses > > It seems to cover 32/64 bit and get the point across. Sounds good, thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.