On Tue, Sep 27, 2016 at 09:19:51PM +0530, Pratyush Anand wrote:
> On Wed, Sep 7, 2016 at 10:03 AM, AKASHI Takahiro
> <takahiro.akashi at linaro.org> wrote:
> > Due to the kernel patch[1], the current code will not be able to identify
>
> [1] is mentioned in cover letter only, not here.
Oops, thanks.
> > the correct value of PHYS_OFFSET if some "reserved" memory region, which
> > is likely to be UEFI runtime services code/data, exists at an address below
> > the first "System RAM" regions.
> >
> > This patch fixes this issue.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > ---
> > kexec/arch/arm64/iomem.h | 7 +++++++
> > kexec/arch/arm64/kexec-arm64.c | 22 +++++++++++++++++-----
> > 2 files changed, 24 insertions(+), 5 deletions(-)
> > create mode 100644 kexec/arch/arm64/iomem.h
> >
> > diff --git a/kexec/arch/arm64/iomem.h b/kexec/arch/arm64/iomem.h
> > new file mode 100644
> > index 0000000..7fd66eb
> > --- /dev/null
> > +++ b/kexec/arch/arm64/iomem.h
> > @@ -0,0 +1,7 @@
> > +#ifndef IOMEM_H
> > +#define IOMEM_H
> > +
> > +#define SYSTEM_RAM "System RAM\n"
> > +#define IOMEM_RESERVED "reserved\n"
> > +
> > +#endif
> > diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> > index 7183dac..bc96c76 100644
> > --- a/kexec/arch/arm64/kexec-arm64.c
> > +++ b/kexec/arch/arm64/kexec-arm64.c
> > @@ -21,6 +21,7 @@
> > #include "crashdump-arm64.h"
> > #include "dt-ops.h"
> > #include "fs2dt.h"
> > +#include "iomem.h"
> > #include "kexec-syscall.h"
> > #include "arch/options.h"
> >
> > @@ -465,18 +466,28 @@ void add_segment(struct kexec_info *info, const void *buf, size_t bufsz,
> > * get_memory_ranges_iomem_cb - Helper for get_memory_ranges_iomem.
> > */
> >
> > +static int count_memory_ranges;
>
> IMHO improving definition of kexec_iomem_for_each_line() and
> callback() for handling NULL match case would have been a better
> choice than introducing a new static variable. callback() can return
> -ve in case of error, 0 in case of no match and 1 in case of (one)
> match.
Maybe. But we have to be careful not to change other arch's results.
Or follow arm's approach.
-Takahiro AKASHI
> ~Pratyush
>
> > +
> > static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
> > unsigned long long base, unsigned long long length)
> > {
> > struct memory_range *r;
> >
> > - if (nr >= KEXEC_SEGMENT_MAX)
> > + if (count_memory_ranges >= KEXEC_SEGMENT_MAX)
> > return -1;
> >
> > - r = (struct memory_range *)data + nr;
> > - r->type = RANGE_RAM;
> > + r = (struct memory_range *)data + count_memory_ranges;
> > +
> > + 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 0;
> > +
> > r->start = base;
> > r->end = base + length - 1;
> > + count_memory_ranges++;
> >
> > set_phys_offset(r->start);
> >
> > @@ -493,9 +504,10 @@ static int get_memory_ranges_iomem_cb(void *data, int nr, char *str,
> > static int get_memory_ranges_iomem(struct memory_range *array,
> > unsigned int *count)
> > {
> > - *count = kexec_iomem_for_each_line("System RAM\n",
> > - get_memory_ranges_iomem_cb, array);
> > + count_memory_ranges = 0;
> > + kexec_iomem_for_each_line(NULL, get_memory_ranges_iomem_cb, array);
> >
> > + *count = count_memory_ranges;
> > if (!*count) {
> > dbgprintf("%s: failed: No RAM found.\n", __func__);
> > return -EFAILED;
> > --
> > 2.9.0
> >
