[PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Dave Thaler]

* Add a "callx" conformance group
* Add callx rows to table
* Update helper function to section to be agnostic between BPF_K vs
  BPF_X
* Rename "legacy" conformance group to "packet"

Based on mailing list discussion at
https://mailarchive.ietf.org/arch/msg/bpf/l5tNEgL-Wo7qSEuaGssOl5VChKk/

v1->v2: Incorporated feedback from Will Hawkins

Signed-off-by: Dave Thaler <dthaler1968@gmail.com>
---
 .../bpf/standardization/instruction-set.rst   | 32 ++++++++++++-------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
index bdfe0cd0e..9861bac6b 100644
--- a/Documentation/bpf/standardization/instruction-set.rst
+++ b/Documentation/bpf/standardization/instruction-set.rst
@@ -127,7 +127,7 @@ This document defines the following conformance groups:
 * divmul32: includes 32-bit division, multiplication, and modulo instructions.
 * divmul64: includes divmul32, plus 64-bit division, multiplication,
   and modulo instructions.
-* legacy: deprecated packet access instructions.
+* packet: deprecated packet access instructions.
 
 Instruction encoding
 ====================
@@ -404,9 +404,12 @@ BPF_JSET  0x4    any  PC += offset if dst & src
 BPF_JNE   0x5    any  PC += offset if dst != src
 BPF_JSGT  0x6    any  PC += offset if dst > src        signed
 BPF_JSGE  0x7    any  PC += offset if dst >= src       signed
-BPF_CALL  0x8    0x0  call helper function by address  BPF_JMP | BPF_K only, see `Helper functions`_
+BPF_CALL  0x8    0x0  call_by_address(imm)             BPF_JMP | BPF_K only
+BPF_CALL  0x8    0x0  call_by_address(reg_val(imm))    BPF_JMP | BPF_X only
 BPF_CALL  0x8    0x1  call PC += imm                   BPF_JMP | BPF_K only, see `Program-local functions`_
-BPF_CALL  0x8    0x2  call helper function by BTF ID   BPF_JMP | BPF_K only, see `Helper functions`_
+BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X only, see `Program-local functions`_
+BPF_CALL  0x8    0x2  call_by_btfid(imm)               BPF_JMP | BPF_K only
+BPF_CALL  0x8    0x2  call_by_btfid(reg_val(imm))      BPF_JMP | BPF_X only
 BPF_EXIT  0x9    0x0  return                           BPF_JMP | BPF_K only
 BPF_JLT   0xa    any  PC += offset if dst < src        unsigned
 BPF_JLE   0xb    any  PC += offset if dst <= src       unsigned
@@ -414,6 +417,12 @@ BPF_JSLT  0xc    any  PC += offset if dst < src        signed
 BPF_JSLE  0xd    any  PC += offset if dst <= src       signed
 ========  =====  ===  ===============================  =============================================
 
+where
+
+* reg_val(imm) gets the value of the register specified by 'imm'
+* call_by_address(value) means to call a helper function by the address specified by 'value' (see `Helper functions`_ for details)
+* call_by_btfid(value) means to call a helper function by the BTF ID specified by 'value' (see `Helper functions`_ for details)
+
 The BPF program needs to store the return value into register R0 before doing a
 ``BPF_EXIT``.
 
@@ -438,8 +447,9 @@ specified by the 'imm' field. A > 16-bit conditional jump may be
 converted to a < 16-bit conditional jump plus a 32-bit unconditional
 jump.
 
-All ``BPF_CALL`` and ``BPF_JA`` instructions belong to the
-base32 conformance group.
+All ``BPF_CALL | BPF_X`` instructions belong to the callx
+conformance group.  All other ``BPF_CALL`` instructions and all
+``BPF_JA`` instructions belong to the base32 conformance group.
 
 Helper functions
 ~~~~~~~~~~~~~~~~
@@ -447,13 +457,13 @@ Helper functions
 Helper functions are a concept whereby BPF programs can call into a
 set of function calls exposed by the underlying platform.
 
