Hi, Thanks for applying the series. On Wed, Mar 8, 2023 at 12:41 AM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Tue, Mar 7, 2023 at 4:04 AM Puranjay Mohan <puranjay12@xxxxxxxxx> wrote: > > > > Parsing of USDT arguments is architecture-specific; on arm it is > > relatively easy since registers used are r[0-10], fp, ip, sp, lr, > > pc. Format is slightly different compared to aarch64; forms are > > > > - "size @ [ reg, #offset ]" for dereferences, for example > > "-8 @ [ sp, #76 ]" ; " -4 @ [ sp ]" > > - "size @ reg" for register values; for example > > "-4@r0" > > - "size @ #value" for raw values; for example > > "-8@#1" > > > > Add support for parsing USDT arguments for ARM architecture. > > > > To test the above changes QEMU's virt[1] board with cortex-a15 > > CPU was used. libbpf-bootstrap's usdt example[2] was modified to attach > > to a test program with DTRACE_PROBE1/2/3/4... probes to test different > > combinations. > > > > [1] https://www.qemu.org/docs/master/system/arm/virt.html > > [2] https://github.com/libbpf/libbpf-bootstrap/blob/master/examples/c/usdt.bpf.c > > > > Signed-off-by: Puranjay Mohan <puranjay12@xxxxxxxxx> > > --- > > tools/lib/bpf/usdt.c | 80 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 80 insertions(+) > > > > diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c > > index 293b7a37f8a1..27a4589eda1c 100644 > > --- a/tools/lib/bpf/usdt.c > > +++ b/tools/lib/bpf/usdt.c > > @@ -1466,6 +1466,86 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec > > return len; > > } > > > > +#elif defined(__arm__) > > + > > +static int calc_pt_regs_off(const char *reg_name) > > +{ > > + static struct { > > + const char *name; > > + size_t pt_regs_off; > > + } reg_map[] = { > > + { "r0", offsetof(struct pt_regs, uregs[0]) }, > > + { "r1", offsetof(struct pt_regs, uregs[1]) }, > > + { "r2", offsetof(struct pt_regs, uregs[2]) }, > > + { "r3", offsetof(struct pt_regs, uregs[3]) }, > > + { "r4", offsetof(struct pt_regs, uregs[4]) }, > > + { "r5", offsetof(struct pt_regs, uregs[5]) }, > > + { "r6", offsetof(struct pt_regs, uregs[6]) }, > > + { "r7", offsetof(struct pt_regs, uregs[7]) }, > > + { "r8", offsetof(struct pt_regs, uregs[8]) }, > > + { "r9", offsetof(struct pt_regs, uregs[9]) }, > > + { "r10", offsetof(struct pt_regs, uregs[10]) }, > > + { "fp", offsetof(struct pt_regs, uregs[11]) }, > > + { "ip", offsetof(struct pt_regs, uregs[12]) }, > > + { "sp", offsetof(struct pt_regs, uregs[13]) }, > > + { "lr", offsetof(struct pt_regs, uregs[14]) }, > > + { "pc", offsetof(struct pt_regs, uregs[15]) }, > > + }; > > + 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, int *arg_sz) > > +{ > > + char reg_name[16]; > > + int len, reg_off; > > + long off; > > + > > + if (sscanf(arg_str, " %d @ \[ %15[a-z0-9], #%ld ] %n", > > I've added space before , and applied to bpf-next. > > Thanks, it's a nice clean up and wider architecture support! > > BTW, I noticed that we don't support fp, ip, lr, and pc (only sp) for > __aarch64__, why such a difference between 32-bit and 64-bit arms? Actually, it is just a naming convention. in AARCH64 R29 is FP and R30 is LR, etc. so, they are decoded with that. Only SP is seperate. in ARM the register names are explicitly named. For ARM: https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#machine-registers For AARCH64: https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#machine-registers P.S.: For adding more BPF features for ARM, I feel the following things are remaining: 1. Adding CI for ARM: - Can you give me the steps needed to do this? 2. Implement BPF trampoline for ARM. - This should be a little complicated and might need other dependent features that might not be available in arm. 3.??? > > > + arg_sz, reg_name, &off, &len) == 3) { > > + /* Memory dereference case, e.g., -4@[fp, #96] */ > > + arg->arg_type = USDT_ARG_REG_DEREF; > > + arg->val_off = off; > > + reg_off = calc_pt_regs_off(reg_name); > > + if (reg_off < 0) > > + return reg_off; > > + arg->reg_off = reg_off; > > + } else if (sscanf(arg_str, " %d @ \[ %15[a-z0-9] ] %n", arg_sz, reg_name, &len) == 2) { > > + /* Memory dereference case, e.g., -4@[sp] */ > > + arg->arg_type = USDT_ARG_REG_DEREF; > > + arg->val_off = 0; > > + reg_off = calc_pt_regs_off(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 @ %15[a-z0-9] %n", arg_sz, reg_name, &len) == 2) { > > + /* Register read case, e.g., -8@r4 */ > > + arg->arg_type = USDT_ARG_REG; > > + arg->val_off = 0; > > + reg_off = calc_pt_regs_off(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; > > + } > > + > > + return len; > > +} > > + > > #else > > > > static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg, int *arg_sz) > > -- > > 2.39.1 > > -- Thanks and Regards Yours Truly, Puranjay Mohan
