Re: [PATCH v2 6/6] crash: option to let arch decide mem range is usable

Hari Bathini
2025-01-24T09:53:11Z

Hi Sourabh,

On 21/01/25 5:24 pm, Sourabh Jain wrote:
> On PowerPC, the memory reserved for the crashkernel can contain
> components like RTAS, TCE, OPAL, etc., which should be avoided when
> loading kexec segments into crashkernel memory. Due to these special
> components, PowerPC has its own set of functions to locate holes in the
> crashkernel memory for loading kexec segments for kdump. However, for
> loading kexec segments in the kexec case, PowerPC uses generic functions
> to locate holes.
>
> So, let's use generic functions to locate memory holes for kdump on
> PowerPC by adding an arch hook to handle such special regions while
> loading kexec segments, and remove the PowerPC functions to locate
> holes.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Baoquan he <bhe@redhat.com>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: kexec@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
> arch/powerpc/include/asm/kexec.h | 6 +-
> arch/powerpc/kexec/file_load_64.c | 259 ++----------------------------
> include/linux/kexec.h | 9 ++
> kernel/kexec_file.c | 12 ++
> 4 files changed, 34 insertions(+), 252 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index 64741558071f..5e4680f9ff35 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -95,8 +95,10 @@ int arch_kexec_kernel_image_probe(struct kimage *image, void *buf, unsigned long
> int arch_kimage_file_post_load_cleanup(struct kimage *image);
> #define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
>
> -int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
> -#define arch_kexec_locate_mem_hole arch_kexec_locate_mem_hole
> +int arch_check_excluded_range(struct kimage *image, unsigned long start,
> + unsigned long end);
> +#define arch_check_excluded_range arch_check_excluded_range
> +
>
> int load_crashdump_segments_ppc64(struct kimage *image,
> struct kexec_buf *kbuf);
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index dc65c1391157..e7ef8b2a2554 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -49,201 +49,18 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
> NULL
> };
>
> -/**
> - * __locate_mem_hole_top_down - Looks top down for a large enough memory hole
> - * in the memory regions between buf_min & buf_max
> - * for the buffer. If found, sets kbuf->mem.
> - * @kbuf: Buffer contents and memory parameters.
> - * @buf_min: Minimum address for the buffer.
> - * @buf_max: Maximum address for the buffer.
> - *
> - * Returns 0 on success, negative errno on error.
> - */
> -static int __locate_mem_hole_top_down(struct kexec_buf *kbuf,
> - u64 buf_min, u64 buf_max)
> -{
> - int ret = -EADDRNOTAVAIL;
> - phys_addr_t start, end;
> - u64 i;
> -
> - for_each_mem_range_rev(i, &start, &end) {
> - /*
> - * memblock uses [start, end) convention while it is
> - * [start, end] here. Fix the off-by-one to have the
> - * same convention.
> - */
> - end -= 1;
> -
> - if (start > buf_max)
> - continue;
> -
> - /* Memory hole not found */
> - if (end < buf_min)
> - break;
> -
> - /* Adjust memory region based on the given range */
> - if (start < buf_min)
> - start = buf_min;
> - if (end > buf_max)
> - end = buf_max;
> -
> - start = ALIGN(start, kbuf->buf_align);
> - if (start < end && (end - start + 1) >= kbuf->memsz) {
> - /* Suitable memory range found. Set kbuf->mem */
> - kbuf->mem = ALIGN_DOWN(end - kbuf->memsz + 1,
> - kbuf->buf_align);
> - ret = 0;
> - break;
> - }
> - }
> -
> - return ret;
> -}
> -
> -/**
> - * locate_mem_hole_top_down_ppc64 - Skip special memory regions to find a
> - * suitable buffer with top down approach.
> - * @kbuf: Buffer contents and memory parameters.
> - * @buf_min: Minimum address for the buffer.
> - * @buf_max: Maximum address for the buffer.
> - * @emem: Exclude memory ranges.
> - *
> - * Returns 0 on success, negative errno on error.
> - */
> -static int locate_mem_hole_top_down_ppc64(struct kexec_buf *kbuf,
> - u64 buf_min, u64 buf_max,
> - const struct crash_mem *emem)
> +int arch_check_excluded_range(struct kimage *image, unsigned long start,
> + unsigned long end)
> {
> - int i, ret = 0, err = -EADDRNOTAVAIL;
> - u64 start, end, tmin, tmax;
> -
> - tmax = buf_max;
> - for (i = (emem->nr_ranges - 1); i >= 0; i--) {
> - start = emem->ranges[i].start;
> - end = emem->ranges[i].end;
> -
> - if (start > tmax)
> - continue;
> -
> - if (end < tmax) {
> - tmin = (end < buf_min ? buf_min : end + 1);
> - ret = __locate_mem_hole_top_down(kbuf, tmin, tmax);
> - if (!ret)
> - return 0;
> - }
> -
> - tmax = start - 1;
> -
> - if (tmax < buf_min) {
> - ret = err;
> - break;
> - }
> - ret = 0;
> - }
> -
> - if (!ret) {
> - tmin = buf_min;
> - ret = __locate_mem_hole_top_down(kbuf, tmin, tmax);
> - }
> - return ret;
> -}
> -
> -/**
> - * __locate_mem_hole_bottom_up - Looks bottom up for a large enough memory hole
> - * in the memory regions between buf_min & buf_max
> - * for the buffer. If found, sets kbuf->mem.
> - * @kbuf: Buffer contents and memory parameters.
> - * @buf_min: Minimum address for the buffer.
> - * @buf_max: Maximum address for the buffer.
> - *
> - * Returns 0 on success, negative errno on error.
> - */
> -static int __locate_mem_hole_bottom_up(struct kexec_buf *kbuf,
> - u64 buf_min, u64 buf_max)
> -{
> - int ret = -EADDRNOTAVAIL;
> - phys_addr_t start, end;
> - u64 i;
> -
> - for_each_mem_range(i, &start, &end) {
> - /*
> - * memblock uses [start, end) convention while it is
> - * [start, end] here. Fix the off-by-one to have the
> - * same convention.
> - */
> - end -= 1;
> -
> - if (end < buf_min)
> - continue;
> -
> - /* Memory hole not found */
> - if (start > buf_max)
> - break;
> -
> - /* Adjust memory region based on the given range */
> - if (start < buf_min)
> - start = buf_min;
> - if (end > buf_max)
> - end = buf_max;
> -
> - start = ALIGN(start, kbuf->buf_align);
> - if (start < end && (end - start + 1) >= kbuf->memsz) {
> - /* Suitable memory range found. Set kbuf->mem */
> - kbuf->mem = start;
> - ret = 0;
> - break;
> - }
> - }
> -
> - return ret;
> -}
> -
> -/**
> - * locate_mem_hole_bottom_up_ppc64 - Skip special memory regions to find a
> - * suitable buffer with bottom up approach.
> - * @kbuf: Buffer contents and memory parameters.
> - * @buf_min: Minimum address for the buffer.
> - * @buf_max: Maximum address for the buffer.
> - * @emem: Exclude memory ranges.
> - *
> - * Returns 0 on success, negative errno on error.
> - */
> -static int locate_mem_hole_bottom_up_ppc64(struct kexec_buf *kbuf,
> - u64 buf_min, u64 buf_max,
> - const struct crash_mem *emem)
> -{
> - int i, ret = 0, err = -EADDRNOTAVAIL;
> - u64 start, end, tmin, tmax;
> -
> - tmin = buf_min;
> - for (i = 0; i < emem->nr_ranges; i++) {
> - start = emem->ranges[i].start;
> - end = emem->ranges[i].end;
> -
> - if (end < tmin)
> - continue;
> -
> - if (start > tmin) {
> - tmax = (start > buf_max ? buf_max : start - 1);
> - ret = __locate_mem_hole_bottom_up(kbuf, tmin, tmax);
> - if (!ret)
> - return 0;
> - }
> -
> - tmin = end + 1;
> + struct crash_mem *emem;
> + int i;
>
> - if (tmin > buf_max) {
> - ret = err;
> - break;
> - }
> - ret = 0;
> - }
> + emem = image->arch.exclude_ranges;
> + for (i = 0; i < emem->nr_ranges; i++)
> + if (start < emem->ranges[i].end && end > emem->ranges[i].start)
> + return 1;
>
> - if (!ret) {
> - tmax = buf_max;
> - ret = __locate_mem_hole_bottom_up(kbuf, tmin, tmax);
> - }
> - return ret;
> + return 0;
> }
>
> #ifdef CONFIG_CRASH_DUMP
> @@ -1004,64 +821,6 @@ int setup_new_fdt_ppc64(const struct kimage *image, void *fdt, struct crash_mem
> return ret;
> }
>
> -/**
> - * arch_kexec_locate_mem_hole - Skip special memory regions like rtas, opal,
> - * tce-table, reserved-ranges & such (exclude
> - * memory ranges) as they can't be used for kexec
> - * segment buffer. Sets kbuf->mem when a suitable
> - * memory hole is found.
> - * @kbuf: Buffer contents and memory parameters.
> - *
> - * Assumes minimum of PAGE_SIZE alignment for kbuf->memsz & kbuf->buf_align.
> - *
> - * Returns 0 on success, negative errno on error.
> - */
> -int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf)
> -{
> - struct crash_mem **emem;
> - u64 buf_min, buf_max;
> - int ret;
> -
> - /* Look up the exclude ranges list while locating the memory hole */
> - emem = &(kbuf->image->arch.exclude_ranges);
> - if (!(*emem) || ((*emem)->nr_ranges == 0)) {
> - pr_warn("No exclude range list. Using the default locate mem hole method\n");
> - return kexec_locate_mem_hole(kbuf);
> - }
> -
> - buf_min = kbuf->buf_min;
> - buf_max = kbuf->buf_max;
> - /* Segments for kdump kernel should be within crashkernel region */
> - if (IS_ENABLED(CONFIG_CRASH_DUMP) && kbuf->image->type == KEXEC_TYPE_CRASH) {
> - buf_min = (buf_min < crashk_res.start ?
> - crashk_res.start : buf_min);
> - buf_max = (buf_max > crashk_res.end ?
> - crashk_res.end : buf_max);
> - }
> -
> - if (buf_min > buf_max) {
> - pr_err("Invalid buffer min and/or max values\n");
> - return -EINVAL;
> - }
> -
> - if (kbuf->top_down)
> - ret = locate_mem_hole_top_down_ppc64(kbuf, buf_min, buf_max,
> - *emem);
> - else
> - ret = locate_mem_hole_bottom_up_ppc64(kbuf, buf_min, buf_max,
> - *emem);
> -
> - /* Add the buffer allocated to the exclude list for the next lookup */
> - if (!ret) {
> - add_mem_range(emem, kbuf->mem, kbuf->memsz);
> - sort_memory_ranges(*emem, true);
> - } else {
> - pr_err("Failed to locate memory buffer of size %lu\n",
> - kbuf->memsz);
> - }
> - return ret;
> -}
> -
> /**
> * arch_kexec_kernel_image_probe - Does additional handling needed to setup
> * kexec segments.

> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index f0e9f8eda7a3..407f8b0346aa 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -205,6 +205,15 @@ static inline int arch_kimage_file_post_load_cleanup(struct kimage *image)
> }
> #endif
>
> +#ifndef arch_check_excluded_range
> +static inline int arch_check_excluded_range(struct kimage *image,
> + unsigned long start,
> + unsigned long end)
> +{
> + return 0;
> +}
> +#endif
> +
> #ifdef CONFIG_KEXEC_SIG
> #ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
> int kexec_kernel_verify_pe_sig(const char *kernel, unsigned long kernel_len);
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 3eedb8c226ad..fba686487e3b 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -464,6 +464,12 @@ static int locate_mem_hole_top_down(unsigned long start, unsigned long end,
> continue;
> }
>
> + /* Make sure this does not conflict with exclude range */
> + if (arch_check_excluded_range(image, temp_start, temp_end)) {
> + temp_start = temp_start - PAGE_SIZE;
> + continue;
> + }
> +
> /* We found a suitable memory range */
> break;
> } while (1);
> @@ -498,6 +504,12 @@ static int locate_mem_hole_bottom_up(unsigned long start, unsigned long end,
> continue;
> }
>
> + /* Make sure this does not conflict with exclude range */
> + if (arch_check_excluded_range(image, temp_start, temp_end)) {
> + temp_start = temp_start + PAGE_SIZE;
> + continue;
> + }
> +
> /* We found a suitable memory range */
> break;
> } while (1);

Please split this arch-independent patch and have it as a preceding
patch. Arch-specific changes can go in a separate patch.

- Hari