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

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Thu, Feb 02, 2017 at 07:25:32PM +0000, 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 */

s/with/without/?

And one nitpick. Please be consistent and use EFI instead of efi everywhere.

Otherwise, LGTM.

Reviewed-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>

Daniel

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