Christoph Hellwig <hch@xxxxxxxxxxxxx> writes: [...] > > +The current Instruction Set Architecture (ISA) version, sometimes > > +referred to in other documents as a "CPU" version, is 3. This document > also covers older versions of the ISA. > > Hmm, I thought the versioning was a bit more complicated based on the > mailing list interactions and the call. Especially with things like the full > atomics not even supported by all gits. Yeah some features, like BPF_ALU, are optional in some versions so yes it's more complicated so some things have to be described at the granularity of a feature. > > + **Note** > > + > > + *Linux implementation*: In the Linux kernel, the exit value for > > +eBPF > > + programs is passed as a 32 bit value. > > Is this Linux, a specific program type, or the ISA? https://docs.cilium.io/en/stable/bpf/ just said > Register r0 is also the register containing the exit value for the BPF program. The > semantics of the exit value are defined by the type of program. Furthermore, when > handing execution back to the kernel, the exit value is passed as a 32 bit value. But other runtimes like ubpf have no such restriction and it is not mentioned in the original instruction-set.rst, so I assumed from the above that it is a Linux implementation specific convention, not part of the ISA. If you have evidence otherwise, let me know. > > + *Linux implementation*: In the Linux kernel, all program types > > +only use > > + R1 which contains the "context", which is typically a structure > > +containing all > > + the inputs needed. > > I also think these Linux notes do not belong into the main instruction set > document, which tries to really just describe the ISA. I asked this question at LPC and no one had any comments about the current approach so assumed the default consensus was to leave them in. The original instruction-set.rst (prior to my proposed updates) combined both Linux specific info and general ISA info but during the LPC discussion we said we'd move the legacy packet instructions to a separate doc on "Linux historical notes", which I have a draft of at https://github.com/dthaler/ebpf-docs/blob/update/isa/kernel.org/linux-historical-notes.rst If we do move Linux notes out of the ISA spec, then we could rename the other doc to just be "Linux implementation notes", and contain all the Linux specific notes. It will be slightly more complicated though to explain what the notes refer to in the main spec if they're not inline line at present. Any other opinions? I don't feel strongly either way. > > - * the basic instruction encoding, which uses 64 bits to encode an > > instruction > > - * the wide instruction encoding, which appends a second 64-bit > > immediate value > > - (imm64) after the basic instruction for a total of 128 bits. > > +* the basic instruction encoding, which uses 64 bits to encode an > > +instruction > > +* the wide instruction encoding, which appends a second 64-bit > > +immediate (i.e., > > Btw, can you explain why you de-indent these? I picked the space before the > * because that seems to be what most Linux RST documents do. Quentin had pointed out to me some rendering problems with the space. You can see the difference in GitHub's renderer in a test file at https://github.com/dthaler/ebpf-docs/blob/test-formatting/test/test-formatting.rst#lists > > + For ISA versions prior to 3, Clang v7.0 and later can enable > > +``BPF_ALU`` support with > > + ``-Xclang -target-feature -Xclang +alu32``. > > I also suspect the clang notes would be better off in a separate document > from the main ISA. No one else raised concerns at LPC when I explicitly asked this, but I have no strong opinion either way other than whatever we do for Linux notes and for Clang notes, the answer should be the same. > > -BPF_XOR | BPF_K | BPF_ALU means:: > > + *Linux implementation*: In the Linux kernel, uint32_t is expressed > > +as u32, > > + uint64_t is expressed as u64, etc. This document uses the > > +standard C terminology > > + as the cross-platform specification. > > I don't think this makes sense in the document. Instead we probably need a > "Conventions" section that defines the type and syntax we use. I'm interpreting this comment as part of the overall Linux note feedback. Would love to hear if anyone else has opinion on the matter of Linux implementation notes. Thanks for the feedback! Dave
