[PATCH 4/4] efi_loader: call smp_kick_all_cpus()

[Heinrich Schuchardt]

Am May 18, 2020 6:30:09 PM UTC schrieb Tom Rini <trini at konsulko.com>:
>On Sat, May 16, 2020 at 05:54:43PM +0200, Michael Walle wrote:
>> [Also adding Tom Rini as ARM maintainer]
>> 
>> Am 2020-05-14 22:17, schrieb Alexander Graf:
>> > On 14.05.20 20:46, Heinrich Schuchardt wrote:
>> > > On 5/14/20 2:38 PM, Michael Walle wrote:
>> > > > On some architectures, specifically the layerscape, the
>> > > > secondary cores
>> > > > wait for an interrupt before entering the spin-tables. This
>> > > > applies only
>> > > > to boards which doesn't have PSCI provided by TF-a and u-boot
>> > > > does the
>> > > %s/TF-a/TF-A/, %s/u-boot/U-Boot/
>> > > 
>> > > > secondary cores handling.
>> > > > bootm/booti already call that function for ARM architecture;
>> > > > also add it
>> > > > to bootelf before switching to EL2. Additionally, provide a
>weak noop
>> > > > function so we don't have to have "#ifdef CONFIG_ARM64" guards.
>> > > > 
>> > > > Signed-off-by: Michael Walle <michael at walle.cc>
>> > > > ---
>> > > >   common/bootm.c             | 9 +++++++++
>> > > >   lib/efi_loader/efi_setup.c | 6 ++++++
>> > > >   2 files changed, 15 insertions(+)
>> > > > 
>> > > > diff --git a/common/bootm.c b/common/bootm.c
>> > > > index db4362a643..65adf29329 100644
>> > > > --- a/common/bootm.c
>> > > > +++ b/common/bootm.c
>> > > > @@ -816,6 +816,15 @@ void __weak
>switch_to_non_secure_mode(void)
>> > > >   {
>> > > >   }
>> > > > 
>> > > > +/**
>> > > > + * smp_kick_all_cpus() - kick all CPUs
>> > > > + *
>> > > > + * This routine is overridden by architectures requiring this
>> > > > feature.
>> > > > + */
>> > > > +void __weak smp_kick_all_cpus(void)
>> > > > +{
>> > > > +}
>> > > > +
>> > > >   #else /* USE_HOSTCC */
>> > > > 
>> > > >   #if defined(CONFIG_FIT_SIGNATURE)
>> > > > diff --git a/lib/efi_loader/efi_setup.c
>b/lib/efi_loader/efi_setup.c
>> > > > index 26a7423203..7e5364adc5 100644
>> > > > --- a/lib/efi_loader/efi_setup.c
>> > > > +++ b/lib/efi_loader/efi_setup.c
>> > > > @@ -132,6 +132,12 @@ efi_status_t efi_init_obj_list(void)
>> > > >   	/* Allow unaligned memory access */
>> > > >   	allow_unaligned();
>> > > > 
>> > > > +	/*
>> > > > +	 * Some architectures need to kick secondary cores to enter
>their
>> > > > +	 * spin table.
>> > > > +	 */
>> > > > +	smp_kick_all_cpus();
>> > > This will not compile with
>> > > 
>> > > CONFIG_CMD_BOOTI=n
>> > > CONFIG_CMD_BOOTM=n
>> > > CONFIG_CMD_BOOTZ=n
>> > 
>> > 
>> > Much worse is that in incurs needless overhead on PSCI capable
>> > platforms. Can we move the smp_kick_all_cpus() to the board or soc
>> > level of the few systems that use spin tables please? :)
>> 
>> Ok, I'm not sure where I should put the smp_kick_all_cpus(). In the
>> bootm/booti path this is done in boot_jump_linux() (unconditionally
>> in do_nonsec_virt_switch()). Therefore, shouldn't the kick be in
>> switch_to_non_secure_mode(), too?
>> 
>> Regarding the overhead, isn't it just called once before starting
>> the OS? Is that really worth adding yet another weak function or
>> ifdef guards? Isn't it better to behave like the bootm case?
>
>So, I think this shows that there's some consolidation needed as
>VxWorks
>and ELF support have also had to solve this problem and are copying
>around the same code that I believe Alex is saying we should in fact
>not
>need on SoCs that handle this via PSCI instead of a spintable?

Michael wants to boot ARMv8 boards using spin-tables via EFI (FSL-Layerscape).

You might argue that the effort would be better invested in getting PSCI properly enabled for these boards.

Best regards

Heinrich