Attachment 203983 Details for Bug 233863 – Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches, preserving peer order, handling Apple oddities so information is preserved

[patch] Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches, preserving peer order, handling Apple oddities so information is preserved

file_233863.txt (text/plain), 9.41 KB, created by Mark Millard on 2019-04-24 21:13:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Millard

Created: 2019-04-24 21:13:35 UTC

Size: 9.41 KB

patch

obsolete

>Index: /usr/src/stand/powerpc/ofw/ofwfdt.c
>===================================================================
>--- /usr/src/stand/powerpc/ofw/ofwfdt.c	(revision 345758)
>+++ /usr/src/stand/powerpc/ofw/ofwfdt.c	(working copy)
>@@ -42,22 +42,54 @@
> }
> 
> static void
>-add_node_to_fdt(void *buffer, phandle_t node, int fdt_offset)
>+add_node_to_fdt(void *buffer, phandle_t parent_node, int fdt_offset)
> {
> 	int i, child_offset, error;
>-	char name[255], *lastprop, *subname;
>+	char name[255+1], *lastprop, *subname; // +1 added for always having a trailing '\0' position.
> 	void *propbuf;
> 	ssize_t proplen;
> 
>-	lastprop = NULL;
>-	while (OF_nextprop(node, lastprop, name) > 0) {
>-		proplen = OF_getproplen(node, name);
>+	// WARNING: fdt_setprop adds to the beginning of the property sequence. So,
>+	// to avoid reversing the sequence, use it last to first for the originals.
> 
>+	int prop_cnt= 0;
>+	lastprop= NULL;
>+	while (0<OF_nextprop(parent_node, lastprop, name)) {
>+		prop_cnt++;
>+		lastprop= name;
>+	}
>+	int prop_want= prop_cnt;
>+	for (; 0!=prop_want; prop_want--) {
>+		lastprop= NULL;
>+		int prop_at= 0;
>+		for(; prop_at!=prop_want; prop_at++) {
>+			OF_nextprop(parent_node, lastprop, name);
>+			lastprop= name;
>+		}
>+
>+		proplen = OF_getproplen(parent_node, name);
>+
> 		/* Detect and correct for errors and strangeness */
>-		if (proplen < 0)
>+		if (proplen < 0) {
>+			printf("proplen was negative: %jd while adding property %s to "
>+			    "parent_node %d\n", (intmax_t)proplen, name, fdt_offset);
> 			proplen = 0;
>-		if (proplen > 1024)
>+		}
>+		if (proplen > 1024) {
>+			printf("proplen was large: %jd while adding property %s to "
>+			    "parent_node %d\n", (intmax_t)proplen, name, fdt_offset);
>+#if 0
>+// WARNING: Some Macintoshes end up with video-card driver code as properties.
>+//          An example PowerMac7,2 configuration had 2 such properties, each
>+//          being 96306 bytes in size. If these were ever used in a truncated
>+//          from things would be messed up. So do not force truncations. There
>+//          are a few other, smaller properties that bigger than 1 KBytes. I
>+//          checked: fdt_platform_load_dtb's 409600 buflen is more than
>+//          sufficient for the example context.
>+
> 			proplen = 1024;
>+#endif
>+		}
> 
> 		propbuf = malloc(proplen);
> 		if (propbuf == NULL) {
>@@ -64,31 +96,99 @@
> 			printf("Cannot allocate memory for prop %s\n", name);
> 			return;
> 		}
>-		OF_getprop(node, name, propbuf, proplen);
>+		OF_getprop(parent_node, name, propbuf, proplen);
> 		error = fdt_setprop(buffer, fdt_offset, name, propbuf, proplen);
> 		free(propbuf);
>-		lastprop = name;
> 		if (error)
> 			printf("Error %d adding property %s to "
>-			    "node %d\n", error, name, fdt_offset);
>+			    "parent_node %d\n", error, name, fdt_offset);
> 	}
> 
>-	if (!OF_hasprop(node, "phandle") && !OF_hasprop(node, "linux,phandle")
>-	    && !OF_hasprop(node, "ibm,phandle"))
>-		fdt_setprop(buffer, fdt_offset, "phandle", &node, sizeof(node));
>+	if (!OF_hasprop(parent_node, "phandle") && !OF_hasprop(parent_node, "linux,phandle")
>+	    && !OF_hasprop(parent_node, "ibm,phandle"))
>+		fdt_setprop(buffer, fdt_offset, "phandle", &parent_node, sizeof(parent_node));
> 
>-	for (node = OF_child(node); node > 0; node = OF_peer(node)) {
>-		OF_package_to_path(node, name, sizeof(name));
>-		subname = strrchr(name, '/');
>+	// WARNING: fdt_add_subnode adds to the beginning of the node sequence (after
>+	// the properties). So, to avoid reversing the sequence, use it last to first
>+	// for the originals.
>+
>+	// WARNING: openfirmware's package-to-path(nd,nm,len) does not place a trailing '\0'
>+	//          character in nm when it returns a full_str_len with len<=full_str_len .
>+	//          For full_str_len<len, a trailing '\0' is placed at nm[full_str_len].
>+
>+	int child_cnt= 0;
>+	phandle_t node= OF_child(parent_node);
>+	for (; 0<node; node= OF_peer(node)) {
>+		child_cnt++;
>+	}
>+	name[255]= '\0'; // Avoid OF_package_to_path leaving no '\0' at end of long path.
>+	int node_want= child_cnt;
>+	for (; 0!=node_want; node_want--) {
>+		node= OF_child(parent_node);
>+		int node_at= 1;
>+		for (; node_at!=node_want; node_at++) {
>+			node = OF_peer(node);
>+		}
>+
>+		int full_str_len= OF_package_to_path(node, name, sizeof(name)-1); // Avoids having trailing '\0' missing.
>+		if (-1==full_str_len) { // Highly unlikely.
>+			printf("add_node_to_fdt got -1 return from OF_packakge_to_path\n");
>+			continue;
>+		}
>+
>+		// WARNING: For some Macintoshes, name can sometimes contain '\0' characters
>+		//          in the middle! So there is avoidance-code because the fdt code is
>+		//          not designed for such.
>+
>+		// WARNING: For some Macintoshes, multiple nodes under a parent can have the
>+		//          same full-path-text (name here). I've adjusted fdt_add_subnode to
>+		//          allow such to be recorded so that such Macintosh information is
>+		//          not lost.
>+
>+		// full_str_len omits the offical trailing '\0' position.
>+		// full_str_len is *not* limited by the sizeof(name)-1 value above: it reports
>+		// the space needed to get all the text (ignoring the official trailing '\0').
>+		if (0==full_str_len) { // Highly unlikely.
>+			printf("Error: Node name has no bytes before trailing null byte\n");
>+			continue;
>+		}
>+		if (255<full_str_len) {
>+			printf("Error: Node path %s (truncated), needs more than 255+1 bytes\n", name);
>+			continue;
>+		}
>+
>+		// Eliminate any internal '\0' full path characters: makes saved results more standard
>+		// and, so, fdt-supported. The extra Macintosh '\0' characters are non-essential.
>+		char *from_pos= name;
>+		char *to_pos= name;
>+		char *original_end= name+full_str_len;
>+		while (from_pos<original_end) {
>+			if ('\0'!=*from_pos)
>+				*to_pos++= *from_pos;
>+			else
>+				full_str_len--;
>+			from_pos++;
>+		}
>+		*to_pos= '\0';
>+		if (to_pos==name) { // Highly unlikely.
>+			printf("Error: Adjusted node name has no bytes before trailing null byte\n");
>+			continue;
>+		}
>+
>+		subname= strrchr(name,'/');
>+		if (NULL==subname) { // Highly unlikely.
>+			printf("Error: Node name %s has no '/'\n", name);
>+			continue;
>+		}
> 		subname++;
> 		child_offset = fdt_add_subnode(buffer, fdt_offset, subname);
>-		if (child_offset < 0) {
>+		if (child_offset < 0) { // Note: fdt_add_subnode was modified to allow duplicate paths.
> 			printf("Error %d adding node %s (%s), skipping\n",
> 			    child_offset, name, subname);
> 			continue;
> 		}
> 	
>-                add_node_to_fdt(buffer, node, child_offset);
>+		add_node_to_fdt(buffer, node, child_offset);
> 	}
> }
> 
>@@ -135,6 +235,19 @@
> 		/* Mark RTAS private data area reserved */
> 		fdt_add_mem_rsv(fdtp, base, len);
> 	} else {
>+#if 0
>+// Without the /memory0/available information the example
>+// MPC750 iMac G3 PowerMac4,1 crashes shortly after booting
>+// --from trashed memory contents from use of RAM that was
>+// not excluded when available was being deleted.
>+//
>+// My guess is that openfirmware may also report to
>+// avoid address ranges for which other things in the
>+// environment are the cause, not just openfirmware's
>+// internal memory use.
>+//
>+// So do not do the following . . .
>+
> 		/*
> 		 * Remove /memory/available properties, which reflect long-gone
> 		 * OF state. Note that this doesn't work if we need RTAS still,
>@@ -141,8 +254,10 @@
> 		 * since that's part of the firmware.
> 		 */
> 		offset = fdt_path_offset(fdtp, "/memory@0");
>+		if (offset<=0) offset= fdt_path_offset(fdtp, "/memory@0,0"); // G5's
> 		if (offset > 0)
> 			fdt_delprop(fdtp, offset, "available");
>+#endif
> 	}
> 
> 	
>@@ -182,19 +297,40 @@
> int
> fdt_platform_load_dtb(void)
> {
>-        void *buffer;
>-        size_t buflen = 409600;
>+	void *buffer;
>+	size_t buflen = 409600;
>+	int ret;
> 
>-        buffer = malloc(buflen);
>-        fdt_create_empty_tree(buffer, buflen);
>-        add_node_to_fdt(buffer, OF_peer(0), fdt_path_offset(buffer, "/"));
>-        ofwfdt_fixups(buffer);
>-        fdt_pack(buffer);
>+	buffer = malloc(buflen);
>+	if (NULL==buffer) {
>+		printf("fdt_platform_load_dtb: Large buffer malloc failed.\n");
>+		return 1; // Give up.
>+	}
> 
>-        fdt_load_dtb_addr(buffer);
>-        free(buffer);
>+	ret= fdt_create_empty_tree(buffer, buflen);
>+	if (ret) {
>+		printf("fdt_platform_load_dtb: fdt_create_empty_tree failed\n");
>+		goto cleanup_and_return;
>+	}
> 
>-        return (0);
>+	add_node_to_fdt(buffer, OF_peer(0), fdt_path_offset(buffer, "/"));
>+	ofwfdt_fixups(buffer);
>+
>+	ret= fdt_pack(buffer);
>+	if (ret) {
>+		printf("fdt_platform_load_dtb: fdt_pack failed\n");
>+		goto cleanup_and_return;
>+	}
>+
>+	ret= fdt_load_dtb_addr(buffer);
>+	if (ret) {
>+		printf("fdt_platform_load_dtb: fdt_load_dtb_addr failed\n");
>+		goto cleanup_and_return;
>+	}
>+
>+cleanup_and_return:
>+	free(buffer);
>+	return ret;
> }
> 
> void
>Index: /usr/src/sys/contrib/libfdt/fdt_rw.c
>===================================================================
>--- /usr/src/sys/contrib/libfdt/fdt_rw.c	(revision 345758)
>+++ /usr/src/sys/contrib/libfdt/fdt_rw.c	(working copy)
>@@ -358,7 +358,14 @@
> 
> 	offset = fdt_subnode_offset_namelen(fdt, parentoffset, name, namelen);
> 	if (offset >= 0)
>+#if 0
>+// Some Macintoshes have identical package-to-pathname results for
>+// multiple nodes of the same type and unit under the parent node.
>+// Avoid blocking this for fdt.
> 		return -FDT_ERR_EXISTS;
>+#else
>+		;
>+#endif
> 	else if (offset != -FDT_ERR_NOTFOUND)
> 		return offset;
>

Actions: View | Diff

Attachments on bug 233863: 199947 | 203622 | 203627 | 203652 | 203683 | 203812 | 203814 | 203844 | 203845 | 203889 | 203980 | 203981 | 203983 | 204307 | 204368 | 204369 | 223108