Date: Fri, 25 Nov 2016 14:12:34 -0800 From: Mark Millard <markmi@dsl-only.net> To: Justin Hibbits <chmeeedalf@gmail.com> Cc: Nathan Whitehorn <nwhitehorn@freebsd.org>, FreeBSD PowerPC ML <freebsd-ppc@freebsd.org> Subject: Re: I think I found why the iMac G3 that I have access to has not booted FreeBSD vintages: 2015-Mar+ . . . [Yep: booted!] Message-ID: <5E0BDD50-FA00-4AF3-B2DF-F0C058AE4835@dsl-only.net> In-Reply-To: <CAHSQbTBLc_eZ7sQgwpVb7f99bL6uTaTUFp2QRGoYj3B-d3HxPA@mail.gmail.com> References: <8915A2F8-0C75-4B8B-BCC1-7252EA02D292@dsl-only.net> <53724219-2378-45E0-B521-8F3EFA85CE41@dsl-only.net> <CAHSQbTBLc_eZ7sQgwpVb7f99bL6uTaTUFp2QRGoYj3B-d3HxPA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2016-Nov-25, at 11:37 AM, Justin Hibbits <chmeeedalf at gmail.com> =
wrote:
> Hi Mark,
>=20
> Nice legwork on this. I just committed r309167 which should fix this =
bug. Can you update and test?
>=20
> - Justin
This is after the clang 3.9.0 changes. I may initially test by =
selectively
including it before updating to include 3.9.0 --after removing my test =
code
of course. It will be later today, or so I expect.
I will note that a couple of mtsrin uses have isync (or other such) and
a couple do not. So some will get double synchronization after the =
change
. . . (The below is from stable/11's -r309125 as it was handy.)
# find /usr/src/sys/powerpc/ -exec grep mtsrin {} \; -print | more
mtsrin(i << ADDR_SR_SHFT, =
kernel_pmap->pm_sr[i]);
/usr/src/sys/powerpc/aim/moea64_native.c
mtsrin(i << ADDR_SR_SHFT, kernel_pmap->pm_sr[i]);
mtsrin(USER_SR << ADDR_SR_SHFT, =
td->td_pcb->pcb_cpu.aim.usr_vsid);
/usr/src/sys/powerpc/aim/mmu_oea.c
mtsrin(USER_SR << ADDR_SR_SHFT, =
td->td_pcb->pcb_cpu.aim.usr_vsid);
/usr/src/sys/powerpc/aim/mmu_oea64.c
{ "mtsrin", 0xfc0007fe, 0x7c0001e4, Op_S | Op_B },
/usr/src/sys/powerpc/powerpc/db_disasm.c
mtsrin(vm_offset_t va, register_t value)
__asm __volatile ("mtsrin %0,%1" :: "r"(value), "r"(va));
/usr/src/sys/powerpc/include/cpufunc.h
moea64_cpu_bootstrap_native has a later isync:
#ifdef __powerpc64__
. . .
#else
for (i =3D 0; i < 16; i++)
mtsrin(i << ADDR_SR_SHFT, =
kernel_pmap->pm_sr[i]);
#endif
=20
/*
* Install page table
*/
=20
__asm __volatile ("ptesync; mtsdr1 %0; isync"
:: "r"((uintptr_t)moea64_pteg_table
| (uintptr_t)(flsl(moea64_pteg_mask >> 11))));
moea_cpu_bootstrap has a later powrpc_aync():
for (i =3D 0; i < 16; i++)
mtsrin(i << ADDR_SR_SHFT, kernel_pmap->pm_sr[i]);
powerpc_sync();
moea_activate did not have an isync (or other such)
(This is the place that I originally reported made the iMac G3
difference.)
moea64_activate did not have an isync or other such:
#ifdef __powerpc64__
PCPU_SET(userslb, pm->pm_slb);
__asm __volatile("slbmte %0, %1; isync" ::
"r"(td->td_pcb->pcb_cpu.aim.usr_vsid), "r"(USER_SLB_SLBE));
#else
PCPU_SET(curpmap, pm->pmap_phys);
mtsrin(USER_SR << ADDR_SR_SHFT, =
td->td_pcb->pcb_cpu.aim.usr_vsid);
#endif
> On Nov 21, 2016 05:22, "Mark Millard" <markmi@dsl-only.net> wrote:
> [Top post of operational confirmation.]
>=20
> I'm now logged in on the iMac G3 under a variant of head -r308874 .
> (Finally after about 1.8 years.)
>=20
> It is currently running a variation of head -r308874 with a debug
> kernel that has the two isync's added around moea_activate's
> mtsrin (and KTR turned back off).
>=20
> With no such isync's (or other such "context-synchronizations")
> the iMac G3 does not boot. (The below likely does not preserve
> tabs.)
>=20
> # svnlite diff /usr/src/sys/powerpc/aim/mmu_oea.c
> Index: /usr/src/sys/powerpc/aim/mmu_oea.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- /usr/src/sys/powerpc/aim/mmu_oea.c (revision 308874)
> +++ /usr/src/sys/powerpc/aim/mmu_oea.c (working copy)
> @@ -991,7 +991,9 @@
> CPU_SET(PCPU_GET(cpuid), &pm->pm_active);
> PCPU_SET(curpmap, pmr);
>=20
> + isync();
> mtsrin(USER_SR << ADDR_SR_SHFT, =
td->td_pcb->pcb_cpu.aim.usr_vsid);
> + isync();
> }
>=20
>=20
> stable/11 should also get such a change, not just head.
>=20
> It would be nice if releng/11 eventually picked up such a
> change so that some release/11.0.? booted on iMac G3's as well.
> Otherwise it waits for release/11.1.0 .
>=20
> I wonder if there might be intermittent problems for
> TARGET_ARCH=3Dpowerpc systems that are (usually) booting
> for release/11.0.x currently.
>=20
> (I only have access to one iMac G3 to test and no access
> to any other kinds of G3's. I have access to a few types
> of PowerMac G4's and 2 types of PowerMac G5's. All the
> PowerPc family machines that I have access to are Apple
> machines.)
>=20
>=20
>=20
>=20
> Note:
>=20
> stable/10 still has the old powerpc/swtch32.S code and so is
> fine for this issue.
>=20
> Part of the context from back in early 2015 was that I
> switched from 10 to 11 as part of getting ready to investigate
> projects/clang380-import for powerpc and powerpc64 use. I
> did not revert back to 10.x despite the iMac G3 not booting.
>=20
> =3D=3D=3D
> Mark Millard
> markmi at dsl-only.net
>=20
> On 2016-Nov-21, at 2:10 AM, Mark Millard <markmi at dsl-only.net> =
wrote:
>=20
> > First I report my understanding of the PowerPc background =
information
> > involved:
> > (then later the code that has that background involved)
> >
> > For reference:
> >
> > 82 mtsrin(vm_offset_t va, register_t value)
> > 83 {
> > 84
> > 85 __asm __volatile ("mtsrin %0,%1" :: "r"(value), =
"r"(va));
> > 86 }
> >
> > PowerPC requirements:
> >
> > mtsr(instruction access): no synchronization required before;
> > context synchronization required after
> > mtsrin(instruction access): no synchronization required before;
> > context synchronization required after
> >
> > So the same criteria. isync, sc, or rfi would be
> > "context-synchronizing".
> >
> > mtsr(data access): context synchronization required before;
> > context synchronization required after
> > mtsrin(data access): context synchronization required before;
> > context synchronization required after
> >
> > So even more required for this context: before and after.
> > Again isync would be "context-synchronizing".
> >
> >
> > Now the code that has that background involved. . .
> >
> > aim/mmu_oea.c's moea_activate does mtsrin without any explicit
> > "context-synchronizing" before or after it --and it replaced
> > code that did have the "context-synchronizing".
> >
> > The modern (2015-Mar-4+) code:
> >
> > /*
> > * Activate a user pmap. The pmap must be activated before it's =
address
> > * space can be accessed in any way.
> > */
> > void
> > moea_activate(mmu_t mmu, struct thread *td)
> > {
> > pmap_t pm, pmr;
> >
> > /*
> > * Load all the data we need up front to encourage the =
compiler to
> > * not issue any loads while we have interrupts disabled =
below.
> > */
> > pm =3D &td->td_proc->p_vmspace->vm_pmap;
> > pmr =3D pm->pmap_phys;
> >
> > CPU_SET(PCPU_GET(cpuid), &pm->pm_active);
> > PCPU_SET(curpmap, pmr);
> >
> > mtsrin(USER_SR << ADDR_SR_SHFT, =
td->td_pcb->pcb_cpu.aim.usr_vsid);
> > }
> >
> > I expect that two isync's are missing.
> >
> > At the assembler level of detail the modern code in my example
> > build is:
> >
> > 0080e3fc <moea_activate> stwu r1,-32(r1)
> > 0080e400 <moea_activate+0x4> stw r31,24(r1)
> > 0080e404 <moea_activate+0x8> mr r31,r1
> > 0080e408 <moea_activate+0xc> lwz r9,4(r4)
> > 0080e40c <moea_activate+0x10> lwz r10,308(r9)
> > 0080e410 <moea_activate+0x14> lwz r8,312(r10)
> > 0080e414 <moea_activate+0x18> mfsprg r9,0
> > 0080e418 <moea_activate+0x1c> lwz r9,36(r9)
> > 0080e41c <moea_activate+0x20> rlwinm r9,r9,27,5,31
> > 0080e420 <moea_activate+0x24> mfsprg r11,0
> > 0080e424 <moea_activate+0x28> rlwinm r9,r9,2,0,29
> > 0080e428 <moea_activate+0x2c> add r9,r9,r10
> > 0080e42c <moea_activate+0x30> addi r9,r9,256
> > 0080e430 <moea_activate+0x34> lwz r11,36(r11)
> > 0080e434 <moea_activate+0x38> clrlwi r11,r11,27
> > 0080e438 <moea_activate+0x3c> li r0,1
> > 0080e43c <moea_activate+0x40> slw r0,r0,r11
> > 0080e440 <moea_activate+0x44> lwz r11,24(r9)
> > 0080e444 <moea_activate+0x48> or r0,r0,r11
> > 0080e448 <moea_activate+0x4c> stw r0,24(r9)
> > 0080e44c <moea_activate+0x50> mfsprg r9,0
> > 0080e450 <moea_activate+0x54> stw r8,304(r9)
> > 0080e454 <moea_activate+0x58> lwz r9,644(r4)
> > 0080e458 <moea_activate+0x5c> lwz r9,1176(r9)
> > 0080e45c <moea_activate+0x60> lis r0,-16384
> > 0080e460 <moea_activate+0x64> mtsrin r9,r0 <<<<<<<<=3D=3D=3D=3D=3D=3D=
=3D "Context-synchronization(s)"?
> > 0080e464 <moea_activate+0x68> lwz r11,0(r1)
> > 0080e468 <moea_activate+0x6c> lwz r31,-8(r11)
> > 0080e46c <moea_activate+0x70> mr r1,r11
> > 0080e470 <moea_activate+0x74> blr
> >
> >
> > But the old, historical code that this replaced did have
> > explicit "context-synchornizing" in powerpc/swtch32.S for
> > AIM ( -r279594 replaced the below on 2015-Mar-4 ):
> >
> > -#ifdef AIM
> > - lwz %r5,PCB_AIM_USR_VSID(%r3) /* Load the USER_SR segment =
reg */
> > - isync
> > - mtsr USER_SR,%r5
> > - isync
> > -#endif
> >
> > (It was part of setting the user pmap during thread switching.)
> >
> > This replacement happened shortly before I discovered that
> > the iMac G3 could no longer boot. It stops in pmap_activate
> > in the lwz r11,0(r1) just after moea_activate returns from
> > its bctrl-based call (modern code again below):
> >
> > . . .
> > 00847954 <pmap_activate+0x94> beq- cr7,00847960 =
<pmap_activate+0xa0>
> > 00847958 <pmap_activate+0x98> lwz r3,1024(r11)
> > 0084795c <pmap_activate+0x9c> bl 004fdfac <kobj_lookup_method>
> > 00847960 <pmap_activate+0xa0> lwz r3,4(r3)
> > 00847964 <pmap_activate+0xa4> mtctr r3
> > 00847968 <pmap_activate+0xa8> mr r3,r29
> > 0084796c <pmap_activate+0xac> mr r4,r28
> > 00847970 <pmap_activate+0xb0> bctrl <<<<<<<<<<=3D=3D=3D=3D=3D=3D=3D=3D=
=3D Calls moea_activate.
> > 00847974 <pmap_activate+0xb4> lwz r11,0(r1) <<<<<<<=3D=3D=3D=3D=3D=
This fails.
> >
> > I end up at the db> prompt (too early for interactive input).
> > (I have ddb automatically execute a compiled-in script.)
> >
> > I've confirmed with show registers that ctl has the address of =
moea_activate
> > (0x80e3fc in my example build of head -r308874) and srr0 has =
pmap_activate+0xb4's
> > value (matching lr). srr1 has the value 0x1032. dar has the value =
0xe43f6a50
> > (matching r1 and r31). dsisr has the value 0x40000000.
> >
> > I also have confirmed with verbose KTR reporting that:
> >
> > cpu0 mi_switch: old thread 100022 (td_sched 0x2x0e6a8, pid 12, irq0: =
pcm0)
> >
> > happens just before it stops at pmap_activate+0xb4, at least
> > as far as visible messages go.
> >
> > Again, I expect that two isync's are missing in moea_activate:
> > one before the mtsrin and one after it, matching the old mtsr
> > handling from before the change.
> >
> > =3D=3D=3D
> > Mark Millard
> > markmi at dsl-only.net
=3D=3D=3D
Mark Millard
markmi at dsl-only.net
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5E0BDD50-FA00-4AF3-B2DF-F0C058AE4835>