On 1/30/24 8:39 AM, dthaler1968@xxxxxxxxxxxxxx wrote:
Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote:
[...]
Although the Linux verifier doesn't support them, the fact that gcc
does support them tells me that it's probably safest to list the DW
and LDX variants as deprecated as well, which is what the draft
already did in the appendix so that's good (nothing to change there,
I think).
DW never existed in classic bpf, so abs/ind never had DW flavor.
If some assembler/compiler decided to "support" them it's on them.
The standard must not list such things as deprecated. They never
existed. So nothing is deprecated.
Ack, I will remove the ABS/IND + DW lines from the appendix.
Same with MSH. BPF_LDX | BPF_MSH | BPF_B is the only insn ever existed.
It's a legacy insn. Just like abs/ind.
Should it be listed in the legacy conformance group then?
Currently it's not mentioned in instruction-set.rst at all, so the opcode is
available to use by any new instruction. If we do list it in instruction-set.rst
then, like abs/ind, it will be avoided by anyone proposing new instructions.
Here's my understanding of this thread so far:
* (IND/ABS) | (W/H/B) | LD : these are accepted by the Linux verifier and are supported
by clang and gcc. They should be in the legacy conformance group of deprecated
instructions.
* (IND/ABS) | DW | (LD/LDX) : these are not accepted by the Linux verifier and were
never used. Clang doesn't generate them but gcc did which is now removed
based on this discussion. They should NOT be in the legacy conformance group of
deprecated instructions because they were never defined in the first place, and
instruction-set.rst should be updated to clarify this.
* (IND/ABS) | (W/H/B) | LDX : these are not accepted by the Linux verifier and were
never used. Clang doesn't generate them but gcc does. They should NOT
be in the legacy conformance group of deprecated instructions because they were
never defined in the first place, and instruction-set.rst should be updated to clarify this.
* (IND/ABS) | (W/H/B/DW) | (ST/STX): these are not accepted by the Linux verifier and were
never used. I don't know whether clang or gcc generates them. They should NOT
be in the legacy conformance group of deprecated instructions because they were
never defined in the first place, and instruction-set.rst should be updated to clarify this.
* MSH | B | LDX: this existed in classic BPF but does not exist in (e)BPF since it is not accepted
by the Linux verifier. I don't know whether clang ever generated them, but gcc never did.
clang never generated this insn either.
The "Legacy BPF Packet access instructions" section of instruction-set.rst says
> BPF previously introduced special instructions for access to packet data that were carried
> over from classic BPF. However, these instructions are deprecated and should no longer be used.
I read Alexei's comment "It's a legacy insn. Just like abs/ind" as a possible argument that MSH|B|LDX
should be mentioned in instruction-set.rst, pointing to the above section, like IND/ABS do.
But Yonghong argued that it was never accepted by the verifier, so need not be mentioned.
It is just my opinion. Standardization is complicated. I guess adding it to the legacy insn
is okay to prevent anybody using the same opcode.
* MSH | (W/H/DW) | (LD/ST/STX): These are not accepted by the Linux verifier and were
never used. They should NOT be in the legacy conformance group of deprecated instructions
because they were never defined in the first place.
Let me know if any of the above is incorrect and I can submit a doc patch.
Dave
