reset on mpc83xx

[Christophe Leroy]

Le 30/04/2020 à 21:06, Rasmus Villemoes a écrit :
> On 30/04/2020 17.03, Christophe Leroy wrote:
>>
>>
>> Le 30/04/2020 à 16:44, Rasmus Villemoes a écrit :
>>> Hi,
>>>
>>> I'm wondering why the sysreset_mpc83xx driver (as well as the similar
>>> code in arch/powerpc/cpu/mpc83xx/cpu.c) removes the MSR_IR and MSR_DR
>>> bits from MSR.
>>>
> [snip]
>>>
>>> But since the arch/powerpc/cpu/mpc83xx/cpu.c code in U-Boot has been
>>> there forever, I assume I'm missing something fundamental.
>>
>> On my mpc8321 it works.
>>
>> However I agree it looks odd.
>>
>> I think it works by chance because we have a 1:1 MMU mapping, but if you
>> have a difference mapping I can't see how you can disable Instruction
>> MMU without going through a write into SRR0/SRR1 and an rfi.
> 
> OK, thanks. Can you test if it still works if you change the code to not
> clear the MSR_IR and MSR_DR bits? (I expect it will, since I assume
> you'd have noticed if reboot from linux didn't work).

Due to the COVID-19 lockdown I'm working from home and don't have access 
to HW tools to recover by boards in case I flash a bad Uboot, so I won't 
do that for now.

However I confirm reboot works properly in Linux.

> 
> I'm actually wondering what the core does after the MSR_IR bit gets
> cleared. If address translation is disabled, what instruction does it
> then do immediately after the mtmsr? I mean, the pc has some value, but
> how does that get interpreted without MSR_IR set?

After the mtmsr it probably continues with a few instructions that are 
already in the pipeline, until the isync. Then after the isync it will 
try to fetch from the physical address having the value of the pc.
If it doesn't have an existing address, you'll get a machine check.

> 
> And yes, the (unused, it seems) disable_addr_trans routine in start.S
> does seem to be somewhat more complicated than a simple RMW of MSR,
> suggesting that if disabling address translation was really needed
> before reset, it should be done using that rather than by just clearing
> the bits directly.

Yes disable_addr_trans() is exactly what has to be done for disabling 
instruction address translation.

For disabling only data address translation can be done with an RMW of 
MSR + isync.

Christophe