All of lore.kernel.org
 help / color / mirror / Atom feed
* [Bpf] Sign extension ISA question
@ 2024-01-16 20:38 dthaler1968=40googlemail.com
  2024-01-16 20:55   ` [Bpf] " dthaler1968=40googlemail.com
  0 siblings, 1 reply; 23+ messages in thread
From: dthaler1968=40googlemail.com @ 2024-01-16 20:38 UTC (permalink / raw)
  To: bpf, bpf


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

Is there any semantic difference between the following two instructions?

 

{.opcode = BPF_ALU64 | BPF_MOV | BPF_K, .offset = 0, .imm = -1}

 

{.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}

 

>From my reading both of them treat imm as a signed 32-bit number and
sign-extend it to 64 bits.

 

Dave


[-- Attachment #1.2: Type: text/html, Size: 2092 bytes --]

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

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Sign extension ISA question
@ 2024-01-16 20:55   ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968 @ 2024-01-16 20:55 UTC (permalink / raw)
  To: bpf, bpf

(Resending since a spam filter seems to have blocked the first attempt.)

Is there any semantic difference between the following two instructions?

{.opcode = BPF_ALU64 | BPF_MOV | BPF_K, .offset = 0, .imm = -1}

{.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}

From my reading both of them treat imm as a signed 32-bit number and
sign-extend it to 64 bits.

Dave


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bpf] Sign extension ISA question
@ 2024-01-16 20:55   ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968=40googlemail.com @ 2024-01-16 20:55 UTC (permalink / raw)
  To: bpf, bpf

(Resending since a spam filter seems to have blocked the first attempt.)

Is there any semantic difference between the following two instructions?

{.opcode = BPF_ALU64 | BPF_MOV | BPF_K, .offset = 0, .imm = -1}

{.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}

>From my reading both of them treat imm as a signed 32-bit number and
sign-extend it to 64 bits.

Dave

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Sign extension ISA question
@ 2024-01-16 22:34     ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-16 22:34 UTC (permalink / raw)
  To: dthaler1968, bpf, bpf


On 1/16/24 12:55 PM, dthaler1968@googlemail.com wrote:
> (Resending since a spam filter seems to have blocked the first attempt.)
>
> Is there any semantic difference between the following two instructions?
>
> {.opcode = BPF_ALU64 | BPF_MOV | BPF_K, .offset = 0, .imm = -1}

This is supported. Sign extension of -1 will be put into ALU64 reg.

>
> {.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}

This is not supported. BPF_MOVSX only supports register extension.
We should make it clear in the doc.

>
>  From my reading both of them treat imm as a signed 32-bit number and
> sign-extend it to 64 bits.
>
> Dave
>
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Bpf] Sign extension ISA question
@ 2024-01-16 22:34     ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-16 22:34 UTC (permalink / raw)
  To: dthaler1968, bpf, bpf


On 1/16/24 12:55 PM, dthaler1968@googlemail.com wrote:
> (Resending since a spam filter seems to have blocked the first attempt.)
>
> Is there any semantic difference between the following two instructions?
>
> {.opcode = BPF_ALU64 | BPF_MOV | BPF_K, .offset = 0, .imm = -1}

This is supported. Sign extension of -1 will be put into ALU64 reg.

>
> {.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}

This is not supported. BPF_MOVSX only supports register extension.
We should make it clear in the doc.

>
>  From my reading both of them treat imm as a signed 32-bit number and
> sign-extend it to 64 bits.
>
> Dave
>
>

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: Sign extension ISA question
@ 2024-01-17  1:56       ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968 @ 2024-01-17  1:56 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

Yonghong Song <yonghong.song@linux.dev> wrote:
> > Is there any semantic difference between the following two instructions?
> >
> > {.opcode = BPF_ALU64 | BPF_MOV | BPF_K, .offset = 0, .imm = -1}
> 
> This is supported. Sign extension of -1 will be put into ALU64 reg.
> 
> >
> > {.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}
> 
> This is not supported. BPF_MOVSX only supports register extension.
> We should make it clear in the doc.

Is that limitation a Linux-specific implementation statement? (i.e., put into
linux-notes.txt)

Or that the meaning is undefined for all runtimes and could be used
for some other purpose in the future?  (i.e., put into instruction-set.rst)

For now I'll interpret it as the latter.

Thanks,
Dave



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Bpf] Sign extension ISA question
@ 2024-01-17  1:56       ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968=40googlemail.com @ 2024-01-17  1:56 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

Yonghong Song <yonghong.song@linux.dev> wrote:
> > Is there any semantic difference between the following two instructions?
> >
> > {.opcode = BPF_ALU64 | BPF_MOV | BPF_K, .offset = 0, .imm = -1}
> 
> This is supported. Sign extension of -1 will be put into ALU64 reg.
> 
> >
> > {.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}
> 
> This is not supported. BPF_MOVSX only supports register extension.
> We should make it clear in the doc.

Is that limitation a Linux-specific implementation statement? (i.e., put into
linux-notes.txt)

Or that the meaning is undefined for all runtimes and could be used
for some other purpose in the future?  (i.e., put into instruction-set.rst)

For now I'll interpret it as the latter.

Thanks,
Dave


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Sign extension ISA question
@ 2024-01-17  3:48         ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-17  3:48 UTC (permalink / raw)
  To: dthaler1968; +Cc: bpf, bpf

On 1/16/24 5:56 PM, dthaler1968@googlemail.com wrote:
> Yonghong Song <yonghong.song@linux.dev> wrote:
>>> Is there any semantic difference between the following two instructions?
>>>
>>> {.opcode = BPF_ALU64 | BPF_MOV | BPF_K, .offset = 0, .imm = -1}
>> This is supported. Sign extension of -1 will be put into ALU64 reg.
>>
>>> {.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}
>> This is not supported. BPF_MOVSX only supports register extension.
>> We should make it clear in the doc.
> Is that limitation a Linux-specific implementation statement? (i.e., put into
> linux-notes.txt)
>
> Or that the meaning is undefined for all runtimes and could be used
> for some other purpose in the future?  (i.e., put into instruction-set.rst)
>
> For now I'll interpret it as the latter.

You are right. The
   {.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}
is not supported by bpf ISA. Currently, it will be an illegal encoding
from kernel perspective.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Bpf] Sign extension ISA question
@ 2024-01-17  3:48         ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-17  3:48 UTC (permalink / raw)
  To: dthaler1968; +Cc: bpf, bpf

On 1/16/24 5:56 PM, dthaler1968@googlemail.com wrote:
> Yonghong Song <yonghong.song@linux.dev> wrote:
>>> Is there any semantic difference between the following two instructions?
>>>
>>> {.opcode = BPF_ALU64 | BPF_MOV | BPF_K, .offset = 0, .imm = -1}
>> This is supported. Sign extension of -1 will be put into ALU64 reg.
>>
>>> {.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}
>> This is not supported. BPF_MOVSX only supports register extension.
>> We should make it clear in the doc.
> Is that limitation a Linux-specific implementation statement? (i.e., put into
> linux-notes.txt)
>
> Or that the meaning is undefined for all runtimes and could be used
> for some other purpose in the future?  (i.e., put into instruction-set.rst)
>
> For now I'll interpret it as the latter.

You are right. The
   {.opcode = BPF_ALU64 | BPF_MOVSX | BPF_K, .offset = 32, .imm = -1}
is not supported by bpf ISA. Currently, it will be an illegal encoding
from kernel perspective.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Jump instructions clarification
@ 2024-01-24  2:07           ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968 @ 2024-01-24  2:07 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

Hi Yonghong,

The MOVSX clarification is now merged, but I just found another similar question for you
regarding jump instructions.

For BPF_CALL (same question for src=0, src=1, and src=2),
are both BPF_JMP and BPF_JMP32 legal?  If so, is there a semantic difference?
If not, then again I think the doc needs clarification.

BPF_JA's use of imm already has a note that it's BPF_JMP32 class only,
but what about BPF_CALL's use of imm?

Similarly about comparisons like BPF_JEQ etc when BPF_K is set. 
E.g., is BPF_JEQ | BPF_K | BPF_JMP permitted?  The document currently
has no restriction against it, but if it's permitted, the meaning is not explained.

Dave



^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bpf] Jump instructions clarification
@ 2024-01-24  2:07           ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968=40googlemail.com @ 2024-01-24  2:07 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

Hi Yonghong,

The MOVSX clarification is now merged, but I just found another similar question for you
regarding jump instructions.

For BPF_CALL (same question for src=0, src=1, and src=2),
are both BPF_JMP and BPF_JMP32 legal?  If so, is there a semantic difference?
If not, then again I think the doc needs clarification.

BPF_JA's use of imm already has a note that it's BPF_JMP32 class only,
but what about BPF_CALL's use of imm?

Similarly about comparisons like BPF_JEQ etc when BPF_K is set. 
E.g., is BPF_JEQ | BPF_K | BPF_JMP permitted?  The document currently
has no restriction against it, but if it's permitted, the meaning is not explained.

Dave


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Jump instructions clarification
@ 2024-01-24 19:33             ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-24 19:33 UTC (permalink / raw)
  To: dthaler1968; +Cc: bpf, bpf


On 1/23/24 6:07 PM, dthaler1968@googlemail.com wrote:
> Hi Yonghong,
>
> The MOVSX clarification is now merged, but I just found another similar question for you
> regarding jump instructions.
>
> For BPF_CALL (same question for src=0, src=1, and src=2),
> are both BPF_JMP and BPF_JMP32 legal?  If so, is there a semantic difference?
> If not, then again I think the doc needs clarification.

BPF_CALL with BPF_JMP32 is illegal. This is true for src=0/1/2.

>
> BPF_JA's use of imm already has a note that it's BPF_JMP32 class only,
> but what about BPF_CALL's use of imm?

The imm field of BPF_CALL insn is used for call target.

>
> Similarly about comparisons like BPF_JEQ etc when BPF_K is set.
> E.g., is BPF_JEQ | BPF_K | BPF_JMP permitted?  The document currently
> has no restriction against it, but if it's permitted, the meaning is not explained.

Yes, it is permetted. It represents a 64bit reg compared to an imm.

>
> Dave
>
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Bpf] Jump instructions clarification
@ 2024-01-24 19:33             ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-24 19:33 UTC (permalink / raw)
  To: dthaler1968; +Cc: bpf, bpf


On 1/23/24 6:07 PM, dthaler1968@googlemail.com wrote:
> Hi Yonghong,
>
> The MOVSX clarification is now merged, but I just found another similar question for you
> regarding jump instructions.
>
> For BPF_CALL (same question for src=0, src=1, and src=2),
> are both BPF_JMP and BPF_JMP32 legal?  If so, is there a semantic difference?
> If not, then again I think the doc needs clarification.

BPF_CALL with BPF_JMP32 is illegal. This is true for src=0/1/2.

>
> BPF_JA's use of imm already has a note that it's BPF_JMP32 class only,
> but what about BPF_CALL's use of imm?

The imm field of BPF_CALL insn is used for call target.

>
> Similarly about comparisons like BPF_JEQ etc when BPF_K is set.
> E.g., is BPF_JEQ | BPF_K | BPF_JMP permitted?  The document currently
> has no restriction against it, but if it's permitted, the meaning is not explained.

Yes, it is permetted. It represents a 64bit reg compared to an imm.

>
> Dave
>
>

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* 64-bit immediate instructions clarification
@ 2024-01-26  1:12               ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968 @ 2024-01-26  1:12 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

The spec defines:
> As discussed below in `64-bit immediate instructions`_, a 64-bit immediate
> instruction uses a 64-bit immediate value that is constructed as follows.
> The 64 bits following the basic instruction contain a pseudo instruction
> using the same format but with opcode, dst_reg, src_reg, and offset all set to zero,
> and imm containing the high 32 bits of the immediate value.
[...]
> imm64 = (next_imm << 32) | imm

The 64-bit immediate instructions section then says:
> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide instruction
> encoding defined in `Instruction encoding`_, and use the 'src' field of the
> basic instruction to hold an opcode subtype.

Some instructions then nicely state how to use the full 64 bit immediate value, such as
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64                                integer      integer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm)) + next_imm  map index    data pointer

Others don't:
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)                       map fd       map
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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

How is next_imm used in those four?  Must it be 0?  Or can it be anything and it's ignored?
Or is it used for something?

Dave


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Bpf] 64-bit immediate instructions clarification
@ 2024-01-26  1:12               ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968=40googlemail.com @ 2024-01-26  1:12 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

The spec defines:
> As discussed below in `64-bit immediate instructions`_, a 64-bit immediate
> instruction uses a 64-bit immediate value that is constructed as follows.
> The 64 bits following the basic instruction contain a pseudo instruction
> using the same format but with opcode, dst_reg, src_reg, and offset all set to zero,
> and imm containing the high 32 bits of the immediate value.
[...]
> imm64 = (next_imm << 32) | imm

The 64-bit immediate instructions section then says:
> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide instruction
> encoding defined in `Instruction encoding`_, and use the 'src' field of the
> basic instruction to hold an opcode subtype.

Some instructions then nicely state how to use the full 64 bit immediate value, such as
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64                                integer      integer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm)) + next_imm  map index    data pointer

Others don't:
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)                       map fd       map
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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

How is next_imm used in those four?  Must it be 0?  Or can it be anything and it's ignored?
Or is it used for something?

Dave

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: 64-bit immediate instructions clarification
@ 2024-01-26  5:34                 ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-26  5:34 UTC (permalink / raw)
  To: dthaler1968; +Cc: bpf, bpf


On 1/25/24 5:12 PM, dthaler1968@googlemail.com wrote:
> The spec defines:
>> As discussed below in `64-bit immediate instructions`_, a 64-bit immediate
>> instruction uses a 64-bit immediate value that is constructed as follows.
>> The 64 bits following the basic instruction contain a pseudo instruction
>> using the same format but with opcode, dst_reg, src_reg, and offset all set to zero,
>> and imm containing the high 32 bits of the immediate value.
> [...]
>> imm64 = (next_imm << 32) | imm
> The 64-bit immediate instructions section then says:
>> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide instruction
>> encoding defined in `Instruction encoding`_, and use the 'src' field of the
>> basic instruction to hold an opcode subtype.
> Some instructions then nicely state how to use the full 64 bit immediate value, such as
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64                                integer      integer
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm)) + next_imm  map index    data pointer
> Others don't:
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)                       map fd       map
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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
> How is next_imm used in those four?  Must it be 0?  Or can it be anything and it's ignored?
> Or is it used for something?

The other four must have next_imm to be 0. No use of next_imm in thee four insns kindly implies this.
See uapi bpf.h for details (search BPF_PSEUDO_MAP_FD).

>
> Dave
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Bpf] 64-bit immediate instructions clarification
@ 2024-01-26  5:34                 ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-26  5:34 UTC (permalink / raw)
  To: dthaler1968; +Cc: bpf, bpf


On 1/25/24 5:12 PM, dthaler1968@googlemail.com wrote:
> The spec defines:
>> As discussed below in `64-bit immediate instructions`_, a 64-bit immediate
>> instruction uses a 64-bit immediate value that is constructed as follows.
>> The 64 bits following the basic instruction contain a pseudo instruction
>> using the same format but with opcode, dst_reg, src_reg, and offset all set to zero,
>> and imm containing the high 32 bits of the immediate value.
> [...]
>> imm64 = (next_imm << 32) | imm
> The 64-bit immediate instructions section then says:
>> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide instruction
>> encoding defined in `Instruction encoding`_, and use the 'src' field of the
>> basic instruction to hold an opcode subtype.
> Some instructions then nicely state how to use the full 64 bit immediate value, such as
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64                                integer      integer
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm)) + next_imm  map index    data pointer
> Others don't:
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)                       map fd       map
>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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
> How is next_imm used in those four?  Must it be 0?  Or can it be anything and it's ignored?
> Or is it used for something?

The other four must have next_imm to be 0. No use of next_imm in thee four insns kindly implies this.
See uapi bpf.h for details (search BPF_PSEUDO_MAP_FD).

>
> Dave
>

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: 64-bit immediate instructions clarification
@ 2024-01-26 22:27                   ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968 @ 2024-01-26 22:27 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

Yonghong Song <yonghong.song@linux.dev> wrote: 
> On 1/25/24 5:12 PM, dthaler1968@googlemail.com wrote:
> > The spec defines:
> >> As discussed below in `64-bit immediate instructions`_, a 64-bit
> >> immediate instruction uses a 64-bit immediate value that is constructed as
> follows.
> >> The 64 bits following the basic instruction contain a pseudo
> >> instruction using the same format but with opcode, dst_reg, src_reg,
> >> and offset all set to zero, and imm containing the high 32 bits of the
> immediate value.
> > [...]
> >> imm64 = (next_imm << 32) | imm
> > The 64-bit immediate instructions section then says:
> >> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide
> >> instruction encoding defined in `Instruction encoding`_, and use the
> >> 'src' field of the basic instruction to hold an opcode subtype.
> > Some instructions then nicely state how to use the full 64 bit
> > immediate value, such as
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64
> integer      integer
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm))
> + next_imm   map fd       data pointer
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm))
> + next_imm  map index    data pointer
> > Others don't:
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)
> map fd       map
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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
> > How is next_imm used in those four?  Must it be 0?  Or can it be anything and
> it's ignored?
> > Or is it used for something?
> 
> The other four must have next_imm to be 0. No use of next_imm in thee four
> insns kindly implies this.
> See uapi bpf.h for details (search BPF_PSEUDO_MAP_FD).

Thanks for confirming.  The "Instruction encoding" section has misleading text
in my opinion.

It nicely says:
> Note that most instructions do not use all of the fields. Unused fields shall be cleared to zero.

But then goes on to say:
> As discussed below in 64-bit immediate instructions (Section 4.4), a 64-bit immediate instruction
> uses a 64-bit immediate value that is constructed as follows.
[...]
> imm64 = (next_imm << 32) | imm

Under a normal English reading, that could imply that all 64-bit immediate instructions use imm64,
which is not the case.  The whole imm64 discussion there only applies today to src=0 (though I
suppose it could be used by future 64-bit immediate instructions).   Minimally I think
"a 64-bit immediate instruction uses" should be "some 64-bit immediate instructions use"
but at present there's only one.

It would actually be simpler to remove the imm64 text and just have the
definition of src 0x0 change from: "dst = imm64" to "dst = (next_imm << 32) | imm".

What do you think?

Dave


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Bpf] 64-bit immediate instructions clarification
@ 2024-01-26 22:27                   ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968=40googlemail.com @ 2024-01-26 22:27 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

Yonghong Song <yonghong.song@linux.dev> wrote: 
> On 1/25/24 5:12 PM, dthaler1968@googlemail.com wrote:
> > The spec defines:
> >> As discussed below in `64-bit immediate instructions`_, a 64-bit
> >> immediate instruction uses a 64-bit immediate value that is constructed as
> follows.
> >> The 64 bits following the basic instruction contain a pseudo
> >> instruction using the same format but with opcode, dst_reg, src_reg,
> >> and offset all set to zero, and imm containing the high 32 bits of the
> immediate value.
> > [...]
> >> imm64 = (next_imm << 32) | imm
> > The 64-bit immediate instructions section then says:
> >> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide
> >> instruction encoding defined in `Instruction encoding`_, and use the
> >> 'src' field of the basic instruction to hold an opcode subtype.
> > Some instructions then nicely state how to use the full 64 bit
> > immediate value, such as
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64
> integer      integer
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm))
> + next_imm   map fd       data pointer
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm))
> + next_imm  map index    data pointer
> > Others don't:
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)
> map fd       map
> >> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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
> > How is next_imm used in those four?  Must it be 0?  Or can it be anything and
> it's ignored?
> > Or is it used for something?
> 
> The other four must have next_imm to be 0. No use of next_imm in thee four
> insns kindly implies this.
> See uapi bpf.h for details (search BPF_PSEUDO_MAP_FD).

Thanks for confirming.  The "Instruction encoding" section has misleading text
in my opinion.

It nicely says:
> Note that most instructions do not use all of the fields. Unused fields shall be cleared to zero.

But then goes on to say:
> As discussed below in 64-bit immediate instructions (Section 4.4), a 64-bit immediate instruction
> uses a 64-bit immediate value that is constructed as follows.
[...]
> imm64 = (next_imm << 32) | imm

Under a normal English reading, that could imply that all 64-bit immediate instructions use imm64,
which is not the case.  The whole imm64 discussion there only applies today to src=0 (though I
suppose it could be used by future 64-bit immediate instructions).   Minimally I think
"a 64-bit immediate instruction uses" should be "some 64-bit immediate instructions use"
but at present there's only one.

It would actually be simpler to remove the imm64 text and just have the
definition of src 0x0 change from: "dst = imm64" to "dst = (next_imm << 32) | imm".

What do you think?

Dave

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: 64-bit immediate instructions clarification
@ 2024-01-27  3:41                     ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-27  3:41 UTC (permalink / raw)
  To: dthaler1968; +Cc: bpf, bpf


On 1/26/24 2:27 PM, dthaler1968@googlemail.com wrote:
> Yonghong Song <yonghong.song@linux.dev> wrote:
>> On 1/25/24 5:12 PM, dthaler1968@googlemail.com wrote:
>>> The spec defines:
>>>> As discussed below in `64-bit immediate instructions`_, a 64-bit
>>>> immediate instruction uses a 64-bit immediate value that is constructed as
>> follows.
>>>> The 64 bits following the basic instruction contain a pseudo
>>>> instruction using the same format but with opcode, dst_reg, src_reg,
>>>> and offset all set to zero, and imm containing the high 32 bits of the
>> immediate value.
>>> [...]
>>>> imm64 = (next_imm << 32) | imm
>>> The 64-bit immediate instructions section then says:
>>>> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide
>>>> instruction encoding defined in `Instruction encoding`_, and use the
>>>> 'src' field of the basic instruction to hold an opcode subtype.
>>> Some instructions then nicely state how to use the full 64 bit
>>> immediate value, such as
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64
>> integer      integer
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm))
>> + next_imm   map fd       data pointer
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm))
>> + next_imm  map index    data pointer
>>> Others don't:
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)
>> map fd       map
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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
>>> How is next_imm used in those four?  Must it be 0?  Or can it be anything and
>> it's ignored?
>>> Or is it used for something?
>> The other four must have next_imm to be 0. No use of next_imm in thee four
>> insns kindly implies this.
>> See uapi bpf.h for details (search BPF_PSEUDO_MAP_FD).
> Thanks for confirming.  The "Instruction encoding" section has misleading text
> in my opinion.
>
> It nicely says:
>> Note that most instructions do not use all of the fields. Unused fields shall be cleared to zero.
> But then goes on to say:
>> As discussed below in 64-bit immediate instructions (Section 4.4), a 64-bit immediate instruction
>> uses a 64-bit immediate value that is constructed as follows.
> [...]
>> imm64 = (next_imm << 32) | imm
> Under a normal English reading, that could imply that all 64-bit immediate instructions use imm64,
> which is not the case.  The whole imm64 discussion there only applies today to src=0 (though I
> suppose it could be used by future 64-bit immediate instructions).   Minimally I think
> "a 64-bit immediate instruction uses" should be "some 64-bit immediate instructions use"
> but at present there's only one.
>
> It would actually be simpler to remove the imm64 text and just have the
> definition of src 0x0 change from: "dst = imm64" to "dst = (next_imm << 32) | imm".
>
> What do you think?

it does sound better. Something like below?

diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
index af43227b6ee4..fceacca46299 100644
--- a/Documentation/bpf/standardization/instruction-set.rst
+++ b/Documentation/bpf/standardization/instruction-set.rst
@@ -166,7 +166,7 @@ Note that most instructions do not use all of the fields.
  Unused fields shall be cleared to zero.
  
  As discussed below in `64-bit immediate instructions`_, a 64-bit immediate
-instruction uses a 64-bit immediate value that is constructed as follows.
+instruction uses two 32-bit immediate values that are constructed as follows.
  The 64 bits following the basic instruction contain a pseudo instruction
  using the same format but with opcode, dst_reg, src_reg, and offset all set to zero,
  and imm containing the high 32 bits of the immediate value.
@@ -181,13 +181,8 @@ This is depicted in the following figure::
                                     '--------------'
                                    pseudo instruction
  
-Thus the 64-bit immediate value is constructed as follows:
-
-  imm64 = (next_imm << 32) | imm
-
-where 'next_imm' refers to the imm value of the pseudo instruction
-following the basic instruction.  The unused bytes in the pseudo
-instruction are reserved and shall be cleared to zero.
+Here, the imm value of the pseudo instruction is called 'next_imm'. The unused
+bytes in the pseudo instruction are reserved and shall be cleared to zero.
  
  Instruction classes
  -------------------
@@ -590,7 +585,7 @@ defined further 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    0x0  dst = (next_imm << 32) | imm               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 = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
  BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_addr(imm)                        variable id  data pointer

>
> Dave
>

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Bpf] 64-bit immediate instructions clarification
@ 2024-01-27  3:41                     ` Yonghong Song
  0 siblings, 0 replies; 23+ messages in thread
