* [PATCH 0/2] MIPS: Implement two workarounds for BPF JIT @ 2023-02-22 16:12 Jiaxun Yang 2023-02-22 16:12 ` [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds Jiaxun Yang 2023-02-22 16:12 ` [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds Jiaxun Yang 0 siblings, 2 replies; 13+ messages in thread From: Jiaxun Yang @ 2023-02-22 16:12 UTC (permalink / raw) To: linux-mips Cc: linux-kernel, tsbogend, johan.almbladh, paulburton, bpf, Jiaxun Yang Hi all, Just noticed eBPF JIT is not working on R4000 when messing around with QEMU. This patchset implements two workarounds that is blocking us from enabling eBPF JIT on R4000. Thanks. - Jiaxun Jiaxun Yang (2): MIPS: ebpf jit: Implement DADDI workarounds MIPS: ebpf jit: Implement R4000 workarounds arch/mips/Kconfig | 5 +---- arch/mips/net/bpf_jit_comp.c | 8 ++++++++ arch/mips/net/bpf_jit_comp32.c | 4 ++++ arch/mips/net/bpf_jit_comp64.c | 3 +++ 4 files changed, 16 insertions(+), 4 deletions(-) -- 2.37.1 (Apple Git-137.1) ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds 2023-02-22 16:12 [PATCH 0/2] MIPS: Implement two workarounds for BPF JIT Jiaxun Yang @ 2023-02-22 16:12 ` Jiaxun Yang 2023-02-23 10:10 ` Johan Almbladh 2023-02-22 16:12 ` [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds Jiaxun Yang 1 sibling, 1 reply; 13+ messages in thread From: Jiaxun Yang @ 2023-02-22 16:12 UTC (permalink / raw) To: linux-mips Cc: linux-kernel, tsbogend, johan.almbladh, paulburton, bpf, Jiaxun Yang For DADDI errata we just workaround by disable immediate operation for BPF_ADD / BPF_SUB to avoid generation of DADDIU. All other use cases in JIT won't cause overflow thus they are all safe. Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> --- arch/mips/Kconfig | 1 - arch/mips/net/bpf_jit_comp.c | 8 ++++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 37072e15b263..df0910e3895c 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -64,7 +64,6 @@ config MIPS select HAVE_DMA_CONTIGUOUS select HAVE_DYNAMIC_FTRACE select HAVE_EBPF_JIT if !CPU_MICROMIPS && \ - !CPU_DADDI_WORKAROUNDS && \ !CPU_R4000_WORKAROUNDS && \ !CPU_R4400_WORKAROUNDS select HAVE_EXIT_THREAD diff --git a/arch/mips/net/bpf_jit_comp.c b/arch/mips/net/bpf_jit_comp.c index b17130d510d4..7110a6687f7a 100644 --- a/arch/mips/net/bpf_jit_comp.c +++ b/arch/mips/net/bpf_jit_comp.c @@ -218,9 +218,17 @@ bool valid_alu_i(u8 op, s32 imm) /* All legal eBPF values are valid */ return true; case BPF_ADD: +#ifdef CONFIG_64BIT + if (IS_ENABLED(CONFIG_CPU_DADDI_WORKAROUNDS)) + return false; +#endif /* imm must be 16 bits */ return imm >= -0x8000 && imm <= 0x7fff; case BPF_SUB: +#ifdef CONFIG_64BIT + if (IS_ENABLED(CONFIG_CPU_DADDI_WORKAROUNDS)) + return false; +#endif /* -imm must be 16 bits */ return imm >= -0x7fff && imm <= 0x8000; case BPF_AND: -- 2.37.1 (Apple Git-137.1) ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds 2023-02-22 16:12 ` [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds Jiaxun Yang @ 2023-02-23 10:10 ` Johan Almbladh 2023-02-23 10:29 ` Jiaxun Yang 0 siblings, 1 reply; 13+ messages in thread From: Johan Almbladh @ 2023-02-23 10:10 UTC (permalink / raw) To: Jiaxun Yang Cc: linux-mips, linux-kernel, tsbogend, paulburton, bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko On Wed, Feb 22, 2023 at 5:12 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > > For DADDI errata we just workaround by disable immediate operation > for BPF_ADD / BPF_SUB to avoid generation of DADDIU. Good, this is an elegant solution to trigger fallback to the register-only operation. Does the DADDI errata only affect the DADDIU, not DADDI? > > All other use cases in JIT won't cause overflow thus they are all safe. There are quite a few other places where DADDIU is emitted. How do you know those are safe? I am interested in your reasoning here, as I don't know what would be safe and not. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > --- > arch/mips/Kconfig | 1 - > arch/mips/net/bpf_jit_comp.c | 8 ++++++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index 37072e15b263..df0910e3895c 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -64,7 +64,6 @@ config MIPS > select HAVE_DMA_CONTIGUOUS > select HAVE_DYNAMIC_FTRACE > select HAVE_EBPF_JIT if !CPU_MICROMIPS && \ > - !CPU_DADDI_WORKAROUNDS && \ > !CPU_R4000_WORKAROUNDS && \ > !CPU_R4400_WORKAROUNDS > select HAVE_EXIT_THREAD > diff --git a/arch/mips/net/bpf_jit_comp.c b/arch/mips/net/bpf_jit_comp.c > index b17130d510d4..7110a6687f7a 100644 > --- a/arch/mips/net/bpf_jit_comp.c > +++ b/arch/mips/net/bpf_jit_comp.c > @@ -218,9 +218,17 @@ bool valid_alu_i(u8 op, s32 imm) > /* All legal eBPF values are valid */ > return true; > case BPF_ADD: > +#ifdef CONFIG_64BIT DADDI/DADDIU are only available on 64-bit CPUs, so the errata would only be applicable to that. No need for the CONFIG_64BIT conditional. > + if (IS_ENABLED(CONFIG_CPU_DADDI_WORKAROUNDS)) > + return false; > +#endif > /* imm must be 16 bits */ > return imm >= -0x8000 && imm <= 0x7fff; > case BPF_SUB: > +#ifdef CONFIG_64BIT > + if (IS_ENABLED(CONFIG_CPU_DADDI_WORKAROUNDS)) > + return false; > +#endif > /* -imm must be 16 bits */ > return imm >= -0x7fff && imm <= 0x8000; > case BPF_AND: > -- > 2.37.1 (Apple Git-137.1) > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds 2023-02-23 10:10 ` Johan Almbladh @ 2023-02-23 10:29 ` Jiaxun Yang 2023-02-27 12:17 ` Johan Almbladh 0 siblings, 1 reply; 13+ messages in thread From: Jiaxun Yang @ 2023-02-23 10:29 UTC (permalink / raw) To: Johan Almbladh Cc: linux-mips@vger.kernel.org, linux-kernel, Thomas Bogendoerfer, paulburton@kernel.org, bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko > 2023年2月23日 10:10，Johan Almbladh <johan.almbladh@anyfinetworks.com> 写道： > > On Wed, Feb 22, 2023 at 5:12 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: >> >> For DADDI errata we just workaround by disable immediate operation >> for BPF_ADD / BPF_SUB to avoid generation of DADDIU. > > Good, this is an elegant solution to trigger fallback to the > register-only operation. Does the DADDI errata only affect the DADDIU, > not DADDI? I didn’t see any place emitting DADDI. > >> >> All other use cases in JIT won't cause overflow thus they are all safe. > > There are quite a few other places where DADDIU is emitted. How do you > know those are safe? I am interested in your reasoning here, as I > don't know what would be safe and not. Yes I analysed all other place, most of them are just calculating memory address offsets and they should never overflow. Other two is doing addition to zero to load immediate, which should be still fine. > >> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >> --- >> arch/mips/Kconfig | 1 - >> arch/mips/net/bpf_jit_comp.c | 8 ++++++++ >> 2 files changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig >> index 37072e15b263..df0910e3895c 100644 >> --- a/arch/mips/Kconfig >> +++ b/arch/mips/Kconfig >> @@ -64,7 +64,6 @@ config MIPS >> select HAVE_DMA_CONTIGUOUS >> select HAVE_DYNAMIC_FTRACE >> select HAVE_EBPF_JIT if !CPU_MICROMIPS && \ >> - !CPU_DADDI_WORKAROUNDS && \ >> !CPU_R4000_WORKAROUNDS && \ >> !CPU_R4400_WORKAROUNDS >> select HAVE_EXIT_THREAD >> diff --git a/arch/mips/net/bpf_jit_comp.c b/arch/mips/net/bpf_jit_comp.c >> index b17130d510d4..7110a6687f7a 100644 >> --- a/arch/mips/net/bpf_jit_comp.c >> +++ b/arch/mips/net/bpf_jit_comp.c >> @@ -218,9 +218,17 @@ bool valid_alu_i(u8 op, s32 imm) >> /* All legal eBPF values are valid */ >> return true; >> case BPF_ADD: >> +#ifdef CONFIG_64BIT > > DADDI/DADDIU are only available on 64-bit CPUs, so the errata would > only be applicable to that. No need for the CONFIG_64BIT conditional. It’s possible to compile a 32bit kernel for R4000 with CONFIG_CPU_DADDI_WORKAROUNDS enabled. Thanks - Jiaxun ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds 2023-02-23 10:29 ` Jiaxun Yang @ 2023-02-27 12:17 ` Johan Almbladh 2023-02-27 15:17 ` Maciej W. Rozycki 0 siblings, 1 reply; 13+ messages in thread From: Johan Almbladh @ 2023-02-27 12:17 UTC (permalink / raw) To: Jiaxun Yang Cc: linux-mips@vger.kernel.org, linux-kernel, Thomas Bogendoerfer, paulburton@kernel.org, bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko On Thu, Feb 23, 2023 at 11:29 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > I didn’t see any place emitting DADDI. Right, the JIT only uses unsigned arithmetics :) > Yes I analysed all other place, most of them are just calculating memory > address offsets and they should never overflow. Other two is doing addition > to zero to load immediate, which should be still fine. Ok. > >> --- a/arch/mips/net/bpf_jit_comp.c > >> +++ b/arch/mips/net/bpf_jit_comp.c > >> @@ -218,9 +218,17 @@ bool valid_alu_i(u8 op, s32 imm) > >> /* All legal eBPF values are valid */ > >> return true; > >> case BPF_ADD: > >> +#ifdef CONFIG_64BIT > > > > DADDI/DADDIU are only available on 64-bit CPUs, so the errata would > > only be applicable to that. No need for the CONFIG_64BIT conditional. > > It’s possible to compile a 32bit kernel for R4000 with CONFIG_CPU_DADDI_WORKAROUNDS > enabled. Yes, but DADDI/DADDIU are 64-bit instructions so they would not be available when compiling the kernel in 32-bit mode for R4000, and hence the workaround would not be applicable, right? If this is correct, I would imagine CONFIG_CPU_DADDI_WORKAROUNDS itself to be conditional on CONFIG_64BIT. That way the this relationship is expressed once in the Kconfig file, instead of being spread out over multiple places in the code. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds 2023-02-27 12:17 ` Johan Almbladh @ 2023-02-27 15:17 ` Maciej W. Rozycki 0 siblings, 0 replies; 13+ messages in thread From: Maciej W. Rozycki @ 2023-02-27 15:17 UTC (permalink / raw) To: Johan Almbladh Cc: Jiaxun Yang, linux-mips@vger.kernel.org, linux-kernel, Thomas Bogendoerfer, paulburton@kernel.org, bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko On Mon, 27 Feb 2023, Johan Almbladh wrote: > > > DADDI/DADDIU are only available on 64-bit CPUs, so the errata would > > > only be applicable to that. No need for the CONFIG_64BIT conditional. > > > > It’s possible to compile a 32bit kernel for R4000 with CONFIG_CPU_DADDI_WORKAROUNDS > > enabled. > > Yes, but DADDI/DADDIU are 64-bit instructions so they would not be > available when compiling the kernel in 32-bit mode for R4000, and > hence the workaround would not be applicable, right? If this is > correct, I would imagine CONFIG_CPU_DADDI_WORKAROUNDS itself to be > conditional on CONFIG_64BIT. That way the this relationship is > expressed once in the Kconfig file, instead of being spread out over > multiple places in the code. It is: select CPU_DADDI_WORKAROUNDS if 64BIT It only applies to 64-bit operations which are not used in 32-bit code. Maciej ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds 2023-02-22 16:12 [PATCH 0/2] MIPS: Implement two workarounds for BPF JIT Jiaxun Yang 2023-02-22 16:12 ` [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds Jiaxun Yang @ 2023-02-22 16:12 ` Jiaxun Yang 2023-02-23 10:22 ` Johan Almbladh 2023-02-26 22:09 ` Philippe Mathieu-Daudé 1 sibling, 2 replies; 13+ messages in thread From: Jiaxun Yang @ 2023-02-22 16:12 UTC (permalink / raw) To: linux-mips Cc: linux-kernel, tsbogend, johan.almbladh, paulburton, bpf, Jiaxun Yang For R4000 erratas around multiplication and division instructions, as our use of those instructions are always followed by mflo/mfhi instructions, the only issue we need care is "MIPS R4000PC/SC Errata, Processor Revision 2.2 and 3.0" Errata 28: "A double-word or a variable shift may give an incorrect result if executed while an integer multiplication is in progress." We just emit a mfhi $0 to ensure the operation is completed after every multiplication instruction accorading to workaround suggestion in the document. Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> --- arch/mips/Kconfig | 4 +--- arch/mips/net/bpf_jit_comp32.c | 4 ++++ arch/mips/net/bpf_jit_comp64.c | 3 +++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index df0910e3895c..5ea07c833c5b 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -63,9 +63,7 @@ config MIPS select HAVE_DEBUG_STACKOVERFLOW select HAVE_DMA_CONTIGUOUS select HAVE_DYNAMIC_FTRACE - select HAVE_EBPF_JIT if !CPU_MICROMIPS && \ - !CPU_R4000_WORKAROUNDS && \ - !CPU_R4400_WORKAROUNDS + select HAVE_EBPF_JIT if !CPU_MICROMIPS select HAVE_EXIT_THREAD select HAVE_FAST_GUP select HAVE_FTRACE_MCOUNT_RECORD diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c index ace5db3fbd17..fee334544d2f 100644 --- a/arch/mips/net/bpf_jit_comp32.c +++ b/arch/mips/net/bpf_jit_comp32.c @@ -446,6 +446,9 @@ static void emit_mul_i64(struct jit_context *ctx, const u8 dst[], s32 imm) } else { emit(ctx, multu, hi(dst), src); emit(ctx, mflo, hi(dst)); + /* Ensure multiplication is completed */ + if (IS_ENABLED(CONFIG_CPU_R4000_WORKAROUNDS)) + emit(ctx, mfhi, MIPS_R_ZERO); } /* hi(dst) = hi(dst) - lo(dst) */ @@ -504,6 +507,7 @@ static void emit_mul_r64(struct jit_context *ctx, } else { emit(ctx, multu, lo(dst), lo(src)); emit(ctx, mflo, lo(dst)); + /* No need for workaround because we have this mfhi */ emit(ctx, mfhi, tmp); } diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c index 0e7c1bdcf914..5f5a93f997bc 100644 --- a/arch/mips/net/bpf_jit_comp64.c +++ b/arch/mips/net/bpf_jit_comp64.c @@ -228,6 +228,9 @@ static void emit_alu_r64(struct jit_context *ctx, u8 dst, u8 src, u8 op) } else { emit(ctx, dmultu, dst, src); emit(ctx, mflo, dst); + /* Ensure multiplication is completed */ + if (IS_ENABLED(CONFIG_CPU_R4000_WORKAROUNDS)) + emit(ctx, mfhi, MIPS_R_ZERO); } break; /* dst = dst / src */ -- 2.37.1 (Apple Git-137.1) ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds 2023-02-22 16:12 ` [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds Jiaxun Yang @ 2023-02-23 10:22 ` Johan Almbladh 2023-02-23 10:31 ` Jiaxun Yang 2023-02-26 22:09 ` Philippe Mathieu-Daudé 1 sibling, 1 reply; 13+ messages in thread From: Johan Almbladh @ 2023-02-23 10:22 UTC (permalink / raw) To: Jiaxun Yang Cc: linux-mips, linux-kernel, tsbogend, paulburton, bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann On Wed, Feb 22, 2023 at 5:12 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > > For R4000 erratas around multiplication and division instructions, > as our use of those instructions are always followed by mflo/mfhi > instructions, the only issue we need care is > > "MIPS R4000PC/SC Errata, Processor Revision 2.2 and 3.0" Errata 28: > "A double-word or a variable shift may give an incorrect result if > executed while an integer multiplication is in progress." > > We just emit a mfhi $0 to ensure the operation is completed after > every multiplication instruction accorading to workaround suggestion > in the document. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > --- > arch/mips/Kconfig | 4 +--- > arch/mips/net/bpf_jit_comp32.c | 4 ++++ > arch/mips/net/bpf_jit_comp64.c | 3 +++ > 3 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index df0910e3895c..5ea07c833c5b 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -63,9 +63,7 @@ config MIPS > select HAVE_DEBUG_STACKOVERFLOW > select HAVE_DMA_CONTIGUOUS > select HAVE_DYNAMIC_FTRACE > - select HAVE_EBPF_JIT if !CPU_MICROMIPS && \ > - !CPU_R4000_WORKAROUNDS && \ > - !CPU_R4400_WORKAROUNDS Is the R4400 errata also covered by this workaround? > + select HAVE_EBPF_JIT if !CPU_MICROMIPS > select HAVE_EXIT_THREAD > select HAVE_FAST_GUP > select HAVE_FTRACE_MCOUNT_RECORD > diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c > index ace5db3fbd17..fee334544d2f 100644 > --- a/arch/mips/net/bpf_jit_comp32.c > +++ b/arch/mips/net/bpf_jit_comp32.c > @@ -446,6 +446,9 @@ static void emit_mul_i64(struct jit_context *ctx, const u8 dst[], s32 imm) > } else { > emit(ctx, multu, hi(dst), src); > emit(ctx, mflo, hi(dst)); > + /* Ensure multiplication is completed */ > + if (IS_ENABLED(CONFIG_CPU_R4000_WORKAROUNDS)) > + emit(ctx, mfhi, MIPS_R_ZERO); > } > > /* hi(dst) = hi(dst) - lo(dst) */ > @@ -504,6 +507,7 @@ static void emit_mul_r64(struct jit_context *ctx, > } else { > emit(ctx, multu, lo(dst), lo(src)); > emit(ctx, mflo, lo(dst)); > + /* No need for workaround because we have this mfhi */ > emit(ctx, mfhi, tmp); > } R4000 is a 64-bit CPU, so the 32-bit JIT implementation will not be used. From the Makefile: ifeq ($(CONFIG_32BIT),y) obj-$(CONFIG_BPF_JIT) += bpf_jit_comp32.o else obj-$(CONFIG_BPF_JIT) += bpf_jit_comp64.o endif > > diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c > index 0e7c1bdcf914..5f5a93f997bc 100644 > --- a/arch/mips/net/bpf_jit_comp64.c > +++ b/arch/mips/net/bpf_jit_comp64.c > @@ -228,6 +228,9 @@ static void emit_alu_r64(struct jit_context *ctx, u8 dst, u8 src, u8 op) > } else { > emit(ctx, dmultu, dst, src); > emit(ctx, mflo, dst); > + /* Ensure multiplication is completed */ > + if (IS_ENABLED(CONFIG_CPU_R4000_WORKAROUNDS)) > + emit(ctx, mfhi, MIPS_R_ZERO); > } > break; > /* dst = dst / src */ > -- > 2.37.1 (Apple Git-137.1) > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds 2023-02-23 10:22 ` Johan Almbladh @ 2023-02-23 10:31 ` Jiaxun Yang 2023-02-27 12:19 ` Johan Almbladh 0 siblings, 1 reply; 13+ messages in thread From: Jiaxun Yang @ 2023-02-23 10:31 UTC (permalink / raw) To: Johan Almbladh Cc: linux-mips@vger.kernel.org, linux-kernel, Thomas Bogendoerfer, paulburton@kernel.org, bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann > 2023年2月23日 10:22，Johan Almbladh <johan.almbladh@anyfinetworks.com> 写道： > > On Wed, Feb 22, 2023 at 5:12 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: >> >> For R4000 erratas around multiplication and division instructions, >> as our use of those instructions are always followed by mflo/mfhi >> instructions, the only issue we need care is >> >> "MIPS R4000PC/SC Errata, Processor Revision 2.2 and 3.0" Errata 28: >> "A double-word or a variable shift may give an incorrect result if >> executed while an integer multiplication is in progress." >> >> We just emit a mfhi $0 to ensure the operation is completed after >> every multiplication instruction accorading to workaround suggestion >> in the document. >> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> >> --- >> arch/mips/Kconfig | 4 +--- >> arch/mips/net/bpf_jit_comp32.c | 4 ++++ >> arch/mips/net/bpf_jit_comp64.c | 3 +++ >> 3 files changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig >> index df0910e3895c..5ea07c833c5b 100644 >> --- a/arch/mips/Kconfig >> +++ b/arch/mips/Kconfig >> @@ -63,9 +63,7 @@ config MIPS >> select HAVE_DEBUG_STACKOVERFLOW >> select HAVE_DMA_CONTIGUOUS >> select HAVE_DYNAMIC_FTRACE >> - select HAVE_EBPF_JIT if !CPU_MICROMIPS && \ >> - !CPU_R4000_WORKAROUNDS && \ >> - !CPU_R4400_WORKAROUNDS > > Is the R4400 errata also covered by this workaround? Yes, R4400 errata is basically a reduced version of R4000 one. They managed to fix some parts of the issue but not all. > >> + select HAVE_EBPF_JIT if !CPU_MICROMIPS >> select HAVE_EXIT_THREAD >> select HAVE_FAST_GUP >> select HAVE_FTRACE_MCOUNT_RECORD >> diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c >> index ace5db3fbd17..fee334544d2f 100644 >> --- a/arch/mips/net/bpf_jit_comp32.c >> +++ b/arch/mips/net/bpf_jit_comp32.c >> @@ -446,6 +446,9 @@ static void emit_mul_i64(struct jit_context *ctx, const u8 dst[], s32 imm) >> } else { >> emit(ctx, multu, hi(dst), src); >> emit(ctx, mflo, hi(dst)); >> + /* Ensure multiplication is completed */ >> + if (IS_ENABLED(CONFIG_CPU_R4000_WORKAROUNDS)) >> + emit(ctx, mfhi, MIPS_R_ZERO); >> } >> >> /* hi(dst) = hi(dst) - lo(dst) */ >> @@ -504,6 +507,7 @@ static void emit_mul_r64(struct jit_context *ctx, >> } else { >> emit(ctx, multu, lo(dst), lo(src)); >> emit(ctx, mflo, lo(dst)); >> + /* No need for workaround because we have this mfhi */ >> emit(ctx, mfhi, tmp); >> } > > R4000 is a 64-bit CPU, so the 32-bit JIT implementation will not be > used. From the Makefile: > > ifeq ($(CONFIG_32BIT),y) > obj-$(CONFIG_BPF_JIT) += bpf_jit_comp32.o > else > obj-$(CONFIG_BPF_JIT) += bpf_jit_comp64.o > endif It’s common practice to run 32-bit kernel on R4000 based systems to save some memory :-) Thanks. - Jiaxun > >> >> diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c >> index 0e7c1bdcf914..5f5a93f997bc 100644 >> --- a/arch/mips/net/bpf_jit_comp64.c >> +++ b/arch/mips/net/bpf_jit_comp64.c >> @@ -228,6 +228,9 @@ static void emit_alu_r64(struct jit_context *ctx, u8 dst, u8 src, u8 op) >> } else { >> emit(ctx, dmultu, dst, src); >> emit(ctx, mflo, dst); >> + /* Ensure multiplication is completed */ >> + if (IS_ENABLED(CONFIG_CPU_R4000_WORKAROUNDS)) >> + emit(ctx, mfhi, MIPS_R_ZERO); >> } >> break; >> /* dst = dst / src */ >> -- >> 2.37.1 (Apple Git-137.1) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds 2023-02-23 10:31 ` Jiaxun Yang @ 2023-02-27 12:19 ` Johan Almbladh 2023-02-27 15:18 ` Maciej W. Rozycki 0 siblings, 1 reply; 13+ messages in thread From: Johan Almbladh @ 2023-02-27 12:19 UTC (permalink / raw) To: Jiaxun Yang Cc: linux-mips@vger.kernel.org, linux-kernel, Thomas Bogendoerfer, paulburton@kernel.org, bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann On Thu, Feb 23, 2023 at 11:32 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote: > >> --- a/arch/mips/Kconfig > >> +++ b/arch/mips/Kconfig > >> @@ -63,9 +63,7 @@ config MIPS > >> select HAVE_DEBUG_STACKOVERFLOW > >> select HAVE_DMA_CONTIGUOUS > >> select HAVE_DYNAMIC_FTRACE > >> - select HAVE_EBPF_JIT if !CPU_MICROMIPS && \ > >> - !CPU_R4000_WORKAROUNDS && \ > >> - !CPU_R4400_WORKAROUNDS > > > > Is the R4400 errata also covered by this workaround? > > Yes, R4400 errata is basically a reduced version of R4000 one. > They managed to fix some parts of the issue but not all. Ok. > >> --- a/arch/mips/net/bpf_jit_comp32.c > >> +++ b/arch/mips/net/bpf_jit_comp32.c > >> @@ -446,6 +446,9 @@ static void emit_mul_i64(struct jit_context *ctx, const u8 dst[], s32 imm) > >> } else { > >> emit(ctx, multu, hi(dst), src); > >> emit(ctx, mflo, hi(dst)); > >> + /* Ensure multiplication is completed */ > >> + if (IS_ENABLED(CONFIG_CPU_R4000_WORKAROUNDS)) > >> + emit(ctx, mfhi, MIPS_R_ZERO); > >> } > >> > >> /* hi(dst) = hi(dst) - lo(dst) */ > >> @@ -504,6 +507,7 @@ static void emit_mul_r64(struct jit_context *ctx, > >> } else { > >> emit(ctx, multu, lo(dst), lo(src)); > >> emit(ctx, mflo, lo(dst)); > >> + /* No need for workaround because we have this mfhi */ For context, please specify which workaround this comment refers to: "workaround" -> "R4000 workaround". > > R4000 is a 64-bit CPU, so the 32-bit JIT implementation will not be > > used. From the Makefile: > > > > ifeq ($(CONFIG_32BIT),y) > > obj-$(CONFIG_BPF_JIT) += bpf_jit_comp32.o > > else > > obj-$(CONFIG_BPF_JIT) += bpf_jit_comp64.o > > endif > > It’s common practice to run 32-bit kernel on R4000 based systems to save some memory :-) Ok, I understand. Looks good! I have run the test_bpf.ko test suite on MIPS and MIPS64 in QEMU with and without the workarounds enabled. With above comment fix: Acked-by: Johan Almbladh <johan.almbladh@anyfinetworks.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds 2023-02-27 12:19 ` Johan Almbladh @ 2023-02-27 15:18 ` Maciej W. Rozycki 2023-02-27 15:47 ` Jiaxun Yang 0 siblings, 1 reply; 13+ messages in thread From: Maciej W. Rozycki @ 2023-02-27 15:18 UTC (permalink / raw) To: Johan Almbladh Cc: Jiaxun Yang, linux-mips@vger.kernel.org, linux-kernel, Thomas Bogendoerfer, paulburton@kernel.org, bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann On Mon, 27 Feb 2023, Johan Almbladh wrote: > > > R4000 is a 64-bit CPU, so the 32-bit JIT implementation will not be > > > used. From the Makefile: > > > > > > ifeq ($(CONFIG_32BIT),y) > > > obj-$(CONFIG_BPF_JIT) += bpf_jit_comp32.o > > > else > > > obj-$(CONFIG_BPF_JIT) += bpf_jit_comp64.o > > > endif > > > > It’s common practice to run 32-bit kernel on R4000 based systems to save some memory :-) > > Ok, I understand. Likewise: select CPU_R4000_WORKAROUNDS if 64BIT select CPU_R4400_WORKAROUNDS if 64BIT This only applies to 64-bit operations, which are not used in 32-bit code (one reason why these early silicon revisions were originally used with 32-bit software only). Maciej ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds 2023-02-27 15:18 ` Maciej W. Rozycki @ 2023-02-27 15:47 ` Jiaxun Yang 0 siblings, 0 replies; 13+ messages in thread From: Jiaxun Yang @ 2023-02-27 15:47 UTC (permalink / raw) To: Maciej W. Rozycki, Johan Almbladh Cc: linux-mips@vger.kernel.org, linux-kernel, Thomas Bogendoerfer, paulburton@kernel.org, bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann 在2023年2月27日二月 下午3:18，Maciej W. Rozycki写道： > On Mon, 27 Feb 2023, Johan Almbladh wrote: > >> > > R4000 is a 64-bit CPU, so the 32-bit JIT implementation will not be >> > > used. From the Makefile: >> > > >> > > ifeq ($(CONFIG_32BIT),y) >> > > obj-$(CONFIG_BPF_JIT) += bpf_jit_comp32.o >> > > else >> > > obj-$(CONFIG_BPF_JIT) += bpf_jit_comp64.o >> > > endif >> > >> > It’s common practice to run 32-bit kernel on R4000 based systems to save some memory :-) >> >> Ok, I understand. > > Likewise: > > select CPU_R4000_WORKAROUNDS if 64BIT > select CPU_R4400_WORKAROUNDS if 64BIT > > This only applies to 64-bit operations, which are not used in 32-bit code > (one reason why these early silicon revisions were originally used with > 32-bit software only). Thanks for the info. Will drop 32bit part from both patch. > > Maciej -- - Jiaxun ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds 2023-02-22 16:12 ` [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds Jiaxun Yang 2023-02-23 10:22 ` Johan Almbladh @ 2023-02-26 22:09 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 13+ messages in thread From: Philippe Mathieu-Daudé @ 2023-02-26 22:09 UTC (permalink / raw) To: Jiaxun Yang, linux-mips Cc: linux-kernel, tsbogend, johan.almbladh, paulburton, bpf On 22/2/23 17:12, Jiaxun Yang wrote: > For R4000 erratas around multiplication and division instructions, > as our use of those instructions are always followed by mflo/mfhi > instructions, the only issue we need care is > > "MIPS R4000PC/SC Errata, Processor Revision 2.2 and 3.0" Errata 28: > "A double-word or a variable shift may give an incorrect result if > executed while an integer multiplication is in progress." > > We just emit a mfhi $0 to ensure the operation is completed after > every multiplication instruction accorading to workaround suggestion Typo "according" > in the document. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > --- > arch/mips/Kconfig | 4 +--- > arch/mips/net/bpf_jit_comp32.c | 4 ++++ > arch/mips/net/bpf_jit_comp64.c | 3 +++ > 3 files changed, 8 insertions(+), 3 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-02-27 15:47 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-22 16:12 [PATCH 0/2] MIPS: Implement two workarounds for BPF JIT Jiaxun Yang 2023-02-22 16:12 ` [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds Jiaxun Yang 2023-02-23 10:10 ` Johan Almbladh 2023-02-23 10:29 ` Jiaxun Yang 2023-02-27 12:17 ` Johan Almbladh 2023-02-27 15:17 ` Maciej W. Rozycki 2023-02-22 16:12 ` [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds Jiaxun Yang 2023-02-23 10:22 ` Johan Almbladh 2023-02-23 10:31 ` Jiaxun Yang 2023-02-27 12:19 ` Johan Almbladh 2023-02-27 15:18 ` Maciej W. Rozycki 2023-02-27 15:47 ` Jiaxun Yang 2023-02-26 22:09 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).