-Historically, each helper function was identified by an address
-encoded in the imm field.  The available helper functions may differ
-for each program type, but address values are unique across all program types.
+Historically, each helper function was identified by an address.
+The available helper functions may differ for each program type,
+but address values are unique across all program types.
 
 Platforms that support the BPF Type Format (BTF) support identifying
-a helper function by a BTF ID encoded in the imm field, where the BTF ID
-identifies the helper name and type.
+a helper function by a BTF ID, where the BTF ID identifies the helper
+name and type.
 
 Program-local functions
 ~~~~~~~~~~~~~~~~~~~~~~~
@@ -660,4 +670,4 @@ carried over from classic BPF. These instructions used an instruction
 class of BPF_LD, a size modifier of BPF_W, BPF_H, or BPF_B, and a
 mode modifier of BPF_ABS or BPF_IND.  However, these instructions are
 deprecated and should no longer be used.  All legacy packet access
-instructions belong to the "legacy" conformance group.
+instructions belong to the "packet" conformance group.
-- 
2.40.1

[Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Dave Thaler]

* Add a "callx" conformance group
* Add callx rows to table
* Update helper function to section to be agnostic between BPF_K vs
  BPF_X
* Rename "legacy" conformance group to "packet"

Based on mailing list discussion at
https://mailarchive.ietf.org/arch/msg/bpf/l5tNEgL-Wo7qSEuaGssOl5VChKk/

v1->v2: Incorporated feedback from Will Hawkins

Signed-off-by: Dave Thaler <dthaler1968@gmail.com>
---
 .../bpf/standardization/instruction-set.rst   | 32 ++++++++++++-------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
index bdfe0cd0e..9861bac6b 100644
--- a/Documentation/bpf/standardization/instruction-set.rst
+++ b/Documentation/bpf/standardization/instruction-set.rst
@@ -127,7 +127,7 @@ This document defines the following conformance groups:
 * divmul32: includes 32-bit division, multiplication, and modulo instructions.
 * divmul64: includes divmul32, plus 64-bit division, multiplication,
   and modulo instructions.
-* legacy: deprecated packet access instructions.
+* packet: deprecated packet access instructions.
 
 Instruction encoding
 ====================
@@ -404,9 +404,12 @@ BPF_JSET  0x4    any  PC += offset if dst & src
 BPF_JNE   0x5    any  PC += offset if dst != src
 BPF_JSGT  0x6    any  PC += offset if dst > src        signed
 BPF_JSGE  0x7    any  PC += offset if dst >= src       signed
-BPF_CALL  0x8    0x0  call helper function by address  BPF_JMP | BPF_K only, see `Helper functions`_
+BPF_CALL  0x8    0x0  call_by_address(imm)             BPF_JMP | BPF_K only
+BPF_CALL  0x8    0x0  call_by_address(reg_val(imm))    BPF_JMP | BPF_X only
 BPF_CALL  0x8    0x1  call PC += imm                   BPF_JMP | BPF_K only, see `Program-local functions`_
-BPF_CALL  0x8    0x2  call helper function by BTF ID   BPF_JMP | BPF_K only, see `Helper functions`_
+BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X only, see `Program-local functions`_
+BPF_CALL  0x8    0x2  call_by_btfid(imm)               BPF_JMP | BPF_K only
+BPF_CALL  0x8    0x2  call_by_btfid(reg_val(imm))      BPF_JMP | BPF_X only
 BPF_EXIT  0x9    0x0  return                           BPF_JMP | BPF_K only
 BPF_JLT   0xa    any  PC += offset if dst < src        unsigned
 BPF_JLE   0xb    any  PC += offset if dst <= src       unsigned
@@ -414,6 +417,12 @@ BPF_JSLT  0xc    any  PC += offset if dst < src        signed
 BPF_JSLE  0xd    any  PC += offset if dst <= src       signed
 ========  =====  ===  ===============================  =============================================
 
