Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1266843 > unrolled thread

[PATCH 0/2] arm64: bpf: add BPF_ST and BPF_XADD instructions support

Started byYang Shi <yang.shi@linaro.org>
First post2015-11-11 00:10 +0100
Last post2015-11-13 04:50 +0100
Articles 7 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/2] arm64: bpf: add BPF_ST and BPF_XADD instructions support Yang Shi <yang.shi@linaro.org> - 2015-11-11 00:10 +0100
    [PATCH 1/2] arm64: bpf: add 'store immediate' instruction Yang Shi <yang.shi@linaro.org> - 2015-11-11 00:10 +0100
      Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction Z Lim <zlim.lnx@gmail.com> - 2015-11-11 03:50 +0100
        Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction Will Deacon <will.deacon@arm.com> - 2015-11-11 13:20 +0100
          Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction Will Deacon <will.deacon@arm.com> - 2015-11-11 13:50 +0100
            Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction "Shi, Yang" <yang.shi@linaro.org> - 2015-11-12 20:40 +0100
              Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction Z Lim <zlim.lnx@gmail.com> - 2015-11-13 04:50 +0100

#1266843 — [PATCH 0/2] arm64: bpf: add BPF_ST and BPF_XADD instructions support

FromYang Shi <yang.shi@linaro.org>
Date2015-11-11 00:10 +0100
Subject[PATCH 0/2] arm64: bpf: add BPF_ST and BPF_XADD instructions support
Message-ID<qtqd4-36K-7@gated-at.bofh.it>
Current ARM64 BPF JIT doesn't have store immediate and XADD instructions
support, and aarch64 doesn't have native instructions for them. Implement
them in instruction sequence. For detail, please refer to the commit log.

The implementation is tested by test_bpf kernel module.

The patches are applied after my BPF JIT stack fix [1].

[1] https://patches.linaro.org/56268/

Yang Shi (2):
      arm64: bpf: add 'store immediate' instruction
      arm64: bpf: add BPF XADD instruction

 arch/arm64/net/bpf_jit_comp.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1266844 — [PATCH 1/2] arm64: bpf: add 'store immediate' instruction

FromYang Shi <yang.shi@linaro.org>
Date2015-11-11 00:10 +0100
Subject[PATCH 1/2] arm64: bpf: add 'store immediate' instruction
Message-ID<qtqd4-36K-19@gated-at.bofh.it>
In reply to#1266843
aarch64 doesn't have native store immediate instruction, such operation
has to be implemented by the below instruction sequence:

Load immediate to register
Store register

Signed-off-by: Yang Shi <yang.shi@linaro.org>
CC: Zi Shen Lim <zlim.lnx@gmail.com>
CC: Xi Wang <xi.wang@gmail.com>
---
 arch/arm64/net/bpf_jit_comp.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 6809647..49c1f1b 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -563,7 +563,25 @@ emit_cond_jmp:
 	case BPF_ST | BPF_MEM | BPF_H:
 	case BPF_ST | BPF_MEM | BPF_B:
 	case BPF_ST | BPF_MEM | BPF_DW:
-		goto notyet;
+		/* Load imm to a register then store it */
+		ctx->tmp_used = 1;
+		emit_a64_mov_i(1, tmp2, off, ctx);
+		emit_a64_mov_i(1, tmp, imm, ctx);
+		switch (BPF_SIZE(code)) {
+		case BPF_W:
+			emit(A64_STR32(tmp, dst, tmp2), ctx);
+			break;
+		case BPF_H:
+			emit(A64_STRH(tmp, dst, tmp2), ctx);
+			break;
+		case BPF_B:
+			emit(A64_STRB(tmp, dst, tmp2), ctx);
+			break;
+		case BPF_DW:
+			emit(A64_STR64(tmp, dst, tmp2), ctx);
+			break;
+		}
+		break;
 
 	/* STX: *(size *)(dst + off) = src */
 	case BPF_STX | BPF_MEM | BPF_W:
-- 
2.0.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1266938 — Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction

FromZ Lim <zlim.lnx@gmail.com>
Date2015-11-11 03:50 +0100
SubjectRe: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction
Message-ID<qttDX-59m-7@gated-at.bofh.it>
In reply to#1266844
On Tue, Nov 10, 2015 at 2:41 PM, Yang Shi <yang.shi@linaro.org> wrote:
> aarch64 doesn't have native store immediate instruction, such operation

Actually, aarch64 does have "STR (immediate)". For arm64 JIT, we can
consider using it as an optimization.

You may also want to consider adding a note about the corresponding test cases:
    commit cffc642d93f9 ("test_bpf: add 173 new testcases for eBPF").

Otherwise, the patch below looks good.
Reviewed-by: Zi Shen Lim <zlim.lnx@gmail.com>

