> Date: Tue, 21 Mar 2017 15:57:46 +0100 (CET) > From: Mark Kettenis <mark.kette...@xs4all.nl> > > > Date: Mon, 20 Mar 2017 12:26:28 -0400 > > From: Dale Rahn <dr...@dalerahn.com> > > > > On Mon, Mar 20, 2017 at 11:54:03AM +0100, Mark Kettenis wrote: > > > > Date: Mon, 20 Mar 2017 13:31:32 +1100 > > > > From: Jonathan Gray <j...@jsg.id.au> > > > > Cc: tech@openbsd.org > > > > Mail-Followup-To: Mark Kettenis <mark.kette...@xs4all.nl>, > > > > tech@openbsd.org > > > > Content-Disposition: inline > > > > X-XS4ALL-DNSBL-Checked: mxdrop306.xs4all.net checked 210.15.216.215 > > > > against DNS blacklists > > > > X-CNFS-Analysis: v=2.2 cv=eoad9chX c=1 sm=0 tr=0 > > > > a=0rbIscUo4apI/L6UuJIdkA==:117 a=0rbIscUo4apI/L6UuJIdkA==:17 > > > > a=kj9zAlcOel0A:10 a=6Iz7jQTuP9IA:10 a=d_dndPjGE7KARIjjoGsA:9 > > > > a=CjuIK1q_8ugA:10 > > > > X-Virus-Scanned: by XS4ALL Virus Scanner > > > > X-XS4ALL-Spam-Score: -0.5 () RP_MATCHES_RCVD, UNPARSEABLE_RELAY > > > > X-XS4ALL-Spam: NO > > > > Envelope-To: mark.kette...@xs4all.nl > > > > > > > > On Mon, Mar 20, 2017 at 01:35:04AM +0100, Mark Kettenis wrote: > > > > > It turns out that pretty much all relevant aarch64 OSes use the same > > > > > layout for transferring registers in their debug interfaces. Except > > > > > for us. That doesn't make sense and would mean I'd have to do > > > > > additional work in my lldb porting efforts. > > > > > > > > > > Diff below revises "struct reg" for amd64 to be compatible with what > > > > > NetBSD provides. That just matches our naming conventions for struct > > > > > reg better. The actual names are largely irrelevant as debuggers > > > > > hardcode the layouts anyway to support cross-debugging. > > > > > > > > > > This struct isn't actually used yet, so these changes don't really > > > > > have any ABI consequences. But once this is in, I'm planning on > > > > > making core dumps and ptrace actually work on arm64. > > > > > > > > > > ok? > > > > > > > > It looks like NetBSD just has a cross compiled toolchain > > > > nothing that runs on actual hardware. > > > > > > > > Given FreeBSD does I'd consider that more relevant, and it > > > > has: > > > > > > > > struct reg { > > > > uint64_t x[30]; > > > > uint64_t lr; > > > > uint64_t sp; > > > > uint64_t elr; > > > > uint32_t spsr; > > > > }; > > > > > > > > struct fpreg { > > > > __uint128_t fp_q[32]; > > > > uint32_t fp_sr; > > > > uint32_t fp_cr; > > > > }; > > > > > > The FreeBSD definition is equivalent to the NetBSD one: > > > > > > lr == x30 > > > elr == pc > > > > > > It's just that the names assigned by NetBSD make a little bit more > > > sense and follow the pattern we use on many other architectures. > > > > > > NetBSD also includes the userland per-thread-register which I think is > > > a good idea. > > > > > > > Naming and order realistically doesn't matter that much, If it is easier to > > move registers around to match a userland structure in process_*regs(), > > that is reasonable. That said: the more I have followed NetBSD's lead in the > > past, the more I have regretted it later. > > Here is an update diff that actually implementes process_read_regs(). > Changed the struct reg definition slightly to make r_rl explicit to > match our struct trapframe a bit closer. That makes the > process_read_regs() implementation more obvious. > > This is enough to make core dumps useful, so I'd like to commit this > partial implementation. Did explicitly zero out the struct fpreg in > process_read_fpregs() to avoid leaking kernel stack contents. > > > using __uint128_t for FPU is a lot better than the uint64 [64] that is there > > now. > > I'd like to address that in a future diff. > > ok? > > Index: arm64/process_machdep.c > =================================================================== > RCS file: /cvs/src/sys/arch/arm64/arm64/process_machdep.c,v > retrieving revision 1.1 > diff -u -p -r1.1 process_machdep.c > --- arm64/process_machdep.c 17 Dec 2016 23:38:33 -0000 1.1 > +++ arm64/process_machdep.c 21 Mar 2017 14:52:35 -0000 > @@ -53,12 +53,22 @@ > int > process_read_regs(struct proc *p, struct reg *regs) > { > + struct trapframe *tf = p->p_addr->u_pcb.pcb_tf; > + > + memcpy(®s->r_reg[0], &tf->tf_x[0], sizeof(regs->r_reg)); > + regs->r_lr = tf->tf_lr; > + regs->r_sp = tf->tf_sp; > + regs->r_pc = tf->tf_elr; > + regs->r_spsr = tf->tf_spsr; > + regs->r_tpidr = (uint64_t)p->p_addr->u_pcb.pcb_tcb; > + > return(0); > } > > int > process_read_fpregs(struct proc *p, struct fpreg *regs) > { > + memset(®s, 0, sizeof(*regs));
That should be: memset(regs, 0, sizeof(*regs)); Thanks to Peter J. Philipp for pointing that out.