+where
+
+* reg_val(imm) gets the value of the register specified by 'imm'
+* call_by_address(value) means to call a helper function by the address specified by 'value' (see `Helper functions`_ for details)
+* call_by_btfid(value) means to call a helper function by the BTF ID specified by 'value' (see `Helper functions`_ for details)
+
 The BPF program needs to store the return value into register R0 before doing a
 ``BPF_EXIT``.
 
@@ -438,8 +447,9 @@ specified by the 'imm' field. A > 16-bit conditional jump may be
 converted to a < 16-bit conditional jump plus a 32-bit unconditional
 jump.
 
-All ``BPF_CALL`` and ``BPF_JA`` instructions belong to the
-base32 conformance group.
+All ``BPF_CALL | BPF_X`` instructions belong to the callx
+conformance group.  All other ``BPF_CALL`` instructions and all
+``BPF_JA`` instructions belong to the base32 conformance group.
 
 Helper functions
 ~~~~~~~~~~~~~~~~
@@ -447,13 +457,13 @@ Helper functions
 Helper functions are a concept whereby BPF programs can call into a
 set of function calls exposed by the underlying platform.
 
-Historically, each helper function was identified by an address
-encoded in the imm field.  The available helper functions may differ
-for each program type, but address values are unique across all program types.
+Historically, each helper function was identified by an address.
+The available helper functions may differ for each program type,
+but address values are unique across all program types.
 
 Platforms that support the BPF Type Format (BTF) support identifying
-a helper function by a BTF ID encoded in the imm field, where the BTF ID
-identifies the helper name and type.
+a helper function by a BTF ID, where the BTF ID identifies the helper
+name and type.
 
 Program-local functions
 ~~~~~~~~~~~~~~~~~~~~~~~
@@ -660,4 +670,4 @@ carried over from classic BPF. These instructions used an instruction
 class of BPF_LD, a size modifier of BPF_W, BPF_H, or BPF_B, and a
 mode modifier of BPF_ABS or BPF_IND.  However, these instructions are
 deprecated and should no longer be used.  All legacy packet access
-instructions belong to the "legacy" conformance group.
+instructions belong to the "packet" conformance group.
-- 
2.40.1

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[David Vernet]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

On Mon, Feb 12, 2024 at 01:13:10PM -0800, Dave Thaler wrote:
> * Add a "callx" conformance group
> * Add callx rows to table
> * Update helper function to section to be agnostic between BPF_K vs
>   BPF_X
> * Rename "legacy" conformance group to "packet"
> 
> Based on mailing list discussion at
> https://mailarchive.ietf.org/arch/msg/bpf/l5tNEgL-Wo7qSEuaGssOl5VChKk/
> 
> v1->v2: Incorporated feedback from Will Hawkins
> 
> Signed-off-by: Dave Thaler <dthaler1968@gmail.com>

Acked-by: David Vernet <void@manifault.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[David Vernet]

[-- Attachment #1.1: Type: text/plain, Size: 538 bytes --]

On Mon, Feb 12, 2024 at 01:13:10PM -0800, Dave Thaler wrote:
> * Add a "callx" conformance group
> * Add callx rows to table
> * Update helper function to section to be agnostic between BPF_K vs
>   BPF_X
> * Rename "legacy" conformance group to "packet"
> 
> Based on mailing list discussion at
> https://mailarchive.ietf.org/arch/msg/bpf/l5tNEgL-Wo7qSEuaGssOl5VChKk/
> 
> v1->v2: Incorporated feedback from Will Hawkins
> 
> Signed-off-by: Dave Thaler <dthaler1968@gmail.com>

Acked-by: David Vernet <void@manifault.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 76 bytes --]

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Jose E. Marchesi]

> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X only, see `Program-local functions`_

If the instruction requires a register operand, why not using one of the
register fields?  Is there any reason for not doing that?

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Jose E. Marchesi]

> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X only, see `Program-local functions`_

If the instruction requires a register operand, why not using one of the
register fields?  Is there any reason for not doing that?

