* Re: [PATCH v4 03/16] powerpc: Use a datatype for instructions
From: Jordan Niethe @ 2020-04-03 0:09 UTC (permalink / raw)
To: Alistair Popple
Cc: Nicholas Piggin, Balamuruhan S, linuxppc-dev, Daniel Axtens
In-Reply-To: <2945912.kUBOMaOL64@townsend>
On Fri, Apr 3, 2020 at 10:45 AM Alistair Popple <alistair@popple.id.au> wrote:
>
> On Thursday, 2 April 2020 10:52:37 AM AEDT Jordan Niethe wrote:
> > On Wed, Apr 1, 2020 at 9:32 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
> > > On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> > > > Currently unsigned ints are used to represent instructions on powerpc.
> > > > This has worked well as instructions have always been 4 byte words.
> > > > However, a future ISA version will introduce some changes to
> > > > instructions that mean this scheme will no longer work as well. This
> > > > change is Prefixed Instructions. A prefixed instruction is made up of a
> > > > word prefix followed by a word suffix to make an 8 byte double word
> > > > instruction. No matter the endianess of the system the prefix always
> > > > comes first. Prefixed instructions are only planned for powerpc64.
> > > >
> > > > Introduce a ppc_inst type to represent both prefixed and word
> > > > instructions on powerpc64 while keeping it possible to exclusively have
> > > > word instructions on powerpc32, A latter patch will expand the type to
> > > > include prefixed instructions but for now just typedef it to a u32.
> > > >
> > > > Later patches will introduce helper functions and macros for
> > > > manipulating the instructions so that powerpc64 and powerpc32 might
> > > > maintain separate type definitions.
> > > >
> > > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > > > ---
> > > >
> > > > arch/powerpc/include/asm/code-patching.h | 31 +++++------
> > > > arch/powerpc/include/asm/inst.h | 53 +++++++++++++++++++
> > > > arch/powerpc/include/asm/sstep.h | 5 +-
> > > > arch/powerpc/kernel/align.c | 2 +-
> > > > arch/powerpc/kernel/hw_breakpoint.c | 3 +-
> > > > arch/powerpc/kernel/kprobes.c | 2 +-
> > > > arch/powerpc/kernel/mce_power.c | 5 +-
> > > > arch/powerpc/kernel/optprobes.c | 10 ++--
> > > > arch/powerpc/kernel/trace/ftrace.c | 66 ++++++++++++------------
> > > > arch/powerpc/kvm/emulate_loadstore.c | 1 +
> > > > arch/powerpc/lib/code-patching.c | 54 +++++++++----------
> > > > arch/powerpc/lib/sstep.c | 4 +-
> > > > arch/powerpc/lib/test_emulate_step.c | 9 ++--
> > > > arch/powerpc/xmon/xmon.c | 12 ++---
> > > > 14 files changed, 160 insertions(+), 97 deletions(-)
> > > > create mode 100644 arch/powerpc/include/asm/inst.h
> > > >
> > > > diff --git a/arch/powerpc/include/asm/code-patching.h
> > > > b/arch/powerpc/include/asm/code-patching.h
> > > > index 898b54262881..cb5106f92d67 100644
> > > > --- a/arch/powerpc/include/asm/code-patching.h
> > > > +++ b/arch/powerpc/include/asm/code-patching.h
> > > > @@ -11,6 +11,7 @@
> > > >
> > > > #include <linux/string.h>
> > > > #include <linux/kallsyms.h>
> > > > #include <asm/asm-compat.h>
> > > >
> > > > +#include <asm/inst.h>
> > > >
> > > > /* Flags for create_branch:
> > > > * "b" == create_branch(addr, target, 0);
> > > >
> > > > @@ -22,27 +23,27 @@
> > > >
> > > > #define BRANCH_ABSOLUTE 0x2
> > > >
> > > > bool is_offset_in_branch_range(long offset);
> > > >
> > > > -unsigned int create_branch(const unsigned int *addr,
> > > > +ppc_inst create_branch(const ppc_inst *addr,
> > > >
> > > > unsigned long target, int flags);
> > > >
> > > > -unsigned int create_cond_branch(const unsigned int *addr,
> > > > +unsigned int create_cond_branch(const ppc_inst *addr,
> > > >
> > > > unsigned long target, int flags);
> > > >
> > > > -int patch_branch(unsigned int *addr, unsigned long target, int flags);
> > > > -int patch_instruction(unsigned int *addr, unsigned int instr);
> > > > -int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> > > > +int patch_branch(ppc_inst *addr, unsigned long target, int flags);
> > > > +int patch_instruction(ppc_inst *addr, ppc_inst instr);
> > >
> > > we need to handle this change for its user in epapr_paravirt.c,
>
> Seeing a similar issue in kgdb.c:
>
> In file included from /linux/include/linux/kgdb.h:20,
> from /linux/arch/powerpc/kernel/kgdb.c:18:
> /linux/arch/powerpc/kernel/kgdb.c: In function 'kgdb_arch_set_breakpoint':
> /linux/arch/powerpc/include/asm/kgdb.h:30:22: error: incompatible type for argument 2 of 'patch_instruction'
> #define BREAK_INSTR 0x7d821008 /* twge r2, r2 */
> ^~~~~~~~~~
> /linux/arch/powerpc/kernel/kgdb.c:427:32: note: in expansion of macro 'BREAK_INSTR'
> err = patch_instruction(addr, BREAK_INSTR);
> ^~~~~~~~~~~
> In file included from /linux/arch/powerpc/kernel/kgdb.c:27:
> /linux/arch/powerpc/include/asm/code-patching.h:31:44: note: expected 'ppc_inst' {aka 'struct ppc_inst'} but argument is of type 'int'
> int patch_instruction(void *addr, ppc_inst instr);
> ~~~~~~~~~^~~~~
> /linux/arch/powerpc/kernel/kgdb.c: In function 'kgdb_arch_remove_breakpoint':
> /linux/arch/powerpc/kernel/kgdb.c:442:32: error: incompatible type for argument 2 of 'patch_instruction'
> err = patch_instruction(addr, instr);
> ^~~~~
> In file included from /linux/arch/powerpc/kernel/kgdb.c:27:
> /linux/arch/powerpc/include/asm/code-patching.h:31:44: note: expected 'ppc_inst' {aka 'struct ppc_inst'} but argument is of type 'unsigned int'
> int patch_instruction(void *addr, ppc_inst instr);
> ~~~~~~~~~^~~~~
> make[3]: *** [/linux/scripts/Makefile.build:267: arch/powerpc/kernel/kgdb.o] Error 1
>
> - Alistair
Thanks, I will check it out.
>
> > Thanks, good catch.
> >
> > > arch/powerpc/kernel/epapr_paravirt.c: In function
> > > 'early_init_dt_scan_epapr': arch/powerpc/kernel/epapr_paravirt.c:40:48:
> > > error: incompatible type for argument 2 of 'patch_instruction'
> > >
> > > 40 | patch_instruction(epapr_hypercall_start + i, inst);
> > >
> > > | ^~~~
> > > |
> > > | u32 {aka unsigned
> > > | int}
> > >
> > > In file included from arch/powerpc/kernel/epapr_paravirt.c:12:
> > > ./arch/powerpc/include/asm/code-patching.h:31:44: note: expected
> > > 'ppc_inst'
> > > {aka 'struct ppc_inst'} but argument is of type 'u32' {aka 'unsigned int'}
> > >
> > > 31 | int patch_instruction(void *addr, ppc_inst instr);
> > >
> > > | ~~~~~~~~~^~~~~
> > >
> > > make[2]: *** [scripts/Makefile.build:268:
> > > arch/powerpc/kernel/epapr_paravirt.o] Error 1
> > > make[1]: *** [scripts/Makefile.build:505: arch/powerpc/kernel] Error 2
> > > make: *** [Makefile:1683: arch/powerpc] Error 2
> > >
> > >
> > > -- Bala
> > >
> > > > +int raw_patch_instruction(ppc_inst *addr, ppc_inst instr);
> > > >
> > > > static inline unsigned long patch_site_addr(s32 *site)
> > > > {
> > > >
> > > > return (unsigned long)site + *site;
> > > >
> > > > }
> > > >
> > > > -static inline int patch_instruction_site(s32 *site, unsigned int instr)
> > > > +static inline int patch_instruction_site(s32 *site, ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > - return patch_instruction((unsigned int *)patch_site_addr(site),
> > > > instr); + return patch_instruction((ppc_inst
> > > > *)patch_site_addr(site), instr);> >
> > > > }
> > > >
> > > > static inline int patch_branch_site(s32 *site, unsigned long target,
> > > > int
> > > >
> > > > flags)
> > > >
> > > > {
> > > >
> > > > - return patch_branch((unsigned int *)patch_site_addr(site), target,
> > > > flags);
> > > > + return patch_branch((ppc_inst *)patch_site_addr(site), target,
> > > > flags);> >
> > > > }
> > > >
> > > > static inline int modify_instruction(unsigned int *addr, unsigned int
> > > > clr,
> > > >
> > > > @@ -56,13 +57,13 @@ static inline int modify_instruction_site(s32 *site,
> > > > unsigned int clr, unsigned
> > > >
> > > > return modify_instruction((unsigned int *)patch_site_addr(site),
> > > > clr,
> > > >
> > > > set);
> > > >
> > > > }
> > > >
> > > > -int instr_is_relative_branch(unsigned int instr);
> > > > -int instr_is_relative_link_branch(unsigned int instr);
> > > > -int instr_is_branch_to_addr(const unsigned int *instr, unsigned long
> > > > addr); -unsigned long branch_target(const unsigned int *instr);
> > > > -unsigned int translate_branch(const unsigned int *dest,
> > > > - const unsigned int *src);
> > > > -extern bool is_conditional_branch(unsigned int instr);
> > > > +int instr_is_relative_branch(ppc_inst instr);
> > > > +int instr_is_relative_link_branch(ppc_inst instr);
> > > > +int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr);
> > > > +unsigned long branch_target(const ppc_inst *instr);
> > > > +ppc_inst translate_branch(const ppc_inst *dest,
> > > > + const ppc_inst *src);
> > > > +extern bool is_conditional_branch(ppc_inst instr);
> > > >
> > > > #ifdef CONFIG_PPC_BOOK3E_64
> > > > void __patch_exception(int exc, unsigned long addr);
> > > > #define patch_exception(exc, name) do { \
> > > >
> > > > diff --git a/arch/powerpc/include/asm/inst.h
> > > > b/arch/powerpc/include/asm/inst.h
> > > > new file mode 100644
> > > > index 000000000000..7c8596ee411e
> > > > --- /dev/null
> > > > +++ b/arch/powerpc/include/asm/inst.h
> > > > @@ -0,0 +1,53 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > > +#ifndef _ASM_INST_H
> > > > +#define _ASM_INST_H
> > > > +
> > > > +/*
> > > > + * Instruction data type for POWER
> > > > + */
> > > > +
> > > > +typedef u32 ppc_inst;
> > > > +
> > > > +#define PPC_INST(x) (x)
> > > > +
> > > > +static inline int ppc_inst_len(ppc_inst x)
> > > > +{
> > > > + return sizeof(ppc_inst);
> > > > +}
> > > > +
> > > > +static inline int ppc_inst_opcode(ppc_inst x)
> > > > +{
> > > > + return x >> 26;
> > > > +}
> > > > +
> > > > +static inline u32 ppc_inst_word(ppc_inst x)
> > > > +{
> > > > + return x;
> > > > +}
> > > > +
> > > > +static inline ppc_inst ppc_inst_read(const ppc_inst *ptr)
> > > > +{
> > > > + return *(ppc_inst *)ptr;
> > > > +}
> > > > +
> > > > +static inline void ppc_inst_write(void *ptr, ppc_inst x)
> > > > +{
> > > > + *(ppc_inst *)ptr = x;
> > > > +}
> > > > +
> > > > +static inline bool ppc_inst_equal(ppc_inst x, ppc_inst y)
> > > > +{
> > > > + return x == y;
> > > > +}
> > > > +
> > > > +static inline bool ppc_inst_null(ppc_inst x)
> > > > +{
> > > > + return x == 0;
> > > > +}
> > > > +
> > > > +static inline u32 ppc_inst_mask(ppc_inst x, u32 mask)
> > > > +{
> > > > + return ppc_inst_word(x) & mask;
> > > > +}
> > > > +
> > > > +#endif /* _ASM_INST_H */
> > > > diff --git a/arch/powerpc/include/asm/sstep.h
> > > > b/arch/powerpc/include/asm/sstep.h
> > > > index 769f055509c9..9353916fcba7 100644
> > > > --- a/arch/powerpc/include/asm/sstep.h
> > > > +++ b/arch/powerpc/include/asm/sstep.h
> > > > @@ -2,6 +2,7 @@
> > > >
> > > > /*
> > > >
> > > > * Copyright (C) 2004 Paul Mackerras <paulus@au.ibm.com>, IBM
> > > > */
> > > >
> > > > +#include <asm/inst.h>
> > > >
> > > > struct pt_regs;
> > > >
> > > > @@ -132,7 +133,7 @@ union vsx_reg {
> > > >
> > > > * otherwise.
> > > > */
> > > >
> > > > extern int analyse_instr(struct instruction_op *op, const struct
> > > > pt_regs
> > > >
> > > > *regs,
> > > > - unsigned int instr);
> > > > + ppc_inst instr);
> > > >
> > > > /*
> > > >
> > > > * Emulate an instruction that can be executed just by updating
> > > >
> > > > @@ -149,7 +150,7 @@ void emulate_update_regs(struct pt_regs *reg, struct
> > > > instruction_op *op);
> > > >
> > > > * 0 if it could not be emulated, or -1 for an instruction that
> > > > * should not be emulated (rfid, mtmsrd clearing MSR_RI, etc.).
> > > > */
> > > >
> > > > -extern int emulate_step(struct pt_regs *regs, unsigned int instr);
> > > > +extern int emulate_step(struct pt_regs *regs, ppc_inst instr);
> > > >
> > > > /*
> > > >
> > > > * Emulate a load or store instruction by reading/writing the
> > > >
> > > > diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> > > > index 92045ed64976..34594aaa44de 100644
> > > > --- a/arch/powerpc/kernel/align.c
> > > > +++ b/arch/powerpc/kernel/align.c
> > > > @@ -293,7 +293,7 @@ static int emulate_spe(struct pt_regs *regs,
> > > > unsigned int reg,
> > > >
> > > > int fix_alignment(struct pt_regs *regs)
> > > > {
> > > >
> > > > - unsigned int instr;
> > > > + ppc_inst instr;
> > > >
> > > > struct instruction_op op;
> > > > int r, type;
> > > >
> > > > diff --git a/arch/powerpc/kernel/hw_breakpoint.c
> > > > b/arch/powerpc/kernel/hw_breakpoint.c
> > > > index 2462cd7c565c..06b97353d231 100644
> > > > --- a/arch/powerpc/kernel/hw_breakpoint.c
> > > > +++ b/arch/powerpc/kernel/hw_breakpoint.c
> > > > @@ -24,6 +24,7 @@
> > > >
> > > > #include <asm/debug.h>
> > > > #include <asm/debugfs.h>
> > > > #include <asm/hvcall.h>
> > > >
> > > > +#include <asm/inst.h>
> > > >
> > > > #include <linux/uaccess.h>
> > > >
> > > > /*
> > > >
> > > > @@ -243,7 +244,7 @@ dar_range_overlaps(unsigned long dar, int size,
> > > > struct
> > > > arch_hw_breakpoint *info)
> > > >
> > > > static bool stepping_handler(struct pt_regs *regs, struct perf_event
> > > > *bp,
> > > >
> > > > struct arch_hw_breakpoint *info)
> > > >
> > > > {
> > > >
> > > > - unsigned int instr = 0;
> > > > + ppc_inst instr = 0;
> > > >
> > > > int ret, type, size;
> > > > struct instruction_op op;
> > > > unsigned long addr = info->address;
> > > >
> > > > diff --git a/arch/powerpc/kernel/kprobes.c
> > > > b/arch/powerpc/kernel/kprobes.c
> > > > index 337516df17d4..e7205adc9820 100644
> > > > --- a/arch/powerpc/kernel/kprobes.c
> > > > +++ b/arch/powerpc/kernel/kprobes.c
> > > > @@ -225,7 +225,7 @@ NOKPROBE_SYMBOL(arch_prepare_kretprobe);
> > > >
> > > > static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
> > > > {
> > > >
> > > > int ret;
> > > >
> > > > - unsigned int insn = *p->ainsn.insn;
> > > > + ppc_inst insn = *p->ainsn.insn;
> > > >
> > > > /* regs->nip is also adjusted if emulate_step returns 1 */
> > > > ret = emulate_step(regs, insn);
> > > >
> > > > diff --git a/arch/powerpc/kernel/mce_power.c
> > > > b/arch/powerpc/kernel/mce_power.c
> > > > index 1cbf7f1a4e3d..e65616bb3a3e 100644
> > > > --- a/arch/powerpc/kernel/mce_power.c
> > > > +++ b/arch/powerpc/kernel/mce_power.c
> > > > @@ -20,6 +20,7 @@
> > > >
> > > > #include <asm/sstep.h>
> > > > #include <asm/exception-64s.h>
> > > > #include <asm/extable.h>
> > > >
> > > > +#include <asm/inst.h>
> > > >
> > > > /*
> > > >
> > > > * Convert an address related to an mm to a PFN. NOTE: we are in real
> > > >
> > > > @@ -365,7 +366,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs
> > > > *regs, uint64_t *addr,
> > > >
> > > > * in real-mode is tricky and can lead to recursive
> > > > * faults
> > > > */
> > > >
> > > > - int instr;
> > > > + ppc_inst instr;
> > > >
> > > > unsigned long pfn, instr_addr;
> > > > struct instruction_op op;
> > > > struct pt_regs tmp = *regs;
> > > >
> > > > @@ -373,7 +374,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs
> > > > *regs, uint64_t *addr,
> > > >
> > > > pfn = addr_to_pfn(regs, regs->nip);
> > > > if (pfn != ULONG_MAX) {
> > > >
> > > > instr_addr = (pfn << PAGE_SHIFT) + (regs->nip &
> > > > ~PAGE_MASK);
> > > >
> > > > - instr = *(unsigned int *)(instr_addr);
> > > > + instr = *(ppc_inst *)(instr_addr);
> > > >
> > > > if (!analyse_instr(&op, &tmp, instr)) {
> > > >
> > > > pfn = addr_to_pfn(regs, op.ea);
> > > > *addr = op.ea;
> > > >
> > > > diff --git a/arch/powerpc/kernel/optprobes.c
> > > > b/arch/powerpc/kernel/optprobes.c
> > > > index 024f7aad1952..f5e8cce438a3 100644
> > > > --- a/arch/powerpc/kernel/optprobes.c
> > > > +++ b/arch/powerpc/kernel/optprobes.c
> > > > @@ -189,8 +189,8 @@ void patch_imm64_load_insns(unsigned long val,
> > > > kprobe_opcode_t *addr)
> > > >
> > > > int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct
> > > > kprobe> >
> > > > *p)
> > > >
> > > > {
> > > >
> > > > - kprobe_opcode_t *buff, branch_op_callback, branch_emulate_step;
> > > > - kprobe_opcode_t *op_callback_addr, *emulate_step_addr;
> > > > + ppc_inst branch_op_callback, branch_emulate_step;
> > > > + kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> > > >
> > > > long b_offset;
> > > > unsigned long nip, size;
> > > > int rc, i;
> > > >
> > > > @@ -251,11 +251,11 @@ int arch_prepare_optimized_kprobe(struct
> > > > optimized_kprobe *op, struct kprobe *p)
> > > >
> > > > goto error;
> > > >
> > > > }
> > > >
> > > > - branch_op_callback = create_branch((unsigned int *)buff +
> > > > TMPL_CALL_HDLR_IDX,
> > > > + branch_op_callback = create_branch((ppc_inst *)buff +
> > > > TMPL_CALL_HDLR_IDX,
> > > >
> > > > (unsigned long)op_callback_addr,
> > > > BRANCH_SET_LINK);
> > > >
> > > > - branch_emulate_step = create_branch((unsigned int *)buff +
> > > > TMPL_EMULATE_IDX,
> > > > + branch_emulate_step = create_branch((ppc_inst *)buff +
> > > > TMPL_EMULATE_IDX,
> > > >
> > > > (unsigned long)emulate_step_addr,
> > > > BRANCH_SET_LINK);
> > > >
> > > > @@ -316,7 +316,7 @@ void arch_optimize_kprobes(struct list_head *oplist)
> > > >
> > > > memcpy(op->optinsn.copied_insn, op->kp.addr,
> > > >
> > > > RELATIVEJUMP_SIZE);
> > > >
> > > > patch_instruction(op->kp.addr,
> > > >
> > > > - create_branch((unsigned int *)op->kp.addr,
> > > > + create_branch((ppc_inst *)op->kp.addr,
> > > >
> > > > (unsigned long)op->optinsn.insn,
> > > > 0));
> > > >
> > > > list_del_init(&op->list);
> > > >
> > > > }
> > > >
> > > > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > > > b/arch/powerpc/kernel/trace/ftrace.c
> > > > index 7ea0ca044b65..5787ccffb4df 100644
> > > > --- a/arch/powerpc/kernel/trace/ftrace.c
> > > > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > > > @@ -27,6 +27,7 @@
> > > >
> > > > #include <asm/code-patching.h>
> > > > #include <asm/ftrace.h>
> > > > #include <asm/syscall.h>
> > > >
> > > > +#include <asm/inst.h>
> > > >
> > > > #ifdef CONFIG_DYNAMIC_FTRACE
> > > >
> > > > @@ -40,23 +41,23 @@
> > > >
> > > > #define NUM_FTRACE_TRAMPS 8
> > > > static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
> > > >
> > > > -static unsigned int
> > > > +static ppc_inst
> > > >
> > > > ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
> > > > {
> > > >
> > > > - unsigned int op;
> > > > + ppc_inst op;
> > > >
> > > > addr = ppc_function_entry((void *)addr);
> > > >
> > > > /* if (link) set op to 'bl' else 'b' */
> > > >
> > > > - op = create_branch((unsigned int *)ip, addr, link ? 1 : 0);
> > > > + op = create_branch((ppc_inst *)ip, addr, link ? 1 : 0);
> > > >
> > > > return op;
> > > >
> > > > }
> > > >
> > > > static int
> > > >
> > > > -ftrace_modify_code(unsigned long ip, unsigned int old, unsigned int
> > > > new)
> > > > +ftrace_modify_code(unsigned long ip, ppc_inst old, ppc_inst new)
> > > >
> > > > {
> > > >
> > > > - unsigned int replaced;
> > > > + ppc_inst replaced;
> > > >
> > > > /*
> > > >
> > > > * Note:
> > > > @@ -78,7 +79,7 @@ ftrace_modify_code(unsigned long ip, unsigned int old,
> > > > unsigned int new)
> > > >
> > > > }
> > > >
> > > > /* replace the text with the new text */
> > > >
> > > > - if (patch_instruction((unsigned int *)ip, new))
> > > > + if (patch_instruction((ppc_inst *)ip, new))
> > > >
> > > > return -EPERM;
> > > >
> > > > return 0;
> > > >
> > > > @@ -87,25 +88,25 @@ ftrace_modify_code(unsigned long ip, unsigned int
> > > > old,
> > > > unsigned int new)
> > > >
> > > > /*
> > > >
> > > > * Helper functions that are the same for both PPC64 and PPC32.
> > > > */
> > > >
> > > > -static int test_24bit_addr(unsigned long ip, unsigned long addr)
> > > > +static ppc_inst test_24bit_addr(unsigned long ip, unsigned long addr)
> > > >
> > > > {
> > > >
> > > > addr = ppc_function_entry((void *)addr);
> > > >
> > > > /* use the create_branch to verify that this offset can be
> > > > branched */
> > > >
> > > > - return create_branch((unsigned int *)ip, addr, 0);
> > > > + return create_branch((ppc_inst *)ip, addr, 0);
> > > >
> > > > }
> > > >
> > > > -static int is_bl_op(unsigned int op)
> > > > +static int is_bl_op(ppc_inst op)
> > > >
> > > > {
> > > >
> > > > return (op & 0xfc000003) == 0x48000001;
> > > >
> > > > }
> > > >
> > > > -static int is_b_op(unsigned int op)
> > > > +static int is_b_op(ppc_inst op)
> > > >
> > > > {
> > > >
> > > > return (op & 0xfc000003) == 0x48000000;
> > > >
> > > > }
> > > >
> > > > -static unsigned long find_bl_target(unsigned long ip, unsigned int op)
> > > > +static unsigned long find_bl_target(unsigned long ip, ppc_inst op)
> > > >
> > > > {
> > > >
> > > > int offset;
> > > >
> > > > @@ -125,7 +126,7 @@ __ftrace_make_nop(struct module *mod,
> > > >
> > > > {
> > > >
> > > > unsigned long entry, ptr, tramp;
> > > > unsigned long ip = rec->ip;
> > > >
> > > > - unsigned int op, pop;
> > > > + ppc_inst op, pop;
> > > >
> > > > /* read where this goes */
> > > > if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
> > > >
> > > > @@ -204,7 +205,7 @@ __ftrace_make_nop(struct module *mod,
> > > >
> > > > }
> > > >
> > > > #endif /* CONFIG_MPROFILE_KERNEL */
> > > >
> > > > - if (patch_instruction((unsigned int *)ip, pop)) {
> > > > + if (patch_instruction((ppc_inst *)ip, pop)) {
> > > >
> > > > pr_err("Patching NOP failed.\n");
> > > > return -EPERM;
> > > >
> > > > }
> > > >
> > > > @@ -217,7 +218,7 @@ static int
> > > >
> > > > __ftrace_make_nop(struct module *mod,
> > > >
> > > > struct dyn_ftrace *rec, unsigned long addr)
> > > >
> > > > {
> > > >
> > > > - unsigned int op;
> > > > + ppc_inst op;
> > > >
> > > > unsigned int jmp[4];
> > > > unsigned long ip = rec->ip;
> > > > unsigned long tramp;
> > > >
> > > > @@ -276,7 +277,7 @@ __ftrace_make_nop(struct module *mod,
> > > >
> > > > op = PPC_INST_NOP;
> > > >
> > > > - if (patch_instruction((unsigned int *)ip, op))
> > > > + if (patch_instruction((ppc_inst *)ip, op))
> > > >
> > > > return -EPERM;
> > > >
> > > > return 0;
> > > >
> > > > @@ -322,7 +323,8 @@ static int add_ftrace_tramp(unsigned long tramp)
> > > >
> > > > */
> > > >
> > > > static int setup_mcount_compiler_tramp(unsigned long tramp)
> > > > {
> > > >
> > > > - int i, op;
> > > > + int i;
> > > > + ppc_inst op;
> > > >
> > > > unsigned long ptr;
> > > > static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
> > > >
> > > > @@ -388,7 +390,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> > > > tramp)
> > > >
> > > > static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned
> > > > long
> > > >
> > > > addr)
> > > >
> > > > {
> > > >
> > > > unsigned long tramp, ip = rec->ip;
> > > >
> > > > - unsigned int op;
> > > > + ppc_inst op;
> > > >
> > > > /* Read where this goes */
> > > > if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
> > > >
> > > > @@ -416,7 +418,7 @@ static int __ftrace_make_nop_kernel(struct
> > > > dyn_ftrace
> > > > *rec, unsigned long addr)
> > > >
> > > > }
> > > >
> > > > }
> > > >
> > > > - if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
> > > > + if (patch_instruction((ppc_inst *)ip, PPC_INST_NOP)) {
> > > >
> > > > pr_err("Patching NOP failed.\n");
> > > > return -EPERM;
> > > >
> > > > }
> > > >
> > > > @@ -428,7 +430,7 @@ int ftrace_make_nop(struct module *mod,
> > > >
> > > > struct dyn_ftrace *rec, unsigned long addr)
> > > >
> > > > {
> > > >
> > > > unsigned long ip = rec->ip;
> > > >
> > > > - unsigned int old, new;
> > > > + ppc_inst old, new;
> > > >
> > > > /*
> > > >
> > > > * If the calling address is more that 24 bits away,
> > > >
> > > > @@ -481,7 +483,7 @@ int ftrace_make_nop(struct module *mod,
> > > >
> > > > */
> > > >
> > > > #ifndef CONFIG_MPROFILE_KERNEL
> > > > static int
> > > >
> > > > -expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1)
> > > > +expected_nop_sequence(void *ip, ppc_inst op0, ppc_inst op1)
> > > >
> > > > {
> > > >
> > > > /*
> > > >
> > > > * We expect to see:
> > > > @@ -510,7 +512,7 @@ expected_nop_sequence(void *ip, unsigned int op0,
> > > > unsigned int op1)
> > > >
> > > > static int
> > > > __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > > > {
> > > >
> > > > - unsigned int op[2];
> > > > + ppc_inst op[2];
> > > >
> > > > void *ip = (void *)rec->ip;
> > > > unsigned long entry, ptr, tramp;
> > > > struct module *mod = rec->arch.mod;
> > > >
> > > > @@ -574,7 +576,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > > long addr)
> > > >
> > > > static int
> > > > __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > > > {
> > > >
> > > > - unsigned int op;
> > > > + ppc_inst op;
> > > >
> > > > unsigned long ip = rec->ip;
> > > >
> > > > /* read where this goes */
> > > >
> > > > @@ -594,7 +596,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > > long addr)
> > > >
> > > > }
> > > >
> > > > /* create the branch to the trampoline */
> > > >
> > > > - op = create_branch((unsigned int *)ip,
> > > > + op = create_branch((ppc_inst *)ip,
> > > >
> > > > rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> > > >
> > > > if (!op) {
> > > >
> > > > pr_err("REL24 out of range!\n");
> > > >
> > > > @@ -613,7 +615,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > > long addr)
> > > >
> > > > static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned
> > > > long
> > > >
> > > > addr)
> > > >
> > > > {
> > > >
> > > > - unsigned int op;
> > > > + ppc_inst op;
> > > >
> > > > void *ip = (void *)rec->ip;
> > > > unsigned long tramp, entry, ptr;
> > > >
> > > > @@ -661,7 +663,7 @@ static int __ftrace_make_call_kernel(struct
> > > > dyn_ftrace
> > > > *rec, unsigned long addr)
> > > >
> > > > int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > > > {
> > > >
> > > > unsigned long ip = rec->ip;
> > > >
> > > > - unsigned int old, new;
> > > > + ppc_inst old, new;
> > > >
> > > > /*
> > > >
> > > > * If the calling address is more that 24 bits away,
> > > >
> > > > @@ -700,7 +702,7 @@ static int
> > > >
> > > > __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
> > > >
> > > > unsigned long addr)
> > > >
> > > > {
> > > >
> > > > - unsigned int op;
> > > > + ppc_inst op;
> > > >
> > > > unsigned long ip = rec->ip;
> > > > unsigned long entry, ptr, tramp;
> > > > struct module *mod = rec->arch.mod;
> > > >
> > > > @@ -748,7 +750,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec,
> > > > unsigned
> > > > long old_addr,
> > > >
> > > > /* The new target may be within range */
> > > > if (test_24bit_addr(ip, addr)) {
> > > >
> > > > /* within range */
> > > >
> > > > - if (patch_branch((unsigned int *)ip, addr,
> > > > BRANCH_SET_LINK)) { + if (patch_branch((ppc_inst *)ip,
> > > > addr, BRANCH_SET_LINK)) {> >
> > > > pr_err("REL24 out of range!\n");
> > > > return -EINVAL;
> > > >
> > > > }
> > > >
> > > > @@ -776,7 +778,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec,
> > > > unsigned
> > > > long old_addr,
> > > >
> > > > }
> > > >
> > > > /* Ensure branch is within 24 bits */
> > > >
> > > > - if (!create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
> > > > + if (!create_branch((ppc_inst *)ip, tramp, BRANCH_SET_LINK)) {
> > > >
> > > > pr_err("Branch out of range\n");
> > > > return -EINVAL;
> > > >
> > > > }
> > > >
> > > > @@ -794,7 +796,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec,
> > > > unsigned
> > > > long old_addr,
> > > >
> > > > unsigned long addr)
> > > >
> > > > {
> > > >
> > > > unsigned long ip = rec->ip;
> > > >
> > > > - unsigned int old, new;
> > > > + ppc_inst old, new;
> > > >
> > > > /*
> > > >
> > > > * If the calling address is more that 24 bits away,
> > > >
> > > > @@ -834,7 +836,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec,
> > > > unsigned
> > > > long old_addr,
> > > >
> > > > int ftrace_update_ftrace_func(ftrace_func_t func)
> > > > {
> > > >
> > > > unsigned long ip = (unsigned long)(&ftrace_call);
> > > >
> > > > - unsigned int old, new;
> > > > + ppc_inst old, new;
> > > >
> > > > int ret;
> > > >
> > > > old = *(unsigned int *)&ftrace_call;
> > > >
> > > > @@ -919,7 +921,7 @@ int ftrace_enable_ftrace_graph_caller(void)
> > > >
> > > > unsigned long ip = (unsigned long)(&ftrace_graph_call);
> > > > unsigned long addr = (unsigned long)(&ftrace_graph_caller);
> > > > unsigned long stub = (unsigned long)(&ftrace_graph_stub);
> > > >
> > > > - unsigned int old, new;
> > > > + ppc_inst old, new;
> > > >
> > > > old = ftrace_call_replace(ip, stub, 0);
> > > > new = ftrace_call_replace(ip, addr, 0);
> > > >
> > > > @@ -932,7 +934,7 @@ int ftrace_disable_ftrace_graph_caller(void)
> > > >
> > > > unsigned long ip = (unsigned long)(&ftrace_graph_call);
> > > > unsigned long addr = (unsigned long)(&ftrace_graph_caller);
> > > > unsigned long stub = (unsigned long)(&ftrace_graph_stub);
> > > >
> > > > - unsigned int old, new;
> > > > + ppc_inst old, new;
> > > >
> > > > old = ftrace_call_replace(ip, addr, 0);
> > > > new = ftrace_call_replace(ip, stub, 0);
> > > >
> > > > diff --git a/arch/powerpc/kvm/emulate_loadstore.c
> > > > b/arch/powerpc/kvm/emulate_loadstore.c
> > > > index 1139bc56e004..1c9bcbfeb924 100644
> > > > --- a/arch/powerpc/kvm/emulate_loadstore.c
> > > > +++ b/arch/powerpc/kvm/emulate_loadstore.c
> > > > @@ -21,6 +21,7 @@
> > > >
> > > > #include <asm/disassemble.h>
> > > > #include <asm/ppc-opcode.h>
> > > > #include <asm/sstep.h>
> > > >
> > > > +#include <asm/inst.h>
> > > >
> > > > #include "timing.h"
> > > > #include "trace.h"
> > > >
> > > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > > > patching.c
> > > > index 3345f039a876..8492b9e2b8db 100644
> > > > --- a/arch/powerpc/lib/code-patching.c
> > > > +++ b/arch/powerpc/lib/code-patching.c
> > > > @@ -17,9 +17,10 @@
> > > >
> > > > #include <asm/page.h>
> > > > #include <asm/code-patching.h>
> > > > #include <asm/setup.h>
> > > >
> > > > +#include <asm/inst.h>
> > > >
> > > > -static int __patch_instruction(unsigned int *exec_addr, unsigned int
> > > > instr, - unsigned int *patch_addr)
> > > > +static int __patch_instruction(ppc_inst *exec_addr, ppc_inst instr,
> > > > + ppc_inst *patch_addr)
> > > >
> > > > {
> > > >
> > > > int err = 0;
> > > >
> > > > @@ -33,7 +34,7 @@ static int __patch_instruction(unsigned int
> > > > *exec_addr,
> > > > unsigned int instr,
> > > >
> > > > return 0;
> > > >
> > > > }
> > > >
> > > > -int raw_patch_instruction(unsigned int *addr, unsigned int instr)
> > > > +int raw_patch_instruction(ppc_inst *addr, ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > return __patch_instruction(addr, instr, addr);
> > > >
> > > > }
> > > >
> > > > @@ -136,10 +137,10 @@ static inline int unmap_patch_area(unsigned long
> > > > addr)> >
> > > > return 0;
> > > >
> > > > }
> > > >
> > > > -static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> > > > +static int do_patch_instruction(ppc_inst *addr, ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > int err;
> > > >
> > > > - unsigned int *patch_addr = NULL;
> > > > + ppc_inst *patch_addr = NULL;
> > > >
> > > > unsigned long flags;
> > > > unsigned long text_poke_addr;
> > > > unsigned long kaddr = (unsigned long)addr;
> > > >
> > > > @@ -176,7 +177,7 @@ static int do_patch_instruction(unsigned int *addr,
> > > > unsigned int instr)
> > > >
> > > > }
> > > > #else /* !CONFIG_STRICT_KERNEL_RWX */
> > > >
> > > > -static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> > > > +static int do_patch_instruction(ppc_inst *addr, ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > return raw_patch_instruction(addr, instr);
> > > >
> > > > }
> > > >
> > > > @@ -194,7 +195,7 @@ int patch_instruction(unsigned int *addr, unsigned
> > > > int
> > > > instr)
> > > >
> > > > }
> > > > NOKPROBE_SYMBOL(patch_instruction);
> > > >
> > > > -int patch_branch(unsigned int *addr, unsigned long target, int flags)
> > > > +int patch_branch(ppc_inst *addr, unsigned long target, int flags)
> > > >
> > > > {
> > > >
> > > > return patch_instruction(addr, create_branch(addr, target,
> > > > flags));
> > > >
> > > > }
> > > >
> > > > @@ -225,7 +226,7 @@ bool is_offset_in_branch_range(long offset)
> > > >
> > > > * Helper to check if a given instruction is a conditional branch
> > > > * Derived from the conditional checks in analyse_instr()
> > > > */
> > > >
> > > > -bool is_conditional_branch(unsigned int instr)
> > > > +bool is_conditional_branch(ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > unsigned int opcode = instr >> 26;
> > > >
> > > > @@ -243,10 +244,10 @@ bool is_conditional_branch(unsigned int instr)
> > > >
> > > > }
> > > > NOKPROBE_SYMBOL(is_conditional_branch);
> > > >
> > > > -unsigned int create_branch(const unsigned int *addr,
> > > > +ppc_inst create_branch(const ppc_inst *addr,
> > > >
> > > > unsigned long target, int flags)
> > > >
> > > > {
> > > >
> > > > - unsigned int instruction;
> > > > + ppc_inst instruction;
> > > >
> > > > long offset;
> > > >
> > > > offset = target;
> > > >
> > > > @@ -266,7 +267,7 @@ unsigned int create_branch(const unsigned int *addr,
> > > >
> > > > unsigned int create_cond_branch(const unsigned int *addr,
> > > >
> > > > unsigned long target, int flags)
> > > >
> > > > {
> > > >
> > > > - unsigned int instruction;
> > > > + ppc_inst instruction;
> > > >
> > > > long offset;
> > > >
> > > > offset = target;
> > > >
> > > > @@ -283,22 +284,22 @@ unsigned int create_cond_branch(const unsigned int
> > > > *addr,
> > > >
> > > > return instruction;
> > > >
> > > > }
> > > >
> > > > -static unsigned int branch_opcode(unsigned int instr)
> > > > +static unsigned int branch_opcode(ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > return (instr >> 26) & 0x3F;
> > > >
> > > > }
> > > >
> > > > -static int instr_is_branch_iform(unsigned int instr)
> > > > +static int instr_is_branch_iform(ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > return branch_opcode(instr) == 18;
> > > >
> > > > }
> > > >
> > > > -static int instr_is_branch_bform(unsigned int instr)
> > > > +static int instr_is_branch_bform(ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > return branch_opcode(instr) == 16;
> > > >
> > > > }
> > > >
> > > > -int instr_is_relative_branch(unsigned int instr)
> > > > +int instr_is_relative_branch(ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > if (instr & BRANCH_ABSOLUTE)
> > > >
> > > > return 0;
> > > >
> > > > @@ -306,12 +307,12 @@ int instr_is_relative_branch(unsigned int instr)
> > > >
> > > > return instr_is_branch_iform(instr) ||
> > > > instr_is_branch_bform(instr);
> > > >
> > > > }
> > > >
> > > > -int instr_is_relative_link_branch(unsigned int instr)
> > > > +int instr_is_relative_link_branch(ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > return instr_is_relative_branch(instr) && (instr &
> > > > BRANCH_SET_LINK);
> > > >
> > > > }
> > > >
> > > > -static unsigned long branch_iform_target(const unsigned int *instr)
> > > > +static unsigned long branch_iform_target(const ppc_inst *instr)
> > > >
> > > > {
> > > >
> > > > signed long imm;
> > > >
> > > > @@ -327,7 +328,7 @@ static unsigned long branch_iform_target(const
> > > > unsigned
> > > > int *instr)
> > > >
> > > > return (unsigned long)imm;
> > > >
> > > > }
> > > >
> > > > -static unsigned long branch_bform_target(const unsigned int *instr)
> > > > +static unsigned long branch_bform_target(const ppc_inst *instr)
> > > >
> > > > {
> > > >
> > > > signed long imm;
> > > >
> > > > @@ -343,7 +344,7 @@ static unsigned long branch_bform_target(const
> > > > unsigned
> > > > int *instr)
> > > >
> > > > return (unsigned long)imm;
> > > >
> > > > }
> > > >
> > > > -unsigned long branch_target(const unsigned int *instr)
> > > > +unsigned long branch_target(const ppc_inst *instr)
> > > >
> > > > {
> > > >
> > > > if (instr_is_branch_iform(*instr))
> > > >
> > > > return branch_iform_target(instr);
> > > >
> > > > @@ -353,7 +354,7 @@ unsigned long branch_target(const unsigned int
> > > > *instr)
> > > >
> > > > return 0;
> > > >
> > > > }
> > > >
> > > > -int instr_is_branch_to_addr(const unsigned int *instr, unsigned long
> > > > addr)
> > > > +int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr)
> > > >
> > > > {
> > > >
> > > > if (instr_is_branch_iform(*instr) ||
> > > > instr_is_branch_bform(*instr))
> > > >
> > > > return branch_target(instr) == addr;
> > > >
> > > > @@ -361,7 +362,7 @@ int instr_is_branch_to_addr(const unsigned int
> > > > *instr,
> > > > unsigned long addr)
> > > >
> > > > return 0;
> > > >
> > > > }
> > > >
> > > > -unsigned int translate_branch(const unsigned int *dest, const unsigned
> > > > int
> > > > *src)
> > > > +ppc_inst translate_branch(const ppc_inst *dest, const ppc_inst *src)
> > > >
> > > > {
> > > >
> > > > unsigned long target;
> > > >
> > > > @@ -403,7 +404,7 @@ static void __init test_trampoline(void)
> > > >
> > > > static void __init test_branch_iform(void)
> > > > {
> > > >
> > > > - unsigned int instr;
> > > > + ppc_inst instr;
> > > >
> > > > unsigned long addr;
> > > >
> > > > addr = (unsigned long)&instr;
> > > >
> > > > @@ -478,11 +479,11 @@ static void __init test_branch_iform(void)
> > > >
> > > > static void __init test_create_function_call(void)
> > > > {
> > > >
> > > > - unsigned int *iptr;
> > > > + ppc_inst *iptr;
> > > >
> > > > unsigned long dest;
> > > >
> > > > /* Check we can create a function call */
> > > >
> > > > - iptr = (unsigned int *)ppc_function_entry(test_trampoline);
> > > > + iptr = (ppc_inst *)ppc_function_entry(test_trampoline);
> > > >
> > > > dest = ppc_function_entry(test_create_function_call);
> > > > patch_instruction(iptr, create_branch(iptr, dest,
> > > > BRANCH_SET_LINK));
> > > > check(instr_is_branch_to_addr(iptr, dest));
> > > >
> > > > @@ -491,7 +492,8 @@ static void __init test_create_function_call(void)
> > > >
> > > > static void __init test_branch_bform(void)
> > > > {
> > > >
> > > > unsigned long addr;
> > > >
> > > > - unsigned int *iptr, instr, flags;
> > > > + ppc_inst *iptr, instr;
> > > > + unsigned int flags;
> > > >
> > > > iptr = &instr;
> > > > addr = (unsigned long)iptr;
> > > >
> > > > @@ -561,7 +563,7 @@ static void __init test_branch_bform(void)
> > > >
> > > > static void __init test_translate_branch(void)
> > > > {
> > > >
> > > > unsigned long addr;
> > > >
> > > > - unsigned int *p, *q;
> > > > + ppc_inst *p, *q;
> > > >
> > > > void *buf;
> > > >
> > > > buf = vmalloc(PAGE_ALIGN(0x2000000 + 1));
> > > >
> > > > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > > > index c077acb983a1..1d9c766a89fe 100644
> > > > --- a/arch/powerpc/lib/sstep.c
> > > > +++ b/arch/powerpc/lib/sstep.c
> > > > @@ -1163,7 +1163,7 @@ static nokprobe_inline int trap_compare(long v1,
> > > > long
> > > > v2)
> > > >
> > > > * otherwise.
> > > > */
> > > >
> > > > int analyse_instr(struct instruction_op *op, const struct pt_regs
> > > > *regs,
> > > >
> > > > - unsigned int instr)
> > > > + ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > unsigned int opcode, ra, rb, rc, rd, spr, u;
> > > > unsigned long int imm;
> > > >
> > > > @@ -3101,7 +3101,7 @@ NOKPROBE_SYMBOL(emulate_loadstore);
> > > >
> > > > * or -1 if the instruction is one that should not be stepped,
> > > > * such as an rfid, or a mtmsrd that would clear MSR_RI.
> > > > */
> > > >
> > > > -int emulate_step(struct pt_regs *regs, unsigned int instr)
> > > > +int emulate_step(struct pt_regs *regs, ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > struct instruction_op op;
> > > > int r, err, type;
> > > >
> > > > diff --git a/arch/powerpc/lib/test_emulate_step.c
> > > > b/arch/powerpc/lib/test_emulate_step.c
> > > > index 42347067739c..158efc8a0f53 100644
> > > > --- a/arch/powerpc/lib/test_emulate_step.c
> > > > +++ b/arch/powerpc/lib/test_emulate_step.c
> > > > @@ -460,7 +460,7 @@ struct compute_test {
> > > >
> > > > struct {
> > > >
> > > > char *descr;
> > > > unsigned long flags;
> > > >
> > > > - unsigned int instr;
> > > > + ppc_inst instr;
> > > >
> > > > struct pt_regs regs;
> > > >
> > > > } subtests[MAX_SUBTESTS + 1];
> > > >
> > > > };
> > > >
> > > > @@ -841,7 +841,7 @@ static struct compute_test compute_tests[] = {
> > > >
> > > > };
> > > >
> > > > static int __init emulate_compute_instr(struct pt_regs *regs,
> > > >
> > > > - unsigned int instr)
> > > > + ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > struct instruction_op op;
> > > >
> > > > @@ -859,7 +859,7 @@ static int __init emulate_compute_instr(struct
> > > > pt_regs
> > > > *regs,
> > > >
> > > > }
> > > >
> > > > static int __init execute_compute_instr(struct pt_regs *regs,
> > > >
> > > > - unsigned int instr)
> > > > + ppc_inst instr)
> > > >
> > > > {
> > > >
> > > > extern int exec_instr(struct pt_regs *regs);
> > > > extern s32 patch__exec_instr;
> > > >
> > > > @@ -890,7 +890,8 @@ static void __init run_tests_compute(void)
> > > >
> > > > unsigned long flags;
> > > > struct compute_test *test;
> > > > struct pt_regs *regs, exp, got;
> > > >
> > > > - unsigned int i, j, k, instr;
> > > > + unsigned int i, j, k;
> > > > + ppc_inst instr;
> > > >
> > > > bool ignore_gpr, ignore_xer, ignore_ccr, passed;
> > > >
> > > > for (i = 0; i < ARRAY_SIZE(compute_tests); i++) {
> > > >
> > > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > > > index 7875d1a37770..a0bc442f9557 100644
> > > > --- a/arch/powerpc/xmon/xmon.c
> > > > +++ b/arch/powerpc/xmon/xmon.c
> > > > @@ -892,7 +892,7 @@ static struct bpt *new_breakpoint(unsigned long a)
> > > >
> > > > static void insert_bpts(void)
> > > > {
> > > >
> > > > int i;
> > > >
> > > > - unsigned int instr;
> > > > + ppc_inst instr;
> > > >
> > > > struct bpt *bp;
> > > >
> > > > bp = bpts;
> > > >
> > > > @@ -914,7 +914,7 @@ static void insert_bpts(void)
> > > >
> > > > patch_instruction(bp->instr, instr);
> > > > if (bp->enabled & BP_CIABR)
> > > >
> > > > continue;
> > > >
> > > > - if (patch_instruction((unsigned int *)bp->address,
> > > > + if (patch_instruction((ppc_inst *)bp->address,
> > > >
> > > > bpinstr) != 0) {
> > > >
> > > > printf("Couldn't write instruction at %lx, "
> > > >
> > > > "disabling breakpoint there\n",
> > > > bp->address);
> > > >
> > > > @@ -943,7 +943,7 @@ static void remove_bpts(void)
> > > >
> > > > {
> > > >
> > > > int i;
> > > > struct bpt *bp;
> > > >
> > > > - unsigned instr;
> > > > + ppc_inst instr;
> > > >
> > > > bp = bpts;
> > > > for (i = 0; i < NBPTS; ++i, ++bp) {
> > > >
> > > > @@ -952,7 +952,7 @@ static void remove_bpts(void)
> > > >
> > > > if (mread(bp->address, &instr, 4) == 4
> > > >
> > > > && instr == bpinstr
> > > > && patch_instruction(
> > > >
> > > > - (unsigned int *)bp->address, bp->instr[0]) != 0)
> > > > + (ppc_inst *)bp->address, bp->instr[0]) != 0)
> > > >
> > > > printf("Couldn't remove breakpoint at %lx\n",
> > > >
> > > > bp->address);
> > > >
> > > > }
> > > >
> > > > @@ -1159,7 +1159,7 @@ static int do_step(struct pt_regs *regs)
> > > >
> > > > */
> > > >
> > > > static int do_step(struct pt_regs *regs)
> > > > {
> > > >
> > > > - unsigned int instr;
> > > > + ppc_inst instr;
> > > >
> > > > int stepped;
> > > >
> > > > force_enable_xmon();
> > > >
> > > > @@ -1325,7 +1325,7 @@ csum(void)
> > > >
> > > > */
> > > >
> > > > static long check_bp_loc(unsigned long addr)
> > > > {
> > > >
> > > > - unsigned int instr;
> > > > + ppc_inst instr;
> > > >
> > > > addr &= ~3;
> > > > if (!is_kernel_addr(addr)) {
>
>
>
>
^ permalink raw reply
* Re: [PATCH 1/4] scsi: cxlflash: Access interrupt trigger page from xive directly
From: Matthew R. Ochs @ 2020-04-02 23:19 UTC (permalink / raw)
To: Frederic Barrat
Cc: ukrishn, ajd, haren, groug, clg, linuxppc-dev, christophe_lombard
In-Reply-To: <20200402154352.586166-2-fbarrat@linux.ibm.com>
On Thu, Apr 02, 2020 at 05:43:49PM +0200, Frederic Barrat wrote:
> xive is already mapping the trigger page in kernel space and it can be
> accessed through standard APIs, so let's reuse it and simplify the code.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
> drivers/scsi/cxlflash/ocxl_hw.c | 17 +++++++----------
> drivers/scsi/cxlflash/ocxl_hw.h | 1 -
> 2 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index 7018cd802569..59452850f71c 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -15,7 +15,8 @@
> #include <linux/pseudo_fs.h>
> #include <linux/poll.h>
> #include <linux/sched/signal.h>
> -
> +#include <linux/interrupt.h>
> +#include <asm/xive.h>
> #include <misc/ocxl.h>
>
> #include <uapi/misc/cxl.h>
> @@ -180,7 +181,7 @@ static int afu_map_irq(u64 flags, struct ocxlflash_context *ctx, int num,
> struct ocxl_hw_afu *afu = ctx->hw_afu;
> struct device *dev = afu->dev;
> struct ocxlflash_irqs *irq;
> - void __iomem *vtrig;
> + struct xive_irq_data *xd;
> u32 virq;
> int rc = 0;
>
> @@ -204,15 +205,14 @@ static int afu_map_irq(u64 flags, struct ocxlflash_context *ctx, int num,
> goto err1;
> }
>
> - vtrig = ioremap(irq->ptrig, PAGE_SIZE);
> - if (unlikely(!vtrig)) {
> - dev_err(dev, "%s: Trigger page mapping failed\n", __func__);
> - rc = -ENOMEM;
> + xd = irq_get_handler_data(virq);
> + if (unlikely(!xd)) {
> + dev_err(dev, "%s: Can't get interrupt data\n", __func__);
The removal of setting the return code injects a bug should this error leg
ever be encountered. So we should either keep the rc statement e.g. -EINVAL,
-ENXIO, -ENODEV, etc., or remove this error leg. I lean towards keeping the
statement.
> goto err2;
> }
>
> irq->virq = virq;
> - irq->vtrig = vtrig;
> + irq->vtrig = xd->trig_mmio;
> out:
>
^ permalink raw reply
* Re: [PATCH v5 1/4] powerpc/papr_scm: Fetch nvdimm health information from PHYP
From: Michael Ellerman @ 2020-04-02 23:53 UTC (permalink / raw)
To: Vaibhav Jain, Vaibhav Jain, linuxppc-dev, linux-nvdimm
Cc: Aneesh Kumar K . V, Jeff Moyer, alastair, Oliver O'Halloran,
Vishal Verma, Vaibhav Jain, Dan Williams
In-Reply-To: <87o8s9g2nv.fsf@vajain21.in.ibm.com>
Vaibhav Jain <vajain21@vajain21.in.ibm.com.in.ibm.com> writes:
> Thanks for reviewing this patch Mpe,
> Michael Ellerman <ellerman@au1.ibm.com> writes:
>> Vaibhav Jain <vaibhav@linux.ibm.com> writes:
...
>>
>>> + /* Check for various masks in bitmap and set the buffer */
>>> + if (health & PAPR_SCM_DIMM_UNARMED_MASK)
>>> + rc += sprintf(buf, "not_armed ");
>>
>> I know buf is "big enough" but using sprintf() in 2020 is a bit ... :)
>>
>> seq_buf is a pretty thin wrapper over a buffer you can use to make this
>> cleaner and also handles overflow for you.
>>
>> See eg. show_user_instructions() for an example.
>
> Unfortunatly seq_buf_printf() is still not an exported symbol hence not
> usable in external modules.
Send a patch? :)
cheers
^ permalink raw reply
* Re: [PATCH v4 03/16] powerpc: Use a datatype for instructions
From: Alistair Popple @ 2020-04-02 23:44 UTC (permalink / raw)
To: Jordan Niethe; +Cc: Nicholas Piggin, Balamuruhan S, linuxppc-dev, Daniel Axtens
In-Reply-To: <CACzsE9rrEx9YaqiLt=yQReQjK=EEdv0Fw7=VUYE6nU8-0RFHZg@mail.gmail.com>
On Thursday, 2 April 2020 10:52:37 AM AEDT Jordan Niethe wrote:
> On Wed, Apr 1, 2020 at 9:32 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
> > On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> > > Currently unsigned ints are used to represent instructions on powerpc.
> > > This has worked well as instructions have always been 4 byte words.
> > > However, a future ISA version will introduce some changes to
> > > instructions that mean this scheme will no longer work as well. This
> > > change is Prefixed Instructions. A prefixed instruction is made up of a
> > > word prefix followed by a word suffix to make an 8 byte double word
> > > instruction. No matter the endianess of the system the prefix always
> > > comes first. Prefixed instructions are only planned for powerpc64.
> > >
> > > Introduce a ppc_inst type to represent both prefixed and word
> > > instructions on powerpc64 while keeping it possible to exclusively have
> > > word instructions on powerpc32, A latter patch will expand the type to
> > > include prefixed instructions but for now just typedef it to a u32.
> > >
> > > Later patches will introduce helper functions and macros for
> > > manipulating the instructions so that powerpc64 and powerpc32 might
> > > maintain separate type definitions.
> > >
> > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > > ---
> > >
> > > arch/powerpc/include/asm/code-patching.h | 31 +++++------
> > > arch/powerpc/include/asm/inst.h | 53 +++++++++++++++++++
> > > arch/powerpc/include/asm/sstep.h | 5 +-
> > > arch/powerpc/kernel/align.c | 2 +-
> > > arch/powerpc/kernel/hw_breakpoint.c | 3 +-
> > > arch/powerpc/kernel/kprobes.c | 2 +-
> > > arch/powerpc/kernel/mce_power.c | 5 +-
> > > arch/powerpc/kernel/optprobes.c | 10 ++--
> > > arch/powerpc/kernel/trace/ftrace.c | 66 ++++++++++++------------
> > > arch/powerpc/kvm/emulate_loadstore.c | 1 +
> > > arch/powerpc/lib/code-patching.c | 54 +++++++++----------
> > > arch/powerpc/lib/sstep.c | 4 +-
> > > arch/powerpc/lib/test_emulate_step.c | 9 ++--
> > > arch/powerpc/xmon/xmon.c | 12 ++---
> > > 14 files changed, 160 insertions(+), 97 deletions(-)
> > > create mode 100644 arch/powerpc/include/asm/inst.h
> > >
> > > diff --git a/arch/powerpc/include/asm/code-patching.h
> > > b/arch/powerpc/include/asm/code-patching.h
> > > index 898b54262881..cb5106f92d67 100644
> > > --- a/arch/powerpc/include/asm/code-patching.h
> > > +++ b/arch/powerpc/include/asm/code-patching.h
> > > @@ -11,6 +11,7 @@
> > >
> > > #include <linux/string.h>
> > > #include <linux/kallsyms.h>
> > > #include <asm/asm-compat.h>
> > >
> > > +#include <asm/inst.h>
> > >
> > > /* Flags for create_branch:
> > > * "b" == create_branch(addr, target, 0);
> > >
> > > @@ -22,27 +23,27 @@
> > >
> > > #define BRANCH_ABSOLUTE 0x2
> > >
> > > bool is_offset_in_branch_range(long offset);
> > >
> > > -unsigned int create_branch(const unsigned int *addr,
> > > +ppc_inst create_branch(const ppc_inst *addr,
> > >
> > > unsigned long target, int flags);
> > >
> > > -unsigned int create_cond_branch(const unsigned int *addr,
> > > +unsigned int create_cond_branch(const ppc_inst *addr,
> > >
> > > unsigned long target, int flags);
> > >
> > > -int patch_branch(unsigned int *addr, unsigned long target, int flags);
> > > -int patch_instruction(unsigned int *addr, unsigned int instr);
> > > -int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> > > +int patch_branch(ppc_inst *addr, unsigned long target, int flags);
> > > +int patch_instruction(ppc_inst *addr, ppc_inst instr);
> >
> > we need to handle this change for its user in epapr_paravirt.c,
Seeing a similar issue in kgdb.c:
In file included from /linux/include/linux/kgdb.h:20,
from /linux/arch/powerpc/kernel/kgdb.c:18:
/linux/arch/powerpc/kernel/kgdb.c: In function 'kgdb_arch_set_breakpoint':
/linux/arch/powerpc/include/asm/kgdb.h:30:22: error: incompatible type for argument 2 of 'patch_instruction'
#define BREAK_INSTR 0x7d821008 /* twge r2, r2 */
^~~~~~~~~~
/linux/arch/powerpc/kernel/kgdb.c:427:32: note: in expansion of macro 'BREAK_INSTR'
err = patch_instruction(addr, BREAK_INSTR);
^~~~~~~~~~~
In file included from /linux/arch/powerpc/kernel/kgdb.c:27:
/linux/arch/powerpc/include/asm/code-patching.h:31:44: note: expected 'ppc_inst' {aka 'struct ppc_inst'} but argument is of type 'int'
int patch_instruction(void *addr, ppc_inst instr);
~~~~~~~~~^~~~~
/linux/arch/powerpc/kernel/kgdb.c: In function 'kgdb_arch_remove_breakpoint':
/linux/arch/powerpc/kernel/kgdb.c:442:32: error: incompatible type for argument 2 of 'patch_instruction'
err = patch_instruction(addr, instr);
^~~~~
In file included from /linux/arch/powerpc/kernel/kgdb.c:27:
/linux/arch/powerpc/include/asm/code-patching.h:31:44: note: expected 'ppc_inst' {aka 'struct ppc_inst'} but argument is of type 'unsigned int'
int patch_instruction(void *addr, ppc_inst instr);
~~~~~~~~~^~~~~
make[3]: *** [/linux/scripts/Makefile.build:267: arch/powerpc/kernel/kgdb.o] Error 1
- Alistair
> Thanks, good catch.
>
> > arch/powerpc/kernel/epapr_paravirt.c: In function
> > 'early_init_dt_scan_epapr': arch/powerpc/kernel/epapr_paravirt.c:40:48:
> > error: incompatible type for argument 2 of 'patch_instruction'
> >
> > 40 | patch_instruction(epapr_hypercall_start + i, inst);
> >
> > | ^~~~
> > |
> > | u32 {aka unsigned
> > | int}
> >
> > In file included from arch/powerpc/kernel/epapr_paravirt.c:12:
> > ./arch/powerpc/include/asm/code-patching.h:31:44: note: expected
> > 'ppc_inst'
> > {aka 'struct ppc_inst'} but argument is of type 'u32' {aka 'unsigned int'}
> >
> > 31 | int patch_instruction(void *addr, ppc_inst instr);
> >
> > | ~~~~~~~~~^~~~~
> >
> > make[2]: *** [scripts/Makefile.build:268:
> > arch/powerpc/kernel/epapr_paravirt.o] Error 1
> > make[1]: *** [scripts/Makefile.build:505: arch/powerpc/kernel] Error 2
> > make: *** [Makefile:1683: arch/powerpc] Error 2
> >
> >
> > -- Bala
> >
> > > +int raw_patch_instruction(ppc_inst *addr, ppc_inst instr);
> > >
> > > static inline unsigned long patch_site_addr(s32 *site)
> > > {
> > >
> > > return (unsigned long)site + *site;
> > >
> > > }
> > >
> > > -static inline int patch_instruction_site(s32 *site, unsigned int instr)
> > > +static inline int patch_instruction_site(s32 *site, ppc_inst instr)
> > >
> > > {
> > >
> > > - return patch_instruction((unsigned int *)patch_site_addr(site),
> > > instr); + return patch_instruction((ppc_inst
> > > *)patch_site_addr(site), instr);> >
> > > }
> > >
> > > static inline int patch_branch_site(s32 *site, unsigned long target,
> > > int
> > >
> > > flags)
> > >
> > > {
> > >
> > > - return patch_branch((unsigned int *)patch_site_addr(site), target,
> > > flags);
> > > + return patch_branch((ppc_inst *)patch_site_addr(site), target,
> > > flags);> >
> > > }
> > >
> > > static inline int modify_instruction(unsigned int *addr, unsigned int
> > > clr,
> > >
> > > @@ -56,13 +57,13 @@ static inline int modify_instruction_site(s32 *site,
> > > unsigned int clr, unsigned
> > >
> > > return modify_instruction((unsigned int *)patch_site_addr(site),
> > > clr,
> > >
> > > set);
> > >
> > > }
> > >
> > > -int instr_is_relative_branch(unsigned int instr);
> > > -int instr_is_relative_link_branch(unsigned int instr);
> > > -int instr_is_branch_to_addr(const unsigned int *instr, unsigned long
> > > addr); -unsigned long branch_target(const unsigned int *instr);
> > > -unsigned int translate_branch(const unsigned int *dest,
> > > - const unsigned int *src);
> > > -extern bool is_conditional_branch(unsigned int instr);
> > > +int instr_is_relative_branch(ppc_inst instr);
> > > +int instr_is_relative_link_branch(ppc_inst instr);
> > > +int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr);
> > > +unsigned long branch_target(const ppc_inst *instr);
> > > +ppc_inst translate_branch(const ppc_inst *dest,
> > > + const ppc_inst *src);
> > > +extern bool is_conditional_branch(ppc_inst instr);
> > >
> > > #ifdef CONFIG_PPC_BOOK3E_64
> > > void __patch_exception(int exc, unsigned long addr);
> > > #define patch_exception(exc, name) do { \
> > >
> > > diff --git a/arch/powerpc/include/asm/inst.h
> > > b/arch/powerpc/include/asm/inst.h
> > > new file mode 100644
> > > index 000000000000..7c8596ee411e
> > > --- /dev/null
> > > +++ b/arch/powerpc/include/asm/inst.h
> > > @@ -0,0 +1,53 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +#ifndef _ASM_INST_H
> > > +#define _ASM_INST_H
> > > +
> > > +/*
> > > + * Instruction data type for POWER
> > > + */
> > > +
> > > +typedef u32 ppc_inst;
> > > +
> > > +#define PPC_INST(x) (x)
> > > +
> > > +static inline int ppc_inst_len(ppc_inst x)
> > > +{
> > > + return sizeof(ppc_inst);
> > > +}
> > > +
> > > +static inline int ppc_inst_opcode(ppc_inst x)
> > > +{
> > > + return x >> 26;
> > > +}
> > > +
> > > +static inline u32 ppc_inst_word(ppc_inst x)
> > > +{
> > > + return x;
> > > +}
> > > +
> > > +static inline ppc_inst ppc_inst_read(const ppc_inst *ptr)
> > > +{
> > > + return *(ppc_inst *)ptr;
> > > +}
> > > +
> > > +static inline void ppc_inst_write(void *ptr, ppc_inst x)
> > > +{
> > > + *(ppc_inst *)ptr = x;
> > > +}
> > > +
> > > +static inline bool ppc_inst_equal(ppc_inst x, ppc_inst y)
> > > +{
> > > + return x == y;
> > > +}
> > > +
> > > +static inline bool ppc_inst_null(ppc_inst x)
> > > +{
> > > + return x == 0;
> > > +}
> > > +
> > > +static inline u32 ppc_inst_mask(ppc_inst x, u32 mask)
> > > +{
> > > + return ppc_inst_word(x) & mask;
> > > +}
> > > +
> > > +#endif /* _ASM_INST_H */
> > > diff --git a/arch/powerpc/include/asm/sstep.h
> > > b/arch/powerpc/include/asm/sstep.h
> > > index 769f055509c9..9353916fcba7 100644
> > > --- a/arch/powerpc/include/asm/sstep.h
> > > +++ b/arch/powerpc/include/asm/sstep.h
> > > @@ -2,6 +2,7 @@
> > >
> > > /*
> > >
> > > * Copyright (C) 2004 Paul Mackerras <paulus@au.ibm.com>, IBM
> > > */
> > >
> > > +#include <asm/inst.h>
> > >
> > > struct pt_regs;
> > >
> > > @@ -132,7 +133,7 @@ union vsx_reg {
> > >
> > > * otherwise.
> > > */
> > >
> > > extern int analyse_instr(struct instruction_op *op, const struct
> > > pt_regs
> > >
> > > *regs,
> > > - unsigned int instr);
> > > + ppc_inst instr);
> > >
> > > /*
> > >
> > > * Emulate an instruction that can be executed just by updating
> > >
> > > @@ -149,7 +150,7 @@ void emulate_update_regs(struct pt_regs *reg, struct
> > > instruction_op *op);
> > >
> > > * 0 if it could not be emulated, or -1 for an instruction that
> > > * should not be emulated (rfid, mtmsrd clearing MSR_RI, etc.).
> > > */
> > >
> > > -extern int emulate_step(struct pt_regs *regs, unsigned int instr);
> > > +extern int emulate_step(struct pt_regs *regs, ppc_inst instr);
> > >
> > > /*
> > >
> > > * Emulate a load or store instruction by reading/writing the
> > >
> > > diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> > > index 92045ed64976..34594aaa44de 100644
> > > --- a/arch/powerpc/kernel/align.c
> > > +++ b/arch/powerpc/kernel/align.c
> > > @@ -293,7 +293,7 @@ static int emulate_spe(struct pt_regs *regs,
> > > unsigned int reg,
> > >
> > > int fix_alignment(struct pt_regs *regs)
> > > {
> > >
> > > - unsigned int instr;
> > > + ppc_inst instr;
> > >
> > > struct instruction_op op;
> > > int r, type;
> > >
> > > diff --git a/arch/powerpc/kernel/hw_breakpoint.c
> > > b/arch/powerpc/kernel/hw_breakpoint.c
> > > index 2462cd7c565c..06b97353d231 100644
> > > --- a/arch/powerpc/kernel/hw_breakpoint.c
> > > +++ b/arch/powerpc/kernel/hw_breakpoint.c
> > > @@ -24,6 +24,7 @@
> > >
> > > #include <asm/debug.h>
> > > #include <asm/debugfs.h>
> > > #include <asm/hvcall.h>
> > >
> > > +#include <asm/inst.h>
> > >
> > > #include <linux/uaccess.h>
> > >
> > > /*
> > >
> > > @@ -243,7 +244,7 @@ dar_range_overlaps(unsigned long dar, int size,
> > > struct
> > > arch_hw_breakpoint *info)
> > >
> > > static bool stepping_handler(struct pt_regs *regs, struct perf_event
> > > *bp,
> > >
> > > struct arch_hw_breakpoint *info)
> > >
> > > {
> > >
> > > - unsigned int instr = 0;
> > > + ppc_inst instr = 0;
> > >
> > > int ret, type, size;
> > > struct instruction_op op;
> > > unsigned long addr = info->address;
> > >
> > > diff --git a/arch/powerpc/kernel/kprobes.c
> > > b/arch/powerpc/kernel/kprobes.c
> > > index 337516df17d4..e7205adc9820 100644
> > > --- a/arch/powerpc/kernel/kprobes.c
> > > +++ b/arch/powerpc/kernel/kprobes.c
> > > @@ -225,7 +225,7 @@ NOKPROBE_SYMBOL(arch_prepare_kretprobe);
> > >
> > > static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
> > > {
> > >
> > > int ret;
> > >
> > > - unsigned int insn = *p->ainsn.insn;
> > > + ppc_inst insn = *p->ainsn.insn;
> > >
> > > /* regs->nip is also adjusted if emulate_step returns 1 */
> > > ret = emulate_step(regs, insn);
> > >
> > > diff --git a/arch/powerpc/kernel/mce_power.c
> > > b/arch/powerpc/kernel/mce_power.c
> > > index 1cbf7f1a4e3d..e65616bb3a3e 100644
> > > --- a/arch/powerpc/kernel/mce_power.c
> > > +++ b/arch/powerpc/kernel/mce_power.c
> > > @@ -20,6 +20,7 @@
> > >
> > > #include <asm/sstep.h>
> > > #include <asm/exception-64s.h>
> > > #include <asm/extable.h>
> > >
> > > +#include <asm/inst.h>
> > >
> > > /*
> > >
> > > * Convert an address related to an mm to a PFN. NOTE: we are in real
> > >
> > > @@ -365,7 +366,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs
> > > *regs, uint64_t *addr,
> > >
> > > * in real-mode is tricky and can lead to recursive
> > > * faults
> > > */
> > >
> > > - int instr;
> > > + ppc_inst instr;
> > >
> > > unsigned long pfn, instr_addr;
> > > struct instruction_op op;
> > > struct pt_regs tmp = *regs;
> > >
> > > @@ -373,7 +374,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs
> > > *regs, uint64_t *addr,
> > >
> > > pfn = addr_to_pfn(regs, regs->nip);
> > > if (pfn != ULONG_MAX) {
> > >
> > > instr_addr = (pfn << PAGE_SHIFT) + (regs->nip &
> > > ~PAGE_MASK);
> > >
> > > - instr = *(unsigned int *)(instr_addr);
> > > + instr = *(ppc_inst *)(instr_addr);
> > >
> > > if (!analyse_instr(&op, &tmp, instr)) {
> > >
> > > pfn = addr_to_pfn(regs, op.ea);
> > > *addr = op.ea;
> > >
> > > diff --git a/arch/powerpc/kernel/optprobes.c
> > > b/arch/powerpc/kernel/optprobes.c
> > > index 024f7aad1952..f5e8cce438a3 100644
> > > --- a/arch/powerpc/kernel/optprobes.c
> > > +++ b/arch/powerpc/kernel/optprobes.c
> > > @@ -189,8 +189,8 @@ void patch_imm64_load_insns(unsigned long val,
> > > kprobe_opcode_t *addr)
> > >
> > > int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct
> > > kprobe> >
> > > *p)
> > >
> > > {
> > >
> > > - kprobe_opcode_t *buff, branch_op_callback, branch_emulate_step;
> > > - kprobe_opcode_t *op_callback_addr, *emulate_step_addr;
> > > + ppc_inst branch_op_callback, branch_emulate_step;
> > > + kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> > >
> > > long b_offset;
> > > unsigned long nip, size;
> > > int rc, i;
> > >
> > > @@ -251,11 +251,11 @@ int arch_prepare_optimized_kprobe(struct
> > > optimized_kprobe *op, struct kprobe *p)
> > >
> > > goto error;
> > >
> > > }
> > >
> > > - branch_op_callback = create_branch((unsigned int *)buff +
> > > TMPL_CALL_HDLR_IDX,
> > > + branch_op_callback = create_branch((ppc_inst *)buff +
> > > TMPL_CALL_HDLR_IDX,
> > >
> > > (unsigned long)op_callback_addr,
> > > BRANCH_SET_LINK);
> > >
> > > - branch_emulate_step = create_branch((unsigned int *)buff +
> > > TMPL_EMULATE_IDX,
> > > + branch_emulate_step = create_branch((ppc_inst *)buff +
> > > TMPL_EMULATE_IDX,
> > >
> > > (unsigned long)emulate_step_addr,
> > > BRANCH_SET_LINK);
> > >
> > > @@ -316,7 +316,7 @@ void arch_optimize_kprobes(struct list_head *oplist)
> > >
> > > memcpy(op->optinsn.copied_insn, op->kp.addr,
> > >
> > > RELATIVEJUMP_SIZE);
> > >
> > > patch_instruction(op->kp.addr,
> > >
> > > - create_branch((unsigned int *)op->kp.addr,
> > > + create_branch((ppc_inst *)op->kp.addr,
> > >
> > > (unsigned long)op->optinsn.insn,
> > > 0));
> > >
> > > list_del_init(&op->list);
> > >
> > > }
> > >
> > > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > > b/arch/powerpc/kernel/trace/ftrace.c
> > > index 7ea0ca044b65..5787ccffb4df 100644
> > > --- a/arch/powerpc/kernel/trace/ftrace.c
> > > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > > @@ -27,6 +27,7 @@
> > >
> > > #include <asm/code-patching.h>
> > > #include <asm/ftrace.h>
> > > #include <asm/syscall.h>
> > >
> > > +#include <asm/inst.h>
> > >
> > > #ifdef CONFIG_DYNAMIC_FTRACE
> > >
> > > @@ -40,23 +41,23 @@
> > >
> > > #define NUM_FTRACE_TRAMPS 8
> > > static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
> > >
> > > -static unsigned int
> > > +static ppc_inst
> > >
> > > ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
> > > {
> > >
> > > - unsigned int op;
> > > + ppc_inst op;
> > >
> > > addr = ppc_function_entry((void *)addr);
> > >
> > > /* if (link) set op to 'bl' else 'b' */
> > >
> > > - op = create_branch((unsigned int *)ip, addr, link ? 1 : 0);
> > > + op = create_branch((ppc_inst *)ip, addr, link ? 1 : 0);
> > >
> > > return op;
> > >
> > > }
> > >
> > > static int
> > >
> > > -ftrace_modify_code(unsigned long ip, unsigned int old, unsigned int
> > > new)
> > > +ftrace_modify_code(unsigned long ip, ppc_inst old, ppc_inst new)
> > >
> > > {
> > >
> > > - unsigned int replaced;
> > > + ppc_inst replaced;
> > >
> > > /*
> > >
> > > * Note:
> > > @@ -78,7 +79,7 @@ ftrace_modify_code(unsigned long ip, unsigned int old,
> > > unsigned int new)
> > >
> > > }
> > >
> > > /* replace the text with the new text */
> > >
> > > - if (patch_instruction((unsigned int *)ip, new))
> > > + if (patch_instruction((ppc_inst *)ip, new))
> > >
> > > return -EPERM;
> > >
> > > return 0;
> > >
> > > @@ -87,25 +88,25 @@ ftrace_modify_code(unsigned long ip, unsigned int
> > > old,
> > > unsigned int new)
> > >
> > > /*
> > >
> > > * Helper functions that are the same for both PPC64 and PPC32.
> > > */
> > >
> > > -static int test_24bit_addr(unsigned long ip, unsigned long addr)
> > > +static ppc_inst test_24bit_addr(unsigned long ip, unsigned long addr)
> > >
> > > {
> > >
> > > addr = ppc_function_entry((void *)addr);
> > >
> > > /* use the create_branch to verify that this offset can be
> > > branched */
> > >
> > > - return create_branch((unsigned int *)ip, addr, 0);
> > > + return create_branch((ppc_inst *)ip, addr, 0);
> > >
> > > }
> > >
> > > -static int is_bl_op(unsigned int op)
> > > +static int is_bl_op(ppc_inst op)
> > >
> > > {
> > >
> > > return (op & 0xfc000003) == 0x48000001;
> > >
> > > }
> > >
> > > -static int is_b_op(unsigned int op)
> > > +static int is_b_op(ppc_inst op)
> > >
> > > {
> > >
> > > return (op & 0xfc000003) == 0x48000000;
> > >
> > > }
> > >
> > > -static unsigned long find_bl_target(unsigned long ip, unsigned int op)
> > > +static unsigned long find_bl_target(unsigned long ip, ppc_inst op)
> > >
> > > {
> > >
> > > int offset;
> > >
> > > @@ -125,7 +126,7 @@ __ftrace_make_nop(struct module *mod,
> > >
> > > {
> > >
> > > unsigned long entry, ptr, tramp;
> > > unsigned long ip = rec->ip;
> > >
> > > - unsigned int op, pop;
> > > + ppc_inst op, pop;
> > >
> > > /* read where this goes */
> > > if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
> > >
> > > @@ -204,7 +205,7 @@ __ftrace_make_nop(struct module *mod,
> > >
> > > }
> > >
> > > #endif /* CONFIG_MPROFILE_KERNEL */
> > >
> > > - if (patch_instruction((unsigned int *)ip, pop)) {
> > > + if (patch_instruction((ppc_inst *)ip, pop)) {
> > >
> > > pr_err("Patching NOP failed.\n");
> > > return -EPERM;
> > >
> > > }
> > >
> > > @@ -217,7 +218,7 @@ static int
> > >
> > > __ftrace_make_nop(struct module *mod,
> > >
> > > struct dyn_ftrace *rec, unsigned long addr)
> > >
> > > {
> > >
> > > - unsigned int op;
> > > + ppc_inst op;
> > >
> > > unsigned int jmp[4];
> > > unsigned long ip = rec->ip;
> > > unsigned long tramp;
> > >
> > > @@ -276,7 +277,7 @@ __ftrace_make_nop(struct module *mod,
> > >
> > > op = PPC_INST_NOP;
> > >
> > > - if (patch_instruction((unsigned int *)ip, op))
> > > + if (patch_instruction((ppc_inst *)ip, op))
> > >
> > > return -EPERM;
> > >
> > > return 0;
> > >
> > > @@ -322,7 +323,8 @@ static int add_ftrace_tramp(unsigned long tramp)
> > >
> > > */
> > >
> > > static int setup_mcount_compiler_tramp(unsigned long tramp)
> > > {
> > >
> > > - int i, op;
> > > + int i;
> > > + ppc_inst op;
> > >
> > > unsigned long ptr;
> > > static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
> > >
> > > @@ -388,7 +390,7 @@ static int setup_mcount_compiler_tramp(unsigned long
> > > tramp)
> > >
> > > static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned
> > > long
> > >
> > > addr)
> > >
> > > {
> > >
> > > unsigned long tramp, ip = rec->ip;
> > >
> > > - unsigned int op;
> > > + ppc_inst op;
> > >
> > > /* Read where this goes */
> > > if (probe_kernel_read(&op, (void *)ip, sizeof(int))) {
> > >
> > > @@ -416,7 +418,7 @@ static int __ftrace_make_nop_kernel(struct
> > > dyn_ftrace
> > > *rec, unsigned long addr)
> > >
> > > }
> > >
> > > }
> > >
> > > - if (patch_instruction((unsigned int *)ip, PPC_INST_NOP)) {
> > > + if (patch_instruction((ppc_inst *)ip, PPC_INST_NOP)) {
> > >
> > > pr_err("Patching NOP failed.\n");
> > > return -EPERM;
> > >
> > > }
> > >
> > > @@ -428,7 +430,7 @@ int ftrace_make_nop(struct module *mod,
> > >
> > > struct dyn_ftrace *rec, unsigned long addr)
> > >
> > > {
> > >
> > > unsigned long ip = rec->ip;
> > >
> > > - unsigned int old, new;
> > > + ppc_inst old, new;
> > >
> > > /*
> > >
> > > * If the calling address is more that 24 bits away,
> > >
> > > @@ -481,7 +483,7 @@ int ftrace_make_nop(struct module *mod,
> > >
> > > */
> > >
> > > #ifndef CONFIG_MPROFILE_KERNEL
> > > static int
> > >
> > > -expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1)
> > > +expected_nop_sequence(void *ip, ppc_inst op0, ppc_inst op1)
> > >
> > > {
> > >
> > > /*
> > >
> > > * We expect to see:
> > > @@ -510,7 +512,7 @@ expected_nop_sequence(void *ip, unsigned int op0,
> > > unsigned int op1)
> > >
> > > static int
> > > __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > > {
> > >
> > > - unsigned int op[2];
> > > + ppc_inst op[2];
> > >
> > > void *ip = (void *)rec->ip;
> > > unsigned long entry, ptr, tramp;
> > > struct module *mod = rec->arch.mod;
> > >
> > > @@ -574,7 +576,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > long addr)
> > >
> > > static int
> > > __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > > {
> > >
> > > - unsigned int op;
> > > + ppc_inst op;
> > >
> > > unsigned long ip = rec->ip;
> > >
> > > /* read where this goes */
> > >
> > > @@ -594,7 +596,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > long addr)
> > >
> > > }
> > >
> > > /* create the branch to the trampoline */
> > >
> > > - op = create_branch((unsigned int *)ip,
> > > + op = create_branch((ppc_inst *)ip,
> > >
> > > rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
> > >
> > > if (!op) {
> > >
> > > pr_err("REL24 out of range!\n");
> > >
> > > @@ -613,7 +615,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned
> > > long addr)
> > >
> > > static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned
> > > long
> > >
> > > addr)
> > >
> > > {
> > >
> > > - unsigned int op;
> > > + ppc_inst op;
> > >
> > > void *ip = (void *)rec->ip;
> > > unsigned long tramp, entry, ptr;
> > >
> > > @@ -661,7 +663,7 @@ static int __ftrace_make_call_kernel(struct
> > > dyn_ftrace
> > > *rec, unsigned long addr)
> > >
> > > int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > > {
> > >
> > > unsigned long ip = rec->ip;
> > >
> > > - unsigned int old, new;
> > > + ppc_inst old, new;
> > >
> > > /*
> > >
> > > * If the calling address is more that 24 bits away,
> > >
> > > @@ -700,7 +702,7 @@ static int
> > >
> > > __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
> > >
> > > unsigned long addr)
> > >
> > > {
> > >
> > > - unsigned int op;
> > > + ppc_inst op;
> > >
> > > unsigned long ip = rec->ip;
> > > unsigned long entry, ptr, tramp;
> > > struct module *mod = rec->arch.mod;
> > >
> > > @@ -748,7 +750,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec,
> > > unsigned
> > > long old_addr,
> > >
> > > /* The new target may be within range */
> > > if (test_24bit_addr(ip, addr)) {
> > >
> > > /* within range */
> > >
> > > - if (patch_branch((unsigned int *)ip, addr,
> > > BRANCH_SET_LINK)) { + if (patch_branch((ppc_inst *)ip,
> > > addr, BRANCH_SET_LINK)) {> >
> > > pr_err("REL24 out of range!\n");
> > > return -EINVAL;
> > >
> > > }
> > >
> > > @@ -776,7 +778,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec,
> > > unsigned
> > > long old_addr,
> > >
> > > }
> > >
> > > /* Ensure branch is within 24 bits */
> > >
> > > - if (!create_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
> > > + if (!create_branch((ppc_inst *)ip, tramp, BRANCH_SET_LINK)) {
> > >
> > > pr_err("Branch out of range\n");
> > > return -EINVAL;
> > >
> > > }
> > >
> > > @@ -794,7 +796,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec,
> > > unsigned
> > > long old_addr,
> > >
> > > unsigned long addr)
> > >
> > > {
> > >
> > > unsigned long ip = rec->ip;
> > >
> > > - unsigned int old, new;
> > > + ppc_inst old, new;
> > >
> > > /*
> > >
> > > * If the calling address is more that 24 bits away,
> > >
> > > @@ -834,7 +836,7 @@ int ftrace_modify_call(struct dyn_ftrace *rec,
> > > unsigned
> > > long old_addr,
> > >
> > > int ftrace_update_ftrace_func(ftrace_func_t func)
> > > {
> > >
> > > unsigned long ip = (unsigned long)(&ftrace_call);
> > >
> > > - unsigned int old, new;
> > > + ppc_inst old, new;
> > >
> > > int ret;
> > >
> > > old = *(unsigned int *)&ftrace_call;
> > >
> > > @@ -919,7 +921,7 @@ int ftrace_enable_ftrace_graph_caller(void)
> > >
> > > unsigned long ip = (unsigned long)(&ftrace_graph_call);
> > > unsigned long addr = (unsigned long)(&ftrace_graph_caller);
> > > unsigned long stub = (unsigned long)(&ftrace_graph_stub);
> > >
> > > - unsigned int old, new;
> > > + ppc_inst old, new;
> > >
> > > old = ftrace_call_replace(ip, stub, 0);
> > > new = ftrace_call_replace(ip, addr, 0);
> > >
> > > @@ -932,7 +934,7 @@ int ftrace_disable_ftrace_graph_caller(void)
> > >
> > > unsigned long ip = (unsigned long)(&ftrace_graph_call);
> > > unsigned long addr = (unsigned long)(&ftrace_graph_caller);
> > > unsigned long stub = (unsigned long)(&ftrace_graph_stub);
> > >
> > > - unsigned int old, new;
> > > + ppc_inst old, new;
> > >
> > > old = ftrace_call_replace(ip, addr, 0);
> > > new = ftrace_call_replace(ip, stub, 0);
> > >
> > > diff --git a/arch/powerpc/kvm/emulate_loadstore.c
> > > b/arch/powerpc/kvm/emulate_loadstore.c
> > > index 1139bc56e004..1c9bcbfeb924 100644
> > > --- a/arch/powerpc/kvm/emulate_loadstore.c
> > > +++ b/arch/powerpc/kvm/emulate_loadstore.c
> > > @@ -21,6 +21,7 @@
> > >
> > > #include <asm/disassemble.h>
> > > #include <asm/ppc-opcode.h>
> > > #include <asm/sstep.h>
> > >
> > > +#include <asm/inst.h>
> > >
> > > #include "timing.h"
> > > #include "trace.h"
> > >
> > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> > > patching.c
> > > index 3345f039a876..8492b9e2b8db 100644
> > > --- a/arch/powerpc/lib/code-patching.c
> > > +++ b/arch/powerpc/lib/code-patching.c
> > > @@ -17,9 +17,10 @@
> > >
> > > #include <asm/page.h>
> > > #include <asm/code-patching.h>
> > > #include <asm/setup.h>
> > >
> > > +#include <asm/inst.h>
> > >
> > > -static int __patch_instruction(unsigned int *exec_addr, unsigned int
> > > instr, - unsigned int *patch_addr)
> > > +static int __patch_instruction(ppc_inst *exec_addr, ppc_inst instr,
> > > + ppc_inst *patch_addr)
> > >
> > > {
> > >
> > > int err = 0;
> > >
> > > @@ -33,7 +34,7 @@ static int __patch_instruction(unsigned int
> > > *exec_addr,
> > > unsigned int instr,
> > >
> > > return 0;
> > >
> > > }
> > >
> > > -int raw_patch_instruction(unsigned int *addr, unsigned int instr)
> > > +int raw_patch_instruction(ppc_inst *addr, ppc_inst instr)
> > >
> > > {
> > >
> > > return __patch_instruction(addr, instr, addr);
> > >
> > > }
> > >
> > > @@ -136,10 +137,10 @@ static inline int unmap_patch_area(unsigned long
> > > addr)> >
> > > return 0;
> > >
> > > }
> > >
> > > -static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> > > +static int do_patch_instruction(ppc_inst *addr, ppc_inst instr)
> > >
> > > {
> > >
> > > int err;
> > >
> > > - unsigned int *patch_addr = NULL;
> > > + ppc_inst *patch_addr = NULL;
> > >
> > > unsigned long flags;
> > > unsigned long text_poke_addr;
> > > unsigned long kaddr = (unsigned long)addr;
> > >
> > > @@ -176,7 +177,7 @@ static int do_patch_instruction(unsigned int *addr,
> > > unsigned int instr)
> > >
> > > }
> > > #else /* !CONFIG_STRICT_KERNEL_RWX */
> > >
> > > -static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> > > +static int do_patch_instruction(ppc_inst *addr, ppc_inst instr)
> > >
> > > {
> > >
> > > return raw_patch_instruction(addr, instr);
> > >
> > > }
> > >
> > > @@ -194,7 +195,7 @@ int patch_instruction(unsigned int *addr, unsigned
> > > int
> > > instr)
> > >
> > > }
> > > NOKPROBE_SYMBOL(patch_instruction);
> > >
> > > -int patch_branch(unsigned int *addr, unsigned long target, int flags)
> > > +int patch_branch(ppc_inst *addr, unsigned long target, int flags)
> > >
> > > {
> > >
> > > return patch_instruction(addr, create_branch(addr, target,
> > > flags));
> > >
> > > }
> > >
> > > @@ -225,7 +226,7 @@ bool is_offset_in_branch_range(long offset)
> > >
> > > * Helper to check if a given instruction is a conditional branch
> > > * Derived from the conditional checks in analyse_instr()
> > > */
> > >
> > > -bool is_conditional_branch(unsigned int instr)
> > > +bool is_conditional_branch(ppc_inst instr)
> > >
> > > {
> > >
> > > unsigned int opcode = instr >> 26;
> > >
> > > @@ -243,10 +244,10 @@ bool is_conditional_branch(unsigned int instr)
> > >
> > > }
> > > NOKPROBE_SYMBOL(is_conditional_branch);
> > >
> > > -unsigned int create_branch(const unsigned int *addr,
> > > +ppc_inst create_branch(const ppc_inst *addr,
> > >
> > > unsigned long target, int flags)
> > >
> > > {
> > >
> > > - unsigned int instruction;
> > > + ppc_inst instruction;
> > >
> > > long offset;
> > >
> > > offset = target;
> > >
> > > @@ -266,7 +267,7 @@ unsigned int create_branch(const unsigned int *addr,
> > >
> > > unsigned int create_cond_branch(const unsigned int *addr,
> > >
> > > unsigned long target, int flags)
> > >
> > > {
> > >
> > > - unsigned int instruction;
> > > + ppc_inst instruction;
> > >
> > > long offset;
> > >
> > > offset = target;
> > >
> > > @@ -283,22 +284,22 @@ unsigned int create_cond_branch(const unsigned int
> > > *addr,
> > >
> > > return instruction;
> > >
> > > }
> > >
> > > -static unsigned int branch_opcode(unsigned int instr)
> > > +static unsigned int branch_opcode(ppc_inst instr)
> > >
> > > {
> > >
> > > return (instr >> 26) & 0x3F;
> > >
> > > }
> > >
> > > -static int instr_is_branch_iform(unsigned int instr)
> > > +static int instr_is_branch_iform(ppc_inst instr)
> > >
> > > {
> > >
> > > return branch_opcode(instr) == 18;
> > >
> > > }
> > >
> > > -static int instr_is_branch_bform(unsigned int instr)
> > > +static int instr_is_branch_bform(ppc_inst instr)
> > >
> > > {
> > >
> > > return branch_opcode(instr) == 16;
> > >
> > > }
> > >
> > > -int instr_is_relative_branch(unsigned int instr)
> > > +int instr_is_relative_branch(ppc_inst instr)
> > >
> > > {
> > >
> > > if (instr & BRANCH_ABSOLUTE)
> > >
> > > return 0;
> > >
> > > @@ -306,12 +307,12 @@ int instr_is_relative_branch(unsigned int instr)
> > >
> > > return instr_is_branch_iform(instr) ||
> > > instr_is_branch_bform(instr);
> > >
> > > }
> > >
> > > -int instr_is_relative_link_branch(unsigned int instr)
> > > +int instr_is_relative_link_branch(ppc_inst instr)
> > >
> > > {
> > >
> > > return instr_is_relative_branch(instr) && (instr &
> > > BRANCH_SET_LINK);
> > >
> > > }
> > >
> > > -static unsigned long branch_iform_target(const unsigned int *instr)
> > > +static unsigned long branch_iform_target(const ppc_inst *instr)
> > >
> > > {
> > >
> > > signed long imm;
> > >
> > > @@ -327,7 +328,7 @@ static unsigned long branch_iform_target(const
> > > unsigned
> > > int *instr)
> > >
> > > return (unsigned long)imm;
> > >
> > > }
> > >
> > > -static unsigned long branch_bform_target(const unsigned int *instr)
> > > +static unsigned long branch_bform_target(const ppc_inst *instr)
> > >
> > > {
> > >
> > > signed long imm;
> > >
> > > @@ -343,7 +344,7 @@ static unsigned long branch_bform_target(const
> > > unsigned
> > > int *instr)
> > >
> > > return (unsigned long)imm;
> > >
> > > }
> > >
> > > -unsigned long branch_target(const unsigned int *instr)
> > > +unsigned long branch_target(const ppc_inst *instr)
> > >
> > > {
> > >
> > > if (instr_is_branch_iform(*instr))
> > >
> > > return branch_iform_target(instr);
> > >
> > > @@ -353,7 +354,7 @@ unsigned long branch_target(const unsigned int
> > > *instr)
> > >
> > > return 0;
> > >
> > > }
> > >
> > > -int instr_is_branch_to_addr(const unsigned int *instr, unsigned long
> > > addr)
> > > +int instr_is_branch_to_addr(const ppc_inst *instr, unsigned long addr)
> > >
> > > {
> > >
> > > if (instr_is_branch_iform(*instr) ||
> > > instr_is_branch_bform(*instr))
> > >
> > > return branch_target(instr) == addr;
> > >
> > > @@ -361,7 +362,7 @@ int instr_is_branch_to_addr(const unsigned int
> > > *instr,
> > > unsigned long addr)
> > >
> > > return 0;
> > >
> > > }
> > >
> > > -unsigned int translate_branch(const unsigned int *dest, const unsigned
> > > int
> > > *src)
> > > +ppc_inst translate_branch(const ppc_inst *dest, const ppc_inst *src)
> > >
> > > {
> > >
> > > unsigned long target;
> > >
> > > @@ -403,7 +404,7 @@ static void __init test_trampoline(void)
> > >
> > > static void __init test_branch_iform(void)
> > > {
> > >
> > > - unsigned int instr;
> > > + ppc_inst instr;
> > >
> > > unsigned long addr;
> > >
> > > addr = (unsigned long)&instr;
> > >
> > > @@ -478,11 +479,11 @@ static void __init test_branch_iform(void)
> > >
> > > static void __init test_create_function_call(void)
> > > {
> > >
> > > - unsigned int *iptr;
> > > + ppc_inst *iptr;
> > >
> > > unsigned long dest;
> > >
> > > /* Check we can create a function call */
> > >
> > > - iptr = (unsigned int *)ppc_function_entry(test_trampoline);
> > > + iptr = (ppc_inst *)ppc_function_entry(test_trampoline);
> > >
> > > dest = ppc_function_entry(test_create_function_call);
> > > patch_instruction(iptr, create_branch(iptr, dest,
> > > BRANCH_SET_LINK));
> > > check(instr_is_branch_to_addr(iptr, dest));
> > >
> > > @@ -491,7 +492,8 @@ static void __init test_create_function_call(void)
> > >
> > > static void __init test_branch_bform(void)
> > > {
> > >
> > > unsigned long addr;
> > >
> > > - unsigned int *iptr, instr, flags;
> > > + ppc_inst *iptr, instr;
> > > + unsigned int flags;
> > >
> > > iptr = &instr;
> > > addr = (unsigned long)iptr;
> > >
> > > @@ -561,7 +563,7 @@ static void __init test_branch_bform(void)
> > >
> > > static void __init test_translate_branch(void)
> > > {
> > >
> > > unsigned long addr;
> > >
> > > - unsigned int *p, *q;
> > > + ppc_inst *p, *q;
> > >
> > > void *buf;
> > >
> > > buf = vmalloc(PAGE_ALIGN(0x2000000 + 1));
> > >
> > > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > > index c077acb983a1..1d9c766a89fe 100644
> > > --- a/arch/powerpc/lib/sstep.c
> > > +++ b/arch/powerpc/lib/sstep.c
> > > @@ -1163,7 +1163,7 @@ static nokprobe_inline int trap_compare(long v1,
> > > long
> > > v2)
> > >
> > > * otherwise.
> > > */
> > >
> > > int analyse_instr(struct instruction_op *op, const struct pt_regs
> > > *regs,
> > >
> > > - unsigned int instr)
> > > + ppc_inst instr)
> > >
> > > {
> > >
> > > unsigned int opcode, ra, rb, rc, rd, spr, u;
> > > unsigned long int imm;
> > >
> > > @@ -3101,7 +3101,7 @@ NOKPROBE_SYMBOL(emulate_loadstore);
> > >
> > > * or -1 if the instruction is one that should not be stepped,
> > > * such as an rfid, or a mtmsrd that would clear MSR_RI.
> > > */
> > >
> > > -int emulate_step(struct pt_regs *regs, unsigned int instr)
> > > +int emulate_step(struct pt_regs *regs, ppc_inst instr)
> > >
> > > {
> > >
> > > struct instruction_op op;
> > > int r, err, type;
> > >
> > > diff --git a/arch/powerpc/lib/test_emulate_step.c
> > > b/arch/powerpc/lib/test_emulate_step.c
> > > index 42347067739c..158efc8a0f53 100644
> > > --- a/arch/powerpc/lib/test_emulate_step.c
> > > +++ b/arch/powerpc/lib/test_emulate_step.c
> > > @@ -460,7 +460,7 @@ struct compute_test {
> > >
> > > struct {
> > >
> > > char *descr;
> > > unsigned long flags;
> > >
> > > - unsigned int instr;
> > > + ppc_inst instr;
> > >
> > > struct pt_regs regs;
> > >
> > > } subtests[MAX_SUBTESTS + 1];
> > >
> > > };
> > >
> > > @@ -841,7 +841,7 @@ static struct compute_test compute_tests[] = {
> > >
> > > };
> > >
> > > static int __init emulate_compute_instr(struct pt_regs *regs,
> > >
> > > - unsigned int instr)
> > > + ppc_inst instr)
> > >
> > > {
> > >
> > > struct instruction_op op;
> > >
> > > @@ -859,7 +859,7 @@ static int __init emulate_compute_instr(struct
> > > pt_regs
> > > *regs,
> > >
> > > }
> > >
> > > static int __init execute_compute_instr(struct pt_regs *regs,
> > >
> > > - unsigned int instr)
> > > + ppc_inst instr)
> > >
> > > {
> > >
> > > extern int exec_instr(struct pt_regs *regs);
> > > extern s32 patch__exec_instr;
> > >
> > > @@ -890,7 +890,8 @@ static void __init run_tests_compute(void)
> > >
> > > unsigned long flags;
> > > struct compute_test *test;
> > > struct pt_regs *regs, exp, got;
> > >
> > > - unsigned int i, j, k, instr;
> > > + unsigned int i, j, k;
> > > + ppc_inst instr;
> > >
> > > bool ignore_gpr, ignore_xer, ignore_ccr, passed;
> > >
> > > for (i = 0; i < ARRAY_SIZE(compute_tests); i++) {
> > >
> > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > > index 7875d1a37770..a0bc442f9557 100644
> > > --- a/arch/powerpc/xmon/xmon.c
> > > +++ b/arch/powerpc/xmon/xmon.c
> > > @@ -892,7 +892,7 @@ static struct bpt *new_breakpoint(unsigned long a)
> > >
> > > static void insert_bpts(void)
> > > {
> > >
> > > int i;
> > >
> > > - unsigned int instr;
> > > + ppc_inst instr;
> > >
> > > struct bpt *bp;
> > >
> > > bp = bpts;
> > >
> > > @@ -914,7 +914,7 @@ static void insert_bpts(void)
> > >
> > > patch_instruction(bp->instr, instr);
> > > if (bp->enabled & BP_CIABR)
> > >
> > > continue;
> > >
> > > - if (patch_instruction((unsigned int *)bp->address,
> > > + if (patch_instruction((ppc_inst *)bp->address,
> > >
> > > bpinstr) != 0) {
> > >
> > > printf("Couldn't write instruction at %lx, "
> > >
> > > "disabling breakpoint there\n",
> > > bp->address);
> > >
> > > @@ -943,7 +943,7 @@ static void remove_bpts(void)
> > >
> > > {
> > >
> > > int i;
> > > struct bpt *bp;
> > >
> > > - unsigned instr;
> > > + ppc_inst instr;
> > >
> > > bp = bpts;
> > > for (i = 0; i < NBPTS; ++i, ++bp) {
> > >
> > > @@ -952,7 +952,7 @@ static void remove_bpts(void)
> > >
> > > if (mread(bp->address, &instr, 4) == 4
> > >
> > > && instr == bpinstr
> > > && patch_instruction(
> > >
> > > - (unsigned int *)bp->address, bp->instr[0]) != 0)
> > > + (ppc_inst *)bp->address, bp->instr[0]) != 0)
> > >
> > > printf("Couldn't remove breakpoint at %lx\n",
> > >
> > > bp->address);
> > >
> > > }
> > >
> > > @@ -1159,7 +1159,7 @@ static int do_step(struct pt_regs *regs)
> > >
> > > */
> > >
> > > static int do_step(struct pt_regs *regs)
> > > {
> > >
> > > - unsigned int instr;
> > > + ppc_inst instr;
> > >
> > > int stepped;
> > >
> > > force_enable_xmon();
> > >
> > > @@ -1325,7 +1325,7 @@ csum(void)
> > >
> > > */
> > >
> > > static long check_bp_loc(unsigned long addr)
> > > {
> > >
> > > - unsigned int instr;
> > > + ppc_inst instr;
> > >
> > > addr &= ~3;
> > > if (!is_kernel_addr(addr)) {
^ permalink raw reply
* Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Leonardo Bras @ 2020-04-02 23:41 UTC (permalink / raw)
To: Oliver O'Halloran
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Claudio Carvalho,
Bharata B Rao, Paul Mackerras, Hari Bathini, Nathan Fontenot,
Thomas Gleixner, linuxppc-dev, Allison Randal
In-Reply-To: <CAOSf1CHSCMsQgJTcMPiRUFDxBF=WVpTRk7-bzyg6iit8bmm7rg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]
On Fri, 2020-04-03 at 10:31 +1100, Oliver O'Halloran wrote:
> On Fri, Apr 3, 2020 at 10:07 AM Leonardo Bras <leonardo@linux.ibm.com> wrote:
> > Hello Oliver, thank you for the feedback.
> > Comments inline:
> >
> > On Fri, 2020-04-03 at 09:46 +1100, Oliver O'Halloran wrote:
> > > I don't really understand why the flag is needed at all. According to
> > > PAPR any memory provided by dynamic reconfiguration can be hot-removed
> > > so why aren't we treating all DR memory as hot removable? The only
> > > memory guaranteed to be there 100% of the time is what's in the
> > > /memory@0 node since that's supposed to cover the real mode area.
> >
> > All LMBs are listed in DR memory, even the base memory.
> >
> > The v1 of the patch would work this way, as qemu would configure it's
> > DR memory with (DRC_INVALID | RESERVED) flags and the hot-added memory
> > with (ASSIGNED) flag. Looking for assigned flag would be enough.
> >
> > But as of today, PowerVM doesn't seem to work that way.
> > When you boot a PowerVM virtual machine with Linux, all memory is added
> > with the same flags (ASSIGNED).
> >
> > To create a solution that doesn't break PowerVM, this new flag was made
> > necessary.
>
> I'm still not convinced it's necessary. Why not check memory@0 and use
> the size as a clip level? Any memory above that level gets marked as
> hotpluggable and anything below doesn't. Seems to me that would work
> on all current platforms, so what am I missing here?
>
Humm, wouldn't that assume that all unmovable memory should be at the
first LMBs?
If we use the recently approved flag, there would be no such
limitation, and there would be possible to do some other things, like
hot-adding more unmovable memory to kernel usage.
What do you think?
Best regards,
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Oliver O'Halloran @ 2020-04-02 23:31 UTC (permalink / raw)
To: Leonardo Bras
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Claudio Carvalho,
Bharata B Rao, Paul Mackerras, Hari Bathini, Nathan Fontenot,
Thomas Gleixner, linuxppc-dev, Allison Randal
In-Reply-To: <6b4a4a0d4f7af723d0a5a12f4267717a507ce3f0.camel@linux.ibm.com>
On Fri, Apr 3, 2020 at 10:07 AM Leonardo Bras <leonardo@linux.ibm.com> wrote:
>
> Hello Oliver, thank you for the feedback.
> Comments inline:
>
> On Fri, 2020-04-03 at 09:46 +1100, Oliver O'Halloran wrote:
> >
> > I don't really understand why the flag is needed at all. According to
> > PAPR any memory provided by dynamic reconfiguration can be hot-removed
> > so why aren't we treating all DR memory as hot removable? The only
> > memory guaranteed to be there 100% of the time is what's in the
> > /memory@0 node since that's supposed to cover the real mode area.
>
> All LMBs are listed in DR memory, even the base memory.
>
> The v1 of the patch would work this way, as qemu would configure it's
> DR memory with (DRC_INVALID | RESERVED) flags and the hot-added memory
> with (ASSIGNED) flag. Looking for assigned flag would be enough.
>
> But as of today, PowerVM doesn't seem to work that way.
> When you boot a PowerVM virtual machine with Linux, all memory is added
> with the same flags (ASSIGNED).
>
> To create a solution that doesn't break PowerVM, this new flag was made
> necessary.
I'm still not convinced it's necessary. Why not check memory@0 and use
the size as a clip level? Any memory above that level gets marked as
hotpluggable and anything below doesn't. Seems to me that would work
on all current platforms, so what am I missing here?
>
> Best regards,
> Leonardo Bras
^ permalink raw reply
* Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Leonardo Bras @ 2020-04-02 23:06 UTC (permalink / raw)
To: Oliver O'Halloran
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Claudio Carvalho,
Bharata B Rao, Paul Mackerras, Hari Bathini, Nathan Fontenot,
Thomas Gleixner, linuxppc-dev, Allison Randal
In-Reply-To: <CAOSf1CEihBDDRny5S_7TA5Cqbdgsh6zN=kdq83OSgaLbRphtrg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]
Hello Oliver, thank you for the feedback.
Comments inline:
On Fri, 2020-04-03 at 09:46 +1100, Oliver O'Halloran wrote:
>
> I don't really understand why the flag is needed at all. According to
> PAPR any memory provided by dynamic reconfiguration can be hot-removed
> so why aren't we treating all DR memory as hot removable? The only
> memory guaranteed to be there 100% of the time is what's in the
> /memory@0 node since that's supposed to cover the real mode area.
All LMBs are listed in DR memory, even the base memory.
The v1 of the patch would work this way, as qemu would configure it's
DR memory with (DRC_INVALID | RESERVED) flags and the hot-added memory
with (ASSIGNED) flag. Looking for assigned flag would be enough.
But as of today, PowerVM doesn't seem to work that way.
When you boot a PowerVM virtual machine with Linux, all memory is added
with the same flags (ASSIGNED).
To create a solution that doesn't break PowerVM, this new flag was made
necessary.
Best regards,
Leonardo Bras
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v8 2/7] powerpc/kprobes: Mark newly allocated probes as RO
From: Russell Currey @ 2020-04-02 23:02 UTC (permalink / raw)
To: Naveen N. Rao, linuxppc-dev; +Cc: kernel-hardening, ajd, npiggin, dja
In-Reply-To: <1585852977.oiikywo1jz.naveen@linux.ibm.com>
On Fri, 2020-04-03 at 00:18 +0530, Naveen N. Rao wrote:
> Naveen N. Rao wrote:
> > Russell Currey wrote:
> > > With CONFIG_STRICT_KERNEL_RWX=y and CONFIG_KPROBES=y, there will
> > > be one
> > > W+X page at boot by default. This can be tested with
> > > CONFIG_PPC_PTDUMP=y and CONFIG_PPC_DEBUG_WX=y set, and checking
> > > the
> > > kernel log during boot.
> > >
> > > powerpc doesn't implement its own alloc() for kprobes like other
> > > architectures do, but we couldn't immediately mark RO anyway
> > > since we do
> > > a memcpy to the page we allocate later. After that, nothing
> > > should be
> > > allowed to modify the page, and write permissions are removed
> > > well
> > > before the kprobe is armed.
> > >
> > > The memcpy() would fail if >1 probes were allocated, so use
> > > patch_instruction() instead which is safe for RO.
> > >
> > > Reviewed-by: Daniel Axtens <dja@axtens.net>
> > > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > ---
> > > arch/powerpc/kernel/kprobes.c | 17 +++++++++++++----
> > > 1 file changed, 13 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/powerpc/kernel/kprobes.c
> > > b/arch/powerpc/kernel/kprobes.c
> > > index 81efb605113e..fa4502b4de35 100644
> > > --- a/arch/powerpc/kernel/kprobes.c
> > > +++ b/arch/powerpc/kernel/kprobes.c
> > > @@ -24,6 +24,8 @@
> > > #include <asm/sstep.h>
> > > #include <asm/sections.h>
> > > #include <linux/uaccess.h>
> > > +#include <linux/set_memory.h>
> > > +#include <linux/vmalloc.h>
> > >
> > > DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
> > > DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> > > @@ -102,6 +104,16 @@ kprobe_opcode_t *kprobe_lookup_name(const
> > > char *name, unsigned int offset)
> > > return addr;
> > > }
> > >
> > > +void *alloc_insn_page(void)
> > > +{
> > > + void *page = vmalloc_exec(PAGE_SIZE);
> > > +
> > > + if (page)
> > > + set_memory_ro((unsigned long)page, 1);
> > > +
> > > + return page;
> > > +}
> > > +
> >
> > This crashes for me with KPROBES_SANITY_TEST during the kretprobe
> > test.
>
> That isn't needed to reproduce this. After bootup, disabling
> optprobes
> also shows the crash with kretprobes:
> sysctl debug.kprobes-optimization=0
>
> The problem happens to be with patch_instruction() in
> arch_prepare_kprobe(). During boot, on kprobe init, we register a
> probe
> on kretprobe_trampoline for use with kretprobes (see
> arch_init_kprobes()). This results in an instruction slot being
> allocated, and arch_prepare_kprobe() to be called for copying the
> instruction (nop) at kretprobe_trampoline. patch_instruction() is
> failing resulting in corrupt instruction which we try to
> emulate/single
> step causing the crash.
Thanks a lot for the info Naveen, I'll investigate.
- Russell
>
>
> - Naveen
>
^ permalink raw reply
* Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Oliver O'Halloran @ 2020-04-02 22:46 UTC (permalink / raw)
To: Leonardo Bras
Cc: Linux Kernel Mailing List, Greg Kroah-Hartman, Claudio Carvalho,
Bharata B Rao, Paul Mackerras, Hari Bathini, Nathan Fontenot,
Thomas Gleixner, linuxppc-dev, Allison Randal
In-Reply-To: <20200402195156.626430-1-leonardo@linux.ibm.com>
On Fri, Apr 3, 2020 at 6:55 AM Leonardo Bras <leonardo@linux.ibm.com> wrote:
>
> While providing guests, it's desirable to resize it's memory on demand.
>
> By now, it's possible to do so by creating a guest with a small base
> memory, hot-plugging all the rest, and using 'movable_node' kernel
> command-line parameter, which puts all hot-plugged memory in
> ZONE_MOVABLE, allowing it to be removed whenever needed.
>
> But there is an issue regarding guest reboot:
> If memory is hot-plugged, and then the guest is rebooted, all hot-plugged
> memory goes to ZONE_NORMAL, which offers no guaranteed hot-removal.
> It usually prevents this memory to be hot-removed from the guest.
>
> It's possible to use device-tree information to fix that behavior, as
> it stores flags for LMB ranges on ibm,dynamic-memory-vN.
> It involves marking each memblock with the correct flags as hotpluggable
> memory, which mm/memblock.c puts in ZONE_MOVABLE during boot if
> 'movable_node' is passed.
I don't really understand why the flag is needed at all. According to
PAPR any memory provided by dynamic reconfiguration can be hot-removed
so why aren't we treating all DR memory as hot removable? The only
memory guaranteed to be there 100% of the time is what's in the
/memory@0 node since that's supposed to cover the real mode area.
^ permalink raw reply
* Re: [PATCH v8 0/7] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events
From: Jiri Olsa @ 2020-04-02 21:27 UTC (permalink / raw)
To: Kajol Jain
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, sukadev, ravi.bangoria, acme,
jmario, namhyung, tglx, mpetlan, gregkh, linux-kernel,
linux-perf-users, jolsa, linuxppc-dev
In-Reply-To: <20200401203340.31402-1-kjain@linux.ibm.com>
On Thu, Apr 02, 2020 at 02:03:33AM +0530, Kajol Jain wrote:
> Patchset adds json file metric support for the hv_24x7 socket/chip level
> events. "hv_24x7" pmu interface events needs system dependent parameter
> like socket/chip/core. For example, hv_24x7 chip level events needs
> specific chip-id to which the data is requested should be added as part
> of pmu events.
>
> So to enable JSON file support to "hv_24x7" interface, patchset reads
> total number of sockets details in sysfs under
> "/sys/devices/hv_24x7/interface/".
>
> Second patch of the patchset adds expr_scanner_ctx object to hold user
> data for the expr scanner, which can be used to hold runtime parameter.
>
> Patch 4 & 6 of the patchset handles perf tool plumbing needed to replace
> the "?" character in the metric expression to proper value and hv_24x7
> json metric file for different Socket/chip resources.
>
> Patch set also enable Hz/hz prinitg for --metric-only option to print
> metric data for bus frequency.
>
> Applied and tested all these patches cleanly on top of jiri's flex changes
> with the changes done by Kan Liang for "Support metric group constraint"
> patchset and made required changes.
>
> Also apply this patch on top of the fix patch send earlier
> for printing metric name incase overlapping events.
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=37cd7f65bf71a48f25eeb6d9be5dacb20d008ea6
>
> Changelog:
> v7 -> v8
> - Add test case for testing parsing of "?" in metric expression
> - Reaname variables name to runtime
Acked-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
^ permalink raw reply
* Re: WARNING in ext4_da_update_reserve_space
From: Murilo Opsfelder Araújo @ 2020-04-02 14:06 UTC (permalink / raw)
To: syzbot
Cc: tytso, mareklindner, sw, b.a.t.m.a.n, a, linux-kernel,
syzkaller-bugs, netdev, adilger.kernel, paulus, linux-ext4,
linuxppc-dev, davem
In-Reply-To: <0000000000008c5a4605a24cbb16@google.com>
On Thursday, April 2, 2020 8:02:11 AM -03 syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 1a147b74 Merge branch 'DSA-mtu'
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=14237713e00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=46ee14d4915944bc
> dashboard link: https://syzkaller.appspot.com/bug?extid=67e4f16db666b1c8253c
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12237713e00000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10ec7c97e00000
>
> The bug was bisected to:
>
> commit 658b0f92bc7003bc734471f61bf7cd56339eb8c3
> Author: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> Date: Wed Aug 1 21:33:15 2018 +0000
>
> powerpc/traps: Print unhandled signals in a separate function
This commit is specific to powerpc and the crash is from an x86_64 system.
There is a bunch of scp errors in the logs:
scp: ./syz-executor998635077: No space left on device
Is it possible that these errors might be misleading the syzbot?
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=15979f5be00000
> final crash: https://syzkaller.appspot.com/x/report.txt?x=17979f5be00000
> console output: https://syzkaller.appspot.com/x/log.txt?x=13979f5be00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+67e4f16db666b1c8253c@syzkaller.appspotmail.com
> Fixes: 658b0f92bc70 ("powerpc/traps: Print unhandled signals in a separate
> function")
>
> EXT4-fs warning (device sda1): ext4_da_update_reserve_space:344:
> ext4_da_update_reserve_space: ino 15722, used 1 with only 0 reserved data
> blocks ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 359 at fs/ext4/inode.c:348
> ext4_da_update_reserve_space+0x622/0x7d0 fs/ext4/inode.c:344 Kernel panic -
> not syncing: panic_on_warn set ...
> CPU: 1 PID: 359 Comm: kworker/u4:5 Not tainted 5.6.0-rc7-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011 Workqueue: writeback wb_workfn (flush-8:0)
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x188/0x20d lib/dump_stack.c:118
> panic+0x2e3/0x75c kernel/panic.c:221
> __warn.cold+0x2f/0x35 kernel/panic.c:582
> report_bug+0x27b/0x2f0 lib/bug.c:195
> fixup_bug arch/x86/kernel/traps.c:174 [inline]
> fixup_bug arch/x86/kernel/traps.c:169 [inline]
> do_error_trap+0x12b/0x220 arch/x86/kernel/traps.c:267
> do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
> invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
> RIP: 0010:ext4_da_update_reserve_space+0x622/0x7d0 fs/ext4/inode.c:348
> Code: 02 00 0f 85 94 01 00 00 48 8b 7d 28 49 c7 c0 20 72 3c 88 41 56 48 c7
> c1 80 60 3c 88 53 ba 58 01 00 00 4c 89 c6 e8 1e 6d 0d 00 <0f> 0b 48 b8 00
> 00 00 00 00 fc ff df 4c 89 ea 48 c1 ea 03 0f b6 04 RSP:
> 0018:ffffc90002197288 EFLAGS: 00010296
> RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffffff820bf066 RDI: fffff52000432e21
> RBP: ffff888086b744c8 R08: 0000000000000091 R09: ffffed1015ce6659
> R10: ffffed1015ce6658 R11: ffff8880ae7332c7 R12: 0000000000000001
> R13: ffff888086b74990 R14: 0000000000000000 R15: ffff888086b74a40
> ext4_ext_map_blocks+0x24aa/0x37d0 fs/ext4/extents.c:4500
> ext4_map_blocks+0x4cb/0x1650 fs/ext4/inode.c:622
> mpage_map_one_extent fs/ext4/inode.c:2365 [inline]
> mpage_map_and_submit_extent fs/ext4/inode.c:2418 [inline]
> ext4_writepages+0x19eb/0x3080 fs/ext4/inode.c:2772
> do_writepages+0xfa/0x2a0 mm/page-writeback.c:2344
> __writeback_single_inode+0x12a/0x1410 fs/fs-writeback.c:1452
> writeback_sb_inodes+0x515/0xdd0 fs/fs-writeback.c:1716
> wb_writeback+0x2a5/0xd90 fs/fs-writeback.c:1892
> wb_do_writeback fs/fs-writeback.c:2037 [inline]
> wb_workfn+0x339/0x11c0 fs/fs-writeback.c:2078
> process_one_work+0x94b/0x1690 kernel/workqueue.c:2266
> worker_thread+0x96/0xe20 kernel/workqueue.c:2412
> kthread+0x357/0x430 kernel/kthread.c:255
> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
--
Murilo
^ permalink raw reply
* Re: [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Linus Torvalds @ 2020-04-02 20:47 UTC (permalink / raw)
To: Kees Cook
Cc: linux-arch, Christian Borntraeger, Dave Airlie, Peter Anvin,
Linux Kernel Mailing List, Russell King, Linux-MM, Paul Mackerras,
Al Viro, Daniel Vetter, Andrew Morton, linuxppc-dev
In-Reply-To: <202004021322.5F80467@keescook>
On Thu, Apr 2, 2020 at 1:27 PM Kees Cook <keescook@chromium.org> wrote:
>
> I was just speaking to design principles in this area: if the "enable"
> is called when already enabled, Something Is Wrong. :)
Well, the "something is wrong" could easily be "the hardware does not
support this".
I'm not at all interested in the crazy code to do this in software.
Nobody sane should ever do that.
Yes, I realize that PaX did software emulation of things like that,
and it was one of the reasons why it was never useful to any normal
use.
Security is not an end goal in itself, it's always secondary to "can I
use this".
Security that means "normal people can't use this, it's only for the
special l33t users" is not security, it's garbage. That "do page
tables in software" was a prime example of garbage.
Linus
^ permalink raw reply
* Re: [PATCH v8 6/7] tools/perf: Enable Hz/hz prinitg for --metric-only option
From: Andi Kleen @ 2020-04-02 20:42 UTC (permalink / raw)
To: Jiri Olsa
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang,
alexander.shishkin, anju, mamatha4, sukadev, ravi.bangoria,
Kajol Jain, acme, jmario, namhyung, tglx, mpetlan, gregkh,
linux-kernel, linux-perf-users, jolsa, linuxppc-dev
In-Reply-To: <20200402124946.GH2518490@krava>
> > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> > index 9e757d18d713..679aaa655824 100644
> > --- a/tools/perf/util/stat-display.c
> > +++ b/tools/perf/util/stat-display.c
> > @@ -237,8 +237,6 @@ static bool valid_only_metric(const char *unit)
> > if (!unit)
> > return false;
> > if (strstr(unit, "/sec") ||
> > - strstr(unit, "hz") ||
> > - strstr(unit, "Hz") ||
>
> will this change output of --metric-only for some setups then?
>
> Andi, are you ok with this?
Yes should be ok
$ grep -i ScaleUnit.*hz arch/x86/*/*
$
Probably was for some metric we later dropped.
-Andi
^ permalink raw reply
* Re: [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Kees Cook @ 2020-04-02 20:27 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-arch, Christian Borntraeger, Dave Airlie, Peter Anvin,
Linux Kernel Mailing List, Russell King, Linux-MM, Paul Mackerras,
Al Viro, Daniel Vetter, Andrew Morton, linuxppc-dev
In-Reply-To: <CAHk-=wg9cSm=AjPmkasNHBDwuW4D10jszjv6EeCKp8V9Qbx2hg@mail.gmail.com>
On Thu, Apr 02, 2020 at 12:26:52PM -0700, Linus Torvalds wrote:
> On Thu, Apr 2, 2020 at 11:36 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > Yup, I think it's a weakness of the ARM implementation and I'd like to
> > not extend it further. AFAIK we should never nest, but I would not be
> > surprised at all if we did.
>
> Wel, at least the user_access_begin/end() sections can't nest. objtool
> verifies and warns about that on x86.
Right, yes, I mentioned that earlier in the thread. I meant I wasn't
100% sure about ARM's corner cases. I would _hope_ it doesn't.
> > If we were looking at a design goal for all architectures, I'd like
> > to be doing what the public PaX patchset
>
> We already do better than PaX ever did. Seriously. Mainline has long
> since passed their hacky garbage.
I was just speaking to design principles in this area: if the "enable"
is called when already enabled, Something Is Wrong. :) (And one thing
still missing in this general subject is that x86 still lacks SMAP
emulation. And yes, I understand it's just not been a priority for anyone
that can work on it, but it is still a gap.)
--
Kees Cook
^ permalink raw reply
* [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Leonardo Bras @ 2020-04-02 19:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Leonardo Bras, Thomas Gleixner, Nathan Fontenot,
Greg Kroah-Hartman, Allison Randal, Bharata B Rao,
Claudio Carvalho, Hari Bathini
Cc: linuxppc-dev, linux-kernel
While providing guests, it's desirable to resize it's memory on demand.
By now, it's possible to do so by creating a guest with a small base
memory, hot-plugging all the rest, and using 'movable_node' kernel
command-line parameter, which puts all hot-plugged memory in
ZONE_MOVABLE, allowing it to be removed whenever needed.
But there is an issue regarding guest reboot:
If memory is hot-plugged, and then the guest is rebooted, all hot-plugged
memory goes to ZONE_NORMAL, which offers no guaranteed hot-removal.
It usually prevents this memory to be hot-removed from the guest.
It's possible to use device-tree information to fix that behavior, as
it stores flags for LMB ranges on ibm,dynamic-memory-vN.
It involves marking each memblock with the correct flags as hotpluggable
memory, which mm/memblock.c puts in ZONE_MOVABLE during boot if
'movable_node' is passed.
For carrying such information, the new flag DRCONF_MEM_HOTREMOVABLE was
proposed and accepted into Power Architecture documentation.
This flag should be:
- true (b=1) if the hypervisor may want to hot-remove it later, and
- false (b=0) if it does not care.
During boot, guest kernel reads the device-tree, early_init_drmem_lmb()
is called for every added LMBs. Here, checking for this new flag and
marking memblocks as hotplugable memory is enough to get the desirable
behavior.
This should cause no change if 'movable_node' parameter is not passed
in kernel command-line.
Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
Reviewed-by: Bharata B Rao <bharata@linux.ibm.com>
---
Changes since v2:
- New flag name changed from DRCONF_MEM_HOTPLUGGED to
DRCONF_MEM_HOTREMOVABLE
Changes since v1:
- Adds new flag, so PowerVM is compatible with the change.
- Fixes mistakes in code
---
arch/powerpc/include/asm/drmem.h | 1 +
arch/powerpc/kernel/prom.c | 9 +++++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index 3d76e1c388c2..ad99e27e5b65 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -65,6 +65,7 @@ struct of_drconf_cell_v2 {
#define DRCONF_MEM_ASSIGNED 0x00000008
#define DRCONF_MEM_AI_INVALID 0x00000040
#define DRCONF_MEM_RESERVED 0x00000080
+#define DRCONF_MEM_HOTREMOVABLE 0x00000100
static inline u32 drmem_lmb_size(void)
{
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 6620f37abe73..abc9b04d03ce 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -515,9 +515,14 @@ static void __init early_init_drmem_lmb(struct drmem_lmb *lmb,
size = 0x80000000ul - base;
}
+ if (!validate_mem_limit(base, &size))
+ continue;
+
DBG("Adding: %llx -> %llx\n", base, size);
- if (validate_mem_limit(base, &size))
- memblock_add(base, size);
+ memblock_add(base, size);
+
+ if (lmb->flags & DRCONF_MEM_HOTREMOVABLE)
+ memblock_mark_hotplug(base, size);
} while (--rngs);
}
#endif /* CONFIG_PPC_PSERIES */
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] qtpm2: Export tpm2_get_cc_attrs_tbl for ibmvtpm driver as module
From: Jarkko Sakkinen @ 2020-04-02 19:31 UTC (permalink / raw)
To: Sachin Sant
Cc: Stefan Berger, LKML, linux-next, linux-integrity, linuxppc-dev,
Stefan Berger
In-Reply-To: <2BF66599-184A-4647-BC57-105A1512F119@linux.vnet.ibm.com>
On Wed, Apr 01, 2020 at 02:40:30PM +0530, Sachin Sant wrote:
>
>
> > On 20-Mar-2020, at 1:27 AM, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Wed, Mar 18, 2020 at 09:00:17PM -0400, Stefan Berger wrote:
> >> From: Stefan Berger <stefanb@linux.ibm.com>
> >>
> >> This patch fixes the following problem when the ibmvtpm driver
> >> is built as a module:
> >>
> >> ERROR: modpost: "tpm2_get_cc_attrs_tbl" [drivers/char/tpm/tpm_ibmvtpm.ko] undefined!
> >> make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 1
> >> make: *** [Makefile:1298: modules] Error 2
> >>
> >> Fixes: 18b3670d79ae ("tpm: ibmvtpm: Add support for TPM2")
> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> >> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> >> Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> >
>
> Ping. This failure can now be seen in mainline (cad18da0af) as well.
It is in my tree
/Jarkko
^ permalink raw reply
* Re: [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Linus Torvalds @ 2020-04-02 19:26 UTC (permalink / raw)
To: Kees Cook
Cc: linux-arch, Christian Borntraeger, Dave Airlie, Peter Anvin,
Linux Kernel Mailing List, Russell King, Linux-MM, Paul Mackerras,
Al Viro, Daniel Vetter, Andrew Morton, linuxppc-dev
In-Reply-To: <202004021132.813F8E88@keescook>
On Thu, Apr 2, 2020 at 11:36 AM Kees Cook <keescook@chromium.org> wrote:
>
> Yup, I think it's a weakness of the ARM implementation and I'd like to
> not extend it further. AFAIK we should never nest, but I would not be
> surprised at all if we did.
Wel, at least the user_access_begin/end() sections can't nest. objtool
verifies and warns about that on x86.
> If we were looking at a design goal for all architectures, I'd like
> to be doing what the public PaX patchset
We already do better than PaX ever did. Seriously. Mainline has long
since passed their hacky garbage.
Plus PaX and grsecurity should be actively shunned. Don't look at it,
don't use it, and tell everybody you know to not use that shit.
Linus
^ permalink raw reply
* Re: [PATCH v8 2/7] powerpc/kprobes: Mark newly allocated probes as RO
From: Naveen N. Rao @ 2020-04-02 18:48 UTC (permalink / raw)
To: linuxppc-dev, Russell Currey; +Cc: kernel-hardening, ajd, npiggin, dja
In-Reply-To: <1585844035.o235bvxmq0.naveen@linux.ibm.com>
Naveen N. Rao wrote:
> Russell Currey wrote:
>> With CONFIG_STRICT_KERNEL_RWX=y and CONFIG_KPROBES=y, there will be one
>> W+X page at boot by default. This can be tested with
>> CONFIG_PPC_PTDUMP=y and CONFIG_PPC_DEBUG_WX=y set, and checking the
>> kernel log during boot.
>>
>> powerpc doesn't implement its own alloc() for kprobes like other
>> architectures do, but we couldn't immediately mark RO anyway since we do
>> a memcpy to the page we allocate later. After that, nothing should be
>> allowed to modify the page, and write permissions are removed well
>> before the kprobe is armed.
>>
>> The memcpy() would fail if >1 probes were allocated, so use
>> patch_instruction() instead which is safe for RO.
>>
>> Reviewed-by: Daniel Axtens <dja@axtens.net>
>> Signed-off-by: Russell Currey <ruscur@russell.cc>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/kernel/kprobes.c | 17 +++++++++++++----
>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
>> index 81efb605113e..fa4502b4de35 100644
>> --- a/arch/powerpc/kernel/kprobes.c
>> +++ b/arch/powerpc/kernel/kprobes.c
>> @@ -24,6 +24,8 @@
>> #include <asm/sstep.h>
>> #include <asm/sections.h>
>> #include <linux/uaccess.h>
>> +#include <linux/set_memory.h>
>> +#include <linux/vmalloc.h>
>>
>> DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
>> DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
>> @@ -102,6 +104,16 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
>> return addr;
>> }
>>
>> +void *alloc_insn_page(void)
>> +{
>> + void *page = vmalloc_exec(PAGE_SIZE);
>> +
>> + if (page)
>> + set_memory_ro((unsigned long)page, 1);
>> +
>> + return page;
>> +}
>> +
>
> This crashes for me with KPROBES_SANITY_TEST during the kretprobe test.
That isn't needed to reproduce this. After bootup, disabling optprobes
also shows the crash with kretprobes:
sysctl debug.kprobes-optimization=0
The problem happens to be with patch_instruction() in
arch_prepare_kprobe(). During boot, on kprobe init, we register a probe
on kretprobe_trampoline for use with kretprobes (see
arch_init_kprobes()). This results in an instruction slot being
allocated, and arch_prepare_kprobe() to be called for copying the
instruction (nop) at kretprobe_trampoline. patch_instruction() is
failing resulting in corrupt instruction which we try to emulate/single
step causing the crash.
- Naveen
^ permalink raw reply
* Re: [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Kees Cook @ 2020-04-02 18:35 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, linuxppc-dev, Christian Borntraeger, airlied, hpa,
linux-kernel, Russell King, linux-mm, Paul Mackerras, daniel,
akpm, torvalds
In-Reply-To: <20200402175032.GH23230@ZenIV.linux.org.uk>
On Thu, Apr 02, 2020 at 06:50:32PM +0100, Al Viro wrote:
> On Thu, Apr 02, 2020 at 07:03:28PM +0200, Christophe Leroy wrote:
>
> > user_access_begin() grants both read and write.
> >
> > This patch adds user_read_access_begin() and user_write_access_begin() but
> > it doesn't remove user_access_begin()
>
> Ouch... So the most generic name is for the rarest case?
>
> > > What should we do about that? Do we prohibit such blocks outside
> > > of arch?
> > >
> > > What should we do about arm and s390? There we want a cookie passed
> > > from beginning of block to its end; should that be a return value?
> >
> > That was the way I implemented it in January, see
> > https://patchwork.ozlabs.org/patch/1227926/
> >
> > There was some discussion around that and most noticeable was:
> >
> > H. Peter (hpa) said about it: "I have *deep* concern with carrying state in
> > a "key" variable: it's a direct attack vector for a crowbar attack,
> > especially since it is by definition live inside a user access region."
>
> > This patch minimises the change by just adding user_read_access_begin() and
> > user_write_access_begin() keeping the same parameters as the existing
> > user_access_begin().
>
> Umm... What about the arm situation? The same concerns would apply there,
> wouldn't they? Currently we have
> static __always_inline unsigned int uaccess_save_and_enable(void)
> {
> #ifdef CONFIG_CPU_SW_DOMAIN_PAN
> unsigned int old_domain = get_domain();
>
> /* Set the current domain access to permit user accesses */
> set_domain((old_domain & ~domain_mask(DOMAIN_USER)) |
> domain_val(DOMAIN_USER, DOMAIN_CLIENT));
>
> return old_domain;
> #else
> return 0;
> #endif
> }
> and
> static __always_inline void uaccess_restore(unsigned int flags)
> {
> #ifdef CONFIG_CPU_SW_DOMAIN_PAN
> /* Restore the user access mask */
> set_domain(flags);
> #endif
> }
>
> How much do we need nesting on those, anyway? rmk?
Yup, I think it's a weakness of the ARM implementation and I'd like to
not extend it further. AFAIK we should never nest, but I would not be
surprised at all if we did.
If we were looking at a design goal for all architectures, I'd like
to be doing what the public PaX patchset did for their memory access
switching, which is to alarm if calling into "enable" found the access
already enabled, etc. Such a condition would show an unexpected nesting
(like we've seen with similar constructs with set_fs() not getting reset
during an exception handler, etc etc).
--
Kees Cook
^ permalink raw reply
* Re: [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Christophe Leroy @ 2020-04-02 18:35 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, linuxppc-dev, keescook, Christian Borntraeger,
airlied, hpa, linux-kernel, Russell King, linux-mm,
Paul Mackerras, daniel, akpm, torvalds
In-Reply-To: <20200402175032.GH23230@ZenIV.linux.org.uk>
Le 02/04/2020 à 19:50, Al Viro a écrit :
> On Thu, Apr 02, 2020 at 07:03:28PM +0200, Christophe Leroy wrote:
>
>> user_access_begin() grants both read and write.
>>
>> This patch adds user_read_access_begin() and user_write_access_begin() but
>> it doesn't remove user_access_begin()
>
> Ouch... So the most generic name is for the rarest case?
>
I can add another patch at the end of the series to rename
user_access_begin() to user_full_access_begin().
Would it suit you ?
Christophe
^ permalink raw reply
* Re: [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Al Viro @ 2020-04-02 17:50 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, linuxppc-dev, keescook, Christian Borntraeger,
airlied, hpa, linux-kernel, Russell King, linux-mm,
Paul Mackerras, daniel, akpm, torvalds
In-Reply-To: <67e21b65-0e2d-7ca5-7518-cec1b7abc46c@c-s.fr>
On Thu, Apr 02, 2020 at 07:03:28PM +0200, Christophe Leroy wrote:
> user_access_begin() grants both read and write.
>
> This patch adds user_read_access_begin() and user_write_access_begin() but
> it doesn't remove user_access_begin()
Ouch... So the most generic name is for the rarest case?
> > What should we do about that? Do we prohibit such blocks outside
> > of arch?
> >
> > What should we do about arm and s390? There we want a cookie passed
> > from beginning of block to its end; should that be a return value?
>
> That was the way I implemented it in January, see
> https://patchwork.ozlabs.org/patch/1227926/
>
> There was some discussion around that and most noticeable was:
>
> H. Peter (hpa) said about it: "I have *deep* concern with carrying state in
> a "key" variable: it's a direct attack vector for a crowbar attack,
> especially since it is by definition live inside a user access region."
> This patch minimises the change by just adding user_read_access_begin() and
> user_write_access_begin() keeping the same parameters as the existing
> user_access_begin().
Umm... What about the arm situation? The same concerns would apply there,
wouldn't they? Currently we have
static __always_inline unsigned int uaccess_save_and_enable(void)
{
#ifdef CONFIG_CPU_SW_DOMAIN_PAN
unsigned int old_domain = get_domain();
/* Set the current domain access to permit user accesses */
set_domain((old_domain & ~domain_mask(DOMAIN_USER)) |
domain_val(DOMAIN_USER, DOMAIN_CLIENT));
return old_domain;
#else
return 0;
#endif
}
and
static __always_inline void uaccess_restore(unsigned int flags)
{
#ifdef CONFIG_CPU_SW_DOMAIN_PAN
/* Restore the user access mask */
set_domain(flags);
#endif
}
How much do we need nesting on those, anyway? rmk?
^ permalink raw reply
* Re: [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Kees Cook @ 2020-04-02 17:38 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, linuxppc-dev, Christian Borntraeger, airlied, hpa,
linux-kernel, Russell King, linux-mm, Paul Mackerras, Al Viro,
daniel, akpm, torvalds
In-Reply-To: <67e21b65-0e2d-7ca5-7518-cec1b7abc46c@c-s.fr>
On Thu, Apr 02, 2020 at 07:03:28PM +0200, Christophe Leroy wrote:
> > What should we do about arm and s390? There we want a cookie passed
> > from beginning of block to its end; should that be a return value?
>
> That was the way I implemented it in January, see
> https://patchwork.ozlabs.org/patch/1227926/
>
> There was some discussion around that and most noticeable was:
>
> H. Peter (hpa) said about it: "I have *deep* concern with carrying state in
> a "key" variable: it's a direct attack vector for a crowbar attack,
> especially since it is by definition live inside a user access region."
I share this concern -- we want to keep user/kernel access as static as
possible. It should be provable with static analysis, etc (e.g. objtool
does this already for x86).
Since this doesn't disrupt existing R+W access, I'd prefer the design of
this series as-is.
--
Kees Cook
^ permalink raw reply
* Re: [RFC PATCH v2 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Leonardo Bras @ 2020-04-02 17:08 UTC (permalink / raw)
To: Bharata B Rao
Cc: linux-kernel@vger.kernel.org, Michael Anderson, Claudio Carvalho,
Mike Rapoport, Bharata B Rao, Paul Mackerras, Greg Kroah-Hartman,
Hari Bathini, Nathan Fontenot, Thomas Gleixner, linuxppc-dev,
Allison Randal
In-Reply-To: <CAGZKiBp7qjH1gMOzRuPgX=qcrJs4b7UgBbfxjgzAEpQPZ0nhHQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 540 bytes --]
Hello Bharata, thank you for reviewing and testing!
During review of this new flag, it was suggested to change it's name to
a better one (on platform's viewpoint).
So I will have to change the flag name from DRCONF_MEM_HOTPLUGGED to
DRCONF_MEM_HOTREMOVABLE.
Everything should work the same as today.
Best regards,
Leonardo
On Thu, 2020-04-02 at 14:44 +0530, Bharata B Rao wrote:
> Looks good to me, also tested with PowerKVM guests.
>
> Reviewed-by: Bharata B Rao <bharata@linux.ibm.com>
>
> Regards,
> Bharata.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Christophe Leroy @ 2020-04-02 17:03 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, linuxppc-dev, keescook, Christian Borntraeger,
airlied, hpa, linux-kernel, Russell King, linux-mm,
Paul Mackerras, daniel, akpm, torvalds
In-Reply-To: <20200402162942.GG23230@ZenIV.linux.org.uk>
Le 02/04/2020 à 18:29, Al Viro a écrit :
> On Thu, Apr 02, 2020 at 07:34:16AM +0000, Christophe Leroy wrote:
>> Some architectures like powerpc64 have the capability to separate
>> read access and write access protection.
>> For get_user() and copy_from_user(), powerpc64 only open read access.
>> For put_user() and copy_to_user(), powerpc64 only open write access.
>> But when using unsafe_get_user() or unsafe_put_user(),
>> user_access_begin open both read and write.
>>
>> Other architectures like powerpc book3s 32 bits only allow write
>> access protection. And on this architecture protection is an heavy
>> operation as it requires locking/unlocking per segment of 256Mbytes.
>> On those architecture it is therefore desirable to do the unlocking
>> only for write access. (Note that book3s/32 ranges from very old
>> powermac from the 90's with powerpc 601 processor, till modern
>> ADSL boxes with PowerQuicc II modern processors for instance so it
>> is still worth considering)
>>
>> In order to avoid any risk based of hacking some variable parameters
>> passed to user_access_begin/end that would allow hacking and
>> leaving user access open or opening too much, it is preferable to
>> use dedicated static functions that can't be overridden.
>>
>> Add a user_read_access_begin and user_read_access_end to only open
>> read access.
>>
>> Add a user_write_access_begin and user_write_access_end to only open
>> write access.
>>
>> By default, when undefined, those new access helpers default on the
>> existing user_access_begin and user_access_end.
>
> The only problem I have is that we'd better choose the calling
> conventions that work for other architectures as well.
>
> AFAICS, aside of ppc and x86 we have (at least) this:
> arm:
> unsigned int __ua_flags = uaccess_save_and_enable();
> ...
> uaccess_restore(__ua_flags);
> arm64:
> uaccess_enable_not_uao();
> ...
> uaccess_disable_not_uao();
> riscv:
> __enable_user_access();
> ...
> __disable_user_access();
> s390/mvc:
> old_fs = enable_sacf_uaccess();
> ...
> disable_sacf_uaccess(old_fs);
>
> arm64 and riscv are easy - they map well on what we have now.
> The interesting ones are ppc, arm and s390.
>
> You wants to specify the kind of access; OK, but... it's not just read
> vs. write - there's read-write as well. AFAICS, there are 3 users of
> that:
> * copy_in_user()
> * arch_futex_atomic_op_inuser()
> * futex_atomic_cmpxchg_inatomic()
> The former is of dubious utility (all users outside of arch are in
> the badly done compat code), but the other two are not going to go
> away.
user_access_begin() grants both read and write.
This patch adds user_read_access_begin() and user_write_access_begin()
but it doesn't remove user_access_begin()
>
> What should we do about that? Do we prohibit such blocks outside
> of arch?
>
> What should we do about arm and s390? There we want a cookie passed
> from beginning of block to its end; should that be a return value?
That was the way I implemented it in January, see
https://patchwork.ozlabs.org/patch/1227926/
There was some discussion around that and most noticeable was:
H. Peter (hpa) said about it: "I have *deep* concern with carrying state
in a "key" variable: it's a direct attack vector for a crowbar attack,
especially since it is by definition live inside a user access region."
>
> And at least on arm that thing nests (see e.g. __clear_user_memset()
> there), so "stash that cookie in current->something" is not a solution...
>
> Folks, let's sort that out while we still have few users of that
> interface; changing the calling conventions later will be much harder.
> Comments?
>
This patch minimises the change by just adding user_read_access_begin()
and user_write_access_begin() keeping the same parameters as the
existing user_access_begin().
So I can come back to a mix of this patch and the January version if it
corresponds to everyone's view, it will also be a bit easier for powerpc
(especially book3s/32). But that didn't seem to be the expected
direction back when we discussed it in January.
Christophe
^ permalink raw reply
* Re: [PATCH v2] sched/core: fix illegal RCU from offline CPUs
From: Paul E. McKenney @ 2020-04-02 16:57 UTC (permalink / raw)
To: Qian Cai
Cc: juri.lelli, James.Bottomley@hansenpartnership.com,
vincent.guittot, linux-parisc, Peter Zijlstra, deller,
Nicholas Piggin, linux-kernel, rostedt, bsegall, linux-mm,
Ingo Molnar, mgorman, tglx, linuxppc-dev, dietmar.eggemann
In-Reply-To: <4134872A-3D1D-4860-9C1B-2FD9C00272BB@lca.pw>
On Thu, Apr 02, 2020 at 12:19:54PM -0400, Qian Cai wrote:
>
>
> > On Apr 2, 2020, at 11:54 AM, Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > I do run this combination quite frequently, but only as part of
> > rcutorture, which might not be a representative workload. For one thing,
> > it has a minimal userspace consisting only of a trivial init program.
> > I don't recall having ever seen this. (I have seen one recent complaint
> > about an IPI being sent to an offline CPU, but I cannot prove that this
> > was not due to RCU bugs that I was chasing at the time.)
>
> Yes, a trivial init is tough while running systemd should be able to catch it as it will use cgroup.
Not planning to add systemd to my rcutorture runs. ;-)
Thanx, Paul
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox