On Wed, Feb 08, 2023 at 09:31:18AM -0800, Alexei Starovoitov wrote: > On Wed, Feb 8, 2023 at 9:26 AM Dave Thaler > <dthaler=40microsoft.com@xxxxxxxxxxxxxx> wrote: > > > > David Vernet wrote: > > > > +Reserved instructions > > > > +==================== > > > > > > small nit: Missing a = > > > > Ack. > > > > > > +Clang will generate the reserved ``BPF_CALL | BPF_X | BPF_JMP`` (0x8d) > > > instruction if ``-O0`` is used. > > > > > > Are we calling this out here to say that BPF_CALL in clang -O0 builds is not > > > supported? That would seem to be the case given that we say that BPF_CALL > > > | BPF_X | BPF_JMP in reserved and not permitted in instruction-set.rst. > > > > Yes, exactly. I could update the language to add something like > > "... so BPF_CALL in clang -O0 builds is not supported". > > That will not be a correct statement. > BPF_CALL is a valid insn regardless of optimization flags. > BPF_CALLX will be a valid insn when the verifier support is added. > Compilers need to make a choice which insn to use on a case by case basis. > When compilers have no choice, but to use call by register they will > use callx. That what happens with = (void *)1 hack that we use for > helpers. > It can happen with -O2 just as well. In that case, I suggest we update the verbiage in instruction-set.rst to say: Note that ``BPF_CALL | BPF_X | BPF_JMP`` (0x8d), where the helper function integer would be read from a specified register, is not currently supported by the verifier. Any programs with this instruction will fail to load until such support is added. And then we can update this section to say something similar, or just remove it altogether per Alexei's point that it's an implementation detail of the compiler which could change at any time.
