On Wed, Jul 12, 2023 at 11:08:47PM -0700, Yonghong Song wrote: > diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst > index 751e657973f0..367f426d09a1 100644 > --- a/Documentation/bpf/standardization/instruction-set.rst > +++ b/Documentation/bpf/standardization/instruction-set.rst > @@ -154,24 +154,27 @@ otherwise identical operations. > The 'code' field encodes the operation as below, where 'src' and 'dst' refer > to the values of the source and destination registers, respectively. > > -======== ===== ========================================================== > -code value description > -======== ===== ========================================================== > -BPF_ADD 0x00 dst += src > -BPF_SUB 0x10 dst -= src > -BPF_MUL 0x20 dst \*= src > -BPF_DIV 0x30 dst = (src != 0) ? (dst / src) : 0 > -BPF_OR 0x40 dst \|= src > -BPF_AND 0x50 dst &= src > -BPF_LSH 0x60 dst <<= (src & mask) > -BPF_RSH 0x70 dst >>= (src & mask) > -BPF_NEG 0x80 dst = -src > -BPF_MOD 0x90 dst = (src != 0) ? (dst % src) : dst > -BPF_XOR 0xa0 dst ^= src > -BPF_MOV 0xb0 dst = src > -BPF_ARSH 0xc0 sign extending dst >>= (src & mask) > -BPF_END 0xd0 byte swap operations (see `Byte swap instructions`_ below) > -======== ===== ========================================================== > +======== ===== ============ ========================================================== > +code value offset value description How about just 'offset' ? > +======== ===== ============ ========================================================== > +BPF_ADD 0x00 0 dst += src > +BPF_SUB 0x10 0 dst -= src > +BPF_MUL 0x20 0 dst \*= src > +BPF_DIV 0x30 0 dst = (src != 0) ? (dst / src) : 0 > +BPF_SDIV 0x30 1 dst = (src != 0) ? (dst s/ src) : 0 > +BPF_OR 0x40 0 dst \|= src > +BPF_AND 0x50 0 dst &= src > +BPF_LSH 0x60 0 dst <<= (src & mask) > +BPF_RSH 0x70 0 dst >>= (src & mask) > +BPF_NEG 0x80 0 dst = -src > +BPF_MOD 0x90 0 dst = (src != 0) ? (dst % src) : dst > +BPF_SMOD 0x90 1 dst = (src != 0) ? (dst s% src) : dst > +BPF_XOR 0xa0 0 dst ^= src > +BPF_MOV 0xb0 0 dst = src > +BPF_MOVSX 0xb0 8/16/32 dst = (s8,16,s32)src > +BPF_ARSH 0xc0 0 sign extending dst >>= (src & mask) > +BPF_END 0xd0 0 byte swap operations (see `Byte swap instructions`_ below) > +======== ===== ============ ========================================================== > > Underflow and overflow are allowed during arithmetic operations, meaning > the 64-bit or 32-bit value will wrap. If eBPF program execution would > @@ -198,11 +201,19 @@ where '(u32)' indicates that the upper 32 bits are zeroed. > > dst = dst ^ imm32 > > -Also note that the division and modulo operations are unsigned. Thus, for > -``BPF_ALU``, 'imm' is first interpreted as an unsigned 32-bit value, whereas > -for ``BPF_ALU64``, 'imm' is first sign extended to 64 bits and the result > -interpreted as an unsigned 64-bit value. There are no instructions for > -signed division or modulo. > +Note that most instructions have instruction offset of 0. But three instructions > +(BPF_SDIV, BPF_SMOD, BPF_MOVSX) have non-zero offset. > + > +The devision and modulo operations support both unsigned and signed flavors. > +For unsigned operation (BPF_DIV and BPF_MOD), for ``BPF_ALU``, 'imm' is first > +interpreted as an unsigned 32-bit value, whereas for ``BPF_ALU64``, 'imm' is > +first sign extended to 64 bits and the result interpreted as an unsigned 64-bit > +value. For signed operation (BPF_SDIV and BPF_SMOD), for both ``BPF_ALU`` and > +``BPF_ALU64``, 'imm' is interpreted as a signed value. Probably worth clarifying that in case of S[DIV|MOD] | ALU64 the imm is sign extended from 32 to 64 and interpreted as signed 64-bit. > + > +Instruction BPF_MOVSX does move operation with sign extension. For ``BPF_ALU`` > +mode, 8-bit and 16-bit sign extensions to 32-bit are supported. For ``BPF_ALU64``, > +8-bit, 16-bit and 32-bit sign extenstions to 64-bit are supported. How about: Instruction BPF_MOVSX does move operation with sign extension. BPF_ALU | MOVSX sign extendes 8-bit and 16-bit into 32-bit and upper 32-bit are zeroed. BPF_ALU64 | MOVSX sign extends 8-bit, 16-bit and 32-bit into 64-bit. > > +``BPF_ALU64 | BPF_TO_LE | BPF_END`` with imm = 16 means:: > + > + dst = bswap16(dst) Worth spelling out imm 32 and 64 too ? > > +The ``BPF_MEMSX`` mode modifier is used to encode sign-extension load > +instructions that transfer data between a register and memory. > + > +``BPF_MEMSX | <size> | BPF_LDX`` means:: > + > + dst = *(sign-extension size *) (src + offset) > + How about: ``BPF_MEM | <size> | BPF_LDX`` means:: dst = *(unsigned size *) (src + offset) ``BPF_MEMSX | <size> | BPF_LDX`` means:: dst = *(signed size *) (src + offset)
