Message ID | 4DE3396E.4090801@in.ibm.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
On 05/30/11 12:00, Suzuki Poulose wrote: > Use the #address-cells, #size-cells information to parse the memory/reg info > from device tree. > > The format of memory/reg is based on the #address-cells,#size-cells. Currently, > the kexec-tools doesn't use the above values in parsing the memory/reg values. > Hence the kexec cannot handle cases where #address-cells, #size-cells are > different, (for e.g, PPC440X ). > > This patch introduces a read_memory_region_limits(), which parses the > memory/reg contents based on the values of #address-cells and #size-cells. > > Changed the add_usable_mem_property() to accept FILE* fp instead of int fd, > as most of the other users of read_memory_region_limits() deals with FILE*. > > Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com> Could you please let me know your thoughts/comments about this patch ? Thanks Suzuki > > --- > kexec/arch/ppc/crashdump-powerpc.c | 23 ------ > kexec/arch/ppc/fs2dt.c | 31 ++------ > kexec/arch/ppc/kexec-ppc.c | 136 ++++++++++++++++++++++++++----------- > kexec/arch/ppc/kexec-ppc.h | 6 + > 4 files changed, 118 insertions(+), 78 deletions(-) > > Index: kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.c > =================================================================== > --- kexec-tools-2.0.4.orig/kexec/arch/ppc/kexec-ppc.c > +++ kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.c > @@ -26,6 +26,7 @@ > > #include "config.h" > > +unsigned long dt_address_cells = 0, dt_size_cells = 0; > uint64_t rmo_top; > unsigned long long crash_base = 0, crash_size = 0; > unsigned long long initrd_base = 0, initrd_size = 0; > @@ -34,6 +35,92 @@ unsigned int rtas_base, rtas_size; > int max_memory_ranges; > const char *ramdisk; > > +/* > + * Reads the #address-cells and #size-cells on this platform. > + * This is used to parse the memory/reg info from the device-tree > + */ > +int init_memory_region_info() > +{ > + size_t res = 0; > + FILE *fp; > + char *file; > + > + file = "/proc/device-tree/#address-cells"; > + fp = fopen(file, "r"); > + if (!fp) { > + fprintf(stderr,"Unable to open %s\n", file); > + return -1; > + } > + > + res = fread(&dt_address_cells,sizeof(unsigned long),1,fp); > + if (res != 1) { > + fprintf(stderr,"Error reading %s\n", file); > + return -1; > + } > + fclose(fp); > + dt_address_cells *= sizeof(unsigned long); > + > + file = "/proc/device-tree/#size-cells"; > + fp = fopen(file, "r"); > + if (!fp) { > + fprintf(stderr,"Unable to open %s\n", file); > + return -1; > + } > + > + res = fread(&dt_size_cells,sizeof(unsigned long),1,fp); > + if (res != 1) { > + fprintf(stderr,"Error reading %s\n", file); > + return -1; > + } > + fclose(fp); > + dt_size_cells *= sizeof(unsigned long); > + > + return 0; > +} > + > +#define MAXBYTES 128 > +/* > + * Reads the memory region info from the device-tree node pointed > + * by @fp and fills the *start, *end with the boundaries of the region > + */ > +int read_memory_region_limits(FILE* fp, unsigned long long *start, > + unsigned long long *end) > +{ > + char buf[MAXBYTES]; > + unsigned long *p; > + unsigned long nbytes = dt_address_cells + dt_size_cells; > + > + if (fread(buf, 1, MAXBYTES, fp) != nbytes) { > + fprintf(stderr, "Error reading the memory region info\n"); > + return -1; > + } > + > + p = (unsigned long*)buf; > + if (dt_address_cells == sizeof(unsigned long)) { > + *start = p[0]; > + p++; > + } else if (dt_address_cells == sizeof(unsigned long long)) { > + *start = ((unsigned long long *)p)[0]; > + p = (unsigned long long *)p + 1; > + } else { > + fprintf(stderr,"Unsupported value for #address-cells : %ld\n", > + dt_address_cells); > + return -1; > + } > + > + if (dt_size_cells == sizeof(unsigned long)) > + *end = *start + p[0]; > + else if (dt_size_cells == sizeof(unsigned long long)) > + *end = *start + ((unsigned long long *)p)[0]; > + else { > + fprintf(stderr,"Unsupported value for #size-cells : %ld\n", > + dt_size_cells); > + return -1; > + } > + > + return 0; > +} > + > void arch_reuse_initrd(void) > { > reuse_initrd = 1; > @@ -182,9 +269,6 @@ static int sort_base_ranges(void) > return 0; > } > > - > -#define MAXBYTES 128 > - > static int realloc_memory_ranges(void) > { > size_t memory_range_len; > @@ -248,6 +332,8 @@ static int get_base_ranges(void) > return -1; > } > while ((mentry = readdir(dmem)) != NULL) { > + unsigned long long start, end; > + > if (strcmp(mentry->d_name, "reg")) > continue; > strcat(fname, "/reg"); > @@ -257,8 +343,7 @@ static int get_base_ranges(void) > closedir(dir); > return -1; > } > - if ((n = fread(buf, 1, MAXBYTES, file)) < 0) { > - perror(fname); > + if (read_memory_region_limits(file, &start, &end) != 0) { > fclose(file); > closedir(dmem); > closedir(dir); > @@ -271,24 +356,8 @@ static int get_base_ranges(void) > } > } > > - if (n == sizeof(uint32_t) * 2) { > - base_memory_range[local_memory_ranges].start = > - ((uint32_t *)buf)[0]; > - base_memory_range[local_memory_ranges].end = > - base_memory_range[local_memory_ranges].start + > - ((uint32_t *)buf)[1]; > - } > - else if (n == sizeof(uint64_t) * 2) { > - base_memory_range[local_memory_ranges].start = > - ((uint64_t *)buf)[0]; > - base_memory_range[local_memory_ranges].end = > - base_memory_range[local_memory_ranges].start + > - ((uint64_t *)buf)[1]; > - } > - else { > - fprintf(stderr, "Mem node has invalid size: %d\n", n); > - return -1; > - } > + base_memory_range[local_memory_ranges].start = start; > + base_memory_range[local_memory_ranges].end = end; > base_memory_range[local_memory_ranges].type = RANGE_RAM; > local_memory_ranges++; > dbgprintf("%016llx-%016llx : %x\n", > @@ -577,20 +646,10 @@ static int get_devtree_details(unsigned > perror(fname); > goto error_opencdir; > } > - if ((n = fread(buf, 1, MAXBYTES, file)) < 0) { > - perror(fname); > - goto error_openfile; > - } > - if (n == sizeof(uint64_t)) { > - rmo_base = ((uint32_t *)buf)[0]; > - rmo_top = rmo_base + ((uint32_t *)buf)[1]; > - } else if (n == 16) { > - rmo_base = ((uint64_t *)buf)[0]; > - rmo_top = rmo_base + ((uint64_t *)buf)[1]; > - } else { > - fprintf(stderr, "Mem node has invalid size: %d\n", n); > + if (read_memory_region_limits(file, &rmo_base, &rmo_top) != 0) { > goto error_openfile; > } > + > if (rmo_top > 0x30000000UL) > rmo_top = 0x30000000UL; > > @@ -664,7 +723,6 @@ error_opendir: > return -1; > } > > - > /* Setup a sorted list of memory ranges. */ > static int setup_memory_ranges(unsigned long kexec_flags) > { > @@ -756,7 +814,6 @@ out: > return -1; > } > > - > /* Return a list of valid memory ranges */ > int get_memory_ranges_dt(struct memory_range **range, int *ranges, > unsigned long kexec_flags) > @@ -778,6 +835,11 @@ int get_memory_ranges_dt(struct memory_r > int get_memory_ranges(struct memory_range **range, int *ranges, > unsigned long kexec_flags) > { > + int res = 0; > + > + res = init_memory_region_info(); > + if (res != 0) > + return res; > #ifdef WITH_GAMECUBE > return get_memory_ranges_gc(range, ranges, kexec_flags); > #else > Index: kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.h > =================================================================== > --- kexec-tools-2.0.4.orig/kexec/arch/ppc/kexec-ppc.h > +++ kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.h > @@ -69,6 +69,12 @@ extern unsigned long long initrd_base, i > extern unsigned long long ramdisk_base, ramdisk_size; > extern unsigned char reuse_initrd; > extern const char *ramdisk; > + > +/* Method to parse the memory/reg nodes in device-tree */ > +extern unsigned long dt_address_cells, dt_size_cells; > +extern int init_memory_region_info(void); > +extern int read_memory_region_limits(FILE *fp, unsigned long long *start, > + unsigned long long *end); > #define COMMAND_LINE_SIZE 512 /* from kernel */ > /*fs2dt*/ > void reserve(unsigned long long where, unsigned long long length); > Index: kexec-tools-2.0.4/kexec/arch/ppc/crashdump-powerpc.c > =================================================================== > --- kexec-tools-2.0.4.orig/kexec/arch/ppc/crashdump-powerpc.c > +++ kexec-tools-2.0.4/kexec/arch/ppc/crashdump-powerpc.c > @@ -130,9 +130,8 @@ static int get_crash_memory_ranges(struc > closedir(dir); > goto err; > } > - n = fread(buf, 1, MAXBYTES, file); > - if (n < 0) { > - perror(fname); > + n = read_memory_region_limits(file, &start, &end); > + if (n != 0) { > fclose(file); > closedir(dmem); > closedir(dir); > @@ -146,24 +145,6 @@ static int get_crash_memory_ranges(struc > goto err; > } > > - /* > - * FIXME: This code fails on platforms that > - * have more than one memory range specified > - * in the device-tree's /memory/reg property. > - * or where the #address-cells and #size-cells > - * are not identical. > - * > - * We should interpret the /memory/reg property > - * based on the values of the #address-cells and > - * #size-cells properites. > - */ > - if (n == (sizeof(unsigned long) * 2)) { > - start = ((unsigned long *)buf)[0]; > - end = start + ((unsigned long *)buf)[1]; > - } else { > - start = ((unsigned long long *)buf)[0]; > - end = start + ((unsigned long long *)buf)[1]; > - } > if (start == 0 && end >= (BACKUP_SRC_END + 1)) > start = BACKUP_SRC_END + 1; > > Index: kexec-tools-2.0.4/kexec/arch/ppc/fs2dt.c > =================================================================== > --- kexec-tools-2.0.4.orig/kexec/arch/ppc/fs2dt.c > +++ kexec-tools-2.0.4/kexec/arch/ppc/fs2dt.c > @@ -122,7 +122,7 @@ static unsigned propnum(const char *name > return offset; > } > > -static void add_usable_mem_property(int fd, int len) > +static void add_usable_mem_property(FILE* fp, int len) > { > char fname[MAXPATH], *bname; > unsigned long buf[2]; > @@ -137,21 +137,11 @@ static void add_usable_mem_property(int > if (strncmp(bname, "/memory@", 8) && strcmp(bname, "/memory")) > return; > > - if (len < 2 * sizeof(unsigned long)) > - die("unrecoverable error: not enough data for mem property\n"); > - len = 2 * sizeof(unsigned long); > - > - if (lseek(fd, 0, SEEK_SET) < 0) > + if (fseek(fp, 0, SEEK_SET) < 0) > die("unrecoverable error: error seeking in \"%s\": %s\n", > pathname, strerror(errno)); > - if (read(fd, buf, len) != len) > - die("unrecoverable error: error reading \"%s\": %s\n", > - pathname, strerror(errno)); > - > - if (~0ULL - buf[0] < buf[1]) > - die("unrecoverable error: mem property overflow\n"); > - base = buf[0]; > - end = base + buf[1]; > + if (read_memory_region_limits(fp, &base, &end) != 0) > + die("unrecoverable error: error parsing memory/reg limits\n"); > > for (range = 0; range < usablemem_rgns.size; range++) { > loc_base = usablemem_rgns.ranges[range].start; > @@ -194,8 +184,9 @@ static void add_usable_mem_property(int > static void putprops(char *fn, struct dirent **nlist, int numlist) > { > struct dirent *dp; > - int i = 0, fd, len; > + int i = 0, len; > struct stat statbuf; > + FILE *fp; > > for (i = 0; i < numlist; i++) { > dp = nlist[i]; > @@ -243,12 +234,12 @@ static void putprops(char *fn, struct di > *dt++ = len; > *dt++ = propnum(fn); > > - fd = open(pathname, O_RDONLY); > - if (fd == -1) > + fp = fopen(pathname, "r"); > + if (fp == NULL) > die("unrecoverable error: could not open \"%s\": %s\n", > pathname, strerror(errno)); > > - if (read(fd, dt, len) != len) > + if (fread(dt, 1, len, fp) != len) > die("unrecoverable error: could not read \"%s\": %s\n", > pathname, strerror(errno)); > > @@ -290,8 +281,8 @@ static void putprops(char *fn, struct di > > dt += (len + 3)/4; > if (!strcmp(dp->d_name, "reg") && usablemem_rgns.size) > - add_usable_mem_property(fd, len); > - close(fd); > + add_usable_mem_property(fp, len); > + fclose(fp); > } > > fn[0] = '\0'; > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec
Suzuki Poulose wrote: > Could you please let me know your thoughts/comments about this patch ? I'm mostly fine with it. Maaxim copied fs2dt.c from ppc64 to ppc. So I guess ppc64 has the same problem. ARM and MIPS is soon having DT support and kexec is probably also on their list so I would hate to see them to either copy the DT parsing file or having their own implementation. Maybe we should try to use libfdt for dt parsing. It has /proc import support so it should be fine for our needs. It is already in tree and used by ppc32 if a basic dtb is specified. I'm not sure if the /proc interface is part of dtc or libfdt. I'm not saying this has to be done now but maybe later before ARM and/or MIPS comes along needs something similar for their needs. If the libfdt is too complex for sucking in the dtb from /proc then maybe something else that generic and can be shared between booth ppc architectures and the other ones. > Thanks > Suzuki > >> >> Index: kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.c >> =================================================================== >> --- kexec-tools-2.0.4.orig/kexec/arch/ppc/kexec-ppc.c >> +++ kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.c >> @@ -34,6 +35,92 @@ unsigned int rtas_base, rtas_size; >> int max_memory_ranges; >> const char *ramdisk; >> >> +/* >> + * Reads the #address-cells and #size-cells on this platform. >> + * This is used to parse the memory/reg info from the device-tree >> + */ >> +int init_memory_region_info() >> +{ >> + size_t res = 0; >> + FILE *fp; >> + char *file; >> + >> + file = "/proc/device-tree/#address-cells"; >> + fp = fopen(file, "r"); >> + if (!fp) { >> + fprintf(stderr,"Unable to open %s\n", file); >> + return -1; >> + } >> + >> + res = fread(&dt_address_cells,sizeof(unsigned long),1,fp); >> + if (res != 1) { >> + fprintf(stderr,"Error reading %s\n", file); >> + return -1; >> + } >> + fclose(fp); >> + dt_address_cells *= sizeof(unsigned long); This should be sizeof(unsigned int). I know we on 32bit. >> + file = "/proc/device-tree/#size-cells"; >> + fp = fopen(file, "r"); >> + if (!fp) { >> + fprintf(stderr,"Unable to open %s\n", file); >> + return -1; >> + } >> + >> + res = fread(&dt_size_cells,sizeof(unsigned long),1,fp); >> + if (res != 1) { >> + fprintf(stderr,"Error reading %s\n", file); >> + return -1; >> + } >> + fclose(fp); >> + dt_size_cells *= sizeof(unsigned long); same here. >> + >> + return 0; >> +} >> + Sebastian
> > Changed the add_usable_mem_property() to accept FILE* fp instead of int fd, > > as most of the other users of read_memory_region_limits() deals with FILE*. > > > > Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com> > > Could you please let me know your thoughts/comments about this patch ? Is the change to use 'FILE *' actually progress? I'd have thought that the randomly aligned read/lseek system calls that this allows to happen are not desirable for anything that isn't a true file. I'd also suggest that the sizeof's should be applied to the actual type of the variable being read/written, not arbitrarily 'long' or 'int', and this probably ought to be some fixed size type. David
On 06/06/11 14:21, Sebastian Andrzej Siewior wrote: > Suzuki Poulose wrote: >> Could you please let me know your thoughts/comments about this patch ? > > I'm mostly fine with it. > > Maaxim copied fs2dt.c from ppc64 to ppc. So I guess ppc64 has the same > problem. Yes, you are right. Porting this patch over to ppc64 is in my TODO list. > ARM and MIPS is soon having DT support and kexec is probably also > on their list so I would hate to see them to either copy the DT parsing > file or having their own implementation. > > Maybe we should try to use libfdt for dt parsing. It has /proc import > support so it should be fine for our needs. It is already in tree and used > by ppc32 if a basic dtb is specified. I'm not sure if the /proc interface > is part of dtc or libfdt. > > I'm not saying this has to be done now but maybe later before ARM and/or > MIPS comes along needs something similar for their needs. If the libfdt is > too complex for sucking in the dtb from /proc then maybe something else > that generic and can be shared between booth ppc architectures and the > other ones. OK >>> Index: kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.c >>> =================================================================== >>> --- kexec-tools-2.0.4.orig/kexec/arch/ppc/kexec-ppc.c >>> +++ kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.c >>> @@ -34,6 +35,92 @@ unsigned int rtas_base, rtas_size; >>> int max_memory_ranges; >>> const char *ramdisk; >>> >>> +/* >>> + * Reads the #address-cells and #size-cells on this platform. >>> + * This is used to parse the memory/reg info from the device-tree >>> + */ >>> +int init_memory_region_info() >>> +{ >>> + size_t res = 0; >>> + FILE *fp; >>> + char *file; >>> + >>> + file = "/proc/device-tree/#address-cells"; >>> + fp = fopen(file, "r"); >>> + if (!fp) { >>> + fprintf(stderr,"Unable to open %s\n", file); >>> + return -1; >>> + } >>> + >>> + res = fread(&dt_address_cells,sizeof(unsigned long),1,fp); >>> + if (res != 1) { >>> + fprintf(stderr,"Error reading %s\n", file); >>> + return -1; >>> + } >>> + fclose(fp); >>> + dt_address_cells *= sizeof(unsigned long); > > This should be sizeof(unsigned int). I know we on 32bit. > OK. I was using (unsigned long) to get the word size on the machine. Given this code is duplicated in ppc64, thought of having a generic code which works fine for all ppcXX. As you mentioned, if we go about moving to a single copy of fdt code, using long would help us. >>> + file = "/proc/device-tree/#size-cells"; >>> + fp = fopen(file, "r"); >>> + if (!fp) { >>> + fprintf(stderr,"Unable to open %s\n", file); >>> + return -1; >>> + } >>> + >>> + res = fread(&dt_size_cells,sizeof(unsigned long),1,fp); >>> + if (res != 1) { >>> + fprintf(stderr,"Error reading %s\n", file); >>> + return -1; >>> + } >>> + fclose(fp); >>> + dt_size_cells *= sizeof(unsigned long); > > same here. Thanks Suzuki
On 06/06/11 14:30, David Laight wrote: >>> Changed the add_usable_mem_property() to accept FILE* fp instead of > int fd, >>> as most of the other users of read_memory_region_limits() deals with > FILE*. >>> >>> Signed-off-by: Suzuki K. Poulose<suzuki@in.ibm.com> >> >> Could you please let me know your thoughts/comments about this patch ? > > Is the change to use 'FILE *' actually progress? > I'd have thought that the randomly aligned read/lseek system calls > that this allows to happen are not desirable for anything that > isn't a true file. I will revert the other users back to 'fd' > > I'd also suggest that the sizeof's should be applied to the > actual type of the variable being read/written, not arbitrarily > 'long' or 'int', and this probably ought to be some fixed size type. I have used 'unsigned long'(for word sized values) or 'unsigned long long' (for double words) just to make sure we get the right values. Is this OK ? Thanks Suzuki
different, (for e.g, PPC440X ). This patch introduces a read_memory_region_limits(), which parses the memory/reg contents based on the values of #address-cells and #size-cells. Changed the add_usable_mem_property() to accept FILE* fp instead of int fd, as most of the other users of read_memory_region_limits() deals with FILE*. Signed-off-by: Suzuki K. Poulose <suzuki@in.ibm.com> --- kexec/arch/ppc/crashdump-powerpc.c | 23 ------ kexec/arch/ppc/fs2dt.c | 31 ++------ kexec/arch/ppc/kexec-ppc.c | 136 ++++++++++++++++++++++++++----------- kexec/arch/ppc/kexec-ppc.h | 6 + 4 files changed, 118 insertions(+), 78 deletions(-) Index: kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.c =================================================================== --- kexec-tools-2.0.4.orig/kexec/arch/ppc/kexec-ppc.c +++ kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.c @@ -26,6 +26,7 @@ #include "config.h" +unsigned long dt_address_cells = 0, dt_size_cells = 0; uint64_t rmo_top; unsigned long long crash_base = 0, crash_size = 0; unsigned long long initrd_base = 0, initrd_size = 0; @@ -34,6 +35,92 @@ unsigned int rtas_base, rtas_size; int max_memory_ranges; const char *ramdisk; +/* + * Reads the #address-cells and #size-cells on this platform. + * This is used to parse the memory/reg info from the device-tree + */ +int init_memory_region_info() +{ + size_t res = 0; + FILE *fp; + char *file; + + file = "/proc/device-tree/#address-cells"; + fp = fopen(file, "r"); + if (!fp) { + fprintf(stderr,"Unable to open %s\n", file); + return -1; + } + + res = fread(&dt_address_cells,sizeof(unsigned long),1,fp); + if (res != 1) { + fprintf(stderr,"Error reading %s\n", file); + return -1; + } + fclose(fp); + dt_address_cells *= sizeof(unsigned long); + + file = "/proc/device-tree/#size-cells"; + fp = fopen(file, "r"); + if (!fp) { + fprintf(stderr,"Unable to open %s\n", file); + return -1; + } + + res = fread(&dt_size_cells,sizeof(unsigned long),1,fp); + if (res != 1) { + fprintf(stderr,"Error reading %s\n", file); + return -1; + } + fclose(fp); + dt_size_cells *= sizeof(unsigned long); + + return 0; +} + +#define MAXBYTES 128 +/* + * Reads the memory region info from the device-tree node pointed + * by @fp and fills the *start, *end with the boundaries of the region + */ +int read_memory_region_limits(FILE* fp, unsigned long long *start, + unsigned long long *end) +{ + char buf[MAXBYTES]; + unsigned long *p; + unsigned long nbytes = dt_address_cells + dt_size_cells; + + if (fread(buf, 1, MAXBYTES, fp) != nbytes) { + fprintf(stderr, "Error reading the memory region info\n"); + return -1; + } + + p = (unsigned long*)buf; + if (dt_address_cells == sizeof(unsigned long)) { + *start = p[0]; + p++; + } else if (dt_address_cells == sizeof(unsigned long long)) { + *start = ((unsigned long long *)p)[0]; + p = (unsigned long long *)p + 1; + } else { + fprintf(stderr,"Unsupported value for #address-cells : %ld\n", + dt_address_cells); + return -1; + } + + if (dt_size_cells == sizeof(unsigned long)) + *end = *start + p[0]; + else if (dt_size_cells == sizeof(unsigned long long)) + *end = *start + ((unsigned long long *)p)[0]; + else { + fprintf(stderr,"Unsupported value for #size-cells : %ld\n", + dt_size_cells); + return -1; + } + + return 0; +} + void arch_reuse_initrd(void) { reuse_initrd = 1; @@ -182,9 +269,6 @@ static int sort_base_ranges(void) return 0; } - -#define MAXBYTES 128 - static int realloc_memory_ranges(void) { size_t memory_range_len; @@ -248,6 +332,8 @@ static int get_base_ranges(void) return -1; } while ((mentry = readdir(dmem)) != NULL) { + unsigned long long start, end; + if (strcmp(mentry->d_name, "reg")) continue; strcat(fname, "/reg"); @@ -257,8 +343,7 @@ static int get_base_ranges(void) closedir(dir); return -1; } - if ((n = fread(buf, 1, MAXBYTES, file)) < 0) { - perror(fname); + if (read_memory_region_limits(file, &start, &end) != 0) { fclose(file); closedir(dmem); closedir(dir); @@ -271,24 +356,8 @@ static int get_base_ranges(void) } } - if (n == sizeof(uint32_t) * 2) { - base_memory_range[local_memory_ranges].start = - ((uint32_t *)buf)[0]; - base_memory_range[local_memory_ranges].end = - base_memory_range[local_memory_ranges].start + - ((uint32_t *)buf)[1]; - } - else if (n == sizeof(uint64_t) * 2) { - base_memory_range[local_memory_ranges].start = - ((uint64_t *)buf)[0]; - base_memory_range[local_memory_ranges].end = - base_memory_range[local_memory_ranges].start + - ((uint64_t *)buf)[1]; - } - else { - fprintf(stderr, "Mem node has invalid size: %d\n", n); - return -1; - } + base_memory_range[local_memory_ranges].start = start; + base_memory_range[local_memory_ranges].end = end; base_memory_range[local_memory_ranges].type = RANGE_RAM; local_memory_ranges++; dbgprintf("%016llx-%016llx : %x\n", @@ -577,20 +646,10 @@ static int get_devtree_details(unsigned perror(fname); goto error_opencdir; } - if ((n = fread(buf, 1, MAXBYTES, file)) < 0) { - perror(fname); - goto error_openfile; - } - if (n == sizeof(uint64_t)) { - rmo_base = ((uint32_t *)buf)[0]; - rmo_top = rmo_base + ((uint32_t *)buf)[1]; - } else if (n == 16) { - rmo_base = ((uint64_t *)buf)[0]; - rmo_top = rmo_base + ((uint64_t *)buf)[1]; - } else { - fprintf(stderr, "Mem node has invalid size: %d\n", n); + if (read_memory_region_limits(file, &rmo_base, &rmo_top) != 0) { goto error_openfile; } + if (rmo_top > 0x30000000UL) rmo_top = 0x30000000UL; @@ -664,7 +723,6 @@ error_opendir: return -1; } - /* Setup a sorted list of memory ranges. */ static int setup_memory_ranges(unsigned long kexec_flags) { @@ -756,7 +814,6 @@ out: return -1; } - /* Return a list of valid memory ranges */ int get_memory_ranges_dt(struct memory_range **range, int *ranges, unsigned long kexec_flags) @@ -778,6 +835,11 @@ int get_memory_ranges_dt(struct memory_r int get_memory_ranges(struct memory_range **range, int *ranges, unsigned long kexec_flags) { + int res = 0; + + res = init_memory_region_info(); + if (res != 0) + return res; #ifdef WITH_GAMECUBE return get_memory_ranges_gc(range, ranges, kexec_flags); #else Index: kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.h =================================================================== --- kexec-tools-2.0.4.orig/kexec/arch/ppc/kexec-ppc.h +++ kexec-tools-2.0.4/kexec/arch/ppc/kexec-ppc.h @@ -69,6 +69,12 @@ extern unsigned long long initrd_base, i extern unsigned long long ramdisk_base, ramdisk_size; extern unsigned char reuse_initrd; extern const char *ramdisk; + +/* Method to parse the memory/reg nodes in device-tree */ +extern unsigned long dt_address_cells, dt_size_cells; +extern int init_memory_region_info(void); +extern int read_memory_region_limits(FILE *fp, unsigned long long *start, + unsigned long long *end); #define COMMAND_LINE_SIZE 512 /* from kernel */ /*fs2dt*/ void reserve(unsigned long long where, unsigned long long length); Index: kexec-tools-2.0.4/kexec/arch/ppc/crashdump-powerpc.c =================================================================== --- kexec-tools-2.0.4.orig/kexec/arch/ppc/crashdump-powerpc.c +++ kexec-tools-2.0.4/kexec/arch/ppc/crashdump-powerpc.c @@ -130,9 +130,8 @@ static int get_crash_memory_ranges(struc closedir(dir); goto err; } - n = fread(buf, 1, MAXBYTES, file); - if (n < 0) { - perror(fname); + n = read_memory_region_limits(file, &start, &end); + if (n != 0) { fclose(file); closedir(dmem); closedir(dir); @@ -146,24 +145,6 @@ static int get_crash_memory_ranges(struc goto err; } - /* - * FIXME: This code fails on platforms that - * have more than one memory range specified - * in the device-tree's /memory/reg property. - * or where the #address-cells and #size-cells - * are not identical. - * - * We should interpret the /memory/reg property - * based on the values of the #address-cells and - * #size-cells properites. - */ - if (n == (sizeof(unsigned long) * 2)) { - start = ((unsigned long *)buf)[0]; - end = start + ((unsigned long *)buf)[1]; - } else { - start = ((unsigned long long *)buf)[0]; - end = start + ((unsigned long long *)buf)[1]; - } if (start == 0 && end >= (BACKUP_SRC_END + 1)) start = BACKUP_SRC_END + 1; Index: kexec-tools-2.0.4/kexec/arch/ppc/fs2dt.c =================================================================== --- kexec-tools-2.0.4.orig/kexec/arch/ppc/fs2dt.c +++ kexec-tools-2.0.4/kexec/arch/ppc/fs2dt.c @@ -122,7 +122,7 @@ static unsigned propnum(const char *name return offset; } -static void add_usable_mem_property(int fd, int len) +static void add_usable_mem_property(FILE* fp, int len) { char fname[MAXPATH], *bname; unsigned long buf[2]; @@ -137,21 +137,11 @@ static void add_usable_mem_property(int if (strncmp(bname, "/memory@", 8) && strcmp(bname, "/memory")) return; - if (len < 2 * sizeof(unsigned long)) - die("unrecoverable error: not enough data for mem property\n"); - len = 2 * sizeof(unsigned long); - - if (lseek(fd, 0, SEEK_SET) < 0) + if (fseek(fp, 0, SEEK_SET) < 0) die("unrecoverable error: error seeking in \"%s\": %s\n", pathname, strerror(errno)); - if (read(fd, buf, len) != len) - die("unrecoverable error: error reading \"%s\": %s\n", - pathname, strerror(errno)); - - if (~0ULL - buf[0] < buf[1]) - die("unrecoverable error: mem property overflow\n"); - base = buf[0]; - end = base + buf[1]; + if (read_memory_region_limits(fp, &base, &end) != 0) + die("unrecoverable error: error parsing memory/reg limits\n"); for (range = 0; range < usablemem_rgns.size; range++) { loc_base = usablemem_rgns.ranges[range].start; @@ -194,8 +184,9 @@ static void add_usable_mem_property(int static void putprops(char *fn, struct dirent **nlist, int numlist) { struct dirent *dp; - int i = 0, fd, len; + int i = 0, len; struct stat statbuf; + FILE *fp; for (i = 0; i < numlist; i++) { dp = nlist[i]; @@ -243,12 +234,12 @@ static void putprops(char *fn, struct di *dt++ = len; *dt++ = propnum(fn); - fd = open(pathname, O_RDONLY); - if (fd == -1) + fp = fopen(pathname, "r"); + if (fp == NULL) die("unrecoverable error: could not open \"%s\": %s\n", pathname, strerror(errno)); - if (read(fd, dt, len) != len) + if (fread(dt, 1, len, fp) != len) die("unrecoverable error: could not read \"%s\": %s\n", pathname, strerror(errno)); @@ -290,8 +281,8 @@ static void putprops(char *fn, struct di dt += (len + 3)/4; if (!strcmp(dp->d_name, "reg") && usablemem_rgns.size) - add_usable_mem_property(fd, len); - close(fd); + add_usable_mem_property(fp, len); + fclose(fp); } fn[0] = '\0';