Re: [PATCH v5 00/21] Initial Prefixed Instruction support

[Jordan Niethe <jniethe5@gmail.com>]

On Thu, Apr 9, 2020 at 4:39 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>
>
> On 04/06/2020 08:09 AM, Jordan Niethe wrote:
> > A future revision of the ISA will introduce prefixed instructions. A
> > prefixed instruction is composed of a 4-byte prefix followed by a
> > 4-byte suffix.
> >
> > All prefixes have the major opcode 1. A prefix will never be a valid
> > word instruction. A suffix may be an existing word instruction or a
> > new instruction.
> >
> > This series enables prefixed instructions and extends the instruction
> > emulation to support them. Then the places where prefixed instructions
> > might need to be emulated are updated.
> >
> > v5 is based on feedback from Nick Piggins, Michael Ellerman, Balamuruhan
> > Suriyakumar and Alistair Popple.
> > The major changes:
> >      - The ppc instruction type is now a struct
> >      - Series now just based on next
> >      - ppc_inst_masked() dropped
> >      - Space for xmon breakpoints allocated in an assembly file
> >      - "Add prefixed instructions to instruction data type" patch seperated in
> >        to smaller patches
> >      - Calling convention for create_branch() is changed
> >      - Some places which had not been updated to use the data type are now updated
>
> Build fails. I have not investigated why:
Thanks, I will check it out.
>
>    CC      arch/powerpc/kernel/process.o
> In file included from ./arch/powerpc/include/asm/code-patching.h:14:0,
>                   from arch/powerpc/kernel/process.c:60:
> ./arch/powerpc/include/asm/inst.h:69:38: error: unknown type name ‘ppc_inst’
>   static inline bool ppc_inst_prefixed(ppc_inst x)
>                                        ^
> ./arch/powerpc/include/asm/inst.h:79:19: error: redefinition of
> ‘ppc_inst_val’
>   static inline u32 ppc_inst_val(struct ppc_inst x)
>                     ^
> ./arch/powerpc/include/asm/inst.h:21:19: note: previous definition of
> ‘ppc_inst_val’ was here
>   static inline u32 ppc_inst_val(struct ppc_inst x)
>                     ^
> ./arch/powerpc/include/asm/inst.h: In function ‘ppc_inst_len’:
> ./arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of
> function ‘ppc_inst_prefixed’ [-Werror=implicit-function-declaration]
>    return (ppc_inst_prefixed(x)) ? 8  : 4;
>            ^
>
> Christophe
>
> >
> > v4 is based on feedback from Nick Piggins, Christophe Leroy and Daniel Axtens.
> > The major changes:
> >      - Move xmon breakpoints from data section to text section
> >      - Introduce a data type for instructions on powerpc
> >
> > v3 is based on feedback from Christophe Leroy. The major changes:
> >      - Completely replacing store_inst() with patch_instruction() in
> >        xmon
> >      - Improve implementation of mread_instr() to not use mread().
> >      - Base the series on top of
> >        https://patchwork.ozlabs.org/patch/1232619/ as this will effect
> >        kprobes.
> >      - Some renaming and simplification of conditionals.
> >
> > v2 incorporates feedback from Daniel Axtens and and Balamuruhan
> > S. The major changes are:
> >      - Squashing together all commits about SRR1 bits
> >      - Squashing all commits for supporting prefixed load stores
> >      - Changing abbreviated references to sufx/prfx -> suffix/prefix
> >      - Introducing macros for returning the length of an instruction
> >      - Removing sign extension flag from pstd/pld in sstep.c
> >      - Dropping patch  "powerpc/fault: Use analyse_instr() to check for
> >        store with updates to sp" from the series, it did not really fit
> >        with prefixed enablement in the first place and as reported by Greg
> >        Kurz did not work correctly.
> >
> >
> > Alistair Popple (1):
> >    powerpc: Enable Prefixed Instructions
> >
> > Jordan Niethe (20):
> >    powerpc/xmon: Remove store_inst() for patch_instruction()
> >    powerpc/xmon: Move out-of-line instructions to text section
> >    powerpc: Change calling convention for create_branch() et. al.
> >    powerpc: Use a macro for creating instructions from u32s
> >    powerpc: Use a function for getting the instruction op code
> >    powerpc: Use an accessor for instructions
> >    powerpc: Use a function for byte swapping instructions
> >    powerpc: Introduce functions for instruction equality
> >    powerpc: Use a datatype for instructions
> >    powerpc: Use a function for reading instructions
> >    powerpc: Define and use __get_user_instr{,inatomic}()
> >    powerpc: Introduce a function for reporting instruction length
> >    powerpc/xmon: Use a function for reading instructions
> >    powerpc/xmon: Move insertion of breakpoint for xol'ing
> >    powerpc: Make test_translate_branch() independent of instruction
> >      length
> >    powerpc: Define new SRR1 bits for a future ISA version
> >    powerpc64: Add prefixed instructions to instruction data type
> >    powerpc: Support prefixed instructions in alignment handler
> >    powerpc sstep: Add support for prefixed load/stores
> >    powerpc sstep: Add support for prefixed fixed-point arithmetic
> >
> >   arch/powerpc/include/asm/code-patching.h |  37 +-
> >   arch/powerpc/include/asm/inst.h          | 106 ++++++
> >   arch/powerpc/include/asm/kprobes.h       |   2 +-
> >   arch/powerpc/include/asm/reg.h           |   7 +-
> >   arch/powerpc/include/asm/sstep.h         |  15 +-
> >   arch/powerpc/include/asm/uaccess.h       |  28 ++
> >   arch/powerpc/include/asm/uprobes.h       |   7 +-
> >   arch/powerpc/kernel/align.c              |  13 +-
> >   arch/powerpc/kernel/epapr_paravirt.c     |   5 +-
> >   arch/powerpc/kernel/hw_breakpoint.c      |   5 +-
> >   arch/powerpc/kernel/jump_label.c         |   5 +-
> >   arch/powerpc/kernel/kgdb.c               |   9 +-
> >   arch/powerpc/kernel/kprobes.c            |  24 +-
> >   arch/powerpc/kernel/mce_power.c          |   5 +-
> >   arch/powerpc/kernel/module_64.c          |   3 +-
> >   arch/powerpc/kernel/optprobes.c          |  91 +++--
> >   arch/powerpc/kernel/optprobes_head.S     |   3 +
> >   arch/powerpc/kernel/security.c           |   9 +-
> >   arch/powerpc/kernel/setup_32.c           |   4 +-
> >   arch/powerpc/kernel/trace/ftrace.c       | 190 ++++++----
> >   arch/powerpc/kernel/traps.c              |  20 +-
> >   arch/powerpc/kernel/uprobes.c            |   3 +-
> >   arch/powerpc/kernel/vecemu.c             |  20 +-
> >   arch/powerpc/kvm/book3s_hv_nested.c      |   2 +-
> >   arch/powerpc/kvm/book3s_hv_rm_mmu.c      |   2 +-
> >   arch/powerpc/kvm/emulate_loadstore.c     |   2 +-
> >   arch/powerpc/lib/code-patching.c         | 289 +++++++-------
> >   arch/powerpc/lib/feature-fixups.c        |  69 ++--
> >   arch/powerpc/lib/sstep.c                 | 455 ++++++++++++++++-------
> >   arch/powerpc/lib/test_emulate_step.c     |  56 +--
> >   arch/powerpc/perf/core-book3s.c          |   4 +-
> >   arch/powerpc/xmon/Makefile               |   2 +-
> >   arch/powerpc/xmon/xmon.c                 |  94 +++--
> >   arch/powerpc/xmon/xmon_bpts.S            |  10 +
> >   arch/powerpc/xmon/xmon_bpts.h            |   8 +
> >   35 files changed, 1042 insertions(+), 562 deletions(-)
> >   create mode 100644 arch/powerpc/include/asm/inst.h
> >   create mode 100644 arch/powerpc/xmon/xmon_bpts.S
> >   create mode 100644 arch/powerpc/xmon/xmon_bpts.h
> >