All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 related	[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 related	[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 related	[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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.