| Message ID | 1464627083-12301-1-git-send-email-srae@broadcom.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, May 30, 2016 at 09:51:22AM -0700, Steve Rae wrote: > From: Andre Przywara <andre.przywara@arm.com> > > Some SPL loaders (like Allwinner's boot0, and Broadcom's boot0) > require a header before the actual U-Boot binary to both check its > validity and to find other data to load. Sometimes this header may > only be a few bytes of information, and sometimes this might simply > be space that needs to be reserved for a post-processing tool. > > Introduce a config option to allow assembler preprocessor commands > to be inserted into the code at the appropriate location; typical > preprocessor commands might be: > .space 1000 > .word 0x12345678 > etc. > Please note that the current code: > start.S (arm64) and > vectors.S (arm) > already jumps over some portion of data already, so this option basically > just increases the size of this region (and the resulting binary). > > For use with Allwinner's boot0 blob there is a tool called boot0img[1], > which fills the header to allow booting A64 based boards. > For the Pine64 we need a 1536 byte header (including the branch > instruction) at the moment, so we add this to the defconfig. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > Signed-off-by: Steve Rae <srae@broadcom.com> I think this is a step in the right direction. [snip] > diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S > index e933021..9202889 100644 > --- a/arch/arm/cpu/armv8/start.S > +++ b/arch/arm/cpu/armv8/start.S > @@ -21,6 +21,10 @@ > _start: > b reset > > +#ifdef CONFIG_BOOT0_CODE > +CONFIG_BOOT0_CODE > +#endif > + > .align 3 I don't like adding things to the CONFIG name space that we can't control the contents of via Kconfig. So I think we need to change the BOOT0_CODE portion to something like ARMV8_SOC_BOOT0_HOOK. And if there's some part of the ARMv8 docs we can reference that explains why it's boot0 and at least 2 different SoCs have done this, we can use a better name here. Then.. > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > index c1ae6f5..d2678c9 100644 > --- a/board/sunxi/Kconfig > +++ b/board/sunxi/Kconfig > @@ -15,6 +15,13 @@ config SUNXI_GEN_SUN6I > separate ahb reset control registers, custom pmic bus, new style > watchdog, etc. > > +config SUNXI_BOOT0 > + bool "prepare for boot0 header" > + ---help--- > + If U-Boot is loaded from the Allwinner provided boot0 blob, it > + expects a header area filled with magic values. > + This option will add some space at the beginning of the image to > + let a tool later on fill in this header with sensible data. This becomes something like ENABLE_ARMV8_SOC_BOOT0_HOOK and generic the text a bit more here and mention the Allwinner and Broadcom examples. > diff --git a/include/configs/bcm28155_ap.h b/include/configs/bcm28155_ap.h > index 889e5db..515552b 100644 > --- a/include/configs/bcm28155_ap.h > +++ b/include/configs/bcm28155_ap.h > @@ -137,4 +137,8 @@ > #define CONFIG_USB_GADGET_BCM_UDC_OTG_PHY > #define CONFIG_USBID_ADDR 0x34052c46 > > +#define CONFIG_BOOT0_CODE \ > + .word 0xbabeface; \ > + .word _end - _start > + Then this goes into arch/arm/include/asm/arch-bcm281xx/boot0.h or similar? > #endif /* __BCM28155_AP_H */ > diff --git a/include/configs/sun50i.h b/include/configs/sun50i.h > index 0fdb4c7..fe9624b 100644 > --- a/include/configs/sun50i.h > +++ b/include/configs/sun50i.h > @@ -17,6 +17,11 @@ > #define GICD_BASE 0x1c81000 > #define GICC_BASE 0x1c82000 > > +#ifdef CONFIG_SUNXI_BOOT0 > +#define CONFIG_BOOT0_CODE \ > + .space 1532 > +#endif and .../arch-sunxi/boot0.h
Hi Tom, On Mon, May 30, 2016 at 11:14 AM, Tom Rini <trini@konsulko.com> wrote: > > On Mon, May 30, 2016 at 09:51:22AM -0700, Steve Rae wrote: > > > From: Andre Przywara <andre.przywara@arm.com> > > > > Some SPL loaders (like Allwinner's boot0, and Broadcom's boot0) > > require a header before the actual U-Boot binary to both check its > > validity and to find other data to load. Sometimes this header may > > only be a few bytes of information, and sometimes this might simply > > be space that needs to be reserved for a post-processing tool. > > > > Introduce a config option to allow assembler preprocessor commands > > to be inserted into the code at the appropriate location; typical > > preprocessor commands might be: > > .space 1000 > > .word 0x12345678 > > etc. > > Please note that the current code: > > start.S (arm64) and > > vectors.S (arm) > > already jumps over some portion of data already, so this option basically > > just increases the size of this region (and the resulting binary). > > > > For use with Allwinner's boot0 blob there is a tool called boot0img[1], > > which fills the header to allow booting A64 based boards. > > For the Pine64 we need a 1536 byte header (including the branch > > instruction) at the moment, so we add this to the defconfig. > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > Signed-off-by: Steve Rae <srae@broadcom.com> > > I think this is a step in the right direction. Thanks.... > > [snip] > > diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S > > index e933021..9202889 100644 > > --- a/arch/arm/cpu/armv8/start.S > > +++ b/arch/arm/cpu/armv8/start.S > > @@ -21,6 +21,10 @@ > > _start: > > b reset > > > > +#ifdef CONFIG_BOOT0_CODE > > +CONFIG_BOOT0_CODE > > +#endif > > + > > .align 3 > > I don't like adding things to the CONFIG name space that we can't > control the contents of via Kconfig. So I think we need to change the > BOOT0_CODE portion to something like ARMV8_SOC_BOOT0_HOOK. And if > there's some part of the ARMv8 docs we can reference that explains why > it's boot0 and at least 2 different SoCs have done this, we can use a > better name here. > OK, I follow your logic -- but this is not ARMV8 specific... In this commit: - Allwinner is arm64, but - Broadcom is arm (armv7l) so maybe just: SOC_BOOT0_HOOK ? And I don't know where the "BOOT0" terminology started - maybe it is in ARM/ARMV8 documentation; I just don't know.... > Then.. > > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > > index c1ae6f5..d2678c9 100644 > > --- a/board/sunxi/Kconfig > > +++ b/board/sunxi/Kconfig > > @@ -15,6 +15,13 @@ config SUNXI_GEN_SUN6I > > separate ahb reset control registers, custom pmic bus, new style > > watchdog, etc. > > > > +config SUNXI_BOOT0 > > + bool "prepare for boot0 header" > > + ---help--- > > + If U-Boot is loaded from the Allwinner provided boot0 blob, it > > + expects a header area filled with magic values. > > + This option will add some space at the beginning of the image to > > + let a tool later on fill in this header with sensible data. > > This becomes something like ENABLE_ARMV8_SOC_BOOT0_HOOK and generic the > text a bit more here and mention the Allwinner and Broadcom examples. OK for the Allwinner... But I don't think the Broadcom boards are going to define SUNXI_BOOT0 ?!?!?! > > > diff --git a/include/configs/bcm28155_ap.h b/include/configs/bcm28155_ap.h > > index 889e5db..515552b 100644 > > --- a/include/configs/bcm28155_ap.h > > +++ b/include/configs/bcm28155_ap.h > > @@ -137,4 +137,8 @@ > > #define CONFIG_USB_GADGET_BCM_UDC_OTG_PHY > > #define CONFIG_USBID_ADDR 0x34052c46 > > > > +#define CONFIG_BOOT0_CODE \ > > + .word 0xbabeface; \ > > + .word _end - _start > > + > > Then this goes into arch/arm/include/asm/arch-bcm281xx/boot0.h or > similar? OK - I get that it shouldn't be in the (deprecated) include/configs file(s).... However, I didn't really want to add more #include lines to: arch/arm/cpu/armv8/start.S or arch/arm/lib/vectors.S So, if I add your suggested filename, will it get "picked up" through the existing #include <config.h> ? Thanks, Steve > > > #endif /* __BCM28155_AP_H */ > > diff --git a/include/configs/sun50i.h b/include/configs/sun50i.h > > index 0fdb4c7..fe9624b 100644 > > --- a/include/configs/sun50i.h > > +++ b/include/configs/sun50i.h > > @@ -17,6 +17,11 @@ > > #define GICD_BASE 0x1c81000 > > #define GICC_BASE 0x1c82000 > > > > +#ifdef CONFIG_SUNXI_BOOT0 > > +#define CONFIG_BOOT0_CODE \ > > + .space 1532 > > +#endif > > and .../arch-sunxi/boot0.h > > -- > Tom
On Mon, May 30, 2016 at 11:37:47AM -0700, Steve Rae wrote: > Hi Tom, > > On Mon, May 30, 2016 at 11:14 AM, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, May 30, 2016 at 09:51:22AM -0700, Steve Rae wrote: > > > > > From: Andre Przywara <andre.przywara@arm.com> > > > > > > Some SPL loaders (like Allwinner's boot0, and Broadcom's boot0) > > > require a header before the actual U-Boot binary to both check its > > > validity and to find other data to load. Sometimes this header may > > > only be a few bytes of information, and sometimes this might simply > > > be space that needs to be reserved for a post-processing tool. > > > > > > Introduce a config option to allow assembler preprocessor commands > > > to be inserted into the code at the appropriate location; typical > > > preprocessor commands might be: > > > .space 1000 > > > .word 0x12345678 > > > etc. > > > Please note that the current code: > > > start.S (arm64) and > > > vectors.S (arm) > > > already jumps over some portion of data already, so this option basically > > > just increases the size of this region (and the resulting binary). > > > > > > For use with Allwinner's boot0 blob there is a tool called boot0img[1], > > > which fills the header to allow booting A64 based boards. > > > For the Pine64 we need a 1536 byte header (including the branch > > > instruction) at the moment, so we add this to the defconfig. > > > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > Signed-off-by: Steve Rae <srae@broadcom.com> > > > > I think this is a step in the right direction. > Thanks.... > > > > > [snip] > > > diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S > > > index e933021..9202889 100644 > > > --- a/arch/arm/cpu/armv8/start.S > > > +++ b/arch/arm/cpu/armv8/start.S > > > @@ -21,6 +21,10 @@ > > > _start: > > > b reset > > > > > > +#ifdef CONFIG_BOOT0_CODE > > > +CONFIG_BOOT0_CODE > > > +#endif > > > + > > > .align 3 > > > > I don't like adding things to the CONFIG name space that we can't > > control the contents of via Kconfig. So I think we need to change the > > BOOT0_CODE portion to something like ARMV8_SOC_BOOT0_HOOK. And if > > there's some part of the ARMv8 docs we can reference that explains why > > it's boot0 and at least 2 different SoCs have done this, we can use a > > better name here. > > > OK, I follow your logic -- but this is not ARMV8 specific... > In this commit: > - Allwinner is arm64, but > - Broadcom is arm (armv7l) > so maybe just: > SOC_BOOT0_HOOK ? > And I don't know where the "BOOT0" terminology started - maybe it is > in ARM/ARMV8 documentation; I just don't know.... OK. Yes, lets call it ARM_SOC_BOOT0_HOOK until/unless someone chimes in with a better still name. > > Then.. > > > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > > > index c1ae6f5..d2678c9 100644 > > > --- a/board/sunxi/Kconfig > > > +++ b/board/sunxi/Kconfig > > > @@ -15,6 +15,13 @@ config SUNXI_GEN_SUN6I > > > separate ahb reset control registers, custom pmic bus, new style > > > watchdog, etc. > > > > > > +config SUNXI_BOOT0 > > > + bool "prepare for boot0 header" > > > + ---help--- > > > + If U-Boot is loaded from the Allwinner provided boot0 blob, it > > > + expects a header area filled with magic values. > > > + This option will add some space at the beginning of the image to > > > + let a tool later on fill in this header with sensible data. > > > > This becomes something like ENABLE_ARMV8_SOC_BOOT0_HOOK and generic the > > text a bit more here and mention the Allwinner and Broadcom examples. > OK for the Allwinner... > But I don't think the Broadcom boards are going to define SUNXI_BOOT0 ?!?!?! Right. But it should become ENABLE_ARM_SOC_BOOT0_HOOK and both platforms (and other future ones probably) would enable it. > > > diff --git a/include/configs/bcm28155_ap.h b/include/configs/bcm28155_ap.h > > > index 889e5db..515552b 100644 > > > --- a/include/configs/bcm28155_ap.h > > > +++ b/include/configs/bcm28155_ap.h > > > @@ -137,4 +137,8 @@ > > > #define CONFIG_USB_GADGET_BCM_UDC_OTG_PHY > > > #define CONFIG_USBID_ADDR 0x34052c46 > > > > > > +#define CONFIG_BOOT0_CODE \ > > > + .word 0xbabeface; \ > > > + .word _end - _start > > > + > > > > Then this goes into arch/arm/include/asm/arch-bcm281xx/boot0.h or > > similar? > OK - I get that it shouldn't be in the (deprecated) include/configs file(s).... > However, I didn't really want to add more #include lines to: > arch/arm/cpu/armv8/start.S or > arch/arm/lib/vectors.S > So, if I add your suggested filename, will it get "picked up" through > the existing > #include <config.h> > ? No, you would need to make both S files include <asm/arch/boot0.h> but I think that's a logical thing to do. I would be: #ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK #include <asm/arch/boot0.h> /* * Various SoCs need something special and SoC-specific up front in * order to boot, allow them to set that in their boot0.h file and then * use it here. */ aRM_SOC_BOOT0_HOOK #endif
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index e933021..9202889 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -21,6 +21,10 @@ _start: b reset +#ifdef CONFIG_BOOT0_CODE +CONFIG_BOOT0_CODE +#endif + .align 3 .globl _TEXT_BASE diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S index 49238ed..5dcfab5 100644 --- a/arch/arm/lib/vectors.S +++ b/arch/arm/lib/vectors.S @@ -60,6 +60,10 @@ _start: ldr pc, _irq ldr pc, _fiq +#ifdef CONFIG_BOOT0_CODE +CONFIG_BOOT0_CODE +#endif + /* ************************************************************************* * diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index c1ae6f5..d2678c9 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -15,6 +15,13 @@ config SUNXI_GEN_SUN6I separate ahb reset control registers, custom pmic bus, new style watchdog, etc. +config SUNXI_BOOT0 + bool "prepare for boot0 header" + ---help--- + If U-Boot is loaded from the Allwinner provided boot0 blob, it + expects a header area filled with magic values. + This option will add some space at the beginning of the image to + let a tool later on fill in this header with sensible data. choice prompt "Sunxi SoC Variant" diff --git a/configs/pine64_plus_defconfig b/configs/pine64_plus_defconfig index 489b75c..ef96dab 100644 --- a/configs/pine64_plus_defconfig +++ b/configs/pine64_plus_defconfig @@ -9,3 +9,4 @@ CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus" # CONFIG_CMD_IMLS is not set # CONFIG_CMD_FLASH is not set # CONFIG_CMD_FPGA is not set +CONFIG_SUNXI_BOOT0=y diff --git a/include/configs/bcm28155_ap.h b/include/configs/bcm28155_ap.h index 889e5db..515552b 100644 --- a/include/configs/bcm28155_ap.h +++ b/include/configs/bcm28155_ap.h @@ -137,4 +137,8 @@ #define CONFIG_USB_GADGET_BCM_UDC_OTG_PHY #define CONFIG_USBID_ADDR 0x34052c46 +#define CONFIG_BOOT0_CODE \ + .word 0xbabeface; \ + .word _end - _start + #endif /* __BCM28155_AP_H */ diff --git a/include/configs/sun50i.h b/include/configs/sun50i.h index 0fdb4c7..fe9624b 100644 --- a/include/configs/sun50i.h +++ b/include/configs/sun50i.h @@ -17,6 +17,11 @@ #define GICD_BASE 0x1c81000 #define GICC_BASE 0x1c82000 +#ifdef CONFIG_SUNXI_BOOT0 +#define CONFIG_BOOT0_CODE \ + .space 1532 +#endif + /* * Include common sunxi configuration where most the settings are */