[RFC][PATCH] powerpc: Use the #address-cells information to parse memory/reg

6 messages, 3 authors, 2011-06-06 · open the first message on its own page

[RFC][PATCH] powerpc: Use the #address-cells information to parse memory/reg

From: Suzuki Poulose <hidden>
Date: 2011-05-30 06:30:22

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 <redacted>

---
  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';

Re: [RFC][PATCH] powerpc: Use the #address-cells information to parse memory/reg

From: Suzuki Poulose <hidden>
Date: 2011-06-06 07:18:50

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 <redacted>
Could you please let me know your thoughts/comments about this patch ?

Thanks
Suzuki
quoted hunk
---
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

Re: [RFC][PATCH] powerpc: Use the #address-cells information to parse memory/reg

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: 2011-06-06 08:51:39

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
quoted
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.
quoted
+ 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.
quoted
+
+ return 0;
+}
+
Sebastian

RE: [RFC][PATCH] powerpc: Use the #address-cells information to parsememory/reg

From: David Laight <hidden>
Date: 2011-06-06 09:02:01

quoted
Changed the add_usable_mem_property() to accept FILE* fp instead of
int fd,
quoted
as most of the other users of read_memory_region_limits() deals with
FILE*.
quoted
Signed-off-by: Suzuki K. Poulose <redacted>
=20
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

Re: [RFC][PATCH] powerpc: Use the #address-cells information to parse memory/reg

From: Suzuki Poulose <hidden>
Date: 2011-06-06 11:02:39

On 06/06/11 14:21, Sebastian Andrzej Siewior wrote:
Suzuki Poulose wrote:
quoted
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
quoted
quoted
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.
quoted
quoted
+ 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

Re: [RFC][PATCH] powerpc: Use the #address-cells information to parsememory/reg

From: Suzuki Poulose <hidden>
Date: 2011-06-06 11:29:38

On 06/06/11 14:30, David Laight wrote:
quoted
quoted
Changed the add_usable_mem_property() to accept FILE* fp instead of
int fd,
quoted
quoted
as most of the other users of read_memory_region_limits() deals with
FILE*.
quoted
quoted
Signed-off-by: Suzuki K. Poulose<redacted>
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help