MIME-Version: 1.0
References: <20230222161222.11879-1-jiaxun.yang@flygoat.com>
 <20230222161222.11879-2-jiaxun.yang@flygoat.com> <CAM1=_QQRmTaAnn0w6wteQ_FKgoF=vGX_okfbiUHdyUB0ZzNghQ@mail.gmail.com>
 <7CAF04EF-FC1D-4BE1-A639-92D677525C63@flygoat.com>
In-Reply-To: <7CAF04EF-FC1D-4BE1-A639-92D677525C63@flygoat.com>
From:   Johan Almbladh <johan.almbladh@anyfinetworks.com>
Date:   Mon, 27 Feb 2023 13:17:53 +0100
Message-ID: <CAM1=_QRVEG0Fw9U99V3ohMe60h0DwMzyWvV_gYdJ=SrQ1D11Fg@mail.gmail.com>
Subject: Re: [PATCH 1/2] MIPS: ebpf jit: Implement DADDI 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>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Feb 23, 2023 at 11:29=E2=80=AFAM Jiaxun Yang <jiaxun.yang@flygoat.c=
om> wrote:
> I didn=E2=80=99t 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 additi=
on
> 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=E2=80=99s 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.