| Message ID | 20200514123831.30157-5-michael@walle.cc |
|---|---|
| State | New |
| Headers | show |
| Series | bootefi fixes for aarch64/layerscape | expand |
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 Best regards Heinrich > + > /* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */ > switch_to_non_secure_mode(); > >
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? :) Alex
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? :) By having a weak function called here instead of smp_kick_all_cpus()?
[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?
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?
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
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?
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(); + /* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */ switch_to_non_secure_mode();
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 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(+)