RE: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[dthaler1968]

Jose E. Marchesi <jose.marchesi@oracle.com> writes:
> > +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
only,
> see `Program-local functions`_
> 
> If the instruction requires a register operand, why not using one of the
> register fields?  Is there any reason for not doing that?

Yeah, the reason is because this is document what clang has done by default
for a long time now.  The IETF WG charter says:

> The BPF working group is initially tasked with documenting the existing
> state of the BPF ecosystem

So extensions can always add new instructions and deprecate old ones
but the initial version of the ISA needs to document "the existing state
of the BPF ecosystem".  I know gcc used a different field but one has to
go out of your way to specify a command line option to get that to happen,
whereas clang uses callx as documented when you don't do -O2, without
requiring any extra command line options.

I agree with you that it would have been better to use the src register
since the BPF_X bit is supposed to mean that, but that ship apparently
sailed long ago with clang.

Dave

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[dthaler1968=40googlemail.com]

Jose E. Marchesi <jose.marchesi@oracle.com> writes:
> > +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
only,
> see `Program-local functions`_
> 
> If the instruction requires a register operand, why not using one of the
> register fields?  Is there any reason for not doing that?

Yeah, the reason is because this is document what clang has done by default
for a long time now.  The IETF WG charter says:

> The BPF working group is initially tasked with documenting the existing
> state of the BPF ecosystem

So extensions can always add new instructions and deprecate old ones
but the initial version of the ISA needs to document "the existing state
of the BPF ecosystem".  I know gcc used a different field but one has to
go out of your way to specify a command line option to get that to happen,
whereas clang uses callx as documented when you don't do -O2, without
requiring any extra command line options.

I agree with you that it would have been better to use the src register
since the BPF_X bit is supposed to mean that, but that ship apparently
sailed long ago with clang.

Dave

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Yonghong Song]

On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X only, see `Program-local functions`_
> If the instruction requires a register operand, why not using one of the
> register fields?  Is there any reason for not doing that?

Talked to Alexei and we think using dst_reg for the register for callx 
insn is better. I will craft a llvm patch for this today. Thanks!

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Yonghong Song]

On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X only, see `Program-local functions`_
> If the instruction requires a register operand, why not using one of the
> register fields?  Is there any reason for not doing that?

Talked to Alexei and we think using dst_reg for the register for callx 
insn is better. I will craft a llvm patch for this today. Thanks!

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

RE: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[dthaler1968]

> -----Original Message-----
> From: Yonghong Song <yonghong.song@linux.dev>
> Sent: Monday, February 12, 2024 1:49 PM
> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
> <dthaler1968=40googlemail.com@dmarc.ietf.org>
> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
> <dthaler1968@gmail.com>
> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
> conformance group
> 
> 
> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
> >> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
> only, see `Program-local functions`_
> > If the instruction requires a register operand, why not using one of
> > the register fields?  Is there any reason for not doing that?
> 
> Talked to Alexei and we think using dst_reg for the register for callx insn is
> better. I will craft a llvm patch for this today. Thanks!

Why dst_reg instead of src_reg?
BPF_X is supposed to mean use src_reg.

But this thread is about reserving/documenting the existing practice,
since anyone trying to use it would run into interop issues because
of existing clang.   Should we document both and list one as deprecated?

Dave

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[dthaler1968=40googlemail.com]

> -----Original Message-----
> From: Yonghong Song <yonghong.song@linux.dev>
> Sent: Monday, February 12, 2024 1:49 PM
> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
> <dthaler1968=40googlemail.com@dmarc.ietf.org>
> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
> <dthaler1968@gmail.com>
> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
> conformance group
> 
> 
> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
> >> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
> only, see `Program-local functions`_
> > If the instruction requires a register operand, why not using one of
> > the register fields?  Is there any reason for not doing that?
> 
> Talked to Alexei and we think using dst_reg for the register for callx insn is
> better. I will craft a llvm patch for this today. Thanks!

Why dst_reg instead of src_reg?
BPF_X is supposed to mean use src_reg.

