* [PATCH] loongarch/bpf: Fix bpf load failed with CONFIG_BPF_JIT_ALWAYS_ON, caused by jit (BPF_ST | BPF_NOSPEC) code
@ 2023-03-26 4:40 George Guo
2023-03-27 9:29 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: George Guo @ 2023-03-26 4:40 UTC (permalink / raw)
To: chenhuacai, masahiroy, michal.lkml
Cc: kernel, ndesaulniers, daniel, ast, loongarch, linux-kernel,
linux-kbuild, bpf
Here just skip the code(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>
---
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..745d344385ed 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -1046,6 +1046,11 @@ static int build_body(struct jit_ctx *ctx, bool extra_pass)
if (ctx->image == NULL)
ctx->offset[i] = ctx->idx;
+ /* skip the code that has no couterpart to the host arch */
+ if(insn->code == (BPF_ST | BPF_NOSPEC)) {
+ continue;
+ }
+
ret = build_insn(insn, ctx, extra_pass);
if (ret > 0) {
i++;
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] loongarch/bpf: Fix bpf load failed with CONFIG_BPF_JIT_ALWAYS_ON, caused by jit (BPF_ST | BPF_NOSPEC) code
2023-03-26 4:40 [PATCH] loongarch/bpf: Fix bpf load failed with CONFIG_BPF_JIT_ALWAYS_ON, caused by jit (BPF_ST | BPF_NOSPEC) code George Guo
@ 2023-03-27 9:29 ` Daniel Borkmann
2023-03-28 7:13 ` [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail George Guo
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2023-03-27 9:29 UTC (permalink / raw)
To: George Guo, chenhuacai, masahiroy, michal.lkml
Cc: kernel, ndesaulniers, ast, loongarch, linux-kernel, linux-kbuild, bpf
On 3/26/23 6:40 AM, George Guo wrote:
> Here just skip the code(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>
> ---
> 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..745d344385ed 100644
> --- a/arch/loongarch/net/bpf_jit.c
> +++ b/arch/loongarch/net/bpf_jit.c
> @@ -1046,6 +1046,11 @@ static int build_body(struct jit_ctx *ctx, bool extra_pass)
> if (ctx->image == NULL)
> ctx->offset[i] = ctx->idx;
>
> + /* skip the code that has no couterpart to the host arch */
> + if(insn->code == (BPF_ST | BPF_NOSPEC)) {
> + continue;
> + }
Small nit, but could we align with other JIT implementations and place it into similar
location for consistency? Above looks a bit out of place and it should really be part
of build_insn.
diff --git a/arch/loongarch/net/bpf_jit.c b/arch/loongarch/net/bpf_jit.c
index 288003a9f0ca..d586df48ecc6 100644
--- a/arch/loongarch/net/bpf_jit.c
+++ b/arch/loongarch/net/bpf_jit.c
@@ -1022,6 +1022,10 @@ 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:
+ break;
+
default:
pr_err("bpf_jit: unknown opcode %02x\n", code);
return -EINVAL;
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail
2023-03-27 9:29 ` Daniel Borkmann
@ 2023-03-28 7:13 ` George Guo
2023-03-28 7:22 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: George Guo @ 2023-03-28 7:13 UTC (permalink / raw)
To: daniel
Cc: ast, bpf, chenhuacai, guodongtai, kernel, linux-kbuild,
linux-kernel, loongarch, masahiroy, michal.lkml, ndesaulniers
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;
+
default:
pr_err("bpf_jit: unknown opcode %02x\n", code);
return -EINVAL;
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail
2023-03-28 7:13 ` [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail George Guo
@ 2023-03-28 7:22 ` Daniel Borkmann
2023-03-28 7:52 ` WANG Xuerui
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2023-03-28 7:22 UTC (permalink / raw)
To: George Guo
Cc: ast, bpf, chenhuacai, kernel, linux-kbuild, linux-kernel,
loongarch, masahiroy, michal.lkml, ndesaulniers, hengqi.chen,
yangtiezhu, tangyouling
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;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail
2023-03-28 7:22 ` Daniel Borkmann
@ 2023-03-28 7:52 ` WANG Xuerui
2023-03-28 8:37 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: WANG Xuerui @ 2023-03-28 7:52 UTC (permalink / raw)
To: Daniel Borkmann, George Guo
Cc: ast, bpf, chenhuacai, linux-kbuild, linux-kernel, loongarch,
masahiroy, michal.lkml, ndesaulniers, hengqi.chen, yangtiezhu,
tangyouling
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;
>>
>
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail
2023-03-28 7:52 ` WANG Xuerui
@ 2023-03-28 8:37 ` Daniel Borkmann
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2023-03-28 8:37 UTC (permalink / raw)
To: WANG Xuerui, George Guo
Cc: ast, bpf, chenhuacai, linux-kbuild, linux-kernel, loongarch,
masahiroy, michal.lkml, ndesaulniers, hengqi.chen, yangtiezhu,
tangyouling
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-28 8:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-26 4:40 [PATCH] loongarch/bpf: Fix bpf load failed with CONFIG_BPF_JIT_ALWAYS_ON, caused by jit (BPF_ST | BPF_NOSPEC) code George Guo
2023-03-27 9:29 ` Daniel Borkmann
2023-03-28 7:13 ` [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail George Guo
2023-03-28 7:22 ` Daniel Borkmann
2023-03-28 7:52 ` WANG Xuerui
2023-03-28 8:37 ` Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®