Feedback-ID: ifd894703:Fastmail
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.300.101.1.3\))
Subject: Re: [PATCH 1/2] MIPS: ebpf jit: Implement DADDI workarounds
From:   Jiaxun Yang <jiaxun.yang@flygoat.com>
In-Reply-To: <CAM1=_QQRmTaAnn0w6wteQ_FKgoF=vGX_okfbiUHdyUB0ZzNghQ@mail.gmail.com>
Date:   Thu, 23 Feb 2023 10:29:28 +0000
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-Transfer-Encoding: quoted-printable
Message-Id: <7CAF04EF-FC1D-4BE1-A639-92D677525C63@flygoat.com>
References: <20230222161222.11879-1-jiaxun.yang@flygoat.com>
 <20230222161222.11879-2-jiaxun.yang@flygoat.com>
 <CAM1=_QQRmTaAnn0w6wteQ_FKgoF=vGX_okfbiUHdyUB0ZzNghQ@mail.gmail.com>
To:     Johan Almbladh <johan.almbladh@anyfinetworks.com>
Precedence: bulk


> 2023=E5=B9=B42=E6=9C=8823=E6=97=A5 10:10=EF=BC=8CJohan Almbladh =
<johan.almbladh@anyfinetworks.com> =E5=86=99=E9=81=93=EF=BC=9A
>=20
> On Wed, Feb 22, 2023 at 5:12 PM Jiaxun Yang <jiaxun.yang@flygoat.com> =
wrote:
>>=20
>> For DADDI errata we just workaround by disable immediate operation
>> for BPF_ADD / BPF_SUB to avoid generation of DADDIU.
>=20
> 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=E2=80=99t see any place emitting DADDI.

>=20
>>=20
>> All other use cases in JIT won't cause overflow thus they are all =
safe.
>=20
> 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.

>=20
>>=20
>> 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(-)
>>=20
>> 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
>=20
> 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.

Thanks
- Jiaxun