On 15/01/2023 16:02:01, Sourabh Jain wrote:
> Add a new member "fdt_index" to kimage_arch struct to hold the index of
> the FDT (Flattened Device Tree) segment in the kexec segment array.
>
> Having direct access to FDT segment will help arch crash hotplug handler
> to avoid looping kexec segment array to identify the FDT segment index
> for every FDT update on hotplug events.
>
> The fdt_index is initialized during the kexec load for both kexec_load and
> kexec_file_load system call.
>
> Signed-off-by: Sourabh Jain <sourabhj...@linux.ibm.com>
> ---
> arch/powerpc/include/asm/kexec.h | 7 +++++++
> arch/powerpc/kexec/core_64.c | 27 +++++++++++++++++++++++++++
> arch/powerpc/kexec/elf_64.c | 6 ++++++
> arch/powerpc/kexec/file_load_64.c | 5 +++++
> 4 files changed, 45 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/kexec.h
> b/arch/powerpc/include/asm/kexec.h
> index 8090ad7d97d9d..5a322c1737661 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -103,6 +103,10 @@ void kexec_copy_flush(struct kimage *image);
> struct crash_mem;
> int update_cpus_node(void *fdt);
> int get_crash_memory_ranges(struct crash_mem **mem_ranges);
> +#if defined(CONFIG_CRASH_HOTPLUG)
> +int machine_kexec_post_load(struct kimage *image);
> +#define machine_kexec_post_load machine_kexec_post_load
> +#endif
> #endif
>
> #if defined(CONFIG_CRASH_DUMP) && defined(CONFIG_PPC_RTAS)
> @@ -118,6 +122,9 @@ extern const struct kexec_file_ops kexec_elf64_ops;
> struct kimage_arch {
> struct crash_mem *exclude_ranges;
>
> +#if defined(CONFIG_CRASH_HOTPLUG)
> + int fdt_index;
> +#endif
> unsigned long backup_start;
> void *backup_buf;
> void *fdt;
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index 0b292f93a74cc..3d4fe1aa6f761 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -77,6 +77,33 @@ int machine_kexec_prepare(struct kimage *image)
> return 0;
> }
>
> +#if defined(CONFIG_CRASH_HOTPLUG)
I think you should add a small function header describing that this
function is recording the index of the FDT segment for later use.
> +int machine_kexec_post_load(struct kimage *kimage)
> +{
> + int i;
> + void *ptr;
> + unsigned long mem;
> +
> + /* Mark fdt_index invalid */
> + kimage->arch.fdt_index = -1;
Is that really needed?
This is already done in arch_kexec_kernel_image_probe() called before this
function, isn't it?
> +
> + if (kimage->type != KEXEC_TYPE_CRASH)
> + return 0;
> +
> + for (i = 0; i < kimage->nr_segments; i++) {
> + mem = kimage->segment[i].mem;
> + ptr = __va(mem);
> +
> + if (ptr && fdt_magic(ptr) == FDT_MAGIC) {
> + kimage->arch.fdt_index = i;
> + break;
> + }
> + }
> +
> + return 0;
> +}
> +#endif
> +
> /* Called during kexec sequence with MMU off */
> static notrace void copy_segments(unsigned long ind)
> {
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index eeb258002d1e0..2a17f171661f1 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -123,6 +123,12 @@ static void *elf64_load(struct kimage *image, char
> *kernel_buf,
> kbuf.buf_align = PAGE_SIZE;
> kbuf.top_down = true;
> kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> +
> +#if defined(CONFIG_CRASH_HOTPLUG)
> + image->arch.fdt_index = image->nr_segments;
I'm sorry, I'm not familliar with that code, could you explain why
fdt_index has to be assigned here, and to that value?
> +#endif
> + kbuf.memsz = fdt_totalsize(fdt);
> +
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> goto out_free_fdt;
> diff --git a/arch/powerpc/kexec/file_load_64.c
> b/arch/powerpc/kexec/file_load_64.c
> index 9bc70b4d8eafc..725f74d1b928c 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -1153,6 +1153,11 @@ int arch_kexec_kernel_image_probe(struct kimage
> *image, void *buf,
> return ret;
> }
>
> +#if defined(CONFIG_CRASH_HOTPLUG)
> + /* Mark fdt_index invalid */
> + image->arch.fdt_index = -1;
> +#endif
> +
> return kexec_image_probe_default(image, buf, buf_len);
> }
>
