Re: Porting DTrace to ARM

2014-03-23 Thread Ryota Ozaki
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

2014-03-17 Thread Ryota Ozaki
 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

2014-03-14 Thread Ryota Ozaki
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

2014-03-10 Thread Ryota Ozaki
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

2014-03-07 Thread Ryota Ozaki
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

2014-03-07 Thread Matt Thomas

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

2014-03-06 Thread David Laight
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

2014-03-06 Thread Christos Zoulas
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

2014-03-06 Thread Christos Zoulas
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

2014-03-06 Thread Ryota Ozaki
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

2014-03-06 Thread Ryota Ozaki
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

2014-03-06 Thread Ryota Ozaki
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

2014-03-05 Thread Ryota Ozaki
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

2014-03-05 Thread Matt Thomas

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

2014-03-05 Thread Ryota Ozaki
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

2014-03-05 Thread Masao Uebayashi
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

2014-03-05 Thread Ryota Ozaki
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-03-05 Thread Masao Uebayashi
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-02-03 Thread Ryota Ozaki



(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 Thread Ryota Ozaki



(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

2014-01-26 Thread Ryota Ozaki

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-24 Thread Ryota Ozaki



(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