| Message ID | 20200610104120.30668-6-rayagonda.kokatanur@broadcom.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add initial support for broadcom NS3 soc | expand |
On Wed, 10 Jun 2020 at 04:41, Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> wrote: > > From: Abhishek Shah <abhishek.shah at broadcom.com> > > Add API to save boot parameters passed from BL31 > > Use assembly implementation of save_boot_params instead of c function. > Because generally ATF does not set up SP_EL2 on exiting. > Thus, usage of a C function immediately after exiting with no stack > setup done by ATF explicitly, may cause SP_EL2 to be not sane, > which in turn causes a crash if this boot was not lucky to get > an SP_EL2 in valid range. Replace C implementation with assembly one > which does not use stack this early, and let u-boot to set up its stack > later. Can this be fixed in ATF? > > Signed-off-by: Abhishek Shah <abhishek.shah at broadcom.com> > Signed-off-by: Rajesh Ravi <rajesh.ravi at broadcom.com> > Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov at broadcom.com> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> > --- > arch/arm/cpu/armv8/bcmns3/lowlevel.S | 9 +++++++ > arch/arm/include/asm/arch-bcmns3/bl33_info.h | 26 ++++++++++++++++++++ > board/broadcom/bcmns3/ns3.c | 10 ++++++++ > 3 files changed, 45 insertions(+) > create mode 100644 arch/arm/include/asm/arch-bcmns3/bl33_info.h > Reviewed-by: Simon Glass <sjg at chromium.org> > diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c > index e38156723c..5e644bd466 100644 > --- a/board/broadcom/bcmns3/ns3.c > +++ b/board/broadcom/bcmns3/ns3.c > @@ -8,6 +8,7 @@ > #include <asm/io.h> > #include <asm/system.h> > #include <asm/armv8/mmu.h> > +#include <asm/arch-bcmns3/bl33_info.h> > > static struct mm_region ns3_mem_map[] = { > { > @@ -33,8 +34,17 @@ struct mm_region *mem_map = ns3_mem_map; > > DECLARE_GLOBAL_DATA_PTR; > > +/* > + * Force the bl33_info to the data-section, as .bss will not be valid > + * when save_boot_params is invoked. > + */ > +struct bl33_info *bl33_info __section(".data"); > + > int board_init(void) > { > + if (bl33_info->version != BL33_INFO_VERSION) > + printf("*** warning: ATF BL31 and u-boot not in sync! ***\n"); Shouldn't this be a fatal error? > + > return 0; > } > > -- > 2.17.1 >
On Fri, Jun 26, 2020 at 6:41 AM Simon Glass <sjg at chromium.org> wrote: > > On Wed, 10 Jun 2020 at 04:41, Rayagonda Kokatanur > <rayagonda.kokatanur at broadcom.com> wrote: > > > > From: Abhishek Shah <abhishek.shah at broadcom.com> > > > > Add API to save boot parameters passed from BL31 > > > > Use assembly implementation of save_boot_params instead of c function. > > Because generally ATF does not set up SP_EL2 on exiting. > > Thus, usage of a C function immediately after exiting with no stack > > setup done by ATF explicitly, may cause SP_EL2 to be not sane, > > which in turn causes a crash if this boot was not lucky to get > > an SP_EL2 in valid range. Replace C implementation with assembly one > > which does not use stack this early, and let u-boot to set up its stack > > later. > > Can this be fixed in ATF? We are passing boot parameters for uboot using arm cpu x0, x1, x2 and x3 registers. > > > > > Signed-off-by: Abhishek Shah <abhishek.shah at broadcom.com> > > Signed-off-by: Rajesh Ravi <rajesh.ravi at broadcom.com> > > Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov at broadcom.com> > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com> > > --- > > arch/arm/cpu/armv8/bcmns3/lowlevel.S | 9 +++++++ > > arch/arm/include/asm/arch-bcmns3/bl33_info.h | 26 ++++++++++++++++++++ > > board/broadcom/bcmns3/ns3.c | 10 ++++++++ > > 3 files changed, 45 insertions(+) > > create mode 100644 arch/arm/include/asm/arch-bcmns3/bl33_info.h > > > > Reviewed-by: Simon Glass <sjg at chromium.org> > > > diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c > > index e38156723c..5e644bd466 100644 > > --- a/board/broadcom/bcmns3/ns3.c > > +++ b/board/broadcom/bcmns3/ns3.c > > @@ -8,6 +8,7 @@ > > #include <asm/io.h> > > #include <asm/system.h> > > #include <asm/armv8/mmu.h> > > +#include <asm/arch-bcmns3/bl33_info.h> > > > > static struct mm_region ns3_mem_map[] = { > > { > > @@ -33,8 +34,17 @@ struct mm_region *mem_map = ns3_mem_map; > > > > DECLARE_GLOBAL_DATA_PTR; > > > > +/* > > + * Force the bl33_info to the data-section, as .bss will not be valid > > + * when save_boot_params is invoked. > > + */ > > +struct bl33_info *bl33_info __section(".data"); > > + > > int board_init(void) > > { > > + if (bl33_info->version != BL33_INFO_VERSION) > > + printf("*** warning: ATF BL31 and u-boot not in sync! ***\n"); > > Shouldn't this be a fatal error? It's not a fatal error. Its warning message for the user indicates that BL31 and BL33 are out of sync. By knowing this warning message, users can upgrade the binarie from uboot prompt. Best regards, Rayagonda > > > > + > > return 0; > > } > > > > -- > > 2.17.1 > >
diff --git a/arch/arm/cpu/armv8/bcmns3/lowlevel.S b/arch/arm/cpu/armv8/bcmns3/lowlevel.S index 202286248e..9d8eb7f117 100644 --- a/arch/arm/cpu/armv8/bcmns3/lowlevel.S +++ b/arch/arm/cpu/armv8/bcmns3/lowlevel.S @@ -88,3 +88,12 @@ ENTRY(__asm_flush_l3_dcache) mov lr, x29 ret ENDPROC(__asm_flush_l3_dcache) + +ENTRY(save_boot_params) +/* + * void set_boot_params(uint64_t x0, uint64_t x1, uint64_t x2, uint64_t x3) + */ + adr x4, bl33_info + str x0, [x4] + b save_boot_params_ret +ENDPROC(save_boot_params) diff --git a/arch/arm/include/asm/arch-bcmns3/bl33_info.h b/arch/arm/include/asm/arch-bcmns3/bl33_info.h new file mode 100644 index 0000000000..bbc95b0186 --- /dev/null +++ b/arch/arm/include/asm/arch-bcmns3/bl33_info.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright 2020 Broadcom. + * + */ + +#ifndef BL33_INFO_H +#define BL33_INFO_H +#include <asm/io.h> + +/* Increase version number each time this file is modified */ +#define BL33_INFO_VERSION 1 + +struct chip_info { + unsigned int chip_id; + unsigned int rev_id; +}; + +struct bl33_info { + unsigned int version; + struct chip_info chip; +}; + +extern struct bl33_info *bl33_info; + +#endif diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c index e38156723c..5e644bd466 100644 --- a/board/broadcom/bcmns3/ns3.c +++ b/board/broadcom/bcmns3/ns3.c @@ -8,6 +8,7 @@ #include <asm/io.h> #include <asm/system.h> #include <asm/armv8/mmu.h> +#include <asm/arch-bcmns3/bl33_info.h> static struct mm_region ns3_mem_map[] = { { @@ -33,8 +34,17 @@ struct mm_region *mem_map = ns3_mem_map; DECLARE_GLOBAL_DATA_PTR; +/* + * Force the bl33_info to the data-section, as .bss will not be valid + * when save_boot_params is invoked. + */ +struct bl33_info *bl33_info __section(".data"); + int board_init(void) { + if (bl33_info->version != BL33_INFO_VERSION) + printf("*** warning: ATF BL31 and u-boot not in sync! ***\n"); + return 0; }