[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANk7y0iFdgHgu+RXYJvP3swaRS+-Lr0CgOAdcQWtjs4VkrOzdQ@mail.gmail.com>
Date: Wed, 13 Sep 2023 01:16:44 +0200
From: Puranjay Mohan <puranjay12@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...gle.com>,
Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
Shubham Bansal <illusionist.neo@...il.com>,
"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
Helge Deller <deller@....de>,
"Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>,
Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Luke Nelson <luke.r.nels@...il.com>,
Xi Wang <xi.wang@...il.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Wang YanQing <udknight@...il.com>, bpf@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-parisc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
linux-riscv@...ts.infradead.org, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next 5/6] bpf, arm32: Always zero extend for LDX with B/H/W
On Wed, Sep 13, 2023 at 1:04 AM Russell King (Oracle)
<linux@...linux.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 instructions
> > 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=kBC_ccqsS0EHAysyA@mail.gmail.com/
[2] https://lore.kernel.org/bpf/CANk7y0hK9sQJ-kRx3nQpVJSxpP=NzzFaLitOYq8=Pb6Dvk9fpg@mail.gmail.com/
Powered by blists - more mailing lists