From: Yonghong Song @ 2024-01-27  3:41 UTC (permalink / raw)
  To: dthaler1968; +Cc: bpf, bpf


On 1/26/24 2:27 PM, dthaler1968@googlemail.com wrote:
> Yonghong Song <yonghong.song@linux.dev> wrote:
>> On 1/25/24 5:12 PM, dthaler1968@googlemail.com wrote:
>>> The spec defines:
>>>> As discussed below in `64-bit immediate instructions`_, a 64-bit
>>>> immediate instruction uses a 64-bit immediate value that is constructed as
>> follows.
>>>> The 64 bits following the basic instruction contain a pseudo
>>>> instruction using the same format but with opcode, dst_reg, src_reg,
>>>> and offset all set to zero, and imm containing the high 32 bits of the
>> immediate value.
>>> [...]
>>>> imm64 = (next_imm << 32) | imm
>>> The 64-bit immediate instructions section then says:
>>>> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide
>>>> instruction encoding defined in `Instruction encoding`_, and use the
>>>> 'src' field of the basic instruction to hold an opcode subtype.
>>> Some instructions then nicely state how to use the full 64 bit
>>> immediate value, such as
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64
>> integer      integer
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm))
>> + next_imm   map fd       data pointer
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm))
>> + next_imm  map index    data pointer
>>> Others don't:
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)
>> map fd       map
>>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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
>>> How is next_imm used in those four?  Must it be 0?  Or can it be anything and
>> it's ignored?
>>> Or is it used for something?
>> The other four must have next_imm to be 0. No use of next_imm in thee four
>> insns kindly implies this.
>> See uapi bpf.h for details (search BPF_PSEUDO_MAP_FD).
> Thanks for confirming.  The "Instruction encoding" section has misleading text
> in my opinion.
>
> It nicely says:
>> Note that most instructions do not use all of the fields. Unused fields shall be cleared to zero.
> But then goes on to say:
>> As discussed below in 64-bit immediate instructions (Section 4.4), a 64-bit immediate instruction
>> uses a 64-bit immediate value that is constructed as follows.
> [...]
>> imm64 = (next_imm << 32) | imm
> Under a normal English reading, that could imply that all 64-bit immediate instructions use imm64,
> which is not the case.  The whole imm64 discussion there only applies today to src=0 (though I
> suppose it could be used by future 64-bit immediate instructions).   Minimally I think
> "a 64-bit immediate instruction uses" should be "some 64-bit immediate instructions use"
> but at present there's only one.
>
> It would actually be simpler to remove the imm64 text and just have the
> definition of src 0x0 change from: "dst = imm64" to "dst = (next_imm << 32) | imm".
>
> What do you think?

