970/PowerMac G5 cpudep_ap_bootstrap slb-related hangup *solved* . . .

[Justin Hibbits]

Hi Mark,

On Fri, May 10, 2019 at 6:23 AM Mark Millard <marklmi at yahoo.com> wrote:
>
> [Having removed all my prior investigatory material, I include
> a svnlite diff that I've booted based on, a comparatively
> minimal diff from the head -r347003 that I started from.]
>
> On 2019-May-10, at 02:15, Mark Millard <marklmi at yahoo.com> wrote:
>
> > [This continues a prior message, but I choose a new subject
> > text for the testing that showed the kind of material working.]
> >
> > I have the slbtrap/handle_kernel_slb_spill working instead
> > of hanging up when it has an slb-miss (and well as when there
> > is no miss).
> >
> > In /usr/src/sys/powerpc/aim/mp_cpudep.c I moved the
> > 970 code for HID0 and HID1 from cpudep_ap_setup, code
> > that looks like,
> >
> >                /* Set HIOR to 0 */
> >                __asm __volatile("mtspr 311,%0" :: "r"(0));
> >                powerpc_sync();
> >
> >                /*
> >                 * The 970 has strange rules about how to update HID registers.
> >                 * See Table 2-3, 970MP manual
> >                 *
> >                 * Note: HID4 and HID5 restored already in
> >                 * cpudep_ap_early_bootstrap()
> >                 */
> >
> >                __asm __volatile("mtasr %0; sync" :: "r"(0));
> >        #ifdef __powerpc64__
> >                __asm __volatile(" \
> >                        sync; isync;                                    \
> >                        mtspr   %1, %0;                                 \
> >                        mfspr   %0, %1; mfspr   %0, %1; mfspr   %0, %1; \
> >                        mfspr   %0, %1; mfspr   %0, %1; mfspr   %0, %1; \
> >                        sync; isync"
> >                    :: "r"(bsp_state[0]), "K"(SPR_HID0));
> >                __asm __volatile("sync; isync;  \
> >                    mtspr %1, %0; mtspr %1, %0; sync; isync"
> >                    :: "r"(bsp_state[1]), "K"(SPR_HID1));
> >        #else
> >                __asm __volatile(" \
> >                        ld      %0,0(%2);                               \
> >                        sync; isync;                                    \
> >                        mtspr   %1, %0;                                 \
> >                        mfspr   %0, %1; mfspr   %0, %1; mfspr   %0, %1; \
> >                        mfspr   %0, %1; mfspr   %0, %1; mfspr   %0, %1; \
> >                        sync; isync"
> >                    : "=r"(reg) : "K"(SPR_HID0), "b"(bsp_state));
> >                __asm __volatile("ld %0, 8(%2); sync; isync;    \
> >                    mtspr %1, %0; mtspr %1, %0; sync; isync"
> >                    : "=r"(reg) : "K"(SPR_HID1), "b"(bsp_state));
> >        #endif
> >
> >                powerpc_sync();
> >
> > Here to? moved it to cpudep_ap_early_bootstrap, just before the
> > code for HID4 and HID5, and I commented out 2 #if/endif lines:
> >
> > void
> > cpudep_ap_early_bootstrap(void)
> > {
> > //#ifndef __powerpc64__
> >        register_t reg;
> > //#endif
> >
> >        switch (mfpvr() >> 16) {
> >        case IBM970:
> >        case IBM970FX:
> >        case IBM970MP:
> >> .>.> INSERT CODE HERE <.<.<.
> >
> >                /* Restore HID4 and HID5, which are necessary for the MMU */
> >
> > #ifdef __powerpc64__
> >                mtspr(SPR_HID4, bsp_state[2]); powerpc_sync(); isync();
> >                mtspr(SPR_HID5, bsp_state[3]); powerpc_sync(); isync();
> > #else
> >                __asm __volatile("ld %0, 16(%2); sync; isync;   \
> >                    mtspr %1, %0; sync; isync;"
> >                    : "=r"(reg) : "K"(SPR_HID4), "b"(bsp_state));
> >                __asm __volatile("ld %0, 24(%2); sync; isync;   \
> >                    mtspr %1, %0; sync; isync;"
> >                    : "=r"(reg) : "K"(SPR_HID5), "b"(bsp_state));
> > #endif
> >                powerpc_sync();
> >                break;
> > . . .
> >
> > This does the initialization before cpudep_ap_bootstrap,
> > instead of after.
> >
> > With things then sufficiently initialized for PSL_IR|PSL_DR
> > code to doing things like pcpup->pc_curthread->td_pcb->
> > that sometimes have slb misses, it boots fine,
> > loading into the slb as needed. No more checkstop status
> > (or whatever it was).
> >
> > I do not know if non-970 contexts should have similar
> > changes in the ordering of initializations or not.
> > But, clearly, the 970 family members do need such.
> >
> > I'm not claiming that other material from other notes
> > that I sent out should be ignored, only that the above
> > changes the observed failing behavior, and so is a big
> > gain all by itself. And it is simple to do without
> > other investigations that might be involved in the
> > more overall context.
>
> Of course, whitespace details, may not be well preserved
> below. (The commenting out of the two #if/#endif lines
> was unnecessary and is not done in the below.)
>
<diff in PR 233863>

Good sleuthing.

I think the whole diff could be reduced to just moving the HIOR.  Can
you give r347463 a shot? It's the reduced diff of just moving HIOR.
If that's not sufficient, then I can move the HID0/HID1
initializations, but they didn't look relevant for early boot
stability when I reviewed.

- Justin