On 2/9/22 00:21, Andrii Nakryiko wrote:
On Tue, Feb 8, 2022 at 3:09 PM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote:
On Tue, 2022-02-08 at 14:05 -0800, Andrii Nakryiko wrote:
On Mon, Feb 7, 2022 at 9:16 PM Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
wrote:
Instead of conditionally overriding PT_REGS_PARM4_SYSCALL, provide
default fallbacks for all __PT_PARMn_REG_SYSCALL macros, so that
architectures can simply override a specific syscall parameter
macro.
Also allow completely overriding PT_REGS_PARM1_SYSCALL for
non-trivial access sequences.
Co-developed-by: Naveen N. Rao <naveen.n.rao@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Naveen N. Rao <naveen.n.rao@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
---
tools/lib/bpf/bpf_tracing.h | 48 +++++++++++++++++++++++++--------
----
1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/tools/lib/bpf/bpf_tracing.h
b/tools/lib/bpf/bpf_tracing.h
index da7e8d5c939c..82f1e935d549 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -265,25 +265,43 @@ struct pt_regs;
#endif
-#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
-#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
-#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
-#ifdef __PT_PARM4_REG_SYSCALL
+#ifndef __PT_PARM1_REG_SYSCALL
+#define __PT_PARM1_REG_SYSCALL __PT_PARM1_REG
+#endif
+#ifndef __PT_PARM2_REG_SYSCALL
+#define __PT_PARM2_REG_SYSCALL __PT_PARM2_REG
+#endif
+#ifndef __PT_PARM3_REG_SYSCALL
+#define __PT_PARM3_REG_SYSCALL __PT_PARM3_REG
+#endif
+#ifndef __PT_PARM4_REG_SYSCALL
+#define __PT_PARM4_REG_SYSCALL __PT_PARM4_REG
+#endif
+#ifndef __PT_PARM5_REG_SYSCALL
+#define __PT_PARM5_REG_SYSCALL __PT_PARM5_REG
+#endif
+
+#ifndef PT_REGS_PARM1_SYSCALL
+#define PT_REGS_PARM1_SYSCALL(x) (__PT_REGS_CAST(x)-
__PT_PARM1_REG_SYSCALL)
+#endif
+#ifndef PT_REGS_PARM2_SYSCALL
+#define PT_REGS_PARM2_SYSCALL(x) (__PT_REGS_CAST(x)-
__PT_PARM2_REG_SYSCALL)
+#endif
+#ifndef PT_REGS_PARM3_SYSCALL
+#define PT_REGS_PARM3_SYSCALL(x) (__PT_REGS_CAST(x)-
__PT_PARM3_REG_SYSCALL)
+#endif
+#ifndef PT_REGS_PARM4_SYSCALL
#define PT_REGS_PARM4_SYSCALL(x) (__PT_REGS_CAST(x)-
__PT_PARM4_REG_SYSCALL)
-#else /* __PT_PARM4_REG_SYSCALL */
-#define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x)
#endif
-#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
+#ifndef PT_REGS_PARM5_SYSCALL
+#define PT_REGS_PARM5_SYSCALL(x) (__PT_REGS_CAST(x)-
__PT_PARM5_REG_SYSCALL)
+#endif
-#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
-#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
-#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
-#ifdef __PT_PARM4_REG_SYSCALL
+#define PT_REGS_PARM1_CORE_SYSCALL(x)
BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM1_REG_SYSCALL)
+#define PT_REGS_PARM2_CORE_SYSCALL(x)
BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM2_REG_SYSCALL)
+#define PT_REGS_PARM3_CORE_SYSCALL(x)
BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM3_REG_SYSCALL)
#define PT_REGS_PARM4_CORE_SYSCALL(x)
BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG_SYSCALL)
-#else /* __PT_PARM4_REG_SYSCALL */
-#define PT_REGS_PARM4_CORE_SYSCALL(x) PT_REGS_PARM4_CORE(x)
-#endif
-#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
+#define PT_REGS_PARM5_CORE_SYSCALL(x)
BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM5_REG_SYSCALL)
No, please don't do it. It makes CORE variants too rigid. We agreed
w/
Naveen that the way you did it in v2 is better and more flexible and
in v3 you did it the other way. Why?
As far as I remember we didn't discuss this proposal from Naveen [1] -
there was another one about moving SYS_PREFIX to libbpf, where
we agreed that it would have bad consequences.
Alright, I guess I never submitted my opposition to what Naveen
proposed. But I did land the v3 version of that patch, didn't I? Why
change something that's already accepted?
Right. Sorry, I just wanted to use this opportunity to clean up things a
little.
Isn't this patch essentially equivalent to the one from my v3 [2],
but with the added ability to override more things and better-looking?
No, it's not. We want to override entire PT_REGS_PARM1_CORE_SYSCALL
definition to be something like BPF_CORE_READ((struct pt_regs___s390x
*)x, orig_gpr2), while you are making PT_REGS_PARM1_CORE_SYSCALL
definition very rigid.
Right, now that we've decided to use flavors, this is no longer useful.
I'll drop it for v5.
I.e.: if we define __PT_PARMn_REG_SYSCALL, then PT_REGS_PARMn_SYSCALL
and PT_REGS_PARMn_CORE_SYSCALL use that, and __PT_PARMn_REG otherwise.
[1]
https://lore.kernel.org/bpf/1643990954.fs9q9mrdxt.naveen@xxxxxxxxxxxxx/
[2]
https://lore.kernel.org/bpf/20220204145018.1983773-5-iii@xxxxxxxxxxxxx/
#else /* defined(bpf_target_defined) */
--
2.34.1
