[PATCH V4 2/3] sim: eBPF simulator
Jose E. Marchesi
jose.marchesi@oracle.com
Sun Jul 12 10:06:38 GMT 2020
Hi Andrew.
> This patch introduces the basics of an instruction-simulator for eBPF.
> The simulator is based on CGEN.
Thanks I took a look through and I'm basically happy. There's a few
small issues that I've highlighted inline below.
Thanks for the review.
>
> sim/ChangeLog:
>
> 2020-07-10 Jose E. Marchesi <jose.marchesi@oracle.com>
> David Faust <david.faust@oracle.com>
>
> * configure.tgt (sim_arch): Add entry for bpf-*-*.
> * configure: Regenerate.
> * MAINTAINERS: Add maintainer for the BPF simulator.
> * bpf/Makefile.in: New file.
> * bpf/bpf-helpers.c: Likewise.
> * bpf/bpf-helpers.def: Likewise.
> * bpf/bpf-helpers.h: Likewise.
> * bpf/bpf-sim.h: Likewise.
> * bpf/bpf.c: Likewise.
> * bpf/config.in: Likewise.
> * bpf/configure.ac: Likewise.
> * bpf/decode.h: Likewise.
> * bpf/eng.h: Likewise.
> * bpf/mloop.in: Likewise.
> * bpf/sim-if.c: Likewise.
> * bpf/sim-main.h: Likewise.
> * bpf/traps.c: Likewise.
> * bpf/configure: Generate.
This file is missing from this message.
Yeah, I removed the thunks containing complete `configure' files to
avoid hitting the mailman zise limit. I can send it in a separated
email if required.
> diff --git a/sim/bpf/Makefile.in b/sim/bpf/Makefile.in
> new file mode 100644
> index 0000000000..b12bb6a7d5
> --- /dev/null
> +++ b/sim/bpf/Makefile.in
> @@ -0,0 +1,205 @@
> +# Makefile template for configure for the eBPF simulator
> +# Copyright (C) 2019 Free Software Foundation, Inc.
This should probably be 2020 here.
Right. Will change.
> +# Dependencies for binaries from CGEN generated source
> +
> +arch.o: arch.c $(SIM_MAIN_DEPS)
> +cpu.o: cpu.c $(BPF_INCLUDE_DEPS)
> +decode-le.o: decode-le.c $(BPF_INCLUDE_DEPS)
> +decode-be.o: decode-be.c $(BPF_INCLUDE_DEPS)
> +# XXX sem-switch.o: sem-switch.c $(BPF_INCLUDE_DEPS)
> +# XXX model.o: model.c $(BPF_INCLUDE_DEPS)
As I mentioned in my review of patch #1, I'm not a fan of these XXX
comments. I'm not going to insist these be removed (where they are
limited to the bpf directories), but I don't think things like the
above add any value, so I'd prefer they be removed, and you shouldn't
assume that these wont be removed in the future.
Actually, these particular comments are obsolete and should be removed.
> diff --git a/sim/bpf/bpf-helpers.def b/sim/bpf/bpf-helpers.def
> new file mode 100644
> index 0000000000..6106ac794a
> --- /dev/null
> +++ b/sim/bpf/bpf-helpers.def
> @@ -0,0 +1,194 @@
> +/* BPF helpers database.
> + Copyright (C) 2019-2020 Free Software Foundation, Inc.
Is this really 2019-2020? If it's copied from some other file that is
2019-2020 then could you mention on the preceding line, just to make
it clear.
Yes, that file (with same name) was contributed as part of the BPF GCC
backend, back in 2019.
More information about the Gdb-patches
mailing list