But this thread is about reserving/documenting the existing practice,
since anyone trying to use it would run into interop issues because
of existing clang.   Should we document both and list one as deprecated?

Dave

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Yonghong Song]

On 2/12/24 1:52 PM, dthaler1968@googlemail.com wrote:
>> -----Original Message-----
>> From: Yonghong Song <yonghong.song@linux.dev>
>> Sent: Monday, February 12, 2024 1:49 PM
>> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
>> <dthaler1968=40googlemail.com@dmarc.ietf.org>
>> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
>> <dthaler1968@gmail.com>
>> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
>> conformance group
>>
>>
>> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
>>>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
>> only, see `Program-local functions`_
>>> If the instruction requires a register operand, why not using one of
>>> the register fields?  Is there any reason for not doing that?
>> Talked to Alexei and we think using dst_reg for the register for callx insn is
>> better. I will craft a llvm patch for this today. Thanks!
> Why dst_reg instead of src_reg?
> BPF_X is supposed to mean use src_reg.

Let us use dst_reg. Currently, for BPF_K, we have src_reg for a bunch
of flags (pseudo call, kfunc call, etc.). So for BPF_X, let us preserve this
property as well in case in the future we will introduce variants for
callx. The following is the llvm diff:

https://github.com/llvm/llvm-project/pull/81546

>
> But this thread is about reserving/documenting the existing practice,
> since anyone trying to use it would run into interop issues because
> of existing clang.   Should we document both and list one as deprecated?

I think just documenting the new encoding is good enough. But other
people can chime in just in case that I missed something.

>
> Dave
>

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Yonghong Song]

On 2/12/24 1:52 PM, dthaler1968@googlemail.com wrote:
>> -----Original Message-----
>> From: Yonghong Song <yonghong.song@linux.dev>
>> Sent: Monday, February 12, 2024 1:49 PM
>> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
>> <dthaler1968=40googlemail.com@dmarc.ietf.org>
>> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
>> <dthaler1968@gmail.com>
>> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
>> conformance group
>>
>>
>> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
>>>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
>> only, see `Program-local functions`_
>>> If the instruction requires a register operand, why not using one of
>>> the register fields?  Is there any reason for not doing that?
>> Talked to Alexei and we think using dst_reg for the register for callx insn is
>> better. I will craft a llvm patch for this today. Thanks!
> Why dst_reg instead of src_reg?
> BPF_X is supposed to mean use src_reg.

Let us use dst_reg. Currently, for BPF_K, we have src_reg for a bunch
of flags (pseudo call, kfunc call, etc.). So for BPF_X, let us preserve this
property as well in case in the future we will introduce variants for
callx. The following is the llvm diff:

https://github.com/llvm/llvm-project/pull/81546

>
> But this thread is about reserving/documenting the existing practice,
> since anyone trying to use it would run into interop issues because
> of existing clang.   Should we document both and list one as deprecated?

I think just documenting the new encoding is good enough. But other
people can chime in just in case that I missed something.

>
> Dave
>

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

RE: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[dthaler1968]

> -----Original Message-----
> From: Yonghong Song <yonghong.song@linux.dev>
> Sent: Monday, February 12, 2024 2:49 PM
> To: dthaler1968@googlemail.com; 'Jose E. Marchesi'
> <jose.marchesi@oracle.com>; 'Dave Thaler'
> <dthaler1968=40googlemail.com@dmarc.ietf.org>
> Cc: bpf@vger.kernel.org; bpf@ietf.org
> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
> conformance group
> 
> 
> On 2/12/24 1:52 PM, dthaler1968@googlemail.com wrote:
> >> -----Original Message-----
> >> From: Yonghong Song <yonghong.song@linux.dev>
> >> Sent: Monday, February 12, 2024 1:49 PM
> >> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
> >> <dthaler1968=40googlemail.com@dmarc.ietf.org>
> >> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
> >> <dthaler1968@gmail.com>
> >> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx
> >> instructions in new conformance group
> >>
> >>
> >> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
> >>>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
> >> only, see `Program-local functions`_
> >>> If the instruction requires a register operand, why not using one of
> >>> the register fields?  Is there any reason for not doing that?
> >> Talked to Alexei and we think using dst_reg for the register for
> >> callx insn is better. I will craft a llvm patch for this today. Thanks!
> > Why dst_reg instead of src_reg?
> > BPF_X is supposed to mean use src_reg.
> 
> Let us use dst_reg. Currently, for BPF_K, we have src_reg for a bunch of flags
> (pseudo call, kfunc call, etc.). So for BPF_X, let us preserve this property as
> well in case in the future we will introduce variants for callx.

