From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AA23FC433F5 for ; Tue, 8 Feb 2022 22:22:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235314AbiBHWWV (ORCPT ); Tue, 8 Feb 2022 17:22:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49350 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1387192AbiBHWGI (ORCPT ); Tue, 8 Feb 2022 17:06:08 -0500 Received: from mail-io1-xd2a.google.com (mail-io1-xd2a.google.com [IPv6:2607:f8b0:4864:20::d2a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C5448C0612BC for ; Tue, 8 Feb 2022 14:06:07 -0800 (PST) Received: by mail-io1-xd2a.google.com with SMTP id s18so804620ioa.12 for ; Tue, 08 Feb 2022 14:06:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=WiGyLg8cWadUpUV/dzImwc2YaiNNyLLzayweJorPV2g=; b=K8N2Xk5fXpj1dSzgipi45E6OlRo4tBgkgPty/ybzKtLMLsr87B9ogjUtjjxXxYbDJF xxe1Sl9Xc3c9tyR9p02SWCT9a4K7GVPB6jAdsIFBbtZWgJayi+PsqgYFj2s7kBXb8MtP 5kgyYaYoBA50D3SQa7VMRXoI91GQSM3wt/pce+t1h5NBqs7TePXDpK+TXs8iJcpbaft0 xESP44JQuRl2Mu0gkiERUihf4/swNbF92CoZ6YPEjfbe2jh9zIQiZzA7BdStimusRdaH qmrLXSLdjz7BxlfnxKq/3QwxICB59LEso0LgHrNaOzu6Ah/5tGJv9frNpktPuqNLfCmo ZsGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=WiGyLg8cWadUpUV/dzImwc2YaiNNyLLzayweJorPV2g=; b=MSISM5dhN1zeNrCFm0bUdQG+gMaavf5e8s0sC1SCxb6PNWeuV1aDRaM2CbQ6HO13Bf RA5tGiwxUPmj3oqPGVICRkWRW2bLQXBRXG3zmvYd4QhGK8xuN5bmirSEBzSF95B/R/Ar aqI9r4sn/gwR80JfezjI67BdHV/Q15tai4cLykg5v7G7mgY56GwP1NSTGlFtmqA5pxt6 YC0Eo5DQAWIC30myAAvb2mRKH0jEud766jhNWP2VOQBX7aZtY4v9TFV32kzHBK4nC5uQ Ml2jZYUBM233u+o6KSTNooROe4pFaalTHHrMvgSFdUMv8KYkXHiLSPJwHh2n/XIDpfgG YDtA== X-Gm-Message-State: AOAM530N+/nrNZYxWVZ49O49jOCqcq/7CY5glaJDJFaucE9yWOdX5MtP /vWnMTSp0s2iKtKOuIFnkZpI+B4pu7aK/glxhJA= X-Google-Smtp-Source: ABdhPJztLzdHcNkzyUCG1EVRSAMQ4cWv9ApMM2+cE6RHBVwrW4JWyPU+eBovXNXfzMIEMtXGRfZ9qJqvrZBuyoq0CXk= X-Received: by 2002:a02:7417:: with SMTP id o23mr3270031jac.145.1644357966993; Tue, 08 Feb 2022 14:06:06 -0800 (PST) MIME-Version: 1.0 References: <20220208051635.2160304-1-iii@linux.ibm.com> <20220208051635.2160304-6-iii@linux.ibm.com> In-Reply-To: <20220208051635.2160304-6-iii@linux.ibm.com> From: Andrii Nakryiko Date: Tue, 8 Feb 2022 14:05:55 -0800 Message-ID: Subject: Re: [PATCH bpf-next v4 05/14] libbpf: Generalize overriding syscall parameter access macros To: Ilya Leoshkevich Cc: Alexei Starovoitov , Daniel Borkmann , Heiko Carstens , Vasily Gorbik , Christian Borntraeger , Alexander Gordeev , Catalin Marinas , Michael Ellerman , Paul Walmsley , "Naveen N . Rao" , Mark Rutland , bpf Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Mon, Feb 7, 2022 at 9:16 PM Ilya Leoshkevich 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 > Signed-off-by: Naveen N. Rao > Signed-off-by: Ilya Leoshkevich > --- > 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? > #else /* defined(bpf_target_defined) */ > > -- > 2.34.1 >