Re: [RFC PATCH v2 0/7] consolidate PowerPC instruction encoding macros

["Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>]

Balamuruhan S wrote:
> ppc-opcode.h have base instruction encoding wrapped with stringify_in_c()
> for raw encoding to have compatibility. But there are redundant macros for
> base instruction encodings in bpf, instruction emulation test infrastructure
> and powerpc selftests.
> 
> Currently PPC_INST_* macros are used for encoding instruction opcode and PPC_*
> for raw instuction encoding, this rfc patchset introduces PPC_RAW_* macros for
> base instruction encoding and reuse it from elsewhere. With this change we can
> avoid redundant macro definitions in multiple files and start adding new
> instructions in ppc-opcode.h in future.
> 
> Changes in v2:
> -------------
> Fix review comments/suggestions from Naveen and Michael Ellerman,
> 
> * Rename PPC_ENCODE_* to PPC_RAW_* for base instruction encoding macros.
> * Split the patches that does mass renaming and make them simpler that just
>   adds new macros.
> * Keep the patch to update all the existing names later (patch 6).
> * Lot of PPC_INST_* macros are used only in ppc-opcode.h for PPC_*  macros,
>   fold PPC_INST_* encoding into PPC_RAW_* to avoid using them accidentally.
> * Fixed clipped macros that was due to a typo/copy-paste
> * Consolidated all the instruction encoding macros from bpf_jit.h to
>   ppc-opcode.h
> * squashed patch that removes the duplicate macro PPC_MR() in bpf_jit.h
> * merge few changes in bpf_jit files from patch 2 into patch 3
> * few fixes in powerpc selftest stringloops Makefile
> * build tested for ppc64le_defconfig, ppc64e_defconfig and pmac32_defconfig
> * Rebased on next branch of linuxppc tree
> 
> Testing:
> -------
> * Tested it by compiling vmlinux and comparing objdump of it with and without
>   the patchset and observed that it remains same,
> 
>   # diff vmlinux_objdump vmlinux_rfc_objdump 
>   2c2
>   < vmlinux:     file format elf64-powerpcle
>   ---
>   > vmlinux_rfc:     file format elf64-powerpcle
> 
> * Tested building it with this changes for Fedora30 config, booted VM
>   with powerpc next and powerpc next + patchset to run powerpc selftest and
>   ftrace selftest. There were couple of failures that were common and
>   patchset did not introduce any new failures.
> 
>   ftrace selftest:
>   ---------------
> 	# # of passed:  96
> 	# # of failed:  1
> 	# # of unresolved:  7
> 	# # of untested:  0
> 	# # of unsupported:  1
> 	# # of xfailed:  1
> 	# # of undefined(test bug):  0
> 	not ok 1 selftests: ftrace: ftracetest # exit=1
> 
>   powerpc selftest:
>   ----------------
>         not ok 7 selftests: powerpc/dscr: dscr_sysfs_thread_test # exit=1
>         not ok 20 selftests: powerpc/pmu/ebb: lost_exception_test # TIMEOUT
>         not ok 2 selftests: powerpc/security: spectre_v2 # exit=1
> 
> Thanks to Naveen, Sandipan and Michael on overall suggestions/improvements.
> 
> I would request for review and suggestions to make it better.
> 
> v1: https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-March/206494.html
> 
> Balamuruhan S (7):
>   powerpc/ppc-opcode: introduce PPC_RAW_* macros for base instruction
>     encoding
>   powerpc/ppc-opcode: move ppc instruction encoding from
>     test_emulate_step
>   powerpc/bpf_jit: reuse instruction macros from ppc-opcode.h
>   powerpc/ppc-opcode: consolidate powerpc instructions from bpf_jit.h
>   powerpc/ppc-opcode: reuse raw instruction macros to stringify
>   powerpc/ppc-opcode: fold PPC_INST_* macros into PPC_RAW_* macros
>   powerpc/selftest: reuse ppc-opcode macros to avoid redundancy
> 
>  arch/powerpc/include/asm/ppc-opcode.h         | 706 +++++++++++-------
>  arch/powerpc/lib/test_emulate_step.c          | 155 ++--
>  arch/powerpc/net/bpf_jit.h                    | 184 +----
>  arch/powerpc/net/bpf_jit32.h                  |  34 +-
>  arch/powerpc/net/bpf_jit64.h                  |  16 +-
>  arch/powerpc/net/bpf_jit_comp.c               | 134 ++--
>  arch/powerpc/net/bpf_jit_comp64.c             | 298 ++++----
>  .../selftests/powerpc/stringloops/Makefile    |  34 +-
>  .../powerpc/stringloops/asm/asm-const.h       |   1 +
>  .../powerpc/stringloops/asm/ppc-opcode.h      |  36 +-
>  10 files changed, 762 insertions(+), 836 deletions(-)
>  create mode 120000 tools/testing/selftests/powerpc/stringloops/asm/asm-const.h
>  mode change 100644 => 120000 tools/testing/selftests/powerpc/stringloops/asm/ppc-opcode.h

This LGTM. Except the last patch:
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Tested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>


Thanks!
- Naveen