Ah yes, that makes sense.

> The following is the llvm diff:
> 
> https://github.com/llvm/llvm-project/pull/81546

Which llvm release is it targeted for?
18.1.0-rc3? 18.1.1?  later?

> > But this thread is about reserving/documenting the existing practice,
> > since anyone trying to use it would run into interop issues because
> > of existing clang.   Should we document both and list one as deprecated?
> 
> I think just documenting the new encoding is good enough. But other
> people can chime in just in case that I missed something.

Ok.

Dave

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[dthaler1968=40googlemail.com]

> -----Original Message-----
> From: Yonghong Song <yonghong.song@linux.dev>
> Sent: Monday, February 12, 2024 2:49 PM
> To: dthaler1968@googlemail.com; 'Jose E. Marchesi'
> <jose.marchesi@oracle.com>; 'Dave Thaler'
> <dthaler1968=40googlemail.com@dmarc.ietf.org>
> Cc: bpf@vger.kernel.org; bpf@ietf.org
> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
> conformance group
> 
> 
> On 2/12/24 1:52 PM, dthaler1968@googlemail.com wrote:
> >> -----Original Message-----
> >> From: Yonghong Song <yonghong.song@linux.dev>
> >> Sent: Monday, February 12, 2024 1:49 PM
> >> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
> >> <dthaler1968=40googlemail.com@dmarc.ietf.org>
> >> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
> >> <dthaler1968@gmail.com>
> >> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx
> >> instructions in new conformance group
> >>
> >>
> >> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
> >>>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
> >> only, see `Program-local functions`_
> >>> If the instruction requires a register operand, why not using one of
> >>> the register fields?  Is there any reason for not doing that?
> >> Talked to Alexei and we think using dst_reg for the register for
> >> callx insn is better. I will craft a llvm patch for this today. Thanks!
> > Why dst_reg instead of src_reg?
> > BPF_X is supposed to mean use src_reg.
> 
> Let us use dst_reg. Currently, for BPF_K, we have src_reg for a bunch of flags
> (pseudo call, kfunc call, etc.). So for BPF_X, let us preserve this property as
> well in case in the future we will introduce variants for callx.

Ah yes, that makes sense.

> The following is the llvm diff:
> 
> https://github.com/llvm/llvm-project/pull/81546

Which llvm release is it targeted for?
18.1.0-rc3? 18.1.1?  later?

> > But this thread is about reserving/documenting the existing practice,
> > since anyone trying to use it would run into interop issues because
> > of existing clang.   Should we document both and list one as deprecated?
> 
> I think just documenting the new encoding is good enough. But other
> people can chime in just in case that I missed something.

Ok.

Dave

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Yonghong Song]

