Re: [PATCH] kexec/ppc64: leverage kexec_file_load support

[Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>]

Hello Hari,

Hari Bathini <hbathini@linux.vnet.ibm.com> writes:

> PPC64 kernel now supports kexec_file_load system call. Leverage it by
> enabling that support here. Note that loading crash kernel with this
> system call is not yet supported in the kernel and trying to load one
> will fail with '-ENOTSUPP' error.
>
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> ---
>  kexec/arch/ppc64/kexec-elf-ppc64.c |   84 ++++++++++++++++++++++++++++++++++++
>  kexec/kexec-syscall.h              |    3 +
>  2 files changed, 87 insertions(+)

Thanks for implementing this! Looks good to me, just one nit below.
Regardless of that:

Reviewed-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>

> diff --git a/kexec/arch/ppc64/kexec-elf-ppc64.c b/kexec/arch/ppc64/kexec-elf-ppc64.c
> index ddd3de8..2742cd6 100644
> --- a/kexec/arch/ppc64/kexec-elf-ppc64.c
> +++ b/kexec/arch/ppc64/kexec-elf-ppc64.c

<snip>

> @@ -117,6 +196,9 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,
>  	uint32_t my_run_at_load;
>  	unsigned int slave_code[256/sizeof (unsigned int)], master_entry;
>
> +	if (info->file_mode)
> +		return elf_ppc64_load_file(argc, argv, info);
> +
>  	/* See options.h -- add any more there, too. */
>  	static const struct option options[] = {
>  		KEXEC_ARCH_OPTIONS

This is placing executable code between variable declarations. It may be
fine for gcc but it's more idiomatic C to put it after all variable
declarations. But perhaps the kexec-tools style is fine with it?

--
Thiago Jung Bauermann
IBM Linux Technology Center


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec