On Tue, May 07, 2019 at 05:29:09AM +0100, Jiong Wang wrote: > > Jiong Wang writes: > > > Alexei Starovoitov writes: > > > >> On Fri, May 03, 2019 at 11:42:31AM +0100, Jiong Wang wrote: > >>> This patch introduce new alu32 insn BPF_ZEXT, and allocate the unused > >>> opcode 0xe0 to it. > >>> > >>> Compared with the other alu32 insns, zero extension on low 32-bit is the > >>> only semantics for this instruction. It also allows various JIT back-ends > >>> to do optimal zero extension code-gen. > >>> > >>> BPF_ZEXT is supposed to be encoded with BPF_ALU only, and is supposed to be > >>> generated by the latter 32-bit optimization code inside verifier for those > >>> arches that do not support hardware implicit zero extension only. > >>> > >>> It is not supposed to be used in user's program directly at the moment. > >>> Therefore, no need to recognize it inside generic verification code. It > >>> just need to be supported for execution on interpreter or related JIT > >>> back-ends. > >> > >> uapi and the doc define it, but "it is not supposed to be used" ?! > >> > >>> Signed-off-by: Jiong Wang <jiong.wang@xxxxxxxxxxxxx> > >>> --- > >>> Documentation/networking/filter.txt | 10 ++++++++++ > >>> include/uapi/linux/bpf.h | 3 +++ > >>> kernel/bpf/core.c | 4 ++++ > >>> tools/include/uapi/linux/bpf.h | 3 +++ > >>> 4 files changed, 20 insertions(+) > >>> > >>> diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt > >>> index 319e5e0..1cb3e42 100644 > >>> --- a/Documentation/networking/filter.txt > >>> +++ b/Documentation/networking/filter.txt > >>> @@ -903,6 +903,16 @@ If BPF_CLASS(code) == BPF_ALU or BPF_ALU64 [ in eBPF ], BPF_OP(code) is one of: > >>> BPF_MOV 0xb0 /* eBPF only: mov reg to reg */ > >>> BPF_ARSH 0xc0 /* eBPF only: sign extending shift right */ > >>> BPF_END 0xd0 /* eBPF only: endianness conversion */ > >>> + BPF_ZEXT 0xe0 /* eBPF BPF_ALU only: zero-extends low 32-bit */ > >>> + > >>> +Compared with BPF_ALU | BPF_MOV which zero-extends low 32-bit implicitly, > >>> +BPF_ALU | BPF_ZEXT zero-extends low 32-bit explicitly. Such zero extension is > >> > >> wait. that's an excellent observation. alu|mov is exactly it. > >> we do not need another insn. > >> we probably can teach the verifier to recognize <<32, >>32 and replace > >> with mov32 > > > > Hmm, I am silly, in v6, patched insn will be conservatively marked as > > always needing zext, so looks like no problem to just insert mov32 as > > zext. But some backends needs minor opt, because this will be special mov, > > with the same src and dst, just need to clear high 32-bit, no need of > > mov. > > I take it back. > > Recalled the reason why new ZEXT was introduced. It was because instruction > insertion based approach doesn't push analysis results down to JIT > back-ends, instead, it removes the clear-high-32bit semantics from > all sub-register write instructions, then insert new explicit ZEXT insn, > either by 64bit shifts combination or this new introduced ZEXT, what's > important, the inserted "ZEXT" should not be affected by > "env->verifier_zext", and be performed unconditionally. > > That is to say, for the current zero extension insertion based approach, > JIT back-ends trust verifier has done full zero extension insertion and > rewritten the instruction sequence once the flag env->verifier_zext is set, > and then JIT back-end do NOT clear high 32-bit for all existing > sub-register write instructions, for example ALU32 and narrowed load, they > rely on those new inserted "unconditional ZEXT" to do the job if it is > needed. So, if we insert mov32, there actually won't be zero extension > insertion performed for it. > > The inserted "ZEXT" needs to have zero extension semantics that is not > affected by env->verifier_zext. BPF_ZEXT was introduced because of this, > low32 zext is its only semantics and should be unconditionally done. > > "mov32" could be used as "ZEXT" only when there is no such removal of zero > extension semantics from alu32, or if JIT back-end could have instruction > level information, for example the analyzed instruction level zero extension > information pushed down to JIT back-ends. The new inserted "mov32" would > then has information like "zext_dst" be true, JIT back-end then will > generate zero extension for it. JITs could simply always do zext for mov32. No need to for extra flags.
