Re: [Xen-devel] [PATCH] xen/arm64: Don't zero BSS when booting using EFI

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 2 Feb 2017, Julien Grall wrote:
> Commit 146786b "efi: create efi_enabled()" introduced a variable
> efi_flags stored in BSS and used to pass information between the stub
> and Xen. However on ARM, BSS is zeroed after the stub has finished to
> run and before Xen is started. This means that the bits set in efi_flags
> will be lost.
> 
> We were not affected before because all the variables used to pass
> information between Xen and the stub are living in initdata or data.
> 
> Looking at the description of the field SizeOfRawData in the PE/COFF
> header (see [1]):
> 
> "If this is less than VirtualSize, the remainder of the section is
> zero-filled. Because the SizeOfRawData field is rounded but the
> VirtualSize field is not, it is possible for SizeOfRawData to be greater
> than VirtualSize as well. When a section contains only uninitialized
> data, this field should be zero."
> 
> Both VirtualSize and SizeOfRawData are correctly set in the header (see
> arch/arm/arm64/head.S) so the EFI firmware will zero BSS for us.
> 
> Therefore we don't need to zero BSS before running the EFI stub and can
> skip the one between the EFI stub and Xen.
> 
> To avoid another branch instruction, slightly refactor the code. The
> register x26 is allocated to hold whether BSS is skipped. The value will
> be:
>     - 0 when the code is running on CPU0 and EFI is not used
>     - 1 when EFI is used or running on other processor than the boot one.
> 
> [1] 
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms680547(v=vs.85).aspx
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> 
> ---
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
>     This patch fix ACPI boot on Xen ARM. Without it Xen thinks it is not
>     running on EFI and will not try to find the RDSP.
> ---
>  xen/arch/arm/arm64/head.S | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 3f63d2a..8cb4602 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -65,7 +65,7 @@
>   *  x23 - UART address
>   *  x24 - cpuid
>   *  x25 - identity map in place
> - *  x26 -
> + *  x26 - skip_zero_bss
>   *  x27 -
>   *  x28 -
>   *  x29 -
> @@ -232,6 +232,10 @@ section_table:
>          .long   0xe0500020       /* Characteristics (section flags) */
>          .align  5
>  real_start:
> +        /* BSS should be zeroed when booting with efi */

Do you mean "without"?

Aside from that:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> +        mov   x26, #0                /* x26 := skip_zero_bss */
> +
> +real_start_efi:
>          msr   DAIFSet, 0xf           /* Disable all interrupts */
>  
>          /* Save the bootloader arguments in less-clobberable registers */
> @@ -261,6 +265,8 @@ GLOBAL(init_secondary)
>          sub   x20, x19, x0           /* x20 := phys-offset */
>  
>          mov   x22, #1                /* x22 := is_secondary_cpu */
> +        /* Skip zero BSS on secondary CPUs to avoid nasty surprises. */
> +        mov   x26, #1                /* X26 := skip_zero_bss */
>  
>  common_start:
>          mov   x24, #0                /* x24 := CPU ID. Initialy zero until we
> @@ -314,8 +320,8 @@ common_start:
>  
>  el2:    PRINT("- Xen starting at EL2 -\r\n")
>  
> -        /* Zero BSS On the boot CPU to avoid nasty surprises */
> -        cbnz  x22, skip_bss
> +        /* Zero BSS only when requested to avoid nasty surprises. */
> +        cbnz  x26, skip_bss
>  
>          PRINT("- Zero BSS -\r\n")
>          ldr   x0, =__bss_start       /* Load start & end of bss */
> @@ -787,7 +793,16 @@ ENTRY(efi_xen_start)
>          mov   x1, xzr
>          mov   x2, xzr
>          mov   x3, xzr
> -        b     real_start
> +        /*
> +         * The EFI stub and Xen may share some information living in
> +         * BSS. Don't zero BSS to avoid loosing them.
> +         *
> +         * Note that the EFI firmware has already zeroed BSS for us
> +         * before jump into the stub.
> +         */
> +        mov   x26, #1               /* x26 := skip_zero_bss */
> +
> +        b     real_start_efi
>  ENDPROC(efi_xen_start)
>  
>  /*

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel