Patches to allow usefdt mode that works on a 2 socket PowerMac3,6 example too --and makes more work on 2-socket/1-core-each PowerMac11,2

Mark Millard marklmi at yahoo.com
Fri Apr 12 19:19:38 UTC 2019


With the following 2 patches for converting
openfirmware to fdt content. . .

PowerMac11,2 example for usefdt mode:
A) bge0 and bge1 are back in their historical places.
B) powerd on the PowerMac11,2 works again.
C) sysctl -a | grep cpufreq lists items for all the cpus
D) probably more.

PowerMac3,6 example for usefdt mode:
E) gem0 is present again and even works.
F) Both CPUs are used again.
F) probably more.
(powerpd and cpufreq's are not operable/present even in
non-usefdt mode.)

This message does not deal with other investigatory
patches for other issues than converting openfirmware
to fdt. But my test environment has all my investigatory
patches in order to avoid other things getting in the
way of my investigations.

The code comments are fairly explicit about
what and why for the changes.

I treat the patches as investigatory, not ready
in form for being official FreeBSD material. There
are likely questions of if the change go in the
right long-term direction --or even if old PowerMacs
will continue to be viewed as worth supporting
(because they compete with time spent on modern
support).

I'll note that I've never had the 2-socket/1-core-each
PowerMac7,2 get any visible behavior after the Kernel
entry point message with any variation of usefdt mode.
Failing so early, I've not figured out any way to
investigate it hanging up. It does boot in non-usefdt
mode with my other investigatory patches in place.

The openfirmware to fdt conversion patches are (white
space details might not have been preserved in the
message):

# svnlite diff /usr/src/stand/powerpc/ofw/ | more
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,25 +96,93 @@
                        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;


# svnlite diff /usr/src/sys/contrib/ | more
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)
@@ -357,9 +357,15 @@
        FDT_RW_CHECK_HEADER(fdt);
 
        offset = fdt_subnode_offset_namelen(fdt, parentoffset, name, namelen);
+#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.
        if (offset >= 0)
                return -FDT_ERR_EXISTS;
-       else if (offset != -FDT_ERR_NOTFOUND)
+       else
+#endif
+       if (offset != -FDT_ERR_NOTFOUND)
                return offset;
 
        /* Try to place the new node after the parent's properties */



===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)



More information about the freebsd-ppc mailing list