On 09/04/20(Thu) 15:57, Peter J. Philipp wrote:
> On Thu, Apr 09, 2020 at 01:08:12PM +0200, Martin Pieuchot wrote:
> > On 09/04/20(Thu) 12:20, Peter J. Philipp wrote:
> > > It's April 9th for me, so no chance for April 1st things. Both patches
> > > didn't
> > > boot (they loaded on ofwboot though) for me. I assume you wanted me to
> > > enable
> > > WITNESS option which I did. The kernel did not print anything so it must
> > > have
> > > done something before openfirmware...
> >
> > Do they boot without WITNESS? They might be other issues to address.
>
> Just an update to list.
>
> GENERIC.MP without WITNESS - did not boot
> GENERIC without WITNESS - did boot
> GENERIC with WITNESS - panic'ed on boot.
Diff below has been Tested by Peter (thanks a lot!) and works with
GENERIC.MP including with WITNESS.
It no longer includes a KASSERT() in stacktrace_save_at() and it keeps
the actual __mp_lock implementation for the pmap. The MI implementation
currently prevents GENERIC.MP from booting, and without access to
hardware I can't really debug the issue. So I renamed the lock, more
cleanups can come afterward.
Hopefully this should be good enough to be able to use WITNESS on powerpc.
More tests are welcome!
Index: arch/powerpc/ddb/db_trace.c
===================================================================
RCS file: /cvs/src/sys/arch/powerpc/ddb/db_trace.c,v
retrieving revision 1.14
diff -u -p -r1.14 db_trace.c
--- arch/powerpc/ddb/db_trace.c 7 Nov 2019 16:08:08 -0000 1.14
+++ arch/powerpc/ddb/db_trace.c 9 Apr 2020 14:04:12 -0000
@@ -31,6 +31,7 @@
#include <sys/systm.h>
#include <sys/proc.h>
#include <sys/user.h>
+#include <sys/stacktrace.h>
#include <uvm/uvm_extern.h>
@@ -221,4 +222,40 @@ db_stack_trace_print(db_expr_t addr, int
--count;
}
(*pr)("end trace frame: 0x%lx, count: %d\n", sp, count);
+}
+
+void
+stacktrace_save_at(struct stacktrace *st, unsigned int skip)
+{
+ vaddr_t lr, sp, lastsp;
+
+ sp = (vaddr_t)__builtin_frame_address(0);
+ if (!INKERNEL(sp) && !ININTSTK(sp))
+ return;
+
+ st->st_count = 0;
+ while (st->st_count < STACKTRACE_MAX) {
+ lr = *(vaddr_t *)(sp + 4) - 4;
+ if (lr & 3)
+ break;
+
+ if (skip == 0)
+ st->st_pc[st->st_count++] = lr;
+ else
+ skip--;
+
+ lastsp = sp;
+ sp = *(vaddr_t *)sp;
+
+ if ((sp == 0) || (sp & 3) || (sp <= lastsp))
+ break;
+ if (!INKERNEL(sp) && !ININTSTK(sp))
+ break;
+ }
+}
+
+void
+stacktrace_save(struct stacktrace *st)
+{
+ return stacktrace_save_at(st, 0);
}
Index: arch/powerpc/include/mplock.h
===================================================================
RCS file: /cvs/src/sys/arch/powerpc/include/mplock.h,v
retrieving revision 1.3
diff -u -p -r1.3 mplock.h
--- arch/powerpc/include/mplock.h 4 Dec 2017 09:51:03 -0000 1.3
+++ arch/powerpc/include/mplock.h 9 Apr 2020 16:21:55 -0000
@@ -27,25 +27,27 @@
#ifndef _POWERPC_MPLOCK_H_
#define _POWERPC_MPLOCK_H_
+#define __USE_MI_MPLOCK
+
/*
* Really simple spinlock implementation with recursive capabilities.
* Correctness is paramount, no fancyness allowed.
*/
-struct __mp_lock {
+struct __ppc_lock {
volatile struct cpu_info *mpl_cpu;
volatile long mpl_count;
};
#ifndef _LOCORE
-void __mp_lock_init(struct __mp_lock *);
-void __mp_lock(struct __mp_lock *);
-void __mp_unlock(struct __mp_lock *);
-int __mp_release_all(struct __mp_lock *);
-int __mp_release_all_but_one(struct __mp_lock *);
-void __mp_acquire_count(struct __mp_lock *, int);
-int __mp_lock_held(struct __mp_lock *, struct cpu_info *);
+void __ppc_lock_init(struct __ppc_lock *);
+void __ppc_lock(struct __ppc_lock *);
+void __ppc_unlock(struct __ppc_lock *);
+int __ppc_release_all(struct __ppc_lock *);
+int __ppc_release_all_but_one(struct __ppc_lock *);
+void __ppc_acquire_count(struct __ppc_lock *, int);
+int __ppc_lock_held(struct __ppc_lock *, struct cpu_info *);
#endif
Index: arch/powerpc/powerpc/lock_machdep.c
===================================================================
RCS file: /cvs/src/sys/arch/powerpc/powerpc/lock_machdep.c,v
retrieving revision 1.8
diff -u -p -r1.8 lock_machdep.c
--- arch/powerpc/powerpc/lock_machdep.c 5 Mar 2020 09:28:31 -0000 1.8
+++ arch/powerpc/powerpc/lock_machdep.c 9 Apr 2020 16:21:01 -0000
@@ -27,7 +27,7 @@
#include <ddb/db_output.h>
void
-__mp_lock_init(struct __mp_lock *lock)
+__ppc_lock_init(struct __ppc_lock *lock)
{
lock->mpl_cpu = NULL;
lock->mpl_count = 0;
@@ -43,7 +43,7 @@ extern int __mp_lock_spinout;
#endif
static __inline void
-__mp_lock_spin(struct __mp_lock *mpl)
+__ppc_lock_spin(struct __ppc_lock *mpl)
{
#ifndef MP_LOCKDEBUG
while (mpl->mpl_count != 0)
@@ -55,14 +55,14 @@ __mp_lock_spin(struct __mp_lock *mpl)
CPU_BUSY_CYCLE();
if (nticks == 0) {
- db_printf("__mp_lock(%p): lock spun out\n", mpl);
+ db_printf("__ppc_lock(%p): lock spun out\n", mpl);
db_enter();
}
#endif
}
void
-__mp_lock(struct __mp_lock *mpl)
+__ppc_lock(struct __ppc_lock *mpl)
{
/*
* Please notice that mpl_count gets incremented twice for the
@@ -92,18 +92,18 @@ __mp_lock(struct __mp_lock *mpl)
}
ppc_intr_enable(s);
- __mp_lock_spin(mpl);
+ __ppc_lock_spin(mpl);
}
}
void
-__mp_unlock(struct __mp_lock *mpl)
+__ppc_unlock(struct __ppc_lock *mpl)
{
int s;
#ifdef MP_LOCKDEBUG
if (mpl->mpl_cpu != curcpu()) {
- db_printf("__mp_unlock(%p): not held lock\n", mpl);
+ db_printf("__ppc_unlock(%p): not held lock\n", mpl);
db_enter();
}
#endif
@@ -118,14 +118,14 @@ __mp_unlock(struct __mp_lock *mpl)
}
int
-__mp_release_all(struct __mp_lock *mpl)
+__ppc_release_all(struct __ppc_lock *mpl)
{
int rv = mpl->mpl_count - 1;
int s;
#ifdef MP_LOCKDEBUG
if (mpl->mpl_cpu != curcpu()) {
- db_printf("__mp_release_all(%p): not held lock\n", mpl);
+ db_printf("__ppc_release_all(%p): not held lock\n", mpl);
db_enter();
}
#endif
@@ -140,13 +140,13 @@ __mp_release_all(struct __mp_lock *mpl)
}
int
-__mp_release_all_but_one(struct __mp_lock *mpl)
+__ppc_release_all_but_one(struct __ppc_lock *mpl)
{
int rv = mpl->mpl_count - 2;
#ifdef MP_LOCKDEBUG
if (mpl->mpl_cpu != curcpu()) {
- db_printf("__mp_release_all_but_one(%p): not held lock\n", mpl);
+ db_printf("__ppc_release_all_but_one(%p): not held lock\n",
mpl);
db_enter();
}
#endif
@@ -157,14 +157,14 @@ __mp_release_all_but_one(struct __mp_loc
}
void
-__mp_acquire_count(struct __mp_lock *mpl, int count)
+__ppc_acquire_count(struct __ppc_lock *mpl, int count)
{
while (count--)
- __mp_lock(mpl);
+ __ppc_lock(mpl);
}
int
-__mp_lock_held(struct __mp_lock *mpl, struct cpu_info *ci)
+__ppc_lock_held(struct __ppc_lock *mpl, struct cpu_info *ci)
{
return mpl->mpl_cpu == ci;
}
Index: arch/powerpc/powerpc/pmap.c
===================================================================
RCS file: /cvs/src/sys/arch/powerpc/powerpc/pmap.c,v
retrieving revision 1.171
diff -u -p -r1.171 pmap.c
--- arch/powerpc/powerpc/pmap.c 5 Sep 2019 03:08:55 -0000 1.171
+++ arch/powerpc/powerpc/pmap.c 9 Apr 2020 16:16:37 -0000
@@ -185,19 +185,19 @@ int physmem;
int physmaxaddr;
#ifdef MULTIPROCESSOR
-struct __mp_lock pmap_hash_lock;
+struct __ppc_lock pmap_hash_lock;
-#define PMAP_HASH_LOCK_INIT() __mp_lock_init(&pmap_hash_lock)
+#define PMAP_HASH_LOCK_INIT() __ppc_lock_init(&pmap_hash_lock)
#define PMAP_HASH_LOCK(s)
\
do { \
s = ppc_intr_disable(); \
- __mp_lock(&pmap_hash_lock); \
+ __ppc_lock(&pmap_hash_lock); \
} while (0)
#define PMAP_HASH_UNLOCK(s)
\
do { \
- __mp_unlock(&pmap_hash_lock); \
+ __ppc_unlock(&pmap_hash_lock); \
ppc_intr_enable(s); \
} while (0)