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

[Hari Bathini <hbathini@linux.ibm.com>]

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