Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
MIME-Version: 1.0
References: <20230912224654.6556-1-puranjay12@gmail.com> <20230912224654.6556-6-puranjay12@gmail.com>
 <ZQDuVTSycDcjDkvi@shell.armlinux.org.uk>
In-Reply-To: <ZQDuVTSycDcjDkvi@shell.armlinux.org.uk>
From:   Puranjay Mohan <puranjay12@gmail.com>
Date:   Wed, 13 Sep 2023 01:16:44 +0200
Message-ID: <CANk7y0iFdgHgu+RXYJvP3swaRS+-Lr0CgOAdcQWtjs4VkrOzdQ@mail.gmail.com>
Subject: Re: [PATCH bpf-next 5/6] bpf, arm32: Always zero extend for LDX with B/H/W
To:     "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>,
        Yonghong Song <yonghong.song@linux.dev>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        Shubham Bansal <illusionist.neo@gmail.com>,
        "James E.J. Bottomley" <James.Bottomley@hansenpartnership.com>,
        Helge Deller <deller@gmx.de>,
        "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Nicholas Piggin <npiggin@gmail.com>,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        Luke Nelson <luke.r.nels@gmail.com>,
        Xi Wang <xi.wang@gmail.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Wang YanQing <udknight@gmail.com>, bpf@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
        linux-riscv@lists.infradead.org, netdev@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Sep 13, 2023 at 1:04=E2=80=AFAM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Tue, Sep 12, 2023 at 10:46:53PM +0000, Puranjay Mohan wrote:
> > The JITs should not depend on the verifier for zero extending the upper
> > 32 bits of the destination register when loading a byte, half-word, or
> > word.
> >
> > A following patch will make the verifier stop patching zext instruction=
s
> > after LDX.
>
> This was introduced by:
>
> 163541e6ba34 ("arm: bpf: eliminate zero extension code-gen")
>
> along with an additional function. So three points:
>
> 1) the commit should probably explain why it has now become undesirable
> to access this verifier state, whereas it appears it was explicitly
> added to permit this optimisation.

I added some details in the cover letter.

For the complete discussion see: [1]

> 2) you state that jits should not depend on this state, but the above
> commit adds more references than you're removing, so aren't there still
> references to the verifier remaining after this patch? I count a total
> of 10, and the patch below removes three.

The JITs should not depend on this state for LDX (loading
a B/H/W.
This patch removes the usage only for LDX.

> 3) what about the bpf_jit_needs_zext() function that was added to
> support the export of this zext state?

That is still applicable, The verifier will still emit zext
instructions for other
instructions like BPF_ALU / BPF_ALU64

>
> Essentially, the logic stated in the commit message doesn't seem to be
> reflected by the proposed code change.

I will try to provide more information.
Currently I have asked Alexei if we really need this in [2].
I still think this optimization is useful and we should keep it.

Thanks,
Puranjay

[1] https://lore.kernel.org/all/CANk7y0j2f-gPgZwd+YfTL71-6wfvky+f=3DkBC_ccq=
sS0EHAysyA@mail.gmail.com/
[2] https://lore.kernel.org/bpf/CANk7y0hK9sQJ-kRx3nQpVJSxpP=3DNzzFaLitOYq8=
=3DPb6Dvk9fpg@mail.gmail.com/