Re: [PATCH v6 03/11] xen/arm: disable EFI boot services for MPU systems

[Julien Grall <julien@xxxxxxx>]

Hi Wei,

On 04/11/2022 10:07, Wei Chen wrote:
> Current EFI boot services support of Arm64 could not
> work well for Armv8-R64 system that only has MPU in
> EL2. That is because EFI boot services may need some
> relocation support or partial PIE/PIC support.
I am a bit confused with argument. We have nothing in Xen today to deal 
with relocation/partial PIE/PIC support. So what is the exact problem? 
Is it because UEFI can load Xen anywwhere?
> But these will not be supported in the initial stage of
> porting Xen to MPU systems. So in this patch, we
> disable EFI boot services support for Arm MPU systems.
>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> ---
>   xen/arch/arm/Kconfig      | 2 +-
>   xen/arch/arm/arm64/head.S | 8 ++++++--
>   2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 1fe5faf847..ad592367bd 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -7,7 +7,7 @@ config ARM_64
>         def_bool y
>         depends on !ARM_32
>         select 64BIT
> -       select ARM_EFI
> +       select ARM_EFI if !HAS_MPU
I think it would make sense to allow ARM_EFI to be disabled even without 
the MPU support. So this would remove nearly 3K lines (just using wc -l 
*.c in the efi directories) for someone that don't need to boot using EFI.
>         select HAS_FAST_MULTIPLY
>   
>   config ARM
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index ad014716db..ccedf20dc7 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -172,8 +172,10 @@ efi_head:
>           .byte   0x52
>           .byte   0x4d
>           .byte   0x64
> -        .long   pe_header - efi_head        /* Offset to the PE header. */
> -
> +#ifndef CONFIG_ARM_EFI
> +        .long   0                    /* 0 means no PE header. */
> +#else
> +        .long   pe_header - efi_head /* Offset to the PE header. */
>           /*
>            * Add the PE/COFF header to the file.  The address of this header
>            * is at offset 0x3c in the file, and is part of Linux "Image"
> @@ -279,6 +281,8 @@ section_table:
>           .short  0                /* NumberOfLineNumbers  (0 for executables) 
> */
>           .long   0xe0500020       /* Characteristics (section flags) */
>           .align  5
> +#endif /* CONFIG_ARM_EFI */
> +
>   real_start:
>           /* BSS should be zeroed when booting without EFI */
>           mov   x26, #0                /* x26 := skip_zero_bss */
Shouldn't the function efi_xen_start be stubbed as well?

Cheers,

--
Julien Grall