| Message ID | 20181029204712.15525-1-simon.k.r.goldschmidt@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | [U-Boot] arm: socfpga: fix bootcounter by reserving SRAM | expand |
Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> schrieb am Mo., 29. Okt. 2018, 21:47: > Commit 768f23dc8ae3 ("ARM: socfpga: Put stack at the end of SRAM") > broke those socfpga boards that keep the bootcounter at the > end of the internal SRAM as the bootcounter needs 8 bytes > by default and thus the very first SPL call to > board_init_f_alloc_reserve overwrites the bootcounter. > > This patch allows to move the initial stack pointer down a > bit to allow boards to reserve some of the internal SRAM for > other features (like the bootcounter). > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > Ok, it seems I have forgotten that I had sent this already: https://patchwork.ozlabs.org/patch/913230/ However, I can't find that mail so I'll respond to Marek here. Moving this to common memory reservation code does not work as the very first call to board_init_f_alloc_reserve() already overwrites the boot counter. This in that function it is too late. By changing the initial stack pointer define, it is ensured that no code writes to that memory. Simon --- > > include/configs/socfpga_common.h | 6 +++++- > include/configs/socfpga_is1.h | 9 +++++---- > include/configs/socfpga_sr1500.h | 3 ++- > 3 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/include/configs/socfpga_common.h > b/include/configs/socfpga_common.h > index 2330143cf1..1e26630330 100644 > --- a/include/configs/socfpga_common.h > +++ b/include/configs/socfpga_common.h > @@ -31,8 +31,12 @@ > #define CONFIG_SYS_INIT_RAM_ADDR 0xFFE00000 > #define CONFIG_SYS_INIT_RAM_SIZE 0x40000 /* 256KB */ > #endif > +#ifndef SOCFPGA_INIT_RAM_END_RESERVE > +#define SOCFPGA_INIT_RAM_END_RESERVE 0 > +#endif > #define CONFIG_SYS_INIT_SP_ADDR \ > - (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE) > + (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE - \ > + SOCFPGA_INIT_RAM_END_RESERVE) > > #define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1 > > diff --git a/include/configs/socfpga_is1.h b/include/configs/socfpga_is1.h > index c233c208a5..3c80c0f45c 100644 > --- a/include/configs/socfpga_is1.h > +++ b/include/configs/socfpga_is1.h > @@ -23,12 +23,13 @@ > /* PHY */ > #endif > > -/* The rest of the configuration is shared */ > -#include <configs/socfpga_common.h> > - > /* > - * Bootcounter > + * Bootcounter (8 bytes at the end of internal SRAM) > */ > #define CONFIG_SYS_BOOTCOUNT_BE > +#define SOCFPGA_INIT_RAM_END_RESERVE 8 > + > +/* The rest of the configuration is shared */ > +#include <configs/socfpga_common.h> > > #endif /* __CONFIG_SOCFPGA_IS1_H__ */ > diff --git a/include/configs/socfpga_sr1500.h > b/include/configs/socfpga_sr1500.h > index 984f1183fd..b7b43fc6af 100644 > --- a/include/configs/socfpga_sr1500.h > +++ b/include/configs/socfpga_sr1500.h > @@ -25,9 +25,10 @@ > #define CONFIG_SPI_N25Q256A_RESET > > /* > - * Bootcounter > + * Bootcounter (8 bytes at the end of internal SRAM) > */ > #define CONFIG_SYS_BOOTCOUNT_BE > +#define SOCFPGA_INIT_RAM_END_RESERVE 8 > > /* Environment setting for SPI flash */ > #define CONFIG_SYS_REDUNDAND_ENVIRONMENT > -- > 2.17.1 > >
Hi Simon, On 29.10.18 21:47, Simon Goldschmidt wrote: > Commit 768f23dc8ae3 ("ARM: socfpga: Put stack at the end of SRAM") > broke those socfpga boards that keep the bootcounter at the > end of the internal SRAM as the bootcounter needs 8 bytes > by default and thus the very first SPL call to > board_init_f_alloc_reserve overwrites the bootcounter. > > This patch allows to move the initial stack pointer down a > bit to allow boards to reserve some of the internal SRAM for > other features (like the bootcounter). > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> Thanks for working on this. I've just found some time to dig into this as well. I'll send a different approach for this shortly. Thanks, Stefan > --- > > include/configs/socfpga_common.h | 6 +++++- > include/configs/socfpga_is1.h | 9 +++++---- > include/configs/socfpga_sr1500.h | 3 ++- > 3 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h > index 2330143cf1..1e26630330 100644 > --- a/include/configs/socfpga_common.h > +++ b/include/configs/socfpga_common.h > @@ -31,8 +31,12 @@ > #define CONFIG_SYS_INIT_RAM_ADDR 0xFFE00000 > #define CONFIG_SYS_INIT_RAM_SIZE 0x40000 /* 256KB */ > #endif > +#ifndef SOCFPGA_INIT_RAM_END_RESERVE > +#define SOCFPGA_INIT_RAM_END_RESERVE 0 > +#endif > #define CONFIG_SYS_INIT_SP_ADDR \ > - (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE) > + (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE - \ > + SOCFPGA_INIT_RAM_END_RESERVE) > > #define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1 > > diff --git a/include/configs/socfpga_is1.h b/include/configs/socfpga_is1.h > index c233c208a5..3c80c0f45c 100644 > --- a/include/configs/socfpga_is1.h > +++ b/include/configs/socfpga_is1.h > @@ -23,12 +23,13 @@ > /* PHY */ > #endif > > -/* The rest of the configuration is shared */ > -#include <configs/socfpga_common.h> > - > /* > - * Bootcounter > + * Bootcounter (8 bytes at the end of internal SRAM) > */ > #define CONFIG_SYS_BOOTCOUNT_BE > +#define SOCFPGA_INIT_RAM_END_RESERVE 8 > + > +/* The rest of the configuration is shared */ > +#include <configs/socfpga_common.h> > > #endif /* __CONFIG_SOCFPGA_IS1_H__ */ > diff --git a/include/configs/socfpga_sr1500.h b/include/configs/socfpga_sr1500.h > index 984f1183fd..b7b43fc6af 100644 > --- a/include/configs/socfpga_sr1500.h > +++ b/include/configs/socfpga_sr1500.h > @@ -25,9 +25,10 @@ > #define CONFIG_SPI_N25Q256A_RESET > > /* > - * Bootcounter > + * Bootcounter (8 bytes at the end of internal SRAM) > */ > #define CONFIG_SYS_BOOTCOUNT_BE > +#define SOCFPGA_INIT_RAM_END_RESERVE 8 > > /* Environment setting for SPI flash */ > #define CONFIG_SYS_REDUNDAND_ENVIRONMENT > Viele Grüße, Stefan
Stefan Roese <sr@denx.de> schrieb am Di., 30. Okt. 2018, 09:52: > Hi Simon, > > On 29.10.18 21:47, Simon Goldschmidt wrote: > > Commit 768f23dc8ae3 ("ARM: socfpga: Put stack at the end of SRAM") > > broke those socfpga boards that keep the bootcounter at the > > end of the internal SRAM as the bootcounter needs 8 bytes > > by default and thus the very first SPL call to > > board_init_f_alloc_reserve overwrites the bootcounter. > > > > This patch allows to move the initial stack pointer down a > > bit to allow boards to reserve some of the internal SRAM for > > other features (like the bootcounter). > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> > > Thanks for working on this. I've just found some time to dig > into this as well. I'll send a different approach for this > shortly. > That would be very welcome! I'm not saying my approach is nice, I only wanted the boot counter to work on my board somehow ;-) Simon > Thanks, > Stefan > > > --- > > > > include/configs/socfpga_common.h | 6 +++++- > > include/configs/socfpga_is1.h | 9 +++++---- > > include/configs/socfpga_sr1500.h | 3 ++- > > 3 files changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/include/configs/socfpga_common.h > b/include/configs/socfpga_common.h > > index 2330143cf1..1e26630330 100644 > > --- a/include/configs/socfpga_common.h > > +++ b/include/configs/socfpga_common.h > > @@ -31,8 +31,12 @@ > > #define CONFIG_SYS_INIT_RAM_ADDR 0xFFE00000 > > #define CONFIG_SYS_INIT_RAM_SIZE 0x40000 /* 256KB */ > > #endif > > +#ifndef SOCFPGA_INIT_RAM_END_RESERVE > > +#define SOCFPGA_INIT_RAM_END_RESERVE 0 > > +#endif > > #define CONFIG_SYS_INIT_SP_ADDR \ > > - (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE) > > + (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE - \ > > + SOCFPGA_INIT_RAM_END_RESERVE) > > > > #define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1 > > > > diff --git a/include/configs/socfpga_is1.h > b/include/configs/socfpga_is1.h > > index c233c208a5..3c80c0f45c 100644 > > --- a/include/configs/socfpga_is1.h > > +++ b/include/configs/socfpga_is1.h > > @@ -23,12 +23,13 @@ > > /* PHY */ > > #endif > > > > -/* The rest of the configuration is shared */ > > -#include <configs/socfpga_common.h> > > - > > /* > > - * Bootcounter > > + * Bootcounter (8 bytes at the end of internal SRAM) > > */ > > #define CONFIG_SYS_BOOTCOUNT_BE > > +#define SOCFPGA_INIT_RAM_END_RESERVE 8 > > + > > +/* The rest of the configuration is shared */ > > +#include <configs/socfpga_common.h> > > > > #endif /* __CONFIG_SOCFPGA_IS1_H__ */ > > diff --git a/include/configs/socfpga_sr1500.h > b/include/configs/socfpga_sr1500.h > > index 984f1183fd..b7b43fc6af 100644 > > --- a/include/configs/socfpga_sr1500.h > > +++ b/include/configs/socfpga_sr1500.h > > @@ -25,9 +25,10 @@ > > #define CONFIG_SPI_N25Q256A_RESET > > > > /* > > - * Bootcounter > > + * Bootcounter (8 bytes at the end of internal SRAM) > > */ > > #define CONFIG_SYS_BOOTCOUNT_BE > > +#define SOCFPGA_INIT_RAM_END_RESERVE 8 > > > > /* Environment setting for SPI flash */ > > #define CONFIG_SYS_REDUNDAND_ENVIRONMENT > > > > Viele Grüße, > Stefan > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de >
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index 2330143cf1..1e26630330 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -31,8 +31,12 @@ #define CONFIG_SYS_INIT_RAM_ADDR 0xFFE00000 #define CONFIG_SYS_INIT_RAM_SIZE 0x40000 /* 256KB */ #endif +#ifndef SOCFPGA_INIT_RAM_END_RESERVE +#define SOCFPGA_INIT_RAM_END_RESERVE 0 +#endif #define CONFIG_SYS_INIT_SP_ADDR \ - (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE) + (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE - \ + SOCFPGA_INIT_RAM_END_RESERVE) #define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1 diff --git a/include/configs/socfpga_is1.h b/include/configs/socfpga_is1.h index c233c208a5..3c80c0f45c 100644 --- a/include/configs/socfpga_is1.h +++ b/include/configs/socfpga_is1.h @@ -23,12 +23,13 @@ /* PHY */ #endif -/* The rest of the configuration is shared */ -#include <configs/socfpga_common.h> - /* - * Bootcounter + * Bootcounter (8 bytes at the end of internal SRAM) */ #define CONFIG_SYS_BOOTCOUNT_BE +#define SOCFPGA_INIT_RAM_END_RESERVE 8 + +/* The rest of the configuration is shared */ +#include <configs/socfpga_common.h> #endif /* __CONFIG_SOCFPGA_IS1_H__ */ diff --git a/include/configs/socfpga_sr1500.h b/include/configs/socfpga_sr1500.h index 984f1183fd..b7b43fc6af 100644 --- a/include/configs/socfpga_sr1500.h +++ b/include/configs/socfpga_sr1500.h @@ -25,9 +25,10 @@ #define CONFIG_SPI_N25Q256A_RESET /* - * Bootcounter + * Bootcounter (8 bytes at the end of internal SRAM) */ #define CONFIG_SYS_BOOTCOUNT_BE +#define SOCFPGA_INIT_RAM_END_RESERVE 8 /* Environment setting for SPI flash */ #define CONFIG_SYS_REDUNDAND_ENVIRONMENT
Commit 768f23dc8ae3 ("ARM: socfpga: Put stack at the end of SRAM") broke those socfpga boards that keep the bootcounter at the end of the internal SRAM as the bootcounter needs 8 bytes by default and thus the very first SPL call to board_init_f_alloc_reserve overwrites the bootcounter. This patch allows to move the initial stack pointer down a bit to allow boards to reserve some of the internal SRAM for other features (like the bootcounter). Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> --- include/configs/socfpga_common.h | 6 +++++- include/configs/socfpga_is1.h | 9 +++++---- include/configs/socfpga_sr1500.h | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-)