[U-Boot-Users] [PATCH] Blackfin: implement go/boote wrappers

Override the default go/boote handlers as we want to disable both data and instruction cache before executing external programs (since Blackfin cache handling requires a software handler in U-Boot which may be overwritten).
Signed-off-by: Mike Frysinger vapier@gentoo.org --- lib_blackfin/Makefile | 2 +- lib_blackfin/{bootm.c => boot.c} | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletions(-) rename lib_blackfin/{bootm.c => boot.c} (80%)
diff --git a/lib_blackfin/Makefile b/lib_blackfin/Makefile index 3617104..879c37f 100644 --- a/lib_blackfin/Makefile +++ b/lib_blackfin/Makefile @@ -37,7 +37,7 @@ SOBJS-y += memmove.o SOBJS-y += memset.o
COBJS-y += board.o -COBJS-y += bootm.o +COBJS-y += boot.o COBJS-y += cache.o COBJS-y += muldi3.o COBJS-y += post.o diff --git a/lib_blackfin/bootm.c b/lib_blackfin/boot.c similarity index 80% rename from lib_blackfin/bootm.c rename to lib_blackfin/boot.c index ef4b112..5ea4afb 100644 --- a/lib_blackfin/bootm.c +++ b/lib_blackfin/boot.c @@ -77,3 +77,23 @@ void do_bootm_linux(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[], if (images->autostart) do_reset (cmdtp, flag, argc, argv); } + +unsigned long do_go_exec(ulong (*entry)(int, char *[]), int argc, char *argv[]) +{ + int d = dcache_status(); + int i = icache_status(); + + dcache_disable(); + icache_disable(); + + int ret = entry(argc, argv); + + if (d) dcache_enable(); + if (i) icache_enable(); + + return ret; +} +unsigned long do_bootelf_exec(ulong (*entry)(int, char *[]), int argc, char *argv[]) +{ + return do_go_exec(entry, argc, argv); +}

In message 1208583950-9454-1-git-send-email-vapier@gentoo.org you wrote:
Override the default go/boote handlers as we want to disable both data and instruction cache before executing external programs (since Blackfin cache handling requires a software handler in U-Boot which may be overwritten).
Umnmm... no. "go" is supposed to be return to U-Boot, i. e. it must not overwrite (or otherwise meddle with) any U-Boot code.
I think you should not change cache status for "go".
Best regards,
Wolfgang Denk

On Sunday 20 April 2008, Wolfgang Denk wrote:
In message 1208583950-9454-1-git-send-email-vapier@gentoo.org you wrote:
Override the default go/boote handlers as we want to disable both data and instruction cache before executing external programs (since Blackfin cache handling requires a software handler in U-Boot which may be overwritten).
Umnmm... no. "go" is supposed to be return to U-Boot, i. e. it must not overwrite (or otherwise meddle with) any U-Boot code.
I think you should not change cache status for "go".
reality is that people often times use go to execute their binary blobs. i also see it this way: go means to jump to some address and most likely never return. -mike

In message 200804202031.31936.vapier@gentoo.org you wrote:
Umnmm... no. "go" is supposed to be return to U-Boot, i. e. it must not overwrite (or otherwise meddle with) any U-Boot code.
I think you should not change cache status for "go".
reality is that people often times use go to execute their binary blobs. i also see it this way: go means to jump to some address and most likely never return.
This may be your private interpretation, but it is wrong.
Intended and documented behaviour is that "go" is used to start standalone applications, which are supposed to return.
Best regards,
Wolfgang Denk

On Monday 21 April 2008, Wolfgang Denk wrote:
In message 200804202031.31936.vapier@gentoo.org you wrote:
Umnmm... no. "go" is supposed to be return to U-Boot, i. e. it must not overwrite (or otherwise meddle with) any U-Boot code.
I think you should not change cache status for "go".
reality is that people often times use go to execute their binary blobs. i also see it this way: go means to jump to some address and most likely never return.
This may be your private interpretation, but it is wrong.
Intended and documented behaviour is that "go" is used to start standalone applications, which are supposed to return.
then how exactly are people supposed to execute their flat binaries ? none of the other boot methods allow for straight jumps that i'm aware of. otherwise i'm going to have to add even more bloat to add a slight variation on the go command: one where the documentation doesnt require it to return. -mike

