On Tue, May 10, 2022 at 1:17 AM Christoph Hellwig <hch@xxxxxx> wrote: > > Thanks everyone who participated. > > Here is my rough memory an action items from the meeting. As I > was on stage and did not take notes these might be a bit off and > may need correction. > > The separate instruction set document wasn't known by everyone but > seens as a good idea. > > The content needs a little more work: > > - document the version levels, based on the clang cpu levels > (I plan to do this ASAP) Turns out that clang -mcpu=v1,v2,v3 are not exactly correct. We've extended ISA more than three times. For example when we added more atomics insns in https://lore.kernel.org/bpf/20210114181751.768687-1-jackmanb@xxxxxxxxxx/ The corresponding llvm diff didn't add a new -mcpu flavor. There was no need to do it. Also llvm flags can turn a subset of insns on and off. Like llvm can turn on alu32, but without <,<= insns. -mcpu=v3 includes both. -mcpu=v2 are only <,<=. So we need a plan B. How about using the commit sha where support was added to the verifier as a 'version' of the ISA ? We can try to use a kernel version, but backports will ruin that picture. Looks like upstream 'commit sha' is the only stable number. Another approach would be to declare the current ISA as 1.0 (or bpf-isa-may-2022) and in the future bump it with every new insn. > - we need to decide to do about the legacy BPF packet access > instrutions. Alexei mentioned that the modern JIT doesn't > even use those internally any more. I think we need to document them as supported in the linux kernel, but deprecated in general. The standard might say "implementation defined" meaning that different run-times don't have to support them. > - we need to document behavior for underflows / overflows and > other behavior not mentioned. The example in the session > was divive by zero behavior. Are there any notes on what > the consensus for a lot of this behavior is, or do we need > to reverse engineer it from the implementation? I'd happily > write the documentation, but I'd be really grateful for any > input into what needs to go into it For div by zero see do_misc_fixups(). We patch it with: 'if src_reg == 0 -> xor dst_reg, dst_reg'. Interpreter and JITs will execute div/0 as-is, but in practice it cannot happen because the verifier patched it. Other undefined/underflows/overflows are implementation defined. Meaning after JIT-ing they may behave differently on different architectures. For example the interpreter for shifts does DST = DST OP (SRC & 63); where OP is <<, >> to avoid undefined behavior in C. The JITs won't be adding the masking insns, since CPU HW will do a mask implicitly. Which could potentially change from one CPU to another. I don't think it's worth documenting all that. I would group all undefined/underflow/overflow as implementation defined and document only things that matter. > > Discussion on where to host a definitive version of the document: > > - I think the rough consensus is to just host regular (hopefully > low cadence) documents and maybe the latest gratest at a eBPF > foundation website. Whom do we need to work with at the fundation > to make this happen? foundation folks cc-ed. > - On a technical side we need to figure out a way how to build a > standalone document from the kerneldoc tree of documents. I > volunteers to look into that as well. +1 > The verifier is not very well documented, and mixes up generic behavior > with that of specific implementations and program types. > > - as idea it was brought up to write a doument with the minimal > verification requirements required for any eBPF implementation > independent of the program type. Again I can volunteer to > draft a documentation, but I need input on what such a consensus > would be. In this case input from the non-Linux verifier > implementors (I only know the Microsoft research one) would > be very helpful as well. The verifier is a moving target. I'd say minimal verification is the one that checks that: - instructions are formed correctly - opcode is valid - no reserved bits are used - registers are within range (r11+ are not used) - combination of opcode+regs+off+imm is valid - simple things like that
