Re: Porting DTrace to ARM
FYI http://www.netbsd.org/~ozaki-r/pub/AsiaBSDCon2014-WIP-ozaki-r.pdf This is my slides about this work, which was presented in WIP session of AsiaBSDCon 2014 last week. Regards, ozaki-r On Mon, Mar 17, 2014 at 8:02 PM, Ryota Ozaki ozak...@netbsd.org wrote: I will commit the patch during AsiaBSDCon 2014 if there is no objection :) I did it. So you can try DTrace on ARM now! You can run some commands and scripts described in http://wiki.netbsd.org/tutorials/how_to_enable_and_run_dtrace/ . Beaglebone (Black) is a recommended machine for it. (Actually I tested DTrace only on a BBB and an eval board of am3352.) Note that ctfmerge doesn't work on ARM for now for some reason. So you cannot build a kernel for DTrace with the existence of (nb)ctfmerge command that is built when build.sh tools with -V MKDTRACE=yes. You have to build something like this: build.sh -m evbarm tools build.sh -m evbarm -V MKDTRACE=yes kernel=BEAGLEBONE modules build.sh -m evbarm -V MKDTRACE=yes build # for dtrace command Acknowledge: Thank you very much, matt and christos! I couldn't get this work done well without your great helps! Have fun, ozaki-r
Re: Porting DTrace to ARM
I will commit the patch during AsiaBSDCon 2014 if there is no objection :) I did it. So you can try DTrace on ARM now! You can run some commands and scripts described in http://wiki.netbsd.org/tutorials/how_to_enable_and_run_dtrace/ . Beaglebone (Black) is a recommended machine for it. (Actually I tested DTrace only on a BBB and an eval board of am3352.) Note that ctfmerge doesn't work on ARM for now for some reason. So you cannot build a kernel for DTrace with the existence of (nb)ctfmerge command that is built when build.sh tools with -V MKDTRACE=yes. You have to build something like this: build.sh -m evbarm tools build.sh -m evbarm -V MKDTRACE=yes kernel=BEAGLEBONE modules build.sh -m evbarm -V MKDTRACE=yes build # for dtrace command Acknowledge: Thank you very much, matt and christos! I couldn't get this work done well without your great helps! Have fun, ozaki-r
Re: Porting DTrace to ARM
On Mon, Mar 10, 2014 at 5:04 PM, Ryota Ozaki ozak...@netbsd.org wrote: Hi, I've updated my patch. https://gist.github.com/ozaki-r/8535846 http://www.netbsd.org/~ozaki-r/dtrace-arm.patch The patch now consists of a few files: $ diffstat dtrace-arm.patch external/cddl/osnet/dev/dtrace/arm/dtrace_asm.S | 13 external/cddl/osnet/dev/dtrace/arm/dtrace_isa.c | 11 external/cddl/osnet/dev/fbt/fbt.c| 576 ++- external/cddl/osnet/dist/uts/common/sys/dtrace.h | 14 sys/arch/arm/arm/undefined.c | 49 + sys/arch/arm/include/trap.h |8 6 files changed, 670 insertions(+), 4 deletions(-) undefined.c and trap.h are already reviewed by matt and may be ready to merge. (two global variables for dtrace/fbt modules has been added to undefined.c since then, although I think it's not a big deal.) Can I commit them? (or by matt is better?) dtrace.h just defines some variables and dtrace_asm.S just hooks up a callback for fbt, which are trivial. I think these files can be committed once other files are committed. fbt.c is the biggest change in this porting work. It explores probe points of functions in the kernel, replaces instructions of the probe points, and emulates the replaced instructions. I want someone to take a look at it before merging. The change of dtrace_isa.c is not big and important, but a bit dirty. It copies macros (yes, FR_* ones) from another place to make it buildable. I know the workaround is not awesome, but it makes dtrace work anyway. Any comments are welcome :-) I will commit the patch during AsiaBSDCon 2014 if there is no objection :) Regards, ozaki-r
Re: Porting DTrace to ARM
Hi, I've updated my patch. https://gist.github.com/ozaki-r/8535846 http://www.netbsd.org/~ozaki-r/dtrace-arm.patch The patch now consists of a few files: $ diffstat dtrace-arm.patch external/cddl/osnet/dev/dtrace/arm/dtrace_asm.S | 13 external/cddl/osnet/dev/dtrace/arm/dtrace_isa.c | 11 external/cddl/osnet/dev/fbt/fbt.c| 576 ++- external/cddl/osnet/dist/uts/common/sys/dtrace.h | 14 sys/arch/arm/arm/undefined.c | 49 + sys/arch/arm/include/trap.h |8 6 files changed, 670 insertions(+), 4 deletions(-) undefined.c and trap.h are already reviewed by matt and may be ready to merge. (two global variables for dtrace/fbt modules has been added to undefined.c since then, although I think it's not a big deal.) Can I commit them? (or by matt is better?) dtrace.h just defines some variables and dtrace_asm.S just hooks up a callback for fbt, which are trivial. I think these files can be committed once other files are committed. fbt.c is the biggest change in this porting work. It explores probe points of functions in the kernel, replaces instructions of the probe points, and emulates the replaced instructions. I want someone to take a look at it before merging. The change of dtrace_isa.c is not big and important, but a bit dirty. It copies macros (yes, FR_* ones) from another place to make it buildable. I know the workaround is not awesome, but it makes dtrace work anyway. Any comments are welcome :-) Best regards, ozaki-r On Sat, Mar 8, 2014 at 1:22 AM, Ryota Ozaki ozak...@netbsd.org wrote: On Sat, Mar 8, 2014 at 12:51 AM, Matt Thomas m...@3am-software.com wrote: On Mar 7, 2014, at 2:27 AM, Ryota Ozaki ozak...@netbsd.org wrote: Hi Matt, So my patch for sys/arch/arm is now rather small than so far (just two files); it's minimum code for the core part to run DTrace on ARM. Can you please check it? The patch for sys/arch/arm is http://www.netbsd.org/~ozaki-r/dtrace-arm-sys.patch The full patch is available at the usual place: https://gist.github.com/ozaki-r/8535846 Best regards, ozaki-r In undefined_init, the KDTRACE_HOOK should be after und_ev.ev_count++. I see. Fixed. Any other problem? Digression ahead. I wonder if it wouldn't be better to use a BKPT instruction on armv5t and later and avoid the undefined trap, and use a prefetch abort with a known FSR code. For pre-armv5t, we'd need a undefined handler but we have one of those now. BKPT instructions have either a 16-bit (ARM) or 8-bit (thumb) value encoded within. But that can be tackled later. Actually this is not so digression because my next target is armv5t... Anyway thanks a lot for checking! ozaki-r
Re: Porting DTrace to ARM
Hi Matt, So my patch for sys/arch/arm is now rather small than so far (just two files); it's minimum code for the core part to run DTrace on ARM. Can you please check it? The patch for sys/arch/arm is http://www.netbsd.org/~ozaki-r/dtrace-arm-sys.patch The full patch is available at the usual place: https://gist.github.com/ozaki-r/8535846 Best regards, ozaki-r
Re: Porting DTrace to ARM
On Mar 7, 2014, at 2:27 AM, Ryota Ozaki ozak...@netbsd.org wrote: Hi Matt, So my patch for sys/arch/arm is now rather small than so far (just two files); it's minimum code for the core part to run DTrace on ARM. Can you please check it? The patch for sys/arch/arm is http://www.netbsd.org/~ozaki-r/dtrace-arm-sys.patch The full patch is available at the usual place: https://gist.github.com/ozaki-r/8535846 Best regards, ozaki-r In undefined_init, the KDTRACE_HOOK should be after und_ev.ev_count++. Digression ahead. I wonder if it wouldn't be better to use a BKPT instruction on armv5t and later and avoid the undefined trap, and use a prefetch abort with a known FSR code. For pre-armv5t, we'd need a undefined handler but we have one of those now. BKPT instructions have either a 16-bit (ARM) or 8-bit (thumb) value encoded within. But that can be tackled later.
Re: Porting DTrace to ARM
On Thu, Mar 06, 2014 at 03:34:18PM +0900, Ryota Ozaki wrote: On Thu, Mar 6, 2014 at 2:21 PM, Masao Uebayashi uebay...@gmail.com wrote: Ah. I misread that schedstate_percpu uses percpu(9)'s fast path, which doesn't exist... Anyway if it's assumed that cpu is not attached at run-time, assigning struct cpu_data::void *cpu_dtraceinfo at module attachment would be just fine. void * is probably good, otherwise we have to pull out structure definitions (ok, there are two: solaris_cpu_t and cpu_core_t) from external/cddl. opensolaris_init in external/cddl/osnet/sys/kern/opensolaris.c is a good place to assign, I think. Using 'void *' causes problems with knowing which pointer is valid for a given call. There is no problem using 'struct foo *' without the contents of 'struct foo' being visible. David -- David Laight: da...@l8s.co.uk
Re: Porting DTrace to ARM
On Mar 6, 1:25pm, ozak...@netbsd.org (Ryota Ozaki) wrote: -- Subject: Re: Porting DTrace to ARM | The problem is that all the functions in cpufunc.h are cpu_xxx | cpuid would be an outlier. change it to static inline? christos
Re: Porting DTrace to ARM
On Mar 6, 1:55pm, ozak...@netbsd.org (Ryota Ozaki) wrote: -- Subject: Re: Porting DTrace to ARM | On Thu, Mar 6, 2014 at 1:26 PM, Matt Thomas m...@3am-software.com wrote: | | On Mar 5, 2014, at 8:25 PM, Ryota Ozaki ozak...@netbsd.org wrote: | | On Thu, Mar 6, 2014 at 12:48 PM, Matt Thomas m...@3am-software.com wrote: | | On Mar 5, 2014, at 7:33 PM, Ryota Ozaki ozak...@netbsd.org wrote: | | On Thu, Mar 6, 2014 at 1:39 AM, Matt Thomas m...@3am-software.com wrote: | | On Mar 5, 2014, at 3:12 AM, Ryota Ozaki ozak...@netbsd.org wrote: | | - Replace cpu_id with cpuid in sys/arch/arm | - Can I commit the change? | | Why? It's just churn for no reason I can see. | | The background is that cpu_id in sys/arch/arm conflicts with | the code in cddl and we have to change either one. I and christos | (he already replied in another mail) decided to change | sys/arch/arm, which seems less pain. | | The problem is that all the functions in cpufunc.h are cpu_xxx | cpuid would be an outlier. | | I think a better solution might be to put a field for dtrace | into cpu_data and just curcpu()-ci_dtraceinfo-foo | to get to it instead have a parallel structure. | | You want to put a dtrace in mi_attach_cpu to initialize/allocate it. | | Sounds reasonable (except that it needs to modify cddl much though). | Can we do attach_cpu in a module? | | Too late for the most part. | | # oh, mi_attach_cpu is correct. I found it now :) | | Okay, so we need to have some code in src/sys. Hmm, big change | (for me). I can do it, but I don't know if it's ok or not. | | Christos, how do you think? It is probably easier to change cddl at this point or make cpu_id() an inline. christos
Re: Porting DTrace to ARM
On Thu, Mar 6, 2014 at 3:50 PM, Masao Uebayashi uebay...@gmail.com wrote: Well, more type is better. :) See [1]. My question was more like, what happens if CPU can be attached at run-time in the future? Per-CPU initialization operations have to be remembered somewhere. There is no concern about hot-plugging in the current dtrace code, so we have to implement it once we embed per-cpu data of dtrace to struct cpu_data. Thanks, ozaki-r
Re: Porting DTrace to ARM
On Thu, Mar 6, 2014 at 5:32 PM, David Laight da...@l8s.co.uk wrote: On Thu, Mar 06, 2014 at 03:34:18PM +0900, Ryota Ozaki wrote: On Thu, Mar 6, 2014 at 2:21 PM, Masao Uebayashi uebay...@gmail.com wrote: Ah. I misread that schedstate_percpu uses percpu(9)'s fast path, which doesn't exist... Anyway if it's assumed that cpu is not attached at run-time, assigning struct cpu_data::void *cpu_dtraceinfo at module attachment would be just fine. void * is probably good, otherwise we have to pull out structure definitions (ok, there are two: solaris_cpu_t and cpu_core_t) from external/cddl. opensolaris_init in external/cddl/osnet/sys/kern/opensolaris.c is a good place to assign, I think. Using 'void *' causes problems with knowing which pointer is valid for a given call. There is no problem using 'struct foo *' without the contents of 'struct foo' being visible. Oh yes, you're right... ozaki-r David -- David Laight: da...@l8s.co.uk
Re: Porting DTrace to ARM
On Thu, Mar 6, 2014 at 10:46 PM, Christos Zoulas chris...@zoulas.com wrote: On Mar 6, 1:55pm, ozak...@netbsd.org (Ryota Ozaki) wrote: -- Subject: Re: Porting DTrace to ARM | On Thu, Mar 6, 2014 at 1:26 PM, Matt Thomas m...@3am-software.com wrote: | | On Mar 5, 2014, at 8:25 PM, Ryota Ozaki ozak...@netbsd.org wrote: | | On Thu, Mar 6, 2014 at 12:48 PM, Matt Thomas m...@3am-software.com wrote: | | On Mar 5, 2014, at 7:33 PM, Ryota Ozaki ozak...@netbsd.org wrote: | | On Thu, Mar 6, 2014 at 1:39 AM, Matt Thomas m...@3am-software.com wrote: | | On Mar 5, 2014, at 3:12 AM, Ryota Ozaki ozak...@netbsd.org wrote: | | - Replace cpu_id with cpuid in sys/arch/arm | - Can I commit the change? | | Why? It's just churn for no reason I can see. | | The background is that cpu_id in sys/arch/arm conflicts with | the code in cddl and we have to change either one. I and christos | (he already replied in another mail) decided to change | sys/arch/arm, which seems less pain. | | The problem is that all the functions in cpufunc.h are cpu_xxx | cpuid would be an outlier. | | I think a better solution might be to put a field for dtrace | into cpu_data and just curcpu()-ci_dtraceinfo-foo | to get to it instead have a parallel structure. | | You want to put a dtrace in mi_attach_cpu to initialize/allocate it. | | Sounds reasonable (except that it needs to modify cddl much though). | Can we do attach_cpu in a module? | | Too late for the most part. | | # oh, mi_attach_cpu is correct. I found it now :) | | Okay, so we need to have some code in src/sys. Hmm, big change | (for me). I can do it, but I don't know if it's ok or not. | | Christos, how do you think? It is probably easier to change cddl at this point or make cpu_id() an inline. Well...I found that -current now doesn't require cpu_id tweaks for both arch/arm and dtrace anymore. I guess changes in arch/arm headers during my development solved the problem without realizing. So I think we can postpone per-cpu data improvement after works for arm finish. I'm listing the per-cpu work in my todo. Regards, ozaki-r
Re: Porting DTrace to ARM
Hi, So I've updated my patch: https://gist.github.com/ozaki-r/8535846 The new patch dropped committed changes, followed KNF (hopefully) and did some tweaks. At this point, what I would like to get feedbacks are some changes under sys/arch/arm: - Replace cpu_id with cpuid in sys/arch/arm - Can I commit the change? - Move INKERNEL and FR_* macros from arm/arm/db_trace.c to arm/include/db_machdep.h to refer them in external/cddl/osnet/dev/dtrace/arm/dtrace_isa.c - Is this change appropriate? - Create arch/evbarm/include/cpufunc.h - It just includes arm/cpufunc.h, then #include machine/cpufunc.h for evbarm properly as well as i386/amd64 - Is this change appropriate? - Have cond_ok in arm/include/armreg.h - Is this appropriate? Other core parts, especially arm/arm/undefined.c, would be revised again, which don't satisfy me yet. Best regards, ozaki-r On Tue, Mar 4, 2014 at 6:59 PM, Ryota Ozaki ozak...@iij.ad.jp wrote: Hi, I'm back to the work. I've already committed some trivial fixes though, I think other core parts should be reviewed by someone (matt or christos?) before committing them. I will update my patches within days. Thanks, ozaki-r
Re: Porting DTrace to ARM
On Mar 5, 2014, at 3:12 AM, Ryota Ozaki ozak...@netbsd.org wrote: - Replace cpu_id with cpuid in sys/arch/arm - Can I commit the change? Why? It's just churn for no reason I can see. - Move INKERNEL and FR_* macros from arm/arm/db_trace.c to arm/include/db_machdep.h to refer them in external/cddl/osnet/dev/dtrace/arm/dtrace_isa.c - Is this change appropriate? Well, we don't use APCS anymore. AAPCS is the new ABI. My question is how useful they'll be. - Create arch/evbarm/include/cpufunc.h - It just includes arm/cpufunc.h, then #include machine/cpufunc.h for evbarm properly as well as i386/amd64 - Is this change appropriate? No. This makes an assumption that isn't valid, most ports don't have machine/cpufunc.h. #ifdef __arm__ #include arm/cpufunc.h #endif - Have cond_ok in arm/include/armreg.h - Is this appropriate? I added arm_cond_ok_p
Re: Porting DTrace to ARM
On Thu, Mar 6, 2014 at 12:48 PM, Matt Thomas m...@3am-software.com wrote: On Mar 5, 2014, at 7:33 PM, Ryota Ozaki ozak...@netbsd.org wrote: On Thu, Mar 6, 2014 at 1:39 AM, Matt Thomas m...@3am-software.com wrote: On Mar 5, 2014, at 3:12 AM, Ryota Ozaki ozak...@netbsd.org wrote: - Replace cpu_id with cpuid in sys/arch/arm - Can I commit the change? Why? It's just churn for no reason I can see. The background is that cpu_id in sys/arch/arm conflicts with the code in cddl and we have to change either one. I and christos (he already replied in another mail) decided to change sys/arch/arm, which seems less pain. The problem is that all the functions in cpufunc.h are cpu_xxx cpuid would be an outlier. I think a better solution might be to put a field for dtrace into cpu_data and just curcpu()-ci_dtraceinfo-foo to get to it instead have a parallel structure. You want to put a dtrace in mi_attach_cpu to initialize/allocate it. Sounds reasonable (except that it needs to modify cddl much though). Can we do attach_cpu in a module? ozaki-r
Re: Porting DTrace to ARM
Ah. I misread that schedstate_percpu uses percpu(9)'s fast path, which doesn't exist... Anyway if it's assumed that cpu is not attached at run-time, assigning struct cpu_data::void *cpu_dtraceinfo at module attachment would be just fine. On Thu, Mar 6, 2014 at 2:08 PM, Matt Thomas m...@3am-software.com wrote: On Mar 5, 2014, at 9:07 PM, Masao Uebayashi uebay...@gmail.com wrote: percpu(9) might help? Not so much. It's better to just extend cpu_data if it's a common component. Much less overhead.
Re: Porting DTrace to ARM
On Thu, Mar 6, 2014 at 2:21 PM, Masao Uebayashi uebay...@gmail.com wrote: Ah. I misread that schedstate_percpu uses percpu(9)'s fast path, which doesn't exist... Anyway if it's assumed that cpu is not attached at run-time, assigning struct cpu_data::void *cpu_dtraceinfo at module attachment would be just fine. void * is probably good, otherwise we have to pull out structure definitions (ok, there are two: solaris_cpu_t and cpu_core_t) from external/cddl. opensolaris_init in external/cddl/osnet/sys/kern/opensolaris.c is a good place to assign, I think. Thanks, ozaki-r
Re: Porting DTrace to ARM
Well, more type is better. :) See [1]. My question was more like, what happens if CPU can be attached at run-time in the future? Per-CPU initialization operations have to be remembered somewhere. [1] http://mail-index.netbsd.org/tech-kern/2014/03/03/msg016699.html On Thu, Mar 6, 2014 at 3:34 PM, Ryota Ozaki ozak...@netbsd.org wrote: On Thu, Mar 6, 2014 at 2:21 PM, Masao Uebayashi uebay...@gmail.com wrote: Ah. I misread that schedstate_percpu uses percpu(9)'s fast path, which doesn't exist... Anyway if it's assumed that cpu is not attached at run-time, assigning struct cpu_data::void *cpu_dtraceinfo at module attachment would be just fine. void * is probably good, otherwise we have to pull out structure definitions (ok, there are two: solaris_cpu_t and cpu_core_t) from external/cddl. opensolaris_init in external/cddl/osnet/sys/kern/opensolaris.c is a good place to assign, I think. Thanks, ozaki-r
Re: Porting DTrace to ARM
(2014/01/27 19:57), Ryota Ozaki wrote: (2014/01/27 14:43), Ryota Ozaki wrote: Hi Matt, Thank you very much for your kind reviewing. (2014/01/24 23:03), Matt Thomas wrote: On Jan 24, 2014, at 2:34 AM, Ryota Ozaki ozak...@iij.ad.jp wrote: Updated as I planned, there is no curious update though. Use of curcpu() or cpu_number() without having done kpreempt_disable() is unsafe. All the functions that I added and calls cpu_number() are called from an interrupt handler, xc_unicast(XC_HIGHPRI, ...) or dtrace_probe() that calls interrupt_disable() prior to calling the functions. In these cases, we don't need kpreempt_disable(), right? dtrace_asm.S doesn't assym.h Do you mean dtrace_asm.S doesn't need assym.h? assembly routines should have a END(name) as well. Use of =foo is frowned upon. ENTRY(dtrace_invop_init) #ldrr0, dtrace_invop_start ldrr1, .Ldtrace_invop ldrr2, .Ldtrace_invop_jump_addr strr1, [r2] RET .align0 .Ldtrace_invop: .worddtrace_invop .Ldtrace_invop_jump_addr .worddtrace_invop_jump_addr END(dtrace_invop_init) ENTRY(dtrace_invop_uninit) movr0, #0 ldrr1, .Ldtrace_invop_jump_addr strr0, [r1] RET END(dtrace_invop_uninit) Thanks, it works. The code is what I wanted to do initially. external/cddl/osnet/dev/fbt/fbt.c: Don't use u_int{8,16,32}_t OK. Will fix. Makefiles: Use include bsd.own.mk, then use .if exists(${.CURDIR}/${MACHINE}) ARCHDIR=${MACHINE} .elif exists(${.CURDIR}/${MACHINE_CPU}) ARCHDIR=${MACHINE_CPU} .else .error ${MACHINE} or ${MACHINE_CPU} not supported. .endif Followed you suggestion, I changed Makefile like this: OSNETDIR=${NETBSDSRCDIR}/external/cddl/osnet .if exists(${OSNETDIR}/dev/cyclic/${MACHINE:S/amd64/i386/}) ARCH=${MACHINE:S/amd64/i386/} .elif exists(${OSNETDIR}/dev/cyclic/${MACHINE_CPU}) ARCH=${MACHINE_CPU} .else .error ${MACHINE} or ${MACHINE_CPU} not supported. .endif Two notes; .CURDIR is changed to the corresponding directory and ARCHDIR is changed to ARCH because for arm ARCHDIR is already defined in share/mk/bsd.kmodule.mk. Updated the patch at https://gist.github.com/ozaki-r/8535846, which reflects the above comments. Now the patch can run some simple D scripts (by fixing dtrace_cas32), although there still remain some (many?) problems, for example, dtrace -n fbt::: cannot be handled correctly. https://gist.github.com/ozaki-r/8535846 The patch is getting in good shape. The version of arm works as well as that of i386/arm64. At least the sample scripts(*) now work fine on it. (*) http://wiki.netbsd.org/tutorials/how_to_enable_and_run_dtrace/ Major changes since the previous version: - move the instruction emulation code to fbt.c from undefined.c - to make easy to debug - to use definitions of dtrace.h under external/cddl/osnet/ - reject some functions from probing - dtrace doesn't work for the functions - see fbt.c - don't enable interrupts before running dtrace_trapper in undefinedinstruction() - dtrace requires it - the patch is dirty :-( - correct instruction emulations Thanks, ozaki-r
Re: Porting DTrace to ARM
(2014/01/27 14:43), Ryota Ozaki wrote: Hi Matt, Thank you very much for your kind reviewing. (2014/01/24 23:03), Matt Thomas wrote: On Jan 24, 2014, at 2:34 AM, Ryota Ozaki ozak...@iij.ad.jp wrote: Updated as I planned, there is no curious update though. Use of curcpu() or cpu_number() without having done kpreempt_disable() is unsafe. All the functions that I added and calls cpu_number() are called from an interrupt handler, xc_unicast(XC_HIGHPRI, ...) or dtrace_probe() that calls interrupt_disable() prior to calling the functions. In these cases, we don't need kpreempt_disable(), right? dtrace_asm.S doesn't assym.h Do you mean dtrace_asm.S doesn't need assym.h? assembly routines should have a END(name) as well. Use of =foo is frowned upon. ENTRY(dtrace_invop_init) #ldrr0, dtrace_invop_start ldrr1, .Ldtrace_invop ldrr2, .Ldtrace_invop_jump_addr strr1, [r2] RET .align0 .Ldtrace_invop: .worddtrace_invop .Ldtrace_invop_jump_addr .worddtrace_invop_jump_addr END(dtrace_invop_init) ENTRY(dtrace_invop_uninit) movr0, #0 ldrr1, .Ldtrace_invop_jump_addr strr0, [r1] RET END(dtrace_invop_uninit) Thanks, it works. The code is what I wanted to do initially. external/cddl/osnet/dev/fbt/fbt.c: Don't use u_int{8,16,32}_t OK. Will fix. Makefiles: Use include bsd.own.mk, then use .if exists(${.CURDIR}/${MACHINE}) ARCHDIR=${MACHINE} .elif exists(${.CURDIR}/${MACHINE_CPU}) ARCHDIR=${MACHINE_CPU} .else .error ${MACHINE} or ${MACHINE_CPU} not supported. .endif Followed you suggestion, I changed Makefile like this: OSNETDIR=${NETBSDSRCDIR}/external/cddl/osnet .if exists(${OSNETDIR}/dev/cyclic/${MACHINE:S/amd64/i386/}) ARCH=${MACHINE:S/amd64/i386/} .elif exists(${OSNETDIR}/dev/cyclic/${MACHINE_CPU}) ARCH=${MACHINE_CPU} .else .error ${MACHINE} or ${MACHINE_CPU} not supported. .endif Two notes; .CURDIR is changed to the corresponding directory and ARCHDIR is changed to ARCH because for arm ARCHDIR is already defined in share/mk/bsd.kmodule.mk. Updated the patch at https://gist.github.com/ozaki-r/8535846, which reflects the above comments. Now the patch can run some simple D scripts (by fixing dtrace_cas32), although there still remain some (many?) problems, for example, dtrace -n fbt::: cannot be handled correctly. Thanks, ozaki-r Thanks, ozaki-r
Re: Porting DTrace to ARM
Hi Matt, Thank you very much for your kind reviewing. (2014/01/24 23:03), Matt Thomas wrote: On Jan 24, 2014, at 2:34 AM, Ryota Ozaki ozak...@iij.ad.jp wrote: Updated as I planned, there is no curious update though. Use of curcpu() or cpu_number() without having done kpreempt_disable() is unsafe. All the functions that I added and calls cpu_number() are called from an interrupt handler, xc_unicast(XC_HIGHPRI, ...) or dtrace_probe() that calls interrupt_disable() prior to calling the functions. In these cases, we don't need kpreempt_disable(), right? dtrace_asm.S doesn't assym.h Do you mean dtrace_asm.S doesn't need assym.h? assembly routines should have a END(name) as well. Use of =foo is frowned upon. ENTRY(dtrace_invop_init) #ldrr0, dtrace_invop_start ldr r1, .Ldtrace_invop ldr r2, .Ldtrace_invop_jump_addr str r1, [r2] RET .align 0 .Ldtrace_invop: .word dtrace_invop .Ldtrace_invop_jump_addr .word dtrace_invop_jump_addr END(dtrace_invop_init) ENTRY(dtrace_invop_uninit) mov r0, #0 ldr r1, .Ldtrace_invop_jump_addr str r0, [r1] RET END(dtrace_invop_uninit) Thanks, it works. The code is what I wanted to do initially. external/cddl/osnet/dev/fbt/fbt.c: Don't use u_int{8,16,32}_t OK. Will fix. Makefiles: Use include bsd.own.mk, then use .if exists(${.CURDIR}/${MACHINE}) ARCHDIR=${MACHINE} .elif exists(${.CURDIR}/${MACHINE_CPU}) ARCHDIR=${MACHINE_CPU} .else .error ${MACHINE} or ${MACHINE_CPU} not supported. .endif Followed you suggestion, I changed Makefile like this: OSNETDIR=${NETBSDSRCDIR}/external/cddl/osnet .if exists(${OSNETDIR}/dev/cyclic/${MACHINE:S/amd64/i386/}) ARCH=${MACHINE:S/amd64/i386/} .elif exists(${OSNETDIR}/dev/cyclic/${MACHINE_CPU}) ARCH=${MACHINE_CPU} .else .error ${MACHINE} or ${MACHINE_CPU} not supported. .endif Two notes; .CURDIR is changed to the corresponding directory and ARCHDIR is changed to ARCH because for arm ARCHDIR is already defined in share/mk/bsd.kmodule.mk. Thanks, ozaki-r
Re: Porting DTrace to ARM
(2014/01/23 20:15), Ryota Ozaki wrote: (2014/01/22 21:36), Christos Zoulas wrote: On Jan 22, 11:00am, ozak...@iij.ad.jp (Ryota Ozaki) wrote: -- Subject: Re: Porting DTrace to ARM | 1. there seem to be some whitespace only changes | | Sorry for the messy code. Will fix. Not a problem :-) | | 2. what's the STRONG_ALIAS to __ffssi2 about? | | This is needed to modload solaris. Without the fix | I got the following error: | # modload solaris | kobj_checksyms, 880: [solaris]: linker error: symbol `__ffssi2' not found | WARNING: module error: unable to affix module `solaris', error 8 | modload: Exec format error | | This problem seems to be not dtrace specific. Other modules, | e.g., nand, are also encountered the error in the case of | evbarm/BEAGLEBONE. | | I want someone to confirm the problem on an evbarm/BEAGLEBONE | machine (and other evbarm/arm machines). | | I think this fix should be a separate patch if the fix is | correct. I think it is fine, I was just asking. OK. I'm sending a patch via sendpr. | 3. what about the deleted code in dtrace_debug.c | | The deletion is because of replacing dtrace_cmpset_long | with atomic_cas_ulong. Please see the reply to 5. for | more discussion. Ok, sounds good (and for 5) | 4. I am torn about the cpuid - cpu_id change. In my version of the | changes, I had fixed the arm code instead in sys/arch/arm. It was | used in very few places there. | | Sure. I changed cpuid - cpu_id because I thought I should | reduce the changes of the kernel core code. So if the change | of the arm code is acceptable, I will do it in my next patch. I think it is; when I compiled dtrace for arm, there were only a handful of places that cpuid was used. I would check with m...@netbsd.org though, who I've CC:ed. OK. I'm changing so. A revised patch would be uploaded at gist in a couple of days. Updated as I planned, there is no curious update though. Regards, ozaki-r Thanks, ozaki-r Thanks again for all the work, christos