| Message ID | 20230328071335.2664966-1-guodongtai@kylinos.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail | expand |
On 3/28/23 9:13 AM, George Guo wrote: > Here just skip the opcode(BPF_ST | BPF_NOSPEC) that has no couterpart to the loongarch. > > To verify, use ltp testcase: > > Without this patch: > $ ./bpf_prog02 > ... ... > bpf_common.c:123: TBROK: Failed verification: ??? (524) > > Summary: > passed 0 > failed 0 > broken 1 > skipped 0 > warnings 0 > > With this patch: > $ ./bpf_prog02 > ... ... > Summary: > passed 0 > failed 0 > broken 0 > skipped 0 > warnings 0 > > Signed-off-by: George Guo <guodongtai@kylinos.cn> > > --- > Changelog: > v2: > - place it to build_insn > - add printing for skipping bpf_jit the opcode > --- > arch/loongarch/net/bpf_jit.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c > index 288003a9f0ca..d3c6b1c4ccbb 100644 > --- a/arch/loongarch/net/bpf_jit.c > +++ b/arch/loongarch/net/bpf_jit.c > @@ -1022,6 +1022,11 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext > emit_atomic(insn, ctx); > break; > > + /* Speculation barrier */ > + case BPF_ST | BPF_NOSPEC: > + pr_info_once("bpf_jit: skip speculation barrier opcode %0x2x\n", code); > + break; Thanks that looks better. Question to LoongArch folks (Cc): There is no equivalent to a speculation barrier here, correct? Either way, I think the pr_info_once() can just be removed given there is little value for a users to have this in the kernel log. I can take care of this while applying, that's fine. > default: > pr_err("bpf_jit: unknown opcode %02x\n", code); > return -EINVAL; >
On 2023/3/28 15:22, Daniel Borkmann wrote: > On 3/28/23 9:13 AM, George Guo wrote: >> Here just skip the opcode(BPF_ST | BPF_NOSPEC) that has no couterpart >> to the loongarch. >> >> <snip> >> >> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c >> index 288003a9f0ca..d3c6b1c4ccbb 100644 >> --- a/arch/loongarch/net/bpf_jit.c >> +++ b/arch/loongarch/net/bpf_jit.c >> @@ -1022,6 +1022,11 @@ static int build_insn(const struct bpf_insn >> *insn, struct jit_ctx *ctx, bool ext >> emit_atomic(insn, ctx); >> break; >> + /* Speculation barrier */ >> + case BPF_ST | BPF_NOSPEC: >> + pr_info_once("bpf_jit: skip speculation barrier opcode >> %0x2x\n", code); >> + break; > > Thanks that looks better. Question to LoongArch folks (Cc): There is no > equivalent > to a speculation barrier here, correct? Either way, I think the > pr_info_once() can > just be removed given there is little value for a users to have this in > the kernel > log. I can take care of this while applying, that's fine. I can confirm there's currently no speculation barrier equivalent on lonogarch. (Loongson says there are builtin mitigations for Spectre-V1 and V2 on their chips, and AFAIK efforts to port the exploits to mips/loongarch have all failed a few years ago.) And yes I'd agree with removing the warning altogether. Thanks for the reviews! Acked-by: WANG Xuerui <git@xen0n.name> > >> default: >> pr_err("bpf_jit: unknown opcode %02x\n", code); >> return -EINVAL; >> >
On 3/28/23 9:52 AM, WANG Xuerui wrote: > On 2023/3/28 15:22, Daniel Borkmann wrote: >> On 3/28/23 9:13 AM, George Guo wrote: >>> Here just skip the opcode(BPF_ST | BPF_NOSPEC) that has no couterpart to the loongarch. >>> >>> <snip> >>> >>> diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c >>> index 288003a9f0ca..d3c6b1c4ccbb 100644 >>> --- a/arch/loongarch/net/bpf_jit.c >>> +++ b/arch/loongarch/net/bpf_jit.c >>> @@ -1022,6 +1022,11 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext >>> emit_atomic(insn, ctx); >>> break; >>> + /* Speculation barrier */ >>> + case BPF_ST | BPF_NOSPEC: >>> + pr_info_once("bpf_jit: skip speculation barrier opcode %0x2x\n", code); >>> + break; >> >> Thanks that looks better. Question to LoongArch folks (Cc): There is no equivalent >> to a speculation barrier here, correct? Either way, I think the pr_info_once() can >> just be removed given there is little value for a users to have this in the kernel >> log. I can take care of this while applying, that's fine. > > I can confirm there's currently no speculation barrier equivalent on lonogarch. (Loongson says there are builtin mitigations for Spectre-V1 and V2 on their chips, and AFAIK efforts to port the exploits to mips/loongarch have all failed a few years ago.) > > And yes I'd agree with removing the warning altogether. Thanks for the reviews! > > Acked-by: WANG Xuerui <git@xen0n.name> Ok, sounds good. I've cleaned this up and applied to bpf tree. Thanks! https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=a6f6a95f25803500079513780d11a911ce551d76
diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c index 288003a9f0ca..d3c6b1c4ccbb 100644 --- a/arch/loongarch/net/bpf_jit.c +++ b/arch/loongarch/net/bpf_jit.c @@ -1022,6 +1022,11 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, bool ext emit_atomic(insn, ctx); break; + /* Speculation barrier */ + case BPF_ST | BPF_NOSPEC: + pr_info_once("bpf_jit: skip speculation barrier opcode %0x2x\n", code); + break; + default: pr_err("bpf_jit: unknown opcode %02x\n", code); return -EINVAL;
Here just skip the opcode(BPF_ST | BPF_NOSPEC) that has no couterpart to the loongarch. To verify, use ltp testcase: Without this patch: $ ./bpf_prog02 ... ... bpf_common.c:123: TBROK: Failed verification: ??? (524) Summary: passed 0 failed 0 broken 1 skipped 0 warnings 0 With this patch: $ ./bpf_prog02 ... ... Summary: passed 0 failed 0 broken 0 skipped 0 warnings 0 Signed-off-by: George Guo <guodongtai@kylinos.cn> --- Changelog: v2: - place it to build_insn - add printing for skipping bpf_jit the opcode --- arch/loongarch/net/bpf_jit.c | 5 +++++ 1 file changed, 5 insertions(+)