Re: [edk2-devel] [PATCH v2 08/31] OvmfPkg/XenResetVector: Allow to jumpstart from either hvmloader or PVH

["Laszlo Ersek" <lersek@redhat.com>]

On 04/09/19 13:08, Anthony PERARD wrote:
> This patch allows the ResetVector to be run indenpendently from build
> time addresses.
> 
> The goal of the patch is to avoid having to create RAM just below 4G
> when creating a Xen PVH guest while been compatible with the way

(1) s/been/being/

> hvmloader currently load OVMF, just below 4G.
> 
> Only the new PVH entry point will do the calculation.
> 
> The ResetVector will figure out its current running address by creating
> a temporary stack, make a call and calculate the difference between the
> build time address and the address at run time.
> 
> This patch copies and make the necessary modification to some other asm
> files:
> - copy of UefiCpuPkg/.../Flat32ToFlat64.asm:
>   Allow Transition32FlatTo64Flat to been runnned from anywhere in memory

(2) s/been runned/be run/

> _ copy of UefiCpuPkg/../SearchForBfvBase.asm:

(3) please replace the underscore (_) with a hyphen (-)

>   Add a extra parameter to indicate where to start the search for the
>   boot firmware volume.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  OvmfPkg/XenResetVector/Ia16/Real16ToFlat32.asm                                    |  3 ++
>  {UefiCpuPkg/ResetVector/Vtf0 => OvmfPkg/XenResetVector}/Ia32/Flat32ToFlat64.asm   | 25 ++++++++++++++--
>  {UefiCpuPkg/ResetVector/Vtf0 => OvmfPkg/XenResetVector}/Ia32/SearchForBfvBase.asm | 19 +++++++++----
>  OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm                                        | 30 ++++++++++++++++++--
>  4 files changed, 66 insertions(+), 11 deletions(-)

(4) For the subject line: please drop the word "to".

