| Message ID | 20200320051809.24332-1-jniethe5@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Initial Prefixed Instruction support | expand |
Jordan Niethe's on March 20, 2020 3:17 pm: > 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. > > The series is based on top of: > https://patchwork.ozlabs.org/patch/1232619/ as this will effect > kprobes. > > 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 Thanks for doing this, looks like a lot of work, I hope it works out :) Thanks, Nick
On Mon, Mar 23, 2020 at 5:22 PM Nicholas Piggin <npiggin@gmail.com> wrote: > > Jordan Niethe's on March 20, 2020 3:17 pm: > > 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. > > > > The series is based on top of: > > https://patchwork.ozlabs.org/patch/1232619/ as this will effect > > kprobes. > > > > 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 > > Thanks for doing this, looks like a lot of work, I hope it works out :) > Yes it did end up touching a lot of places. I started thinking that that maybe it would be simpler to just use a u64 instead of the struct for instructions. If we always keep the word instruction / prefix in the lower bytes, all of the current masking should still work and we can use operators again instead of ppc_inst_equal(), etc. It also makes printing easier. We could just #define INST_FMT %llx or #define INST_FMT %x on powerpc32 and use that for printing out instructions. > Thanks, > Nick
Jordan Niethe's on March 23, 2020 7:25 pm: > On Mon, Mar 23, 2020 at 5:22 PM Nicholas Piggin <npiggin@gmail.com> wrote: >> >> Jordan Niethe's on March 20, 2020 3:17 pm: >> > 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. >> > >> > The series is based on top of: >> > https://patchwork.ozlabs.org/patch/1232619/ as this will effect >> > kprobes. >> > >> > 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 >> >> Thanks for doing this, looks like a lot of work, I hope it works out :) >> > Yes it did end up touching a lot of places. I started thinking that > that maybe it would be simpler to just use a u64 instead of the struct > for instructions. > If we always keep the word instruction / prefix in the lower bytes, > all of the current masking should still work and we can use operators > again instead of ppc_inst_equal(), etc. Yeah.. I think now that you've done it, I prefer it this way. > It also makes printing easier. We could just #define INST_FMT %llx or > #define INST_FMT %x on powerpc32 and use that for printing out > instructions. Well, not sure about that. Would it make endian concerns more complicated? Print format for prefix might be '%016llx', but we don't want that for all instructions only prefixed ones, and I don't know if that is the way to go either. We'll want to adopt some convention for displaying prefixed instruction bytes, but I don't know what what works best. I wonder if binutils or any userspace tools have a convention. Which reminds me, you might have missed show_instructions()? Although maybe you don't need that until we start using them in the kernel. Thanks, Nick
On Mon, Mar 23, 2020 at 9:21 PM Nicholas Piggin <npiggin@gmail.com> wrote: > > Jordan Niethe's on March 23, 2020 7:25 pm: > > On Mon, Mar 23, 2020 at 5:22 PM Nicholas Piggin <npiggin@gmail.com> wrote: > >> > >> Jordan Niethe's on March 20, 2020 3:17 pm: > >> > 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. > >> > > >> > The series is based on top of: > >> > https://patchwork.ozlabs.org/patch/1232619/ as this will effect > >> > kprobes. > >> > > >> > 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 > >> > >> Thanks for doing this, looks like a lot of work, I hope it works out :) > >> > > Yes it did end up touching a lot of places. I started thinking that > > that maybe it would be simpler to just use a u64 instead of the struct > > for instructions. > > If we always keep the word instruction / prefix in the lower bytes, > > all of the current masking should still work and we can use operators > > again instead of ppc_inst_equal(), etc. > > Yeah.. I think now that you've done it, I prefer it this way. Sorry, just to be clear which way do you mean? > > > It also makes printing easier. We could just #define INST_FMT %llx or > > #define INST_FMT %x on powerpc32 and use that for printing out > > instructions. > > Well, not sure about that. Would it make endian concerns more > complicated? Print format for prefix might be '%016llx', but we > don't want that for all instructions only prefixed ones, and I > don't know if that is the way to go either. Hm yeah that is true. > > We'll want to adopt some convention for displaying prefixed > instruction bytes, but I don't know what what works best. I wonder > if binutils or any userspace tools have a convention. binutils-gdb upstream has supports disassembling prefixed instructions. Here is what objdump looks like: 44: 00 00 00 60 nop 48: 00 00 00 07 pnop 4c: 00 00 00 00 50: 01 00 20 39 li r9,1 54: 00 00 00 06 paddi r4,r9,3 58: 03 00 89 38 5c: 00 00 62 3c addis r3,r2,0 > > Which reminds me, you might have missed show_instructions()? > Although maybe you don't need that until we start using them in > the kernel. You are right I missed that here. > > Thanks, > Nick
On Tue, Mar 24, 2020 at 1:54 PM Jordan Niethe <jniethe5@gmail.com> wrote: > > On Mon, Mar 23, 2020 at 9:21 PM Nicholas Piggin <npiggin@gmail.com> wrote: > > > > Jordan Niethe's on March 23, 2020 7:25 pm: > > > On Mon, Mar 23, 2020 at 5:22 PM Nicholas Piggin <npiggin@gmail.com> wrote: > > >> > > >> Jordan Niethe's on March 20, 2020 3:17 pm: > > >> > 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. > > >> > > > >> > The series is based on top of: > > >> > https://patchwork.ozlabs.org/patch/1232619/ as this will effect > > >> > kprobes. > > >> > > > >> > 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 > > >> > > >> Thanks for doing this, looks like a lot of work, I hope it works out :) > > >> > > > Yes it did end up touching a lot of places. I started thinking that > > > that maybe it would be simpler to just use a u64 instead of the struct > > > for instructions. > > > If we always keep the word instruction / prefix in the lower bytes, > > > all of the current masking should still work and we can use operators > > > again instead of ppc_inst_equal(), etc. > > > > Yeah.. I think now that you've done it, I prefer it this way. > Sorry, just to be clear which way do you mean? > > > > > It also makes printing easier. We could just #define INST_FMT %llx or > > > #define INST_FMT %x on powerpc32 and use that for printing out > > > instructions. > > > > Well, not sure about that. Would it make endian concerns more > > complicated? Print format for prefix might be '%016llx', but we > > don't want that for all instructions only prefixed ones, and I > > don't know if that is the way to go either. > Hm yeah that is true. > > > > We'll want to adopt some convention for displaying prefixed > > instruction bytes, but I don't know what what works best. I wonder > > if binutils or any userspace tools have a convention. > binutils-gdb upstream has supports disassembling prefixed instructions. > Here is what objdump looks like: > 44: 00 00 00 60 nop > 48: 00 00 00 07 pnop > 4c: 00 00 00 00 > 50: 01 00 20 39 li r9,1 > 54: 00 00 00 06 paddi r4,r9,3 > 58: 03 00 89 38 > 5c: 00 00 62 3c addis r3,r2,0 And this is what it looks like if you use objdump with -w 44: 00 00 00 60 nop 48: 00 00 00 07 00 00 00 00 pnop 50: 01 00 20 39 li r9,1 54: 00 00 00 06 03 00 89 38 paddi r4,r9,3 5c: 00 00 62 3c addis r3,r2,0 5c: R_PPC64_TOC16_HA .toc+0x10 > > > > Which reminds me, you might have missed show_instructions()? > > Although maybe you don't need that until we start using them in > > the kernel. > You are right I missed that here. > > > > Thanks, > > Nick
Jordan Niethe's on March 24, 2020 12:54 pm: > On Mon, Mar 23, 2020 at 9:21 PM Nicholas Piggin <npiggin@gmail.com> wrote: >> >> Jordan Niethe's on March 23, 2020 7:25 pm: >> > On Mon, Mar 23, 2020 at 5:22 PM Nicholas Piggin <npiggin@gmail.com> wrote: >> >> >> >> Jordan Niethe's on March 20, 2020 3:17 pm: >> >> > 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. >> >> > >> >> > The series is based on top of: >> >> > https://patchwork.ozlabs.org/patch/1232619/ as this will effect >> >> > kprobes. >> >> > >> >> > 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 >> >> >> >> Thanks for doing this, looks like a lot of work, I hope it works out :) >> >> >> > Yes it did end up touching a lot of places. I started thinking that >> > that maybe it would be simpler to just use a u64 instead of the struct >> > for instructions. >> > If we always keep the word instruction / prefix in the lower bytes, >> > all of the current masking should still work and we can use operators >> > again instead of ppc_inst_equal(), etc. >> >> Yeah.. I think now that you've done it, I prefer it this way. > Sorry, just to be clear which way do you mean? With the struct, not u64 scalar. mpe's preferred way is fine by me. >> We'll want to adopt some convention for displaying prefixed >> instruction bytes, but I don't know what what works best. I wonder >> if binutils or any userspace tools have a convention. > binutils-gdb upstream has supports disassembling prefixed instructions. > Here is what objdump looks like: > 44: 00 00 00 60 nop > 48: 00 00 00 07 pnop > 4c: 00 00 00 00 > 50: 01 00 20 39 li r9,1 > 54: 00 00 00 06 paddi r4,r9,3 > 58: 03 00 89 38 > 5c: 00 00 62 3c addis r3,r2,0 >> >> Which reminds me, you might have missed show_instructions()? >> Although maybe you don't need that until we start using them in >> the kernel. > You are right I missed that here. So binutils doesn't do anything special, I guess you can make something up. Thanks, Nick
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. The series is based on top of: https://patchwork.ozlabs.org/patch/1232619/ as this will effect kprobes. 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 (15): powerpc/xmon: Remove store_inst() for patch_instruction() xmon: Move out-of-line instructions to text section powerpc: Use a datatype for instructions powerpc: Use a macro for creating instructions from u32s powerpc: Use a function for masking instructions powerpc: Use a function for getting the instruction op code powerpc: Introduce functions for instruction nullity and equality powerpc: Use an accessor for word instructions powerpc: Use a function for reading instructions powerpc: Make test_translate_branch() independent of instruction length powerpc: Define new SRR1 bits for a future ISA version powerpc: Support prefixed instructions in alignment handler powerpc64: Add prefixed instructions to instruction data type powerpc sstep: Add support for prefixed load/stores powerpc sstep: Add support for prefixed fixed-point arithmetic arch/powerpc/include/asm/code-patching.h | 33 +- arch/powerpc/include/asm/inst.h | 143 +++++++ 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 | 22 ++ arch/powerpc/include/asm/uprobes.h | 6 +- arch/powerpc/kernel/align.c | 13 +- arch/powerpc/kernel/hw_breakpoint.c | 5 +- arch/powerpc/kernel/jump_label.c | 2 +- arch/powerpc/kernel/kprobes.c | 19 +- arch/powerpc/kernel/mce_power.c | 5 +- arch/powerpc/kernel/module_64.c | 2 +- arch/powerpc/kernel/optprobes.c | 68 ++-- arch/powerpc/kernel/optprobes_head.S | 3 + arch/powerpc/kernel/security.c | 8 +- arch/powerpc/kernel/trace/ftrace.c | 179 +++++---- arch/powerpc/kernel/traps.c | 22 +- arch/powerpc/kernel/uprobes.c | 4 +- arch/powerpc/kernel/vmlinux.lds.S | 2 +- arch/powerpc/kvm/book3s_hv_nested.c | 2 +- arch/powerpc/kvm/book3s_hv_rm_mmu.c | 2 +- arch/powerpc/kvm/emulate_loadstore.c | 3 +- arch/powerpc/lib/code-patching.c | 184 ++++----- arch/powerpc/lib/feature-fixups.c | 47 +-- arch/powerpc/lib/sstep.c | 455 ++++++++++++++++------- arch/powerpc/lib/test_emulate_step.c | 56 +-- arch/powerpc/xmon/xmon.c | 93 +++-- 28 files changed, 925 insertions(+), 477 deletions(-) create mode 100644 arch/powerpc/include/asm/inst.h