> has to be implemented by the below instruction sequence:
>
> Load immediate to register
> Store register
>
> Signed-off-by: Yang Shi <yang.shi@linaro.org>
> CC: Zi Shen Lim <zlim.lnx@gmail.com>
> CC: Xi Wang <xi.wang@gmail.com>
> ---
>  arch/arm64/net/bpf_jit_comp.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index 6809647..49c1f1b 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -563,7 +563,25 @@ emit_cond_jmp:
>         case BPF_ST | BPF_MEM | BPF_H:
>         case BPF_ST | BPF_MEM | BPF_B:
>         case BPF_ST | BPF_MEM | BPF_DW:
> -               goto notyet;
> +               /* Load imm to a register then store it */
> +               ctx->tmp_used = 1;
> +               emit_a64_mov_i(1, tmp2, off, ctx);
> +               emit_a64_mov_i(1, tmp, imm, ctx);
> +               switch (BPF_SIZE(code)) {
> +               case BPF_W:
> +                       emit(A64_STR32(tmp, dst, tmp2), ctx);
> +                       break;
> +               case BPF_H:
> +                       emit(A64_STRH(tmp, dst, tmp2), ctx);
> +                       break;
> +               case BPF_B:
> +                       emit(A64_STRB(tmp, dst, tmp2), ctx);
> +                       break;
> +               case BPF_DW:
> +                       emit(A64_STR64(tmp, dst, tmp2), ctx);
> +                       break;
> +               }
> +               break;
>
>         /* STX: *(size *)(dst + off) = src */
>         case BPF_STX | BPF_MEM | BPF_W:
> --
> 2.0.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267148 — Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction

FromWill Deacon <will.deacon@arm.com>
Date2015-11-11 13:20 +0100
SubjectRe: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction
Message-ID<qtCxB-2EB-13@gated-at.bofh.it>
In reply to#1266938
On Tue, Nov 10, 2015 at 06:45:39PM -0800, Z Lim wrote:
> On Tue, Nov 10, 2015 at 2:41 PM, Yang Shi <yang.shi@linaro.org> wrote:
> > aarch64 doesn't have native store immediate instruction, such operation
> 
> Actually, aarch64 does have "STR (immediate)". For arm64 JIT, we can
> consider using it as an optimization.

Yes, I'd definitely like to see that in preference to moving via a
temporary register.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267166 — Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction

FromWill Deacon <will.deacon@arm.com>
Date2015-11-11 13:50 +0100
SubjectRe: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction
Message-ID<qtD0C-2OK-15@gated-at.bofh.it>
In reply to#1267148
On Wed, Nov 11, 2015 at 12:12:56PM +0000, Will Deacon wrote:
> On Tue, Nov 10, 2015 at 06:45:39PM -0800, Z Lim wrote:
> > On Tue, Nov 10, 2015 at 2:41 PM, Yang Shi <yang.shi@linaro.org> wrote:
> > > aarch64 doesn't have native store immediate instruction, such operation
> > 
> > Actually, aarch64 does have "STR (immediate)". For arm64 JIT, we can
> > consider using it as an optimization.
> 
> Yes, I'd definitely like to see that in preference to moving via a
> temporary register.

Wait a second, we're both talking rubbish here :) The STR (immediate)
form is referring to the addressing mode, whereas this patch wants to
store an immediate value to memory, which does need moving to a register
first.

So the original patch is fine.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1268230 — Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction

From"Shi, Yang" <yang.shi@linaro.org>
Date2015-11-12 20:40 +0100
SubjectRe: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction
Message-ID<qu5SW-4DS-13@gated-at.bofh.it>
In reply to#1267166
On 11/11/2015 4:39 AM, Will Deacon wrote:
> On Wed, Nov 11, 2015 at 12:12:56PM +0000, Will Deacon wrote:
>> On Tue, Nov 10, 2015 at 06:45:39PM -0800, Z Lim wrote:
>>> On Tue, Nov 10, 2015 at 2:41 PM, Yang Shi <yang.shi@linaro.org> wrote:
>>>> aarch64 doesn't have native store immediate instruction, such operation
>>>
>>> Actually, aarch64 does have "STR (immediate)". For arm64 JIT, we can
>>> consider using it as an optimization.
>>
>> Yes, I'd definitely like to see that in preference to moving via a
>> temporary register.
>
> Wait a second, we're both talking rubbish here :) The STR (immediate)
> form is referring to the addressing mode, whereas this patch wants to
> store an immediate value to memory, which does need moving to a register
> first.

Yes, the immediate means immediate offset for addressing index. Doesn't 
mean to store immediate to memory.

I don't think any load-store architecture has store immediate instruction.

Thanks,
Yang

>
> So the original patch is fine.
>
> Will
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1268570 — Re: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction

FromZ Lim <zlim.lnx@gmail.com>
Date2015-11-13 04:50 +0100
SubjectRe: [PATCH 1/2] arm64: bpf: add 'store immediate' instruction
Message-ID<qudx7-18i-1@gated-at.bofh.it>
In reply to#1268230
On Thu, Nov 12, 2015 at 11:33 AM, Shi, Yang <yang.shi@linaro.org> wrote:
> On 11/11/2015 4:39 AM, Will Deacon wrote:
>>
>> Wait a second, we're both talking rubbish here :) The STR (immediate)
>> form is referring to the addressing mode, whereas this patch wants to
>> store an immediate value to memory, which does need moving to a register
>> first.
>
>
> Yes, the immediate means immediate offset for addressing index. Doesn't mean
> to store immediate to memory.
>
> I don't think any load-store architecture has store immediate instruction.
>

Indeed. Sorry for the noise.

Somehow Will caught a whiff of whatever I was smoking then :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web