[PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail

George Guo posted 1 patch 1 year ago
arch/loongarch/net/bpf_jit.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail
Posted by George Guo 1 year ago
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
Re: [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail
Posted by Daniel Borkmann 1 year ago
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;
>
Re: [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail
Posted by WANG Xuerui 1 year ago
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/

Re: [PATCH v2] loongarch/bpf: Skip speculation barrier opcode, which caused ltp testcase bpf_prog02 to fail
Posted by Daniel Borkmann 1 year ago
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