On 2/12/24 5:18 PM, dthaler1968@googlemail.com wrote:
>> -----Original Message-----
>> From: Yonghong Song <yonghong.song@linux.dev>
>> Sent: Monday, February 12, 2024 2:49 PM
>> To: dthaler1968@googlemail.com; 'Jose E. Marchesi'
>> <jose.marchesi@oracle.com>; 'Dave Thaler'
>> <dthaler1968=40googlemail.com@dmarc.ietf.org>
>> Cc: bpf@vger.kernel.org; bpf@ietf.org
>> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
>> conformance group
>>
>>
>> On 2/12/24 1:52 PM, dthaler1968@googlemail.com wrote:
>>>> -----Original Message-----
>>>> From: Yonghong Song <yonghong.song@linux.dev>
>>>> Sent: Monday, February 12, 2024 1:49 PM
>>>> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
>>>> <dthaler1968=40googlemail.com@dmarc.ietf.org>
>>>> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
>>>> <dthaler1968@gmail.com>
>>>> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx
>>>> instructions in new conformance group
>>>>
>>>>
>>>> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
>>>>>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
>>>> only, see `Program-local functions`_
>>>>> If the instruction requires a register operand, why not using one of
>>>>> the register fields?  Is there any reason for not doing that?
>>>> Talked to Alexei and we think using dst_reg for the register for
>>>> callx insn is better. I will craft a llvm patch for this today. Thanks!
>>> Why dst_reg instead of src_reg?
>>> BPF_X is supposed to mean use src_reg.
>> Let us use dst_reg. Currently, for BPF_K, we have src_reg for a bunch of flags
>> (pseudo call, kfunc call, etc.). So for BPF_X, let us preserve this property as
>> well in case in the future we will introduce variants for callx.
> Ah yes, that makes sense.
>
>> The following is the llvm diff:
>>
>> https://github.com/llvm/llvm-project/pull/81546
> Which llvm release is it targeted for?
> 18.1.0-rc3? 18.1.1?  later?

llvm19

>
>>> But this thread is about reserving/documenting the existing practice,
>>> since anyone trying to use it would run into interop issues because
>>> of existing clang.   Should we document both and list one as deprecated?
>> I think just documenting the new encoding is good enough. But other
>> people can chime in just in case that I missed something.
> Ok.
>
> Dave
>

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Yonghong Song]

On 2/12/24 5:18 PM, dthaler1968@googlemail.com wrote:
>> -----Original Message-----
>> From: Yonghong Song <yonghong.song@linux.dev>
>> Sent: Monday, February 12, 2024 2:49 PM
>> To: dthaler1968@googlemail.com; 'Jose E. Marchesi'
>> <jose.marchesi@oracle.com>; 'Dave Thaler'
>> <dthaler1968=40googlemail.com@dmarc.ietf.org>
>> Cc: bpf@vger.kernel.org; bpf@ietf.org
>> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
>> conformance group
>>
>>
>> On 2/12/24 1:52 PM, dthaler1968@googlemail.com wrote:
>>>> -----Original Message-----
>>>> From: Yonghong Song <yonghong.song@linux.dev>
>>>> Sent: Monday, February 12, 2024 1:49 PM
>>>> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
>>>> <dthaler1968=40googlemail.com@dmarc.ietf.org>
>>>> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
>>>> <dthaler1968@gmail.com>
>>>> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx
>>>> instructions in new conformance group
>>>>
>>>>
>>>> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
>>>>>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
>>>> only, see `Program-local functions`_
>>>>> If the instruction requires a register operand, why not using one of
>>>>> the register fields?  Is there any reason for not doing that?
>>>> Talked to Alexei and we think using dst_reg for the register for
>>>> callx insn is better. I will craft a llvm patch for this today. Thanks!
>>> Why dst_reg instead of src_reg?
>>> BPF_X is supposed to mean use src_reg.
>> Let us use dst_reg. Currently, for BPF_K, we have src_reg for a bunch of flags
>> (pseudo call, kfunc call, etc.). So for BPF_X, let us preserve this property as
>> well in case in the future we will introduce variants for callx.
> Ah yes, that makes sense.
>
>> The following is the llvm diff:
>>
>> https://github.com/llvm/llvm-project/pull/81546
> Which llvm release is it targeted for?
> 18.1.0-rc3? 18.1.1?  later?

llvm19

>
>>> But this thread is about reserving/documenting the existing practice,
>>> since anyone trying to use it would run into interop issues because
>>> of existing clang.   Should we document both and list one as deprecated?
>> I think just documenting the new encoding is good enough. But other
>> people can chime in just in case that I missed something.
> Ok.
>
> Dave
>

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Jose E. Marchesi]

