dthaler / ebpf-docs

11 stars 0 forks source link

Update instruction set doc #1

Closed dthaler closed 2 years ago

dthaler commented 2 years ago

The dest = imm (0x18) and call (0x85) instructions have a different semantic when their src register is set to a special flag. I think this is also part of the ISA and should be documented? See commits 2 to 7 of this PR (and their description) for a quick reference.

I can't understand the convention, is there somewhere that elaborates? The PR has:

0x18 (src == 0) | lddw dst, imm | dst = imm 0x18 (src == 1) | lddw dst, map | dst = imm with imm == map fd 0x18 (src == 2) | lddw dst, map value | dst = map[0] + insn[1].imm with insn[0] == map fd 0x18 (src == 3) | lddw dst, kernel var | dst = imm with imm == BTF id of var 0x18 (src == 4) | lddw dst, BPF func | dst = imm with imm == insn offset of BPF callback 0x18 (src == 5) | lddw dst, imm | dst = imm with imm == map index 0x18 (src == 6) | lddw dst, map value | dst = map[0] + insn[1].imm with insn[0] == map index

But what does "map[0]" mean? What does "insn[0]" mean, is that relative to the PC or absolute from the start of the program or what? Also the ISA does not currently define the existence / meaning of a "map fd" or a "BTF id of var" or a "map index" or a "BPF callback". I'm concerned about adding these to the ISA without definitions.

qmonnet commented 2 years ago

The dest = imm (0x18) and call (0x85) instructions have a different semantic when their src register is set to a special flag. I think this is also part of the ISA and should be documented? See commits 2 to 7 of this PR (and their description) for a quick reference.

I can't understand the convention, is there somewhere that elaborates?

To some extent, the commit descriptions in my PR. Otherwise I don't think these are documented, other than in kernel commit descriptions (also linked from the PR, although this one was missing, I updated the PR) and briefly in the UAPI header.

The PR has:

0x18 (src == 0) | lddw dst, imm | dst = imm 0x18 (src == 1) | lddw dst, map | dst = imm with imm == map fd 0x18 (src == 2) | lddw dst, map value | dst = map[0] + insn[1].imm with insn[0] == map fd 0x18 (src == 3) | lddw dst, kernel var | dst = imm with imm == BTF id of var 0x18 (src == 4) | lddw dst, BPF func | dst = imm with imm == insn offset of BPF callback 0x18 (src == 5) | lddw dst, imm | dst = imm with imm == map index 0x18 (src == 6) | lddw dst, map value | dst = map[0] + insn[1].imm with insn[0] == map index

But what does "map[0]" mean? What does "insn[0]" mean, is that relative to the PC or absolute from the start of the program or what?

Sorry if the above is unclear, I wanted it to fit in the array and that's limiting. I reused the convention from the UAPI header. So map[0] means a map value; for now it's only the one at index 0, because this syntax is currently only supported for 1-entry arrays. Regarding insn[0].imm and insn[1].imm, they refer to the immediate fields of the first and second 64-bit halves of the 128-bit lddw instruction. And insn[0].imm is the offset into that value (and must be lower than the size of a value), indicating at which address we want to read. So we would have something along:

code dst src off imm
0x18 any 2   0   <map fd>
0x00 0   0   0   <offset into map value>

And this would load the address residing in the first entry of the map, at the provided offset, into the destination register.

The commit log mentions that the offset fields might be used as indices in the future to support maps with multiple entries, but this hasn't been implemented so far by lack of a use case.

Also the ISA does not currently define the existence / meaning of a "map fd" or a "BTF id of var" or a "map index" or a "BPF callback". I'm concerned about adding these to the ISA without definitions.

I understand, the concern sounds legitimate. I don't know if they should be part of the ISA or not (or just a Linux extension), this should maybe be debated with other folks. But I see that this is what Linux currently does and that this conflicts with the generic constraint we have on fields not covered in the spec:

Note that most instructions do not use all of the fields.
Unused fields must be set to zero.

So we probably want to address this one way or another.