Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > +The eBPF instruction set consists of eleven 64 bit registers, a > > +program counter, and 512 bytes of stack space. > > I would not add 512 to a doc. > That's what we have now, but we might relax that in the future. I think it's important to at least give a minimum. Will change to "at least 512". [...] > > +Registers R0 - R5 are scratch registers, meaning the BPF program > > +needs to either spill them to the BPF stack or move them to callee > > +saved registers if these arguments are to be reused across multiple > > +function calls. Spilling means that the value in the register is > > +moved to the BPF stack. The reverse operation of moving the variable > from the BPF stack to the register is called filling. > > +The reason for spilling/filling is due to the limited number of registers. > > More canonical way would be to say that r0-r5 are caller saved. Will change "scratch" to "caller-saved" [...] > > -``BPF_ADD | BPF_X | BPF_ALU64`` means:: > > +``BPF_ADD | BPF_X | BPF_ALU64`` (0x0f) means:: > > I don't think adding hex values help here. I found it very helpful in verifying that the Appendix table was correct, and providing a correlation to the text here that shows the construction of the value. So I'd like to keep them. [...] > > -The 1-bit source operand field in the opcode is used to to select > > what byte -order the operation convert from or to: > > +Byte swap instructions use non-default semantics of the 1-bit > > +'source' field in > > I would drop 'non-default'. Many fields have different meanings depending > on opcode. > BPF_SRC() macro just reads that bit. Will reword to say: Byte swap instructions use the 1-bit 'source' field in the 'opcode' field as follows. Instead of indicating the source operator, it is instead used to select what byte order the operation converts from or to: [...] > > +* mnenomic indicates a short form that might be displayed by some > > +tools such as disassemblers > > +* 'htoleNN()' indicates converting a NN-bit value from host byte > > +order to little-endian byte order > > +* 'htobeNN()' indicates converting a NN-bit value from host byte > > +order to big-endian byte order > > I think we need to add normal bswap insn. > These to_le and to_be are very awkward to use. > As soon as we have new insn the compiler will be using it. > There is no equivalent of to_be and to_le in C. It wasn't good ISA design. I will interpret this as a request for someone to do code work, rather than any request for immediately changes to the doc :) [...] > > Regular load and store operations > > --------------------------------- > > > > The ``BPF_MEM`` mode modifier is used to encode regular load and > > store instructions that transfer data between a register and memory. > > > > -``BPF_MEM | <size> | BPF_STX`` means:: > > - > > - *(size *) (dst + offset) = src_reg > > - > > -``BPF_MEM | <size> | BPF_ST`` means:: > > - > > - *(size *) (dst + offset) = imm32 > > - > > -``BPF_MEM | <size> | BPF_LDX`` means:: > > - > > - dst = *(size *) (src + offset) > > - > > -Where size is one of: ``BPF_B``, ``BPF_H``, ``BPF_W``, or ``BPF_DW``. > > +============================= ========= > ==================================== > > +opcode construction opcode pseudocode > > +============================= ========= > ==================================== > > +BPF_MEM | BPF_B | BPF_LDX 0x71 dst = \*(uint8_t \*) (src + offset) > > +BPF_MEM | BPF_H | BPF_LDX 0x69 dst = \*(uint16_t \*) (src + > offset) > > +BPF_MEM | BPF_W | BPF_LDX 0x61 dst = \*(uint32_t \*) (src + > offset) > > +BPF_MEM | BPF_DW | BPF_LDX 0x79 dst = \*(uint64_t \*) (src + > offset) > > +BPF_MEM | BPF_B | BPF_ST 0x72 \*(uint8_t \*) (dst + offset) = imm > > +BPF_MEM | BPF_H | BPF_ST 0x6a \*(uint16_t \*) (dst + offset) = > imm > > +BPF_MEM | BPF_W | BPF_ST 0x62 \*(uint32_t \*) (dst + offset) = > imm > > +BPF_MEM | BPF_DW | BPF_ST 0x7a \*(uint64_t \*) (dst + offset) = > imm > > +BPF_MEM | BPF_B | BPF_STX 0x73 \*(uint8_t \*) (dst + offset) = src > > +BPF_MEM | BPF_H | BPF_STX 0x6b \*(uint16_t \*) (dst + offset) = > src > > +BPF_MEM | BPF_W | BPF_STX 0x63 \*(uint32_t \*) (dst + offset) = > src > > +BPF_MEM | BPF_DW | BPF_STX 0x7b \*(uint64_t \*) (dst + offset) = > src > > +============================= ========= > > +==================================== > > I think the table is more verbose and less readable than the original text. Will change back to original text. Dave