> On 2/12/24 1:52 PM, dthaler1968@googlemail.com wrote:
>>> -----Original Message-----
>>> From: Yonghong Song <yonghong.song@linux.dev>
>>> Sent: Monday, February 12, 2024 1:49 PM
>>> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
>>> <dthaler1968=40googlemail.com@dmarc.ietf.org>
>>> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
>>> <dthaler1968@gmail.com>
>>> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
>>> conformance group
>>>
>>>
>>> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
>>>>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
>>> only, see `Program-local functions`_
>>>> If the instruction requires a register operand, why not using one of
>>>> the register fields?  Is there any reason for not doing that?
>>> Talked to Alexei and we think using dst_reg for the register for callx insn is
>>> better. I will craft a llvm patch for this today. Thanks!
>> Why dst_reg instead of src_reg?
>> BPF_X is supposed to mean use src_reg.
>
> Let us use dst_reg. Currently, for BPF_K, we have src_reg for a bunch
> of flags (pseudo call, kfunc call, etc.). So for BPF_X, let us preserve this
> property as well in case in the future we will introduce variants for
> callx. The following is the llvm diff:
>
> https://github.com/llvm/llvm-project/pull/81546

Thank you.

I believe Will will be sending a patch to binutils to change the
pseudo-C syntax for the instruction to callx instead of callr.  We will
then adapt GCC accordingly, and both compilers will be doing exactly the
same regarding callx.

>>
>> But this thread is about reserving/documenting the existing practice,
>> since anyone trying to use it would run into interop issues because
>> of existing clang.   Should we document both and list one as deprecated?
>
> I think just documenting the new encoding is good enough. But other
> people can chime in just in case that I missed something.
>
>>
>> Dave
>>

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new conformance group

[Jose E. Marchesi]

> On 2/12/24 1:52 PM, dthaler1968@googlemail.com wrote:
>>> -----Original Message-----
>>> From: Yonghong Song <yonghong.song@linux.dev>
>>> Sent: Monday, February 12, 2024 1:49 PM
>>> To: Jose E. Marchesi <jose.marchesi@oracle.com>; Dave Thaler
>>> <dthaler1968=40googlemail.com@dmarc.ietf.org>
>>> Cc: bpf@vger.kernel.org; bpf@ietf.org; Dave Thaler
>>> <dthaler1968@gmail.com>
>>> Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add callx instructions in new
>>> conformance group
>>>
>>>
>>> On 2/12/24 1:28 PM, Jose E. Marchesi wrote:
>>>>> +BPF_CALL  0x8    0x1  call PC += reg_val(imm)          BPF_JMP | BPF_X
>>> only, see `Program-local functions`_
>>>> If the instruction requires a register operand, why not using one of
>>>> the register fields?  Is there any reason for not doing that?
>>> Talked to Alexei and we think using dst_reg for the register for callx insn is
>>> better. I will craft a llvm patch for this today. Thanks!
>> Why dst_reg instead of src_reg?
>> BPF_X is supposed to mean use src_reg.
>
> Let us use dst_reg. Currently, for BPF_K, we have src_reg for a bunch
> of flags (pseudo call, kfunc call, etc.). So for BPF_X, let us preserve this
> property as well in case in the future we will introduce variants for
> callx. The following is the llvm diff:
>
> https://github.com/llvm/llvm-project/pull/81546

Thank you.

I believe Will will be sending a patch to binutils to change the
pseudo-C syntax for the instruction to callx instead of callr.  We will
then adapt GCC accordingly, and both compilers will be doing exactly the
same regarding callx.

>>
>> But this thread is about reserving/documenting the existing practice,
>> since anyone trying to use it would run into interop issues because
>> of existing clang.   Should we document both and list one as deprecated?
>
> I think just documenting the new encoding is good enough. But other
> people can chime in just in case that I missed something.
>
>>
>> Dave
>>