Bug 233863 - Various PowerMac G5 models may require kern.smp.disabled=1 and must set usefdt=1 which causes net interface reorder
Summary: Various PowerMac G5 models may require kern.smp.disabled=1 and must set usefd...
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: powerpc Any
: --- Affects Some People
Assignee: freebsd-ppc mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-08 05:42 UTC by Dennis Clarke
Modified: 2019-05-24 01:52 UTC (History)
3 users (show)

See Also:


Attachments
dmesg output from r341705 on ppc64 with kern.smp.disabled=1 and usefdt=1 (92.54 KB, text/plain)
2018-12-08 05:42 UTC, Dennis Clarke
no flags Details
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches (7.29 KB, patch)
2019-04-12 21:17 UTC, Mark Millard
no flags Details | Diff
Patches for investigatory narrowing of slb race on AIM powerpc64: aim/mp_cpudep.c and aim/slb.c (hack that avoids a mis-handled slb-miss) (1.72 KB, patch)
2019-04-12 22:57 UTC, Mark Millard
no flags Details | Diff
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches (7.05 KB, patch)
2019-04-13 20:46 UTC, Mark Millard
no flags Details | Diff
Investigatory sys/powerpc/powermac/hrowpic.c sys/powerpc/powermac/uninorth.c sys/powerpc/powerpc/openpic.c patches (OF_xref_from_node and OF_node_from_xref use) (1.98 KB, patch)
2019-04-15 07:01 UTC, Mark Millard
no flags Details | Diff
Investigatory sys/powerpc/powerpc/mp_machdep.c patch to avoid stuck-sleeping problem (1.05 KB, patch)
2019-04-20 03:06 UTC, Mark Millard
no flags Details | Diff
Investigatory sys/powerpc/powerpc/mp_machdep.c patch to help limit stuck-sleeping problem (1.07 KB, patch)
2019-04-20 05:17 UTC, Mark Millard
no flags Details | Diff
Investigatory patch for sys/powerpc/aim/mmu_oea64.c (1.25 KB, patch)
2019-04-20 23:24 UTC, Mark Millard
no flags Details | Diff
Investigatory patch for sys/powerpc/aim/mmu_oea64.c (for filling translations[] from trans_cells[]) (1.05 KB, patch)
2019-04-20 23:33 UTC, Mark Millard
no flags Details | Diff
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches (8.43 KB, patch)
2019-04-22 09:55 UTC, Mark Millard
no flags Details | Diff
Investigatory /sys/powerpc/aim/trap_subr32.S patch (add missing isync's) (618 bytes, patch)
2019-04-24 19:51 UTC, Mark Millard
no flags Details | Diff
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches (9.18 KB, patch)
2019-04-24 20:07 UTC, Mark Millard
no flags Details | Diff
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches, preserving peer order, handling Apple oddities so information is preserved (9.41 KB, patch)
2019-04-24 21:13 UTC, Mark Millard
no flags Details | Diff
Patch for aim/mp_cpudep.c that fixes slb-miss problem in cpudep_ap_bootstrap for PPC970/PowerMacG5 contexts (3.34 KB, patch)
2019-05-10 10:47 UTC, Mark Millard
no flags Details | Diff
Mostly: Investigatory patch for sys/powerpc/aim/mmu_oea64.c (for filling translations[] from trans_cells[]) (1.46 KB, patch)
2019-05-14 02:38 UTC, Mark Millard
no flags Details | Diff
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches, preserving peer order, handling Apple oddities so information is preserved (11.58 KB, patch)
2019-05-14 02:47 UTC, Mark Millard
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dennis Clarke 2018-12-08 05:42:47 UTC
Created attachment 199947 [details]
dmesg output from r341705 on ppc64 with kern.smp.disabled=1 and usefdt=1

As discovered while working on bugid 233579 we require usefdt=1 as well 
as kern.smp.disabled=1 in order for the system to boot. The unit will
then boot normally however network devices are status "no carrier".
Also the devices are listing in a different order from RC3 r341271.

See dmesg attached.
Comment 1 Dennis Clarke 2018-12-09 22:33:33 UTC
It was seen that the physical MAC addresses of the bge0 and bge1
interfaces were exchanged in the output from 'ifconfig -a' and thus the
flat device tree option usefdt=1 may be the cause. 

Once this was seen it was trivial to bring up both interfaces and to
assign the correct ip to bge1 and then stop start ntpd and sshd etc.

eris# uname -aU
FreeBSD eris 13.0-CURRENT FreeBSD 13.0-CURRENT r341705 GENERIC  powerpc 1200086
eris# ifconfig -a 
fwe0: flags=8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=8<VLAN_MTU>
        ether 02:11:24:e5:13:d0
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
        ch 1 dma -1
bge0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=8009b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM,LINKSTATE>
        ether 00:14:51:64:67:11
        inet 172.16.35.8 netmask 0xffffffc0 broadcast 172.16.35.63 
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
        media: Ethernet autoselect (none)
        status: no carrier
bge1: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=8009b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM,LINKSTATE>
        ether 00:14:51:64:67:10
        inet 172.16.35.7 netmask 0xffffffc0 broadcast 172.16.35.63 
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
        media: Ethernet autoselect (1000baseT <full-duplex,master>)
        status: active
lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> metric 0 mtu 16384
        options=680003<RXCSUM,TXCSUM,LINKSTATE,RXCSUM_IPV6,TXCSUM_IPV6>
        inet6 ::1 prefixlen 128 
        inet6 fe80::1%lo0 prefixlen 64 scopeid 0x4 
        inet 127.0.0.1 netmask 0xff000000 
        nd6 options=21<PERFORMNUD,AUTO_LINKLOCAL>
        groups: lo 
eris#
Comment 2 Dennis Clarke 2018-12-09 23:04:31 UTC
As yet another variable in the mix we see all four cpu's are online
if and only if one performs are warm boot aka shutdown -r 'now' : 

1) power on system and wait for loader
2) interrupt the load kernel process
3) set kern.smp.disabled=1 
4) set usefdt=1
5) boot -sv
6) once the shell is running simply shutdown -r 'now'
7) interrupt the load kernel process
8) set usefdt=1
9) boot normally 

