From owner-freebsd-ppc@freebsd.org Thu Apr 11 03:05:18 2019 Return-Path: Delivered-To: freebsd-ppc@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E338B1576074 for ; Thu, 11 Apr 2019 03:05:17 +0000 (UTC) (envelope-from marklmi@yahoo.com) Received: from sonic314-22.consmr.mail.ne1.yahoo.com (sonic314-22.consmr.mail.ne1.yahoo.com [66.163.189.148]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0B45270D68 for ; Thu, 11 Apr 2019 03:05:15 +0000 (UTC) (envelope-from marklmi@yahoo.com) X-YMail-OSG: mPxoA9kVM1nYQuQdkgwp1c7BZ5nlUD02pMA.BhGAl5g0o07qVDZBCIEHaqIol4V _o1tZKQrn63yTXA6jZjK9fK93Nz6RJvnizPSMfTrB78Q_3_2il9GzzOPa3t0d5o6J77ZdqGN2aVe WV3ze1ZlEjr_AiHhvvSwBm6JAPF78f8mEPN4U0VRQHZreobvWKEpu9fqloUFI1N4xb1FRCCRfUBU eRQW27vjnzZMwcFSj1T6xWM8mfqVpyR8K3_hXD7PPPtzJsbXoJxBvrkquL.WGnQRZ7lQo6Y._SoC ThekqE7CpZHJsww.uonGFCyBxb2kmw1Rnay_upzQjLroOxtWw1LPtI1Ssf1r.4gyVzy4K.EDcjFb AsANTvlEYMNLiXuf4CtAe4Psqnzo8buWe_obdT1OK3vVx46tSifQ90Azk8p5oPEWhf7b7IdIyOuK 2YtdsAiNm2n6EW3_f2oeFR1lEubx4.K8vumTyqxsXByHrD733W3jW8_jWWXeVCP4RgBUQ8RU5e8W r0phKK5ve498s_p4Vzx_Y28fHfwbqYRJPH6IfoQlokAEr1Ts4jSc5cfyXkcS2WnxPbVGRhkuosFr pkZXV6e9iU.9PRexw2w57V7_KrUe1CqC2J_wEzKXwn5VH1RlZBNOVAVSU8U.c79YIekA4TGFxfxG JVJYpI5oXHPm6B7138QcVXLeHrLqGI48ICUVcLnKfEgaFRCugAZIwXF8XvLDKGJ8WPsrb0HGXmQ_ Tj5CmQT7YUfo3l62.rIel5FZZDOyCdbjrKuGSbLvoFox5M1Tg_1QiQRW50Pmq_7QPt6bSef15i5z FPvorJuJGNhY.cLZVivCqDTmyRyDru1xj_7hp5U1_.DfUPiSbJ1XY3VQImstHvkqiWB_GdywKQDh 3FuQWoiPbmBfCwEzIYPmQESgKkOM05JQL6SqGykB_ShDvo8QJ3d_MGd_nE2BLKSFwlLbjXJnSRg_ gTycTzT8jBww5FkUde1ZmZd06l8d991NygMhld85lFOzqaIIp0uvpnvh8FBGQKXXQDVxNOR__1tE mYupftZx0WzIFR68cx.n5W1EOT0toDLzHbBA0t9XDKcofiSJdWHbPbg8ZJJWQ7yrCrV1Uv7d4uPV hDZxTOTW7Lv8- Received: from sonic.gate.mail.ne1.yahoo.com by sonic314.consmr.mail.ne1.yahoo.com with HTTP; Thu, 11 Apr 2019 03:05:08 +0000 Received: from c-76-115-7-162.hsd1.or.comcast.net (EHLO [192.168.1.103]) ([76.115.7.162]) by smtp402.mail.ne1.yahoo.com (Oath Hermes SMTP Server) with ESMTPA ID 7f153287cc54c7509045e836daa3743d; Thu, 11 Apr 2019 03:05:05 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.8\)) Subject: Re: head -r345758: usefdt=1 style boot fails on PowerMac7,2 G5 (1 core per socket): Error -2 adding node /cpus/PowerpC,970 [G4 failures too, investigatory patch] From: Mark Millard In-Reply-To: Date: Wed, 10 Apr 2019 20:05:02 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: <7383CCAA-FEB4-4CE6-ABEB-74781A18CCF4@yahoo.com> References: <98A19824-3C07-4ED6-A848-5A634F95E1CF@yahoo.com> <9A2F72B7-A25C-478C-B1AD-0661278F0B46@yahoo.com> <88C954A4-6D5A-4AB9-AA26-0ACD6D298605@yahoo.com> <5A26E6F0-13FB-4177-B284-45DA6FBED78E@yahoo.com> To: FreeBSD PowerPC ML , Justin Hibbits X-Mailer: Apple Mail (2.3445.104.8) X-Rspamd-Queue-Id: 0B45270D68 X-Spamd-Bar: ++ X-Spamd-Result: default: False [2.74 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_SPF_ALLOW(-0.20)[+ptr:yahoo.com]; MV_CASE(0.50)[]; FREEMAIL_FROM(0.00)[yahoo.com]; RCVD_COUNT_THREE(0.00)[3]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[yahoo.com:+]; MX_GOOD(-0.01)[cached: mta6.am0.yahoodns.net]; DMARC_POLICY_ALLOW(-0.50)[yahoo.com,reject]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_TLS_LAST(0.00)[]; FREEMAIL_ENVFROM(0.00)[yahoo.com]; ASN(0.00)[asn:36646, ipnet:66.163.184.0/21, country:US]; MID_RHS_MATCH_FROM(0.00)[]; DWL_DNSWL_NONE(0.00)[yahoo.com.dwl.dnswl.org : 127.0.5.0]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[yahoo.com:s=s2048]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; NEURAL_SPAM_SHORT(0.74)[0.743,0]; MIME_GOOD(-0.10)[text/plain]; IP_SCORE(1.63)[ip: (5.61), ipnet: 66.163.184.0/21(1.45), asn: 36646(1.16), country: US(-0.06)]; NEURAL_SPAM_MEDIUM(0.11)[0.112,0]; TO_MATCH_ENVRCPT_SOME(0.00)[]; NEURAL_SPAM_LONG(0.76)[0.763,0]; RCVD_IN_DNSWL_NONE(0.00)[148.189.163.66.list.dnswl.org : 127.0.5.0]; RWL_MAILSPIKE_POSSIBLE(0.00)[148.189.163.66.rep.mailspike.net : 127.0.0.17] X-BeenThere: freebsd-ppc@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Porting FreeBSD to the PowerPC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Apr 2019 03:05:18 -0000 [I show an investigatory patch and indicate its consequences. This ends up indicating what is not a "extra '\0's in paths problem".] On 2019-Apr-10, at 02:30, Mark Millard wrote: > On 2019-Apr-9, at 19:44, Mark Millard wrote: >=20 >> . . . >=20 > I discovered a specific PowerMac11,2 vs. PowerMac7,2/PowerMac3,6 > difference that is involved: >=20 > The difference is where nulls are vs. are not . . . >=20 > I found a linux comment (after the later path evidence was > observed): >=20 > /* Fixup an Apple bug where they have bogus \0 chars in = the > * middle of the path in some properties, and extract > * the unit name (everything after the last '/'). > */ >=20 > This was in the context of package-to-path use. >=20 > I have evidence of this (though not from OF_package_to_path use but > ofw_getprop_alloc use) . . . >=20 > The PowerMac11,2 has, for example, in the notation of my > dumping tool for looking at diff's of the (sorted) dumps: >=20 > /device-tree/cpus/PowerPC,G5/cpu-version: hex_bytes_line# 0: 00 44 01 = 01 > /device-tree/cpus/PowerPC,G5/cpu-version: txt_bytes_line# 0: = \000D\^A\^A > /device-tree/cpus/PowerPC,G5/cpu-version: hex_bytes_line# 0: 00 44 01 = 01 > /device-tree/cpus/PowerPC,G5/cpu-version: txt_bytes_line# 0: = \000D\^A\^A > /device-tree/cpus/PowerPC,G5/cpu-version: hex_bytes_line# 0: 00 44 01 = 01 > /device-tree/cpus/PowerPC,G5/cpu-version: txt_bytes_line# 0: = \000D\^A\^A > /device-tree/cpus/PowerPC,G5/cpu-version: hex_bytes_line# 0: 00 44 01 = 01 > /device-tree/cpus/PowerPC,G5/cpu-version: txt_bytes_line# 0: = \000D\^A\^A >=20 > But the PowerMac7,2 and PowerMac3,6 have a null character in the = analogous > prefix (produced by the same criteria), here shown with ^@: >=20 > /device-tree/cpus/PowerPC,970^@/cpu-version: hex_bytes_line# 0: 00 39 = 02 02 > /device-tree/cpus/PowerPC,970^@/cpu-version: txt_bytes_line# 0: = \0009\^B\^B > /device-tree/cpus/PowerPC,970^@/cpu-version: hex_bytes_line# 0: 00 39 = 02 02 > /device-tree/cpus/PowerPC,970^@/cpu-version: txt_bytes_line# 0: = \0009\^B\^B >=20 > /device-tree/cpus/PowerPC,G4^@/cpu-version: hex_bytes_line# 0: 80 01 = 03 03 > /device-tree/cpus/PowerPC,G4^@/cpu-version: txt_bytes_line# 0: = \M^@\^A\^C\^C > /device-tree/cpus/PowerPC,G4^@/cpu-version: hex_bytes_line# 0: 80 01 = 03 03 > /device-tree/cpus/PowerPC,G4^@/cpu-version: txt_bytes_line# 0: = \M^@\^A\^C\^C >=20 > The code that produces a name for a ofw node, as shown after a / = above, > is (C++17 notation): >=20 > auto name_for_ofw_node=3D [&ofw_fd](auto ofw_nd) -> auto > { > std::string nd_id_text{}; >=20 > void* name_buf=3D nullptr; > int name_buf_len=3D 0; > auto const name_len=3D = ofw_getprop_alloc(ofw_fd,ofw_nd,"name",&name_buf,&name_buf_len,1); > if (0 nd_id_text+=3D std::string{static_cast(name_buf), static_cast(name_len-1)}; > else > { > . . . (does not happen) . . . > } >=20 > return nd_id_text; > }; >=20 > [With name_len instead of name_len-1 there would be one more '\0' > character in each such name after a / (before the first ":"): > the terminating null character would be included.] >=20 > If such a before-the-end-of-path ^@ shows up in the likes of > add_node_to_fdt via its use of OF_package_to_path: >=20 > char name[255], *lastprop, *subname; > . . . > for (node =3D OF_child(node); node > 0; node =3D OF_peer(node)) = { > OF_package_to_path(node, name, sizeof(name)); > subname =3D strrchr(name, '/'); > subname++; > child_offset =3D fdt_add_subnode(buffer, fdt_offset, = subname); > if (child_offset < 0) { > printf("Error %d adding node %s (%s), = skipping\n", > child_offset, name, subname); > continue; > } >=20 > then the strrchr will not work as intended and the error message > will result (as it does for PowerMac7,2/PowerMac3,6). >=20 > I'll note that the return value of OF_package_to_path is ignored > above but the description I find is: >=20 > QUOTE > package-to-path > IN: phandle, [address] buf, buflen OUT: length >=20 > Returns the fully qualified pathname corresponding to the node = identifier phandle, storing, at most, buflen bytes as a null-terminated = string in the memory buffer starting at the address buf. If the length = of the null- terminated pathname is greater than buflen, the trailing = characters and the null terminator are not stored. Length is the length = of the fully qualified pathname excluding any null terminator, or =E2=80=93= 1 if phandle is invalid. > END QUOTE >=20 > The linux code does use the return value in order to not be fooled by > null characters in the middle of the path. The following investigatory patch prevents some of the "Error -2 adding node . . ." notices. It: A) makes no change for the PowerMac11,2 G5 (2 sockets, 2 cores each) B) eliminates the message for PowerMac7,2 G5 (2 sockets, 1 core each), eliminated is: Error -2 adding node /cpus/PowerPC,970 (PowerPC,970), = skipping C) eliminates one message for PowerMac3,6 G4 (2 sockets, 1 core each), = the one is: Error -2 adding node /cpus/PowerPC,G4 (PowerPC,G4), skipping For (A): Apparently extra '\0's is not the reason the PowerMac11,2 gets one such message. I do not know why it does. Both powerpc64 and 32-bit powerpc FreeBSD still report: Error -2 adding node /ht@0,f2000000/pci@8/mac-io@7/i2c@18000/i2c-bus@0 = (i2c-bus@0), skipping For (B): Eliminating the message is still followed by the boot hanging up after "Kernel entry at . . .". It does not get as far as clearing the console screen. Something else is going on for this. The powerpc64 and 32-bit powerpc FreeBSD results are the same. But I've no clue how to isolate it. (Booting without usefdt mode works fine.) For (C): Both CPUs are now used in usefdt mode but ethernet is still not present. The messages still generated are: Error -2 adding node /pci@f2000000//mac-io@17/gpio@50/gpio5@6f = (gpio5@6f), skipping Error -2 adding node /pci@f2000000//mac-io@17/gpio@50/gpio6@70 = (gpio6@70), skipping Error -2 adding node /pci@f2000000//mac-io@17/gpio@50/gpio11@75 = (gpio11@75), skipping Error -2 adding node /pci@f2000000//mac-io@17/extint-gpio@15@67 = (extint-gpio@15@67), skipping While the patch is justified by Macintosh problems, it should be valid for openfirmware that has no "extra '\0' character" problems in paths. My only testing environment is the old PowerMacs, however. The patch has add_node_to_fdt eliminate the extra/internal '\0' characters in paths/names it creates, instead of preserving them carefully. Why? If preserved, lots of other code seems to need to be modified to deal with them. Another type of alternative would have been to replace the '\0' characters with some other, say '_'. (I'm not aware of a reason that name and path lengths would need to be preserved.) The patch is: (I do not claim to have coded for direct acceptance into FreeBSD's code base: investigatory material.) Index: /usr/src/stand/powerpc/ofw/ofwfdt.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- /usr/src/stand/powerpc/ofw/ofwfdt.c (revision 345758) +++ /usr/src/stand/powerpc/ofw/ofwfdt.c (working copy) @@ -45,7 +45,7 @@ add_node_to_fdt(void *buffer, phandle_t 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; =20 @@ -77,9 +77,54 @@ && !OF_hasprop(node, "ibm,phandle")) fdt_setprop(buffer, fdt_offset, "phandle", &node, = sizeof(node)); =20 + // 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<=3Dfull_str_len . + // For full_str_len 0; node =3D OF_peer(node)) = { - OF_package_to_path(node, name, sizeof(name)); - subname =3D strrchr(name, '/'); + int full_str_len=3D OF_package_to_path(node, name, = sizeof(name)-1); // Avoids having trailing '\0' missing. + if (-1=3D=3Dfull_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 (before what will be subname)! + + // 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=3D=3Dfull_str_len) { // Highly unlikely. + printf("Error: Node name has no bytes before = trailing null byte\n"); + continue; + } + if (255