* [PATCH] bpf: use the expected instruction for NOPs
@ 2020-09-14 19:48 David Faust
2020-09-15 9:37 ` Nick Clifton
0 siblings, 1 reply; 3+ messages in thread
From: David Faust @ 2020-09-14 19:48 UTC (permalink / raw)
To: binutils
The BPF ISA does not have a no-op instruction. There are different ways
to achieve no-op, but in practice the Linux kernel verifier expects a
particular form, namely "ja 0". Update the define md_single_noop_insn
for BPF to reflect this.
For more detail, see the corresponding GCC back end patch:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553893.html
gas/
* config/tc-bpf.h (md_single_noop_insn): Use 'ja 0' for no-op.
---
gas/config/tc-bpf.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gas/config/tc-bpf.h b/gas/config/tc-bpf.h
index cb02d6c1338..e7e505cfaa5 100644
--- a/gas/config/tc-bpf.h
+++ b/gas/config/tc-bpf.h
@@ -48,4 +48,6 @@
/* Values passed to md_apply_fix don't include the symbol value. */
#define MD_APPLY_SYM_VALUE(FIX) 0
-#define md_single_noop_insn "mov %r1,%r1"
+/* The Linux kernel verifier expects NOPs to be encoded in this way;
+ a jump to offset 0 means jump to the next instruction. */
+#define md_single_noop_insn "ja 0"
--
2.26.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bpf: use the expected instruction for NOPs
2020-09-14 19:48 [PATCH] bpf: use the expected instruction for NOPs David Faust
@ 2020-09-15 9:37 ` Nick Clifton
2020-09-15 16:18 ` David Faust
0 siblings, 1 reply; 3+ messages in thread
From: Nick Clifton @ 2020-09-15 9:37 UTC (permalink / raw)
To: David Faust, binutils
Hi David,
> The BPF ISA does not have a no-op instruction.
Just an aside - wouldn't it be a good idea if the ISA did define an explicit
encoding for no-ops ?
> There are different ways
> to achieve no-op, but in practice the Linux kernel verifier expects a
> particular form, namely "ja 0". Update the define md_single_noop_insn
> for BPF to reflect this.
Thanks for pointing this out. I took the original encoding from the
gas/testsuite/gas/all/org-1.s file, in the hopes that it would be correct
one.
> gas/
> * config/tc-bpf.h (md_single_noop_insn): Use 'ja 0' for no-op.
Approved and applied.
Cheers
Nick
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bpf: use the expected instruction for NOPs
2020-09-15 9:37 ` Nick Clifton
@ 2020-09-15 16:18 ` David Faust
0 siblings, 0 replies; 3+ messages in thread
From: David Faust @ 2020-09-15 16:18 UTC (permalink / raw)
To: Nick Clifton, binutils
Hi Nick!
>> The BPF ISA does not have a no-op instruction.
>
> Just an aside - wouldn't it be a good idea if the ISA did define an explicit
> encoding for no-ops ?
In my opinion, yes that would be good. But, eBPF is effectively defined
by its implementation in the kernel, so we have to support whatever the
kernel decides for the ISA.
>
>> There are different ways
>> to achieve no-op, but in practice the Linux kernel verifier expects a
>> particular form, namely "ja 0". Update the define md_single_noop_insn
>> for BPF to reflect this.
>
> Thanks for pointing this out. I took the original encoding from the
> gas/testsuite/gas/all/org-1.s file, in the hopes that it would be correct
> one.
>
I wasn't actually aware of this until yesterday. It was your patch
adding the .nop directive that prompted digging into precisely what the
kernel expects for no-op; thank you.
(The kernel eBPF verifier has an optimization built into it to remove
no-ops, and it defines no-op as 'ja 0'.)
>> gas/
>> * config/tc-bpf.h (md_single_noop_insn): Use 'ja 0' for no-op.
>
> Approved and applied.
>
> Cheers
> Nick
>
Thanks!
David
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-15 16:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 19:48 [PATCH] bpf: use the expected instruction for NOPs David Faust
2020-09-15 9:37 ` Nick Clifton
2020-09-15 16:18 ` David Faust
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).