basic stuff implemented
Details
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
sys/arm64/arm64/db_trace.c | ||
---|---|---|
77 ↗ | (On Diff #5965) | I think the changes in this file could be committed independently |
sys/arm64/arm64/db_trace.c | ||
---|---|---|
77 ↗ | (On Diff #5965) | sounds reasonable, thanks! |
lib/libproc/proc_regs.c | ||
---|---|---|
64 | This file could be split out and committed as is (although it won't work as we don't have ptrace support). | |
sys/arm64/arm64/exception.S | ||
71–82 | what is this change for? | |
sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c | ||
11887–11888 | This should be sorted, aarch64 should be before amd64. | |
sys/cddl/dev/dtrace/aarch64/dtrace_asm.S | ||
59–62 | This doesn't disable interrupts, only sets them to be disabled when returning from an exception. To disable them you need the daif register. | |
70–74 | See dtrace_interrupt_disable above | |
82 | Why not ldrb w0, [x0]? Then there is no need for the mov. | |
124–127 | This is wrong, it loads & stores data in 4-byte chunks, but then increments the address by 1. I expect you were intendign to use ldrb and strb here. | |
166–167 | You will need tow versions of this as sizeof(uint32_t) != sizeof(void *) | |
sys/cddl/dev/fbt/aarch64/fbt_isa.c | ||
62 | This coment is incorrect, the 5th parameter is in x4. |
sys/arm64/arm64/exception.S | ||
---|---|---|
71–82 | so in order to emulate 'store pair' instruction we have to reserve space (minimum required 96 bytes, I set 128). in case frame->tf_sp not changed during trap (say dtrace turned off), we have to skip 128 bytes reserved space. If frame->tf_sp changed, it will be used as new SP. |
sys/arm64/arm64/exception.S | ||
---|---|---|
71–82 | I've reworked the code to simplify this. frame->tf_sp is the value of sp when entering the exception handler, you can adjust it as needed in save_registers after the mov x18, sp instruction. |
sys/arm64/arm64/exception.S | ||
---|---|---|
71–82 | thanks! - it is much easier now |
cddl/contrib/opensolaris/lib/libdtrace/aarch64/dt_isadep.c | ||
---|---|---|
83 | What is 0x3 for? If it's to check an alignment we should add a macro for this. | |
lib/libproc/proc_bkpt.c | ||
64 | Is this just used in userland? We should allocate a unique value for it. | |
sys/cddl/dev/dtrace/aarch64/dtrace_subr.c | ||
260 | Why 128? | |
261–262 | This looks wrong for the 32-bit store, it will write to sp and sp + 8 bytes, where it should write to sp and sp + 4 bytes. | |
266–267 | This has the same issue as above with the 32-bit instructions. do we need them? I would only expect to see them used when using the 32-bit ABI in userland. | |
276 | We need #define INSN_SIZE 4 in a header for use here. | |
sys/cddl/dev/fbt/aarch64/fbt_isa.c | ||
157 | Why are you searching for ldp and not ret? |
cddl/contrib/opensolaris/lib/libdtrace/common/dt_link.c | ||
---|---|---|
230–236 | What do we need to do here (for arm/mips in general, not just aarch64)? | |
432–437 | and what do we need to do here? | |
lib/libproc/proc_regs.c | ||
64 | Yes, I'd like to see this committed as is. The aarch64 #ifs should be resorted to be first though to keep alpha order. |
lib/libproc/proc_bkpt.c | ||
---|---|---|
64 |
cddl/contrib/opensolaris/lib/libdtrace/common/dt_link.c | ||
---|---|---|
233 | This should be in alphabetical order, aarch64 should be before arm. | |
434 | And this | |
839 | And this | |
cddl/lib/Makefile | ||
31 | This should be sorted, it was missed when arm was added, but that shouldn't mean we do the same with AArch64. | |
cddl/lib/libdtrace/Makefile | ||
89 | Alphabetical | |
lib/Makefile | ||
220 | Alphabetical | |
lib/libproc/proc_bkpt.c | ||
54 | Alphabetical | |
55 | Can you use a different brk value? It might also pay to document them on the wiki. | |
sys/arm64/arm64/trap.c | ||
267 | You should also check the breakpoint instruction has the correct value so we don't hit the ddb brk. | |
sys/cddl/dev/dtrace/aarch64/dtrace_asm.S | ||
156–157 | Why not replace these two instructions with cbnz? | |
171–172 | And here | |
sys/cddl/dev/fbt/aarch64/fbt_isa.c | ||
43 | This should be a unique value in the EL1 range. | |
116 | Does this function exist on arm64? | |
123 | This list should be checked in some central location. | |
167 | There is no branch delay slot on arm. It's also a bug in the 32-bit dtrace code. | |
195 | POPM? Shouldn't it be named DTRACE_INVOP_RET? |