[bpf-next] bpf, docs: clarify sign extension of 64-bit use of 32-bit imm

Commit Message

Dave Thaler May 17, 2024, 4:16 p.m. UTC

imm is defined as a 32-bit signed integer.

{MOV, K, ALU64} says it does "dst = src" (where src is 'imm') but it does
not sign extend, but instead does dst = (u32)src.  The "Jump instructions"
section has "unsigned" by some instructions, but the "Arithmetic instructions"
section has no such note about the MOV instruction, so added an example to
make this more clear.

{JLE, K, JMP} says it does "PC += offset if dst <= src" (where src is 'imm',
and the comparison is unsigned). This was apparently ambiguous to some
readers as to whether the comparison was "dst <= (u64)(u32)imm" or
"dst <= (u64)(s64)imm", since the correct assumption would be the latter
except that the MOV instruction doesn't follow that, so added an example
to make this more clear.

Signed-off-by: Dave Thaler <[e-mail]>
---
 .../bpf/standardization/instruction-set.rst       | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Yonghong Song May 20, 2024, 8:08 p.m. UTC | #1

On 5/17/24 10:16 AM, Dave Thaler wrote:
> imm is defined as a 32-bit signed integer.
>
> {MOV, K, ALU64} says it does "dst = src" (where src is 'imm') but it does
> not sign extend, but instead does dst = (u32)src.  The "Jump instructions"

I am not sure about this. In kernel/bpf/core.c, we have
         ALU64_MOV_K:
                 DST = IMM;
                 CONT;
here DST is u64 and IMM is s32. IIUC, IMM needs to extend to s64 and then
convert to u64.

> section has "unsigned" by some instructions, but the "Arithmetic instructions"
> section has no such note about the MOV instruction, so added an example to
> make this more clear.
>
> {JLE, K, JMP} says it does "PC += offset if dst <= src" (where src is 'imm',
> and the comparison is unsigned). This was apparently ambiguous to some
> readers as to whether the comparison was "dst <= (u64)(u32)imm" or
> "dst <= (u64)(s64)imm", since the correct assumption would be the latter
> except that the MOV instruction doesn't follow that, so added an example
> to make this more clear.
>
> Signed-off-by: Dave Thaler <[e-mail]>
> ---
>   .../bpf/standardization/instruction-set.rst       | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
> index 997560aba..f96ebb169 100644
> --- a/Documentation/bpf/standardization/instruction-set.rst
> +++ b/Documentation/bpf/standardization/instruction-set.rst
> @@ -378,13 +378,22 @@ etc. This specification requires that signed modulo use truncated division
>   
>      a % n = a - n * trunc(a / n)
>   
> -The ``MOVSX`` instruction does a move operation with sign extension.
> +The ``MOV`` instruction does a move operation without sign extension, whereas
> +the ``MOVSX`` instruction does a move operation with sign extension.
>   ``{MOVSX, X, ALU}`` :term:`sign extends<Sign Extend>` 8-bit and 16-bit operands into
>   32-bit operands, and zeroes the remaining upper 32 bits.
>   ``{MOVSX, X, ALU64}`` :term:`sign extends<Sign Extend>` 8-bit, 16-bit, and 32-bit
>   operands into 64-bit operands.  Unlike other arithmetic instructions,
>   ``MOVSX`` is only defined for register source operands (``X``).
>   
> +``{MOV, K, ALU}`` means::
> +
> +  dst = (u32) imm
> +
> +``{MOVSX, X, ALU}`` with 'offset' 32 means::
> +
> +  dst = (s32) src

For {MOVSX, X, ALU}, offset 32 is not supported. The correct offset value
is 8 and 16. For example for offset 8, we have dst = (u32)(s8)src.

> +
>   The ``NEG`` instruction is only defined when the source bit is clear
>   (``K``).
>   
> @@ -486,6 +495,10 @@ Example:
>   
>   where 's>=' indicates a signed '>=' comparison.
>   
> +``{JLE, K, JMP}`` means::
> +
> +  if dst <= (u64)(s64)imm goto +offset
> +
>   ``{JA, K, JMP32}`` means::
>   
>     gotol +imm

Patch

diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
index 997560aba..f96ebb169 100644
--- a/Documentation/bpf/standardization/instruction-set.rst
+++ b/Documentation/bpf/standardization/instruction-set.rst
@@ -378,13 +378,22 @@  etc. This specification requires that signed modulo use truncated division
 
    a % n = a - n * trunc(a / n)
 
-The ``MOVSX`` instruction does a move operation with sign extension.
+The ``MOV`` instruction does a move operation without sign extension, whereas
+the ``MOVSX`` instruction does a move operation with sign extension.
 ``{MOVSX, X, ALU}`` :term:`sign extends<Sign Extend>` 8-bit and 16-bit operands into
 32-bit operands, and zeroes the remaining upper 32 bits.
 ``{MOVSX, X, ALU64}`` :term:`sign extends<Sign Extend>` 8-bit, 16-bit, and 32-bit
 operands into 64-bit operands.  Unlike other arithmetic instructions,
 ``MOVSX`` is only defined for register source operands (``X``).
 
+``{MOV, K, ALU}`` means::
+
+  dst = (u32) imm
+
+``{MOVSX, X, ALU}`` with 'offset' 32 means::
+
+  dst = (s32) src
+
 The ``NEG`` instruction is only defined when the source bit is clear
 (``K``).
 
@@ -486,6 +495,10 @@  Example:
 
 where 's>=' indicates a signed '>=' comparison.
 
+``{JLE, K, JMP}`` means::
+
+  if dst <= (u64)(s64)imm goto +offset
+
 ``{JA, K, JMP32}`` means::
 
   gotol +imm