diff mbox

[v4,5/5] init: efi: arm: enable (U)EFI runtime services on arm

Message ID 1389445524-30623-6-git-send-email-leif.lindholm@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Leif Lindholm Jan. 11, 2014, 1:05 p.m. UTC
Since the efi_set_virtual_address_map call has strict init ordering
requirements, add an explicit hook in the required place.

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
 init/main.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Arnd Bergmann Jan. 13, 2014, 6:29 p.m. UTC | #1
On Saturday 11 January 2014, Leif Lindholm wrote:
> diff --git a/init/main.c b/init/main.c
> index febc511..1331829 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -905,6 +905,10 @@ static noinline void __init kernel_init_freeable(void)
>         smp_prepare_cpus(setup_max_cpus);
>  
>         do_pre_smp_initcalls();
> +
> +       if (IS_ENABLED(CONFIG_ARM) && efi_enabled(EFI_BOOT))
> +               efi_enter_virtual_mode();

What is the dependency on CONFIG_ARM here? Wouldn't most other
architectures need the same? I'd rather not see this turn into
a long list of CONFIG_$(ARCH) checks if other architectures
enable it in the same place.

I also wonder why the three architectures implementing it all
call this from wildly different places during init/main.c, namely
(very early) setup_arch() on ia64, (relatively early) start_kernel
on x86 and (relatively late) kernel_init_freeable on arm.

In general, I'd be happy with adding this as late in the startup
code as possible, but it may be better to use the same place as
x86 in order to avoid surprises with unexpected dependencies.
One such dependency that may cause problems is the fact that
we (try to) call efi_late_init() before efi_enter_virtual_mode()
now.

	Arnd
Leif Lindholm Jan. 13, 2014, 6:57 p.m. UTC | #2
On Mon, Jan 13, 2014 at 07:29:06PM +0100, Arnd Bergmann wrote:
> On Saturday 11 January 2014, Leif Lindholm wrote:
> > diff --git a/init/main.c b/init/main.c
> > index febc511..1331829 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -905,6 +905,10 @@ static noinline void __init kernel_init_freeable(void)
> >         smp_prepare_cpus(setup_max_cpus);
> >  
> >         do_pre_smp_initcalls();
> > +
> > +       if (IS_ENABLED(CONFIG_ARM) && efi_enabled(EFI_BOOT))
> > +               efi_enter_virtual_mode();
> 
> What is the dependency on CONFIG_ARM here? Wouldn't most other
> architectures need the same?

Most 64-bit architectures could get away from it.
x86 does it where its particular init environment forces it to.

For arm, the strict ordering requirement is for efi_enter_virtual_mode
to be called after init_static_idmap.

If ordering between early_initcalls was possible in a sane way, I could
do that instead, but I don't think a patch that swapped order of kernel/
and mm/ in arch/arm/Makefile would be accepted :)

> I'd rather not see this turn into
> a long list of CONFIG_$(ARCH) checks if other architectures
> enable it in the same place.
> 
> I also wonder why the three architectures implementing it all
> call this from wildly different places during init/main.c, namely
> (very early) setup_arch() on ia64,

Likewise arm64.

> (relatively early) start_kernel
> on x86 and (relatively late) kernel_init_freeable on arm.

As I said - the pure 64-bit archs have less of an issue, since they
can have their kernel somewhere that won't clash with the 1:1 mapping
of RAM required by UEFI SetVirtualAddressMap.

> In general, I'd be happy with adding this as late in the startup
> code as possible, but it may be better to use the same place as
> x86 in order to avoid surprises with unexpected dependencies.

I _really_ don't want to call SetVirtualAddressMap after smp_init.

> One such dependency that may cause problems is the fact that
> we (try to) call efi_late_init() before efi_enter_virtual_mode()
> now.

Well, efi_late_init() is an inline {} on everything !x86.

/
    Leif
diff mbox

Patch

diff --git a/init/main.c b/init/main.c
index febc511..1331829 100644
--- a/init/main.c
+++ b/init/main.c
@@ -905,6 +905,10 @@  static noinline void __init kernel_init_freeable(void)
 	smp_prepare_cpus(setup_max_cpus);
 
 	do_pre_smp_initcalls();
+
+	if (IS_ENABLED(CONFIG_ARM) && efi_enabled(EFI_BOOT))
+		efi_enter_virtual_mode();
+
 	lockup_detector_init();
 
 	smp_init();