In message 200804210125.45077.vapier@gentoo.org you wrote:
Intended and documented behaviour is that "go" is used to start standalone applications, which are supposed to return.
then how exactly are people supposed to execute their flat binaries ? none of
I am not an expert for the Blackfin implementation. But "go" is for standalone applications. The "boot*" commands are intended to boot OS images.
the other boot methods allow for straight jumps that i'm aware of. otherwise i'm going to have to add even more bloat to add a slight variation on the go command: one where the documentation doesnt require it to return.
That makes no sense to me. If you want to boot some OS, use a "boot*" commands, not "go" or the like.
Best regards,
Wolfgang Denk

On Monday 21 April 2008, Wolfgang Denk wrote:
In message 200804210125.45077.vapier@gentoo.org you wrote:
Intended and documented behaviour is that "go" is used to start standalone applications, which are supposed to return.
then how exactly are people supposed to execute their flat binaries ? none of
I am not an expert for the Blackfin implementation. But "go" is for standalone applications. The "boot*" commands are intended to boot OS images.
the other boot methods allow for straight jumps that i'm aware of. otherwise i'm going to have to add even more bloat to add a slight variation on the go command: one where the documentation doesnt require it to return.
That makes no sense to me. If you want to boot some OS, use a "boot*" commands, not "go" or the like.
i'm not talking about operating system code, i'm talking about flat binary applications that run bare metal. something like "u-boot.bin" where it's just a binary blob that gets loaded and u-boot jumps to it and never returns.
since "go" isnt appropriate according to you (and seeing as how none of the boot commands are appropriate), then the only next step is to implement a "jump" command that is exactly like "go" except it doesnt return. -mike

In message 200804210349.27495.vapier@gentoo.org you wrote:
That makes no sense to me. If you want to boot some OS, use a "boot*" commands, not "go" or the like.
i'm not talking about operating system code, i'm talking about flat binary applications that run bare metal. something like "u-boot.bin" where it's just a binary blob that gets loaded and u-boot jumps to it and never returns. since "go" isnt appropriate according to you (and seeing as how none of the> boot commands are appropriate), then the only next step is to implement a "jump" command that is exactly like "go" except it doesnt return.
This makes no sense. If it is ``exactly like "go"'' it doesn't matter if the code returns or not (and actually this is what I'm trying to point out all the time).
It's just that "go" shall retain the standard U-Boot environment for application it runs, and that the applications need to take care if they need to meddle with interrupts, exception handlers, etc.
Best regards,
Wolfgang Denk

On Monday 21 April 2008, Wolfgang Denk wrote:
In message 200804210349.27495.vapier@gentoo.org you wrote:
That makes no sense to me. If you want to boot some OS, use a "boot*" commands, not "go" or the like.
i'm not talking about operating system code, i'm talking about flat binary applications that run bare metal. something like "u-boot.bin" where it's just a binary blob that gets loaded and u-boot jumps to it and never returns. since "go" isnt appropriate according to you (and seeing as how none of the> boot commands are appropriate), then the only next step is to implement a "jump" command that is exactly like "go" except it doesnt return.
This makes no sense. If it is ``exactly like "go"'' it doesn't matter if the code returns or not (and actually this is what I'm trying to point out all the time).
the obvious implication is that i would add the cache disabling hooks to the jump command instead of the go command since you wont allow the hook around go.
It's just that "go" shall retain the standard U-Boot environment for application it runs, and that the applications need to take care if they need to meddle with interrupts, exception handlers, etc.
U-Boot sets up no interrupts and the only exceptions that occur on the Blackfin are for cache handling. disabling the caches forces a sane environment where applications dont have to deal with a chicken/egg of having to setup exception handlers before taking an exception. we've seen it already with customers. -mike

In message 200804210541.41085.vapier@gentoo.org you wrote:
This makes no sense. If it is ``exactly like "go"'' it doesn't matter if the code returns or not (and actually this is what I'm trying to point out all the time).
the obvious implication is that i would add the cache disabling hooks to the jump command instead of the go command since you wont allow the hook around go.
So it would NOT be ``exactly like "go"''.
Providing both a "go" and a "jump" command which differ just in cahce handling seems broken to me. If you want to add such a feature, then I recommend to do it as part of "go", but make it optional, i. e. in- troduce a new optional argument to the "go" command, something like
go [ -cache={off,d-off,i-off,on,d-on,i-on} ] addr [ args ... ]
It's just that "go" shall retain the standard U-Boot environment for application it runs, and that the applications need to take care if they need to meddle with interrupts, exception handlers, etc.
U-Boot sets up no interrupts and the only exceptions that occur on the Blackfin are for cache handling. disabling the caches forces a sane
There is more procvessors in this world than just Blackfin, and others *do* enable interrupts, etc. It is important to me that implementations behave the same no matter which architecture you are using.
Best regards,
Wolfgang Denk

