On Wed, Mar 08, 2023 at 08:46:23PM +0000, Dave Thaler wrote: > From: Dave Thaler <dthaler@xxxxxxxxxxxxx> > > Add docs on extended 64-bit immediate instructions, including six instructions > previously undocumented. Include a brief description of map objects, and variables, > as used by those instructions. > > --- > V1 - V2: rebased on top of latest master > > Signed-off-by: Dave Thaler <dthaler@xxxxxxxxxxxxx> > --- > Documentation/bpf/instruction-set.rst | 56 +++++++++++++++++++++++---- > Documentation/bpf/linux-notes.rst | 10 +++++ > 2 files changed, 58 insertions(+), 8 deletions(-) > > diff --git a/Documentation/bpf/instruction-set.rst b/Documentation/bpf/instruction-set.rst > index db8789e6969..89087913fbf 100644 > --- a/Documentation/bpf/instruction-set.rst > +++ b/Documentation/bpf/instruction-set.rst > @@ -385,14 +385,54 @@ and loaded back to ``R0``. > ----------------------------- > > Instructions with the ``BPF_IMM`` 'mode' modifier use the wide instruction > -encoding for an extra imm64 value. > - > -There is currently only one such instruction. > - > -``BPF_LD | BPF_DW | BPF_IMM`` means:: > - > - dst = imm64 > - > +encoding defined in `Instruction encoding`_, and use the 'src' field of the > +basic instruction to hold an opcode subtype. > + > +The following instructions are defined, and use additional concepts defined below: > + > +========================= ====== === ===================================== =========== ============== > +opcode construction opcode src pseudocode imm type dst type > +========================= ====== === ===================================== =========== ============== > +BPF_IMM | BPF_DW | BPF_LD 0x18 0x0 dst = imm64 integer integer > +BPF_IMM | BPF_DW | BPF_LD 0x18 0x1 dst = map_by_fd(imm) map fd map > +BPF_IMM | BPF_DW | BPF_LD 0x18 0x2 dst = mva(map_by_fd(imm)) + next_imm map fd data pointer > +BPF_IMM | BPF_DW | BPF_LD 0x18 0x3 dst = variable_addr(imm) variable id data pointer > +BPF_IMM | BPF_DW | BPF_LD 0x18 0x4 dst = code_addr(imm) integer code pointer > +BPF_IMM | BPF_DW | BPF_LD 0x18 0x5 dst = map_by_idx(imm) map index map > +BPF_IMM | BPF_DW | BPF_LD 0x18 0x6 dst = mva(map_by_idx(imm)) + next_imm map index data pointer > +========================= ====== === ===================================== =========== ============== > + > +where > + > +* map_by_fd(fd) means to convert a 32-bit POSIX file descriptor into an address of a map object (see `Map objects`_) should it be map_by_fd(imm) here or map_by_fd(fd) in the table? let's make it consistent. > +* map_by_index(index) means to convert a 32-bit index into an address of a map object Should it be map_by_idx to match the table? > +* mva(map) gets the address of the first value in a given map object 'mva' looks to cryptic especially, since it's first introduced in the table and explained later. Maybe use map_val(map) ? > +* variable_addr(id) gets the address of a variable (see `Variables`_) with a given id var_addr(imm) to make it shorter? > +* code_addr(offset) gets the address of the instruction at a specified relative offset in units of 64-bit blocks code_addr(imm) for consistency? instead of "units of 64-bit blocks" it's better to use "in number of instructions". > +* the 'imm type' can be used by disassemblers for display > +* the 'dst type' can be used for verification and JIT compilation purposes > + > +Map objects > +~~~~~~~~~~~ > + > +Maps are shared memory regions accessible by eBPF programs on some platforms, where we use the term "map object" > +to refer to an object containing the data and metadata (e.g., size) about the memory region. > +A map can have various semantics as defined in a separate document, and may or may not have a single > +contiguous memory region, but the 'mva(map)' is currently only defined for maps that do have a single > +contiguous memory region. Support for maps is optional. I'm not sure why mention "Support for maps is optional." at all. BPF can be used without programs too. The user space can use bpf maps as a storage for user space data. Should we say "Support for programs is optional." ? I suggest to drop that sentence. > + > +Each map object can have a POSIX file descriptor (fd) if supported by the platform, > +where 'map_by_fd(fd)' means to get the map with the specified file descriptor. > +Each eBPF program can also be defined to use a set of maps associated with the program > +at load time, and 'map_by_index(index)' means to get the map with the given index in the set > +associated with the eBPF program containing the instruction. "in the set associated" I think it needs to be clarified. No one will understand that this part refers to 'fd_array'. > + > +Variables > +~~~~~~~~~ > + > +Variables are memory regions, identified by integer ids, accessible by eBPF programs on > +some platforms. The 'variable_addr(id)' operation means to get the address of the memory region > +identified by the given id. Support for such variables is optional. "Support for such variables is optional." Unnecessary statement. Equally confusing. It's probably worth clarifying that var_addr() is the address of kernel variable and not a bpf program variable.
