Date: Mon, 22 Apr 2019 15:27:37 -0700 From: Mark Millard <marklmi@yahoo.com> To: FreeBSD PowerPC ML <freebsd-ppc@freebsd.org>, Justin Hibbits <chmeeedalf@gmail.com> Subject: MPC750 (iMac G3) and disabling interrupts: sync after mtsr required according to the manual (at least isync in places?) Message-ID: <52A9F96F-3615-44C9-BA2B-355EDAA4A1AF@yahoo.com>
next in thread | raw e-mail | index | archive | help
The below was prompted by observed iMac G3 behavior
with modern FreeBSD, so MPC750 behavior.
First I list the most worrisome code that I noticed,
before the more general background of which the
below is an example.
For the 32-bit aim code:
CNAME(trapexit):
/* Disable interrupts: */
mfmsr %r3
andi. %r3,%r3,~PSL_EE@l
mtmsr %r3
/* Test AST pending: */
lwz %r5,FRAME_SRR1+8(%r1)
mtcr %r5
bf 17,1f /* branch if PSL_PR is false */
GET_CPUINFO(%r3) /* get per-CPU pointer */
Note: the 64-bit code has an isync before the
"Test AST pending" code. (No sync.)
The below gets into why that 32-bit code (and
possibly more) is a worry: lack of sync (or at
least isync) after using mtmsr to disable
interrupts.
https://www.nxp.com/docs/en/reference-manual/MPC750UM.pdf
reports that a sync after the mtmsr is required for disabling
interrupts fully before whatever follows. (The example also
disables ME, FE0, and FE1, intended to be used before explicitly
loading the cache in contexts where all 4 forms of exceptions
are a worry.)
It also has wording that mtmsr "does not ensure subsequent
instructions execute in the newly established environment".
A similar example is made relative to sync after setting MSR[IR]
and/or MSR[DR]. It mentions isync instead of sync relative to
MSR[PR] via mtmsr. I'll ignore MSR[PR] below.
(The MPC750 seems to have just one sync instruction with
one official encoding, no lwsync or other such variants
with alternate field values in the encoding.)
As for the code generally . . .
powerpc64 and 32-bit powerpc FreeBSD currently have:
static __inline register_t
intr_disable(void)
{
register_t msr;
msr = mfmsr();
mtmsr(msr & ~PSL_EE);
return (msr);
}
where:
static __inline void
mtmsr(register_t value)
{
__asm __volatile ("mtmsr %0; isync" :: "r"(value));
}
(So there is an implicit isync in mtmsr(...) use.)
There are lots of places using PSL_EE to disable interrupts
without calling intr_disable --but they still use mtmsr(...)
[other than assembler contexts].
For things that are involved on the MPC750, I'm mostly
worried about examples that have neither sync nor isync,
secondarily about not having sync. (I note some
other points as well.)
Relative to PDL_EE:
powerpc_init has a disable with no sync.
cpu_est_clockrate seem to use a DELAY(1000) for
the MPC750 (and some others). But other settings
are also involved. cpu_dep_bootstrap has no sync.
(Not that cpu_dep_bootstrap is likely to be used
with MPC750s, certainly not with iMac G3s.)
aim's trapexit for 32-bit has no sync or isync
for its disable but 64-bit has just isync. 32-bit
has just isync for the enable. FRAME_LEAVE uses
only isync (32-bit and 64-bit). breakpoint (32-bit
and 64-bit) uses only isync. (Assembler contexts.)
write_scom uses isync. read_scom uses isync. pcr_set
uses isync. (So double isync's(?), one from mtmsr(...)
use?)
flush_disable_caches uses a following powerpc_sync();
isync() sequence. (Again a double isync(?): isync;
sync; isync; sequence?)
Relative to PSL_IR and PSL_DR:
aim_early_init has an example of no use of sync
when ofwentry==0. aim_cpu_init has an example of no use
of sync. moea_bootstrap has 2 examples of no use of
sync.
FRAME_SETUP uses just isync. FRAME_LEAVE uses just isync
(but also has a PSL_ME involved). Both 32-bit and 64-bit
for those. cpu_wakeup_handler (64-bit) uses just isync.
(Assembler contexts.)
flush_disable_caches has sync and isync after a PDL_DR
use. (Again a double isync(?): isync; sync; isync;
sequence?) [PSL_DR is mixed with PSL_EE. as I remember.]
===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?52A9F96F-3615-44C9-BA2B-355EDAA4A1AF>