Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote: > There is a strange feature to set global_data to a data-section variable > early in SPL. This only works if SPL actually has access to SRAM which is > not the case on x86, for eaxmple. Add a comment to this effect. > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
Am 09.11.2018 um 19:43 schrieb Tom Rini: On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote: There is a strange feature to set global_data to a data-section variable early in SPL. This only works if SPL actually has access to SRAM which is not the case on x86, for eaxmple. Add a comment to this effect. Seems like I missed that one back in October. Does anyone have an idea why this variable ('static bd_t bdata') is hard-coded into section ".data"? To me this seems pretty unportable... For example, for a specific feature on socfpga (warm reboot CRC check), I would prefer to have the ".data" section empty and put 'bdata' into bss. This is currently not possible. But before sending a patch that somehow changes this behaviour, I'd like to know why this variable is put into ".data" instead of ".bss" Thanks, Simon Signed-off-by: Simon Glass Reviewed-by: Tom Rini ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
Hi Simon, On Mon, 11 Feb 2019 at 14:00, Simon Goldschmidt wrote: > > Am 09.11.2018 um 19:43 schrieb Tom Rini: > > On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote: > > > >> There is a strange feature to set global_data to a data-section variable > >> early in SPL. This only works if SPL actually has access to SRAM which is > >> not the case on x86, for eaxmple. Add a comment to this effect. > > Seems like I missed that one back in October. > > Does anyone have an idea why this variable ('static bd_t bdata') is > hard-coded into section ".data"? To me this seems pretty unportable... > > For example, for a specific feature on socfpga (warm reboot CRC check), > I would prefer to have the ".data" section empty and put 'bdata' into > bss. This is currently not possible. > > But before sending a patch that somehow changes this behaviour, I'd like > to know why this variable is put into ".data" instead of ".bss" BSS normally appears after data and is not actually part of the u-boot-spi.bin image. If the device tree is appended to SPL then it overlaps with BSS. Thus accessing BSS overwrites the DT. There are two features to get around that - one is to pad out the BSS space so that: cat u-boot-spl-nodtb.bin u-boot-spl.dtb >u-boot-spl.bin places the DT after BSS. The other is to specify an address (typically in SDRAM) for BSS, so that it is completely separate from the image. Note also that BSS is not always available in SPL. For example if SPL is running from flash then the BSS may be mapped either to SDRAM (which is not set up until later in SPL) or flash (which is not writable at all). Of course the data section has the same problem. Putting gd in the data section avoids one of the above problems. Others may have more insight here. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
Am Do., 21. Feb. 2019, 03:48 hat Simon Glass geschrieben: > Hi Simon, > > On Mon, 11 Feb 2019 at 14:00, Simon Goldschmidt > wrote: > > > > Am 09.11.2018 um 19:43 schrieb Tom Rini: > > > On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote: > > > > > >> There is a strange feature to set global_data to a data-section > variable > > >> early in SPL. This only works if SPL actually has access to SRAM > which is > > >> not the case on x86, for eaxmple. Add a comment to this effect. > > > > Seems like I missed that one back in October. > > > > Does anyone have an idea why this variable ('static bd_t bdata') is > > hard-coded into section ".data"? To me this seems pretty unportable... > > > > For example, for a specific feature on socfpga (warm reboot CRC check), > > I would prefer to have the ".data" section empty and put 'bdata' into > > bss. This is currently not possible. > > > > But before sending a patch that somehow changes this behaviour, I'd like > > to know why this variable is put into ".data" instead of ".bss" > > BSS normally appears after data and is not actually part of the > u-boot-spi.bin image. If the device tree is appended to SPL then it > overlaps with BSS. Thus accessing BSS overwrites the DT. There are two > features to get around that - one is to pad out the BSS space so that: > > cat u-boot-spl-nodtb.bin u-boot-spl.dtb >u-boot-spl.bin > > places the DT after BSS. > > The other is to specify an address (typically in SDRAM) for BSS, so > that it is completely separate from the image. > > Note also that BSS is not always available in SPL. For example if SPL > is running from flash then the BSS may be mapped either to SDRAM > (which is not set up until later in SPL) or flash (which is not > writable at all). Of course the data section has the same problem. > Ok, thanks for the insight. Putting gd in the data section avoids one of the above problems. > Actually it's bd here, not gd, so it's a little less widely used. And while I can follow your explanation, it convinces me even more that this should be configurable, not hadcoded. It wastes space in SPL binary and may not even work on some platforms... Regards, Simon > Others may have more insight here. > > Regards, > Simon > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [U-Boot,v2,11/23] spl: Add a comment to spl_set_bd()
Hi Simon, On Wed, 20 Feb 2019 at 22:44, Simon Goldschmidt wrote: > > > > Am Do., 21. Feb. 2019, 03:48 hat Simon Glass geschrieben: >> >> Hi Simon, >> >> On Mon, 11 Feb 2019 at 14:00, Simon Goldschmidt >> wrote: >> > >> > Am 09.11.2018 um 19:43 schrieb Tom Rini: >> > > On Tue, Oct 02, 2018 at 05:22:41AM -0600, Simon Glass wrote: >> > > >> > >> There is a strange feature to set global_data to a data-section variable >> > >> early in SPL. This only works if SPL actually has access to SRAM which >> > >> is >> > >> not the case on x86, for eaxmple. Add a comment to this effect. >> > >> > Seems like I missed that one back in October. >> > >> > Does anyone have an idea why this variable ('static bd_t bdata') is >> > hard-coded into section ".data"? To me this seems pretty unportable... >> > >> > For example, for a specific feature on socfpga (warm reboot CRC check), >> > I would prefer to have the ".data" section empty and put 'bdata' into >> > bss. This is currently not possible. >> > >> > But before sending a patch that somehow changes this behaviour, I'd like >> > to know why this variable is put into ".data" instead of ".bss" >> >> BSS normally appears after data and is not actually part of the >> u-boot-spi.bin image. If the device tree is appended to SPL then it >> overlaps with BSS. Thus accessing BSS overwrites the DT. There are two >> features to get around that - one is to pad out the BSS space so that: >> >> cat u-boot-spl-nodtb.bin u-boot-spl.dtb >u-boot-spl.bin >> >> places the DT after BSS. >> >> The other is to specify an address (typically in SDRAM) for BSS, so >> that it is completely separate from the image. >> >> Note also that BSS is not always available in SPL. For example if SPL >> is running from flash then the BSS may be mapped either to SDRAM >> (which is not set up until later in SPL) or flash (which is not >> writable at all). Of course the data section has the same problem. > > > Ok, thanks for the insight. > >> Putting gd in the data section avoids one of the above problems. > > > Actually it's bd here, not gd, so it's a little less widely used. > > And while I can follow your explanation, it convinces me even more that this > should be configurable, not hadcoded. It wastes space in SPL binary and may > not even work on some platforms... Yes my comments were on gd, sorry about that. Ideally it would default to something safe (i.e. not being in BSS or data). The best things is probably to malloc() it, assuming that this is enabled in SPL. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot