| Message ID | 20231204002642.895926-2-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Complete decoupling of bootm logic from commands | expand |
> From: Simon Glass <sjg@chromium.org> > Date: Sun, 3 Dec 2023 17:26:17 -0700 Hi Simon, There is a typo in first line of the commit message: s/netbds/netbsd/. > It isn't clear how useful it is to pass the arguments of bootm to the > OS. For example, if "bootm 1000 2000 3000" is used, the three arguments > at the end are passed to the OS. This seems like a strange approach, > since the argument have already been parsed by U-Boot and processed. > > Rely instead on the "bootargs" mechanism, which is the standard > approach. It is a very Linuxy approach though. I suspect this feature was added to pass kernel arguments for "one-off" boots. For example bootm -s could be used to boot NetBSD in single-user mode and is quite a bit more convenient than: setenv bootargs -s bootm That said, I'm not sure to what extent the bootm command is used to boot NetBSD these days. So this may not really matter. Cheers, Mark > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > boot/bootm_os.c | 16 +++------------- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/boot/bootm_os.c b/boot/bootm_os.c > index b92422171a84..b5055d78706c 100644 > --- a/boot/bootm_os.c > +++ b/boot/bootm_os.c > @@ -102,19 +102,9 @@ static int do_bootm_netbsd(int flag, int argc, char *const argv[], > os_hdr = hdr; > } > > - if (argc > 0) { > - ulong len; > - int i; > - > - for (i = 0, len = 0; i < argc; i += 1) > - len += strlen(argv[i]) + 1; > - cmdline = malloc(len); > - copy_args(cmdline, argc, argv, ' '); > - } else { > - cmdline = env_get("bootargs"); > - if (cmdline == NULL) > - cmdline = ""; > - } > + cmdline = env_get("bootargs"); > + if (!cmdline) > + cmdline = ""; > > loader = (void (*)(struct bd_info *, struct legacy_img_hdr *, char *, char *))images->ep; > > -- > 2.43.0.rc2.451.g8631bc7472-goog > >
On Mon, Dec 04, 2023 at 11:48:17AM +0100, Mark Kettenis wrote: > That said, I'm not sure to what extent the bootm command is used to > boot NetBSD these days. So this may not really matter. It is used on most 32bit ARM platforms. Martin
Hi, I am replying to this email since it still has the context below. On Mon, 4 Dec 2023 at 03:48, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > From: Simon Glass <sjg@chromium.org> > > Date: Sun, 3 Dec 2023 17:26:17 -0700 > > Hi Simon, > > There is a typo in first line of the commit message: s/netbds/netbsd/. > > > It isn't clear how useful it is to pass the arguments of bootm to the > > OS. For example, if "bootm 1000 2000 3000" is used, the three arguments > > at the end are passed to the OS. This seems like a strange approach, > > since the argument have already been parsed by U-Boot and processed. > > > > Rely instead on the "bootargs" mechanism, which is the standard > > approach. > > It is a very Linuxy approach though. > > I suspect this feature was added to pass kernel arguments for > "one-off" boots. For example > > bootm -s Martin says that this is used with NetBSD. So do we rely on U-Boot parsing the '-s' as 0 and then using loadaddr as the default? I wonder if we could come up with another mechanism? Perhaps 'bootm -- args go here' ? Or define that if the first arg starts with '-' then the args are not parsed? Martin, would it be possible to send a patch to doc/ describing how to boot NetBSD? > > could be used to boot NetBSD in single-user mode and is quite a bit > more convenient than: > > setenv bootargs -s > bootm > > That said, I'm not sure to what extent the bootm command is used to > boot NetBSD these days. So this may not really matter. > > Cheers, > > Mark > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > boot/bootm_os.c | 16 +++------------- > > 1 file changed, 3 insertions(+), 13 deletions(-) > > > > diff --git a/boot/bootm_os.c b/boot/bootm_os.c > > index b92422171a84..b5055d78706c 100644 > > --- a/boot/bootm_os.c > > +++ b/boot/bootm_os.c > > @@ -102,19 +102,9 @@ static int do_bootm_netbsd(int flag, int argc, char *const argv[], > > os_hdr = hdr; > > } > > > > - if (argc > 0) { > > - ulong len; > > - int i; > > - > > - for (i = 0, len = 0; i < argc; i += 1) > > - len += strlen(argv[i]) + 1; > > - cmdline = malloc(len); > > - copy_args(cmdline, argc, argv, ' '); > > - } else { > > - cmdline = env_get("bootargs"); > > - if (cmdline == NULL) > > - cmdline = ""; > > - } > > + cmdline = env_get("bootargs"); > > + if (!cmdline) > > + cmdline = ""; > > > > loader = (void (*)(struct bd_info *, struct legacy_img_hdr *, char *, char *))images->ep; > > > > -- > > 2.43.0.rc2.451.g8631bc7472-goog > > > >
On Tue, Dec 05, 2023 at 08:53:58PM -0700, Simon Glass wrote: > Hi, > > I am replying to this email since it still has the context below. > > On Mon, 4 Dec 2023 at 03:48, Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > > > From: Simon Glass <sjg@chromium.org> > > > Date: Sun, 3 Dec 2023 17:26:17 -0700 > > > > Hi Simon, > > > > There is a typo in first line of the commit message: s/netbds/netbsd/. > > > > > It isn't clear how useful it is to pass the arguments of bootm to the > > > OS. For example, if "bootm 1000 2000 3000" is used, the three arguments > > > at the end are passed to the OS. This seems like a strange approach, > > > since the argument have already been parsed by U-Boot and processed. > > > > > > Rely instead on the "bootargs" mechanism, which is the standard > > > approach. > > > > It is a very Linuxy approach though. > > > > I suspect this feature was added to pass kernel arguments for > > "one-off" boots. For example > > > > bootm -s > > Martin says that this is used with NetBSD. So do we rely on U-Boot > parsing the '-s' as 0 and then using loadaddr as the default? > > I wonder if we could come up with another mechanism? Perhaps 'bootm -- > args go here' ? Or define that if the first arg starts with '-' then > the args are not parsed? > > Martin, would it be possible to send a patch to doc/ describing how to > boot NetBSD? A patch describing how booting is done today, especially including some of the potentially less common but still valid usages would be much appreciated. We can go from there to decide what changes if any to make to what's exposed to the user in this case as I am loathe to change things like this which amount of an ABI.
diff --git a/boot/bootm_os.c b/boot/bootm_os.c index b92422171a84..b5055d78706c 100644 --- a/boot/bootm_os.c +++ b/boot/bootm_os.c @@ -102,19 +102,9 @@ static int do_bootm_netbsd(int flag, int argc, char *const argv[], os_hdr = hdr; } - if (argc > 0) { - ulong len; - int i; - - for (i = 0, len = 0; i < argc; i += 1) - len += strlen(argv[i]) + 1; - cmdline = malloc(len); - copy_args(cmdline, argc, argv, ' '); - } else { - cmdline = env_get("bootargs"); - if (cmdline == NULL) - cmdline = ""; - } + cmdline = env_get("bootargs"); + if (!cmdline) + cmdline = ""; loader = (void (*)(struct bd_info *, struct legacy_img_hdr *, char *, char *))images->ep;
It isn't clear how useful it is to pass the arguments of bootm to the OS. For example, if "bootm 1000 2000 3000" is used, the three arguments at the end are passed to the OS. This seems like a strange approach, since the argument have already been parsed by U-Boot and processed. Rely instead on the "bootargs" mechanism, which is the standard approach. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootm_os.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-)