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

[Tom Rini]

On Mon, May 18, 2020 at 06:45:17PM +0000, Heinrich Schuchardt wrote:
> 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.

Lets assume that it's somewhere on the TODO list for Layerscape but
since it's not done today it won't be done tomorrow and we should figure
out how to solve this, and if possible in a way that unifies the
approach that we have for Linux, VxWorks, ELF and binary applications
which I think only ZynqMP platforms handle today even.  The latter of
which implies that perhaps Zynq is in the same boat here?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200518/66a25373/attachment.sig>