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

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 2022年11月7日 3:12
> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: nd <nd@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Bertrand
> Marquis <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>
> Subject: Re: [PATCH v6 03/11] xen/arm: disable EFI boot services for MPU
> systems
> 
> 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.
> 

Ok, how about address this comment in NUMA patch set#3 (Arm implementation),
because about making ARM_EFI invisible to users, we had some discussions
for NUMA RFC and V1.

> >     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?
> 

Reply for your next email:
Yes, this is a good point, efi_xen_start should be protected. I will fix it.

Cheers,
Wei Chen


> Cheers,
> 
> --
> Julien Grall