[PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Michal Suchanek]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Baoquan He]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Petr Tesarik]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Michal Suchánek]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Dave Young]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Baoquan He]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Petr Tesarik]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Michal Suchánek]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Dave Young]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Michal Suchánek]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Dave Young]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Petr Tesarik]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Dave Young]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Dave Young]

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

Re: [PATCH] kexec: do KEXEC_FILE_LOAD and fallback to KEXEC_LOAD if not supported.

[Michal Suchánek]

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