Received-SPF: pass (google.com: domain of linux-kernel+bounces-112453-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240323103057.26499-1-puranjay12@gmail.com> <20240323103057.26499-2-puranjay12@gmail.com>
In-Reply-To: <20240323103057.26499-2-puranjay12@gmail.com>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Sat, 23 Mar 2024 11:48:34 -0700
Message-ID: <CAADnVQ+-Wt=OohzVJL9g3r2pLkaomGXDLLdLdGeGYE4D+f4cAA@mail.gmail.com>
Subject: Re: [PATCH bpf-next v3 1/2] bpf: Add arm64 JIT support for
 PROBE_MEM32 pseudo instructions.
To: Puranjay Mohan <puranjay12@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, 
	John Fastabend <john.fastabend@gmail.com>, Andrii Nakryiko <andrii@kernel.org>, 
	Martin KaFai Lau <martin.lau@linux.dev>, Eduard Zingerman <eddyz87@gmail.com>, Song Liu <song@kernel.org>, 
	Yonghong Song <yonghong.song@linux.dev>, KP Singh <kpsingh@kernel.org>, 
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>, 
	bpf <bpf@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, 
	Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, 
	Zi Shen Lim <zlim.lnx@gmail.com>, Xu Kuohai <xukuohai@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, Mar 23, 2024 at 3:31=E2=80=AFAM Puranjay Mohan <puranjay12@gmail.co=
m> wrote:
>
> +#define PROBE_MEM32_BASE (MAX_BPF_JIT_REG + 5)
>
>  #define check_imm(bits, imm) do {                              \
>         if ((((imm) > 0) && ((imm) >> (bits))) ||               \
> @@ -67,6 +68,8 @@ static const int bpf2a64[] =3D {
>         /* temporary register for blinding constants */
>         [BPF_REG_AX] =3D A64_R(9),
>         [FP_BOTTOM] =3D A64_R(27),
> +       /* callee saved register for kern_vm_start address */
> +       [PROBE_MEM32_BASE] =3D A64_R(28),
>  };
>
>  struct jit_ctx {
> @@ -295,7 +298,7 @@ static bool is_lsi_offset(int offset, int scale)
>  #define PROLOGUE_OFFSET (BTI_INSNS + 2 + PAC_INSNS + 8)
>
>  static int build_prologue(struct jit_ctx *ctx, bool ebpf_from_cbpf,
> -                         bool is_exception_cb)
> +                         bool is_exception_cb, u64 arena_vm_start)
>  {
>         const struct bpf_prog *prog =3D ctx->prog;
>         const bool is_main_prog =3D !bpf_is_subprog(prog);
> @@ -306,6 +309,7 @@ static int build_prologue(struct jit_ctx *ctx, bool e=
bpf_from_cbpf,
>         const u8 fp =3D bpf2a64[BPF_REG_FP];
>         const u8 tcc =3D bpf2a64[TCALL_CNT];
>         const u8 fpb =3D bpf2a64[FP_BOTTOM];
> +       const u8 pb =3D bpf2a64[PROBE_MEM32_BASE];

In addition to riscv comments please use more verbose name here.
'pb' is too cryptic.
'mem32_base' ?

I would also drop PROBE prefix and use:
#define MEM32_BASE (MAX_BPF_JIT_REG + 5)

From the verifier pov the ld/st mode is BPF_PROBE_MEM32,
since it's asking JIT to emit code to probe read/write such arena address,
but from JIT pov the base is a real base that it got from
bpf_arena_get_kern_vm_start().
#define KERN_VM_START (MAX_BPF_JIT_REG + 5)
would be an alternative name that also fits.
or
#define ARENA_VM_START ...