On Mon, Dec 16, 2019 at 11:22:56AM +0530, Bhupesh Sharma wrote:
> Thanks Masa,
>
> On Sat, Dec 14, 2019 at 1:34 AM Masayoshi Mizuma <msys.miz...@gmail.com>
> 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.
Thank you for your follow up and I'm sorry I didn't explain it.
mem_regions_alloc_and_exclude() may fail in case realloc() or
mem_region_exclude() fail, so it would be better to add the error
handling.
>
> 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.
Sounds great!
- Masa
>
> @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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
