Re: [PATCH v2 2/3] arm64: kexec: allocate memory space avoiding reserved regions

[Masayoshi Mizuma <msys.mizuma@gmail.com>]

some nits as below:

On Fri, Jan 11, 2019 at 06:59:45PM +0900, AKASHI Takahiro wrote:
> On UEFI/ACPI-only system, some memory regions, including but not limited
> to UEFI memory map and ACPI tables, must be preserved across kexec'ing.
> Otherwise, they can be corrupted and result in early failure in booting
> a new kernel.
> 
> In recent kernels, /proc/iomem now has an extended file format like:
> 	40000000-5871ffff : System RAM
> 	  41800000-426affff : Kernel code
> 	  426b0000-42aaffff : reserved
> 	  42ab0000-42c64fff : Kernel data
> 	  54400000-583fffff : Crash kernel
> 	  58590000-585effff : reserved
> 	  58700000-5871ffff : reserved
> 	58720000-58b5ffff : reserved
> 	58b60000-5be3ffff : System RAM
> 	  58b61000-58b61fff : reserved
> 	  59a77000-59a77fff : reserved
> 	5be40000-5becffff : reserved
> 	5bed0000-5bedffff : System RAM
> 	5bee0000-5bffffff : reserved
> 	5c000000-5fffffff : System RAM
> 	  5da00000-5e9fffff : reserved
> 	  5ec00000-5edfffff : reserved
> 	  5ef6a000-5ef6afff : reserved
> 	  5ef6b000-5efcafff : reserved
> 	  5efcd000-5efcffff : reserved
> 	  5efd0000-5effffff : reserved
> 	  5f000000-5fffffff : reserved
> 
> where the "reserved" entries at the top level or under System RAM (and
> its descendant resources) are ones of such kind and should not be regarded
> as usable memory ranges where several free spaces for loading kexec data
> will be allocated.
> 
> With this patch, get_memory_ranges() will handle this format of file
> correctly. Note that, for safety, unknown regions, in addition to
> "reserved" ones, will also be excluded.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  kexec/arch/arm64/kexec-arm64.c | 146 ++++++++++++++++++++-------------
>  1 file changed, 87 insertions(+), 59 deletions(-)
> 
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 1cde75d1a771..2e923b54f5b1 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -10,7 +10,9 @@
>  #include <inttypes.h>
>  #include <libfdt.h>
>  #include <limits.h>
> +#include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
>  #include <sys/stat.h>
>  #include <linux/elf-em.h>
>  #include <elf.h>
> @@ -29,6 +31,7 @@
>  #include "fs2dt.h"
>  #include "iomem.h"
>  #include "kexec-syscall.h"
> +#include "mem_regions.h"
>  #include "arch/options.h"
>  
>  #define ROOT_NODE_ADDR_CELLS_DEFAULT 1
> @@ -899,19 +902,33 @@ int get_phys_base_from_pt_load(unsigned long *phys_offset)
>  	return 0;
>  }
>  
> +static bool to_be_excluded(char *str)
> +{
> +	if (!strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM)) ||
> +	    !strncmp(str, KERNEL_CODE, strlen(KERNEL_CODE)) ||
> +	    !strncmp(str, KERNEL_DATA, strlen(KERNEL_DATA)) ||
> +	    !strncmp(str, CRASH_KERNEL, strlen(CRASH_KERNEL)))
> +		return false;
> +	else
> +		return true;
> +}
> +
>  /**
> - * get_memory_ranges_iomem_cb - Helper for get_memory_ranges_iomem.
> + * get_memory_ranges - Try to get the memory ranges from
> + * /proc/iomem.
>   */
> -
> -static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
> -	unsigned long long base, unsigned long long length)
> +int get_memory_ranges(struct memory_range **range, int *ranges,
> +	unsigned long kexec_flags)
>  {
> -	int ret;
>  	unsigned long phys_offset = UINT64_MAX;
> -	struct memory_range *r;
> -
> -	if (nr >= KEXEC_SEGMENT_MAX)
> -		return -1;
> +	FILE *fp;
> +	const char *iomem = proc_iomem();
> +	char line[MAX_LINE], *str;
> +	unsigned long long start, end;
> +	int n, consumed;
> +	struct memory_ranges memranges;
> +	struct memory_range *last, excl_range;
> +	int ret;
>  
>  	if (!try_read_phys_offset_from_kcore) {
>  		/* Since kernel version 4.19, 'kcore' contains
> @@ -945,17 +962,65 @@ static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
>  		try_read_phys_offset_from_kcore = true;
>  	}
>  
> -	r = (struct memory_range *)data + nr;
> +	fp = fopen(iomem, "r");
> +	if (!fp)
> +		die("Cannot open %s\n", iomem);
> +
> +	memranges.ranges = NULL;
> +	memranges.size = memranges.max_size  = 0;
> +
> +	while (fgets(line, sizeof(line), fp) != 0) {
> +		n = sscanf(line, "%llx-%llx : %n", &start, &end, &consumed);
> +		if (n != 2)
> +			continue;
> +		str = line + consumed;
> +
> +		if (!strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM))) {
> +			ret = mem_regions_alloc_and_add(&memranges,
> +					start, end - start + 1, RANGE_RAM);
> +			if (ret) {
> +				fprintf(stderr,
> +					"Cannot allocate memory for ranges\n");

				fclose(fp);

> +				return -ENOMEM;
> +			}
>  
> -	if (!strncmp(str, SYSTEM_RAM, strlen(SYSTEM_RAM)))
> -		r->type = RANGE_RAM;
> -	else if (!strncmp(str, IOMEM_RESERVED, strlen(IOMEM_RESERVED)))
> -		r->type = RANGE_RESERVED;
> -	else
> -		return 1;
> +			dbgprintf("%s:+[%d] %016llx - %016llx\n", __func__,
> +				memranges.size - 1,
> +				memranges.ranges[memranges.size - 1].start,
> +				memranges.ranges[memranges.size - 1].end);
> +		} else if (to_be_excluded(str)) {
> +			if (!memranges.size)
> +				continue;
> +
> +			/*
> +			 * Note: mem_regions_exclude() doesn't guarantee
> +			 * that the ranges are sorted out, but as long as
> +			 * we cope with /proc/iomem, we only operate on
> +			 * the last entry and so it is safe.
> +			 */
>  
> -	r->start = base;
> -	r->end = base + length - 1;
> +			/* The last System RAM range */
> +			last = &memranges.ranges[memranges.size - 1];
> +
> +			if (last->end < start)
> +				/* New resource outside of System RAM */
> +				continue;
> +			if (end < last->start)
> +				/* Already excluded by parent resource */
> +				continue;
> +
> +			excl_range.start = start;
> +			excl_range.end = end;

> +			mem_regions_alloc_and_exclude(&memranges, &excl_range);
			ret = mem_regions_alloc_and_exclude(&memranges, &excl_range);
			if (ret) {
				fprintf(stderr,
					"Cannot allocate memory for ranges (exclude)\n");
				fclose(fp);
				return -ENOMEM;
			}

Thanks,
Masa

> +			dbgprintf("%s:-      %016llx - %016llx\n",
> +					__func__, start, end);
> +		}
> +	}
> +
> +	fclose(fp);
> +
> +	*range = memranges.ranges;
> +	*ranges = memranges.size;
>  
>  	/* As a fallback option, we can try determining the PHYS_OFFSET
>  	 * value from the '/proc/iomem' entries as well.
> @@ -976,52 +1041,15 @@ static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
>  	 * between the user-space and kernel space 'PHYS_OFFSET'
>  	 * value.
>  	 */
> -	set_phys_offset(r->start, "iomem");
> +	if (memranges.size)
> +		set_phys_offset(memranges.ranges[0].start, "iomem");
>  
> -	dbgprintf("%s: %016llx - %016llx : %s", __func__, r->start,
> -		r->end, str);
> +	dbgprint_mem_range("System RAM ranges;",
> +				memranges.ranges, memranges.size);
>  
>  	return 0;
>  }
>  
> -/**
> - * get_memory_ranges_iomem - Try to get the memory ranges from
> - * /proc/iomem.
> - */
> -
> -static int get_memory_ranges_iomem(struct memory_range *array,
> -	unsigned int *count)
> -{
> -	*count = kexec_iomem_for_each_line(NULL,
> -		get_memory_ranges_iomem_cb, array);
> -
> -	if (!*count) {
> -		dbgprintf("%s: failed: No RAM found.\n", __func__);
> -		return EFAILED;
> -	}
> -
> -	return 0;
> -}
> -
> -/**
> - * get_memory_ranges - Try to get the memory ranges some how.
> - */
> -
> -int get_memory_ranges(struct memory_range **range, int *ranges,
> -	unsigned long kexec_flags)
> -{
> -	static struct memory_range array[KEXEC_SEGMENT_MAX];
> -	unsigned int count;
> -	int result;
> -
> -	result = get_memory_ranges_iomem(array, &count);
> -
> -	*range = result ? NULL : array;
> -	*ranges = result ? 0 : count;
> -
> -	return result;
> -}
> -
>  int arch_compat_trampoline(struct kexec_info *info)
>  {
>  	return 0;
> -- 
> 2.19.1
> 
> 

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