Thanks Masa,
On Sat, Dec 14, 2019 at 1:34 AM Masayoshi Mizuma <msys.mizuma@xxxxxxxxx> wrote:
>
> 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;
> }
Since this is an old thread, it would be useful for people looking at
the same, if you can add some comments/details about why you think
this nit is needed.
Also if Akashi agrees with the same, it would be better if he could
send a rebased version of the patchset (with your comments addressed),
so that the same can be picked for upstream kexec-tools cleanly.
@Akashi- Hi Akashi, Please let us know your views.
Thanks,
Bhupesh
> + 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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