it does sound better. Something like below?

diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
index af43227b6ee4..fceacca46299 100644
--- a/Documentation/bpf/standardization/instruction-set.rst
+++ b/Documentation/bpf/standardization/instruction-set.rst
@@ -166,7 +166,7 @@ Note that most instructions do not use all of the fields.
  Unused fields shall be cleared to zero.
  
  As discussed below in `64-bit immediate instructions`_, a 64-bit immediate
-instruction uses a 64-bit immediate value that is constructed as follows.
+instruction uses two 32-bit immediate values that are constructed as follows.
  The 64 bits following the basic instruction contain a pseudo instruction
  using the same format but with opcode, dst_reg, src_reg, and offset all set to zero,
  and imm containing the high 32 bits of the immediate value.
@@ -181,13 +181,8 @@ This is depicted in the following figure::
                                     '--------------'
                                    pseudo instruction
  
-Thus the 64-bit immediate value is constructed as follows:
-
-  imm64 = (next_imm << 32) | imm
-
-where 'next_imm' refers to the imm value of the pseudo instruction
-following the basic instruction.  The unused bytes in the pseudo
-instruction are reserved and shall be cleared to zero.
+Here, the imm value of the pseudo instruction is called 'next_imm'. The unused
+bytes in the pseudo instruction are reserved and shall be cleared to zero.
  
  Instruction classes
  -------------------
