| Message ID | 20220418042222.2464199-1-pulehui@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [bpf-next] libbpf: Support riscv USDT argument parsing logic | expand |
On Sun, Apr 17, 2022 at 8:53 PM Pu Lehui <pulehui@huawei.com> wrote: > > Add riscv-specific USDT argument specification parsing logic. > riscv USDT argument format is shown below: > - Memory dereference case: > "size@off(reg)", e.g. "-8@-88(s0)" > - Constant value case: > "size@val", e.g. "4@5" > - Register read case: > "size@reg", e.g. "-8@a1" > > s8 will be marked as poison while it's a reg of riscv, we need > to alias it in advance. > > Signed-off-by: Pu Lehui <pulehui@huawei.com> > --- Can you please mention briefly the testing you performed as I'm not able to test this locally. > tools/lib/bpf/usdt.c | 107 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 107 insertions(+) > > diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c > index 934c25301ac1..b8af409cc763 100644 > --- a/tools/lib/bpf/usdt.c > +++ b/tools/lib/bpf/usdt.c > @@ -10,6 +10,11 @@ > #include <linux/ptrace.h> > #include <linux/kernel.h> > > +/* s8 will be marked as poison while it's a reg of riscv */ > +#if defined(__riscv) > +#define rv_s8 s8 > +#endif > + > #include "bpf.h" > #include "libbpf.h" > #include "libbpf_common.h" > @@ -1400,6 +1405,108 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec > return len; > } > > +#elif defined(__riscv) > + > +static int calc_pt_regs_off(const char *reg_name) > +{ > + static struct { > + const char *name; > + size_t pt_regs_off; > + } reg_map[] = { > + { "ra", offsetof(struct user_regs_struct, ra) }, > + { "sp", offsetof(struct user_regs_struct, sp) }, > + { "gp", offsetof(struct user_regs_struct, gp) }, > + { "tp", offsetof(struct user_regs_struct, tp) }, > + { "t0", offsetof(struct user_regs_struct, t0) }, > + { "t1", offsetof(struct user_regs_struct, t1) }, > + { "t2", offsetof(struct user_regs_struct, t2) }, > + { "s0", offsetof(struct user_regs_struct, s0) }, > + { "s1", offsetof(struct user_regs_struct, s1) }, > + { "a0", offsetof(struct user_regs_struct, a0) }, > + { "a1", offsetof(struct user_regs_struct, a1) }, > + { "a2", offsetof(struct user_regs_struct, a2) }, > + { "a3", offsetof(struct user_regs_struct, a3) }, > + { "a4", offsetof(struct user_regs_struct, a4) }, > + { "a5", offsetof(struct user_regs_struct, a5) }, > + { "a6", offsetof(struct user_regs_struct, a6) }, > + { "a7", offsetof(struct user_regs_struct, a7) }, > + { "s2", offsetof(struct user_regs_struct, s2) }, > + { "s3", offsetof(struct user_regs_struct, s3) }, > + { "s4", offsetof(struct user_regs_struct, s4) }, > + { "s5", offsetof(struct user_regs_struct, s5) }, > + { "s6", offsetof(struct user_regs_struct, s6) }, > + { "s7", offsetof(struct user_regs_struct, s7) }, > + { "s8", offsetof(struct user_regs_struct, rv_s8) }, > + { "s9", offsetof(struct user_regs_struct, s9) }, > + { "s10", offsetof(struct user_regs_struct, s10) }, > + { "s11", offsetof(struct user_regs_struct, s11) }, > + { "t3", offsetof(struct user_regs_struct, t3) }, > + { "t4", offsetof(struct user_regs_struct, t4) }, > + { "t5", offsetof(struct user_regs_struct, t5) }, > + { "t6", offsetof(struct user_regs_struct, t6) }, would it make sense to order registers a bit more "logically"? Like s0-s11, t0-t6, etc. Right now it looks very random and it's hard to see if all the registers from some range of registers are defined. > + }; > + int i; > + [...]
On 2022/4/19 12:33, Andrii Nakryiko wrote: > On Sun, Apr 17, 2022 at 8:53 PM Pu Lehui <pulehui@huawei.com> wrote: >> >> Add riscv-specific USDT argument specification parsing logic. >> riscv USDT argument format is shown below: >> - Memory dereference case: >> "size@off(reg)", e.g. "-8@-88(s0)" >> - Constant value case: >> "size@val", e.g. "4@5" >> - Register read case: >> "size@reg", e.g. "-8@a1" >> >> s8 will be marked as poison while it's a reg of riscv, we need >> to alias it in advance. >> >> Signed-off-by: Pu Lehui <pulehui@huawei.com> >> --- > > Can you please mention briefly the testing you performed as I'm not > able to test this locally. > Both RV32 and RV64 have been tested. I will attach the test result in v2. Meanwhile, I found a small problem with libbpf USDT, and will be post in v2. >> tools/lib/bpf/usdt.c | 107 +++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 107 insertions(+) >> >> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c >> index 934c25301ac1..b8af409cc763 100644 >> --- a/tools/lib/bpf/usdt.c >> +++ b/tools/lib/bpf/usdt.c >> @@ -10,6 +10,11 @@ >> #include <linux/ptrace.h> >> #include <linux/kernel.h> >> >> +/* s8 will be marked as poison while it's a reg of riscv */ >> +#if defined(__riscv) >> +#define rv_s8 s8 >> +#endif >> + >> #include "bpf.h" >> #include "libbpf.h" >> #include "libbpf_common.h" >> @@ -1400,6 +1405,108 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec >> return len; >> } >> >> +#elif defined(__riscv) >> + >> +static int calc_pt_regs_off(const char *reg_name) >> +{ >> + static struct { >> + const char *name; >> + size_t pt_regs_off; >> + } reg_map[] = { >> + { "ra", offsetof(struct user_regs_struct, ra) }, >> + { "sp", offsetof(struct user_regs_struct, sp) }, >> + { "gp", offsetof(struct user_regs_struct, gp) }, >> + { "tp", offsetof(struct user_regs_struct, tp) }, >> + { "t0", offsetof(struct user_regs_struct, t0) }, >> + { "t1", offsetof(struct user_regs_struct, t1) }, >> + { "t2", offsetof(struct user_regs_struct, t2) }, >> + { "s0", offsetof(struct user_regs_struct, s0) }, >> + { "s1", offsetof(struct user_regs_struct, s1) }, >> + { "a0", offsetof(struct user_regs_struct, a0) }, >> + { "a1", offsetof(struct user_regs_struct, a1) }, >> + { "a2", offsetof(struct user_regs_struct, a2) }, >> + { "a3", offsetof(struct user_regs_struct, a3) }, >> + { "a4", offsetof(struct user_regs_struct, a4) }, >> + { "a5", offsetof(struct user_regs_struct, a5) }, >> + { "a6", offsetof(struct user_regs_struct, a6) }, >> + { "a7", offsetof(struct user_regs_struct, a7) }, >> + { "s2", offsetof(struct user_regs_struct, s2) }, >> + { "s3", offsetof(struct user_regs_struct, s3) }, >> + { "s4", offsetof(struct user_regs_struct, s4) }, >> + { "s5", offsetof(struct user_regs_struct, s5) }, >> + { "s6", offsetof(struct user_regs_struct, s6) }, >> + { "s7", offsetof(struct user_regs_struct, s7) }, >> + { "s8", offsetof(struct user_regs_struct, rv_s8) }, >> + { "s9", offsetof(struct user_regs_struct, s9) }, >> + { "s10", offsetof(struct user_regs_struct, s10) }, >> + { "s11", offsetof(struct user_regs_struct, s11) }, >> + { "t3", offsetof(struct user_regs_struct, t3) }, >> + { "t4", offsetof(struct user_regs_struct, t4) }, >> + { "t5", offsetof(struct user_regs_struct, t5) }, >> + { "t6", offsetof(struct user_regs_struct, t6) }, > > would it make sense to order registers a bit more "logically"? Like > s0-s11, t0-t6, etc. Right now it looks very random and it's hard to > see if all the registers from some range of registers are defined. > I code it according to the RISCV specification, and for sure, we can make it more intuitive. Thanks, Lehui >> + }; >> + int i; >> + > > [...] > . >
diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c index 934c25301ac1..b8af409cc763 100644 --- a/tools/lib/bpf/usdt.c +++ b/tools/lib/bpf/usdt.c @@ -10,6 +10,11 @@ #include <linux/ptrace.h> #include <linux/kernel.h> +/* s8 will be marked as poison while it's a reg of riscv */ +#if defined(__riscv) +#define rv_s8 s8 +#endif + #include "bpf.h" #include "libbpf.h" #include "libbpf_common.h" @@ -1400,6 +1405,108 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec return len; } +#elif defined(__riscv) + +static int calc_pt_regs_off(const char *reg_name) +{ + static struct { + const char *name; + size_t pt_regs_off; + } reg_map[] = { + { "ra", offsetof(struct user_regs_struct, ra) }, + { "sp", offsetof(struct user_regs_struct, sp) }, + { "gp", offsetof(struct user_regs_struct, gp) }, + { "tp", offsetof(struct user_regs_struct, tp) }, + { "t0", offsetof(struct user_regs_struct, t0) }, + { "t1", offsetof(struct user_regs_struct, t1) }, + { "t2", offsetof(struct user_regs_struct, t2) }, + { "s0", offsetof(struct user_regs_struct, s0) }, + { "s1", offsetof(struct user_regs_struct, s1) }, + { "a0", offsetof(struct user_regs_struct, a0) }, + { "a1", offsetof(struct user_regs_struct, a1) }, + { "a2", offsetof(struct user_regs_struct, a2) }, + { "a3", offsetof(struct user_regs_struct, a3) }, + { "a4", offsetof(struct user_regs_struct, a4) }, + { "a5", offsetof(struct user_regs_struct, a5) }, + { "a6", offsetof(struct user_regs_struct, a6) }, + { "a7", offsetof(struct user_regs_struct, a7) }, + { "s2", offsetof(struct user_regs_struct, s2) }, + { "s3", offsetof(struct user_regs_struct, s3) }, + { "s4", offsetof(struct user_regs_struct, s4) }, + { "s5", offsetof(struct user_regs_struct, s5) }, + { "s6", offsetof(struct user_regs_struct, s6) }, + { "s7", offsetof(struct user_regs_struct, s7) }, + { "s8", offsetof(struct user_regs_struct, rv_s8) }, + { "s9", offsetof(struct user_regs_struct, s9) }, + { "s10", offsetof(struct user_regs_struct, s10) }, + { "s11", offsetof(struct user_regs_struct, s11) }, + { "t3", offsetof(struct user_regs_struct, t3) }, + { "t4", offsetof(struct user_regs_struct, t4) }, + { "t5", offsetof(struct user_regs_struct, t5) }, + { "t6", offsetof(struct user_regs_struct, t6) }, + }; + int i; + + for (i = 0; i < ARRAY_SIZE(reg_map); i++) { + if (strcmp(reg_name, reg_map[i].name) == 0) + return reg_map[i].pt_regs_off; + } + + pr_warn("usdt: unrecognized register '%s'\n", reg_name); + return -ENOENT; +} + +static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg) +{ + char *reg_name = NULL; + int arg_sz, len, reg_off; + long off; + + if (sscanf(arg_str, " %d @ %ld ( %m[a-z0-9] ) %n", &arg_sz, &off, ®_name, &len) == 3) { + /* Memory dereference case, e.g., -8@-88(s0) */ + arg->arg_type = USDT_ARG_REG_DEREF; + arg->val_off = off; + reg_off = calc_pt_regs_off(reg_name); + free(reg_name); + if (reg_off < 0) + return reg_off; + arg->reg_off = reg_off; + } else if (sscanf(arg_str, " %d @ %ld %n", &arg_sz, &off, &len) == 2) { + /* Constant value case, e.g., 4@5 */ + arg->arg_type = USDT_ARG_CONST; + arg->val_off = off; + arg->reg_off = 0; + } else if (sscanf(arg_str, " %d @ %m[a-z0-9] %n", &arg_sz, ®_name, &len) == 2) { + /* Register read case, e.g., -8@a1 */ + arg->arg_type = USDT_ARG_REG; + arg->val_off = 0; + reg_off = calc_pt_regs_off(reg_name); + free(reg_name); + if (reg_off < 0) + return reg_off; + arg->reg_off = reg_off; + } else { + pr_warn("usdt: unrecognized arg #%d spec '%s'\n", arg_num, arg_str); + return -EINVAL; + } + + arg->arg_signed = arg_sz < 0; + if (arg_sz < 0) + arg_sz = -arg_sz; + + switch (arg_sz) { + case 1: case 2: case 4: case 8: + arg->arg_bitshift = 64 - arg_sz * 8; + break; + default: + pr_warn("usdt: unsupported arg #%d (spec '%s') size: %d\n", + arg_num, arg_str, arg_sz); + return -EINVAL; + } + + return len; +} + #else static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg)
Add riscv-specific USDT argument specification parsing logic. riscv USDT argument format is shown below: - Memory dereference case: "size@off(reg)", e.g. "-8@-88(s0)" - Constant value case: "size@val", e.g. "4@5" - Register read case: "size@reg", e.g. "-8@a1" s8 will be marked as poison while it's a reg of riscv, we need to alias it in advance. Signed-off-by: Pu Lehui <pulehui@huawei.com> --- tools/lib/bpf/usdt.c | 107 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+)