MIME-Version: 1.0
References: <20230222161222.11879-1-jiaxun.yang@flygoat.com>
 <20230222161222.11879-3-jiaxun.yang@flygoat.com> <CAM1=_QTDkYJANgxYwkgPZB+hUX6Rr_Pvnn7cFwSJFHQtLrpQMA@mail.gmail.com>
 <70C80F6D-A727-48FD-A767-A2CA54AA7C1E@flygoat.com>
In-Reply-To: <70C80F6D-A727-48FD-A767-A2CA54AA7C1E@flygoat.com>
From:   Johan Almbladh <johan.almbladh@anyfinetworks.com>
Date:   Mon, 27 Feb 2023 13:19:49 +0100
Message-ID: <CAM1=_QS_ewcFdrZ1ypV15wOkK_SKkb0UUe5_Ozi_CDBdxF5JmA@mail.gmail.com>
Subject: Re: [PATCH 2/2] MIPS: ebpf jit: Implement R4000 workarounds
To:     Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc:     "linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
        linux-kernel@vger.kernel.org,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        "paulburton@kernel.org" <paulburton@kernel.org>,
        bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Feb 23, 2023 at 11:32=E2=80=AFAM Jiaxun Yang <jiaxun.yang@flygoat.c=
om> 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) =3D 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) +=3D bpf_jit_comp32.o
> > else
> >        obj-$(CONFIG_BPF_JIT) +=3D bpf_jit_comp64.o
> > endif
>
> It=E2=80=99s 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>