* [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. @ 2018-02-22 22:24 Michal Suchanek 2018-02-22 23:20 ` Baoquan He 0 siblings, 1 reply; 15+ messages in thread From: Michal Suchanek @ 2018-02-22 22:24 UTC (permalink / raw) To: kexec; +Cc: Tony Jones, Michal Suchanek, Petr Tesarik, horms The new KEXEC_FILE_LOAD is preferred in the case the platform supports it because it allows kexec in locked down secure boot mode. However, some platforms do not support it so fall back to the old syscall there. Also provide an option to call the old syscall in case the new syscall fails with other reason than ENOSYS. Also document the options. Signed-off-by: Michal Suchanek <msuchanek@suse.de> --- kexec/kexec.8 | 9 +++++++++ kexec/kexec.c | 41 +++++++++++++++-------------------------- kexec/kexec.h | 2 ++ 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/kexec/kexec.8 b/kexec/kexec.8 index e0131b4ea827..7e4df723251d 100644 --- a/kexec/kexec.8 +++ b/kexec/kexec.8 @@ -144,6 +144,15 @@ Load the new kernel for use on panic. Specify that the new kernel is of this .I type. .TP +.BI \-s\ (\-\-kexec-file-syscall) +Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively. +Otherwise KEXEC_FILE_LOAD is tried and when not supported KEXEC_LOAD is used. +.I type. +.TP +.BI \-c\ (\-\-kexec-syscall) +Specify that the old KEXEC_LOAD syscall should be used exclusively. +.I type. +.TP .B \-u\ (\-\-unload) Unload the current .B kexec diff --git a/kexec/kexec.c b/kexec/kexec.c index cfd837c1b6bb..25328c02b508 100644 --- a/kexec/kexec.c +++ b/kexec/kexec.c @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv, if (!is_kexec_file_load_implemented()) { fprintf(stderr, "syscall kexec_file_load not available.\n"); - return -1; + return -ENOSYS; } if (argc - fileind <= 0) { @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[]) int do_unload = 0; int do_reuse_initrd = 0; int do_kexec_file_syscall = 0; + int do_kexec_syscall = 0; int do_status = 0; void *entry = 0; char *type = 0; @@ -1256,19 +1257,6 @@ int main(int argc, char *argv[]) }; static const char short_options[] = KEXEC_ALL_OPT_STR; - /* - * First check if --use-kexec-file-syscall is set. That changes lot of - * things - */ - while ((opt = getopt_long(argc, argv, short_options, - options, 0)) != -1) { - switch(opt) { - case OPT_KEXEC_FILE_SYSCALL: - do_kexec_file_syscall = 1; - break; - } - } - /* Reset getopt for the next pass. */ opterr = 1; optind = 1; @@ -1310,8 +1298,7 @@ int main(int argc, char *argv[]) do_shutdown = 0; do_sync = 0; do_unload = 1; - if (do_kexec_file_syscall) - kexec_file_flags |= KEXEC_FILE_UNLOAD; + kexec_file_flags |= KEXEC_FILE_UNLOAD; break; case OPT_EXEC: do_load = 0; @@ -1354,11 +1341,8 @@ int main(int argc, char *argv[]) do_exec = 0; do_shutdown = 0; do_sync = 0; - if (do_kexec_file_syscall) - kexec_file_flags |= KEXEC_FILE_ON_CRASH; - else - kexec_flags = KEXEC_ON_CRASH; - break; + kexec_file_flags |= KEXEC_FILE_ON_CRASH; + kexec_flags = KEXEC_ON_CRASH; case OPT_MEM_MIN: mem_min = strtoul(optarg, &endptr, 0); if (*endptr) { @@ -1383,7 +1367,12 @@ int main(int argc, char *argv[]) do_reuse_initrd = 1; break; case OPT_KEXEC_FILE_SYSCALL: - /* We already parsed it. Nothing to do. */ + do_kexec_file_syscall = 1; + do_kexec_syscall = 0; + break; + case OPT_KEXEC_SYSCALL: + do_kexec_file_syscall = 0; + do_kexec_syscall = 1; break; case OPT_STATUS: do_status = 1; @@ -1456,16 +1445,16 @@ int main(int argc, char *argv[]) result = k_status(kexec_flags); } if (do_unload) { - if (do_kexec_file_syscall) + if (!do_kexec_syscall) result = kexec_file_unload(kexec_file_flags); - else + if ((result == -ENOSYS) || !do_kexec_file_syscall) result = k_unload(kexec_flags); } if (do_load && (result == 0)) { - if (do_kexec_file_syscall) + if (!do_kexec_syscall) result = do_kexec_file_load(fileind, argc, argv, kexec_file_flags); - else + if ((result == -ENOSYS) || !do_kexec_file_syscall) result = my_load(type, fileind, argc, argv, kexec_flags, entry); } diff --git a/kexec/kexec.h b/kexec/kexec.h index 26225d2c002a..7abcec796cae 100644 --- a/kexec/kexec.h +++ b/kexec/kexec.h @@ -219,6 +219,7 @@ extern int file_types; #define OPT_TYPE 't' #define OPT_PANIC 'p' #define OPT_KEXEC_FILE_SYSCALL 's' +#define OPT_KEXEC_SYSCALL 'c' #define OPT_STATUS 'S' #define OPT_MEM_MIN 256 #define OPT_MEM_MAX 257 @@ -246,6 +247,7 @@ extern int file_types; { "mem-max", 1, 0, OPT_MEM_MAX }, \ { "reuseinitrd", 0, 0, OPT_REUSE_INITRD }, \ { "kexec-file-syscall", 0, 0, OPT_KEXEC_FILE_SYSCALL }, \ + { "kexec-syscall", 0, 0, OPT_KEXEC_SYSCALL }, \ { "debug", 0, 0, OPT_DEBUG }, \ { "status", 0, 0, OPT_STATUS }, \ { "print-ckr-size", 0, 0, OPT_PRINT_CKR_SIZE }, \ -- 2.13.6 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-22 22:24 [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported Michal Suchanek @ 2018-02-22 23:20 ` Baoquan He 2018-02-23 8:29 ` Petr Tesarik 2018-02-23 19:21 ` Michal Suchánek 0 siblings, 2 replies; 15+ messages in thread From: Baoquan He @ 2018-02-22 23:20 UTC (permalink / raw) To: Michal Suchanek; +Cc: Tony Jones, horms, kexec, Petr Tesarik Hi Michal, On 02/22/18 at 11:24pm, Michal Suchanek wrote: > The new KEXEC_FILE_LOAD is preferred in the case the platform supports > it because it allows kexec in locked down secure boot mode. > > However, some platforms do not support it so fall back to the old > syscall there. I didn't read code change, just from patch log, I tend to not agree. There are two options KEXEC_FILE_LOAD and KEXEC_LOAD, some platforms do not support, why does some platforms not choose KEXEC_LOAD, the working one? Why bother to make change in code? I believe there's returned message telling if KEXEC_FILE_LOAD works or not. Thanks Baoquan > > Also provide an option to call the old syscall in case the new syscall > fails with other reason than ENOSYS. > > Also document the options. > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > --- > kexec/kexec.8 | 9 +++++++++ > kexec/kexec.c | 41 +++++++++++++++-------------------------- > kexec/kexec.h | 2 ++ > 3 files changed, 26 insertions(+), 26 deletions(-) > > diff --git a/kexec/kexec.8 b/kexec/kexec.8 > index e0131b4ea827..7e4df723251d 100644 > --- a/kexec/kexec.8 > +++ b/kexec/kexec.8 > @@ -144,6 +144,15 @@ Load the new kernel for use on panic. > Specify that the new kernel is of this > .I type. > .TP > +.BI \-s\ (\-\-kexec-file-syscall) > +Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively. > +Otherwise KEXEC_FILE_LOAD is tried and when not supported KEXEC_LOAD is used. > +.I type. > +.TP > +.BI \-c\ (\-\-kexec-syscall) > +Specify that the old KEXEC_LOAD syscall should be used exclusively. > +.I type. > +.TP > .B \-u\ (\-\-unload) > Unload the current > .B kexec > diff --git a/kexec/kexec.c b/kexec/kexec.c > index cfd837c1b6bb..25328c02b508 100644 > --- a/kexec/kexec.c > +++ b/kexec/kexec.c > @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv, > > if (!is_kexec_file_load_implemented()) { > fprintf(stderr, "syscall kexec_file_load not available.\n"); > - return -1; > + return -ENOSYS; > } > > if (argc - fileind <= 0) { > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[]) > int do_unload = 0; > int do_reuse_initrd = 0; > int do_kexec_file_syscall = 0; > + int do_kexec_syscall = 0; > int do_status = 0; > void *entry = 0; > char *type = 0; > @@ -1256,19 +1257,6 @@ int main(int argc, char *argv[]) > }; > static const char short_options[] = KEXEC_ALL_OPT_STR; > > - /* > - * First check if --use-kexec-file-syscall is set. That changes lot of > - * things > - */ > - while ((opt = getopt_long(argc, argv, short_options, > - options, 0)) != -1) { > - switch(opt) { > - case OPT_KEXEC_FILE_SYSCALL: > - do_kexec_file_syscall = 1; > - break; > - } > - } > - > /* Reset getopt for the next pass. */ > opterr = 1; > optind = 1; > @@ -1310,8 +1298,7 @@ int main(int argc, char *argv[]) > do_shutdown = 0; > do_sync = 0; > do_unload = 1; > - if (do_kexec_file_syscall) > - kexec_file_flags |= KEXEC_FILE_UNLOAD; > + kexec_file_flags |= KEXEC_FILE_UNLOAD; > break; > case OPT_EXEC: > do_load = 0; > @@ -1354,11 +1341,8 @@ int main(int argc, char *argv[]) > do_exec = 0; > do_shutdown = 0; > do_sync = 0; > - if (do_kexec_file_syscall) > - kexec_file_flags |= KEXEC_FILE_ON_CRASH; > - else > - kexec_flags = KEXEC_ON_CRASH; > - break; > + kexec_file_flags |= KEXEC_FILE_ON_CRASH; > + kexec_flags = KEXEC_ON_CRASH; > case OPT_MEM_MIN: > mem_min = strtoul(optarg, &endptr, 0); > if (*endptr) { > @@ -1383,7 +1367,12 @@ int main(int argc, char *argv[]) > do_reuse_initrd = 1; > break; > case OPT_KEXEC_FILE_SYSCALL: > - /* We already parsed it. Nothing to do. */ > + do_kexec_file_syscall = 1; > + do_kexec_syscall = 0; > + break; > + case OPT_KEXEC_SYSCALL: > + do_kexec_file_syscall = 0; > + do_kexec_syscall = 1; > break; > case OPT_STATUS: > do_status = 1; > @@ -1456,16 +1445,16 @@ int main(int argc, char *argv[]) > result = k_status(kexec_flags); > } > if (do_unload) { > - if (do_kexec_file_syscall) > + if (!do_kexec_syscall) > result = kexec_file_unload(kexec_file_flags); > - else > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > result = k_unload(kexec_flags); > } > if (do_load && (result == 0)) { > - if (do_kexec_file_syscall) > + if (!do_kexec_syscall) > result = do_kexec_file_load(fileind, argc, argv, > kexec_file_flags); > - else > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > result = my_load(type, fileind, argc, argv, > kexec_flags, entry); > } > diff --git a/kexec/kexec.h b/kexec/kexec.h > index 26225d2c002a..7abcec796cae 100644 > --- a/kexec/kexec.h > +++ b/kexec/kexec.h > @@ -219,6 +219,7 @@ extern int file_types; > #define OPT_TYPE 't' > #define OPT_PANIC 'p' > #define OPT_KEXEC_FILE_SYSCALL 's' > +#define OPT_KEXEC_SYSCALL 'c' > #define OPT_STATUS 'S' > #define OPT_MEM_MIN 256 > #define OPT_MEM_MAX 257 > @@ -246,6 +247,7 @@ extern int file_types; > { "mem-max", 1, 0, OPT_MEM_MAX }, \ > { "reuseinitrd", 0, 0, OPT_REUSE_INITRD }, \ > { "kexec-file-syscall", 0, 0, OPT_KEXEC_FILE_SYSCALL }, \ > + { "kexec-syscall", 0, 0, OPT_KEXEC_SYSCALL }, \ > { "debug", 0, 0, OPT_DEBUG }, \ > { "status", 0, 0, OPT_STATUS }, \ > { "print-ckr-size", 0, 0, OPT_PRINT_CKR_SIZE }, \ > -- > 2.13.6 > > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-22 23:20 ` Baoquan He @ 2018-02-23 8:29 ` Petr Tesarik 2018-02-24 1:43 ` Dave Young 2018-02-24 2:19 ` Baoquan He 2018-02-23 19:21 ` Michal Suchánek 1 sibling, 2 replies; 15+ messages in thread From: Petr Tesarik @ 2018-02-23 8:29 UTC (permalink / raw) To: Baoquan He; +Cc: Tony Jones, Michal Suchanek, kexec, horms Hi Baoquan, On Fri, 23 Feb 2018 07:20:43 +0800 Baoquan He <bhe@redhat.com> wrote: > Hi Michal, > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > The new KEXEC_FILE_LOAD is preferred in the case the platform supports > > it because it allows kexec in locked down secure boot mode. > > > > However, some platforms do not support it so fall back to the old > > syscall there. > > I didn't read code change, just from patch log, I tend to not agree. There > are two options KEXEC_FILE_LOAD and KEXEC_LOAD, some platforms do not > support, why does some platforms not choose KEXEC_LOAD, the working one? > Why bother to make change in code? I believe there's returned message > telling if KEXEC_FILE_LOAD works or not. Well... let me give a bit of background. As you have probably noticed, this syscall was originally available only for x86_64, but more and more architectures are also adding it now. Next, kexec is actually called by a script (which locates a suitable kernel and initrd, constructs the kernel command line, etc.). The script must either: A. know somehow if the currently running kernel implements kexec_file_load(2), or B. try one method first, and if it fails, retry with the other. I agree that kexec(1) should probably allow the user to force a specific method, but I don't see the benefit of implementing fallback in an external script and not in kexec-tools itself. OTOH if you want to push the fallback logic out of kexec-tools, then I would like to get better diagnostic at least. Letting my script parse kexec output is, um, suboptimal. Petr T > Thanks > Baoquan > > > > Also provide an option to call the old syscall in case the new syscall > > fails with other reason than ENOSYS. > > > > Also document the options. > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > --- > > kexec/kexec.8 | 9 +++++++++ > > kexec/kexec.c | 41 +++++++++++++++-------------------------- > > kexec/kexec.h | 2 ++ > > 3 files changed, 26 insertions(+), 26 deletions(-) > > > > diff --git a/kexec/kexec.8 b/kexec/kexec.8 > > index e0131b4ea827..7e4df723251d 100644 > > --- a/kexec/kexec.8 > > +++ b/kexec/kexec.8 > > @@ -144,6 +144,15 @@ Load the new kernel for use on panic. > > Specify that the new kernel is of this > > .I type. > > .TP > > +.BI \-s\ (\-\-kexec-file-syscall) > > +Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively. > > +Otherwise KEXEC_FILE_LOAD is tried and when not supported KEXEC_LOAD is used. > > +.I type. > > +.TP > > +.BI \-c\ (\-\-kexec-syscall) > > +Specify that the old KEXEC_LOAD syscall should be used exclusively. > > +.I type. > > +.TP > > .B \-u\ (\-\-unload) > > Unload the current > > .B kexec > > diff --git a/kexec/kexec.c b/kexec/kexec.c > > index cfd837c1b6bb..25328c02b508 100644 > > --- a/kexec/kexec.c > > +++ b/kexec/kexec.c > > @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv, > > > > if (!is_kexec_file_load_implemented()) { > > fprintf(stderr, "syscall kexec_file_load not available.\n"); > > - return -1; > > + return -ENOSYS; > > } > > > > if (argc - fileind <= 0) { > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[]) > > int do_unload = 0; > > int do_reuse_initrd = 0; > > int do_kexec_file_syscall = 0; > > + int do_kexec_syscall = 0; > > int do_status = 0; > > void *entry = 0; > > char *type = 0; > > @@ -1256,19 +1257,6 @@ int main(int argc, char *argv[]) > > }; > > static const char short_options[] = KEXEC_ALL_OPT_STR; > > > > - /* > > - * First check if --use-kexec-file-syscall is set. That changes lot of > > - * things > > - */ > > - while ((opt = getopt_long(argc, argv, short_options, > > - options, 0)) != -1) { > > - switch(opt) { > > - case OPT_KEXEC_FILE_SYSCALL: > > - do_kexec_file_syscall = 1; > > - break; > > - } > > - } > > - > > /* Reset getopt for the next pass. */ > > opterr = 1; > > optind = 1; > > @@ -1310,8 +1298,7 @@ int main(int argc, char *argv[]) > > do_shutdown = 0; > > do_sync = 0; > > do_unload = 1; > > - if (do_kexec_file_syscall) > > - kexec_file_flags |= KEXEC_FILE_UNLOAD; > > + kexec_file_flags |= KEXEC_FILE_UNLOAD; > > break; > > case OPT_EXEC: > > do_load = 0; > > @@ -1354,11 +1341,8 @@ int main(int argc, char *argv[]) > > do_exec = 0; > > do_shutdown = 0; > > do_sync = 0; > > - if (do_kexec_file_syscall) > > - kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > - else > > - kexec_flags = KEXEC_ON_CRASH; > > - break; > > + kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > + kexec_flags = KEXEC_ON_CRASH; > > case OPT_MEM_MIN: > > mem_min = strtoul(optarg, &endptr, 0); > > if (*endptr) { > > @@ -1383,7 +1367,12 @@ int main(int argc, char *argv[]) > > do_reuse_initrd = 1; > > break; > > case OPT_KEXEC_FILE_SYSCALL: > > - /* We already parsed it. Nothing to do. */ > > + do_kexec_file_syscall = 1; > > + do_kexec_syscall = 0; > > + break; > > + case OPT_KEXEC_SYSCALL: > > + do_kexec_file_syscall = 0; > > + do_kexec_syscall = 1; > > break; > > case OPT_STATUS: > > do_status = 1; > > @@ -1456,16 +1445,16 @@ int main(int argc, char *argv[]) > > result = k_status(kexec_flags); > > } > > if (do_unload) { > > - if (do_kexec_file_syscall) > > + if (!do_kexec_syscall) > > result = kexec_file_unload(kexec_file_flags); > > - else > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > result = k_unload(kexec_flags); > > } > > if (do_load && (result == 0)) { > > - if (do_kexec_file_syscall) > > + if (!do_kexec_syscall) > > result = do_kexec_file_load(fileind, argc, argv, > > kexec_file_flags); > > - else > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > result = my_load(type, fileind, argc, argv, > > kexec_flags, entry); > > } > > diff --git a/kexec/kexec.h b/kexec/kexec.h > > index 26225d2c002a..7abcec796cae 100644 > > --- a/kexec/kexec.h > > +++ b/kexec/kexec.h > > @@ -219,6 +219,7 @@ extern int file_types; > > #define OPT_TYPE 't' > > #define OPT_PANIC 'p' > > #define OPT_KEXEC_FILE_SYSCALL 's' > > +#define OPT_KEXEC_SYSCALL 'c' > > #define OPT_STATUS 'S' > > #define OPT_MEM_MIN 256 > > #define OPT_MEM_MAX 257 > > @@ -246,6 +247,7 @@ extern int file_types; > > { "mem-max", 1, 0, OPT_MEM_MAX }, \ > > { "reuseinitrd", 0, 0, OPT_REUSE_INITRD }, \ > > { "kexec-file-syscall", 0, 0, OPT_KEXEC_FILE_SYSCALL }, \ > > + { "kexec-syscall", 0, 0, OPT_KEXEC_SYSCALL }, \ > > { "debug", 0, 0, OPT_DEBUG }, \ > > { "status", 0, 0, OPT_STATUS }, \ > > { "print-ckr-size", 0, 0, OPT_PRINT_CKR_SIZE }, \ > > -- > > 2.13.6 > > > > > > _______________________________________________ > > kexec mailing list > > kexec@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/kexec > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-23 8:29 ` Petr Tesarik @ 2018-02-24 1:43 ` Dave Young 2018-02-24 16:34 ` Petr Tesarik 2018-02-24 20:02 ` Michal Suchánek 2018-02-24 2:19 ` Baoquan He 1 sibling, 2 replies; 15+ messages in thread From: Dave Young @ 2018-02-24 1:43 UTC (permalink / raw) To: Petr Tesarik; +Cc: horms, Tony Jones, Michal Suchanek, kexec, Baoquan He On 02/23/18 at 09:29am, Petr Tesarik wrote: > Hi Baoquan, > > On Fri, 23 Feb 2018 07:20:43 +0800 > Baoquan He <bhe@redhat.com> wrote: > > > Hi Michal, > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > The new KEXEC_FILE_LOAD is preferred in the case the platform supports > > > it because it allows kexec in locked down secure boot mode. > > > > > > However, some platforms do not support it so fall back to the old > > > syscall there. > > > > I didn't read code change, just from patch log, I tend to not agree. There > > are two options KEXEC_FILE_LOAD and KEXEC_LOAD, some platforms do not > > support, why does some platforms not choose KEXEC_LOAD, the working one? > > Why bother to make change in code? I believe there's returned message > > telling if KEXEC_FILE_LOAD works or not. > > Well... let me give a bit of background. As you have probably noticed, > this syscall was originally available only for x86_64, but more and > more architectures are also adding it now. > > Next, kexec is actually called by a script (which locates a suitable > kernel and initrd, constructs the kernel command line, etc.). The > script must either: > > A. know somehow if the currently running kernel implements > kexec_file_load(2), or > > B. try one method first, and if it fails, retry with the other. > > I agree that kexec(1) should probably allow the user to force a > specific method, but I don't see the benefit of implementing fallback > in an external script and not in kexec-tools itself. > > OTOH if you want to push the fallback logic out of kexec-tools, then I > would like to get better diagnostic at least. Letting my script parse > kexec output is, um, suboptimal. In Fedora/RHEL we use this in scripts by checking the arch first, for distribution it is enough? There are also some other arch dependent options in kexec-tools, there is no way to just use same for every different platform without checking in scripts. If your scripts is not for a distribution, I agree that it is indeed a problem. > > Petr T > > > Thanks > > Baoquan > > > > > > Also provide an option to call the old syscall in case the new syscall > > > fails with other reason than ENOSYS. > > > > > > Also document the options. > > > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > --- > > > kexec/kexec.8 | 9 +++++++++ > > > kexec/kexec.c | 41 +++++++++++++++-------------------------- > > > kexec/kexec.h | 2 ++ > > > 3 files changed, 26 insertions(+), 26 deletions(-) > > > > > > diff --git a/kexec/kexec.8 b/kexec/kexec.8 > > > index e0131b4ea827..7e4df723251d 100644 > > > --- a/kexec/kexec.8 > > > +++ b/kexec/kexec.8 > > > @@ -144,6 +144,15 @@ Load the new kernel for use on panic. > > > Specify that the new kernel is of this > > > .I type. > > > .TP > > > +.BI \-s\ (\-\-kexec-file-syscall) > > > +Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively. > > > +Otherwise KEXEC_FILE_LOAD is tried and when not supported KEXEC_LOAD is used. > > > +.I type. > > > +.TP > > > +.BI \-c\ (\-\-kexec-syscall) > > > +Specify that the old KEXEC_LOAD syscall should be used exclusively. > > > +.I type. > > > +.TP > > > .B \-u\ (\-\-unload) > > > Unload the current > > > .B kexec > > > diff --git a/kexec/kexec.c b/kexec/kexec.c > > > index cfd837c1b6bb..25328c02b508 100644 > > > --- a/kexec/kexec.c > > > +++ b/kexec/kexec.c > > > @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv, > > > > > > if (!is_kexec_file_load_implemented()) { > > > fprintf(stderr, "syscall kexec_file_load not available.\n"); > > > - return -1; > > > + return -ENOSYS; > > > } > > > > > > if (argc - fileind <= 0) { > > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[]) > > > int do_unload = 0; > > > int do_reuse_initrd = 0; > > > int do_kexec_file_syscall = 0; > > > + int do_kexec_syscall = 0; > > > int do_status = 0; > > > void *entry = 0; > > > char *type = 0; > > > @@ -1256,19 +1257,6 @@ int main(int argc, char *argv[]) > > > }; > > > static const char short_options[] = KEXEC_ALL_OPT_STR; > > > > > > - /* > > > - * First check if --use-kexec-file-syscall is set. That changes lot of > > > - * things > > > - */ > > > - while ((opt = getopt_long(argc, argv, short_options, > > > - options, 0)) != -1) { > > > - switch(opt) { > > > - case OPT_KEXEC_FILE_SYSCALL: > > > - do_kexec_file_syscall = 1; > > > - break; > > > - } > > > - } > > > - > > > /* Reset getopt for the next pass. */ > > > opterr = 1; > > > optind = 1; > > > @@ -1310,8 +1298,7 @@ int main(int argc, char *argv[]) > > > do_shutdown = 0; > > > do_sync = 0; > > > do_unload = 1; > > > - if (do_kexec_file_syscall) > > > - kexec_file_flags |= KEXEC_FILE_UNLOAD; > > > + kexec_file_flags |= KEXEC_FILE_UNLOAD; > > > break; > > > case OPT_EXEC: > > > do_load = 0; > > > @@ -1354,11 +1341,8 @@ int main(int argc, char *argv[]) > > > do_exec = 0; > > > do_shutdown = 0; > > > do_sync = 0; > > > - if (do_kexec_file_syscall) > > > - kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > > - else > > > - kexec_flags = KEXEC_ON_CRASH; > > > - break; > > > + kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > > + kexec_flags = KEXEC_ON_CRASH; > > > case OPT_MEM_MIN: > > > mem_min = strtoul(optarg, &endptr, 0); > > > if (*endptr) { > > > @@ -1383,7 +1367,12 @@ int main(int argc, char *argv[]) > > > do_reuse_initrd = 1; > > > break; > > > case OPT_KEXEC_FILE_SYSCALL: > > > - /* We already parsed it. Nothing to do. */ > > > + do_kexec_file_syscall = 1; > > > + do_kexec_syscall = 0; > > > + break; > > > + case OPT_KEXEC_SYSCALL: > > > + do_kexec_file_syscall = 0; > > > + do_kexec_syscall = 1; > > > break; > > > case OPT_STATUS: > > > do_status = 1; > > > @@ -1456,16 +1445,16 @@ int main(int argc, char *argv[]) > > > result = k_status(kexec_flags); > > > } > > > if (do_unload) { > > > - if (do_kexec_file_syscall) > > > + if (!do_kexec_syscall) > > > result = kexec_file_unload(kexec_file_flags); > > > - else > > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > > result = k_unload(kexec_flags); > > > } > > > if (do_load && (result == 0)) { > > > - if (do_kexec_file_syscall) > > > + if (!do_kexec_syscall) > > > result = do_kexec_file_load(fileind, argc, argv, > > > kexec_file_flags); > > > - else > > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > > result = my_load(type, fileind, argc, argv, > > > kexec_flags, entry); > > > } > > > diff --git a/kexec/kexec.h b/kexec/kexec.h > > > index 26225d2c002a..7abcec796cae 100644 > > > --- a/kexec/kexec.h > > > +++ b/kexec/kexec.h > > > @@ -219,6 +219,7 @@ extern int file_types; > > > #define OPT_TYPE 't' > > > #define OPT_PANIC 'p' > > > #define OPT_KEXEC_FILE_SYSCALL 's' > > > +#define OPT_KEXEC_SYSCALL 'c' > > > #define OPT_STATUS 'S' > > > #define OPT_MEM_MIN 256 > > > #define OPT_MEM_MAX 257 > > > @@ -246,6 +247,7 @@ extern int file_types; > > > { "mem-max", 1, 0, OPT_MEM_MAX }, \ > > > { "reuseinitrd", 0, 0, OPT_REUSE_INITRD }, \ > > > { "kexec-file-syscall", 0, 0, OPT_KEXEC_FILE_SYSCALL }, \ > > > + { "kexec-syscall", 0, 0, OPT_KEXEC_SYSCALL }, \ > > > { "debug", 0, 0, OPT_DEBUG }, \ > > > { "status", 0, 0, OPT_STATUS }, \ > > > { "print-ckr-size", 0, 0, OPT_PRINT_CKR_SIZE }, \ > > > -- > > > 2.13.6 > > > > > > > > > _______________________________________________ > > > kexec mailing list > > > kexec@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/kexec > > > > _______________________________________________ > > kexec mailing list > > kexec@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/kexec > > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec Thanks Dave _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-24 1:43 ` Dave Young @ 2018-02-24 16:34 ` Petr Tesarik 2018-02-26 1:45 ` Dave Young 2018-02-24 20:02 ` Michal Suchánek 1 sibling, 1 reply; 15+ messages in thread From: Petr Tesarik @ 2018-02-24 16:34 UTC (permalink / raw) To: Dave Young; +Cc: horms, Tony Jones, Michal Suchanek, kexec, Baoquan He On Sat, 24 Feb 2018 09:43:42 +0800 Dave Young <dyoung@redhat.com> wrote: > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > Hi Baoquan, > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > Baoquan He <bhe@redhat.com> wrote: > > > > > Hi Michal, > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > > The new KEXEC_FILE_LOAD is preferred in the case the platform supports > > > > it because it allows kexec in locked down secure boot mode. > > > > > > > > However, some platforms do not support it so fall back to the old > > > > syscall there. > > > > > > I didn't read code change, just from patch log, I tend to not agree. There > > > are two options KEXEC_FILE_LOAD and KEXEC_LOAD, some platforms do not > > > support, why does some platforms not choose KEXEC_LOAD, the working one? > > > Why bother to make change in code? I believe there's returned message > > > telling if KEXEC_FILE_LOAD works or not. > > > > Well... let me give a bit of background. As you have probably noticed, > > this syscall was originally available only for x86_64, but more and > > more architectures are also adding it now. > > > > Next, kexec is actually called by a script (which locates a suitable > > kernel and initrd, constructs the kernel command line, etc.). The > > script must either: > > > > A. know somehow if the currently running kernel implements > > kexec_file_load(2), or > > > > B. try one method first, and if it fails, retry with the other. > > > > I agree that kexec(1) should probably allow the user to force a > > specific method, but I don't see the benefit of implementing fallback > > in an external script and not in kexec-tools itself. > > > > OTOH if you want to push the fallback logic out of kexec-tools, then I > > would like to get better diagnostic at least. Letting my script parse > > kexec output is, um, suboptimal. > > In Fedora/RHEL we use this in scripts by checking the arch first, > for distribution it is enough? No. First, you would also have to check the kernel version (and maintain an ugly mapping of which kernel version introduced kexec_file_load on which architecture). Second, it's not just the architecture. kexec_load(2) will fail if SecureBoot is active. OTOH kexec_file_load(2) will fail if the kernel is not signed. For kernel hackers who don't use SecureBoot, signing self-built kernels is just overkill. So, you should also check the state of SecureBoot, possibly also whether the kernel image is signed with a valid key, repeating a bit too much of the kernel logic, and quite likely introducing subtle differences... Petr T > There are also some other arch dependent > options in kexec-tools, there is no way to just use same for every > different platform without checking in scripts. > > If your scripts is not for a distribution, I agree that it is indeed a > problem. > > > > Petr T > > > > > Thanks > > > Baoquan > > > > > > > > Also provide an option to call the old syscall in case the new syscall > > > > fails with other reason than ENOSYS. > > > > > > > > Also document the options. > > > > > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > > --- > > > > kexec/kexec.8 | 9 +++++++++ > > > > kexec/kexec.c | 41 +++++++++++++++-------------------------- > > > > kexec/kexec.h | 2 ++ > > > > 3 files changed, 26 insertions(+), 26 deletions(-) > > > > > > > > diff --git a/kexec/kexec.8 b/kexec/kexec.8 > > > > index e0131b4ea827..7e4df723251d 100644 > > > > --- a/kexec/kexec.8 > > > > +++ b/kexec/kexec.8 > > > > @@ -144,6 +144,15 @@ Load the new kernel for use on panic. > > > > Specify that the new kernel is of this > > > > .I type. > > > > .TP > > > > +.BI \-s\ (\-\-kexec-file-syscall) > > > > +Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively. > > > > +Otherwise KEXEC_FILE_LOAD is tried and when not supported KEXEC_LOAD is used. > > > > +.I type. > > > > +.TP > > > > +.BI \-c\ (\-\-kexec-syscall) > > > > +Specify that the old KEXEC_LOAD syscall should be used exclusively. > > > > +.I type. > > > > +.TP > > > > .B \-u\ (\-\-unload) > > > > Unload the current > > > > .B kexec > > > > diff --git a/kexec/kexec.c b/kexec/kexec.c > > > > index cfd837c1b6bb..25328c02b508 100644 > > > > --- a/kexec/kexec.c > > > > +++ b/kexec/kexec.c > > > > @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv, > > > > > > > > if (!is_kexec_file_load_implemented()) { > > > > fprintf(stderr, "syscall kexec_file_load not available.\n"); > > > > - return -1; > > > > + return -ENOSYS; > > > > } > > > > > > > > if (argc - fileind <= 0) { > > > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[]) > > > > int do_unload = 0; > > > > int do_reuse_initrd = 0; > > > > int do_kexec_file_syscall = 0; > > > > + int do_kexec_syscall = 0; > > > > int do_status = 0; > > > > void *entry = 0; > > > > char *type = 0; > > > > @@ -1256,19 +1257,6 @@ int main(int argc, char *argv[]) > > > > }; > > > > static const char short_options[] = KEXEC_ALL_OPT_STR; > > > > > > > > - /* > > > > - * First check if --use-kexec-file-syscall is set. That changes lot of > > > > - * things > > > > - */ > > > > - while ((opt = getopt_long(argc, argv, short_options, > > > > - options, 0)) != -1) { > > > > - switch(opt) { > > > > - case OPT_KEXEC_FILE_SYSCALL: > > > > - do_kexec_file_syscall = 1; > > > > - break; > > > > - } > > > > - } > > > > - > > > > /* Reset getopt for the next pass. */ > > > > opterr = 1; > > > > optind = 1; > > > > @@ -1310,8 +1298,7 @@ int main(int argc, char *argv[]) > > > > do_shutdown = 0; > > > > do_sync = 0; > > > > do_unload = 1; > > > > - if (do_kexec_file_syscall) > > > > - kexec_file_flags |= KEXEC_FILE_UNLOAD; > > > > + kexec_file_flags |= KEXEC_FILE_UNLOAD; > > > > break; > > > > case OPT_EXEC: > > > > do_load = 0; > > > > @@ -1354,11 +1341,8 @@ int main(int argc, char *argv[]) > > > > do_exec = 0; > > > > do_shutdown = 0; > > > > do_sync = 0; > > > > - if (do_kexec_file_syscall) > > > > - kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > > > - else > > > > - kexec_flags = KEXEC_ON_CRASH; > > > > - break; > > > > + kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > > > + kexec_flags = KEXEC_ON_CRASH; > > > > case OPT_MEM_MIN: > > > > mem_min = strtoul(optarg, &endptr, 0); > > > > if (*endptr) { > > > > @@ -1383,7 +1367,12 @@ int main(int argc, char *argv[]) > > > > do_reuse_initrd = 1; > > > > break; > > > > case OPT_KEXEC_FILE_SYSCALL: > > > > - /* We already parsed it. Nothing to do. */ > > > > + do_kexec_file_syscall = 1; > > > > + do_kexec_syscall = 0; > > > > + break; > > > > + case OPT_KEXEC_SYSCALL: > > > > + do_kexec_file_syscall = 0; > > > > + do_kexec_syscall = 1; > > > > break; > > > > case OPT_STATUS: > > > > do_status = 1; > > > > @@ -1456,16 +1445,16 @@ int main(int argc, char *argv[]) > > > > result = k_status(kexec_flags); > > > > } > > > > if (do_unload) { > > > > - if (do_kexec_file_syscall) > > > > + if (!do_kexec_syscall) > > > > result = kexec_file_unload(kexec_file_flags); > > > > - else > > > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > > > result = k_unload(kexec_flags); > > > > } > > > > if (do_load && (result == 0)) { > > > > - if (do_kexec_file_syscall) > > > > + if (!do_kexec_syscall) > > > > result = do_kexec_file_load(fileind, argc, argv, > > > > kexec_file_flags); > > > > - else > > > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > > > result = my_load(type, fileind, argc, argv, > > > > kexec_flags, entry); > > > > } > > > > diff --git a/kexec/kexec.h b/kexec/kexec.h > > > > index 26225d2c002a..7abcec796cae 100644 > > > > --- a/kexec/kexec.h > > > > +++ b/kexec/kexec.h > > > > @@ -219,6 +219,7 @@ extern int file_types; > > > > #define OPT_TYPE 't' > > > > #define OPT_PANIC 'p' > > > > #define OPT_KEXEC_FILE_SYSCALL 's' > > > > +#define OPT_KEXEC_SYSCALL 'c' > > > > #define OPT_STATUS 'S' > > > > #define OPT_MEM_MIN 256 > > > > #define OPT_MEM_MAX 257 > > > > @@ -246,6 +247,7 @@ extern int file_types; > > > > { "mem-max", 1, 0, OPT_MEM_MAX }, \ > > > > { "reuseinitrd", 0, 0, OPT_REUSE_INITRD }, \ > > > > { "kexec-file-syscall", 0, 0, OPT_KEXEC_FILE_SYSCALL }, \ > > > > + { "kexec-syscall", 0, 0, OPT_KEXEC_SYSCALL }, \ > > > > { "debug", 0, 0, OPT_DEBUG }, \ > > > > { "status", 0, 0, OPT_STATUS }, \ > > > > { "print-ckr-size", 0, 0, OPT_PRINT_CKR_SIZE }, \ > > > > -- > > > > 2.13.6 > > > > > > > > > > > > _______________________________________________ > > > > kexec mailing list > > > > kexec@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/kexec > > > > > > _______________________________________________ > > > kexec mailing list > > > kexec@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/kexec > > > > > > _______________________________________________ > > kexec mailing list > > kexec@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/kexec > > Thanks > Dave _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-24 16:34 ` Petr Tesarik @ 2018-02-26 1:45 ` Dave Young 2018-02-26 12:08 ` Michal Suchánek 0 siblings, 1 reply; 15+ messages in thread From: Dave Young @ 2018-02-26 1:45 UTC (permalink / raw) To: Petr Tesarik; +Cc: horms, Tony Jones, Michal Suchanek, kexec, Baoquan He On 02/24/18 at 05:34pm, Petr Tesarik wrote: > On Sat, 24 Feb 2018 09:43:42 +0800 > Dave Young <dyoung@redhat.com> wrote: > > > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > > Hi Baoquan, > > > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > > Baoquan He <bhe@redhat.com> wrote: > > > > > > > Hi Michal, > > > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > > > The new KEXEC_FILE_LOAD is preferred in the case the platform supports > > > > > it because it allows kexec in locked down secure boot mode. > > > > > > > > > > However, some platforms do not support it so fall back to the old > > > > > syscall there. > > > > > > > > I didn't read code change, just from patch log, I tend to not agree. There > > > > are two options KEXEC_FILE_LOAD and KEXEC_LOAD, some platforms do not > > > > support, why does some platforms not choose KEXEC_LOAD, the working one? > > > > Why bother to make change in code? I believe there's returned message > > > > telling if KEXEC_FILE_LOAD works or not. > > > > > > Well... let me give a bit of background. As you have probably noticed, > > > this syscall was originally available only for x86_64, but more and > > > more architectures are also adding it now. > > > > > > Next, kexec is actually called by a script (which locates a suitable > > > kernel and initrd, constructs the kernel command line, etc.). The > > > script must either: > > > > > > A. know somehow if the currently running kernel implements > > > kexec_file_load(2), or > > > > > > B. try one method first, and if it fails, retry with the other. > > > > > > I agree that kexec(1) should probably allow the user to force a > > > specific method, but I don't see the benefit of implementing fallback > > > in an external script and not in kexec-tools itself. > > > > > > OTOH if you want to push the fallback logic out of kexec-tools, then I > > > would like to get better diagnostic at least. Letting my script parse > > > kexec output is, um, suboptimal. > > > > In Fedora/RHEL we use this in scripts by checking the arch first, > > for distribution it is enough? > > No. > > First, you would also have to check the kernel version (and > maintain an ugly mapping of which kernel version introduced > kexec_file_load on which architecture). The kernel version update is rare for these new syscall added, but it is indeed needed to match with them > > Second, it's not just the architecture. kexec_load(2) will fail if > SecureBoot is active. OTOH kexec_file_load(2) will fail if the kernel > is not signed. For kernel hackers who don't use SecureBoot, signing > self-built kernels is just overkill. So, you should also check the > state of SecureBoot, possibly also whether the kernel image is signed > with a valid key, repeating a bit too much of the kernel logic, and > quite likely introducing subtle differences... Hmm, I did not say the exact details, yes, we checked the Secure Boot state and only use kexec_file_load for that special case. kexec_file and kexec_file_load is not exactly same so if one want to use one instead of another for a specific functionality it seems not good to automatically switch to another if one failed. For example which one should be the first choice, it is hard to say. > > Petr T > > > There are also some other arch dependent > > options in kexec-tools, there is no way to just use same for every > > different platform without checking in scripts. > > > > If your scripts is not for a distribution, I agree that it is indeed a > > problem. > > > > > > Petr T > > > > > > > Thanks > > > > Baoquan > > > > > > > > > > Also provide an option to call the old syscall in case the new syscall > > > > > fails with other reason than ENOSYS. > > > > > > > > > > Also document the options. > > > > > > > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > > > --- > > > > > kexec/kexec.8 | 9 +++++++++ > > > > > kexec/kexec.c | 41 +++++++++++++++-------------------------- > > > > > kexec/kexec.h | 2 ++ > > > > > 3 files changed, 26 insertions(+), 26 deletions(-) > > > > > > > > > > diff --git a/kexec/kexec.8 b/kexec/kexec.8 > > > > > index e0131b4ea827..7e4df723251d 100644 > > > > > --- a/kexec/kexec.8 > > > > > +++ b/kexec/kexec.8 > > > > > @@ -144,6 +144,15 @@ Load the new kernel for use on panic. > > > > > Specify that the new kernel is of this > > > > > .I type. > > > > > .TP > > > > > +.BI \-s\ (\-\-kexec-file-syscall) > > > > > +Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively. > > > > > +Otherwise KEXEC_FILE_LOAD is tried and when not supported KEXEC_LOAD is used. > > > > > +.I type. > > > > > +.TP > > > > > +.BI \-c\ (\-\-kexec-syscall) > > > > > +Specify that the old KEXEC_LOAD syscall should be used exclusively. > > > > > +.I type. > > > > > +.TP > > > > > .B \-u\ (\-\-unload) > > > > > Unload the current > > > > > .B kexec > > > > > diff --git a/kexec/kexec.c b/kexec/kexec.c > > > > > index cfd837c1b6bb..25328c02b508 100644 > > > > > --- a/kexec/kexec.c > > > > > +++ b/kexec/kexec.c > > > > > @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv, > > > > > > > > > > if (!is_kexec_file_load_implemented()) { > > > > > fprintf(stderr, "syscall kexec_file_load not available.\n"); > > > > > - return -1; > > > > > + return -ENOSYS; > > > > > } > > > > > > > > > > if (argc - fileind <= 0) { > > > > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[]) > > > > > int do_unload = 0; > > > > > int do_reuse_initrd = 0; > > > > > int do_kexec_file_syscall = 0; > > > > > + int do_kexec_syscall = 0; > > > > > int do_status = 0; > > > > > void *entry = 0; > > > > > char *type = 0; > > > > > @@ -1256,19 +1257,6 @@ int main(int argc, char *argv[]) > > > > > }; > > > > > static const char short_options[] = KEXEC_ALL_OPT_STR; > > > > > > > > > > - /* > > > > > - * First check if --use-kexec-file-syscall is set. That changes lot of > > > > > - * things > > > > > - */ > > > > > - while ((opt = getopt_long(argc, argv, short_options, > > > > > - options, 0)) != -1) { > > > > > - switch(opt) { > > > > > - case OPT_KEXEC_FILE_SYSCALL: > > > > > - do_kexec_file_syscall = 1; > > > > > - break; > > > > > - } > > > > > - } > > > > > - > > > > > /* Reset getopt for the next pass. */ > > > > > opterr = 1; > > > > > optind = 1; > > > > > @@ -1310,8 +1298,7 @@ int main(int argc, char *argv[]) > > > > > do_shutdown = 0; > > > > > do_sync = 0; > > > > > do_unload = 1; > > > > > - if (do_kexec_file_syscall) > > > > > - kexec_file_flags |= KEXEC_FILE_UNLOAD; > > > > > + kexec_file_flags |= KEXEC_FILE_UNLOAD; > > > > > break; > > > > > case OPT_EXEC: > > > > > do_load = 0; > > > > > @@ -1354,11 +1341,8 @@ int main(int argc, char *argv[]) > > > > > do_exec = 0; > > > > > do_shutdown = 0; > > > > > do_sync = 0; > > > > > - if (do_kexec_file_syscall) > > > > > - kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > > > > - else > > > > > - kexec_flags = KEXEC_ON_CRASH; > > > > > - break; > > > > > + kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > > > > + kexec_flags = KEXEC_ON_CRASH; > > > > > case OPT_MEM_MIN: > > > > > mem_min = strtoul(optarg, &endptr, 0); > > > > > if (*endptr) { > > > > > @@ -1383,7 +1367,12 @@ int main(int argc, char *argv[]) > > > > > do_reuse_initrd = 1; > > > > > break; > > > > > case OPT_KEXEC_FILE_SYSCALL: > > > > > - /* We already parsed it. Nothing to do. */ > > > > > + do_kexec_file_syscall = 1; > > > > > + do_kexec_syscall = 0; > > > > > + break; > > > > > + case OPT_KEXEC_SYSCALL: > > > > > + do_kexec_file_syscall = 0; > > > > > + do_kexec_syscall = 1; > > > > > break; > > > > > case OPT_STATUS: > > > > > do_status = 1; > > > > > @@ -1456,16 +1445,16 @@ int main(int argc, char *argv[]) > > > > > result = k_status(kexec_flags); > > > > > } > > > > > if (do_unload) { > > > > > - if (do_kexec_file_syscall) > > > > > + if (!do_kexec_syscall) > > > > > result = kexec_file_unload(kexec_file_flags); > > > > > - else > > > > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > > > > result = k_unload(kexec_flags); > > > > > } > > > > > if (do_load && (result == 0)) { > > > > > - if (do_kexec_file_syscall) > > > > > + if (!do_kexec_syscall) > > > > > result = do_kexec_file_load(fileind, argc, argv, > > > > > kexec_file_flags); > > > > > - else > > > > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > > > > result = my_load(type, fileind, argc, argv, > > > > > kexec_flags, entry); > > > > > } > > > > > diff --git a/kexec/kexec.h b/kexec/kexec.h > > > > > index 26225d2c002a..7abcec796cae 100644 > > > > > --- a/kexec/kexec.h > > > > > +++ b/kexec/kexec.h > > > > > @@ -219,6 +219,7 @@ extern int file_types; > > > > > #define OPT_TYPE 't' > > > > > #define OPT_PANIC 'p' > > > > > #define OPT_KEXEC_FILE_SYSCALL 's' > > > > > +#define OPT_KEXEC_SYSCALL 'c' > > > > > #define OPT_STATUS 'S' > > > > > #define OPT_MEM_MIN 256 > > > > > #define OPT_MEM_MAX 257 > > > > > @@ -246,6 +247,7 @@ extern int file_types; > > > > > { "mem-max", 1, 0, OPT_MEM_MAX }, \ > > > > > { "reuseinitrd", 0, 0, OPT_REUSE_INITRD }, \ > > > > > { "kexec-file-syscall", 0, 0, OPT_KEXEC_FILE_SYSCALL }, \ > > > > > + { "kexec-syscall", 0, 0, OPT_KEXEC_SYSCALL }, \ > > > > > { "debug", 0, 0, OPT_DEBUG }, \ > > > > > { "status", 0, 0, OPT_STATUS }, \ > > > > > { "print-ckr-size", 0, 0, OPT_PRINT_CKR_SIZE }, \ > > > > > -- > > > > > 2.13.6 > > > > > > > > > > > > > > > _______________________________________________ > > > > > kexec mailing list > > > > > kexec@lists.infradead.org > > > > > http://lists.infradead.org/mailman/listinfo/kexec > > > > > > > > _______________________________________________ > > > > kexec mailing list > > > > kexec@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/kexec > > > > > > > > > _______________________________________________ > > > kexec mailing list > > > kexec@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/kexec > > > > Thanks > > Dave > Thanks Dave _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-26 1:45 ` Dave Young @ 2018-02-26 12:08 ` Michal Suchánek 2018-02-27 1:15 ` Dave Young 0 siblings, 1 reply; 15+ messages in thread From: Michal Suchánek @ 2018-02-26 12:08 UTC (permalink / raw) To: Dave Young; +Cc: Tony Jones, horms, Petr Tesarik, kexec, Baoquan He On Mon, 26 Feb 2018 09:45:15 +0800 Dave Young <dyoung@redhat.com> wrote: > On 02/24/18 at 05:34pm, Petr Tesarik wrote: > > On Sat, 24 Feb 2018 09:43:42 +0800 > > Dave Young <dyoung@redhat.com> wrote: > > > > > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > > > Hi Baoquan, > > > > > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > > > Baoquan He <bhe@redhat.com> wrote: > > > > > > > > > Hi Michal, > > > > > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > > > > The new KEXEC_FILE_LOAD is preferred in the case the > > > > > > platform supports it because it allows kexec in locked down > > > > > > secure boot mode. > > > > > > > > > > > > However, some platforms do not support it so fall back to > > > > > > the old syscall there. > > > > > > > > > > I didn't read code change, just from patch log, I tend to not > > > > > agree. There are two options KEXEC_FILE_LOAD and KEXEC_LOAD, > > > > > some platforms do not support, why does some platforms not > > > > > choose KEXEC_LOAD, the working one? Why bother to make change > > > > > in code? I believe there's returned message telling if > > > > > KEXEC_FILE_LOAD works or not. > > > > > > > > Well... let me give a bit of background. As you have probably > > > > noticed, this syscall was originally available only for x86_64, > > > > but more and more architectures are also adding it now. > > > > > > > > Next, kexec is actually called by a script (which locates a > > > > suitable kernel and initrd, constructs the kernel command line, > > > > etc.). The script must either: > > > > > > > > A. know somehow if the currently running kernel implements > > > > kexec_file_load(2), or > > > > > > > > B. try one method first, and if it fails, retry with the > > > > other. > > > > > > > > I agree that kexec(1) should probably allow the user to force a > > > > specific method, but I don't see the benefit of implementing > > > > fallback in an external script and not in kexec-tools itself. > > > > > > > > OTOH if you want to push the fallback logic out of kexec-tools, > > > > then I would like to get better diagnostic at least. Letting my > > > > script parse kexec output is, um, suboptimal. > > > > > > In Fedora/RHEL we use this in scripts by checking the arch first, > > > for distribution it is enough? > > > > No. > > > > First, you would also have to check the kernel version (and > > maintain an ugly mapping of which kernel version introduced > > kexec_file_load on which architecture). > > The kernel version update is rare for these new syscall added, but > it is indeed needed to match with them > > > > > Second, it's not just the architecture. kexec_load(2) will fail if > > SecureBoot is active. OTOH kexec_file_load(2) will fail if the > > kernel is not signed. For kernel hackers who don't use SecureBoot, > > signing self-built kernels is just overkill. So, you should also > > check the state of SecureBoot, possibly also whether the kernel > > image is signed with a valid key, repeating a bit too much of the > > kernel logic, and quite likely introducing subtle differences... > > Hmm, I did not say the exact details, yes, we checked the Secure Boot > state and only use kexec_file_load for that special case. > > kexec_file and kexec_file_load is not exactly same so if one want to > use one instead of another for a specific functionality it seems not > good to automatically switch to another if one failed. For example > which one should be the first choice, it is hard to say. Hard to say indeed, However, I would assume that architectures that implement kexec_file_load do so because it is required in some case and hence it will be actively maintained when available. However, some architectures may not require it and will be slow to implement it. So using kexec_file_load when available sounds like the right thing. Technically the implementation details are different but for most users this does not matter. For those that do care I provide option to select one or the other. Thanks Michal _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-26 12:08 ` Michal Suchánek @ 2018-02-27 1:15 ` Dave Young 2018-02-27 8:39 ` Petr Tesarik 0 siblings, 1 reply; 15+ messages in thread From: Dave Young @ 2018-02-27 1:15 UTC (permalink / raw) To: Michal Suchánek; +Cc: Tony Jones, horms, Petr Tesarik, kexec, Baoquan He On 02/26/18 at 01:08pm, Michal Suchánek wrote: > On Mon, 26 Feb 2018 09:45:15 +0800 > Dave Young <dyoung@redhat.com> wrote: > > > On 02/24/18 at 05:34pm, Petr Tesarik wrote: > > > On Sat, 24 Feb 2018 09:43:42 +0800 > > > Dave Young <dyoung@redhat.com> wrote: > > > > > > > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > > > > Hi Baoquan, > > > > > > > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > > > > Baoquan He <bhe@redhat.com> wrote: > > > > > > > > > > > Hi Michal, > > > > > > > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > > > > > The new KEXEC_FILE_LOAD is preferred in the case the > > > > > > > platform supports it because it allows kexec in locked down > > > > > > > secure boot mode. > > > > > > > > > > > > > > However, some platforms do not support it so fall back to > > > > > > > the old syscall there. > > > > > > > > > > > > I didn't read code change, just from patch log, I tend to not > > > > > > agree. There are two options KEXEC_FILE_LOAD and KEXEC_LOAD, > > > > > > some platforms do not support, why does some platforms not > > > > > > choose KEXEC_LOAD, the working one? Why bother to make change > > > > > > in code? I believe there's returned message telling if > > > > > > KEXEC_FILE_LOAD works or not. > > > > > > > > > > Well... let me give a bit of background. As you have probably > > > > > noticed, this syscall was originally available only for x86_64, > > > > > but more and more architectures are also adding it now. > > > > > > > > > > Next, kexec is actually called by a script (which locates a > > > > > suitable kernel and initrd, constructs the kernel command line, > > > > > etc.). The script must either: > > > > > > > > > > A. know somehow if the currently running kernel implements > > > > > kexec_file_load(2), or > > > > > > > > > > B. try one method first, and if it fails, retry with the > > > > > other. > > > > > > > > > > I agree that kexec(1) should probably allow the user to force a > > > > > specific method, but I don't see the benefit of implementing > > > > > fallback in an external script and not in kexec-tools itself. > > > > > > > > > > OTOH if you want to push the fallback logic out of kexec-tools, > > > > > then I would like to get better diagnostic at least. Letting my > > > > > script parse kexec output is, um, suboptimal. > > > > > > > > In Fedora/RHEL we use this in scripts by checking the arch first, > > > > for distribution it is enough? > > > > > > No. > > > > > > First, you would also have to check the kernel version (and > > > maintain an ugly mapping of which kernel version introduced > > > kexec_file_load on which architecture). > > > > The kernel version update is rare for these new syscall added, but > > it is indeed needed to match with them > > > > > > > > Second, it's not just the architecture. kexec_load(2) will fail if > > > SecureBoot is active. OTOH kexec_file_load(2) will fail if the > > > kernel is not signed. For kernel hackers who don't use SecureBoot, > > > signing self-built kernels is just overkill. So, you should also > > > check the state of SecureBoot, possibly also whether the kernel > > > image is signed with a valid key, repeating a bit too much of the > > > kernel logic, and quite likely introducing subtle differences... > > > > Hmm, I did not say the exact details, yes, we checked the Secure Boot > > state and only use kexec_file_load for that special case. > > > > kexec_file and kexec_file_load is not exactly same so if one want to > > use one instead of another for a specific functionality it seems not > > good to automatically switch to another if one failed. For example > > which one should be the first choice, it is hard to say. > > Hard to say indeed, However, I would assume that architectures that > implement kexec_file_load do so because it is required in some case and > hence it will be actively maintained when available. However, some > architectures may not require it and will be slow to implement it. So > using kexec_file_load when available sounds like the right thing. > Technically the implementation details are different but for most users > this does not matter. > > For those that do care I provide option to select one or the other. I would say it breaks things, a better way should be introducing another kexec-tools option for example kexec --auto for this purpose. Probably add some --auto=... to select the first chance. > > Thanks > > Michal > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec Thanks Dave _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-27 1:15 ` Dave Young @ 2018-02-27 8:39 ` Petr Tesarik 2018-02-27 9:09 ` Dave Young 0 siblings, 1 reply; 15+ messages in thread From: Petr Tesarik @ 2018-02-27 8:39 UTC (permalink / raw) To: Dave Young; +Cc: Tony Jones, Michal Suchánek, kexec, Baoquan He, horms On Tue, 27 Feb 2018 09:15:10 +0800 Dave Young <dyoung@redhat.com> wrote: > On 02/26/18 at 01:08pm, Michal Suchánek wrote: > > On Mon, 26 Feb 2018 09:45:15 +0800 > > Dave Young <dyoung@redhat.com> wrote: > > > > > On 02/24/18 at 05:34pm, Petr Tesarik wrote: > > > > On Sat, 24 Feb 2018 09:43:42 +0800 > > > > Dave Young <dyoung@redhat.com> wrote: > > > > > > > > > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > > > > > Hi Baoquan, > > > > > > > > > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > > > > > Baoquan He <bhe@redhat.com> wrote: > > > > > > > > > > > > > Hi Michal, > > > > > > > > > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > > > > > > The new KEXEC_FILE_LOAD is preferred in the case the > > > > > > > > platform supports it because it allows kexec in locked down > > > > > > > > secure boot mode. > > > > > > > > > > > > > > > > However, some platforms do not support it so fall back to > > > > > > > > the old syscall there. > > > > > > > > > > > > > > I didn't read code change, just from patch log, I tend to not > > > > > > > agree. There are two options KEXEC_FILE_LOAD and KEXEC_LOAD, > > > > > > > some platforms do not support, why does some platforms not > > > > > > > choose KEXEC_LOAD, the working one? Why bother to make change > > > > > > > in code? I believe there's returned message telling if > > > > > > > KEXEC_FILE_LOAD works or not. > > > > > > > > > > > > Well... let me give a bit of background. As you have probably > > > > > > noticed, this syscall was originally available only for x86_64, > > > > > > but more and more architectures are also adding it now. > > > > > > > > > > > > Next, kexec is actually called by a script (which locates a > > > > > > suitable kernel and initrd, constructs the kernel command line, > > > > > > etc.). The script must either: > > > > > > > > > > > > A. know somehow if the currently running kernel implements > > > > > > kexec_file_load(2), or > > > > > > > > > > > > B. try one method first, and if it fails, retry with the > > > > > > other. > > > > > > > > > > > > I agree that kexec(1) should probably allow the user to force a > > > > > > specific method, but I don't see the benefit of implementing > > > > > > fallback in an external script and not in kexec-tools itself. > > > > > > > > > > > > OTOH if you want to push the fallback logic out of kexec-tools, > > > > > > then I would like to get better diagnostic at least. Letting my > > > > > > script parse kexec output is, um, suboptimal. > > > > > > > > > > In Fedora/RHEL we use this in scripts by checking the arch first, > > > > > for distribution it is enough? > > > > > > > > No. > > > > > > > > First, you would also have to check the kernel version (and > > > > maintain an ugly mapping of which kernel version introduced > > > > kexec_file_load on which architecture). > > > > > > The kernel version update is rare for these new syscall added, but > > > it is indeed needed to match with them > > > > > > > > > > > Second, it's not just the architecture. kexec_load(2) will fail if > > > > SecureBoot is active. OTOH kexec_file_load(2) will fail if the > > > > kernel is not signed. For kernel hackers who don't use SecureBoot, > > > > signing self-built kernels is just overkill. So, you should also > > > > check the state of SecureBoot, possibly also whether the kernel > > > > image is signed with a valid key, repeating a bit too much of the > > > > kernel logic, and quite likely introducing subtle differences... > > > > > > Hmm, I did not say the exact details, yes, we checked the Secure Boot > > > state and only use kexec_file_load for that special case. > > > > > > kexec_file and kexec_file_load is not exactly same so if one want to > > > use one instead of another for a specific functionality it seems not > > > good to automatically switch to another if one failed. For example > > > which one should be the first choice, it is hard to say. > > > > Hard to say indeed, However, I would assume that architectures that > > implement kexec_file_load do so because it is required in some case and > > hence it will be actively maintained when available. However, some > > architectures may not require it and will be slow to implement it. So > > using kexec_file_load when available sounds like the right thing. > > Technically the implementation details are different but for most users > > this does not matter. > > > > For those that do care I provide option to select one or the other. > > I would say it breaks things, a better way should be introducing another > kexec-tools option for example kexec --auto for this purpose. Probably > add some --auto=... to select the first chance. Yes! This is the way to go. But then I wouldn't call it --auto. I would call it "--method=", so you could specify: --method=kernel (to use the in-kernel loader aka kexec_file_load) --method=user (to use the traditional user-space loader) --method=kernel,user (to prefer kexec_file_load) --method=user,kernel (to prefer kexc_load) I'm not quite sure if it also makes sense to provide "--method=auto", which would use whatever default is considered sane for the running system. Petr T _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-27 8:39 ` Petr Tesarik @ 2018-02-27 9:09 ` Dave Young 2018-02-27 9:14 ` Dave Young 0 siblings, 1 reply; 15+ messages in thread From: Dave Young @ 2018-02-27 9:09 UTC (permalink / raw) To: Petr Tesarik; +Cc: Tony Jones, Michal Suchánek, kexec, Baoquan He, horms On 02/27/18 at 09:39am, Petr Tesarik wrote: > On Tue, 27 Feb 2018 09:15:10 +0800 > Dave Young <dyoung@redhat.com> wrote: > > > On 02/26/18 at 01:08pm, Michal Suchánek wrote: > > > On Mon, 26 Feb 2018 09:45:15 +0800 > > > Dave Young <dyoung@redhat.com> wrote: > > > > > > > On 02/24/18 at 05:34pm, Petr Tesarik wrote: > > > > > On Sat, 24 Feb 2018 09:43:42 +0800 > > > > > Dave Young <dyoung@redhat.com> wrote: > > > > > > > > > > > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > > > > > > Hi Baoquan, > > > > > > > > > > > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > > > > > > Baoquan He <bhe@redhat.com> wrote: > > > > > > > > > > > > > > > Hi Michal, > > > > > > > > > > > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > > > > > > > The new KEXEC_FILE_LOAD is preferred in the case the > > > > > > > > > platform supports it because it allows kexec in locked down > > > > > > > > > secure boot mode. > > > > > > > > > > > > > > > > > > However, some platforms do not support it so fall back to > > > > > > > > > the old syscall there. > > > > > > > > > > > > > > > > I didn't read code change, just from patch log, I tend to not > > > > > > > > agree. There are two options KEXEC_FILE_LOAD and KEXEC_LOAD, > > > > > > > > some platforms do not support, why does some platforms not > > > > > > > > choose KEXEC_LOAD, the working one? Why bother to make change > > > > > > > > in code? I believe there's returned message telling if > > > > > > > > KEXEC_FILE_LOAD works or not. > > > > > > > > > > > > > > Well... let me give a bit of background. As you have probably > > > > > > > noticed, this syscall was originally available only for x86_64, > > > > > > > but more and more architectures are also adding it now. > > > > > > > > > > > > > > Next, kexec is actually called by a script (which locates a > > > > > > > suitable kernel and initrd, constructs the kernel command line, > > > > > > > etc.). The script must either: > > > > > > > > > > > > > > A. know somehow if the currently running kernel implements > > > > > > > kexec_file_load(2), or > > > > > > > > > > > > > > B. try one method first, and if it fails, retry with the > > > > > > > other. > > > > > > > > > > > > > > I agree that kexec(1) should probably allow the user to force a > > > > > > > specific method, but I don't see the benefit of implementing > > > > > > > fallback in an external script and not in kexec-tools itself. > > > > > > > > > > > > > > OTOH if you want to push the fallback logic out of kexec-tools, > > > > > > > then I would like to get better diagnostic at least. Letting my > > > > > > > script parse kexec output is, um, suboptimal. > > > > > > > > > > > > In Fedora/RHEL we use this in scripts by checking the arch first, > > > > > > for distribution it is enough? > > > > > > > > > > No. > > > > > > > > > > First, you would also have to check the kernel version (and > > > > > maintain an ugly mapping of which kernel version introduced > > > > > kexec_file_load on which architecture). > > > > > > > > The kernel version update is rare for these new syscall added, but > > > > it is indeed needed to match with them > > > > > > > > > > > > > > Second, it's not just the architecture. kexec_load(2) will fail if > > > > > SecureBoot is active. OTOH kexec_file_load(2) will fail if the > > > > > kernel is not signed. For kernel hackers who don't use SecureBoot, > > > > > signing self-built kernels is just overkill. So, you should also > > > > > check the state of SecureBoot, possibly also whether the kernel > > > > > image is signed with a valid key, repeating a bit too much of the > > > > > kernel logic, and quite likely introducing subtle differences... > > > > > > > > Hmm, I did not say the exact details, yes, we checked the Secure Boot > > > > state and only use kexec_file_load for that special case. > > > > > > > > kexec_file and kexec_file_load is not exactly same so if one want to > > > > use one instead of another for a specific functionality it seems not > > > > good to automatically switch to another if one failed. For example > > > > which one should be the first choice, it is hard to say. > > > > > > Hard to say indeed, However, I would assume that architectures that > > > implement kexec_file_load do so because it is required in some case and > > > hence it will be actively maintained when available. However, some > > > architectures may not require it and will be slow to implement it. So > > > using kexec_file_load when available sounds like the right thing. > > > Technically the implementation details are different but for most users > > > this does not matter. > > > > > > For those that do care I provide option to select one or the other. > > > > I would say it breaks things, a better way should be introducing another > > kexec-tools option for example kexec --auto for this purpose. Probably > > add some --auto=... to select the first chance. > > Yes! This is the way to go. But then I wouldn't call it --auto. > I would call it "--method=", so you could specify: > > --method=kernel (to use the in-kernel loader aka kexec_file_load) > --method=user (to use the traditional user-space loader) > --method=kernel,user (to prefer kexec_file_load) > --method=user,kernel (to prefer kexc_load) > > I'm not quite sure if it also makes sense to provide "--method=auto", > which would use whatever default is considered sane for the running > system. If we have --method=kernel,user and --method=user,kernel then no need "auto" anymore.. But I would prefer that original default behavior is kept, kexec -l without "--method" will use userspace loader, kexec -s -l will use kernel space loader > > Petr T Thanks Dave _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-27 9:09 ` Dave Young @ 2018-02-27 9:14 ` Dave Young 2018-02-27 9:49 ` Michal Suchánek 0 siblings, 1 reply; 15+ messages in thread From: Dave Young @ 2018-02-27 9:14 UTC (permalink / raw) To: Petr Tesarik; +Cc: Tony Jones, Michal Suchánek, kexec, Baoquan He, horms On 02/27/18 at 05:09pm, Dave Young wrote: > On 02/27/18 at 09:39am, Petr Tesarik wrote: > > On Tue, 27 Feb 2018 09:15:10 +0800 > > Dave Young <dyoung@redhat.com> wrote: > > > > > On 02/26/18 at 01:08pm, Michal Suchánek wrote: > > > > On Mon, 26 Feb 2018 09:45:15 +0800 > > > > Dave Young <dyoung@redhat.com> wrote: > > > > > > > > > On 02/24/18 at 05:34pm, Petr Tesarik wrote: > > > > > > On Sat, 24 Feb 2018 09:43:42 +0800 > > > > > > Dave Young <dyoung@redhat.com> wrote: > > > > > > > > > > > > > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > > > > > > > Hi Baoquan, > > > > > > > > > > > > > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > > > > > > > Baoquan He <bhe@redhat.com> wrote: > > > > > > > > > > > > > > > > > Hi Michal, > > > > > > > > > > > > > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > > > > > > > > The new KEXEC_FILE_LOAD is preferred in the case the > > > > > > > > > > platform supports it because it allows kexec in locked down > > > > > > > > > > secure boot mode. > > > > > > > > > > > > > > > > > > > > However, some platforms do not support it so fall back to > > > > > > > > > > the old syscall there. > > > > > > > > > > > > > > > > > > I didn't read code change, just from patch log, I tend to not > > > > > > > > > agree. There are two options KEXEC_FILE_LOAD and KEXEC_LOAD, > > > > > > > > > some platforms do not support, why does some platforms not > > > > > > > > > choose KEXEC_LOAD, the working one? Why bother to make change > > > > > > > > > in code? I believe there's returned message telling if > > > > > > > > > KEXEC_FILE_LOAD works or not. > > > > > > > > > > > > > > > > Well... let me give a bit of background. As you have probably > > > > > > > > noticed, this syscall was originally available only for x86_64, > > > > > > > > but more and more architectures are also adding it now. > > > > > > > > > > > > > > > > Next, kexec is actually called by a script (which locates a > > > > > > > > suitable kernel and initrd, constructs the kernel command line, > > > > > > > > etc.). The script must either: > > > > > > > > > > > > > > > > A. know somehow if the currently running kernel implements > > > > > > > > kexec_file_load(2), or > > > > > > > > > > > > > > > > B. try one method first, and if it fails, retry with the > > > > > > > > other. > > > > > > > > > > > > > > > > I agree that kexec(1) should probably allow the user to force a > > > > > > > > specific method, but I don't see the benefit of implementing > > > > > > > > fallback in an external script and not in kexec-tools itself. > > > > > > > > > > > > > > > > OTOH if you want to push the fallback logic out of kexec-tools, > > > > > > > > then I would like to get better diagnostic at least. Letting my > > > > > > > > script parse kexec output is, um, suboptimal. > > > > > > > > > > > > > > In Fedora/RHEL we use this in scripts by checking the arch first, > > > > > > > for distribution it is enough? > > > > > > > > > > > > No. > > > > > > > > > > > > First, you would also have to check the kernel version (and > > > > > > maintain an ugly mapping of which kernel version introduced > > > > > > kexec_file_load on which architecture). > > > > > > > > > > The kernel version update is rare for these new syscall added, but > > > > > it is indeed needed to match with them > > > > > > > > > > > > > > > > > Second, it's not just the architecture. kexec_load(2) will fail if > > > > > > SecureBoot is active. OTOH kexec_file_load(2) will fail if the > > > > > > kernel is not signed. For kernel hackers who don't use SecureBoot, > > > > > > signing self-built kernels is just overkill. So, you should also > > > > > > check the state of SecureBoot, possibly also whether the kernel > > > > > > image is signed with a valid key, repeating a bit too much of the > > > > > > kernel logic, and quite likely introducing subtle differences... > > > > > > > > > > Hmm, I did not say the exact details, yes, we checked the Secure Boot > > > > > state and only use kexec_file_load for that special case. > > > > > > > > > > kexec_file and kexec_file_load is not exactly same so if one want to > > > > > use one instead of another for a specific functionality it seems not > > > > > good to automatically switch to another if one failed. For example > > > > > which one should be the first choice, it is hard to say. > > > > > > > > Hard to say indeed, However, I would assume that architectures that > > > > implement kexec_file_load do so because it is required in some case and > > > > hence it will be actively maintained when available. However, some > > > > architectures may not require it and will be slow to implement it. So > > > > using kexec_file_load when available sounds like the right thing. > > > > Technically the implementation details are different but for most users > > > > this does not matter. > > > > > > > > For those that do care I provide option to select one or the other. > > > > > > I would say it breaks things, a better way should be introducing another > > > kexec-tools option for example kexec --auto for this purpose. Probably > > > add some --auto=... to select the first chance. > > > > Yes! This is the way to go. But then I wouldn't call it --auto. > > I would call it "--method=", so you could specify: > > > > --method=kernel (to use the in-kernel loader aka kexec_file_load) > > --method=user (to use the traditional user-space loader) > > --method=kernel,user (to prefer kexec_file_load) > > --method=user,kernel (to prefer kexc_load) > > > > I'm not quite sure if it also makes sense to provide "--method=auto", > > which would use whatever default is considered sane for the running > > system. > > If we have --method=kernel,user and --method=user,kernel then no need > "auto" anymore.. But I would prefer that original default behavior is kept, > kexec -l without "--method" will use userspace loader, kexec -s -l will > use kernel space loader If one use kexec -s --method=user then kexec-tools should fail, talked with Baoquan privately he has another idea to extend the kexec -s with --fallback: "kexec -s --fallback -l" can fallback to userspace loader this seems also a good option. > > > > > Petr T > > Thanks > Dave _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-27 9:14 ` Dave Young @ 2018-02-27 9:49 ` Michal Suchánek 0 siblings, 0 replies; 15+ messages in thread From: Michal Suchánek @ 2018-02-27 9:49 UTC (permalink / raw) To: Dave Young; +Cc: Tony Jones, horms, Petr Tesarik, kexec, Baoquan He On Tue, 27 Feb 2018 17:14:02 +0800 Dave Young <dyoung@redhat.com> wrote: > On 02/27/18 at 05:09pm, Dave Young wrote: > > On 02/27/18 at 09:39am, Petr Tesarik wrote: > > > On Tue, 27 Feb 2018 09:15:10 +0800 > > > Dave Young <dyoung@redhat.com> wrote: > > > > > > > On 02/26/18 at 01:08pm, Michal Suchánek wrote: > > > > > On Mon, 26 Feb 2018 09:45:15 +0800 > > > > > Dave Young <dyoung@redhat.com> wrote: > > > > > > > > > > > On 02/24/18 at 05:34pm, Petr Tesarik wrote: > > > > > > > On Sat, 24 Feb 2018 09:43:42 +0800 > > > > > > > Dave Young <dyoung@redhat.com> wrote: > > > > > > > > > > > > > > > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > > > > > > > > Hi Baoquan, > > > > > > > > > > > > > > > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > > > > > > > > Baoquan He <bhe@redhat.com> wrote: > > > > > > > > > > > > > > > > > > > Hi Michal, > > > > > > > > > > > > > > > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek > > > > > > > > > > wrote: > > > > > > > > > > > The new KEXEC_FILE_LOAD is preferred in the case > > > > > > > > > > > the platform supports it because it allows kexec > > > > > > > > > > > in locked down secure boot mode. > > > > > > > > > > > > > > > > > > > > > > However, some platforms do not support it so fall > > > > > > > > > > > back to the old syscall there. > > > > > > > > > > > > > > > > > > > > I didn't read code change, just from patch log, I > > > > > > > > > > tend to not agree. There are two options > > > > > > > > > > KEXEC_FILE_LOAD and KEXEC_LOAD, some platforms do > > > > > > > > > > not support, why does some platforms not choose > > > > > > > > > > KEXEC_LOAD, the working one? Why bother to make > > > > > > > > > > change in code? I believe there's returned message > > > > > > > > > > telling if KEXEC_FILE_LOAD works or not. > > > > > > > > > > > > > > > > > > Well... let me give a bit of background. As you have > > > > > > > > > probably noticed, this syscall was originally > > > > > > > > > available only for x86_64, but more and more > > > > > > > > > architectures are also adding it now. > > > > > > > > > > > > > > > > > > Next, kexec is actually called by a script (which > > > > > > > > > locates a suitable kernel and initrd, constructs the > > > > > > > > > kernel command line, etc.). The script must either: > > > > > > > > > > > > > > > > > > A. know somehow if the currently running kernel > > > > > > > > > implements kexec_file_load(2), or > > > > > > > > > > > > > > > > > > B. try one method first, and if it fails, retry > > > > > > > > > with the other. > > > > > > > > > > > > > > > > > > I agree that kexec(1) should probably allow the user > > > > > > > > > to force a specific method, but I don't see the > > > > > > > > > benefit of implementing fallback in an external > > > > > > > > > script and not in kexec-tools itself. > > > > > > > > > > > > > > > > > > OTOH if you want to push the fallback logic out of > > > > > > > > > kexec-tools, then I would like to get better > > > > > > > > > diagnostic at least. Letting my script parse kexec > > > > > > > > > output is, um, suboptimal. > > > > > > > > > > > > > > > > In Fedora/RHEL we use this in scripts by checking the > > > > > > > > arch first, for distribution it is enough? > > > > > > > > > > > > > > No. > > > > > > > > > > > > > > First, you would also have to check the kernel version > > > > > > > (and maintain an ugly mapping of which kernel version > > > > > > > introduced kexec_file_load on which architecture). > > > > > > > > > > > > The kernel version update is rare for these new syscall > > > > > > added, but it is indeed needed to match with them > > > > > > > > > > > > > > > > > > > > Second, it's not just the architecture. kexec_load(2) > > > > > > > will fail if SecureBoot is active. OTOH > > > > > > > kexec_file_load(2) will fail if the kernel is not signed. > > > > > > > For kernel hackers who don't use SecureBoot, signing > > > > > > > self-built kernels is just overkill. So, you should also > > > > > > > check the state of SecureBoot, possibly also whether the > > > > > > > kernel image is signed with a valid key, repeating a bit > > > > > > > too much of the kernel logic, and quite likely > > > > > > > introducing subtle differences... > > > > > > > > > > > > Hmm, I did not say the exact details, yes, we checked the > > > > > > Secure Boot state and only use kexec_file_load for that > > > > > > special case. > > > > > > > > > > > > kexec_file and kexec_file_load is not exactly same so if > > > > > > one want to use one instead of another for a specific > > > > > > functionality it seems not good to automatically switch to > > > > > > another if one failed. For example which one should be the > > > > > > first choice, it is hard to say. > > > > > > > > > > Hard to say indeed, However, I would assume that > > > > > architectures that implement kexec_file_load do so because it > > > > > is required in some case and hence it will be actively > > > > > maintained when available. However, some architectures may > > > > > not require it and will be slow to implement it. So using > > > > > kexec_file_load when available sounds like the right thing. > > > > > Technically the implementation details are different but for > > > > > most users this does not matter. > > > > > > > > > > For those that do care I provide option to select one or the > > > > > other. > > > > > > > > I would say it breaks things, a better way should be > > > > introducing another kexec-tools option for example kexec --auto > > > > for this purpose. Probably add some --auto=... to select the > > > > first chance. > > > > > > Yes! This is the way to go. But then I wouldn't call it --auto. > > > I would call it "--method=", so you could specify: > > > > > > --method=kernel (to use the in-kernel loader aka > > > kexec_file_load) --method=user (to use the traditional > > > user-space loader) --method=kernel,user (to prefer > > > kexec_file_load) --method=user,kernel (to prefer kexc_load) > > > > > > I'm not quite sure if it also makes sense to provide > > > "--method=auto", which would use whatever default is considered > > > sane for the running system. > > > > If we have --method=kernel,user and --method=user,kernel then no > > need "auto" anymore.. But I would prefer that original default > > behavior is kept, kexec -l without "--method" will use userspace > > loader, kexec -s -l will use kernel space loader > > If one use kexec -s --method=user then kexec-tools should fail, > talked with Baoquan privately he has another idea to extend the kexec > -s with --fallback: "kexec -s --fallback -l" can fallback to > userspace loader this seems also a good option. In the updated series it is easy to turn on or off the fallback so an option can be added for that. However, the fallback only works by trying -s first. Implementing fallback in either direction would complicate things a bit. Especially since error codes of kexec-tools do not tend to be nice and consistent. Thanks Michal _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-24 1:43 ` Dave Young 2018-02-24 16:34 ` Petr Tesarik @ 2018-02-24 20:02 ` Michal Suchánek 1 sibling, 0 replies; 15+ messages in thread From: Michal Suchánek @ 2018-02-24 20:02 UTC (permalink / raw) To: Dave Young; +Cc: Tony Jones, horms, Petr Tesarik, kexec, Baoquan He Hello, On Sat, 24 Feb 2018 10:19:09 +0800 Baoquan He <bhe@redhat.com> wrote: > Hi Petr, > > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > Hi Baoquan, > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > Baoquan He <bhe@redhat.com> wrote: > > > > > Hi Michal, > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > > The new KEXEC_FILE_LOAD is preferred in the case the platform > > > > supports it because it allows kexec in locked down secure boot > > > > mode. > > > > > > > > However, some platforms do not support it so fall back to the > > > > old syscall there. > > > > > > I didn't read code change, just from patch log, I tend to not > > > agree. There are two options KEXEC_FILE_LOAD and KEXEC_LOAD, some > > > platforms do not support, why does some platforms not choose > > > KEXEC_LOAD, the working one? Why bother to make change in code? I > > > believe there's returned message telling if KEXEC_FILE_LOAD works > > > or not. > > > > Well... let me give a bit of background. As you have probably > > noticed, this syscall was originally available only for x86_64, but > > more and more architectures are also adding it now. > > > > Next, kexec is actually called by a script (which locates a suitable > > kernel and initrd, constructs the kernel command line, etc.). The > > script must either: > > > > A. know somehow if the currently running kernel implements > > kexec_file_load(2), or > > > > B. try one method first, and if it fails, retry with the other. > > > > I agree that kexec(1) should probably allow the user to force a > > specific method, but I don't see the benefit of implementing > > fallback in an external script and not in kexec-tools itself. > > > > OTOH if you want to push the fallback logic out of kexec-tools, > > then I would like to get better diagnostic at least. Letting my > > script parse kexec output is, um, suboptimal. > > Thanks for the details! > > Firstly, this patch is very ugly. It mixs the falling back issue, doc > adding, code cleanup in one patch. It's not easy to see how many lines > of code change involved. The -s option is special-cased and I needed to un-special-case it to be able to use either syscall. I suppose this can be split. > > Secondly, I personally like better the A or B two options. Which the patch does provide. If you want A you can select it. if you want B you can select it as well. If you do not select either kexec tries A and if not supported tries B. > For A, > checking ARCH in script might be a easier thing. What does checking ARCH in a script tell you? You need to check that the syscall is supported both in kexec and the running kernel. And only kexec can do that. > And for B, just check > if "syscall kexec_file_load not available" is printed, then retry the > KEXEC_LOAD. Oh right, that's totally robust programming. If kexec does not know how to call KEXEC_FILE_LOAD then the message is printed to stderr and -1 is returned whereas when kexec thinks it knows how to call KEXEC_FILE_LOAD and the kernel does not support it nothing is printed and the return code from kernel is passed through which is hopefully -ENOSYS. Can we at least make the return code consistent? > > And the falling back may give people a feeling that that ARCH support > the file mode loading. Why? How? Previously they had to select KEXEC_FILE_LOAD with an undocumented option. It is now documented and when used the fallback is disabled - no change. > Besides, if fall back to KEXEC_LOAD when > KEXEC_FILE_LOAD is tried but not supported this time, next time people > may want to do fall back when KEXEC_FILE_LOAD is tried but failed, > since there has been a precedent. This might be not good for keeping > code logic simple, add complexity to maintaining. What complexity are you talking about? That is exactly what the fallback does except it checks that the error code was ENOSYS. It might make sense to also use the fallback when the file format is not recognized by the kernel. KEXEC_FILE_LOAD currently does not support multiboot and uImage formats. So if a specific error code is returned in this case (as opposed to known image type which is invalid) it might make sense to use the fallback as well. > > My personal opinion. > > Thanks > Baoquan > On Sat, 24 Feb 2018 09:43:42 +0800 Dave Young <dyoung@redhat.com> wrote: > On 02/23/18 at 09:29am, Petr Tesarik wrote: > > Hi Baoquan, > > > > On Fri, 23 Feb 2018 07:20:43 +0800 > > Baoquan He <bhe@redhat.com> wrote: > > > > > Hi Michal, > > > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > > The new KEXEC_FILE_LOAD is preferred in the case the platform > > > > supports it because it allows kexec in locked down secure boot > > > > mode. > > > > > > > > However, some platforms do not support it so fall back to the > > > > old syscall there. > > > > > > I didn't read code change, just from patch log, I tend to not > > > agree. There are two options KEXEC_FILE_LOAD and KEXEC_LOAD, some > > > platforms do not support, why does some platforms not choose > > > KEXEC_LOAD, the working one? Why bother to make change in code? I > > > believe there's returned message telling if KEXEC_FILE_LOAD works > > > or not. > > > > Well... let me give a bit of background. As you have probably > > noticed, this syscall was originally available only for x86_64, but > > more and more architectures are also adding it now. > > > > Next, kexec is actually called by a script (which locates a suitable > > kernel and initrd, constructs the kernel command line, etc.). The > > script must either: > > > > A. know somehow if the currently running kernel implements > > kexec_file_load(2), or > > > > B. try one method first, and if it fails, retry with the other. > > > > I agree that kexec(1) should probably allow the user to force a > > specific method, but I don't see the benefit of implementing > > fallback in an external script and not in kexec-tools itself. > > > > OTOH if you want to push the fallback logic out of kexec-tools, > > then I would like to get better diagnostic at least. Letting my > > script parse kexec output is, um, suboptimal. > > In Fedora/RHEL we use this in scripts by checking the arch first, > for distribution it is enough? No. That means you need to update the script when you implement KEXEC_FILE_LOAD on a new arch. Worse, you need to check the kernel version so it works on old kernel as well. > There are also some other arch > dependent options in kexec-tools, there is no way to just use same > for every different platform without checking in scripts. Like? And here I mean options that are specific to the platform and not a particular system. Because system-specific options have to be set case by case, obviously. I see no kexec option that would generally apply on one arch and not other except -s. Thanks Michal _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-23 8:29 ` Petr Tesarik 2018-02-24 1:43 ` Dave Young @ 2018-02-24 2:19 ` Baoquan He 1 sibling, 0 replies; 15+ messages in thread From: Baoquan He @ 2018-02-24 2:19 UTC (permalink / raw) To: Petr Tesarik; +Cc: Tony Jones, Michal Suchanek, kexec, horms Hi Petr, On 02/23/18 at 09:29am, Petr Tesarik wrote: > Hi Baoquan, > > On Fri, 23 Feb 2018 07:20:43 +0800 > Baoquan He <bhe@redhat.com> wrote: > > > Hi Michal, > > > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > > The new KEXEC_FILE_LOAD is preferred in the case the platform supports > > > it because it allows kexec in locked down secure boot mode. > > > > > > However, some platforms do not support it so fall back to the old > > > syscall there. > > > > I didn't read code change, just from patch log, I tend to not agree. There > > are two options KEXEC_FILE_LOAD and KEXEC_LOAD, some platforms do not > > support, why does some platforms not choose KEXEC_LOAD, the working one? > > Why bother to make change in code? I believe there's returned message > > telling if KEXEC_FILE_LOAD works or not. > > Well... let me give a bit of background. As you have probably noticed, > this syscall was originally available only for x86_64, but more and > more architectures are also adding it now. > > Next, kexec is actually called by a script (which locates a suitable > kernel and initrd, constructs the kernel command line, etc.). The > script must either: > > A. know somehow if the currently running kernel implements > kexec_file_load(2), or > > B. try one method first, and if it fails, retry with the other. > > I agree that kexec(1) should probably allow the user to force a > specific method, but I don't see the benefit of implementing fallback > in an external script and not in kexec-tools itself. > > OTOH if you want to push the fallback logic out of kexec-tools, then I > would like to get better diagnostic at least. Letting my script parse > kexec output is, um, suboptimal. Thanks for the details! Firstly, this patch is very ugly. It mixs the falling back issue, doc adding, code cleanup in one patch. It's not easy to see how many lines of code change involved. Secondly, I personally like better the A or B two options. For A, checking ARCH in script might be a easier thing. And for B, just check if "syscall kexec_file_load not available" is printed, then retry the KEXEC_LOAD. And the falling back may give people a feeling that that ARCH support the file mode loading. Besides, if fall back to KEXEC_LOAD when KEXEC_FILE_LOAD is tried but not supported this time, next time people may want to do fall back when KEXEC_FILE_LOAD is tried but failed, since there has been a precedent. This might be not good for keeping code logic simple, add complexity to maintaining. My personal opinion. Thanks Baoquan > > > Also provide an option to call the old syscall in case the new syscall > > > fails with other reason than ENOSYS. > > > > > > Also document the options. > > > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de> > > > --- > > > kexec/kexec.8 | 9 +++++++++ > > > kexec/kexec.c | 41 +++++++++++++++-------------------------- > > > kexec/kexec.h | 2 ++ > > > 3 files changed, 26 insertions(+), 26 deletions(-) > > > > > > diff --git a/kexec/kexec.8 b/kexec/kexec.8 > > > index e0131b4ea827..7e4df723251d 100644 > > > --- a/kexec/kexec.8 > > > +++ b/kexec/kexec.8 > > > @@ -144,6 +144,15 @@ Load the new kernel for use on panic. > > > Specify that the new kernel is of this > > > .I type. > > > .TP > > > +.BI \-s\ (\-\-kexec-file-syscall) > > > +Specify that the new KEXEC_FILE_LOAD syscall should be used exclusively. > > > +Otherwise KEXEC_FILE_LOAD is tried and when not supported KEXEC_LOAD is used. > > > +.I type. > > > +.TP > > > +.BI \-c\ (\-\-kexec-syscall) > > > +Specify that the old KEXEC_LOAD syscall should be used exclusively. > > > +.I type. > > > +.TP > > > .B \-u\ (\-\-unload) > > > Unload the current > > > .B kexec > > > diff --git a/kexec/kexec.c b/kexec/kexec.c > > > index cfd837c1b6bb..25328c02b508 100644 > > > --- a/kexec/kexec.c > > > +++ b/kexec/kexec.c > > > @@ -1166,7 +1166,7 @@ static int do_kexec_file_load(int fileind, int argc, char **argv, > > > > > > if (!is_kexec_file_load_implemented()) { > > > fprintf(stderr, "syscall kexec_file_load not available.\n"); > > > - return -1; > > > + return -ENOSYS; > > > } > > > > > > if (argc - fileind <= 0) { > > > @@ -1243,6 +1243,7 @@ int main(int argc, char *argv[]) > > > int do_unload = 0; > > > int do_reuse_initrd = 0; > > > int do_kexec_file_syscall = 0; > > > + int do_kexec_syscall = 0; > > > int do_status = 0; > > > void *entry = 0; > > > char *type = 0; > > > @@ -1256,19 +1257,6 @@ int main(int argc, char *argv[]) > > > }; > > > static const char short_options[] = KEXEC_ALL_OPT_STR; > > > > > > - /* > > > - * First check if --use-kexec-file-syscall is set. That changes lot of > > > - * things > > > - */ > > > - while ((opt = getopt_long(argc, argv, short_options, > > > - options, 0)) != -1) { > > > - switch(opt) { > > > - case OPT_KEXEC_FILE_SYSCALL: > > > - do_kexec_file_syscall = 1; > > > - break; > > > - } > > > - } > > > - > > > /* Reset getopt for the next pass. */ > > > opterr = 1; > > > optind = 1; > > > @@ -1310,8 +1298,7 @@ int main(int argc, char *argv[]) > > > do_shutdown = 0; > > > do_sync = 0; > > > do_unload = 1; > > > - if (do_kexec_file_syscall) > > > - kexec_file_flags |= KEXEC_FILE_UNLOAD; > > > + kexec_file_flags |= KEXEC_FILE_UNLOAD; > > > break; > > > case OPT_EXEC: > > > do_load = 0; > > > @@ -1354,11 +1341,8 @@ int main(int argc, char *argv[]) > > > do_exec = 0; > > > do_shutdown = 0; > > > do_sync = 0; > > > - if (do_kexec_file_syscall) > > > - kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > > - else > > > - kexec_flags = KEXEC_ON_CRASH; > > > - break; > > > + kexec_file_flags |= KEXEC_FILE_ON_CRASH; > > > + kexec_flags = KEXEC_ON_CRASH; > > > case OPT_MEM_MIN: > > > mem_min = strtoul(optarg, &endptr, 0); > > > if (*endptr) { > > > @@ -1383,7 +1367,12 @@ int main(int argc, char *argv[]) > > > do_reuse_initrd = 1; > > > break; > > > case OPT_KEXEC_FILE_SYSCALL: > > > - /* We already parsed it. Nothing to do. */ > > > + do_kexec_file_syscall = 1; > > > + do_kexec_syscall = 0; > > > + break; > > > + case OPT_KEXEC_SYSCALL: > > > + do_kexec_file_syscall = 0; > > > + do_kexec_syscall = 1; > > > break; > > > case OPT_STATUS: > > > do_status = 1; > > > @@ -1456,16 +1445,16 @@ int main(int argc, char *argv[]) > > > result = k_status(kexec_flags); > > > } > > > if (do_unload) { > > > - if (do_kexec_file_syscall) > > > + if (!do_kexec_syscall) > > > result = kexec_file_unload(kexec_file_flags); > > > - else > > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > > result = k_unload(kexec_flags); > > > } > > > if (do_load && (result == 0)) { > > > - if (do_kexec_file_syscall) > > > + if (!do_kexec_syscall) > > > result = do_kexec_file_load(fileind, argc, argv, > > > kexec_file_flags); > > > - else > > > + if ((result == -ENOSYS) || !do_kexec_file_syscall) > > > result = my_load(type, fileind, argc, argv, > > > kexec_flags, entry); > > > } > > > diff --git a/kexec/kexec.h b/kexec/kexec.h > > > index 26225d2c002a..7abcec796cae 100644 > > > --- a/kexec/kexec.h > > > +++ b/kexec/kexec.h > > > @@ -219,6 +219,7 @@ extern int file_types; > > > #define OPT_TYPE 't' > > > #define OPT_PANIC 'p' > > > #define OPT_KEXEC_FILE_SYSCALL 's' > > > +#define OPT_KEXEC_SYSCALL 'c' > > > #define OPT_STATUS 'S' > > > #define OPT_MEM_MIN 256 > > > #define OPT_MEM_MAX 257 > > > @@ -246,6 +247,7 @@ extern int file_types; > > > { "mem-max", 1, 0, OPT_MEM_MAX }, \ > > > { "reuseinitrd", 0, 0, OPT_REUSE_INITRD }, \ > > > { "kexec-file-syscall", 0, 0, OPT_KEXEC_FILE_SYSCALL }, \ > > > + { "kexec-syscall", 0, 0, OPT_KEXEC_SYSCALL }, \ > > > { "debug", 0, 0, OPT_DEBUG }, \ > > > { "status", 0, 0, OPT_STATUS }, \ > > > { "print-ckr-size", 0, 0, OPT_PRINT_CKR_SIZE }, \ > > > -- > > > 2.13.6 > > > > > > > > > _______________________________________________ > > > kexec mailing list > > > kexec@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/kexec > > > > _______________________________________________ > > kexec mailing list > > kexec@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/kexec > > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported. 2018-02-22 23:20 ` Baoquan He 2018-02-23 8:29 ` Petr Tesarik @ 2018-02-23 19:21 ` Michal Suchánek 1 sibling, 0 replies; 15+ messages in thread From: Michal Suchánek @ 2018-02-23 19:21 UTC (permalink / raw) To: Baoquan He; +Cc: Tony Jones, horms, kexec, Petr Tesarik On Fri, 23 Feb 2018 07:20:43 +0800 Baoquan He <bhe@redhat.com> wrote: > Hi Michal, > > On 02/22/18 at 11:24pm, Michal Suchanek wrote: > > The new KEXEC_FILE_LOAD is preferred in the case the platform > > supports it because it allows kexec in locked down secure boot mode. > > > > However, some platforms do not support it so fall back to the old > > syscall there. > > I didn't read code change, just from patch log, I tend to not agree. > There are two options KEXEC_FILE_LOAD and KEXEC_LOAD, some platforms > do not support, why does some platforms not choose KEXEC_LOAD, the > working one? Because nobody wrote the support. If you volunteer to write support for KEXEC_FILE_LOAD for every platform Linux supports and add the respective syscall numbers to kexec so it knows how to execute the syscall on every platform I will consider it alternative fix. Some people will argue that not everyone applies the patches to support KEXEC_FILE_LOAD in the kernel overnight, though. > Why bother to make change in code? Because it is unusable as is. Just calling kexec fails with locked-down secure boot. Calling kexec -s fails on almost every platform except x86. > I believe there's > returned message telling if KEXEC_FILE_LOAD works or not. That is not a solution. A way to call kexec that actually works is needed. This patch removes the need to use the undocumented -s option to get the new superior syscall you seem to prefer. It will just do the right thing in most cases. It allows the user to select either syscall explicitly as well. I do not see the problem with that. Thanks Michal _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-02-27 9:50 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-22 22:24 [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported Michal Suchanek 2018-02-22 23:20 ` Baoquan He 2018-02-23 8:29 ` Petr Tesarik 2018-02-24 1:43 ` Dave Young 2018-02-24 16:34 ` Petr Tesarik 2018-02-26 1:45 ` Dave Young 2018-02-26 12:08 ` Michal Suchánek 2018-02-27 1:15 ` Dave Young 2018-02-27 8:39 ` Petr Tesarik 2018-02-27 9:09 ` Dave Young 2018-02-27 9:14 ` Dave Young 2018-02-27 9:49 ` Michal Suchánek 2018-02-24 20:02 ` Michal Suchánek 2018-02-24 2:19 ` Baoquan He 2018-02-23 19:21 ` Michal Suchánek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.