On Fri, Aug 25, 2023 at 3:01 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Sat, 2023-08-26 at 00:10 +0300, Eduard Zingerman wrote:
> [...]
> > >
> > > Instead of referencing BPF_MOV specifically, would it be useful to
> > > incorporate all the different instructions that can be relocated?
> > > bpf_core_patch_insn comment has a nice summary, maybe we can somehow
> > > reuse it in this doc as well?
> > >
> > > * Currently supported classes of BPF instruction are:
> > > * 1. rX = <imm> (assignment with immediate operand);
> > > * 2. rX += <imm> (arithmetic operations with immediate operand);
> > > * 3. rX = <imm64> (load with 64-bit immediate value);
> > > * 4. rX = *(T *)(rY + <off>), where T is one of {u8, u16, u32, u64};
> > > * 5. *(T *)(rX + <off>) = rY, where T is one of {u8, u16, u32, u64};
> > > * 6. *(T *)(rX + <off>) = <imm>, where T is one of {u8, u16, u32, u64}.
> >
> > Good point. I will keep the BPF_MOV as an example for relocation kind
> > groups description and add this comment describing all relocatable
> > instructions.
> >
>
> Actually, this comment does not mention atomic instructions or loads
> with sign extension. bpf_core_patch_insn() operates basing on
> instruction class. I'll describe it as follows:
>
> Field to patch is selected basing on the instruction class:
>
> * For BPF_ALU, BPF_ALU64, BPF_LD immediate field is patched;
> * For BPF_LDX, BPF_STX, BPF_ST offset field is patched;
>
> WDYT?
SGTM. As a human I still like the `rX += <imm>` notation, but for
documentation BPF_ALU would be more precise and appropriate.
>
> [...]
