cpudep_ap_early_bootstrap: IBM970 SPR_HID4 and SPR_HID5 updates are not following documented, 970-family-specific rules

[Mark Millard]

On 2019-May-9, at 22:38, Mark Millard <marklmi at yahoo.com> wrote:

> On 2019-May-9, at 21:11, Mark Millard <marklmi at yahoo.com> wrote:
> 
>> cpudep_ap_early_bootstrap has:
>> 
>>       switch (mfpvr() >> 16) {
>>       case IBM970:
>>       case IBM970FX:
>>       case IBM970MP:
>>               /* 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
>> . . .
>> 
>> 
>> 
>> But 970MP_um.2008MAR07_pub.pdf reports some explicit instruction
>> sequences and words about more instruction if some specific bits
>> might change:
>> 
>> QUOTE
>> 	• The following sequence must be used when modifying HID4:
>> 
>> sync
>> mtspr HID4,Rx
>> isync
>> 
>> When HID4[23] is changed, the previous sequence should be preceded by a Move to Segment Register (mtsr) and Synchronize (sync) instruction, which will cause the effective-to-real-address translations (ERATs) to be flushed.

It turns out that on the ap's:

before: HID4 0x0000'0100'0000'0000
after:  HID4 0x0000'0010'0000'0000

Which means that HID4[23] (i.e, rm_ci) goes from 1 to 0. Thus it
appears that the mtsr ...;sync to first cause and be-ready-for an
ERATs-flush is documented as required on the 970MP.

>> 	• The following sequence must be used when modifying HID5:
>> 
>> sync
>> mtspr HID5,Rx
>> isync
>> 
>> Whenever HID5[56] or HID5[57] is changed, the entire instruction cache must be flushed to ensure that any succeeding Data Cache Block Set to Zero (dcbz) instruction is executed in the context of the new HID5 bit settings.
>> END QUOTE

HID5 stays all-zero.

> Looks like the code in aim_early_init also does not follow the
> documented 970 requirements for HID5 for the bsp. Note that
> HID5_970_DCBZ_SIZE_HI is for the mentioned HID5[56]. The ap's
> also would set this bit (copied from bsp settings).
> 
>        /* Various very early CPU fix ups */
>        switch (mfpvr() >> 16) {
>                /*
>                 * PowerPC 970 CPUs have a misfeature requested by Apple that
>                 * makes them pretend they have a 32-byte cacheline. Turn this
>                 * off before we measure the cacheline size.
>                 */
>                case IBM970:
>                case IBM970FX:
>                case IBM970MP:
>                case IBM970GX:
>                        scratch = mfspr(SPR_HID5);
>                        scratch &= ~HID5_970_DCBZ_SIZE_HI;
>                        mtspr(SPR_HID5, scratch);
>                        break;
> . . .
> 
> Note that mtspr(...) is not providing any extra instructions:
> 
> #ifndef _LOCORE
> #define mtspr(reg, val)                                                 \
>        __asm __volatile("mtspr %0,%1" : : "K"(reg), "r"(val))
> . . .
> 
> 
> There is also:
> 
> QUOTE
> 	• Although it is not necessary to use synchronizing instructions when modifying the MMCR(0,1,A) registers, it is recommended that the following sequence be used:
> 
> sync
> mtspr MMCRz,Rx
> isync
> END QUOTE
> 
> vs. cpu_est_clockrate:
> 
>                case IBM970:
>                case IBM970FX:
>                case IBM970MP:
>                        isync();
>                        mtspr(SPR_970MMCR0, SPR_MMCR0_FC);
>                        isync();
>                        mtspr(SPR_970MMCR1, 0);
>                        mtspr(SPR_970MMCRA, 0);
>                        mtspr(SPR_970PMC1, 0);
>                        mtspr(SPR_970MMCR0,
>                            SPR_970MMCR0_PMC1SEL(PMC970N_CYCLES));
>                        isync();
>                        DELAY(1000);
>                        powerpc_sync();
> . . .
> 
> (Again a prior sync.)




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