| Message ID | 20191203213956.24339-5-lukas.auer@aisec.fraunhofer.de |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | Fixes for RISC-V U-Boot SPL / OpenSBI boot flow | expand |
Hi Lukas, > From: Lukas Auer [mailto:lukas.auer@aisec.fraunhofer.de] > Sent: Wednesday, December 04, 2019 5:40 AM > To: u-boot@lists.denx.de > Cc: Rick Jian-Zhi Chen(陳建志); Anup Patel; Bin Meng; Lukas Auer; Anup Patel > Subject: [PATCH 4/4] spl: opensbi: wait for ack from secondary harts before entering OpenSBI > > At the start, OpenSBI relocates itself to its link address. If the link address ranges of U-Boot SPL and OpenSBI overlap, the relocation can lead to code corruption if a hart is still running U-Boot SPL during relocation. To avoid this problem, the main hart is specified as the preferred boot hart to perform the relocation. This fixes the code corruption problems based on the assumption that since the main hart schedules the secondary harts to enter OpenSBI, it will be the last to enter OpenSBI. However it was reported that this assumption is not always correct. > > To make sure the assumption always holds true, wait for all secondary harts to acknowledge the call-function request before entering OpenSBI on the main hart. > > Reported-by: Rick Chen <rick@andestech.com> > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > --- Reviewed-by: Rick Chen <rick@andestech.com> Tested-by: Rick Chen <rick@andestech.com> > > common/spl/spl_opensbi.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c index 91a411a3db..5ea59d423f 100644 > --- a/common/spl/spl_opensbi.c > +++ b/common/spl/spl_opensbi.c > @@ -75,9 +75,19 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image) > invalidate_icache_all(); > > #ifdef CONFIG_SMP > + /* > + * Start OpenSBI on all secondary harts and wait for acknowledgment. > + * > + * OpenSBI first relocates itself to its link address. This is done by > + * the main hart. To make sure no hart is still running U-Boot SPL > + * during relocation, we wait for all secondary harts to acknowledge > + * the call-function request before entering OpenSBI on the main hart. > + * Otherwise, code corruption can occur if the link address ranges of > + * U-Boot SPL and OpenSBI overlap. > + */ > ret = smp_call_function((ulong)spl_image->entry_point, > (ulong)spl_image->fdt_addr, > - (ulong)&opensbi_info, 0); > + (ulong)&opensbi_info, 1); > if (ret) > hang(); > #endif > -- > 2.21.0 >
On Fri, Dec 6, 2019 at 2:50 PM Rick Chen <rickchen36@gmail.com> wrote: > > Hi Lukas, > > > From: Lukas Auer [mailto:lukas.auer@aisec.fraunhofer.de] > > Sent: Wednesday, December 04, 2019 5:40 AM > > To: u-boot@lists.denx.de > > Cc: Rick Jian-Zhi Chen(陳建志); Anup Patel; Bin Meng; Lukas Auer; Anup Patel > > Subject: [PATCH 4/4] spl: opensbi: wait for ack from secondary harts before entering OpenSBI > > > > At the start, OpenSBI relocates itself to its link address. If the link address ranges of U-Boot SPL and OpenSBI overlap, the relocation can lead to code corruption if a hart is still running U-Boot SPL during relocation. To avoid this problem, the main hart is specified as the preferred boot hart to perform the relocation. This fixes the code corruption problems based on the assumption that since the main hart schedules the secondary harts to enter OpenSBI, it will be the last to enter OpenSBI. However it was reported that this assumption is not always correct. > > > > To make sure the assumption always holds true, wait for all secondary harts to acknowledge the call-function request before entering OpenSBI on the main hart. > > > > Reported-by: Rick Chen <rick@andestech.com> > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > > --- > > Reviewed-by: Rick Chen <rick@andestech.com> > Tested-by: Rick Chen <rick@andestech.com> Looks good to me. Reviewed-by: Anup Patel <anup.patel@wdc.com> Regards, Anup > > > > > common/spl/spl_opensbi.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c index 91a411a3db..5ea59d423f 100644 > > --- a/common/spl/spl_opensbi.c > > +++ b/common/spl/spl_opensbi.c > > @@ -75,9 +75,19 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image) > > invalidate_icache_all(); > > > > #ifdef CONFIG_SMP > > + /* > > + * Start OpenSBI on all secondary harts and wait for acknowledgment. > > + * > > + * OpenSBI first relocates itself to its link address. This is done by > > + * the main hart. To make sure no hart is still running U-Boot SPL > > + * during relocation, we wait for all secondary harts to acknowledge > > + * the call-function request before entering OpenSBI on the main hart. > > + * Otherwise, code corruption can occur if the link address ranges of > > + * U-Boot SPL and OpenSBI overlap. > > + */ > > ret = smp_call_function((ulong)spl_image->entry_point, > > (ulong)spl_image->fdt_addr, > > - (ulong)&opensbi_info, 0); > > + (ulong)&opensbi_info, 1); > > if (ret) > > hang(); > > #endif > > -- > > 2.21.0 > >
diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c index 91a411a3db..5ea59d423f 100644 --- a/common/spl/spl_opensbi.c +++ b/common/spl/spl_opensbi.c @@ -75,9 +75,19 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image) invalidate_icache_all(); #ifdef CONFIG_SMP + /* + * Start OpenSBI on all secondary harts and wait for acknowledgment. + * + * OpenSBI first relocates itself to its link address. This is done by + * the main hart. To make sure no hart is still running U-Boot SPL + * during relocation, we wait for all secondary harts to acknowledge + * the call-function request before entering OpenSBI on the main hart. + * Otherwise, code corruption can occur if the link address ranges of + * U-Boot SPL and OpenSBI overlap. + */ ret = smp_call_function((ulong)spl_image->entry_point, (ulong)spl_image->fdt_addr, - (ulong)&opensbi_info, 0); + (ulong)&opensbi_info, 1); if (ret) hang(); #endif
At the start, OpenSBI relocates itself to its link address. If the link address ranges of U-Boot SPL and OpenSBI overlap, the relocation can lead to code corruption if a hart is still running U-Boot SPL during relocation. To avoid this problem, the main hart is specified as the preferred boot hart to perform the relocation. This fixes the code corruption problems based on the assumption that since the main hart schedules the secondary harts to enter OpenSBI, it will be the last to enter OpenSBI. However it was reported that this assumption is not always correct. To make sure the assumption always holds true, wait for all secondary harts to acknowledge the call-function request before entering OpenSBI on the main hart. Reported-by: Rick Chen <rick@andestech.com> Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> --- common/spl/spl_opensbi.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)