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

[Julien Grall <julien@xxxxxxx>]

On 06/11/2022 19:12, Julien Grall wrote:
> 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?
Sorry, I mean protected rather than stubbed because there will be no 
user when !CONFIG_ARM_EFI.
Cheers,

--
Julien Grall