On Monday 21 April 2008, Wolfgang Denk wrote:
In message 200804210541.41085.vapier@gentoo.org you wrote:
This makes no sense. If it is ``exactly like "go"'' it doesn't matter if the code returns or not (and actually this is what I'm trying to point out all the time).
the obvious implication is that i would add the cache disabling hooks to the jump command instead of the go command since you wont allow the hook around go.
So it would NOT be ``exactly like "go"''.
well, duh. my point was that you're making "go" get duplicated just to conform to documentation. the command name itself "go" doesnt really conjurn up usage of "executing an application and returning" ... fits better with "go to this location and never come back". "run" probably would have been a better name. but that's hindsight for you.
Providing both a "go" and a "jump" command which differ just in cahce handling seems broken to me. If you want to add such a feature, then I recommend to do it as part of "go", but make it optional, i. e. in- troduce a new optional argument to the "go" command, something like
go [ -cache={off,d-off,i-off,on,d-on,i-on} ] addr [ args ... ]
cache is just an example. other arches may want to do other sort of "system breakdown/cleanup" before relinquishing control. option flags to commands really doesnt fit the style of u-boot (i expect that sort of painful option parsing in redboot, not really u-boot).
so we can do: go [-noret] addr [args...] or we can add "jump" to cmd_boot.c and merge the differences by just using a function pointer to "do_go_exec" or "do_jump_exec".
It's just that "go" shall retain the standard U-Boot environment for application it runs, and that the applications need to take care if they need to meddle with interrupts, exception handlers, etc.
U-Boot sets up no interrupts and the only exceptions that occur on the Blackfin are for cache handling. disabling the caches forces a sane
There is more procvessors in this world than just Blackfin, and others *do* enable interrupts, etc. It is important to me that implementations behave the same no matter which architecture you are using.
i never said Blackfin was the only thing that mattered. in fact, my goal is to make it so that people using this facility get a more standard initial environment before they start taking over the system. i guess i wont point out the U-Boot policy about not using interrupts ... -mike

On Monday 21 April 2008, Mike Frysinger wrote:
On Monday 21 April 2008, Wolfgang Denk wrote:
Providing both a "go" and a "jump" command which differ just in cahce handling seems broken to me. If you want to add such a feature, then I recommend to do it as part of "go", but make it optional, i. e. in- troduce a new optional argument to the "go" command, something like
go [ -cache={off,d-off,i-off,on,d-on,i-on} ] addr [ args ... ]
cache is just an example. other arches may want to do other sort of "system breakdown/cleanup" before relinquishing control. option flags to commands really doesnt fit the style of u-boot (i expect that sort of painful option parsing in redboot, not really u-boot).
so we can do: go [-noret] addr [args...] or we can add "jump" to cmd_boot.c and merge the differences by just using a function pointer to "do_go_exec" or "do_jump_exec".
untested poc attached -mike

In message 200804210659.43481.vapier@gentoo.org you wrote:
or we can add "jump" to cmd_boot.c and merge the differences by just using a function pointer to "do_go_exec" or "do_jump_exec".
untested poc attached
Don't waste efforts on this. I will not accept it.
Best regards,
Wolfgang Denk

In message 200804210635.08416.vapier@gentoo.org you wrote:
well, duh. my point was that you're making "go" get duplicated just to conform to documentation. the command name itself "go" doesnt really conjurn up usage of "executing an application and returning" ... fits better with "go to this location and never come back". "run" probably would have been a better name. but that's hindsight for you.
You are right. Today I would probably name this "call".
go [ -cache={off,d-off,i-off,on,d-on,i-on} ] addr [ args ... ]
cache is just an example. other arches may want to do other sort of "system breakdown/cleanup" before relinquishing control. option flags to commands
As the situation is, other arches seem to be just fine as it. It's only BF which needs a pork sausage.
really doesnt fit the style of u-boot (i expect that sort of painful option parsing in redboot, not really u-boot).
I don;t like it either, but just ading random new commands is even worse.
so we can do: go [-noret] addr [args...]
Ummm... "no return" programs is just an example. Other pograms may want to do other sort of "system breakdown/cleanup" before relin- quishing control.
or we can add "jump" to cmd_boot.c and merge the differences by just using > a function pointer to "do_go_exec" or "do_jump_exec".
Adn then we add "call" and "exec" and "do" and so oon just for other needed options? I say no.
i never said Blackfin was the only thing that mattered. in fact, my goal is to make it so that people using this facility get a more standard initial environment before they start taking over the system. i guess i wont point out the U-Boot policy about not using interrupts ...
Are you aware that U-Boot does use interrupts here and there? That we actually provide functions to register interrupt handlers to standalone programs, etc. ?
Best regards,
Wolfgang Denk

