| Message ID | 1293779628-9804-1-git-send-email-macpaul@andestech.com |
|---|---|
| State | Rejected |
| Headers | show |
On Friday, December 31, 2010 02:13:48 Macpaul Lin wrote: > There are several way to reset the u-boot. > Some platform will use watchdog timeout to reset the system. > Some platfrom will jump to 0x0 to reload the u-boot. > Some platform will jump to CONFIG_SYS_TEXT_BASE to do fast reset. > > This patch fixed the problem of static varible didn't cleared on > the platforms which "CONFIG_SKIP_LOWLEVEL_INIT" is enabled and do > software reset by simply jump to the address "CONFIG_SYS_TEXT_BASE". i have no idea what "CONFIG_SKIP_LOWLEVEL_INIT" is, so i cant really comment on what you're trying to do > +void reinit_bin() needs to be (void) > +extern void reinit_malloc_pool() here too, and putting "extern" on the actual func def makes no sense -mike
Dear Macpaul Lin, In message <1293779628-9804-1-git-send-email-macpaul@andestech.com> you wrote: > There are several way to reset the u-boot. To avoid confusion, we should try and agree on certain terms. Reset means (at least in U-Boot context) to bring a system to an initial state, similar to what whappens when you press the reset button. "Initial state" also means that the hardware, including memory and peripherals, are reset to an initial state where they can reliably be accessed. As a result of a "reset", the CPU will begin execution at it's reset entry point, loading code from the configured boot device. This differs from "restart", where an existing instance of a software component (which might be an U-Boot image loaded to memory) begins execution from some starting point. In U-Boot context, a different term for a restart might be "warm start" or "warm boot". U-Boot only supports "reset". It does NOT support any way of restart. THere are a number of reasons for that: 1) We do not keep a copy of the initial state of the data segment. Thus any changes to data that are used by the code as indicators for steps already performed will remain, which will cause such steps will be missing or will run based on incorrect assumptions. 2) There are situations where a restart will not be possible, For example, if you have a system which boots from NOR flash, and you attempt a restart when the flash chips just happen to be in some command mode. The you cannot read normal data from them, and the CPU will not be able to fetch instructions from the boot device. > Some platform will use watchdog timeout to reset the system. Correct. Using a watchdog reset is one of the common ways to cause a hard reset on some systems. > Some platfrom will jump to 0x0 to reload the u-boot. Wrong. This may attempt to restart U-Boot, and if you are lucky it will eventually even load a fresh copy of U-Boot to RAM. But this is highly SoC and board specific, and it is only a restart, and NOT a reset. > Some platform will jump to CONFIG_SYS_TEXT_BASE to do fast reset. Wrong. This is not a reset either. Even worse, here you don;t even attempt to reload U-Boot and thus to re-initialize the data and bss segments, which is just begging for all sorts of trouble. This is a fundamentally broken design. Please do not try to paper over problems, fix the design. > This patch fixed the problem of static varible didn't cleared on > the platforms which "CONFIG_SKIP_LOWLEVEL_INIT" is enabled and do > software reset by simply jump to the address "CONFIG_SYS_TEXT_BASE". It makes no sense to try and fix individual variables to their initial state - you would have to reload the whole data segment from some copy holding it's original values, and you have to re-initialize the whole bss segment to zero. [And even then you are still trying to o somthing which is unsupported.] > This patch also introduce a new define named "CONFIG_FAST_RESET" for > developer to distinguish this method from other reset methods. This is a misnomer in severasl ways: it does not perform any reset operation of the system (but attempts to perform a (unsupported) restart. > +- CONFIG_FAST_RESET > + If both CONFIG_SKIP_LOWLEVEL_INIT and this varible is defined, > + the system could use do fast software reset by simply reinit > + dlmalloc module then jump to the starting address > + "CONFIG_SYS_TEXT_BASE". > + > + To use this feature, the extern function "reinit_malloc_pool()" > + in dlmalloc.c should be called inside > + reset_cpu(CONFIG_SYS_TEXT_BASE). This makes the problems of your design obvious. Why should resetting the processor include a reinitalization of malloc? I will not accept any such changes. Please fix your design and implement a real reset. If you want to extend the U-Boot design by adding support for a restart feature (or "warm start"), then this needs to be done right; setting of a few selected variables is definitely not sufficient. Best regards, Wolfgang Denk
HI Wolfgang, 2010/12/31 Wolfgang Denk <wd@denx.de>: > Dear Macpaul Lin, > > In message <1293779628-9804-1-git-send-email-macpaul@andestech.com> you wrote: >> There are several way to reset the u-boot. > > To avoid confusion, we should try and agree on certain terms. > > Reset means (at least in U-Boot context) to bring a system to an > initial state, similar to what whappens when you press the reset > button. "Initial state" also means that the hardware, including > memory and peripherals, are reset to an initial state where they can > reliably be accessed. As a result of a "reset", the CPU will begin > execution at it's reset entry point, loading code from the configured > boot device. > This differs from "restart", where an existing instance of a software > component (which might be an U-Boot image loaded to memory) begins > execution from some starting point. In U-Boot context, a different > term for a restart might be "warm start" or "warm boot". Thanks for your clarification. The reason of the need of implementing "warm start" is for the following scenario. There are 2 boot loaders in the same flash on our evaluation board "ADP-AG101". The flash layout of evb "ADP-AG101" is like this: The starting address of flash is 0x80400000 0x80400000: evb_loader 0x80500000: u-boot 0x80600000: non-OS demo programs 0x80800000: OS (Linux) 0x80980000: rootfs One is customized boot loader, e.g, "an evaluation boot loader" (evb_loader), which is used for quick application demo and diagnostic the peripherals.This evaluation boot loader also deals with loading and execution OS (LInux, RTOS) and non-OS application in the memory. This boot loader also load and jump to generic u-boot (CONFIG_SKIP_LOWLEVEL_INIT is enabled in this u-boot) for demo, training, and development purpose. The second boot loader is generic u-boot. Since the evaluation board is booted by evb_loader, then the memory controller and mem relocation will be initialized by evb_loader. User could evaluate the board function with evb_board, then develops the product software by using u-boot and Linux kernel to simulate the real product behavior. Once the user has done development process, he/she can replace the evb_loader with real generic u-boot (with CONFIG_SKIP_LOWLEVEL_INIT is disabled). That's why we may need "warm start" or "warm boot" which let user restart the u-boot without resetting the board and jumping to the evb_loader again.. Even there exist a need with "warm boot", the NDS32 architecture still do CPU and memory reset. In the current reset_cpu() function whihc NDS32 has provided (it could be hooked to watchdog reset or warm boot), reset still do the following reset process. 1. do_reset(). (cpu.c) 2. disable interrupts. (cpu.c) 3. reset_cpu(). (cpu.c) 3.1. invalidate I/D cache. 3.2. reset MMU, TLB, 3.3. flush TLB, 3.4. reset cache control registers. if use watchdog reset 3.5 telling watchdog to reset the hardware. 4. system loads the boot loader from flash. (start.S) else if use warm boot. 3.5. jump the to the CONFIG_SYS_TEXT_BASE, 4. execute start up code in memory. (start.S) > U-Boot only supports "reset". It does NOT support any way of > restart. There are a number of reasons for that: > > 1) We do not keep a copy of the initial state of the data segment. > Thus any changes to data that are used by the code as indicators > for steps already performed will remain, which will cause such > steps will be missing or will run based on incorrect assumptions. > > 2) There are situations where a restart will not be possible, For > example, if you have a system which boots from NOR flash, and you > attempt a restart when the flash chips just happen to be in some > command mode. Then you cannot read normal data from them, and the > CPU will not be able to fetch instructions from the boot device. > [deleted] > Why should resetting the processor include a reinitalization of malloc? > > I will not accept any such changes. Please fix your design and > implement a real reset. > > > If you want to extend the U-Boot design by adding support for a > restart feature (or "warm start"), then this needs to be done right; > setting of a few selected variables is definitely not sufficient. > Okay, if I want to make U-boot supports "restart feature", it seems I need to find a reasonable method to reload the data segment and clear bss. It seems the most secure method is to load code (data segment) from flash. Do you have further idea of supporting feature "restart" (warm start)?
Dear Macpaul Lin, In message <AANLkTimK=c1+aVsJcRwtPG4hPKp6Lvvu0Ji6a_7B3O_u@mail.gmail.com> you wrote: > > User could evaluate the board function with evb_board, then develops the product > software by using u-boot and Linux kernel to simulate the real product behavior. > Once the user has done development process, he/she can replace the evb_loader > with real generic u-boot (with CONFIG_SKIP_LOWLEVEL_INIT is disabled). > > That's why we may need "warm start" or "warm boot" which let user > restart the u-boot > without resetting the board and jumping to the evb_loader again.. All understood, except for the "That's why" - IO do not see any part in your explanation above why the "reset" in U-Boot should not perform a real reset operation? [Of course I wonder which features your evb_loader provides that are missing in U-Boot and cannot be added there, but that's your decision.] > Even there exist a need with "warm boot", the NDS32 architecture still do > CPU and memory reset. In the current reset_cpu() function whihc NDS32 > has provided > (it could be hooked to watchdog reset or warm boot), reset still do > the following reset > process. Why? All this is bogus if you perform a real hardware reset as you are supposed to do. > > If you want to extend the U-Boot design by adding support for a > > restart feature (or "warm start"), then this needs to be done right; > > setting of a few selected variables is definitely not sufficient. > > Okay, if I want to make U-boot supports "restart feature", it seems I need to > find a reasonable method to reload the data segment and clear bss. It seems > the most secure method is to load code (data segment) from flash. ...or form another copy of the data segment. > Do you have further idea of supporting feature "restart" (warm start)? Well, I can just say that I see no need for such a fature - doing a real reset is much more reliable and convenient, as you do not need any extra code. Best regards, Wolfgang Denk
Dear Wolfgang, 2011/1/3 Wolfgang Denk <wd@denx.de>: > Dear Macpaul Lin, > > In message <AANLkTimK=c1+aVsJcRwtPG4hPKp6Lvvu0Ji6a_7B3O_u@mail.gmail.com> you wrote: >> >> User could evaluate the board function with evb_board, then develops the product >> software by using u-boot and Linux kernel to simulate the real product behavior. >> Once the user has done development process, he/she can replace the evb_loader >> with real generic u-boot (with CONFIG_SKIP_LOWLEVEL_INIT is disabled). >> >> That's why we may need "warm start" or "warm boot" which let user >> restart the u-boot >> without resetting the board and jumping to the evb_loader again.. > > All understood, except for the "That's why" - IO do not see any part > in your explanation above why the "reset" in U-Boot should not perform > a real reset operation? Sorry for the description before was not clear enough. I agree that "reset" in u-boot should perform real reset no matter the u-boot is the 1st boot loader or the 2nd boot loader. Of course, the flaw of the original design should be changed. However, according to the current design specification, AndesTech is trying to provide a feature which made user could "warm restart" back into u-boot when u-boot is the 2nd boot loader. > [Of course I wonder which features your evb_loader provides that are > missing in U-Boot and cannot be added there, but that's your decision.] The start.S and lowlevel_init.S in evb_loader and in u-boot are the same. They perform code copying, relocation, setup stack, and etc. Only the difference is NDS32 support (CONFIG_SKIP_LOWLEVEL_INIT) which skips some memory controller and remap configuration. If user disable CONFIG_SKIP_LOWLEVEL_INIT, u-boot could replace that evb_loader for mass production purpose. >> Even there exist a need with "warm boot", the NDS32 architecture still do >> CPU and memory reset. In the current reset_cpu() function whihc NDS32 >> has provided >> (it could be hooked to watchdog reset or warm boot), reset still do >> the following reset >> process. > > Why? All this is bogus if you perform a real hardware reset as you > are supposed to do. The above is just the description of the current design. If we want to have a warm restart, I fully agree there must be another program logic or API inside the u-boot to support the correctness of "restart". > Well, I can just say that I see no need for such a fature - doing a > real reset is much more reliable and convenient, as you do not need > any extra code. > > Best regards, > > Wolfgang Denk > Okay, understood.
Dear Macpaul Lin, In message <AANLkTinXXE=CLRNXe48AHBOM9m5JGYxncOU+x0-6c1T=@mail.gmail.com> you wrote: > > However, according to the current design specification, AndesTech is trying to > provide a feature which made user could "warm restart" back into u-boot when > u-boot is the 2nd boot loader. What would be the benefit of such an option? > The start.S and lowlevel_init.S in evb_loader and in u-boot are the same. Then your evb_loader is GPLed, too? Best regards, Wolfgang Denk
Dear Wolfgang, 2011/1/3 Wolfgang Denk <wd@denx.de>: > Dear Macpaul Lin, > > In message <AANLkTinXXE=CLRNXe48AHBOM9m5JGYxncOU+x0-6c1T=@mail.gmail.com> you wrote: >> >> However, according to the current design specification, AndesTech is trying to >> provide a feature which made user could "warm restart" back into u-boot when >> u-boot is the 2nd boot loader. > > What would be the benefit of such an option? For example, if user have did some env setting manually before booting OS, and then he want's to drop all those previous env modification. He just need to restart the u-boot which is already in the memory, then the default env parameters will comes back. >> The start.S and lowlevel_init.S in evb_loader and in u-boot are the same. > > Then your evb_loader is GPLed, too? Yes, although the evb_loader department have created two different evb_loader, one is proprietary license, and the other is GPL license. Which evb_loader I'm working with is the GPL one. All our customers who bought the evb could choose which one software package to be shipped (supported) with. However, I'm not belong to that department. > > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > Es gibt immer genug fuer die Beduerfnisse aller, aber niemals genug > fuer die Gier einzelner. -- Ghandi >
Dear Macpaul Lin, In message <AANLkTin9L0NhBASkgFDP6xR6W=LjRsoYeNkS_RRWpf_b@mail.gmail.com> you wrote: > > > What would be the benefit of such an option? > > For example, if user have did some env setting manually before booting OS, > and then he want's to drop all those previous env modification. He > just need to restart the > u-boot which is already in the memory, then the default env parameters > will comes back. Entering the command "env default -f" will do the same. No restart needed at all. Anything else? Best regards, Wolfgang Denk
Dear Wolfgang, 2011/1/3 Wolfgang Denk <wd@denx.de>: > Dear Macpaul Lin, > > In message <AANLkTin9L0NhBASkgFDP6xR6W=LjRsoYeNkS_RRWpf_b@mail.gmail.com> you wrote: >> > > Anything else? > Thanks for your explanation about this during your vacation. I'll report this issue to suggestion the company to modify the origin spec. And also tell the customer to use the correct command instead of their requests.
Dear Macpaul Lin, In message <AANLkTikX=V1ZLj70dGfdOMTLvA5b1n6MT6Xi8ncrjmGU@mail.gmail.com> you wrote: > > Thanks for your explanation about this during your vacation. You're welcome. > I'll report this issue to suggestion the company to modify the origin spec. > And also tell the customer to use the correct command instead of their requests. Actually this is a pretty common pattern: when a customer asks for some new feature, it always makes sense to ask what exactly he is trying to do with that feature, and then think if the same purpose might be acchieved using existing, standard commands. Best regards, Wolfgang Denk
diff --git a/README b/README index 1acf9a3..ec00294 100644 --- a/README +++ b/README @@ -2846,6 +2846,16 @@ Low Level (hardware related) configuration options: other boot loader or by a debugger which performs these initializations itself. +- CONFIG_FAST_RESET + If both CONFIG_SKIP_LOWLEVEL_INIT and this varible is defined, + the system could use do fast software reset by simply reinit + dlmalloc module then jump to the starting address + "CONFIG_SYS_TEXT_BASE". + + To use this feature, the extern function "reinit_malloc_pool()" + in dlmalloc.c should be called inside + reset_cpu(CONFIG_SYS_TEXT_BASE). + - CONFIG_PRELOADER Modifies the behaviour of start.S when compiling a loader that is executed before the actual U-Boot. E.g. when diff --git a/common/dlmalloc.c b/common/dlmalloc.c index e9bab09..97849a5 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -3233,6 +3233,30 @@ int mALLOPt(param_number, value) int param_number; int value; } } +#if defined(CONFIG_FAST_RESET) || defined(CONFIG_SKIP_LOWLEVEL_INIT) +void reinit_bin() +{ + int i = 0; + + av_[0] = 0; + av_[1] = 0; + + for (i = 0; i < NAV ; i++) { + av_[i*2+2] = bin_at(i); + av_[i*2+2+1] = bin_at(i); + } +} + +extern void reinit_malloc_pool() +{ + /* fast reset sbrk_base */ + sbrk_base = (char *)(-1); + + /* re-initial bin */ + reinit_bin(); +} +#endif + /* History:
There are several way to reset the u-boot. Some platform will use watchdog timeout to reset the system. Some platfrom will jump to 0x0 to reload the u-boot. Some platform will jump to CONFIG_SYS_TEXT_BASE to do fast reset. This patch fixed the problem of static varible didn't cleared on the platforms which "CONFIG_SKIP_LOWLEVEL_INIT" is enabled and do software reset by simply jump to the address "CONFIG_SYS_TEXT_BASE". This patch also introduce a new define named "CONFIG_FAST_RESET" for developer to distinguish this method from other reset methods. Signed-off-by: Macpaul Lin <macpaul@andestech.com> --- README | 10 ++++++++++ common/dlmalloc.c | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 0 deletions(-)