Page MenuHomeFreeBSD

dtrace: Blacklist riscv exception handlers for fbt
ClosedPublic

Authored by kp on Dec 24 2020, 11:00 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 18, 1:57 AM
Unknown Object (File)
Thu, Nov 14, 6:08 AM
Unknown Object (File)
Wed, Nov 13, 9:14 AM
Unknown Object (File)
Mon, Nov 11, 6:15 AM
Unknown Object (File)
Sun, Nov 10, 11:30 AM
Unknown Object (File)
Sun, Nov 10, 11:18 AM
Unknown Object (File)
Sat, Nov 9, 10:33 AM
Unknown Object (File)
Fri, Oct 25, 4:50 PM
Subscribers

Details

Summary

We can't safely instrument those exception handlers, so blacklist them.

Test case: dtrace -n :::

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 35665
Build 32555: arc lint + arc unit

Event Timeline

kp requested review of this revision.Dec 24 2020, 11:00 AM
markj added inline comments.
sys/cddl/dev/fbt/fbt.c
175

I think you can get away with explicitly specifying cpu_exception_handler and cpu_exception_handler_supervisor.

sys/cddl/dev/fbt/fbt.c
175

We also need cpu_exception_handler_user. I can change this, but then we've got 4 entries (cpu_exception_handler, cpu_exception_handler_user, cpu_exception_handler_supervisor and do_trap_supervisor) rather than two.

sys/cddl/dev/fbt/fbt.c
175

Why does cpu_exception_handler_user need to be excluded? If I understand correctly it is called only when handling user-mode exceptions and so shouldn't be callable from dtrace_probe().

sys/cddl/dev/fbt/fbt.c
175

I don't know. I've tested without and wedge the machine doing dtrace -n :::, with cpu_exception_handler_user added to the exclude list that does work.

Perhaps the problem isn't so much about when the probe gets called as it is the assembly function not being safe to instrument.

sys/cddl/dev/fbt/fbt.c
175

Perhaps the problem isn't so much about when the probe gets called as it is the assembly function not being safe to instrument.

That seems possible. I noticed that the code which identifies function entry probes on riscv simply looks for a store of ra to sp, ignoring a non-zero offset. The save_registers macro emits "sd ra, (TF_RA)(sp)" which seems like it could be misinterpreted as a function entry probe. I don't have riscv kernel disassembly handy but does a typical function prologue store ra to sp with offset 0? If so we could probably fix part of the problem by making fbt_provide_module_function() stricter.

sys/cddl/dev/fbt/fbt.c
175

Functions typically start by setting the stack pointer:

ffffffc0000083ba <ck_rhs_map_create>:
{
ffffffc0000083ba:       7139                    addi    sp,sp,-64
        --v;
ffffffc0000083bc:       fc06                    sd      ra,56(sp)
ffffffc0000083be:       f822                    sd      s0,48(sp)
ffffffc0000083c0:       f426                    sd      s1,40(sp)
ffffffc0000083c2:       f04a                    sd      s2,32(sp)
ffffffc0000083c4:       ec4e                    sd      s3,24(sp)
ffffffc0000083c6:       e852                    sd      s4,16(sp)
ffffffc0000083c8:       e456                    sd      s5,8(sp)
ffffffc0000083ca:       e05a                    sd      s6,0(sp)
ffffffc0000083cc:       0080                    addi    s0,sp,64
ffffffc0000083ce:       15fd                    addi    a1,a1,-1
        v |= v >> 1;
ffffffc0000083d0:       0015d61b                srliw   a2,a1,0x1
ffffffc0000083d4:       8dd1                    or      a1,a1,a2
        v |= v >> 2;
sys/cddl/dev/fbt/fbt.c
175

Yuck. So I guess this is not so easy to fix in FBT. In that case, could you please add a comment explaining why we have to exclude those functions?

Note why we can 't instrument assembly trap handlers.

This revision is now accepted and ready to land.Dec 27 2020, 4:23 PM