On 09/04/20(Thu) 10:53, Mark Kettenis wrote:
> > Date: Thu, 9 Apr 2020 10:01:09 +0200
> > From: Martin Pieuchot <[email protected]>
> > [...]
> > + lastsp = sp;
> > + sp = *(vaddr_t *)sp;
> > +
> > + if ((sp == 0) || (sp <= lastsp))
> > + break;
>
> I think checking the alignment of sp here like you do for lr would be
> a good idea. Otherwise an unaligned access might happen. Since
> powerpc uses separate interrupt stacks, the (sp <= lastsp) check might
> truncate the stack trace.
Something like in the diff below?
Index: arch/powerpc/conf/files.powerpc
===================================================================
RCS file: /cvs/src/sys/arch/powerpc/conf/files.powerpc,v
retrieving revision 1.55
diff -u -p -r1.55 files.powerpc
--- arch/powerpc/conf/files.powerpc 25 Jan 2018 15:06:29 -0000 1.55
+++ arch/powerpc/conf/files.powerpc 9 Apr 2020 07:38:37 -0000
@@ -13,7 +13,6 @@ file arch/powerpc/powerpc/process_machde
file arch/powerpc/powerpc/sys_machdep.c
file arch/powerpc/powerpc/trap.c
file arch/powerpc/powerpc/vm_machdep.c
-file arch/powerpc/powerpc/lock_machdep.c multiprocessor
file arch/powerpc/powerpc/intr.c
file arch/powerpc/powerpc/softintr.c
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 09:07:17 -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,39 @@ 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);
+ KASSERT(INKERNEL(sp) || ININTSTK(sp));
+
+ 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 07:37:25 -0000
@@ -1,52 +1,10 @@
/* $OpenBSD: mplock.h,v 1.3 2017/12/04 09:51:03 mpi Exp $ */
-/*
- * Copyright (c) 2004 Niklas Hallqvist. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
- * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
+/* public domain */
#ifndef _POWERPC_MPLOCK_H_
#define _POWERPC_MPLOCK_H_
-/*
- * Really simple spinlock implementation with recursive capabilities.
- * Correctness is paramount, no fancyness allowed.
- */
-
-struct __mp_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 *);
-
-#endif
+#define __USE_MI_MPLOCK
#endif /* !_POWERPC_MPLOCK_H */
Index: arch/macppc/conf/GENERIC.MP
===================================================================
RCS file: /cvs/src/sys/arch/macppc/conf/GENERIC.MP,v
retrieving revision 1.1
diff -u -p -r1.1 GENERIC.MP
--- arch/macppc/conf/GENERIC.MP 3 May 2007 21:21:33 -0000 1.1
+++ arch/macppc/conf/GENERIC.MP 9 Apr 2020 07:39:26 -0000
@@ -4,5 +4,6 @@ include "arch/macppc/conf/GENERIC"
option MULTIPROCESSOR
#option MP_LOCKDEBUG
+#option WITNESS
cpu* at mainbus?
