On Thu, Jul 28, 2016 at 06:32:27PM -0500, alar...@ddci.com wrote: > The target-ppc/excp_helper.c:powerpc_excp() case POWERPC_EXCP_FP fails > to set "env->spr[SPR_BOOKE_ESR] = ESR_FP;". I can submit a patch for > that,
Ok, please do.
> or anyone can add it, but I notice that in the other cases where
> SPR_BOOKE_ESR is set, the "msr" is ALSO set. Since the "msr" is used
> to initialize SRR1, there is a possibility of inadvertently enabling
> BookE MSR bits indirectly.
> Given that this code is not performance
> sensitive, I think it would be safer to set EITHER msr or the ESR, but
> not BOTH. For example:
>
> if (excp_model == POWERPC_EXCP_BOOKE)
> env->spr[SPR_BOOKE_ESR] = ESR_FP;
> else
> msr |= 0x00100000;
That does seem sensible, assuming those srr1 bits are no correct for
BookE, which they're not IIRC (ESR takes their place).
> I did a quick check of the bits set in the POWERPC_EXCP_PROGRAM case.
> The classic PPC sets SRR1 bits 11--15 depending on the exception. In
> Book E these correspond to bits 43--47,
Um.. what? I'm not understanding where this bit shift is coming
from. The exception code is setting MSR directly, so it should be the
same bit numbers, although they might have a different meaning on
BookE to BookS.
> of which (according to my
> EREF) only 45 and 46 are currently defined. BookE MSR bits 45 (Wait
> state enable) and 46 (Critical Enable) correspond to classic SRR1 bits
> 13 (exception is TRAP) and 14 ("SRR0 is not faulting instruction").
> If I understand the current code, given this aliasing then when a TRAP
> exception occurs on a book E processor it will effectively enable wait
> state, and an FP exception (which sets bit 14/46) will set "Critical
> Enable". I'm not sure that either of these features is currently
> implemented so this may not have a downstream effect, but never the
> less it seems incorrect.
I'm not sure about your analysis of which bits are affect, but yes
this definitely does seem wrong.
> I can submit a patch for the ESR_FP, and/or a change to have the
> "either or but not both" settings of MSR and ESR. Please let me know
> which you'd prefer.
Both fixes please, as separate patches.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