On Monday 21 April 2008, Wolfgang Denk wrote:
In message 200804210635.08416.vapier@gentoo.org you wrote:
go [ -cache={off,d-off,i-off,on,d-on,i-on} ] addr [ args ... ]
cache is just an example. other arches may want to do other sort of "system breakdown/cleanup" before relinquishing control. option flags to commands
As the situation is, other arches seem to be just fine as it. It's only BF which needs a pork sausage.
this is incorrect. any architecture which wants to overwrite u-boot sanely needs to worry about causing exceptions or interrupts. i just want to make the obvious easy since it has caused more than enough support requests.
really doesnt fit the style of u-boot (i expect that sort of painful option parsing in redboot, not really u-boot).
I don;t like it either, but just ading random new commands is even worse.
so we can do: go [-noret] addr [args...]
Ummm... "no return" programs is just an example. Other pograms may want to do other sort of "system breakdown/cleanup" before relin- quishing control.
or we can add "jump" to cmd_boot.c and merge the differences by just using > a function pointer to "do_go_exec" or "do_jump_exec".
Adn then we add "call" and "exec" and "do" and so oon just for other needed options? I say no.
then implement whatever. in the mean time i'll keep forking the Blackfin code.
i never said Blackfin was the only thing that mattered. in fact, my goal is to make it so that people using this facility get a more standard initial environment before they start taking over the system. i guess i wont point out the U-Boot policy about not using interrupts ...
Are you aware that U-Boot does use interrupts here and there? That we actually provide functions to register interrupt handlers to standalone programs, etc. ?
yes i'm aware, but we arent talking about standalone applications here, so those really dont matter. -mike

In message 200804211444.43917.vapier@gentoo.org you wrote:
Adn then we add "call" and "exec" and "do" and so oon just for other needed options? I say no.
then implement whatever. in the mean time i'll keep forking the Blackfin code.
OK. I offered you a way out. It's your decision.
I tried understanding what you are trying to do, and even though I feel it's not exactly an important or frequently used feature for most of the users I tried to come up with a compromize that allows you to do what you want to do without hurting others and without polluting the command name space.
I'm sorry that I wasted my time on thinking about this.
Are you aware that U-Boot does use interrupts here and there? That we actually provide functions to register interrupt handlers to standalone programs, etc. ?
yes i'm aware, but we arent talking about standalone applications here, so those really dont matter.
You have a really, really funny way of deciding what matters and what not. All the things you have in mind do matter (even if they are documented to be unsupported), while other existing and legal ways of doing things don't matter. That doesn't make working with you an easy task.
Best regards,
Wolfgang Denk

On Monday 21 April 2008, Wolfgang Denk wrote:
In message 200804211444.43917.vapier@gentoo.org you wrote:
Adn then we add "call" and "exec" and "do" and so oon just for other needed options? I say no.
then implement whatever. in the mean time i'll keep forking the Blackfin code.
OK. I offered you a way out. It's your decision.
I tried understanding what you are trying to do, and even though I feel it's not exactly an important or frequently used feature for most of the users I tried to come up with a compromize that allows you to do what you want to do without hurting others and without polluting the command name space.
i consider the cache one aspect of it. the users shouldnt have to know "oh i gotta turn off cache", they just have to know "i'm loading up my code and it's going to take over the system". that is why a "-noret" flags makes sense instead of trying to break down specific aspects. also adding a myriad of cache options to one function achieves the same thing as doing: dcache off; icache off; go <address>
Are you aware that U-Boot does use interrupts here and there? That we actually provide functions to register interrupt handlers to standalone programs, etc. ?
yes i'm aware, but we arent talking about standalone applications here, so those really dont matter.
You have a really, really funny way of deciding what matters and what not. All the things you have in mind do matter (even if they are documented to be unsupported), while other existing and legal ways of doing things don't matter. That doesn't make working with you an easy task.
it's really quite simple. the need is to jump to an address to execute a blob and never return to u-boot. what features are available to standalone u-boot applications (while useful) dont really matter. such applications rely on u-boot remaining resident which is the opposite of what i'm talking about. -mike