With those:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> 
> diff --git a/OvmfPkg/XenResetVector/Ia16/Real16ToFlat32.asm b/OvmfPkg/XenResetVector/Ia16/Real16ToFlat32.asm
> index e22e92c8a6..eebced6ced 100644
> --- a/OvmfPkg/XenResetVector/Ia16/Real16ToFlat32.asm
> +++ b/OvmfPkg/XenResetVector/Ia16/Real16ToFlat32.asm
> @@ -61,6 +61,9 @@ jumpTo32BitAndLandHere:
>      mov     gs, ax
>      mov     ss, ax
>  
> +    ; parameter for Flat32SearchForBfvBase
> +    xor     eax, eax ; Start searching from top of 4GB for BfvBase
> +
>      OneTimeCallRet TransitionFromReal16To32BitFlat
>  
>  ALIGN   2
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm b/OvmfPkg/XenResetVector/Ia32/Flat32ToFlat64.asm
> similarity index 69%
> copy from UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm
> copy to OvmfPkg/XenResetVector/Ia32/Flat32ToFlat64.asm
> index 5b6b375330..ca03ea43e0 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm
> +++ b/OvmfPkg/XenResetVector/Ia32/Flat32ToFlat64.asm
> @@ -3,6 +3,8 @@
>  ; Transition from 32 bit flat protected mode into 64 bit flat protected mode
>  ;
>  ; Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2019, Citrix Systems, Inc.
> +;
>  ; This program and the accompanying materials
>  ; are licensed and made available under the terms and conditions of the BSD License
>  ; which accompanies this distribution.  The full text of the license may be found at
> @@ -16,7 +18,7 @@
>  BITS    32
>  
>  ;
> -; Modified:  EAX
> +; Modified:  EAX, EBX, ECX, EDX, ESP
>  ;
>  Transition32FlatTo64Flat:
>  
> @@ -35,10 +37,29 @@ Transition32FlatTo64Flat:
>      bts     eax, 31                     ; set PG
>      mov     cr0, eax                    ; enable paging
>  
> -    jmp     LINEAR_CODE64_SEL:ADDR_OF(jumpTo64BitAndLandHere)
> +    ; backup ESP
> +    mov     ebx, esp
> +
> +    ;; recalculate delta
> +    mov     esp, PVH_SPACE(16)
> +    call    .delta
> +.delta:
> +    pop     edx
> +    sub     edx, ADDR_OF(.delta)
> +
> +    ; push return addr and seg to the stack, then return far
> +    push    dword LINEAR_CODE64_SEL
> +    mov     eax, ADDR_OF(jumpTo64BitAndLandHere)
> +    add     eax, edx ; add delta
> +    push    eax
> +    retf
> +
>  BITS    64
>  jumpTo64BitAndLandHere:
>  
> +    ; restore ESP
> +    mov     esp, ebx
> +
>      debugShowPostCode POSTCODE_64BIT_MODE
>  
>      OneTimeCallRet Transition32FlatTo64Flat
> diff --git a/UefiCpuPkg/ResetVector/Vtf0/Ia32/SearchForBfvBase.asm b/OvmfPkg/XenResetVector/Ia32/SearchForBfvBase.asm
> similarity index 83%
> copy from UefiCpuPkg/ResetVector/Vtf0/Ia32/SearchForBfvBase.asm
> copy to OvmfPkg/XenResetVector/Ia32/SearchForBfvBase.asm
> index d0c2d8c39c..0519e05601 100644
> --- a/UefiCpuPkg/ResetVector/Vtf0/Ia32/SearchForBfvBase.asm
> +++ b/OvmfPkg/XenResetVector/Ia32/SearchForBfvBase.asm
> @@ -3,6 +3,8 @@
>  ; Search for the Boot Firmware Volume (BFV) base address
>  ;
>  ; Copyright (c) 2008 - 2009, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2019, Citrix Systems, Inc.
> +;
>  ; This program and the accompanying materials
>  ; are licensed and made available under the terms and conditions of the BSD License
>  ; which accompanies this distribution.  The full text of the license may be found at
> @@ -23,22 +25,26 @@
>  BITS    32
>  
>  ;
> -; Modified:  EAX, EBX
> +; Modified:  EAX, EBX, ECX
>  ; Preserved: EDI, ESP
>  ;
> +; @param[in]   EAX  Start search from here
>  ; @param[out]  EBP  Address of Boot Firmware Volume (BFV)
>  ;
>  Flat32SearchForBfvBase:
>  
> -    xor     eax, eax
> +    mov     ecx, eax
>  searchingForBfvHeaderLoop:
>      ;
> -    ; We check for a firmware volume at every 4KB address in the top 16MB
> -    ; just below 4GB.  (Addresses at 0xffHHH000 where H is any hex digit.)
> +    ; We check for a firmware volume at every 4KB address in the 16MB
> +    ; just below where we started, ECX.
>      ;
>      sub     eax, 0x1000
> -    cmp     eax, 0xff000000
> -    jb      searchedForBfvHeaderButNotFound
> +    mov     ebx, ecx
> +    sub     ebx, eax
> +    cmp     ebx, 0x01000000
> +    ; if ECX-EAX > 16MB; jump notfound
> +    ja      searchedForBfvHeaderButNotFound
>  
>      ;
>      ; Check FFS GUID
> @@ -59,6 +65,7 @@ searchingForBfvHeaderLoop:
>      jne     searchingForBfvHeaderLoop
>      mov     ebx, eax
>      add     ebx, dword [eax + 0x20]
> +    cmp     ebx, ecx
>      jnz     searchingForBfvHeaderLoop
>  
>      jmp     searchedForBfvHeaderAndItWasFound
> diff --git a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
> index 4e55b0ac1f..612b2e9c44 100644
> --- a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
> +++ b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
> @@ -19,22 +19,39 @@ BITS    32
>  xenPVHMain:
>      mov     di, 'BP'
>  
> -    ; ESP -  Initial value of the EAX register (BIST: Built-in Self Test)
> -    mov     esp, eax
> +    ; EBP -  Initial value of the EAX register (BIST: Built-in Self Test)
> +    mov     ebp, eax
>  
>      ;; Store "Start of day" struct pointer for later use
>      mov     dword[PVH_SPACE (0)], ebx
>      mov     dword[PVH_SPACE (4)], 'XPVH'
>  
> +    ;; calculate delta between build-addr and run position
> +    mov     esp, PVH_SPACE(16)          ; create a temporary stack
> +    call    .delta
> +.delta:
> +    pop     edx                         ; get addr of .delta
> +    sub     edx, ADDR_OF(.delta)        ; calculate delta
> +
>      cli
>  
> +    ;; Find address of GDT and gdtr and fix the later
>      mov     ebx, ADDR_OF(gdtr)
> +    add     ebx, edx                    ; add delta gdtr
> +    mov     eax, ADDR_OF(GDT_BASE)
> +    add     eax, edx                    ; add delta to GDT_BASE
> +    mov     dword[ebx + 2], eax         ; fix GDT_BASE addr in gdtr
>      lgdt    [ebx]
>  
>      mov     eax, SEC_DEFAULT_CR0
>      mov     cr0, eax
>  
> -    jmp     LINEAR_CODE_SEL:ADDR_OF(.jmpToNewCodeSeg)
> +    ;; push return addr to the stack, then return far
> +    push    dword LINEAR_CODE_SEL       ; segment to select
> +    mov     eax, ADDR_OF(.jmpToNewCodeSeg) ; return addr
> +    add     eax, edx                    ; add delta to return addr
> +    push    eax
> +    retf
>  .jmpToNewCodeSeg:
>  
>      mov     eax, SEC_DEFAULT_CR4
> @@ -47,5 +64,12 @@ xenPVHMain:
>      mov     gs, ax
>      mov     ss, ax
>  
> +    ; ESP -  Initial value of the EAX register (BIST: Built-in Self Test)
> +    mov     esp, ebp
> +
> +    ; parameter for Flat32SearchForBfvBase
> +    mov     eax, ADDR_OF(fourGigabytes)
> +    add     eax, edx ; add delta
> +
>      ; return to the Main16
>      OneTimeCallRet TransitionFromReal16To32BitFlat
>