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

2015-11-23 Thread Shi, Yang

Hi folks,

Any more comments on this patch (store immediate only)?

I need more time to add XADD (I'm supposed everyone agrees it is 
equivalent to atomic_add). However, this one is irrelevant to XADD, so 
we may be able to apply it first?


Thanks,
Yang


On 11/12/2015 7:45 PM, Z Lim wrote:

On Thu, Nov 12, 2015 at 11:33 AM, Shi, Yang  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 netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-11-12 Thread Z Lim
On Thu, Nov 12, 2015 at 11:33 AM, Shi, Yang  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 netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-11-12 Thread Shi, Yang

On 11/11/2015 4:39 AM, Will Deacon wrote:

On Wed, Nov 11, 2015 at 12:12:56PM +, 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  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 netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-11-11 Thread Will Deacon
On Wed, Nov 11, 2015 at 12:12:56PM +, 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  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 netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-11-11 Thread Will Deacon
On Tue, Nov 10, 2015 at 06:45:39PM -0800, Z Lim wrote:
> On Tue, Nov 10, 2015 at 2:41 PM, Yang Shi  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 netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-11-10 Thread Z Lim
On Tue, Nov 10, 2015 at 2:41 PM, Yang Shi  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 

> has to be implemented by the below instruction sequence:
>
> Load immediate to register
> Store register
>
> Signed-off-by: Yang Shi 
> CC: Zi Shen Lim 
> CC: Xi Wang 
> ---
>  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 netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-11-10 Thread Yang Shi
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 
CC: Zi Shen Lim 
CC: Xi Wang 
---
 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 netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html