On Sat, Jan 27, 2024 at 09:03:14AM -0800, Dave Thaler wrote: > This patch attempts to update the ISA specification according > to the latest mailing list discussion about conformance groups, > in a way that is intended to be consistent with IANA registry > processes and IETF 118 WG meeting discussion. > > It does the following: > * Split basic into base32 and base64 for 32-bit vs 64-bit base > instructions > * Split division/modulo instructions out of base groups > * Split atomic instructions out of base groups > > There may be additional changes as discussion continues, > but there seems to be consensus on the principles above. > > Signed-off-by: Dave Thaler <dthaler1968@xxxxxxxxx> Thanks a lot for working on this. I think it's getting very close. Left a few comments below. > --- > .../bpf/standardization/instruction-set.rst | 44 ++++++++++++++----- > 1 file changed, 32 insertions(+), 12 deletions(-) > > diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst > index af43227b6..a090b5131 100644 > --- a/Documentation/bpf/standardization/instruction-set.rst > +++ b/Documentation/bpf/standardization/instruction-set.rst > @@ -102,7 +102,7 @@ Conformance groups > > An implementation does not need to support all instructions specified in this > document (e.g., deprecated instructions). Instead, a number of conformance > -groups are specified. An implementation must support the "basic" conformance > +groups are specified. An implementation must support the base32 conformance > group and may support additional conformance groups, where supporting a > conformance group means it must support all instructions in that conformance > group. > @@ -112,12 +112,20 @@ that executes instructions, and tools as such compilers that generate > instructions for the runtime. Thus, capability discovery in terms of > conformance groups might be done manually by users or automatically by tools. > > -Each conformance group has a short ASCII label (e.g., "basic") that > +Each conformance group has a short ASCII label (e.g., "base32") that > corresponds to a set of instructions that are mandatory. That is, each > instruction has one or more conformance groups of which it is a member. > > -The "basic" conformance group includes all instructions defined in this > -specification unless otherwise noted. > +This document defines the following conformance groups: > +* base32: includes all instructions defined in this > + specification unless otherwise noted. > +* base64: includes base32, plus instructions explicited noted s/explicited/explicitly > + as being in the base64 conformance group. > +* atom32: includes 32-bit atomic operation instructions (see `Atomic operations`_). > +* atom64: includes atom32, plus 64-bit atomic operation instructions. > +* div32: includes 32-bit division and modulo instructions. Did we want to separate division and modulo? It looks like Netronome doesn't support modulo [0], presumably because it's not as useful as in tracing. Jakub -- can you confirm? If so, how difficult would it have been to add modulo support, and do you think it would have provided any value? [0]: https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/netronome/nfp/bpf/jit.c#L3421 > +* div64: includes div32, plus 64-bit division and modulo instructions. > +* legacy: deprecated packet access instructions. > > Instruction encoding > ==================== > @@ -239,7 +247,8 @@ Arithmetic instructions > ----------------------- > > ``BPF_ALU`` uses 32-bit wide operands while ``BPF_ALU64`` uses 64-bit wide operands for > -otherwise identical operations. > +otherwise identical operations. ``BPF_ALU64`` instructions belong to the > +base64 conformance group unless noted otherwise. > The 'code' field encodes the operation as below, where 'src' and 'dst' refer > to the values of the source and destination registers, respectively. > > @@ -293,6 +302,9 @@ where '(u32)' indicates that the upper 32 bits are zeroed. > Note that most instructions have instruction offset of 0. Only three instructions > (``BPF_SDIV``, ``BPF_SMOD``, ``BPF_MOVSX``) have a non-zero offset. > > +Division and modulo operations for ``BPF_ALU`` are part of the "div32" > +conformance group, and division and modulo operations for ``BPF_ALU64`` > +are part of the "div64" conformance group. > The division and modulo operations support both unsigned and signed flavors. > > For unsigned operations (``BPF_DIV`` and ``BPF_MOD``), for ``BPF_ALU``, > @@ -349,7 +361,9 @@ BPF_ALU64 Reserved 0x00 do byte swap unconditionally > ========= ========= ===== ================================================= > > The 'imm' field encodes the width of the swap operations. The following widths > -are supported: 16, 32 and 64. > +are supported: 16, 32 and 64. Width 64 operations belong to the base64 > +conformance group and other swap operations belong to the base32 > +conformance group. > > Examples: > > @@ -374,8 +388,8 @@ Examples: > Jump instructions > ----------------- > > -``BPF_JMP32`` uses 32-bit wide operands while ``BPF_JMP`` uses 64-bit wide operands for > -otherwise identical operations. > +``BPF_JMP32`` uses 32-bit wide operands and indicates the base32 conformance group, while ``BPF_JMP`` uses 64-bit wide operands for Could you please wrap this line? > +otherwise identical operations, and indicates the base64 conformance group unless otherwise specified. > The 'code' field encodes the operation as below: [...] Seems reasonable other than the division vs. modulo question above. Alexei, Christoph, Jose, what do you think? Thanks, David
Attachment:
signature.asc
Description: PGP signature
