cmd_bootm: Add command line arguments to Plan 9
PATCH 1/1 cmd_bootm: Add command line arguments to Plan 9
This patch introduces support for command line arguments to Plan 9. Plan 9 generally dedicates a small region of kernel memory (known as CONFADDR) for runtime configuration. A new environment variable named confaddr was introduced to indicate this location when copying arguments.
--- common/cmd_bootm.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-)
common/cmd_bootm.c +19 -0
@@ -1533,6 +1533,7 @@ static int do_bootm_plan9(int flag, int argc, char * const argv[], bootm_headers_t *images) {void (*entry_point)(void ); +char *s;if ((flag !=0 ) && (flag !=BOOTM_STATE_OS_GO ))return 1 ;
@@ -1544,6 +1545,24 @@ static int do_bootm_plan9(int flag, int argc, char * const argv[], }#endif +if ((s =getenv ("confaddr" )) !=NULL ) { +char *confaddr = (char *)simple_strtoul (s,NULL ,16 ); + +if (argc >2 ) { +int i; + + s = confaddr; +for (i =2 ; i < argc; i++) { +if (i >2 ) + *s++ ='\n' ; +strcpy (s, argv[i]); + s +=strlen (argv[i]); + } + }else if ((s =getenv ("bootargs" )) !=NULL ) { +strcpy (confaddr, s); + } + } + entry_point = (void (*)(void ))images->ep; printf("## Transferring control to Plan 9 (at address %08lx) ...\n",
Signature · 1 line
1.7.0.4
PATCH 1/1 cmd_bootm: Add command line arguments to Plan 9
Dear Steven Stallion,
Quoted 5 lines · In message <[email protected]> you wrote:
This patch introduces support for command line arguments to Plan 9. Plan 9 generally dedicates a small region of kernel memory (known as CONFADDR) for runtime configuration. A new environment variable named confaddr was introduced to indicate this location when copying arguments.
Please make this code configurable, so that people who never intend to use Plan 9 do not suffer from the increased code size.
Then please run your patch through ckeckpatch - as is, this reports two errors:
ERROR: do not use assignment in if condition
Finally:
Quoted 7 lines
common/cmd_bootm.c +7
-0+if ((s =getenv ("confaddr" )) !=NULL ) { +char *confaddr = (char *)simple_strtoul (s,NULL ,16 ); + +if (argc >2 ) { +int i; + + s = confaddr;
common/cmd_bootm.c static int do_bootm_plan9(int flag, int argc, char * const argv[], · line 1564
Quoted 10 lines
common/cmd_bootm.c +10
-0+for (i =2 ; i < argc; i++) { +if (i >2 ) + *s++ ='\n' ; +strcpy (s, argv[i]); + s +=strlen (argv[i]); + } + }else if ((s =getenv ("bootargs" )) !=NULL ) { +strcpy (confaddr, s); + } + }
This design looks pretty much inconsistent and non-intuitive to me.
There is absolutely no length checking in this code. Is the size of the available area truely unlimited? I doubt that...
Why do you make this (completely undocumented!!!) distinction between "bootm" being used with one or more arguments? Why can you not simply _always_ use bootargs ?
What if the user did not set the "confaddr" environment variable? Then the memory reagion there is left uninitialized? Does this not cause undefined behaviour when booting Plan 9?
And how does Plan 9 learn where to find this date? I cannot see how you pass this address on to Plan 9?
Even worse - this code is actually pretty dangerous: "confaddr" is neither a reserved name, nor is it in any way exotic enough to be sure nobody else would use this in his environment. But if somebody does, he will suddenly find that some (for him random) data is copied unex- pectedly to that memory range. This may cause nasty crashes or other undefined behaviour which is very difficult to debug.
Best regards,
Wolfgang Denk
Signature · 4 lines
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 at denx.de Bus error -- please leave by the rear door.
A message that is not in the archive (sent off-list or not yet received): CAGGHmKHmLAd_85SgHyC=cEUmHu8u4ENQyj3Wt3rqyVdzAtWoSw@mail.gmail.com
PATCH 1/1 cmd_bootm: Add command line arguments to Plan 9
Dear Steven,
please always keep the mailing list on Cc: - this is especially important for patch reviews, where such messages need to be picked up for patchwork, too.
Quoted 7 lines · In message <CAGGHmKHmLAd_85SgHyC=cEUmHu8u4ENQyj3Wt3rqyVdzAtWoSw@mail.gmail.com>…
Quoted 3 lines
Please make this code configurable, so that people who never intend to use Plan 9 do not suffer from the increased code size.
This is already done, if you look at the
do_bootm_plan9function, you'll see it is guarded byCONFIG_BOOTM_PLAN9. These changes only affect users that are booting Plan 9.
I see. Hm... I wonder which version of U-Boot your patch is against? The line numbers in your patch are off by at least 126 lines, and common/cmd_bootm.c has not been touched for many months ?
Quoted 5 lines
Quoted 1 line
ERROR: do not use assignment in if condition
I noticed the errors, however the style and format of my changes are the same as those in other functions in
cmd_bootm;do_bootm_netbsdprobably being the closest example. I did not watch to introduce style drift.
But we should not add more bad style code either. Feel free to send a cleanup patch for the existing code parts. In any case, do not add more such stuff.
Quoted 3 lines
It's as unlimited as you have memory :-) That said, adjacent pages to CONFADDR are zeroed out at boot, so there is no possibility of overflow once you branch to the kernel.
Adjacent pages being zeroed - that means that you _are_ actually limited to one page size?
Quoted 6 lines
Quoted 3 lines
Why do you make this (completely undocumented!!!) distinction between "bootm" being used with one or more arguments? Why can you not simply _always_ use bootargs ?
This is to support passing arguments via bootm. This behavior is consistent with NetBSD.
...which I consider a really weird desiign that IMO should not be followed without need.
If you decide to do so nevertheless, then please
1) document the behaviour
2) factor out the common code instead of copying it
Quoted 6 lines
Quoted 3 lines
What if the user did not set the "confaddr" environment variable? Then the memory reagion there is left uninitialized? Does this not cause undefined behaviour when booting Plan 9?
There are checks which account for uninitialized memory at boot. It's ugly, but it's how the OS deals with configuration. I don't like it either.
How does it detect if there are valid arguments (versus random crap) in the memory range starting at "confaddr"? I can see no checksum or similar?
Quoted 6 lines
And how does Plan 9 learn where to find this date? I cannot see how
Quoted 1 line
you pass this address on to Plan 9?
Like most things in Plan 9, it is a compiled offset (defined in
mem.h). CONFADDR is fixed, so as long as the configuration is dumped to the right location (which can change between kernels), it will work.
But then makes no sense to use a "confaddr" environment variable for this - the user has no real choice of setting this variable: either it matches the fixed CONFADDR value, in which case it works, or it is different, in which case it will silently fail. This is bad. I think you should use a CONFIG_SYS_CONFADDR constant instead.
Quoted 11 lines
Even worse - this code is actually pretty dangerous: "confaddr" is
Quoted 5 lines
neither a reserved name, nor is it in any way exotic enough to be sure nobody else would use this in his environment. But if somebody does, he will suddenly find that some (for him random) data is copied unex- pectedly to that memory range. This may cause nasty crashes or other undefined behaviour which is very difficult to debug.
True, but it's no different than loadaddr. This only affects Plan 9 users, and this behavior will be documented for the kernels which support U-Boot. I don't think there is too much danger here, though documentation will be important.
It is not documented in U-Boot. But actually I think this is void - using an envrionment variable here makes no sense in the first place.
Best regards,
Wolfgang Denk
Signature · 4 lines
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 at denx.de To be is to program.
PATCH 1/1 cmd_bootm: Add command line arguments to Plan 9
Quoted 6 lines · Wolfgang Denk
In message <CAGGHmKHmLAd_85SgHyC=
Quoted 4 lines · cEUmHu8u4ENQyj3Wt3rqyVdzAtWoSw at mail.gmail.com> you wrote:
Quoted 3 lines
Please make this code configurable, so that people who never intend to use Plan 9 do not suffer from the increased code size.
Quoted 8 lines
Quoted 3 lines
This is already done, if you look at the
do_bootm_plan9function, you'll see it is guarded byCONFIG_BOOTM_PLAN9. These changes only affect users that are booting Plan 9.I see. Hm... I wonder which version of U-Boot your patch is against? The line numbers in your patch are off by at least 126 lines, and
common/cmd_bootm.chas not been touched for many months ?
That's odd. I just double checked and my repository seems to be in sync. I'll do some poking around to make sure I didn't miss something.
Quoted 10 lines
Quoted 5 lines
ERROR: do not use assignment in if condition
I noticed the errors, however the style and format of my changes are the same as those in other functions in
cmd_bootm;do_bootm_netbsdprobably being the closest example. I did not watch to introduce style drift.But we should not add more bad style code either. Feel free to send a cleanup patch for the existing code parts. In any case, do not add more such stuff.
Will do.
Quoted 15 lines
It's as unlimited as you have memory :-) That said, adjacent pages to
Quoted 2 lines
CONFADDR are zeroed out at boot, so there is no possibility of overflow once you branch to the kernel.
Adjacent pages being zeroed - that means that you _are_ actually limited to one page size?
Quoted 5 lines
Quoted 3 lines
Why do you make this (completely undocumented!!!) distinction between "bootm" being used with one or more arguments? Why can you not simply _always_ use bootargs ?
This is to support passing arguments via bootm. This behavior is
consistent
Quoted 1 line
with NetBSD.
Quoted 8 lines
...which I consider a really weird desiign that IMO should not be followed without need.
If you decide to do so nevertheless, then please
1) document the behaviour
2) factor out the common code instead of copying it
Hmm. Are you arguing against supporting command line arguments to bootm, or that bootm should copy these arguments to bootargs prior to boot? This has actually been very useful to test changes without having to update my bootargs environment variable.
Where is the best place to document the behavior, README? The code looks as though it's common, but unfortunately it's not. plan9.ini(8) requires that arguments be terminated with a newline, the NetBSD loader uses spaces.
Quoted 4 lines
Quoted 4 lines
What if the user did not set the "confaddr" environment variable?
Quoted 2 lines
Then the memory reagion there is left uninitialized? Does this not cause undefined behaviour when booting Plan 9?
Quoted 8 lines
Quoted 1 line
There are checks which account for uninitialized memory at boot. It's
ugly,
Quoted 1 line
but it's how the OS deals with configuration. I don't like it either.
How does it detect if there are valid arguments (versus random crap) in the memory range starting at "confaddr"? I can see no checksum or similar?
Some ports (such as OMAP) will stop once it encounters the first non-ASCII character. In general, the parsing for the configuration is fairly strict and is only a small risk if a user configures the system incorrectly.
There is also something subtle in not specifying confaddr (or bootargs for that matter). Many ports support loading configuration from a FAT file system. U-Boot would be no different.
I realize this probably seems very foreign. Plan 9 is somewhat unique in that it relies on a variety of loaders. Some are more intelligent than others, but in the end, configuration has to be dropped in a known location for the kernel to parse. There is no second-stage loader, so passing a pointer has limited utility since the code to parse the config happens after the MMU has been initialized.
Quoted 7 lines
And how does Plan 9 learn where to find this date? I cannot see how
Quoted 5 lines
Quoted 1 line
you pass this address on to Plan 9?
Like most things in Plan 9, it is a compiled offset (defined in
mem.h). CONFADDR is fixed, so as long as the configuration is dumped to the right location (which can change between kernels), it will work.
Quoted 6 lines
But then makes no sense to use a "confaddr" environment variable for this - the user has no real choice of setting this variable: either it matches the fixed CONFADDR value, in which case it works, or it is different, in which case it will silently fail. This is bad. I think you should use a
CONFIG_SYS_CONFADDRconstant instead.
Ah, this is another subtlety. CONFADDR can change depending on the kernel you are booting. Some ports use as much as 64K to store configuration. Having to recompile U-Boot and reflash based on a kernel change would add a lot of complication and frustration. Having confaddr also makes it somewhat simpler to write a generic boot command which will do a fatload rather than use bootargs.
Quoted 8 lines
Even worse - this code is actually pretty dangerous: "confaddr" is
Quoted 7 lines
Quoted 5 lines
neither a reserved name, nor is it in any way exotic enough to be sure nobody else would use this in his environment. But if somebody does, he will suddenly find that some (for him random) data is copied unex- pectedly to that memory range. This may cause nasty crashes or other undefined behaviour which is very difficult to debug.
True, but it's no different than loadaddr. This only affects Plan 9
Quoted 9 lines
users,
Quoted 1 line
and this behavior will be documented for the kernels which support
U-Boot.
Quoted 2 lines
I don't think there is too much danger here, though documentation will be important.
It is not documented in U-Boot. But actually I think this is void - using an envrionment variable here makes no sense in the first place.
Is there a better method to allow confaddr to change without forcing a re-compile, or duplication if a user decides to do a fatload rather than define bootargs?
Cheers,
Steve
PATCH 1/1 cmd_bootm: Add command line arguments to Plan 9
Dear Steven,
Quoted 5 lines · In message <CAGGHmKH2NsDoQjBRUcb5XjSjcUOYfcO6CZ8MsjtXDsHh6P6PoQ@mail.gmail.com>…
Hmm. Are you arguing against supporting command line arguments to bootm, or that bootm should copy these arguments to bootargs prior to boot? This has actually been very useful to test changes without having to update my bootargs environment variable.
"bootm" has a well-known an documented API: it takes up to three
arguments: kernel address, ramdisk address, and device tree address. Nothing else.
If you want to implement a different interface, this should at least be documented - but then I doubt if this should be named "bootm". If I use it with 3 arguments, I expect the well-known behaviour, on all systems.
Quoted 3 lines
Where is the best place to document the behavior, README? The code looks as though it's common, but unfortunately it's not.
plan9.ini(8)requires that arguments be terminated with a newline, the NetBSD loader uses spaces.
A readme in doc/ is OK, too.
Quoted 7 lines
Quoted 3 lines
How does it detect if there are valid arguments (versus random crap) in the memory range starting at "confaddr"? I can see no checksum or similar?
Some ports (such as OMAP) will stop once it encounters the first non-ASCII character. In general, the parsing for the configuration is fairly strict and is only a small risk if a user configures the system incorrectly.
Hm. This is just a subterfuge for there is no security at all, and you are invoking undefined behaviour ...
Quoted 3 lines
There is also something subtle in not specifying confaddr (or bootargs for that matter). Many ports support loading configuration from a FAT file system. U-Boot would be no different.
I don't see what this has to do with it?
Quoted 6 lines
Quoted 5 lines
But then makes no sense to use a "confaddr" environment variable for this - the user has no real choice of setting this variable: either it matches the fixed CONFADDR value, in which case it works, or it is different, in which case it will silently fail. This is bad. I think you should use a
CONFIG_SYS_CONFADDRconstant instead.
Quoted 6 lines
Ah, this is another subtlety. CONFADDR can change depending on the kernel you are booting. Some ports use as much as 64K to store configuration. Having to recompile U-Boot and reflash based on a kernel change would add a lot of complication and frustration. Having confaddr also makes it somewhat simpler to write a generic boot command which will do a fatload rather than use bootargs.
You have no way to check for valid data, and you have no way to know the correct address, because it is neither fixed nor known to both the producer and the consumer? I'm sorry, but this is crap.
Quoted 3 lines
Is there a better method to allow confaddr to change without forcing a re-compile, or duplication if a user decides to do a fatload rather than define bootargs?
I'm sorry, but it appears this design is completely borked, so how should I answer this? If you have no way to know the correct adddress, and the consumer has no way to verify the data it recives, it's all just trial and error. Not exactly a robust design, that is.
Quoted 9 lines
--485b395e78db9b448d04de92b574
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable<div dir=3D"ltr">On Fri, Jun 7, 2013 at 1:16 AM, Wolfgang Denk <span dir=3D= "ltr"><<a href=3D"mailto:[email protected]" target=3D"_blank">wd at denx.de</a>>= ;</span> wrote:<br><div class=3D"gmail_extra"><div class=3D"gmail_quote"><b= lockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px = #ccc solid;padding-left:1ex">
Can you PLEASE stop sending HTML? Thanks.
Best regards,
Wolfgang Denk
Signature · 5 lines
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 at denx.de All men should freely use those seven words which have the power to make any marriage run smoothly: You know dear, you may be right.
PATCH 1/1 cmd_bootm: Add command line arguments to Plan 9
Quoted 4 lines · Wolfgang Denk
Dear Steven,
Quoted 1 line · In message <CAGGHmKH2NsDoQjBRUcb5XjSjcUOYfcO6CZ8MsjtXDsHh6P6PoQ@mail.gmail.com>…
Quoted 8 lines
Quoted 4 lines
Hmm. Are you arguing against supporting command line arguments to bootm, or that bootm should copy these arguments to bootargs prior to boot? This has actually been very useful to test changes without having to update my bootargs environment variable.
"bootm" has a well-known an documented API: it takes up to three
arguments: kernel address, ramdisk address, and device tree address. Nothing else.
Is there a reason that this should only be used by Linux? The changes I have submitted follow the same behavior as NetBSD. VxWorks and QNX also have their own quirks that don't follow the same path/usage as Linux.
Quoted 4 lines
If you want to implement a different interface, this should at least be documented - but then I doubt if this should be named "bootm". If I use it with 3 arguments, I expect the well-known behaviour, on all systems.
The usage seems to indicate this is a valid approach:
Usage:
bootm [addr [arg ...]]
If this is such a contentious change, I'm happy to drop it. I was following the NetBSD approach since it was the most similar. It would be a shame to let it go - it's useful.
Quoted 6 lines
Quoted 3 lines
Some ports (such as OMAP) will stop once it encounters the first non-ASCII character. In general, the parsing for the configuration is fairly strict and is only a small risk if a user configures the system incorrectly.
Hm. This is just a subterfuge for there is no security at all, and you are invoking undefined behaviour ...
This would only happen if a user did not configure the loader appropriately.
Quoted 5 lines
Quoted 3 lines
There is also something subtle in not specifying confaddr (or bootargs for that matter). Many ports support loading configuration from a FAT file system. U-Boot would be no different.
I don't see what this has to do with it?
Plan 9 was traditionally loaded from FAT on PC architectures. Much of that support still exists today. Typically, a small FAT partition exists, which houses the kernel and the configuration (plan9.ini). With U-Boot, to emulate this behavior a fatload would be issued to copy the file to the proper location. This allows users to modify their configuration and reboot without having to drop into the U-Boot shell.
If do_plan9_bootm writes a NUL byte if no bootargs are defined, this would break the fatload method.
Quoted 10 lines
Quoted 6 lines
Ah, this is another subtlety. CONFADDR can change depending on the kernel you are booting. Some ports use as much as 64K to store configuration. Having to recompile U-Boot and reflash based on a kernel change would add a lot of complication and frustration. Having confaddr also makes it somewhat simpler to write a generic boot command which will do a fatload rather than use bootargs.
You have no way to check for valid data, and you have no way to know the correct address, because it is neither fixed nor known to both the producer and the consumer? I'm sorry, but this is crap.
It's known to both the producer and consumer, but can change during development. Having it compiled in also means that you do not have quick access to the value to use for a fatload, though this is a minor annoyance.
Quoted 8 lines
Quoted 3 lines
Is there a better method to allow confaddr to change without forcing a re-compile, or duplication if a user decides to do a fatload rather than define bootargs?
I'm sorry, but it appears this design is completely borked, so how should I answer this? If you have no way to know the correct adddress, and the consumer has no way to verify the data it recives, it's all just trial and error. Not exactly a robust design, that is.
It's a known address for known kernels but needs to have the flexibility to change without a recompile. Personal feelings aside, this is how the kernel handles configuration at boot - I can only do so much.
I very much want these changes (or an acceptable version of them) to go upstream. Most users tend to just hack up U-Boot for the boards they use and maintain private forks, but I would like to see better support in both directions. I'm happy to keep a fork, but these changes do not seem onerous, especially given that other operating systems that are already supported follow this exact behavior.
Cheers,
Steve
PATCH 1/1 cmd_bootm: Add command line arguments to Plan 9
Dear Steven Stallion,
Quoted 9 lines · In message <CAGGHmKH3W6SQSwG3qKJHQmRaxS7ZRVJmbLw_anvgNA61=pcevw@mail.gmail.com>…
Quoted 4 lines
"bootm" has a well-known an documented API: it takes up to three
arguments: kernel address, ramdisk address, and device tree address. Nothing else.Is there a reason that this should only be used by Linux? The changes I have submitted follow the same behavior as NetBSD. VxWorks and QNX also have their own quirks that don't follow the same path/usage as Linux.
Agreed. But this should be documented, then.
Quoted 8 lines
The usage seems to indicate this is a valid approach:
Usage:
bootm [addr [arg ...]]If this is such a contentious change, I'm happy to drop it. I was following the NetBSD approach since it was the most similar. It would be a shame to let it go - it's useful.
Please do not misunderstand me - all I'm asking is to document the exact behaviour.
Personally, I consider the interface a bad design, especially the different handling (or ignorance) of the "bootargs" setting depending on the number of arguments. But I agree that this has already crept in. It's a pity yo want to use exactly this interface, but it's you who has to use (and suffer from) it, so I won;t really block it. I've raised my concerns, that's all.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Some ports (such as OMAP) will stop once it encounters the first non-ASCII character. In general, the parsing for the configuration is fairly strict and is only a small risk if a user configures the system incorrectly.
Hm. This is just a subterfuge for there is no security at all, and you are invoking undefined behaviour ...
This would only happen if a user did not configure the loader appropriately.
...which is just a typo away.
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
There is also something subtle in not specifying confaddr (or bootargs for that matter). Many ports support loading configuration from a FAT file system. U-Boot would be no different.
I don't see what this has to do with it?
Quoted 7 lines
Plan 9 was traditionally loaded from FAT on PC architectures. Much of that support still exists today. Typically, a small FAT partition exists, which houses the kernel and the configuration (plan9.ini). With U-Boot, to emulate this behavior a fatload would be issued to copy the file to the proper location. This allows users to modify their configuration and reboot without having to drop into the U-Boot shell.
You can still do this with U-Boot. I see no problems here.
Quoted 2 lines
If
do_plan9_bootmwrites a NUL byte if no bootargs are defined, this would break the fatload method.
Oops? I cannot parse this.
Quoted 8 lines
Quoted 3 lines
You have no way to check for valid data, and you have no way to know the correct address, because it is neither fixed nor known to both the producer and the consumer? I'm sorry, but this is crap.
It's known to both the producer and consumer, but can change during development. Having it compiled in also means that you do not have quick access to the value to use for a fatload, though this is a minor annoyance.
I disagree. It appears that when you load a kernel image, there is no way to find out what the required confaddr would be. You have to know it. If you have several images with different settings, you always have to know exactly what is what. That's not exactly a robust (nor intelligent) setup.
Quoted 9 lines
Quoted 4 lines
I'm sorry, but it appears this design is completely borked, so how should I answer this? If you have no way to know the correct adddress, and the consumer has no way to verify the data it recives, it's all just trial and error. Not exactly a robust design, that is.
It's a known address for known kernels but needs to have the flexibility to change without a recompile. Personal feelings aside, this is how the kernel handles configuration at boot - I can only do so much.
I don't know about Plan 9, but is there no way to pass this information to the boot loader? For example, if you were using a device tree, you could ncode it there, so UBoot could pick it up. Alternatively, you could use FIT images and include an extra blob with information (and if it's just the confaddr environment setting) for use in U-Boot, etc. ...
Quoted 6 lines
I very much want these changes (or an acceptable version of them) to go upstream. Most users tend to just hack up U-Boot for the boards they use and maintain private forks, but I would like to see better support in both directions. I'm happy to keep a fork, but these changes do not seem onerous, especially given that other operating systems that are already supported follow this exact behavior.
I do not intend to block this - I'm just pointing out issues I'm anticipating.
Best regards,
Wolfgang Denk
Signature · 5 lines
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 at denx.de "It may be that our role on this planet is not to worship God but to create him." - Arthur C. Clarke
PATCH v2 1/1 Add command line arguments to Plan 9
In reply to Steven StallionThis patch introduces support for command line arguments to Plan 9. Plan 9 generally dedicates a small region of kernel memory (known as CONFADDR) for runtime configuration. A new environment variable named confaddr was introduced to indicate this location when copying arguments.
---
Changes for v2:
- Corrected checkpatch misses
- Refactored common code to copy command line arguments
- Added documentation for odd Plan 9 behavior
common/cmd_bootm.c | 36 +++++++++++++++++++++++++++++------- doc/README.plan9 | 18 ++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 doc/README.plan9
common/cmd_bootm.c +29 -7
@@ -1346,6 +1346,19 @@ static void fixup_silent_linux(void) }#endif /* CONFIG_SILENT_CONSOLE */ +#if defined(CONFIG_BOOTM_NETBSD ) || defined(CONFIG_BOOTM_PLAN9 ) +static void copy_args (char *dest,int argc,char *const argv[],char delim) +{ +int i; + +for (i =0 ; i < argc; i++) { +if (i >0 ) + *dest++ = delim; +strcpy (dest, argv[i]); + dest +=strlen (argv[i]); + } +} +#endif /*******************************************************************/ /* OS booting routines */
@@ -1408,13 +1421,7 @@ static int do_bootm_netbsd(int flag, int argc, char * const argv[],for (i =2 , len =0 ; i < argc; i +=1 ) len +=strlen (argv[i]) +1 ; cmdline =malloc (len); - -for (i =2 , len =0 ; i < argc; i +=1 ) { -if (i >2 ) - cmdline[len++] =' ' ; -strcpy (&cmdline[len], argv[i]); - len +=strlen (argv[i]); - } +copy_args (cmdline, argc-2 , argv+2 ,' ' ); }else if ((cmdline =getenv ("bootargs" )) ==NULL ) { cmdline ="" ; }
@@ -1533,6 +1540,7 @@ static int do_bootm_plan9(int flag, int argc, char * const argv[], bootm_headers_t *images) {void (*entry_point)(void ); +char *s;if ((flag !=0 ) && (flag !=BOOTM_STATE_OS_GO ))return 1 ;
@@ -1544,6 +1552,20 @@ static int do_bootm_plan9(int flag, int argc, char * const argv[], }#endif +/* See README.plan9 */ + s =getenv ("confaddr" ); +if (s !=NULL ) { +char *confaddr = (char *)simple_strtoul (s,NULL ,16 ); + +if (argc >2 ) { +copy_args (confaddr, argc-2 , argv+2 ,'\n' ); + }else { + s =getenv ("bootargs" ); +if (s !=NULL ) +strcpy (confaddr, s); + } + } + entry_point = (void (*)(void ))images->ep; printf("## Transferring control to Plan 9 (at address %08lx) ...\n",
doc/README.plan9 new file +18 -0
@@ -0,0 +1,18 @@ +Plan 9 from Bell Labs kernel images require additional setup to pass +configuration information to the kernel. An environment variable named +confaddr must be defined with the same value as CONFADDR (see mem.h). +Use of this facility is optional, but should be preferable to manual +configuration. + +When booting an image, arguments supplied to the bootm command will be +copied to CONFADDR. If no arguments are specified, the contents of the +bootargs environment variable will be copied. + +If no command line arguments or bootargs are defined, CONFADDR is left +uninitialized to permit manual configuration. For example, PC-style +configuration could be simulated by issuing a fatload in bootcmd: + + # setenv bootcmd fatload mmc 0 $confaddr plan9.ini; ...; bootm + +Steven Stallion +June 2013
Signature · 1 line
1.7.0.4
PATCH v3 1/1 cmd_bootm: Add command line arguments to Plan 9
In reply to Steven StallionThis patch introduces support for command line arguments to Plan 9. Plan 9 generally dedicates a small region of kernel memory (known as CONFADDR) for runtime configuration. A new environment variable named confaddr was introduced to indicate this location when copying arguments.
---
Changes for v2:
- Corrected checkpatch misses
- Refactored common code to copy command line arguments
- Added documentation for odd Plan 9 behavior
Changes for v3:
- Corrected patch Subject
common/cmd_bootm.c | 36 +++++++++++++++++++++++++++++------- doc/README.plan9 | 18 ++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 doc/README.plan9
common/cmd_bootm.c +29 -7
@@ -1346,6 +1346,19 @@ static void fixup_silent_linux(void) }#endif /* CONFIG_SILENT_CONSOLE */ +#if defined(CONFIG_BOOTM_NETBSD ) || defined(CONFIG_BOOTM_PLAN9 ) +static void copy_args (char *dest,int argc,char *const argv[],char delim) +{ +int i; + +for (i =0 ; i < argc; i++) { +if (i >0 ) + *dest++ = delim; +strcpy (dest, argv[i]); + dest +=strlen (argv[i]); + } +} +#endif /*******************************************************************/ /* OS booting routines */
@@ -1408,13 +1421,7 @@ static int do_bootm_netbsd(int flag, int argc, char * const argv[],for (i =2 , len =0 ; i < argc; i +=1 ) len +=strlen (argv[i]) +1 ; cmdline =malloc (len); - -for (i =2 , len =0 ; i < argc; i +=1 ) { -if (i >2 ) - cmdline[len++] =' ' ; -strcpy (&cmdline[len], argv[i]); - len +=strlen (argv[i]); - } +copy_args (cmdline, argc-2 , argv+2 ,' ' ); }else if ((cmdline =getenv ("bootargs" )) ==NULL ) { cmdline ="" ; }
@@ -1533,6 +1540,7 @@ static int do_bootm_plan9(int flag, int argc, char * const argv[], bootm_headers_t *images) {void (*entry_point)(void ); +char *s;if ((flag !=0 ) && (flag !=BOOTM_STATE_OS_GO ))return 1 ;
@@ -1544,6 +1552,20 @@ static int do_bootm_plan9(int flag, int argc, char * const argv[], }#endif +/* See README.plan9 */ + s =getenv ("confaddr" ); +if (s !=NULL ) { +char *confaddr = (char *)simple_strtoul (s,NULL ,16 ); + +if (argc >2 ) { +copy_args (confaddr, argc-2 , argv+2 ,'\n' ); + }else { + s =getenv ("bootargs" ); +if (s !=NULL ) +strcpy (confaddr, s); + } + } + entry_point = (void (*)(void ))images->ep; printf("## Transferring control to Plan 9 (at address %08lx) ...\n",
doc/README.plan9 new file +18 -0
@@ -0,0 +1,18 @@ +Plan 9 from Bell Labs kernel images require additional setup to pass +configuration information to the kernel. An environment variable named +confaddr must be defined with the same value as CONFADDR (see mem.h). +Use of this facility is optional, but should be preferable to manual +configuration. + +When booting an image, arguments supplied to the bootm command will be +copied to CONFADDR. If no arguments are specified, the contents of the +bootargs environment variable will be copied. + +If no command line arguments or bootargs are defined, CONFADDR is left +uninitialized to permit manual configuration. For example, PC-style +configuration could be simulated by issuing a fatload in bootcmd: + + # setenv bootcmd fatload mmc 0 $confaddr plan9.ini; ...; bootm + +Steven Stallion +June 2013
Signature · 1 line
1.7.0.4
PATCH v3 1/1 cmd_bootm: Add command line arguments to Plan 9
Quoted 7 lines · Steven Stallion
This patch introduces support for command line arguments to Plan 9. Plan 9 generally dedicates a small region of kernel memory (known as CONFADDR) for runtime configuration. A new environment variable named confaddr was introduced to indicate this location when copying arguments.
Signed-off-by Steven Stallion <[email protected]>
With a slight re-factor due to some changes from Simon Glass, applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130626/38e09f69/attachment.pgp>