@@ -590,7 +585,7 @@ defined further 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    0x0  dst = (next_imm << 32) | imm               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 = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
  BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_addr(imm)                        variable id  data pointer

>
> Dave
>

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* RE: 64-bit immediate instructions clarification
@ 2024-01-27  6:56                       ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968 @ 2024-01-27  6:56 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

> -----Original Message-----
> From: Yonghong Song <yonghong.song@linux.dev>
> Sent: Friday, January 26, 2024 7:41 PM
> To: dthaler1968@googlemail.com
> Cc: bpf@ietf.org; bpf@vger.kernel.org
> Subject: Re: 64-bit immediate instructions clarification
> 
> 
> On 1/26/24 2:27 PM, dthaler1968@googlemail.com wrote:
> > Yonghong Song <yonghong.song@linux.dev> wrote:
> >> On 1/25/24 5:12 PM, dthaler1968@googlemail.com wrote:
> >>> The spec defines:
> >>>> As discussed below in `64-bit immediate instructions`_, a 64-bit
> >>>> immediate instruction uses a 64-bit immediate value that is
> >>>> constructed as
> >> follows.
> >>>> The 64 bits following the basic instruction contain a pseudo
> >>>> instruction using the same format but with opcode, dst_reg,
> >>>> src_reg, and offset all set to zero, and imm containing the high 32
> >>>> bits of the
> >> immediate value.
> >>> [...]
> >>>> imm64 = (next_imm << 32) | imm
> >>> The 64-bit immediate instructions section then says:
> >>>> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide
> >>>> instruction encoding defined in `Instruction encoding`_, and use
> >>>> the 'src' field of the basic instruction to hold an opcode subtype.
> >>> Some instructions then nicely state how to use the full 64 bit
> >>> immediate value, such as
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64
> >> integer      integer
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst =
> map_val(map_by_fd(imm))
> >> + next_imm   map fd       data pointer
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst =
> map_val(map_by_idx(imm))
> >> + next_imm  map index    data pointer
> >>> Others don't:
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)
> >> map fd       map
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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
> >>> How is next_imm used in those four?  Must it be 0?  Or can it be
> >>> anything and
> >> it's ignored?
> >>> Or is it used for something?
> >> The other four must have next_imm to be 0. No use of next_imm in thee
> >> four insns kindly implies this.
> >> See uapi bpf.h for details (search BPF_PSEUDO_MAP_FD).
> > Thanks for confirming.  The "Instruction encoding" section has
> > misleading text in my opinion.
> >
> > It nicely says:
> >> Note that most instructions do not use all of the fields. Unused fields shall
> be cleared to zero.
> > But then goes on to say:
> >> As discussed below in 64-bit immediate instructions (Section 4.4), a
> >> 64-bit immediate instruction uses a 64-bit immediate value that is
> constructed as follows.
> > [...]
> >> imm64 = (next_imm << 32) | imm
> > Under a normal English reading, that could imply that all 64-bit
> > immediate instructions use imm64, which is not the case.  The whole imm64
> discussion there only applies today to src=0 (though I
> > suppose it could be used by future 64-bit immediate instructions).   Minimally
> I think
> > "a 64-bit immediate instruction uses" should be "some 64-bit immediate
> instructions use"
> > but at present there's only one.
> >
> > It would actually be simpler to remove the imm64 text and just have
> > the definition of src 0x0 change from: "dst = imm64" to "dst = (next_imm <<
> 32) | imm".
> >
> > What do you think?
> 
> it does sound better. Something like below?
> 
> diff --git a/Documentation/bpf/standardization/instruction-set.rst
> b/Documentation/bpf/standardization/instruction-set.rst
> index af43227b6ee4..fceacca46299 100644
> --- a/Documentation/bpf/standardization/instruction-set.rst
> +++ b/Documentation/bpf/standardization/instruction-set.rst
> @@ -166,7 +166,7 @@ Note that most instructions do not use all of the fields.
>   Unused fields shall be cleared to zero.
> 
>   As discussed below in `64-bit immediate instructions`_, a 64-bit immediate -
> instruction uses a 64-bit immediate value that is constructed as follows.
> +instruction uses two 32-bit immediate values that are constructed as follows.
>   The 64 bits following the basic instruction contain a pseudo instruction
>   using the same format but with opcode, dst_reg, src_reg, and offset all set to
> zero,
>   and imm containing the high 32 bits of the immediate value.
> @@ -181,13 +181,8 @@ This is depicted in the following figure::
>                                      '--------------'
>                                     pseudo instruction
> 
> -Thus the 64-bit immediate value is constructed as follows:
> -
> -  imm64 = (next_imm << 32) | imm
> -
> -where 'next_imm' refers to the imm value of the pseudo instruction -following
> the basic instruction.  The unused bytes in the pseudo -instruction are reserved
> and shall be cleared to zero.
> +Here, the imm value of the pseudo instruction is called 'next_imm'. The
> +unused bytes in the pseudo instruction are reserved and shall be cleared to
> zero.
> 
>   Instruction classes
>   -------------------
> @@ -590,7 +585,7 @@ defined further 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    0x0  dst = (next_imm << 32) | imm
> 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 = map_val(map_by_fd(imm)) +
> next_imm   map fd       data pointer
>   BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_addr(imm)
> variable id  data pointer