In message 200804211658.44036.vapier@gentoo.org you wrote:
I tried understanding what you are trying to do, and even though I feel it's not exactly an important or frequently used feature for most of the users I tried to come up with a compromize that allows you to do what you want to do without hurting others and without polluting the command name space.
i consider the cache one aspect of it. the users shouldnt have to know "oh i gotta turn off cache", they just have to know "i'm loading up my code and
I agree that cache is just one aspect; I disagree that users shouldn't know what they are doing. On contrary, I want to give them all necessary control to do what they may want to do. It is a matter of higher software levels to provide more convenient abstractions.
it's going to take over the system". that is why a "-noret" flags makes
Who says that *my* application which doesn;t return does the same as your application? Maybe I *want* to have the caches on for perfor- mance?
I accept that the default settings may be not optimal for your use case, so please accept that your settings may not be always optimal, either. As a solution I imagine options to the "go" command. If you consider this too complicated for your users, please feel free to provide an alias in an envrionment variable which your users can "run".
sense instead of trying to break down specific aspects. also adding a myriad of cache options to one function achieves the same thing as doing: dcache off; icache off; go <address>
Ah! You see - there is no need at all for any changes - you can put *that* sequence in a variable and run it.
So what exactly was the reason we need a new command?
it's really quite simple. the need is to jump to an address to execute a blob and never return to u-boot. what features are available to standalone u-boot
"go" does that. If your application does not attempt to return, that's fine.
applications (while useful) dont really matter. such applications rely on u-boot remaining resident which is the opposite of what i'm talking about.
But then it doesn't hurt, does it? Your application can do anything it wants as long as it does not attempt to return to U-Boot.
I see zero justification for a new command (and very little for changes to the implementation of "go", but I am still willing to allow for such extensions if you think it's necessary or more convenient).
I thik this is my last word on this.
Best regards,
Wolfgang Denk

On Monday 21 April 2008, Wolfgang Denk wrote:
In message 200804211658.44036.vapier@gentoo.org you wrote:
I tried understanding what you are trying to do, and even though I feel it's not exactly an important or frequently used feature for most of the users I tried to come up with a compromize that allows you to do what you want to do without hurting others and without polluting the command name space.
i consider the cache one aspect of it. the users shouldnt have to know "oh i gotta turn off cache", they just have to know "i'm loading up my code and
I agree that cache is just one aspect; I disagree that users shouldn't know what they are doing. On contrary, I want to give them all necessary control to do what they may want to do. It is a matter of higher software levels to provide more convenient abstractions.
yes, but the fact that we have to turn off caches is not for performance reasons. users should not need to be aware of these system deficiencies which is why i'm proposing a "noret" flag.
it's going to take over the system". that is why a "-noret" flags makes
Who says that *my* application which doesn;t return does the same as your application? Maybe I *want* to have the caches on for perfor- mance?
i'm not concerned with the performance aspect. caches on the Blackfin are intertwined with protection, and protection handlers require a software vector to process exceptions. the act of executing a program that takes over the system means the exception handler is no longer reliable so i have to guarantee it wont be called. based on information given to me from our hardware team, this can only be accomplished implicitly by disabling caches.
I accept that the default settings may be not optimal for your use case, so please accept that your settings may not be always optimal, either. As a solution I imagine options to the "go" command. If you consider this too complicated for your users, please feel free to provide an alias in an envrionment variable which your users can "run".
i completely accept that the patch i posted to turn off caches for all things to "go" result in a suboptimal environment for standalone u-boot applications. for Blackfin usage though, that takes a back seat to things silently crashing.
applications (while useful) dont really matter. such applications rely on u-boot remaining resident which is the opposite of what i'm talking about.
But then it doesn't hurt, does it? Your application can do anything it wants as long as it does not attempt to return to U-Boot.
except that it may inadvertently do so on Blackfin. -mike
participants (2)
-
Mike Frysinger
-
Wolfgang Denk