linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	linux-doc@vger.kernel.org, netdev@vger.kernel.org,
	bpf@vger.kernel.org
Subject: Re: [PATCH 3/4] bpf, docs: Generate nicer tables for instruction encodings
Date: Thu, 30 Dec 2021 16:43:24 -0800	[thread overview]
Message-ID: <20211231004324.wvfqqgntnpswhzby@ast-mbp> (raw)
In-Reply-To: <20211223101906.977624-4-hch@lst.de>

On Thu, Dec 23, 2021 at 11:19:05AM +0100, Christoph Hellwig wrote:
>  
> +For class BPF_ALU or BPF_ALU64:
> +
> +  ========  =====  =========================
> +  code      value  description
> +  ========  =====  =========================
>    BPF_ADD   0x00
>    BPF_SUB   0x10
>    BPF_MUL   0x20
> @@ -68,26 +76,31 @@ If BPF_CLASS(code) == BPF_ALU or BPF_ALU64 BPF_OP(code) is one of::
>    BPF_NEG   0x80
>    BPF_MOD   0x90
>    BPF_XOR   0xa0
> -  BPF_MOV   0xb0  /* mov reg to reg */
> -  BPF_ARSH  0xc0  /* sign extending shift right */
> -  BPF_END   0xd0  /* endianness conversion */
> +  BPF_MOV   0xb0   mov reg to reg
> +  BPF_ARSH  0xc0   sign extending shift right
> +  BPF_END   0xd0   endianness conversion
> +  ========  =====  =========================
>  
> -If BPF_CLASS(code) == BPF_JMP or BPF_JMP32 BPF_OP(code) is one of::
> +For class BPF_JMP or BPF_JMP32:
>  
> -  BPF_JA    0x00  /* BPF_JMP only */
> +  ========  =====  =========================
> +  code      value  description
> +  ========  =====  =========================
> +  BPF_JA    0x00   BPF_JMP only
>    BPF_JEQ   0x10
>    BPF_JGT   0x20
>    BPF_JGE   0x30
>    BPF_JSET  0x40

Not your fault, but the new table looks odd with
only some opcodes documented.
Same issue with BPF_ALU table.
In the past the documented opcodes were for eBPF only and
not documented in both, so it wasn't that bad.
At least there was a reason for discrepancy.
Now it just odd.
May be add a comment to all rows?

> -  BPF_JNE   0x50  /* jump != */
> -  BPF_JSGT  0x60  /* signed '>' */
> -  BPF_JSGE  0x70  /* signed '>=' */
> -  BPF_CALL  0x80  /* function call */
> -  BPF_EXIT  0x90  /*  function return */
> -  BPF_JLT   0xa0  /* unsigned '<' */
> -  BPF_JLE   0xb0  /* unsigned '<=' */
> -  BPF_JSLT  0xc0  /* signed '<' */
> -  BPF_JSLE  0xd0  /* signed '<=' */
> +  BPF_JNE   0x50   jump '!='
> +  BPF_JSGT  0x60   signed '>'
> +  BPF_JSGE  0x70   signed '>='
> +  BPF_CALL  0x80   function call
> +  BPF_EXIT  0x90   function return
> +  BPF_JLT   0xa0   unsigned '<'
> +  BPF_JLE   0xb0   unsigned '<='
> +  BPF_JSLT  0xc0   signed '<'
> +  BPF_JSLE  0xd0   signed '<='
> +  ========  =====  =========================
>  
>  So BPF_ADD | BPF_X | BPF_ALU means::
>  
> @@ -108,37 +121,58 @@ the return value into register R0 before doing a BPF_EXIT. Class 6 is used as
>  BPF_JMP32 to mean exactly the same operations as BPF_JMP, but with 32-bit wide
>  operands for the comparisons instead.
>  
> -For load and store instructions the 8-bit 'code' field is divided as::
>  
> -  +--------+--------+-------------------+
> -  | 3 bits | 2 bits |   3 bits          |
> -  |  mode  |  size  | instruction class |
> -  +--------+--------+-------------------+
> -  (MSB)                             (LSB)
> +Load and store instructions
> +===========================
> +
> +For load and store instructions (BPF_LD, BPF_LDX, BPF_ST and BPF_STX), the
> +8-bit 'opcode' field is divided as:
> +
> +  ============  ======  =================
> +  3 bits (MSB)  2 bits  3 bits (LSB)
> +  ============  ======  =================
> +  mode          size    instruction class
> +  ============  ======  =================
> +
> +The size modifier is one of:
>  
> -Size modifier is one of ...
> +  =============  =====  =====================
> +  size modifier  value  description
> +  =============  =====  =====================
> +  BPF_W          0x00   word        (4 bytes)
> +  BPF_H          0x08   half word   (2 bytes)
> +  BPF_B          0x10   byte
> +  BPF_DW         0x18   double word (8 bytes)
> +  =============  =====  =====================
>  
> -::
> +The mode modifier is one of:
>  
> -  BPF_W   0x00    /* word */
> -  BPF_H   0x08    /* half word */
> -  BPF_B   0x10    /* byte */
> -  BPF_DW  0x18    /* double word */
> +  =============  =====  =====================
> +  mode modifier  value  description
> +  =============  =====  =====================
> +  BPF_IMM        0x00   used for 64-bit mov
> +  BPF_ABS        0x20
> +  BPF_IND        0x40
> +  BPF_MEM        0x60

May be say here that ABS and IND are legacy for compat with classic only?
and MEM is the most common modifier for load/store?

> +  BPF_ATOMIC     0xc0   atomic operations 

I removed trailing space in above line.

And applied all patches to bpf-next. Thanks!

  reply	other threads:[~2021-12-31  0:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-23 10:19 improve the eBPF documentation v2 Christoph Hellwig
2021-12-23 10:19 ` [PATCH 1/4] bpf, docs: Fix verifier references Christoph Hellwig
2021-12-23 10:19 ` [PATCH 2/4] bpf, docs: Split the comparism to classic BPF from instruction-set.rst Christoph Hellwig
2021-12-23 10:19 ` [PATCH 3/4] bpf, docs: Generate nicer tables for instruction encodings Christoph Hellwig
2021-12-31  0:43   ` Alexei Starovoitov [this message]
2022-01-03  9:57     ` Christoph Hellwig
2021-12-23 10:19 ` [PATCH 4/4] bpf, docs: Move the packet access instructions last in instruction-set.rst Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211231004324.wvfqqgntnpswhzby@ast-mbp \
    --to=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=corbet@lwn.net \
    --cc=daniel@iogearbox.net \
    --cc=hch@lst.de \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).