On Fri, Feb 10, 2023 at 10:54 AM Yonghong Song <yhs@xxxxxxxx> wrote: > > > > On 2/9/23 3:39 PM, Andrii Nakryiko wrote: > > On Thu, Feb 9, 2023 at 2:55 PM Yonghong Song <yhs@xxxxxxxx> wrote: > >> > >> Over the past, there are some discussions to extend bpf > >> instruction ISA to accommodate some new use cases or > >> fix some potential issues. These new instructions will > >> be included in new cpu flavor -mcpu=v4. > >> > >> The following are the proposal > >> to add new instructions in 6 different categories. > >> The proposal is a little bit rough. You can find bpf insn > >> background information in Documentation/bpf/instruction-set.rst. > >> You comments or suggestions are welcome! > >> > > > > Great that we are trying to fix and complete the instruction set! Just > > one comment/question below for condition jumps. > > > > [...] > > > >> > >> 32-bit JA > >> ========= > >> > >> Currently, the whole range of operations with BPF_JMP32/BPF_JMP insn are > >> implemented like below > >> > >> ======== ===== ========================= ============ > >> code value description notes > >> ======== ===== ========================= ============ > >> BPF_JA 0x00 PC += off BPF_JMP only > >> BPF_JEQ 0x10 PC += off if dst == src > >> BPF_JGT 0x20 PC += off if dst > src unsigned > >> BPF_JGE 0x30 PC += off if dst >= src unsigned > >> BPF_JSET 0x40 PC += off if dst & src > >> BPF_JNE 0x50 PC += off if dst != src > >> BPF_JSGT 0x60 PC += off if dst > src signed > >> BPF_JSGE 0x70 PC += off if dst >= src signed > >> BPF_CALL 0x80 function call > >> BPF_EXIT 0x90 function / program return BPF_JMP only > >> BPF_JLT 0xa0 PC += off if dst < src unsigned > >> BPF_JLE 0xb0 PC += off if dst <= src unsigned > >> BPF_JSLT 0xc0 PC += off if dst < src signed > >> BPF_JSLE 0xd0 PC += off if dst <= src signed > >> ======== ===== ========================= ============ > >> > >> Here the 'off' is 16 bit so the range of jump is [-32768, 32767]. > >> In rare cases, people may have large programs or have loops fully unrolled. > >> This may cause some jump offset beyond the above range. In current > >> llvm implementation, wrong code (after truncation) will be generated. > >> > >> To fix this issue, the following new insn is proposed > >> > >> ======== ===== ========================= ============ > >> code value description notes > >> ======== ===== ========================= ============ > >> BPF_JA 0x00 PC += imm BPF_JMP32 only, src = 1 > >> > >> The way, the jump offset range become [-2^31, 2^31 - 1]. > >> > >> For other jump instructions, e.g., BPF_JEQ, with a jmp offset > >> beyond [-32768, 32767]. It can be simulated with a > >> 'BPF_JA (PC += imm)' followed by the original > >> BPF_JEQ with the range 'off', or BPF_JEQ with a short range followed > >> by a BPF_JA. > > > > Why not implement the same approach (using imm if src = 1) for all the > > conditional jumps? Just too much JIT work or some other reasons? > > We cannot use 'src' since 'src' is used in conditional jump, e.g., Right, I missed this part, thanks. > > ======== ===== ========================= ============ > code value description notes > ======== ===== ========================= ============ > BPF_JEQ 0x10 PC += off if dst == src > > In this particular case, there is no good way to extend > the insn with range [-2^31, 2^31 - 1] as 'off/dst/src' all > used by the above insn. The sample extension to original > BPF_JEQ seems not working so I came up with the above > BPF_JA (32bit range) + BPF_JEQ(16 bit range) approach. > It is ugly and increase implementation complexity, but > considering this is a corner case. It may not be > worthwhile to design a whole range of 32bit range of > BPF_JEQ/JGT/... instructions. > > > > > [...]