Acked-by: Dave Thaler <dthaler1968@gmail.com>



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Bpf] 64-bit immediate instructions clarification
@ 2024-01-27  6:56                       ` dthaler1968=40googlemail.com
  0 siblings, 0 replies; 23+ messages in thread
From: dthaler1968=40googlemail.com @ 2024-01-27  6:56 UTC (permalink / raw)
  To: 'Yonghong Song'; +Cc: bpf, bpf

> -----Original Message-----
> From: Yonghong Song <yonghong.song@linux.dev>
> Sent: Friday, January 26, 2024 7:41 PM
> To: dthaler1968@googlemail.com
> Cc: bpf@ietf.org; bpf@vger.kernel.org
> Subject: Re: 64-bit immediate instructions clarification
> 
> 
> On 1/26/24 2:27 PM, dthaler1968@googlemail.com wrote:
> > Yonghong Song <yonghong.song@linux.dev> wrote:
> >> On 1/25/24 5:12 PM, dthaler1968@googlemail.com wrote:
> >>> The spec defines:
> >>>> As discussed below in `64-bit immediate instructions`_, a 64-bit
> >>>> immediate instruction uses a 64-bit immediate value that is
> >>>> constructed as
> >> follows.
> >>>> The 64 bits following the basic instruction contain a pseudo
> >>>> instruction using the same format but with opcode, dst_reg,
> >>>> src_reg, and offset all set to zero, and imm containing the high 32
> >>>> bits of the
> >> immediate value.
> >>> [...]
> >>>> imm64 = (next_imm << 32) | imm
> >>> The 64-bit immediate instructions section then says:
> >>>> Instructions with the ``BPF_IMM`` 'mode' modifier use the wide
> >>>> instruction encoding defined in `Instruction encoding`_, and use
> >>>> the 'src' field of the basic instruction to hold an opcode subtype.
> >>> Some instructions then nicely state how to use the full 64 bit
> >>> immediate value, such as
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64
> >> integer      integer
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst =
> map_val(map_by_fd(imm))
> >> + next_imm   map fd       data pointer
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst =
> map_val(map_by_idx(imm))
> >> + next_imm  map index    data pointer
> >>> Others don't:
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)
> >> map fd       map
> >>>> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_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
> >>> How is next_imm used in those four?  Must it be 0?  Or can it be
> >>> anything and
> >> it's ignored?
> >>> Or is it used for something?
> >> The other four must have next_imm to be 0. No use of next_imm in thee
> >> four insns kindly implies this.
> >> See uapi bpf.h for details (search BPF_PSEUDO_MAP_FD).
> > Thanks for confirming.  The "Instruction encoding" section has
> > misleading text in my opinion.
> >
> > It nicely says:
> >> Note that most instructions do not use all of the fields. Unused fields shall
> be cleared to zero.
> > But then goes on to say:
> >> As discussed below in 64-bit immediate instructions (Section 4.4), a
> >> 64-bit immediate instruction uses a 64-bit immediate value that is
> constructed as follows.
> > [...]
> >> imm64 = (next_imm << 32) | imm
> > Under a normal English reading, that could imply that all 64-bit
> > immediate instructions use imm64, which is not the case.  The whole imm64
> discussion there only applies today to src=0 (though I
> > suppose it could be used by future 64-bit immediate instructions).   Minimally
> I think
> > "a 64-bit immediate instruction uses" should be "some 64-bit immediate
> instructions use"
> > but at present there's only one.
> >
> > It would actually be simpler to remove the imm64 text and just have
> > the definition of src 0x0 change from: "dst = imm64" to "dst = (next_imm <<
> 32) | imm".
> >
> > What do you think?
> 
> it does sound better. Something like below?
> 
> diff --git a/Documentation/bpf/standardization/instruction-set.rst
> b/Documentation/bpf/standardization/instruction-set.rst
> index af43227b6ee4..fceacca46299 100644
> --- a/Documentation/bpf/standardization/instruction-set.rst
> +++ b/Documentation/bpf/standardization/instruction-set.rst
> @@ -166,7 +166,7 @@ Note that most instructions do not use all of the fields.
>   Unused fields shall be cleared to zero.
> 
>   As discussed below in `64-bit immediate instructions`_, a 64-bit immediate -
> instruction uses a 64-bit immediate value that is constructed as follows.
> +instruction uses two 32-bit immediate values that are constructed as follows.
>   The 64 bits following the basic instruction contain a pseudo instruction
>   using the same format but with opcode, dst_reg, src_reg, and offset all set to
> zero,
>   and imm containing the high 32 bits of the immediate value.
> @@ -181,13 +181,8 @@ This is depicted in the following figure::
>                                      '--------------'
>                                     pseudo instruction
> 
> -Thus the 64-bit immediate value is constructed as follows:
> -
> -  imm64 = (next_imm << 32) | imm
> -
> -where 'next_imm' refers to the imm value of the pseudo instruction -following
> the basic instruction.  The unused bytes in the pseudo -instruction are reserved
> and shall be cleared to zero.
> +Here, the imm value of the pseudo instruction is called 'next_imm'. The
> +unused bytes in the pseudo instruction are reserved and shall be cleared to
> zero.
> 
>   Instruction classes
>   -------------------
> @@ -590,7 +585,7 @@ defined further 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    0x0  dst = (next_imm << 32) | imm
> 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 = map_val(map_by_fd(imm)) +
> next_imm   map fd       data pointer
>   BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_addr(imm)
> variable id  data pointer

Acked-by: Dave Thaler <dthaler1968@gmail.com>


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2024-01-27  6:56 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-16 20:38 [Bpf] Sign extension ISA question dthaler1968=40googlemail.com
2024-01-16 20:55 ` dthaler1968
2024-01-16 20:55   ` [Bpf] " dthaler1968=40googlemail.com
2024-01-16 22:34   ` Yonghong Song
2024-01-16 22:34     ` [Bpf] " Yonghong Song
2024-01-17  1:56     ` dthaler1968
2024-01-17  1:56       ` [Bpf] " dthaler1968=40googlemail.com
2024-01-17  3:48       ` Yonghong Song
2024-01-17  3:48         ` [Bpf] " Yonghong Song
2024-01-24  2:07         ` Jump instructions clarification dthaler1968
2024-01-24  2:07           ` [Bpf] " dthaler1968=40googlemail.com
2024-01-24 19:33           ` Yonghong Song
2024-01-24 19:33             ` [Bpf] " Yonghong Song
2024-01-26  1:12             ` 64-bit immediate " dthaler1968
2024-01-26  1:12               ` [Bpf] " dthaler1968=40googlemail.com
2024-01-26  5:34               ` Yonghong Song
2024-01-26  5:34                 ` [Bpf] " Yonghong Song
2024-01-26 22:27                 ` dthaler1968
2024-01-26 22:27                   ` [Bpf] " dthaler1968=40googlemail.com
2024-01-27  3:41                   ` Yonghong Song
2024-01-27  3:41                     ` [Bpf] " Yonghong Song
2024-01-27  6:56                     ` dthaler1968
2024-01-27  6:56                       ` [Bpf] " dthaler1968=40googlemail.com

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.