All processor cores wwill come online : 


eris# uname -aU
FreeBSD eris 13.0-CURRENT FreeBSD 13.0-CURRENT r341705 GENERIC  powerpc 1200086

eris# uptime
11:04PM  up 15 mins, 1 user, load averages: 0.18, 0.21, 0.17

eris# sysctl kern.smp.cpus hw.ncpu 
kern.smp.cpus: 4
hw.ncpu: 4
Comment 3 Mark Millard 2018-12-10 04:30:31 UTC
(In reply to Dennis Clarke from comment #2)

Just an FYI about my from-source builds based on devel/powerpc64-xtoolchain-gcc
and the reverted-to-before /usr/src/sys/powerpc/include/vmparam.h -r334498 .

I jumped from -r340287 to -r341766 . It booted fine as smp.

I tried booting with "set usefdt=1" and the ethernet connection moved to
the other plugin position. It also booted fine as smp and the ethernet
works fine.

[I have occasional access to a G5 Quad Core (system total count) again.]
Comment 4 Mark Millard 2018-12-10 08:55:48 UTC
(In reply to Mark Millard from comment #3)

It appears that one difference I get with set usefdt=1 is
that for shutdown -r or -p bufdeamon, bufspacedaemon-1,
and bugspacedaemon-2 time out.

Another difference may be that the fans may be more likely
to go full blast.
Comment 5 Dennis Clarke 2019-03-24 11:41:51 UTC
123456789+123456789+123456789+123456789+123456789+123456789+123456789+12

This feels like the bug that simply won't go away however I did a test
build of r345425 and no amount of coaxing nor kern.smp.disabled would
get past fans roaring after seeing a wake up call to CPU3. A step back
to r344744 seems to work fine provided that one provide the usefdt=1
and then boot -sv :

hydra$ uname -a 
FreeBSD hydra 13.0-CURRENT FreeBSD 13.0-CURRENT r344744 GENERIC  powerpc
hydra$ sysctl -a kern.smp.cpus
kern.smp.cpus: 4
hydra$ 

Also bge0 and bge1 network interfaces are reversed however this is 
merely an annoyance : 

bge0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=8009b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM,LINKSTATE>
        ether 00:14:51:64:67:11
        inet 172.16.35.8 netmask 0xffffffc0 broadcast 172.16.35.63
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
        media: Ethernet autoselect (1000baseT <full-duplex,master>)
        status: active
bge1: flags=8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=8009b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM,LINKSTATE>
        ether 00:14:51:64:67:10
        nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>
        media: Ethernet autoselect

hydra$ date -u
Sun Mar 24 11:41:28 UTC 2019

-- 
Dennis Clarke
RISC-V/SPARC/PPC/ARM/CISC
UNIX and Linux spoken
GreyBeard and suspenders optional
Comment 6 Mark Millard 2019-03-24 19:10:14 UTC
(In reply to Dennis Clarke from comment #5)

The following notes are currently based on variations on head -r335044 .
It has usefdt=1 enabled always (by changing the code) but does not have
smp disabled (even manually). The context does not involve reverting the
newer kernel maximum figure that I used to revert.

I have a hack that avoid the fans problem on the 2-socket/2cores-each G5
that I currently have access to. It also avoids problems with other
processes that also get stuck waiting for sleep. But it is a hack that is
not even approximately a general/proper solution to the original problem,
it just avoids the consequences. The hack involves an experimentally
found approximate bound that it tests against. That bound might vary
for related G5 contexts.

I've another hack that makes it rare that booting gets hung up: it
narrows a race condition's time frame. But I've have observed a few
examples of hang-up out of hundreds of boots. The hack basically
proves what is going on for the hang-ups based on the large change of
behavior for narrowing the race-condition's time frame.

There is a patch-review active for llvm's libunwind that makes it
handle r2 (the TOC register) correctly for powerpc64 both for
system-clang based buildworld's and devel/powerpc64-gcc based
buildworld's, both ELFv1 ABI and ELFv2 ABI. This makes thrown C++
exceptions work. (This does not cover WITH_LIB32= for exception
handling when WITH_LLVM_LIBUNWIND= is in use in the buildworld.)
Comment 7 Mark Millard 2019-04-12 21:17:37 UTC
Created attachment 203622 [details]
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches

I was asked to make these patches available here.

They are, in part, tied to working around old
PowerMac openfirmware-node-tree oddities, while not
invalidating contexts that follow standards better
They are also tied to FreeBSD-code-assumptions about
relative node positions in the PowerMac trees that
the prior openfirmware->fdt translation did not
preserve.

Be warned that nothing I've done has allowed the
2-socket/1-core-each PowerMac7,2 example to boot to
any visible activity after the "Kernel entry at"
message in usefdt mode. The improvements were
observed on a 2-socket/1-core-each G4 PowerMac3,6
and a 2-socket/2-cores-each G5 PowerMac11,2 .

Other hacks and patches are involved in my
environment but they are not included here.

The 2 patches here are not coded in a form for
acceptable check-in to FreeBSD's code base. As
stands, they are just tools for use in my
investigations. They might form the basis for
official patches if the relevant folks classify
direction of the changes appropriate.

See https://lists.freebsd.org/pipermail/freebsd-ppc/2019-April/010268.html
and any later messages for more context.
Comment 8 Mark Millard 2019-04-12 22:57:58 UTC
Created attachment 203627 [details]
Patches for investigatory narrowing of slb race on AIM powerpc64: aim/mp_cpudep.c and aim/slb.c (hack that avoids a mis-handled slb-miss)

This pair of patches narrows the time period over which threads
from the stages:

        SI_SUB_KTHREAD_INIT     = 0xe000000,    /* init process*/
        SI_SUB_KTHREAD_PAGE     = 0xe400000,    /* pageout daemon*/
        SI_SUB_KTHREAD_VM       = 0xe800000,    /* vm daemon*/
        SI_SUB_KTHREAD_BUF      = 0xea00000,    /* buffer daemon*/
        SI_SUB_KTHREAD_UPDATE   = 0xec00000,    /* update daemon*/
        SI_SUB_KTHREAD_IDLE     = 0xee00000,    /* idle procs*/
#ifndef EARLY_AP_STARTUP
        SI_SUB_SMP              = 0xf000000,    /* start the APs*/
#endif 

can conflict with starting an AP via an slb replacement position
picked via expressions like mftb()%n_slbs . It does this by explicitly
picking and setting up a slot just before starting the AP.

(The AP has to be part way along before it can do its own
auto-slb-replacements from what I can tell.)

This does not remove the race and still does sometimes fail to
prevent getting a hang-up on a AP start. BUt it greatly decreased
the rate of hangups in my testing.

If EARLY_AP_STARTUP was supported and used, this would not be a
problem.
Comment 9 Mark Millard 2019-04-13 03:52:39 UTC
(In reply to Mark Millard from comment #7)

One thing I've noticed with the openfirmware->fdt patches
is that the PowerMac11,2 in usefdt mode does not turn off
the power for "shutdown -p now".

(I do not know about 7,2 or 3,6 at this point.)
Comment 10 Mark Millard 2019-04-13 07:44:25 UTC
(In reply to Mark Millard from comment #9)

I tried:

shutdown -r now

and it does not reboot.
Comment 11 Mark Millard 2019-04-13 18:40:22 UTC
(In reply to Mark Millard from comment #10)

The disabling of blocking duplicate paths in fdt_add_subnode_namelen
was done incorrectly. I'll replace the attachment after building
and testing. I think this is the explanation for the PowerMac11,2
shutdown -r or -p problems.

The code should have just disabled the return, more like:

        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;

Instead the messed up change did the "return offset;" and
so did not do the addition of the node, instead returning
the pre-existing one to be manipulated.
Comment 12 Mark Millard 2019-04-13 20:40:04 UTC
Comment on attachment 203622 [details]
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches

The change to sys/contrib/libfdt/fdt_rw.c was wrong.
I'll add a corrected attachment.
Comment 13 Mark Millard 2019-04-13 20:46:01 UTC
Created attachment 203652 [details]
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches

This fixes the patch to /usr/src/sys/contrib/libfdt/fdt_rw.c
for allowing identical package-to-pathname results since
Macintoshes have such.
Comment 14 Mark Millard 2019-04-13 20:48:29 UTC
(In reply to Mark Millard from comment #13)

The corrected fix lead to shutdown -r and -p working
on the PowerMac11,2 example for usefdt mode.
Comment 15 Mark Millard 2019-04-15 07:01:01 UTC
Created attachment 203683 [details]
Investigatory sys/powerpc/powermac/hrowpic.c sys/powerpc/powermac/uninorth.c sys/powerpc/powerpc/openpic.c patches (OF_xref_from_node and OF_node_from_xref use)

Besides the other patches, adding 2 instances of using OF_xref_from_node
and 1 instance of using OF_node_from_xref has finally enabled also booting
the 2-socket/1-core-each G5 PowerMac7,2 in usefdt mode.
Comment 16 Mark Millard 2019-04-15 08:27:57 UTC
(In reply to Mark Millard from comment #15)

I should have noted that for the PowerMac7,2 configuration involved,
booting with kern..vty=vt instead of kern.vty=sc is required.

For scons ( kern.vty=sc ) the display stops updating just after the
"Kernel entry at . . ." message, before even it clears the screen.
Comment 17 Mark Millard 2019-04-20 03:06:28 UTC
Created attachment 203812 [details]
Investigatory sys/powerpc/powerpc/mp_machdep.c patch to avoid stuck-sleeping problem

So far in preliminary testing on the PowerMac11,2, avoiding
the hardware priority change from "or 31,31,31" use when looping
to find ap_letgo changed has avoided my hack reporting any cases
of applying the workaround. (Nor does the code now do a
"or 6,6,6" to put back the orginal hardware priority.)

I changed things to be more like sequentially consistent handling
and made the bsp slightly more like the APs in hopes of getting
more similar timing to when platform_smp_timebase_sync happens.

I changed code in machdep_ap_bootstrap and in cpu_mp_unleash .

I've also tested a 2-socket/1-core-each 970 G5 PowerMac11,2 a
little. It still booted fine and still has never reported the
workaround ever having to been applied. I will build 32-bit
powerpc FreeBSD and try it as well, including on a
2-socket/1-core-each 7455 G4 PowerMac3,6.

I'm actually not surprised that only the multi-core context
actually behaves differently for "or 31,31,31" use (setting
lowest priority mode): I do not see that being something that
happens across sockets but only more locally.
Comment 18 Mark Millard 2019-04-20 05:17:36 UTC
Created attachment 203814 [details]
Investigatory sys/powerpc/powerpc/mp_machdep.c patch to help limit stuck-sleeping problem

Justin Hibbits reported that the "or 31,31,31" and "or 6,6,6"
could be left in (mixed with my other changes) and the 970MP's
would not change their boot behavior for the PwerMac11,2.

I tried it and he seems to be correct based on a little
quick testing.

This might avoid needing palatform-specific code for handling
the ap_letgo behavior.
Comment 19 Mark Millard 2019-04-20 23:24:02 UTC
Created attachment 203844 [details]
Investigatory patch for sys/powerpc/aim/mmu_oea64.c

I had forgotten my old 2019-Jan-29/30 reports and patch tied
to a debug-build KASSERT failure on a old PowerMac.

See: https://lists.freebsd.org/pipermail/freebsd-ppc/2019-January/010020.html

(the Jan-30 udpate).

It was tied to activity like:

unload
load /boot/kernel/kernel
boot

or:

unload
boot -v /boot/kernel/kernel

for seeing the specific KASSERT examples that I got.

That justified looking into the code, but looking seemed
to indicate the more general problems noted on the list.
I'll not repeat the material here.
Comment 20 Mark Millard 2019-04-20 23:33:08 UTC
Created attachment 203845 [details]
Investigatory patch for sys/powerpc/aim/mmu_oea64.c (for filling translations[] from trans_cells[])

The original paste of attachment content (from the original
time frame) did not seem to have preserved whitespace details.
So I'm just trying again via a modern svnlite diff.
Comment 21 Mark Millard 2019-04-21 02:33:09 UTC
(In reply to Mark Millard from comment #8)

I went back through and my notes in comment #8 are wrong
about what the slb mismatches are from.

I apparently confused some prior context with the relevant
one.

The patch is still relevant and appropriate for the actual
cases of needing a slb entry.
Comment 22 Mark Millard 2019-04-21 02:36:54 UTC
One thing I'm not sure of for usefdt-mode is what
the consequences of small memory machines might be.
/usr/src/stand/powerpc/ofw/ofwfdt.c has:

int
fdt_platform_load_dtb(void)
{
        void *buffer;
        size_t buflen = 409600;

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

        fdt_load_dtb_addr(buffer);
        free(buffer);

        return (0);
}

On a 1 GiByte machine, with some other overhead
use of RAM, that 409600 could be around half the
available memory --or more. Even though
fdt_load_dtb_addr will make a smaller copy, and
then the bigger one will be freed, the copy's
address range is constrained by the existing big
allocation at the time.

On a sufficiently small memory machine the 409600
would just not be possible. (That might even be
true for a 1 GiByte machine.) Note the lack of
any check on the malloc(409600)'s success vs.
failure status. (By contrast, fdt_load_dtb_addr
does check for a NULL return, even though
fdt_platform_load_dtb ignores the fdt_load_dtb_addr
return value.)

Nothing that I've done so far attempts to deal with
such issues. The behavior for insufficient problems
is probably a mess and it may not be obvious that
any odd behavior is tied to insufficient RAM for the
existing implementation.
Comment 23 Mark Millard 2019-04-21 05:16:48 UTC
(In reply to Mark Millard from comment #20)

The original rejection by a debug build that lead to
the discovery of out of bounds access was tied to the
original conversion to fdt code truncating the translation
property via:

		if (proplen > 1024) {
 			proplen = 1024;
		}

in add_node_to_fdt in stand/powerpc/ofw/ofwfdt.c .
This changed a 1040==208*5 total to a 1024==256*4
total. (1024 is not a multiple of 5.)

So the problem goes away when the truncation logic
is removed, which is part of what my patches do.

Still, the truncation did expose some coding problems in
the translation map extraction, such as out of bounds access
for such a truncated case. But it would take some forced
bad property size to see the problem again if mmu_oea64.c is
not patched.

The change to the KASSERT in my patch may well be inappropriate,
given the above context that is now known.

Having an incomplete set of translations does not seem like
an appropriate thing: so the truncation to 1024 needs to be
avoided.
Comment 24 Mark Millard 2019-04-22 03:27:15 UTC
I did some experimenting on a 1 GiByte iMac G3 with
the SSD that I've been using for the 32-bit powerpc
FreeBSD testing with these patches.

For non-usefdt mode, it seemed to boot and operate
okay for the little bit I did with the iMac G3.

But for usefdt mode, towards the end of booting or
not all that long after logging in, it crashed. The
details were not stable for where or the type of
panic.

What was sort of consistent was moea_* involvement,
despite just-what varying:

moea_pvo_find_va (via moea_is_prefaultable)
moea_pvo_to_pte
moea_pvo_enter

Unfortunately, I happened to have a non-debug kernel
context at the time. If I try again, it will be with
a debug kernel. The swap/page space was rather large
for the RAM. I could try with no swap enabled.
Comment 25 Mark Millard 2019-04-22 06:06:37 UTC
(In reply to Mark Millard from comment #24)

As for the usefdt mode G3 exploration:

The debug kernel did not report anything special.
Disabling swap made no general change to the
behaviors.

An interesting comparison is:

Under the debug kernel I got a machine check, reported
to be at the end of:

<moea_pvo_enter+0xbc> bl      00503bf0 <__mtx_lock_flags>
<moea_pvo_enter+0xc0> rlwinm  r26,r21,2,0,29
<moea_pvo_enter+0xc4> lwz     r9,-32748(r30)
<moea_pvo_enter+0xc8> lwz     r9,0(r9)
<moea_pvo_enter+0xcc> lwzx    r29,r26,r9
<moea_pvo_enter+0xd0> cmpwi   cr7,r29,0
<moea_pvo_enter+0xd4> beq-    cr7,008e9c8c <moea_pvo_enter+0x19c>
<moea_pvo_enter+0xd8> lwz     r0,52(r29)
<moea_pvo_enter+0xdc> cmpw    cr7,r0,r28

(But machine-checks are imprecise.)

Under a non-debug kernel I got a Data Storage Interrupt,
reported to be at the end of:

<moea_pvo_enter+0x110> bl      0054da44 <__mtx_lock_sleep>
<moea_pvo_enter+0x114> rlwinm  r26,r21,2,0,29
<moea_pvo_enter+0x118> lwz     r9,-32744(r30)
<moea_pvo_enter+0x11c> lwz     r9,0(r9)
<moea_pvo_enter+0x120> lwzx    r29,r26,r9
<moea_pvo_enter+0x124> cmpwi   cr7,r29,0
<moea_pvo_enter+0x128> beq-    cr7,0098ef74 <moea_pvo_enter+0x244>
<moea_pvo_enter+0x12c> lwz     r0,52(r29)

Both of these are tied to:

	mtx_lock(&moea_table_mutex);
	LIST_FOREACH(pvo, &moea_pvo_table[ptegidx], pvo_olink) {
		if (pvo->pvo_pmap == pm && PVO_VADDR(pvo) == va) {

with the "lwz r0,52(r29)" appearing to be the access for pvo->pvo_pmap
from what I can tell. In turn it suggests that, on the G3, values for
pvo result that are inappropriate. (Not that I have a clue why.)
Comment 26 Mark Millard 2019-04-22 07:36:23 UTC
(In reply to Mark Millard from comment #25)

Hmm. DSI at moea_pvo_find_va+0xec (non-debug
kernel example):

<moea_pvo_find_va+0xd0> bl      0054da44 <__mtx_lock_sleep>
<moea_pvo_find_va+0xd4> rlwinm  r11,r26,2,0,29
<moea_pvo_find_va+0xd8> lwz     r9,-32744(r30)
<moea_pvo_find_va+0xdc> lwz     r9,0(r9)
<moea_pvo_find_va+0xe0> lwzx    r29,r11,r9
<moea_pvo_find_va+0xe4> cmpwi   cr7,r29,0
<moea_pvo_find_va+0xe8> beq-    cr7,0098a71c <moea_pvo_find_va+0x150>
<moea_pvo_find_va+0xec> lwz     r0,52(r29)

which looks like:

	mtx_lock(&moea_table_mutex);
	LIST_FOREACH(pvo, &moea_pvo_table[ptegidx], pvo_olink) {
		if (pvo->pvo_pmap == pm && PVO_VADDR(pvo) == va) {

It appears that the problem is seen at a common type of
code structure that is not factored out.
Comment 27 Mark Millard 2019-04-22 09:55:22 UTC
Created attachment 203889 [details]
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches

stand/powerpc/ofw/ofwfdt.c is updated because:

A) ofwfdt_fixups had the odd mix for context handling:

G3's/G4's (4,1 and 3,6 PowerMac examples) eliminated /memory@0/available
but. . .
G5's (7,2 and 11,2 examples) did not eliminate /memory0,0/available

Now it does both.

B) fdt_platform_load_dtb did no checking of return status values
or reporting of its own of such failures.


Note: sys/contrib/libfdt/fdt_rw.c is unchanged.
Comment 28 Mark Millard 2019-04-23 16:38:40 UTC
(In reply to Mark Millard from comment #26)

More iMag G3 (MPC750) notes . . .

In the non-debug kernel __mtx_lock_flags was inlined
and that code normally leads to __mtx_lock_sleep not
being called in the failures.

My machine check and data store interrupt notes were not
meant to imply debug always gets one or that non-debug
always gets the other.

The following seems to have eliminated the machine check
type of error, turning them all into data storage interrupts
so far:

Index: /usr/src/sys/powerpc/aim/trap_subr32.S
===================================================================
--- /usr/src/sys/powerpc/aim/trap_subr32.S	(revision 345758)
+++ /usr/src/sys/powerpc/aim/trap_subr32.S	(working copy)
@@ -68,7 +68,7 @@
	lwzu	sr,PM_SR(pmap); \
	RESTORE_SRS(pmap,sr) \
	/* Restore SR 12 */ \
-	lwz	sr,12*4(pmap);	mtsr	12,sr
+	lwz	sr,12*4(pmap);	mtsr	12,sr; isync

/*
 * Kernel SRs are loaded directly from kernel_pmap_
@@ -799,6 +799,7 @@
	mfmsr	%r3
	andi.	%r3,%r3,~PSL_EE@l
	mtmsr	%r3
+	isync
/* Test AST pending: */
	lwz	%r5,FRAME_SRR1+8(%r1)
	mtcr	%r5

But the general failures remain. The above (or something
like it) may be necessary but is not sufficient.
Comment 29 Mark Millard 2019-04-24 19:51:05 UTC
Created attachment 203980 [details]
Investigatory /sys/powerpc/aim/trap_subr32.S patch (add missing isync's)

This has avoided getting some Machine Checks instead of
Data Storage Interrupts in the MPC750 iMac G4 PowerMac4,1.
Of itself it does not make the iMac G3 example usable.

The PowerPC docuemntation indicates the MPC750 need for
an isync after an mtsr. I also found a reference for
needing one after mtmsr for the PSL_EE case. These are
special to the MPC750's instead of architectural for
32-bit.

There is other code around with mtsr and mtmsr with
PDL_EE involved that used isync's. It looks like these
were just missed.
Comment 30 Mark Millard 2019-04-24 20:07:52 UTC
Created attachment 203981 [details]
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches

The MPC750 iMac G4 PowerMac4,1 needs to have its
/memory0/available information used and respected
or the "extra" address range usage will end up
trashed. openfirmware may well report to exclude
some address ranges having nothing to do with
openfirmware's internal memory use.

With this change the MPC750 iMac G3 PowerMac4,1
example boots and runs in *BOTH* non-usefdt mode
and usefdt mode.

It also means that every PowerMac that I have access
to that non-usefdt mode historically worked on now
also works for *BOTH* non-usefdt mode and usefdt mode:
G3, G4, and G5.

Textually the patches would need adjustments to be
acceptable for FreeBSD check-in. Also it is possible
that some things might have other directions things
could instead be taken. But they give solid evidence
about what needs to be addressed.

Note:

There are list messages with notes about other things
I ran into while trying to find what would make things
work again for both non-usefdt mode and usefdt mode.
From what I observe, none are important for this bugzilla
(beyond what the various attached patches deal with).
Comment 31 Mark Millard 2019-04-24 20:15:44 UTC
(In reply to Mark Millard from comment #30)

The patches are still based on -r345758 and some of
them likely would not apply cleanly/appropriately to
sufficiently more recent vintages of head.

An example is the code still using the old ori 31,31,31
and ori 6,6,6 : there is now a call interface and the
6,6,6 as the default was changed.
Comment 32 Mark Millard 2019-04-24 21:13:35 UTC
Created attachment 203983 [details]
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches, preserving peer order, handling Apple oddities so information is preserved

I noticed some whitespace oddities in the ofwfdt.c
update's indentation and so adjusted them.

No other type of change intended.
Comment 33 Mark Millard 2019-04-25 01:07:05 UTC
(In reply to Mark Millard from comment #28)

Justin Hibbits checked in the the sys/powerpc/aim/trap_subr32.S
patch as head -r346619 .
Comment 34 Mark Millard 2019-04-25 01:17:56 UTC
(In reply to Mark Millard from comment #18)

Note: I've been testing without my hack for the sleep-gets-stuck
problem for some days, mostly in usefdt mode.

I've finally had pmac_thermal end up stuck-sleeping during a
from-scratch buildworld buildkernel on the 2-socket/1-core-each
G5 PowerMac11,2 running in usefdt mode.

So the improvement in mp_machdep.c for better matching the Time
Base Registers across CPUs/cores/threads still is not enough to
guarantee lack of some sleeps getting stuck.

(Without the hack I've no recording of about how far out of order
the problem times where.)

(I may end up running with my hack generally.)
Comment 35 Mark Millard 2019-04-25 01:33:10 UTC
(In reply to Mark Millard from comment #34)

Dumb typo in comment #34: it should have said there were
2 cores per socket on the machine were I saw the
stuck-sleeping problem for pmac_thermal . . .

2-socket/2-core-each G5 PowerMac11,2 running in usefdt mode
Comment 36 Mark Millard 2019-05-10 10:47:25 UTC
Created attachment 204307 [details]
Patch for aim/mp_cpudep.c that fixes slb-miss problem in cpudep_ap_bootstrap for PPC970/PowerMacG5 contexts

No longer is aimslb.c involved.

More than the 970 might be appropriate for
similar changes, but as stands this is a PPC970
specific fix that makes the slbtrap/handle_kernel_slb_spill
code work in the cpudep_ap_bootstrap time frame.
It basically just moves initialization of HIOR, HID0,
and HID1 to earlier, before cpudep_ap_bootstrap
is called.

The patch was originally developed on head -r347003
without the patches from this bugzilla (or others
that I have in/from my investigations).
Comment 37 commit-hook freebsd_committer 2019-05-10 19:36:24 UTC
A commit references this bug:

Author: jhibbits
Date: Fri May 10 19:36:15 UTC 2019
New revision: 347463
URL: https://svnweb.freebsd.org/changeset/base/347463

Log:
  powerpc: Initialize the Hardware Interrupt Offset Register (HIOR) earlier for ppc970

  Since we now have a much larger KVA on powerpc64, it's possible to get SLB
  traps earlier in boot, possibly even before the HIOR is properly configured
  for us.  Move the HIOR setup to immediately after reset, so that we use our
  exception handlers instead of Open Firmware's.

  PR:		233863
  Submitted by:	Mark Millard (partial)
  Reported by:	Mark Millard
  MFC after:	2 weeks

Changes:
  head/sys/powerpc/aim/mp_cpudep.c
Comment 38 Mark Millard 2019-05-10 23:04:25 UTC
Comment on attachment 204307 [details]
Patch for aim/mp_cpudep.c that fixes slb-miss problem in cpudep_ap_bootstrap for PPC970/PowerMacG5 contexts

While I have no clue why Justin H. thinks that later-is-better
for most of the code that I moved in my latest aim/mp_cpudep.c
patch, Justin has checked in -r347463 with just the HIOR part
moved:

		/* Set HIOR to 0 */
		__asm __volatile("mtspr 311,%0" :: "r"(0));
		powerpc_sync();

This certainly has allowed booting in my testing of a
-r347463 build.
Comment 39 Mark Millard 2019-05-10 23:29:27 UTC
(In reply to Mark Millard from comment #38)

I forgot to warn that head -r347463 without any patching
has world messed up for older powerpc64 processors like
the 970 family: it is using the newer instruction cmpb in
its strcmp implementation in libc.

I dealt with this issue with a patch that avoids cmpb:
(not appropriate or required for head -r345758 were
the previouosly-attached patches are targeted)

# svnlite diff /mnt/usr/src/
Index: /mnt/usr/src/lib/libc/powerpc64/string/strcmp.S
===================================================================
--- /mnt/usr/src/lib/libc/powerpc64/string/strcmp.S	(revision 347463)
+++ /mnt/usr/src/lib/libc/powerpc64/string/strcmp.S	(working copy)
@@ -88,9 +88,16 @@
.Lstrcmp_compare_by_word:
	ld	%r5,0(%r3)	/* Load double words. */
	ld	%r6,0(%r4)
-	xor	%r8,%r8,%r8	/* %r8 <- Zero. */
+	lis     %r8,32639	/* 0x7f7f */
+	ori     %r8,%r8,32639	/* 0x7f7f7f7f */
+	rldimi  %r8,%r8,32,0	/* 0x7f7f7f7f'7f7f7f7f */
	xor	%r0,%r5,%r6	/* Check if double words are different. */
-	cmpb	%r7,%r5,%r8	/* Check if double words contain zero. */
+				/* Check for zero vs. not bytes: */
+	and	%r9,%r5,%r8	/* 0x00->0x00, 0x80->0x00, other->ms-bit-in-byte==0 */
+	add	%r9,%r9,%r8	/*     ->0x7f,     ->0x7f,      ->ms-bit-in-byte==1 */
+	nor	%r7,%r9,%r5	/*     ->0x80,     ->0x00,      ->ms-bit-in-byte==0 */
+	andc	%r7,%r7,%r8	/*     ->0x80,     ->0x00,      ->0x00 */
+				/* sort of like cmpb %r7,%r5,%r8 for %r8 being zero */

	/*
	 * If double words are different or contain zero,
@@ -104,7 +111,12 @@
	ldu	%r5,8(%r3)	/* Load double words. */
	ldu	%r6,8(%r4)
	xor	%r0,%r5,%r6	/* Check if double words are different. */
-	cmpb	%r7,%r5,%r8	/* Check if double words contain zero. */
+				/* Check for zero vs. not bytes: */
+	and	%r9,%r5,%r8	/* 0x00->0x00, 0x80->0x00, other->ms-bit-in-byte==0 */
+	add	%r9,%r9,%r8	/*     ->0x7f,     ->0x7f,      ->ms-bit-in-byte==1 */
+	nor	%r7,%r9,%r5	/*     ->0x80,     ->0x00,      ->ms-bit-in-byte==0 */
+	andc	%r7,%r7,%r8	/*     ->0x80,     ->0x00,      ->0x00 */
+				/* sort of like cmpb %r7,%r5,%r8 for %r8 being zero */

	/*
	 * If double words are different or contain zero,
Comment 40 Justin Hibbits freebsd_committer 2019-05-11 15:25:50 UTC
(In reply to Mark Millard from comment #38)

It's not a question of "Good" or "Better", it's a question of "what solves the problem with the minimal change."  Since HID0/HID1 config is handled in cpudep_ap_setup() for other CPU types, it makes sense to keep it there for PPC970 as well.  If there is a real need to move it earlier then I can do so, but since it works as-is, it makes sense to just keep it where it is.
Comment 41 Mark Millard 2019-05-11 18:27:35 UTC
(In reply to Mark Millard from comment #39)

Justin H. has reverted -r346588 in -r347492: so the patch
to allow string comparison to work would no longer be
involved. The file patched is no longer present.
Comment 42 Mark Millard 2019-05-12 01:58:36 UTC
Comment on attachment 203814 [details]
Investigatory sys/powerpc/powerpc/mp_machdep.c patch to help limit stuck-sleeping problem

I'm withdrawing the attempt to have the various
cpus end up with TBR values that are closer to
matching: Overall its results have been mixed.
No systematic improvement.
Comment 43 Mark Millard 2019-05-12 08:37:08 UTC
(In reply to Justin Hibbits from comment #40)

Continuity and common structure certainly are
considerations.

For reference, I updated my code to dump out
the HID0, HID1, HID4, and HDI5 values for the
ap's, dumping values from both before FreeBSD
changes them and after they are changed. I was
curious. The result was:

HIOR(311) before: fff00000, 0 after
before: HID0 fc1c000000000000
after:  HID0 0151108100000000
before: HDI1                0
after:  HDI1 fd3c200000000000
before: HID4 10000000000, HID5 0
after:  HID4  1000000000, HID5 0

So differences in:
(Hopefully I matched everything up correctly)

HID0: one_ppc, do_single, isync_sc, ser-gp, reserved bits 4:5,
deep nap, doze, nhr, ext_tb_en, reserved bit 24, en_attn.

HID1: bht_pm, en_ls, en_cc, en_ic, pf_mode, en_if_cach,
en_ic_rec, ic_pe.

HID4: rm_ci, en_sp_dtw.

HID5: none.

I'll note that en_ic is described with:

QUOTE
Enable instruction cache (must be ‘1’ for proper functioning).
END QUOTE

(Not that there is a description of what the improper functioning
of the 970MP would be.) This was the wording I was most
worried about.

[There are some other bits that have differences from "preferred
state", not that I know the importance of them.]

[slbtrap and handle_kernel_slb_spill are using mftb() and so
ext_tb_en being different is in use before HID0 is updated.]
Comment 44 Mark Millard 2019-05-12 09:19:15 UTC
Another solid powerpc64 FreeBSD incompatibility for
the PowerMac G5's is the following, not directly
addressed by any of the attachments, but a working
and in-use usefdt mode does avoid the problem. (So
it is indirectly addressed to some extent.)

When the "direct map" to high memory addresses, parts
of openfimware and FreeBSD became solidly, reliably
incompatible, leading to reproducible crashes.

Unless you want the powerpc64 FreeBSD to crash, one
thing to avoid is significant use of openfirmware,
such as via use of ofwdump, say ofwdump -ap > /dev/null .

When I wanted to use ofwdump, I booted 32-bit powerpc
FreeBSD on the G5 instead and use ofwdump from there.
(This was before I could put usefdt mode to use. Now
I've two types of contexts from which I can run ofwdump
without crashing.)
Comment 45 Mark Millard 2019-05-12 23:06:02 UTC
(In reply to Justin Hibbits from comment #40)

All 5 of my attachments are tied to enabling
booting various old PowerMac models in at least
some mode (mostly usefdt mode).

3 of the 5 attachments are associated with
not being able to boot and operate various old
PowerMacs in usefdt mode. I originally started
because I could not follow the "must set usefdt"
part of the one line description on the old
PowerMac7,2 G5 (2-sockets/1-core-each) and some
G4s and a G3.

You may well not want those problems covered by
your new "in process" status for the defect. If
not, then we need to do something about keeping
the usefdt-mode-booting information someplace.
(I'm hoping that usefdt becomes official and the
default on the old PowerMacs.)

The aim/trap_subr32.S isync's patch you have already
been dealing with. It was tied to a lack of ability
to boot G3s: It avoided getting machine-checks, there
by allowing seeing the more informative type of
failure information. I'm not sure if it is MFC'd
everywhere yet.

Again, if not considered as part of the original
submittal's problems, we may need to do something
to be sure it is not lost (unless it is MFC'd
everywhere already?).

That is 4 of the 5 attachments, the 5th is my
original version of initializing sufficiently,
early enough, on ap's. That may be the only one
you want the "in progress" status to cover (using
your variant).
Comment 46 Mark Millard 2019-05-14 02:14:33 UTC
Comment on attachment 204307 [details]
Patch for aim/mp_cpudep.c that fixes slb-miss problem in cpudep_ap_bootstrap for PPC970/PowerMacG5 contexts

Now that my context is past -r347463 (now: -r346549) I'm
withdrawing this attachment, as it overlaps with the
modern code. (The material is still available for
reference.)
Comment 47 Mark Millard 2019-05-14 02:18:40 UTC
Comment on attachment 203980 [details]
Investigatory /sys/powerpc/aim/trap_subr32.S patch (add missing isync's)

Now that my context is past -r346619 (now: -r347549) I'm
withdrawing this attachment, as it overlaps with the
modern code. (The material is still available for
reference.)
Comment 48 Mark Millard 2019-05-14 02:21:17 UTC
(In reply to Mark Millard from comment #46)

Just noting a dumb typo in my #46 material:

My powerpc64 context is now based on -r347549 (not -r346549).
Comment 49 Mark Millard 2019-05-14 02:27:49 UTC
Comment on attachment 203683 [details]
Investigatory sys/powerpc/powermac/hrowpic.c sys/powerpc/powermac/uninorth.c sys/powerpc/powerpc/openpic.c patches (OF_xref_from_node and OF_node_from_xref use)

Just fixing a typo in the description.
Comment 50 Mark Millard 2019-05-14 02:38:24 UTC
Created attachment 204368 [details]
Mostly: Investigatory patch for sys/powerpc/aim/mmu_oea64.c (for filling translations[] from trans_cells[])

-r346174 had changed the mmu_oea64.c source. So update
the patch to be based on -r347549 that my powerpc64
environment is now based on.

It now also has a change to the comment about mapping
the KVA range in the slb, noting that the slb may not
be able to hold the whole range and "random" entry
replacements may happen.
Comment 51 Mark Millard 2019-05-14 02:47:50 UTC
Created attachment 204369 [details]
Investigatory stand/powerpc/ofw/ofwfdt.c and sys/contrib/libfdt/fdt_rw.c patches, preserving peer order, handling Apple oddities so information is preserved

-r346132 had changed the ofwfdt.c source. So update
the patches to be based on -r347549 that my powerpc64
environment is now based on.
Comment 52 commit-hook freebsd_committer 2019-05-24 01:52:30 UTC
A commit references this bug:

Author: jhibbits
Date: Fri May 24 01:51:58 UTC 2019
New revision: 348218
URL: https://svnweb.freebsd.org/changeset/base/348218

Log:
  MFC r347463:

  powerpc: Initialize the Hardware Interrupt Offset Register (HIOR) earlier for
  ppc970

  Since we now have a much larger KVA on powerpc64, it's possible to get SLB
  traps earlier in boot, possibly even before the HIOR is properly configured
  for us.  Move the HIOR setup to immediately after reset, so that we use our
  exception handlers instead of Open Firmware's.

  PR:		233863

Changes:
_U  stable/12/
  stable/12/sys/powerpc/aim/mp_cpudep.c