LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 6/9] tty: hvc_console: Fix coding style issues of block comments
From: Johan Hovold @ 2021-05-20 13:52 UTC (permalink / raw)
  To: Xiaofei Tan; +Cc: gregkh, linuxppc-dev, jirislaby, linux-kernel, linuxarm
In-Reply-To: <7e63a708-64c4-b369-066b-7f83d65bf178@huawei.com>

On Thu, May 20, 2021 at 09:21:25PM +0800, Xiaofei Tan wrote:

> > Checkpatch already has too many checks IMO and I'm a bit surprised that
> > it doesn't check this already. Perhaps it's because you used the -f to
> > run checkpatch on in-kernel code, which you should not.
> >
> >>> Second, that sentence is not capitalised so why do add a period?
> >>>
> >>
> >> How about capitalize the sentence, or just remove the period ?
> >
> > How about just leaving this unchanged?
> 
> OK
> And I will keep the patch 8/9, and combine space issues into
> one new patch, and remove the others.

Yeah, 8/9 is arguably a fix even if it's for a very minor issue
(repeated words in a comment).

It doesn't look like any of the white space issues are worth fixing,
though. Such pedantry can usually be addressed when the code in question
is being modified for other reasons.

Johan

^ permalink raw reply

* [PATCH v2 12/12] powerpc/optprobes: use PPC_RAW_ macros
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

Use PPC_RAW_ macros to simplify the code.

And use PPC_LO/PPC_HI instead of IMM_L/IMM_H which are for
internal use inside ppc-opcode.h

Those macros are self explanatory, comments can go as well.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/ppc-opcode.h | 11 ++++----
 arch/powerpc/kernel/optprobes.c       | 39 +++++----------------------
 2 files changed, 12 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index ac41776661e9..87802e8073e6 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -261,9 +261,6 @@
 #define PPC_INST_BCTR			0x4e800420
 #define PPC_INST_BCTRL			0x4e800421
 #define PPC_INST_DIVD			0x7c0003d2
-#define PPC_INST_RLDICR			0x78000004
-#define PPC_INST_ORI			0x60000000
-#define PPC_INST_ORIS			0x64000000
 #define PPC_INST_BRANCH			0x48000000
 #define PPC_INST_BL			0x48000001
 #define PPC_INST_BRANCH_COND		0x40800000
@@ -323,6 +320,8 @@
 #define PPC_LO(v)	((v) & 0xffff)
 #define PPC_HI(v)	(((v) >> 16) & 0xffff)
 #define PPC_HA(v)	PPC_HI((v) + 0x8000)
+#define PPC_HIGHER(v)	(((v) >> 32) & 0xffff)
+#define PPC_HIGHEST(v)	(((v) >> 48) & 0xffff)
 
 /*
  * Only use the larx hint bit on 64bit CPUs. e500v1/v2 based CPUs will treat a
@@ -499,8 +498,8 @@
 #define PPC_RAW_AND_DOT(d, a, b)	(0x7c000039 | ___PPC_RA(d) | ___PPC_RS(a) | ___PPC_RB(b))
 #define PPC_RAW_OR(d, a, b)		(0x7c000378 | ___PPC_RA(d) | ___PPC_RS(a) | ___PPC_RB(b))
 #define PPC_RAW_MR(d, a)		PPC_RAW_OR(d, a, a)
-#define PPC_RAW_ORI(d, a, i)		(PPC_INST_ORI | ___PPC_RA(d) | ___PPC_RS(a) | IMM_L(i))
-#define PPC_RAW_ORIS(d, a, i)		(PPC_INST_ORIS | ___PPC_RA(d) | ___PPC_RS(a) | IMM_L(i))
+#define PPC_RAW_ORI(d, a, i)		(0x60000000 | ___PPC_RA(d) | ___PPC_RS(a) | IMM_L(i))
+#define PPC_RAW_ORIS(d, a, i)		(0x64000000 | ___PPC_RA(d) | ___PPC_RS(a) | IMM_L(i))
 #define PPC_RAW_NOR(d, a, b)		(0x7c0000f8 | ___PPC_RA(d) | ___PPC_RS(a) | ___PPC_RB(b))
 #define PPC_RAW_XOR(d, a, b)		(0x7c000278 | ___PPC_RA(d) | ___PPC_RS(a) | ___PPC_RB(b))
 #define PPC_RAW_XORI(d, a, i)		(0x68000000 | ___PPC_RA(d) | ___PPC_RS(a) | IMM_L(i))
@@ -519,7 +518,7 @@
 					(0x54000001 | ___PPC_RA(d) | ___PPC_RS(a) | __PPC_SH(i) | __PPC_MB(mb) | __PPC_ME(me))
 #define PPC_RAW_RLWIMI(d, a, i, mb, me) (0x50000000 | ___PPC_RA(d) | ___PPC_RS(a) | __PPC_SH(i) | __PPC_MB(mb) | __PPC_ME(me))
 #define PPC_RAW_RLDICL(d, a, i, mb)     (0x78000000 | ___PPC_RA(d) | ___PPC_RS(a) | __PPC_SH64(i) | __PPC_MB64(mb))
-#define PPC_RAW_RLDICR(d, a, i, me)     (PPC_INST_RLDICR | ___PPC_RA(d) | ___PPC_RS(a) | __PPC_SH64(i) | __PPC_ME64(me))
+#define PPC_RAW_RLDICR(d, a, i, me)     (0x78000004 | ___PPC_RA(d) | ___PPC_RS(a) | __PPC_SH64(i) | __PPC_ME64(me))
 
 /* slwi = rlwinm Rx, Ry, n, 0, 31-n */
 #define PPC_RAW_SLWI(d, a, i)		PPC_RAW_RLWINM(d, a, i, 0, 31-(i))
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 9c1c8de8c06d..1488d4e9b34d 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -137,10 +137,8 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
 
 static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
 {
-	patch_instruction(addr, ppc_inst(PPC_RAW_LIS(reg, IMM_H(val))));
-	addr++;
-
-	patch_instruction(addr, ppc_inst(PPC_RAW_ORI(reg, reg, IMM_L(val))));
+	patch_instruction(addr++, ppc_inst(PPC_RAW_LIS(reg, PPC_HI(val))));
+	patch_instruction(addr, ppc_inst(PPC_RAW_ORI(reg, reg, PPC_LO(val))));
 }
 
 /*
@@ -149,34 +147,11 @@ static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *
  */
 static void patch_imm64_load_insns(unsigned long long val, int reg, kprobe_opcode_t *addr)
 {
-	/* lis reg,(op)@highest */
-	patch_instruction(addr,
-			  ppc_inst(PPC_INST_ADDIS | ___PPC_RT(reg) |
-				   ((val >> 48) & 0xffff)));
-	addr++;
-
-	/* ori reg,reg,(op)@higher */
-	patch_instruction(addr,
-			  ppc_inst(PPC_INST_ORI | ___PPC_RA(reg) |
-				   ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
-	addr++;
-
-	/* rldicr reg,reg,32,31 */
-	patch_instruction(addr,
-			  ppc_inst(PPC_INST_RLDICR | ___PPC_RA(reg) |
-				   ___PPC_RS(reg) | __PPC_SH64(32) | __PPC_ME64(31)));
-	addr++;
-
-	/* oris reg,reg,(op)@h */
-	patch_instruction(addr,
-			  ppc_inst(PPC_INST_ORIS | ___PPC_RA(reg) |
-				   ___PPC_RS(reg) | ((val >> 16) & 0xffff)));
-	addr++;
-
-	/* ori reg,reg,(op)@l */
-	patch_instruction(addr,
-			  ppc_inst(PPC_INST_ORI | ___PPC_RA(reg) |
-				   ___PPC_RS(reg) | (val & 0xffff)));
+	patch_instruction(addr++, ppc_inst(PPC_RAW_LIS(reg, PPC_HIGHEST(val))));
+	patch_instruction(addr++, ppc_inst(PPC_RAW_ORI(reg, reg, PPC_HIGHER(val))));
+	patch_instruction(addr++, ppc_inst(PPC_RAW_SLDI(reg, reg, 32)));
+	patch_instruction(addr++, ppc_inst(PPC_RAW_ORIS(reg, reg, PPC_HI(val))));
+	patch_instruction(addr, ppc_inst(PPC_RAW_ORI(reg, reg, PPC_LO(val))));
 }
 
 static void patch_imm_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 11/12] powerpc/optprobes: Compact code source a bit.
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

Now that lines can be up to 100 chars long, minimise the
amount of split lines to increase readability.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/optprobes.c | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 8c08ca15faf3..9c1c8de8c06d 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -18,18 +18,12 @@
 #include <asm/ppc-opcode.h>
 #include <asm/inst.h>
 
-#define TMPL_CALL_HDLR_IDX	\
-	(optprobe_template_call_handler - optprobe_template_entry)
-#define TMPL_EMULATE_IDX	\
-	(optprobe_template_call_emulate - optprobe_template_entry)
-#define TMPL_RET_IDX		\
-	(optprobe_template_ret - optprobe_template_entry)
-#define TMPL_OP_IDX		\
-	(optprobe_template_op_address - optprobe_template_entry)
-#define TMPL_INSN_IDX		\
-	(optprobe_template_insn - optprobe_template_entry)
-#define TMPL_END_IDX		\
-	(optprobe_template_end - optprobe_template_entry)
+#define TMPL_CALL_HDLR_IDX	(optprobe_template_call_handler - optprobe_template_entry)
+#define TMPL_EMULATE_IDX	(optprobe_template_call_emulate - optprobe_template_entry)
+#define TMPL_RET_IDX		(optprobe_template_ret - optprobe_template_entry)
+#define TMPL_OP_IDX		(optprobe_template_op_address - optprobe_template_entry)
+#define TMPL_INSN_IDX		(optprobe_template_insn - optprobe_template_entry)
+#define TMPL_END_IDX		(optprobe_template_end - optprobe_template_entry)
 
 DEFINE_INSN_CACHE_OPS(ppc_optinsn);
 
@@ -280,8 +274,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 	 */
 	patch_branch(buff + TMPL_RET_IDX, nip, 0);
 
-	flush_icache_range((unsigned long)buff,
-			   (unsigned long)(&buff[TMPL_END_IDX]));
+	flush_icache_range((unsigned long)buff, (unsigned long)(&buff[TMPL_END_IDX]));
 
 	op->optinsn.insn = buff;
 
@@ -319,10 +312,8 @@ void arch_optimize_kprobes(struct list_head *oplist)
 		 * Backup instructions which will be replaced
 		 * by jump address
 		 */
-		memcpy(op->optinsn.copied_insn, op->kp.addr,
-					       RELATIVEJUMP_SIZE);
-		create_branch(&instr, op->kp.addr,
-			      (unsigned long)op->optinsn.insn, 0);
+		memcpy(op->optinsn.copied_insn, op->kp.addr, RELATIVEJUMP_SIZE);
+		create_branch(&instr, op->kp.addr, (unsigned long)op->optinsn.insn, 0);
 		patch_instruction(op->kp.addr, instr);
 		list_del_init(&op->list);
 	}
@@ -333,8 +324,7 @@ void arch_unoptimize_kprobe(struct optimized_kprobe *op)
 	arch_arm_kprobe(&op->kp);
 }
 
-void arch_unoptimize_kprobes(struct list_head *oplist,
-			     struct list_head *done_list)
+void arch_unoptimize_kprobes(struct list_head *oplist, struct list_head *done_list)
 {
 	struct optimized_kprobe *op;
 	struct optimized_kprobe *tmp;
@@ -345,8 +335,7 @@ void arch_unoptimize_kprobes(struct list_head *oplist,
 	}
 }
 
-int arch_within_optimized_kprobe(struct optimized_kprobe *op,
-				 unsigned long addr)
+int arch_within_optimized_kprobe(struct optimized_kprobe *op, unsigned long addr)
 {
 	return ((unsigned long)op->kp.addr <= addr &&
 		(unsigned long)op->kp.addr + RELATIVEJUMP_SIZE > addr);
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 10/12] powerpc/optprobes: Minimise casts
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

nip is already an unsigned long, no cast needed.

op_callback_addr and emulate_step_addr are kprobe_opcode_t *.
There value is obtained with ppc_kallsyms_lookup_name() which
returns 'unsigned long', and there values are used create_branch()
which expects 'unsigned long'. So change them to 'unsigned long'
to avoid casting them back and forth.

can_optimize() used p->addr several times as 'unsigned long'.
Use a local 'unsigned long' variable and avoid casting multiple times.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/optprobes.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index adaf31157f6d..8c08ca15faf3 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -66,6 +66,7 @@ static unsigned long can_optimize(struct kprobe *p)
 	struct pt_regs regs;
 	struct instruction_op op;
 	unsigned long nip = 0;
+	unsigned long addr = (unsigned long)p->addr;
 
 	/*
 	 * kprobe placed for kretprobe during boot time
@@ -73,7 +74,7 @@ static unsigned long can_optimize(struct kprobe *p)
 	 * So further checks can be skipped.
 	 */
 	if (p->addr == (kprobe_opcode_t *)&kretprobe_trampoline)
-		return (unsigned long)p->addr + sizeof(kprobe_opcode_t);
+		return addr + sizeof(kprobe_opcode_t);
 
 	/*
 	 * We only support optimizing kernel addresses, but not
@@ -81,11 +82,11 @@ static unsigned long can_optimize(struct kprobe *p)
 	 *
 	 * FIXME: Optimize kprobes placed in module addresses.
 	 */
-	if (!is_kernel_addr((unsigned long)p->addr))
+	if (!is_kernel_addr(addr))
 		return 0;
 
 	memset(&regs, 0, sizeof(struct pt_regs));
-	regs.nip = (unsigned long)p->addr;
+	regs.nip = addr;
 	regs.trap = 0x0;
 	regs.msr = MSR_KERNEL;
 
@@ -195,7 +196,8 @@ static void patch_imm_load_insns(unsigned long val, int reg, kprobe_opcode_t *ad
 int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 {
 	struct ppc_inst branch_op_callback, branch_emulate_step, temp;
-	kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
+	unsigned long op_callback_addr, emulate_step_addr;
+	kprobe_opcode_t *buff;
 	long b_offset;
 	unsigned long nip, size;
 	int rc, i;
@@ -225,8 +227,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 		goto error;
 
 	/* Check if the return address is also within 32MB range */
-	b_offset = (unsigned long)(buff + TMPL_RET_IDX) -
-			(unsigned long)nip;
+	b_offset = (unsigned long)(buff + TMPL_RET_IDX) - nip;
 	if (!is_offset_in_branch_range(b_offset))
 		goto error;
 
@@ -249,20 +250,18 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 	/*
 	 * 2. branch to optimized_callback() and emulate_step()
 	 */
-	op_callback_addr = (kprobe_opcode_t *)ppc_kallsyms_lookup_name("optimized_callback");
-	emulate_step_addr = (kprobe_opcode_t *)ppc_kallsyms_lookup_name("emulate_step");
+	op_callback_addr = ppc_kallsyms_lookup_name("optimized_callback");
+	emulate_step_addr = ppc_kallsyms_lookup_name("emulate_step");
 	if (!op_callback_addr || !emulate_step_addr) {
 		WARN(1, "Unable to lookup optimized_callback()/emulate_step()\n");
 		goto error;
 	}
 
 	rc = create_branch(&branch_op_callback, buff + TMPL_CALL_HDLR_IDX,
-			   (unsigned long)op_callback_addr,
-			   BRANCH_SET_LINK);
+			   op_callback_addr, BRANCH_SET_LINK);
 
 	rc |= create_branch(&branch_emulate_step, buff + TMPL_EMULATE_IDX,
-			    (unsigned long)emulate_step_addr,
-			    BRANCH_SET_LINK);
+			    emulate_step_addr, BRANCH_SET_LINK);
 
 	if (rc)
 		goto error;
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 09/12] powerpc/inst: Refactor PPC32 and PPC64 versions
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

ppc_inst() ppc_inst_prefixed() ppc_inst_swab() can easily
be made common to both PPC32 and PPC64.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/inst.h | 49 +++++++++------------------------
 1 file changed, 13 insertions(+), 36 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 32d318c3b180..e009e94e90b2 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -60,9 +60,9 @@ static inline int ppc_inst_primary_opcode(struct ppc_inst x)
 	return ppc_inst_val(x) >> 26;
 }
 
-#ifdef CONFIG_PPC64
 #define ppc_inst(x) ((struct ppc_inst){ .val = (x) })
 
+#ifdef CONFIG_PPC64
 #define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix = (y) })
 
 static inline u32 ppc_inst_suffix(struct ppc_inst x)
@@ -70,57 +70,34 @@ static inline u32 ppc_inst_suffix(struct ppc_inst x)
 	return x.suffix;
 }
 
-static inline bool ppc_inst_prefixed(struct ppc_inst x)
-{
-	return ppc_inst_primary_opcode(x) == OP_PREFIX;
-}
+#else
+#define ppc_inst_prefix(x, y) ppc_inst(x)
 
-static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
+static inline u32 ppc_inst_suffix(struct ppc_inst x)
 {
-	return ppc_inst_prefix(swab32(ppc_inst_val(x)), swab32(ppc_inst_suffix(x)));
+	return 0;
 }
 
+#endif /* CONFIG_PPC64 */
+
 static inline struct ppc_inst ppc_inst_read(const unsigned int *ptr)
 {
-	u32 val, suffix;
-
-	val = *ptr;
-	if ((val >> 26) == OP_PREFIX) {
-		suffix = *(ptr + 1);
-		return ppc_inst_prefix(val, suffix);
-	} else {
-		return ppc_inst(val);
-	}
+	if (IS_ENABLED(CONFIG_PPC64) && (*ptr >> 26) == OP_PREFIX)
+		return ppc_inst_prefix(*ptr, *(ptr + 1));
+	else
+		return ppc_inst(*ptr);
 }
 
-#else
-
-#define ppc_inst(x) ((struct ppc_inst){ .val = x })
-
-#define ppc_inst_prefix(x, y) ppc_inst(x)
-
 static inline bool ppc_inst_prefixed(struct ppc_inst x)
 {
-	return false;
-}
-
-static inline u32 ppc_inst_suffix(struct ppc_inst x)
-{
-	return 0;
+	return IS_ENABLED(CONFIG_PPC64) && ppc_inst_primary_opcode(x) == OP_PREFIX;
 }
 
 static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
 {
-	return ppc_inst(swab32(ppc_inst_val(x)));
-}
-
-static inline struct ppc_inst ppc_inst_read(const unsigned int *ptr)
-{
-	return ppc_inst(*ptr);
+	return ppc_inst_prefix(swab32(ppc_inst_val(x)), swab32(ppc_inst_suffix(x)));
 }
 
-#endif /* CONFIG_PPC64 */
-
 static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
 {
 	if (ppc_inst_val(x) != ppc_inst_val(y))
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 08/12] powerpc: Don't use 'struct ppc_inst' to reference instruction location
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

'struct ppc_inst' is an internal representation of an instruction, but
in-memory instructions are and will remain a table of 'unsigned int'
forever.

Replace all 'struct ppc_inst *' used for locating an instruction in
memory by 'unsigned int *'. This removes a lot of undue casts
to 'struct ppc_inst *'.

It also helps locating ab-use of 'struct ppc_inst' dereference.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Fix a create_branch() in ftrace and take copy_inst_from_kernel_nofault() into account.
---
 arch/powerpc/include/asm/code-patching.h  | 22 +++---
 arch/powerpc/include/asm/inst.h           | 14 ++--
 arch/powerpc/include/asm/uprobes.h        |  4 +-
 arch/powerpc/kernel/crash_dump.c          |  4 +-
 arch/powerpc/kernel/epapr_paravirt.c      |  4 +-
 arch/powerpc/kernel/jump_label.c          |  2 +-
 arch/powerpc/kernel/kgdb.c                |  6 +-
 arch/powerpc/kernel/kprobes.c             | 17 ++--
 arch/powerpc/kernel/mce_power.c           |  2 +-
 arch/powerpc/kernel/optprobes.c           | 45 +++++------
 arch/powerpc/kernel/setup_32.c            |  2 +-
 arch/powerpc/kernel/trace/ftrace.c        | 33 ++++----
 arch/powerpc/kernel/uprobes.c             |  6 +-
 arch/powerpc/lib/code-patching.c          | 50 ++++++------
 arch/powerpc/lib/feature-fixups.c         | 96 +++++++++++------------
 arch/powerpc/mm/maccess.c                 |  4 +-
 arch/powerpc/perf/core-book3s.c           |  4 +-
 arch/powerpc/platforms/86xx/mpc86xx_smp.c |  2 +-
 arch/powerpc/platforms/powermac/smp.c     |  4 +-
 arch/powerpc/xmon/xmon.c                  | 10 +--
 20 files changed, 160 insertions(+), 171 deletions(-)

diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index f9bd1397b696..b20ee4e1e6fb 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -23,13 +23,13 @@
 #define BRANCH_ABSOLUTE	0x2
 
 bool is_offset_in_branch_range(long offset);
-int create_branch(struct ppc_inst *instr, const struct ppc_inst *addr,
+int create_branch(struct ppc_inst *instr, const unsigned int *addr,
 		  unsigned long target, int flags);
-int create_cond_branch(struct ppc_inst *instr, const struct ppc_inst *addr,
+int create_cond_branch(struct ppc_inst *instr, const unsigned int *addr,
 		       unsigned long target, int flags);
-int patch_branch(struct ppc_inst *addr, unsigned long target, int flags);
-int patch_instruction(struct ppc_inst *addr, struct ppc_inst instr);
-int raw_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr);
+int patch_branch(unsigned int *addr, unsigned long target, int flags);
+int patch_instruction(unsigned int *addr, struct ppc_inst instr);
+int raw_patch_instruction(unsigned int *addr, struct ppc_inst instr);
 
 static inline unsigned long patch_site_addr(s32 *site)
 {
@@ -38,18 +38,18 @@ static inline unsigned long patch_site_addr(s32 *site)
 
 static inline int patch_instruction_site(s32 *site, struct ppc_inst instr)
 {
-	return patch_instruction((struct ppc_inst *)patch_site_addr(site), instr);
+	return patch_instruction((unsigned int *)patch_site_addr(site), instr);
 }
 
 static inline int patch_branch_site(s32 *site, unsigned long target, int flags)
 {
-	return patch_branch((struct ppc_inst *)patch_site_addr(site), target, flags);
+	return patch_branch((unsigned int *)patch_site_addr(site), target, flags);
 }
 
 static inline int modify_instruction(unsigned int *addr, unsigned int clr,
 				     unsigned int set)
 {
-	return patch_instruction((struct ppc_inst *)addr, ppc_inst((*addr & ~clr) | set));
+	return patch_instruction(addr, ppc_inst((*addr & ~clr) | set));
 }
 
 static inline int modify_instruction_site(s32 *site, unsigned int clr, unsigned int set)
@@ -59,9 +59,9 @@ static inline int modify_instruction_site(s32 *site, unsigned int clr, unsigned
 
 int instr_is_relative_branch(struct ppc_inst instr);
 int instr_is_relative_link_branch(struct ppc_inst instr);
-unsigned long branch_target(const struct ppc_inst *instr);
-int translate_branch(struct ppc_inst *instr, const struct ppc_inst *dest,
-		     const struct ppc_inst *src);
+unsigned long branch_target(const unsigned int *instr);
+int translate_branch(struct ppc_inst *instr, const unsigned int *dest,
+		     const unsigned int *src);
 extern bool is_conditional_branch(struct ppc_inst instr);
 #ifdef CONFIG_PPC_BOOK3E_64
 void __patch_exception(int exc, unsigned long addr);
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 5a0740ebf132..32d318c3b180 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -80,13 +80,13 @@ static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
 	return ppc_inst_prefix(swab32(ppc_inst_val(x)), swab32(ppc_inst_suffix(x)));
 }
 
-static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
+static inline struct ppc_inst ppc_inst_read(const unsigned int *ptr)
 {
 	u32 val, suffix;
 
-	val = *(u32 *)ptr;
+	val = *ptr;
 	if ((val >> 26) == OP_PREFIX) {
-		suffix = *((u32 *)ptr + 1);
+		suffix = *(ptr + 1);
 		return ppc_inst_prefix(val, suffix);
 	} else {
 		return ppc_inst(val);
@@ -114,9 +114,9 @@ static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
 	return ppc_inst(swab32(ppc_inst_val(x)));
 }
 
-static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
+static inline struct ppc_inst ppc_inst_read(const unsigned int *ptr)
 {
-	return *ptr;
+	return ppc_inst(*ptr);
 }
 
 #endif /* CONFIG_PPC64 */
@@ -139,7 +139,7 @@ static inline int ppc_inst_len(struct ppc_inst x)
  * Return the address of the next instruction, if the instruction @value was
  * located at @location.
  */
-static inline struct ppc_inst *ppc_inst_next(void *location, struct ppc_inst *value)
+static inline unsigned int *ppc_inst_next(unsigned int *location, unsigned int *value)
 {
 	struct ppc_inst tmp;
 
@@ -177,6 +177,6 @@ static inline char *__ppc_inst_as_str(char str[PPC_INST_STR_LEN], struct ppc_ins
 	__str;				\
 })
 
-int copy_inst_from_kernel_nofault(struct ppc_inst *inst, struct ppc_inst *src);
+int copy_inst_from_kernel_nofault(struct ppc_inst *inst, unsigned int *src);
 
 #endif /* _ASM_POWERPC_INST_H */
diff --git a/arch/powerpc/include/asm/uprobes.h b/arch/powerpc/include/asm/uprobes.h
index 5bf65f5d44a9..fe683371336f 100644
--- a/arch/powerpc/include/asm/uprobes.h
+++ b/arch/powerpc/include/asm/uprobes.h
@@ -24,8 +24,8 @@ typedef ppc_opcode_t uprobe_opcode_t;
 
 struct arch_uprobe {
 	union {
-		struct ppc_inst	insn;
-		struct ppc_inst	ixol;
+		u32 insn[2];
+		u32 ixol[2];
 	};
 };
 
diff --git a/arch/powerpc/kernel/crash_dump.c b/arch/powerpc/kernel/crash_dump.c
index 735e89337398..1476ae72a432 100644
--- a/arch/powerpc/kernel/crash_dump.c
+++ b/arch/powerpc/kernel/crash_dump.c
@@ -35,7 +35,7 @@ void __init reserve_kdump_trampoline(void)
 
 static void __init create_trampoline(unsigned long addr)
 {
-	struct ppc_inst *p = (struct ppc_inst *)addr;
+	unsigned int *p = (unsigned int *)addr;
 
 	/* The maximum range of a single instruction branch, is the current
 	 * instruction's address + (32 MB - 4) bytes. For the trampoline we
@@ -46,7 +46,7 @@ static void __init create_trampoline(unsigned long addr)
 	 * two instructions it doesn't require any registers.
 	 */
 	patch_instruction(p, ppc_inst(PPC_INST_NOP));
-	patch_branch((void *)p + 4, addr + PHYSICAL_START, 0);
+	patch_branch(p + 1, addr + PHYSICAL_START, 0);
 }
 
 void __init setup_kdump_trampoline(void)
diff --git a/arch/powerpc/kernel/epapr_paravirt.c b/arch/powerpc/kernel/epapr_paravirt.c
index 2ed14d4a47f5..93b0f3ec8fb0 100644
--- a/arch/powerpc/kernel/epapr_paravirt.c
+++ b/arch/powerpc/kernel/epapr_paravirt.c
@@ -38,9 +38,9 @@ static int __init early_init_dt_scan_epapr(unsigned long node,
 
 	for (i = 0; i < (len / 4); i++) {
 		struct ppc_inst inst = ppc_inst(be32_to_cpu(insts[i]));
-		patch_instruction((struct ppc_inst *)(epapr_hypercall_start + i), inst);
+		patch_instruction(epapr_hypercall_start + i, inst);
 #if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64)
-		patch_instruction((struct ppc_inst *)(epapr_ev_idle_start + i), inst);
+		patch_instruction(epapr_ev_idle_start + i, inst);
 #endif
 	}
 
diff --git a/arch/powerpc/kernel/jump_label.c b/arch/powerpc/kernel/jump_label.c
index ce87dc5ea23c..b58d5a0d8c28 100644
--- a/arch/powerpc/kernel/jump_label.c
+++ b/arch/powerpc/kernel/jump_label.c
@@ -11,7 +11,7 @@
 void arch_jump_label_transform(struct jump_entry *entry,
 			       enum jump_label_type type)
 {
-	struct ppc_inst *addr = (struct ppc_inst *)jump_entry_code(entry);
+	unsigned int *addr = (unsigned int *)jump_entry_code(entry);
 
 	if (type == JUMP_LABEL_JMP)
 		patch_branch(addr, jump_entry_target(entry), 0);
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 7dd2ad3603ad..22c82ccddaf9 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -419,9 +419,9 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
 {
 	int err;
 	unsigned int instr;
-	struct ppc_inst *addr = (struct ppc_inst *)bpt->bpt_addr;
+	unsigned int *addr = (unsigned int *)bpt->bpt_addr;
 
-	err = get_kernel_nofault(instr, (unsigned *) addr);
+	err = get_kernel_nofault(instr, addr);
 	if (err)
 		return err;
 
@@ -438,7 +438,7 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
 {
 	int err;
 	unsigned int instr = *(unsigned int *)bpt->saved_instr;
-	struct ppc_inst *addr = (struct ppc_inst *)bpt->bpt_addr;
+	unsigned int *addr = (unsigned int *)bpt->bpt_addr;
 
 	err = patch_instruction(addr, ppc_inst(instr));
 	if (err)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 01ab2163659e..e6dc2c5bb2cd 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -107,8 +107,8 @@ int arch_prepare_kprobe(struct kprobe *p)
 {
 	int ret = 0;
 	struct kprobe *prev;
-	struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->addr);
-	struct ppc_inst prefix = ppc_inst_read((struct ppc_inst *)(p->addr - 1));
+	struct ppc_inst insn = ppc_inst_read(p->addr);
+	struct ppc_inst prefix = ppc_inst_read(p->addr - 1);
 
 	if ((unsigned long)p->addr & 0x03) {
 		printk("Attempt to register kprobe at an unaligned address\n");
@@ -123,8 +123,7 @@ int arch_prepare_kprobe(struct kprobe *p)
 	preempt_disable();
 	prev = get_kprobe(p->addr - 1);
 	preempt_enable_no_resched();
-	if (prev &&
-	    ppc_inst_prefixed(ppc_inst_read((struct ppc_inst *)prev->ainsn.insn))) {
+	if (prev && ppc_inst_prefixed(ppc_inst_read(prev->ainsn.insn))) {
 		printk("Cannot register a kprobe on the second word of prefixed instruction\n");
 		ret = -EINVAL;
 	}
@@ -138,7 +137,7 @@ int arch_prepare_kprobe(struct kprobe *p)
 	}
 
 	if (!ret) {
-		patch_instruction((struct ppc_inst *)p->ainsn.insn, insn);
+		patch_instruction(p->ainsn.insn, insn);
 		p->opcode = ppc_inst_val(insn);
 	}
 
@@ -149,13 +148,13 @@ NOKPROBE_SYMBOL(arch_prepare_kprobe);
 
 void arch_arm_kprobe(struct kprobe *p)
 {
-	patch_instruction((struct ppc_inst *)p->addr, ppc_inst(BREAKPOINT_INSTRUCTION));
+	patch_instruction(p->addr, ppc_inst(BREAKPOINT_INSTRUCTION));
 }
 NOKPROBE_SYMBOL(arch_arm_kprobe);
 
 void arch_disarm_kprobe(struct kprobe *p)
 {
-	patch_instruction((struct ppc_inst *)p->addr, ppc_inst(p->opcode));
+	patch_instruction(p->addr, ppc_inst(p->opcode));
 }
 NOKPROBE_SYMBOL(arch_disarm_kprobe);
 
@@ -228,7 +227,7 @@ NOKPROBE_SYMBOL(arch_prepare_kretprobe);
 static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
 {
 	int ret;
-	struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
+	struct ppc_inst insn = ppc_inst_read(p->ainsn.insn);
 
 	/* regs->nip is also adjusted if emulate_step returns 1 */
 	ret = emulate_step(regs, insn);
@@ -439,7 +438,7 @@ int kprobe_post_handler(struct pt_regs *regs)
 	if (!cur || user_mode(regs))
 		return 0;
 
-	len = ppc_inst_len(ppc_inst_read((struct ppc_inst *)cur->ainsn.insn));
+	len = ppc_inst_len(ppc_inst_read(cur->ainsn.insn));
 	/* make sure we got here for instruction we have a kprobe on */
 	if (((unsigned long)cur->ainsn.insn + len) != regs->nip)
 		return 0;
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index 667104d4c455..f3b5097768b6 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -463,7 +463,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 = ppc_inst_read((struct ppc_inst *)instr_addr);
+		instr = ppc_inst_read((unsigned int *)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 cdf87086fa33..adaf31157f6d 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -100,9 +100,8 @@ static unsigned long can_optimize(struct kprobe *p)
 	 * Ensure that the instruction is not a conditional branch,
 	 * and that can be emulated.
 	 */
-	if (!is_conditional_branch(ppc_inst_read((struct ppc_inst *)p->ainsn.insn)) &&
-	    analyse_instr(&op, &regs,
-			  ppc_inst_read((struct ppc_inst *)p->ainsn.insn)) == 1) {
+	if (!is_conditional_branch(ppc_inst_read(p->ainsn.insn)) &&
+	    analyse_instr(&op, &regs, ppc_inst_read(p->ainsn.insn)) == 1) {
 		emulate_update_regs(&regs, &op);
 		nip = regs.nip;
 	}
@@ -143,12 +142,10 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
 
 static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
 {
-	patch_instruction((struct ppc_inst *)addr,
-			  ppc_inst(PPC_RAW_LIS(reg, IMM_H(val))));
+	patch_instruction(addr, ppc_inst(PPC_RAW_LIS(reg, IMM_H(val))));
 	addr++;
 
-	patch_instruction((struct ppc_inst *)addr,
-			  ppc_inst(PPC_RAW_ORI(reg, reg, IMM_L(val))));
+	patch_instruction(addr, ppc_inst(PPC_RAW_ORI(reg, reg, IMM_L(val))));
 }
 
 /*
@@ -158,31 +155,31 @@ static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *
 static void patch_imm64_load_insns(unsigned long long val, int reg, kprobe_opcode_t *addr)
 {
 	/* lis reg,(op)@highest */
-	patch_instruction((struct ppc_inst *)addr,
+	patch_instruction(addr,
 			  ppc_inst(PPC_INST_ADDIS | ___PPC_RT(reg) |
 				   ((val >> 48) & 0xffff)));
 	addr++;
 
 	/* ori reg,reg,(op)@higher */
-	patch_instruction((struct ppc_inst *)addr,
+	patch_instruction(addr,
 			  ppc_inst(PPC_INST_ORI | ___PPC_RA(reg) |
 				   ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
 	addr++;
 
 	/* rldicr reg,reg,32,31 */
-	patch_instruction((struct ppc_inst *)addr,
+	patch_instruction(addr,
 			  ppc_inst(PPC_INST_RLDICR | ___PPC_RA(reg) |
 				   ___PPC_RS(reg) | __PPC_SH64(32) | __PPC_ME64(31)));
 	addr++;
 
 	/* oris reg,reg,(op)@h */
-	patch_instruction((struct ppc_inst *)addr,
+	patch_instruction(addr,
 			  ppc_inst(PPC_INST_ORIS | ___PPC_RA(reg) |
 				   ___PPC_RS(reg) | ((val >> 16) & 0xffff)));
 	addr++;
 
 	/* ori reg,reg,(op)@l */
-	patch_instruction((struct ppc_inst *)addr,
+	patch_instruction(addr,
 			  ppc_inst(PPC_INST_ORI | ___PPC_RA(reg) |
 				   ___PPC_RS(reg) | (val & 0xffff)));
 }
@@ -238,8 +235,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 	size = (TMPL_END_IDX * sizeof(kprobe_opcode_t)) / sizeof(int);
 	pr_devel("Copying template to %p, size %lu\n", buff, size);
 	for (i = 0; i < size; i++) {
-		rc = patch_instruction((struct ppc_inst *)(buff + i),
-				       ppc_inst(*(optprobe_template_entry + i)));
+		rc = patch_instruction(buff + i, ppc_inst(*(optprobe_template_entry + i)));
 		if (rc < 0)
 			goto error;
 	}
@@ -260,34 +256,30 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 		goto error;
 	}
 
-	rc = create_branch(&branch_op_callback,
-			   (struct ppc_inst *)(buff + TMPL_CALL_HDLR_IDX),
+	rc = create_branch(&branch_op_callback, buff + TMPL_CALL_HDLR_IDX,
 			   (unsigned long)op_callback_addr,
 			   BRANCH_SET_LINK);
 
-	rc |= create_branch(&branch_emulate_step,
-			    (struct ppc_inst *)(buff + TMPL_EMULATE_IDX),
+	rc |= create_branch(&branch_emulate_step, buff + TMPL_EMULATE_IDX,
 			    (unsigned long)emulate_step_addr,
 			    BRANCH_SET_LINK);
 
 	if (rc)
 		goto error;
 
-	patch_instruction((struct ppc_inst *)(buff + TMPL_CALL_HDLR_IDX),
-			  branch_op_callback);
-	patch_instruction((struct ppc_inst *)(buff + TMPL_EMULATE_IDX),
-			  branch_emulate_step);
+	patch_instruction(buff + TMPL_CALL_HDLR_IDX, branch_op_callback);
+	patch_instruction(buff + TMPL_EMULATE_IDX, branch_emulate_step);
 
 	/*
 	 * 3. load instruction to be emulated into relevant register, and
 	 */
-	temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
+	temp = ppc_inst_read(p->ainsn.insn);
 	patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
 
 	/*
 	 * 4. branch back from trampoline
 	 */
-	patch_branch((struct ppc_inst *)(buff + TMPL_RET_IDX), (unsigned long)nip, 0);
+	patch_branch(buff + TMPL_RET_IDX, nip, 0);
 
 	flush_icache_range((unsigned long)buff,
 			   (unsigned long)(&buff[TMPL_END_IDX]));
@@ -330,10 +322,9 @@ void arch_optimize_kprobes(struct list_head *oplist)
 		 */
 		memcpy(op->optinsn.copied_insn, op->kp.addr,
 					       RELATIVEJUMP_SIZE);
-		create_branch(&instr,
-			      (struct ppc_inst *)op->kp.addr,
+		create_branch(&instr, op->kp.addr,
 			      (unsigned long)op->optinsn.insn, 0);
-		patch_instruction((struct ppc_inst *)op->kp.addr, instr);
+		patch_instruction(op->kp.addr, instr);
 		list_del_init(&op->list);
 	}
 }
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index d7c1f92152af..77590ecf83b1 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -74,7 +74,7 @@ EXPORT_SYMBOL(DMA_MODE_WRITE);
  */
 notrace void __init machine_init(u64 dt_ptr)
 {
-	struct ppc_inst *addr = (struct ppc_inst *)patch_site_addr(&patch__memset_nocache);
+	unsigned int *addr = (unsigned int *)patch_site_addr(&patch__memset_nocache);
 	struct ppc_inst insn;
 
 	/* Configure static keys first, now that we're relocated. */
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index ffe9537195aa..fc7453a27175 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -49,7 +49,7 @@ ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
 	addr = ppc_function_entry((void *)addr);
 
 	/* if (link) set op to 'bl' else 'b' */
-	create_branch(&op, (struct ppc_inst *)ip, addr, link ? 1 : 0);
+	create_branch(&op, (unsigned int *)ip, addr, link ? 1 : 0);
 
 	return op;
 }
@@ -79,7 +79,7 @@ ftrace_modify_code(unsigned long ip, struct ppc_inst old, struct ppc_inst new)
 	}
 
 	/* replace the text with the new text */
-	if (patch_instruction((struct ppc_inst *)ip, new))
+	if (patch_instruction((unsigned int *)ip, new))
 		return -EPERM;
 
 	return 0;
@@ -94,7 +94,7 @@ static int 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(&op, (struct ppc_inst *)ip, addr, 0) == 0;
+	return create_branch(&op, (unsigned int *)ip, addr, 0) == 0;
 }
 
 static int is_bl_op(struct ppc_inst op)
@@ -208,7 +208,7 @@ __ftrace_make_nop(struct module *mod,
 	}
 #endif /* CONFIG_MPROFILE_KERNEL */
 
-	if (patch_instruction((struct ppc_inst *)ip, pop)) {
+	if (patch_instruction((unsigned int *)ip, pop)) {
 		pr_err("Patching NOP failed.\n");
 		return -EPERM;
 	}
@@ -280,7 +280,7 @@ __ftrace_make_nop(struct module *mod,
 
 	op = ppc_inst(PPC_INST_NOP);
 
-	if (patch_instruction((struct ppc_inst *)ip, op))
+	if (patch_instruction((unsigned int *)ip, op))
 		return -EPERM;
 
 	return 0;
@@ -380,7 +380,7 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 		return -1;
 	}
 
-	if (patch_branch((struct ppc_inst *)tramp, ptr, 0)) {
+	if (patch_branch((unsigned int *)tramp, ptr, 0)) {
 		pr_debug("REL24 out of range!\n");
 		return -1;
 	}
@@ -424,7 +424,7 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		}
 	}
 
-	if (patch_instruction((struct ppc_inst *)ip, ppc_inst(PPC_INST_NOP))) {
+	if (patch_instruction((unsigned int *)ip, ppc_inst(PPC_INST_NOP))) {
 		pr_err("Patching NOP failed.\n");
 		return -EPERM;
 	}
@@ -589,10 +589,10 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	int err;
 	struct ppc_inst op;
-	unsigned long ip = rec->ip;
+	unsigned int *ip = (unsigned int *)rec->ip;
 
 	/* read where this goes */
-	if (copy_inst_from_kernel_nofault(&op, (void *)ip))
+	if (copy_inst_from_kernel_nofault(&op, ip))
 		return -EFAULT;
 
 	/* It should be pointing to a nop */
@@ -608,8 +608,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	}
 
 	/* create the branch to the trampoline */
-	err = create_branch(&op, (struct ppc_inst *)ip,
-			    rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
+	err = create_branch(&op, ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
 	if (err) {
 		pr_err("REL24 out of range!\n");
 		return -EINVAL;
@@ -617,7 +616,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 
 	pr_devel("write to %lx\n", rec->ip);
 
-	if (patch_instruction((struct ppc_inst *)ip, op))
+	if (patch_instruction(ip, op))
 		return -EPERM;
 
 	return 0;
@@ -762,7 +761,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((struct ppc_inst *)ip, addr, BRANCH_SET_LINK)) {
+		if (patch_branch((unsigned int *)ip, addr, BRANCH_SET_LINK)) {
 			pr_err("REL24 out of range!\n");
 			return -EINVAL;
 		}
@@ -790,12 +789,12 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 	}
 
 	/* Ensure branch is within 24 bits */
-	if (create_branch(&op, (struct ppc_inst *)ip, tramp, BRANCH_SET_LINK)) {
+	if (create_branch(&op, (unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
 		pr_err("Branch out of range\n");
 		return -EINVAL;
 	}
 
-	if (patch_branch((struct ppc_inst *)ip, tramp, BRANCH_SET_LINK)) {
+	if (patch_branch((unsigned int *)ip, tramp, BRANCH_SET_LINK)) {
 		pr_err("REL24 out of range!\n");
 		return -EINVAL;
 	}
@@ -851,7 +850,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 	struct ppc_inst old, new;
 	int ret;
 
-	old = ppc_inst_read((struct ppc_inst *)&ftrace_call);
+	old = ppc_inst_read((unsigned int *)&ftrace_call);
 	new = ftrace_call_replace(ip, (unsigned long)func, 1);
 	ret = ftrace_modify_code(ip, old, new);
 
@@ -859,7 +858,7 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 	/* Also update the regs callback function */
 	if (!ret) {
 		ip = (unsigned long)(&ftrace_regs_call);
-		old = ppc_inst_read((struct ppc_inst *)&ftrace_regs_call);
+		old = ppc_inst_read((unsigned int *)&ftrace_regs_call);
 		new = ftrace_call_replace(ip, (unsigned long)func, 1);
 		ret = ftrace_modify_code(ip, old, new);
 	}
diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index 46971bb41d05..1aefd2ecbb8a 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -42,7 +42,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
 		return -EINVAL;
 
 	if (cpu_has_feature(CPU_FTR_ARCH_31) &&
-	    ppc_inst_prefixed(ppc_inst_read(&auprobe->insn)) &&
+	    ppc_inst_prefixed(ppc_inst_read(auprobe->insn)) &&
 	    (addr & 0x3f) == 60) {
 		pr_info_ratelimited("Cannot register a uprobe on 64 byte unaligned prefixed instruction\n");
 		return -EINVAL;
@@ -119,7 +119,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
 	 * support doesn't exist and have to fix-up the next instruction
 	 * to be executed.
 	 */
-	regs->nip = (unsigned long)ppc_inst_next((void *)utask->vaddr, &auprobe->insn);
+	regs->nip = (unsigned long)ppc_inst_next((void *)utask->vaddr, auprobe->insn);
 
 	user_disable_single_step(current);
 	return 0;
@@ -182,7 +182,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, struct pt_regs *regs)
 	 * emulate_step() returns 1 if the insn was successfully emulated.
 	 * For all other cases, we need to single-step in hardware.
 	 */
-	ret = emulate_step(regs, ppc_inst_read(&auprobe->insn));
+	ret = emulate_step(regs, ppc_inst_read(auprobe->insn));
 	if (ret > 0)
 		return true;
 
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 508e9511ca96..bab8bd2d2714 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -18,8 +18,8 @@
 #include <asm/setup.h>
 #include <asm/inst.h>
 
-static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr,
-			       struct ppc_inst *patch_addr)
+static int __patch_instruction(unsigned int *exec_addr, struct ppc_inst instr,
+			       unsigned int *patch_addr)
 {
 	if (!ppc_inst_prefixed(instr)) {
 		u32 val = ppc_inst_val(instr);
@@ -40,7 +40,7 @@ static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr
 	return -EFAULT;
 }
 
-int raw_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
+int raw_patch_instruction(unsigned int *addr, struct ppc_inst instr)
 {
 	return __patch_instruction(addr, instr, addr);
 }
@@ -148,10 +148,10 @@ static inline int unmap_patch_area(unsigned long addr)
 	return 0;
 }
 
-static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
+static int do_patch_instruction(unsigned int *addr, struct ppc_inst instr)
 {
 	int err;
-	struct ppc_inst *patch_addr = NULL;
+	unsigned int *patch_addr = NULL;
 	unsigned long flags;
 	unsigned long text_poke_addr;
 	unsigned long kaddr = (unsigned long)addr;
@@ -172,7 +172,7 @@ static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
 		goto out;
 	}
 
-	patch_addr = (struct ppc_inst *)(text_poke_addr + (kaddr & ~PAGE_MASK));
+	patch_addr = (unsigned int *)(text_poke_addr + (kaddr & ~PAGE_MASK));
 
 	__patch_instruction(addr, instr, patch_addr);
 
@@ -187,14 +187,14 @@ static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
 }
 #else /* !CONFIG_STRICT_KERNEL_RWX */
 
-static int do_patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
+static int do_patch_instruction(unsigned int *addr, struct ppc_inst instr)
 {
 	return raw_patch_instruction(addr, instr);
 }
 
 #endif /* CONFIG_STRICT_KERNEL_RWX */
 
-int patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
+int patch_instruction(unsigned int *addr, struct ppc_inst instr)
 {
 	/* Make sure we aren't patching a freed init section */
 	if (init_mem_is_free && init_section_contains(addr, 4)) {
@@ -205,7 +205,7 @@ int patch_instruction(struct ppc_inst *addr, struct ppc_inst instr)
 }
 NOKPROBE_SYMBOL(patch_instruction);
 
-int patch_branch(struct ppc_inst *addr, unsigned long target, int flags)
+int patch_branch(unsigned int *addr, unsigned long target, int flags)
 {
 	struct ppc_inst instr;
 
@@ -258,7 +258,7 @@ bool is_conditional_branch(struct ppc_inst instr)
 NOKPROBE_SYMBOL(is_conditional_branch);
 
 int create_branch(struct ppc_inst *instr,
-		  const struct ppc_inst *addr,
+		  const unsigned int *addr,
 		  unsigned long target, int flags)
 {
 	long offset;
@@ -278,7 +278,7 @@ int create_branch(struct ppc_inst *instr,
 	return 0;
 }
 
-int create_cond_branch(struct ppc_inst *instr, const struct ppc_inst *addr,
+int create_cond_branch(struct ppc_inst *instr, const unsigned int *addr,
 		       unsigned long target, int flags)
 {
 	long offset;
@@ -325,7 +325,7 @@ int instr_is_relative_link_branch(struct ppc_inst instr)
 	return instr_is_relative_branch(instr) && (ppc_inst_val(instr) & BRANCH_SET_LINK);
 }
 
-static unsigned long branch_iform_target(const struct ppc_inst *instr)
+static unsigned long branch_iform_target(const unsigned int *instr)
 {
 	signed long imm;
 
@@ -341,7 +341,7 @@ static unsigned long branch_iform_target(const struct ppc_inst *instr)
 	return (unsigned long)imm;
 }
 
-static unsigned long branch_bform_target(const struct ppc_inst *instr)
+static unsigned long branch_bform_target(const unsigned int *instr)
 {
 	signed long imm;
 
@@ -357,7 +357,7 @@ static unsigned long branch_bform_target(const struct ppc_inst *instr)
 	return (unsigned long)imm;
 }
 
-unsigned long branch_target(const struct ppc_inst *instr)
+unsigned long branch_target(const unsigned int *instr)
 {
 	if (instr_is_branch_iform(ppc_inst_read(instr)))
 		return branch_iform_target(instr);
@@ -367,8 +367,8 @@ unsigned long branch_target(const struct ppc_inst *instr)
 	return 0;
 }
 
-int translate_branch(struct ppc_inst *instr, const struct ppc_inst *dest,
-		     const struct ppc_inst *src)
+int translate_branch(struct ppc_inst *instr, const unsigned int *dest,
+		     const unsigned int *src)
 {
 	unsigned long target;
 	target = branch_target(src);
@@ -395,13 +395,13 @@ void __patch_exception(int exc, unsigned long addr)
 	 * instruction of the exception, not the first one
 	 */
 
-	patch_branch((struct ppc_inst *)(ibase + (exc / 4) + 1), addr, 0);
+	patch_branch(ibase + (exc / 4) + 1, addr, 0);
 }
 #endif
 
 #ifdef CONFIG_CODE_PATCHING_SELFTEST
 
-static int instr_is_branch_to_addr(const struct ppc_inst *instr, unsigned long addr)
+static int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr)
 {
 	if (instr_is_branch_iform(ppc_inst_read(instr)) ||
 	    instr_is_branch_bform(ppc_inst_read(instr)))
@@ -423,7 +423,7 @@ static void __init test_branch_iform(void)
 	int err;
 	struct ppc_inst instr;
 	unsigned int tmp[2];
-	struct ppc_inst *iptr = (struct ppc_inst *)tmp;
+	unsigned int *iptr = tmp;
 	unsigned long addr = (unsigned long)tmp;
 
 	/* The simplest case, branch to self, no flags */
@@ -501,12 +501,12 @@ static void __init test_branch_iform(void)
 
 static void __init test_create_function_call(void)
 {
-	struct ppc_inst *iptr;
+	unsigned int *iptr;
 	unsigned long dest;
 	struct ppc_inst instr;
 
 	/* Check we can create a function call */
-	iptr = (struct ppc_inst *)ppc_function_entry(test_trampoline);
+	iptr = (unsigned int *)ppc_function_entry(test_trampoline);
 	dest = ppc_function_entry(test_create_function_call);
 	create_branch(&instr, iptr, dest, BRANCH_SET_LINK);
 	patch_instruction(iptr, instr);
@@ -517,11 +517,11 @@ static void __init test_branch_bform(void)
 {
 	int err;
 	unsigned long addr;
-	struct ppc_inst *iptr, instr;
+	struct ppc_inst instr;
 	unsigned int tmp[2];
+	unsigned int *iptr = tmp;
 	unsigned int flags;
 
-	iptr = (struct ppc_inst *)tmp;
 	addr = (unsigned long)iptr;
 
 	/* The simplest case, branch to self, no flags */
@@ -726,9 +726,9 @@ static void __init test_prefixed_patching(void)
 	extern unsigned int code_patching_test1_expected[];
 	extern unsigned int end_code_patching_test1[];
 
-	__patch_instruction((struct ppc_inst *)code_patching_test1,
+	__patch_instruction(code_patching_test1,
 			    ppc_inst_prefix(OP_PREFIX << 26, 0x00000000),
-			    (struct ppc_inst *)code_patching_test1);
+			    code_patching_test1);
 
 	check(!memcmp(code_patching_test1,
 		      code_patching_test1_expected,
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 8905b53109bc..801f34530133 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -33,18 +33,18 @@ struct fixup_entry {
 	long		alt_end_off;
 };
 
-static struct ppc_inst *calc_addr(struct fixup_entry *fcur, long offset)
+static unsigned int *calc_addr(struct fixup_entry *fcur, long offset)
 {
 	/*
 	 * We store the offset to the code as a negative offset from
 	 * the start of the alt_entry, to support the VDSO. This
 	 * routine converts that back into an actual address.
 	 */
-	return (struct ppc_inst *)((unsigned long)fcur + offset);
+	return (unsigned int *)((unsigned long)fcur + offset);
 }
 
-static int patch_alt_instruction(struct ppc_inst *src, struct ppc_inst *dest,
-				 struct ppc_inst *alt_start, struct ppc_inst *alt_end)
+static int patch_alt_instruction(unsigned int *src, unsigned int *dest,
+				 unsigned int *alt_start, unsigned int *alt_end)
 {
 	int err;
 	struct ppc_inst instr;
@@ -52,7 +52,7 @@ static int patch_alt_instruction(struct ppc_inst *src, struct ppc_inst *dest,
 	instr = ppc_inst_read(src);
 
 	if (instr_is_relative_branch(ppc_inst_read(src))) {
-		struct ppc_inst *target = (struct ppc_inst *)branch_target(src);
+		unsigned int *target = (unsigned int *)branch_target(src);
 
 		/* Branch within the section doesn't need translating */
 		if (target < alt_start || target > alt_end) {
@@ -69,7 +69,7 @@ static int patch_alt_instruction(struct ppc_inst *src, struct ppc_inst *dest,
 
 static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
 {
-	struct ppc_inst *start, *end, *alt_start, *alt_end, *src, *dest, nop;
+	unsigned int *start, *end, *alt_start, *alt_end, *src, *dest;
 
 	start = calc_addr(fcur, fcur->start_off);
 	end = calc_addr(fcur, fcur->end_off);
@@ -91,9 +91,8 @@ static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
 			return 1;
 	}
 
-	nop = ppc_inst(PPC_INST_NOP);
-	for (; dest < end; dest = ppc_inst_next(dest, &nop))
-		raw_patch_instruction(dest, nop);
+	for (; dest < end; dest++)
+		raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
 
 	return 0;
 }
@@ -152,14 +151,14 @@ static void do_stf_entry_barrier_fixups(enum stf_barrier_type types)
 
 		// See comment in do_entry_flush_fixups() RE order of patching
 		if (types & STF_BARRIER_FALLBACK) {
-			patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
-			patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
-			patch_branch((struct ppc_inst *)(dest + 1),
+			patch_instruction(dest, ppc_inst(instrs[0]));
+			patch_instruction(dest + 2, ppc_inst(instrs[2]));
+			patch_branch(dest + 1,
 				     (unsigned long)&stf_barrier_fallback, BRANCH_SET_LINK);
 		} else {
-			patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
-			patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
-			patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+			patch_instruction(dest + 1, ppc_inst(instrs[1]));
+			patch_instruction(dest + 2, ppc_inst(instrs[2]));
+			patch_instruction(dest, ppc_inst(instrs[0]));
 		}
 	}
 
@@ -213,12 +212,12 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
 
 		pr_devel("patching dest %lx\n", (unsigned long)dest);
 
-		patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
-		patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
-		patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
-		patch_instruction((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
-		patch_instruction((struct ppc_inst *)(dest + 4), ppc_inst(instrs[4]));
-		patch_instruction((struct ppc_inst *)(dest + 5), ppc_inst(instrs[5]));
+		patch_instruction(dest, ppc_inst(instrs[0]));
+		patch_instruction(dest + 1, ppc_inst(instrs[1]));
+		patch_instruction(dest + 2, ppc_inst(instrs[2]));
+		patch_instruction(dest + 3, ppc_inst(instrs[3]));
+		patch_instruction(dest + 4, ppc_inst(instrs[4]));
+		patch_instruction(dest + 5, ppc_inst(instrs[5]));
 	}
 	printk(KERN_DEBUG "stf-barrier: patched %d exit locations (%s barrier)\n", i,
 		(types == STF_BARRIER_NONE)                  ? "no" :
@@ -282,11 +281,11 @@ void do_uaccess_flush_fixups(enum l1d_flush_type types)
 
 		pr_devel("patching dest %lx\n", (unsigned long)dest);
 
-		patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+		patch_instruction(dest, ppc_inst(instrs[0]));
 
-		patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
-		patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
-		patch_instruction((struct ppc_inst *)(dest + 3), ppc_inst(instrs[3]));
+		patch_instruction(dest + 1, ppc_inst(instrs[1]));
+		patch_instruction(dest + 2, ppc_inst(instrs[2]));
+		patch_instruction(dest + 3, ppc_inst(instrs[3]));
 	}
 
 	printk(KERN_DEBUG "uaccess-flush: patched %d locations (%s flush)\n", i,
@@ -358,14 +357,14 @@ static int __do_entry_flush_fixups(void *data)
 		pr_devel("patching dest %lx\n", (unsigned long)dest);
 
 		if (types == L1D_FLUSH_FALLBACK) {
-			patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
-			patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
-			patch_branch((struct ppc_inst *)(dest + 1),
+			patch_instruction(dest, ppc_inst(instrs[0]));
+			patch_instruction(dest + 2, ppc_inst(instrs[2]));
+			patch_branch(dest + 1,
 				     (unsigned long)&entry_flush_fallback, BRANCH_SET_LINK);
 		} else {
-			patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
-			patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
-			patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+			patch_instruction(dest + 1, ppc_inst(instrs[1]));
+			patch_instruction(dest + 2, ppc_inst(instrs[2]));
+			patch_instruction(dest, ppc_inst(instrs[0]));
 		}
 	}
 
@@ -377,14 +376,14 @@ static int __do_entry_flush_fixups(void *data)
 		pr_devel("patching dest %lx\n", (unsigned long)dest);
 
 		if (types == L1D_FLUSH_FALLBACK) {
-			patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
-			patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
-			patch_branch((struct ppc_inst *)(dest + 1),
+			patch_instruction(dest, ppc_inst(instrs[0]));
+			patch_instruction(dest + 2, ppc_inst(instrs[2]));
+			patch_branch(dest + 1,
 				     (unsigned long)&scv_entry_flush_fallback, BRANCH_SET_LINK);
 		} else {
-			patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
-			patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
-			patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
+			patch_instruction(dest + 1, ppc_inst(instrs[1]));
+			patch_instruction(dest + 2, ppc_inst(instrs[2]));
+			patch_instruction(dest, ppc_inst(instrs[0]));
 		}
 	}
 
@@ -443,9 +442,9 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
 
 		pr_devel("patching dest %lx\n", (unsigned long)dest);
 
-		patch_instruction((struct ppc_inst *)dest, ppc_inst(instrs[0]));
-		patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instrs[1]));
-		patch_instruction((struct ppc_inst *)(dest + 2), ppc_inst(instrs[2]));
+		patch_instruction(dest, ppc_inst(instrs[0]));
+		patch_instruction(dest + 1, ppc_inst(instrs[1]));
+		patch_instruction(dest + 2, ppc_inst(instrs[2]));
 	}
 
 	printk(KERN_DEBUG "rfi-flush: patched %d locations (%s flush)\n", i,
@@ -478,7 +477,7 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
 		dest = (void *)start + *start;
 
 		pr_devel("patching dest %lx\n", (unsigned long)dest);
-		patch_instruction((struct ppc_inst *)dest, ppc_inst(instr));
+		patch_instruction(dest, ppc_inst(instr));
 	}
 
 	printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
@@ -521,8 +520,8 @@ void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_
 		dest = (void *)start + *start;
 
 		pr_devel("patching dest %lx\n", (unsigned long)dest);
-		patch_instruction((struct ppc_inst *)dest, ppc_inst(instr[0]));
-		patch_instruction((struct ppc_inst *)(dest + 1), ppc_inst(instr[1]));
+		patch_instruction(dest, ppc_inst(instr[0]));
+		patch_instruction(dest + 1, ppc_inst(instr[1]));
 	}
 
 	printk(KERN_DEBUG "barrier-nospec: patched %d locations\n", i);
@@ -536,7 +535,7 @@ static void patch_btb_flush_section(long *curr)
 	end = (void *)curr + *(curr + 1);
 	for (; start < end; start++) {
 		pr_devel("patching dest %lx\n", (unsigned long)start);
-		patch_instruction((struct ppc_inst *)start, ppc_inst(PPC_INST_NOP));
+		patch_instruction(start, ppc_inst(PPC_INST_NOP));
 	}
 }
 
@@ -555,7 +554,7 @@ void do_btb_flush_fixups(void)
 void do_lwsync_fixups(unsigned long value, void *fixup_start, void *fixup_end)
 {
 	long *start, *end;
-	struct ppc_inst *dest;
+	unsigned int *dest;
 
 	if (!(value & CPU_FTR_LWSYNC))
 		return ;
@@ -572,13 +571,14 @@ void do_lwsync_fixups(unsigned long value, void *fixup_start, void *fixup_end)
 static void do_final_fixups(void)
 {
 #if defined(CONFIG_PPC64) && defined(CONFIG_RELOCATABLE)
-	struct ppc_inst inst, *src, *dest, *end;
+	struct ppc_inst inst;
+	unsigned int *src, *dest, *end;
 
 	if (PHYSICAL_START == 0)
 		return;
 
-	src = (struct ppc_inst *)(KERNELBASE + PHYSICAL_START);
-	dest = (struct ppc_inst *)KERNELBASE;
+	src = (unsigned int *)(KERNELBASE + PHYSICAL_START);
+	dest = (unsigned int *)KERNELBASE;
 	end = (void *)src + (__end_interrupts - _stext);
 
 	while (src < end) {
diff --git a/arch/powerpc/mm/maccess.c b/arch/powerpc/mm/maccess.c
index a3c30a884076..de4279f770ed 100644
--- a/arch/powerpc/mm/maccess.c
+++ b/arch/powerpc/mm/maccess.c
@@ -12,7 +12,7 @@ bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
 	return is_kernel_addr((unsigned long)unsafe_src);
 }
 
-int copy_inst_from_kernel_nofault(struct ppc_inst *inst, struct ppc_inst *src)
+int copy_inst_from_kernel_nofault(struct ppc_inst *inst, unsigned int *src)
 {
 	unsigned int val, suffix;
 	int err;
@@ -21,7 +21,7 @@ int copy_inst_from_kernel_nofault(struct ppc_inst *inst, struct ppc_inst *src)
 	if (err)
 		return err;
 	if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
-		err = copy_from_kernel_nofault(&suffix, (void *)src + 4, 4);
+		err = copy_from_kernel_nofault(&suffix, src + 1, sizeof(suffix));
 		*inst = ppc_inst_prefix(val, suffix);
 	} else {
 		*inst = ppc_inst(val);
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 16d4d1b6a1ff..3e864feaacf7 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -460,7 +460,7 @@ static __u64 power_pmu_bhrb_to(u64 addr)
 				sizeof(instr)))
 			return 0;
 
-		return branch_target((struct ppc_inst *)&instr);
+		return branch_target(&instr);
 	}
 
 	/* Userspace: need copy instruction here then translate it */
@@ -468,7 +468,7 @@ static __u64 power_pmu_bhrb_to(u64 addr)
 			sizeof(instr)))
 		return 0;
 
-	target = branch_target((struct ppc_inst *)&instr);
+	target = branch_target(&instr);
 	if ((!target) || (instr & BRANCH_ABSOLUTE))
 		return target;
 
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_smp.c b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
index 87f524e4b09c..302f2a1e0361 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_smp.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
@@ -83,7 +83,7 @@ smp_86xx_kick_cpu(int nr)
 		mdelay(1);
 
 	/* Restore the exception vector */
-	patch_instruction((struct ppc_inst *)vector, ppc_inst(save_vector));
+	patch_instruction(vector, ppc_inst(save_vector));
 
 	local_irq_restore(flags);
 
diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
index adae2a6712e1..bdfea6d6ab69 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -810,7 +810,7 @@ static int smp_core99_kick_cpu(int nr)
 	 *   b __secondary_start_pmac_0 + nr*8
 	 */
 	target = (unsigned long) __secondary_start_pmac_0 + nr * 8;
-	patch_branch((struct ppc_inst *)vector, target, BRANCH_SET_LINK);
+	patch_branch(vector, target, BRANCH_SET_LINK);
 
 	/* Put some life in our friend */
 	pmac_call_feature(PMAC_FTR_RESET_CPU, NULL, nr, 0);
@@ -823,7 +823,7 @@ static int smp_core99_kick_cpu(int nr)
 	mdelay(1);
 
 	/* Restore our exception vector */
-	patch_instruction((struct ppc_inst *)vector, ppc_inst(save_vector));
+	patch_instruction(vector, ppc_inst(save_vector));
 
 	local_irq_restore(flags);
 	if (ppc_md.progress) ppc_md.progress("smp_core99_kick_cpu done", 0x347);
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index c8173e92f19d..3b8770455551 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -100,7 +100,7 @@ static long *xmon_fault_jmp[NR_CPUS];
 /* Breakpoint stuff */
 struct bpt {
 	unsigned long	address;
-	struct ppc_inst	*instr;
+	unsigned int	*instr;
 	atomic_t	ref_count;
 	int		enabled;
 	unsigned long	pad;
@@ -946,11 +946,11 @@ static void insert_bpts(void)
 		}
 
 		patch_instruction(bp->instr, instr);
-		patch_instruction(ppc_inst_next(bp->instr, &instr),
+		patch_instruction(ppc_inst_next(bp->instr, bp->instr),
 				  ppc_inst(bpinstr));
 		if (bp->enabled & BP_CIABR)
 			continue;
-		if (patch_instruction((struct ppc_inst *)bp->address,
+		if (patch_instruction((unsigned int *)bp->address,
 				      ppc_inst(bpinstr)) != 0) {
 			printf("Couldn't write instruction at %lx, "
 			       "disabling breakpoint there\n", bp->address);
@@ -992,7 +992,7 @@ static void remove_bpts(void)
 		if (mread_instr(bp->address, &instr)
 		    && ppc_inst_equal(instr, ppc_inst(bpinstr))
 		    && patch_instruction(
-			(struct ppc_inst *)bp->address, ppc_inst_read(bp->instr)) != 0)
+			(unsigned int *)bp->address, ppc_inst_read(bp->instr)) != 0)
 			printf("Couldn't remove breakpoint at %lx\n",
 			       bp->address);
 	}
@@ -2214,7 +2214,7 @@ mread_instr(unsigned long adrs, struct ppc_inst *instr)
 	if (setjmp(bus_error_jmp) == 0) {
 		catch_memory_errors = 1;
 		sync();
-		*instr = ppc_inst_read((struct ppc_inst *)adrs);
+		*instr = ppc_inst_read((unsigned int *)adrs);
 		sync();
 		/* wait a little while to see if we get a machine check */
 		__delay(200);
-- 
2.25.0




^ permalink raw reply related

* [PATCH v2 07/12] powerpc/lib/code-patching: Don't use struct 'ppc_inst' for runnable code in tests.
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

'struct ppc_inst' is meant to represent an instruction internally, it
is not meant to dereference code in memory.

For testing code patching, use patch_instruction() to properly
write into memory the code to be tested.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/lib/code-patching.c | 95 ++++++++++++++++++--------------
 1 file changed, 53 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 82f2c1edb498..508e9511ca96 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -422,9 +422,9 @@ static void __init test_branch_iform(void)
 {
 	int err;
 	struct ppc_inst instr;
-	unsigned long addr;
-
-	addr = (unsigned long)&instr;
+	unsigned int tmp[2];
+	struct ppc_inst *iptr = (struct ppc_inst *)tmp;
+	unsigned long addr = (unsigned long)tmp;
 
 	/* The simplest case, branch to self, no flags */
 	check(instr_is_branch_iform(ppc_inst(0x48000000)));
@@ -445,52 +445,57 @@ static void __init test_branch_iform(void)
 	check(!instr_is_branch_iform(ppc_inst(0x7bfffffd)));
 
 	/* Absolute branch to 0x100 */
-	instr = ppc_inst(0x48000103);
-	check(instr_is_branch_to_addr(&instr, 0x100));
+	patch_instruction(iptr, ppc_inst(0x48000103));
+	check(instr_is_branch_to_addr(iptr, 0x100));
 	/* Absolute branch to 0x420fc */
-	instr = ppc_inst(0x480420ff);
-	check(instr_is_branch_to_addr(&instr, 0x420fc));
+	patch_instruction(iptr, ppc_inst(0x480420ff));
+	check(instr_is_branch_to_addr(iptr, 0x420fc));
 	/* Maximum positive relative branch, + 20MB - 4B */
-	instr = ppc_inst(0x49fffffc);
-	check(instr_is_branch_to_addr(&instr, addr + 0x1FFFFFC));
+	patch_instruction(iptr, ppc_inst(0x49fffffc));
+	check(instr_is_branch_to_addr(iptr, addr + 0x1FFFFFC));
 	/* Smallest negative relative branch, - 4B */
-	instr = ppc_inst(0x4bfffffc);
-	check(instr_is_branch_to_addr(&instr, addr - 4));
+	patch_instruction(iptr, ppc_inst(0x4bfffffc));
+	check(instr_is_branch_to_addr(iptr, addr - 4));
 	/* Largest negative relative branch, - 32 MB */
-	instr = ppc_inst(0x4a000000);
-	check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
+	patch_instruction(iptr, ppc_inst(0x4a000000));
+	check(instr_is_branch_to_addr(iptr, addr - 0x2000000));
 
 	/* Branch to self, with link */
-	err = create_branch(&instr, &instr, addr, BRANCH_SET_LINK);
-	check(instr_is_branch_to_addr(&instr, addr));
+	err = create_branch(&instr, iptr, addr, BRANCH_SET_LINK);
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr));
 
 	/* Branch to self - 0x100, with link */
-	err = create_branch(&instr, &instr, addr - 0x100, BRANCH_SET_LINK);
-	check(instr_is_branch_to_addr(&instr, addr - 0x100));
+	err = create_branch(&instr, iptr, addr - 0x100, BRANCH_SET_LINK);
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr - 0x100));
 
 	/* Branch to self + 0x100, no link */
-	err = create_branch(&instr, &instr, addr + 0x100, 0);
-	check(instr_is_branch_to_addr(&instr, addr + 0x100));
+	err = create_branch(&instr, iptr, addr + 0x100, 0);
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr + 0x100));
 
 	/* Maximum relative negative offset, - 32 MB */
-	err = create_branch(&instr, &instr, addr - 0x2000000, BRANCH_SET_LINK);
-	check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
+	err = create_branch(&instr, iptr, addr - 0x2000000, BRANCH_SET_LINK);
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr - 0x2000000));
 
 	/* Out of range relative negative offset, - 32 MB + 4*/
-	err = create_branch(&instr, &instr, addr - 0x2000004, BRANCH_SET_LINK);
+	err = create_branch(&instr, iptr, addr - 0x2000004, BRANCH_SET_LINK);
 	check(err);
 
 	/* Out of range relative positive offset, + 32 MB */
-	err = create_branch(&instr, &instr, addr + 0x2000000, BRANCH_SET_LINK);
+	err = create_branch(&instr, iptr, addr + 0x2000000, BRANCH_SET_LINK);
 	check(err);
 
 	/* Unaligned target */
-	err = create_branch(&instr, &instr, addr + 3, BRANCH_SET_LINK);
+	err = create_branch(&instr, iptr, addr + 3, BRANCH_SET_LINK);
 	check(err);
 
 	/* Check flags are masked correctly */
-	err = create_branch(&instr, &instr, addr, 0xFFFFFFFC);
-	check(instr_is_branch_to_addr(&instr, addr));
+	err = create_branch(&instr, iptr, addr, 0xFFFFFFFC);
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr));
 	check(ppc_inst_equal(instr, ppc_inst(0x48000000)));
 }
 
@@ -513,9 +518,10 @@ static void __init test_branch_bform(void)
 	int err;
 	unsigned long addr;
 	struct ppc_inst *iptr, instr;
+	unsigned int tmp[2];
 	unsigned int flags;
 
-	iptr = &instr;
+	iptr = (struct ppc_inst *)tmp;
 	addr = (unsigned long)iptr;
 
 	/* The simplest case, branch to self, no flags */
@@ -528,39 +534,43 @@ static void __init test_branch_bform(void)
 	check(!instr_is_branch_bform(ppc_inst(0x7bffffff)));
 
 	/* Absolute conditional branch to 0x100 */
-	instr = ppc_inst(0x43ff0103);
-	check(instr_is_branch_to_addr(&instr, 0x100));
+	patch_instruction(iptr, ppc_inst(0x43ff0103));
+	check(instr_is_branch_to_addr(iptr, 0x100));
 	/* Absolute conditional branch to 0x20fc */
-	instr = ppc_inst(0x43ff20ff);
-	check(instr_is_branch_to_addr(&instr, 0x20fc));
+	patch_instruction(iptr, ppc_inst(0x43ff20ff));
+	check(instr_is_branch_to_addr(iptr, 0x20fc));
 	/* Maximum positive relative conditional branch, + 32 KB - 4B */
-	instr = ppc_inst(0x43ff7ffc);
-	check(instr_is_branch_to_addr(&instr, addr + 0x7FFC));
+	patch_instruction(iptr, ppc_inst(0x43ff7ffc));
+	check(instr_is_branch_to_addr(iptr, addr + 0x7FFC));
 	/* Smallest negative relative conditional branch, - 4B */
-	instr = ppc_inst(0x43fffffc);
-	check(instr_is_branch_to_addr(&instr, addr - 4));
+	patch_instruction(iptr, ppc_inst(0x43fffffc));
+	check(instr_is_branch_to_addr(iptr, addr - 4));
 	/* Largest negative relative conditional branch, - 32 KB */
-	instr = ppc_inst(0x43ff8000);
-	check(instr_is_branch_to_addr(&instr, addr - 0x8000));
+	patch_instruction(iptr, ppc_inst(0x43ff8000));
+	check(instr_is_branch_to_addr(iptr, addr - 0x8000));
 
 	/* All condition code bits set & link */
 	flags = 0x3ff000 | BRANCH_SET_LINK;
 
 	/* Branch to self */
 	err = create_cond_branch(&instr, iptr, addr, flags);
-	check(instr_is_branch_to_addr(&instr, addr));
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr));
 
 	/* Branch to self - 0x100 */
 	err = create_cond_branch(&instr, iptr, addr - 0x100, flags);
-	check(instr_is_branch_to_addr(&instr, addr - 0x100));
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr - 0x100));
 
 	/* Branch to self + 0x100 */
 	err = create_cond_branch(&instr, iptr, addr + 0x100, flags);
-	check(instr_is_branch_to_addr(&instr, addr + 0x100));
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr + 0x100));
 
 	/* Maximum relative negative offset, - 32 KB */
 	err = create_cond_branch(&instr, iptr, addr - 0x8000, flags);
-	check(instr_is_branch_to_addr(&instr, addr - 0x8000));
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr - 0x8000));
 
 	/* Out of range relative negative offset, - 32 KB + 4*/
 	err = create_cond_branch(&instr, iptr, addr - 0x8004, flags);
@@ -576,7 +586,8 @@ static void __init test_branch_bform(void)
 
 	/* Check flags are masked correctly */
 	err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC);
-	check(instr_is_branch_to_addr(&instr, addr));
+	patch_instruction(iptr, instr);
+	check(instr_is_branch_to_addr(iptr, addr));
 	check(ppc_inst_equal(instr, ppc_inst(0x43FF0000)));
 }
 
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 06/12] powerpc/lib/code-patching: Make instr_is_branch_to_addr() static
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

instr_is_branch_to_addr() is only used in code-patching.c

Make it static.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/code-patching.h |  1 -
 arch/powerpc/lib/code-patching.c         | 18 +++++++++---------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index f1d029bf906e..f9bd1397b696 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -59,7 +59,6 @@ static inline int modify_instruction_site(s32 *site, unsigned int clr, unsigned
 
 int instr_is_relative_branch(struct ppc_inst instr);
 int instr_is_relative_link_branch(struct ppc_inst instr);
-int instr_is_branch_to_addr(const struct ppc_inst *instr, unsigned long addr);
 unsigned long branch_target(const struct ppc_inst *instr);
 int translate_branch(struct ppc_inst *instr, const struct ppc_inst *dest,
 		     const struct ppc_inst *src);
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 0308429b0d1a..82f2c1edb498 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -367,15 +367,6 @@ unsigned long branch_target(const struct ppc_inst *instr)
 	return 0;
 }
 
-int instr_is_branch_to_addr(const struct ppc_inst *instr, unsigned long addr)
-{
-	if (instr_is_branch_iform(ppc_inst_read(instr)) ||
-	    instr_is_branch_bform(ppc_inst_read(instr)))
-		return branch_target(instr) == addr;
-
-	return 0;
-}
-
 int translate_branch(struct ppc_inst *instr, const struct ppc_inst *dest,
 		     const struct ppc_inst *src)
 {
@@ -410,6 +401,15 @@ void __patch_exception(int exc, unsigned long addr)
 
 #ifdef CONFIG_CODE_PATCHING_SELFTEST
 
+static int instr_is_branch_to_addr(const struct ppc_inst *instr, unsigned long addr)
+{
+	if (instr_is_branch_iform(ppc_inst_read(instr)) ||
+	    instr_is_branch_bform(ppc_inst_read(instr)))
+		return branch_target(instr) == addr;
+
+	return 0;
+}
+
 static void __init test_trampoline(void)
 {
 	asm ("nop;\n");
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 05/12] powerpc: Do not dereference code as 'struct ppc_inst' (uprobe, code-patching, feature-fixups)
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

'struct ppc_inst' is an internal structure to represent an instruction,
it is not directly the representation of that instruction in text code.
It is not meant to map and dereference code.

Dereferencing code directly through 'struct ppc_inst' has two main issues:
- On powerpc, structs are expected to be 8 bytes aligned while code is
spread every 4 byte.
- Should a non prefixed instruction lie at the end of the page and the
following page not be mapped, it would generate a page fault.

In-memory code must be accessed with ppc_inst_read().

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/uprobes.c     | 2 +-
 arch/powerpc/lib/code-patching.c  | 8 ++++----
 arch/powerpc/lib/feature-fixups.c | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index 186f69b11e94..46971bb41d05 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -42,7 +42,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
 		return -EINVAL;
 
 	if (cpu_has_feature(CPU_FTR_ARCH_31) &&
-	    ppc_inst_prefixed(auprobe->insn) &&
+	    ppc_inst_prefixed(ppc_inst_read(&auprobe->insn)) &&
 	    (addr & 0x3f) == 60) {
 		pr_info_ratelimited("Cannot register a uprobe on 64 byte unaligned prefixed instruction\n");
 		return -EINVAL;
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 870b30d9be2f..0308429b0d1a 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -329,13 +329,13 @@ static unsigned long branch_iform_target(const struct ppc_inst *instr)
 {
 	signed long imm;
 
-	imm = ppc_inst_val(*instr) & 0x3FFFFFC;
+	imm = ppc_inst_val(ppc_inst_read(instr)) & 0x3FFFFFC;
 
 	/* If the top bit of the immediate value is set this is negative */
 	if (imm & 0x2000000)
 		imm -= 0x4000000;
 
-	if ((ppc_inst_val(*instr) & BRANCH_ABSOLUTE) == 0)
+	if ((ppc_inst_val(ppc_inst_read(instr)) & BRANCH_ABSOLUTE) == 0)
 		imm += (unsigned long)instr;
 
 	return (unsigned long)imm;
@@ -345,13 +345,13 @@ static unsigned long branch_bform_target(const struct ppc_inst *instr)
 {
 	signed long imm;
 
-	imm = ppc_inst_val(*instr) & 0xFFFC;
+	imm = ppc_inst_val(ppc_inst_read(instr)) & 0xFFFC;
 
 	/* If the top bit of the immediate value is set this is negative */
 	if (imm & 0x8000)
 		imm -= 0x10000;
 
-	if ((ppc_inst_val(*instr) & BRANCH_ABSOLUTE) == 0)
+	if ((ppc_inst_val(ppc_inst_read(instr)) & BRANCH_ABSOLUTE) == 0)
 		imm += (unsigned long)instr;
 
 	return (unsigned long)imm;
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index fe26f2fa0f3f..8905b53109bc 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -51,7 +51,7 @@ static int patch_alt_instruction(struct ppc_inst *src, struct ppc_inst *dest,
 
 	instr = ppc_inst_read(src);
 
-	if (instr_is_relative_branch(*src)) {
+	if (instr_is_relative_branch(ppc_inst_read(src))) {
 		struct ppc_inst *target = (struct ppc_inst *)branch_target(src);
 
 		/* Branch within the section doesn't need translating */
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 04/12] powerpc/inst: Avoid pointer dereferencing in ppc_inst_equal()
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

Avoid casting/dereferencing ppc_inst() as u64* , check each member
of the struct when relevant.

And remove the 0xff initialisation of the suffix for non
prefixed instruction. An instruction with 0xff as a suffix
might be invalid, but still is a prefixed instruction and
has to be considered as this.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/inst.h | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index fc6adef528a5..5a0740ebf132 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -61,7 +61,7 @@ static inline int ppc_inst_primary_opcode(struct ppc_inst x)
 }
 
 #ifdef CONFIG_PPC64
-#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
+#define ppc_inst(x) ((struct ppc_inst){ .val = (x) })
 
 #define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix = (y) })
 
@@ -72,7 +72,7 @@ static inline u32 ppc_inst_suffix(struct ppc_inst x)
 
 static inline bool ppc_inst_prefixed(struct ppc_inst x)
 {
-	return ppc_inst_primary_opcode(x) == OP_PREFIX && ppc_inst_suffix(x) != 0xff;
+	return ppc_inst_primary_opcode(x) == OP_PREFIX;
 }
 
 static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
@@ -93,11 +93,6 @@ static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
 	}
 }
 
-static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
-{
-	return *(u64 *)&x == *(u64 *)&y;
-}
-
 #else
 
 #define ppc_inst(x) ((struct ppc_inst){ .val = x })
@@ -124,13 +119,17 @@ static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
 	return *ptr;
 }
 
+#endif /* CONFIG_PPC64 */
+
 static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
 {
-	return ppc_inst_val(x) == ppc_inst_val(y);
+	if (ppc_inst_val(x) != ppc_inst_val(y))
+		return false;
+	if (!ppc_inst_prefixed(x))
+		return true;
+	return ppc_inst_suffix(x) == ppc_inst_suffix(y);
 }
 
-#endif /* CONFIG_PPC64 */
-
 static inline int ppc_inst_len(struct ppc_inst x)
 {
 	return ppc_inst_prefixed(x) ? 8 : 4;
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 03/12] powerpc/inst: Improve readability of get_user_instr() and friends
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

Remove unneeded line splits.

And remove unneeded local variable initialisation.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Don't use get_op() for now as including asm/disassemble.h conflicts with drivers/staging/rtl8723bs/include/wifi.h
---
 arch/powerpc/include/asm/inst.h | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 57c31e712e67..fc6adef528a5 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -8,7 +8,7 @@
 
 #define ___get_user_instr(gu_op, dest, ptr)				\
 ({									\
-	long __gui_ret = 0;						\
+	long __gui_ret;							\
 	unsigned int __user *__gui_ptr = (unsigned int __user *)ptr;	\
 	struct ppc_inst __gui_inst;					\
 	unsigned int __prefix, __suffix;				\
@@ -18,8 +18,7 @@
 	if (__gui_ret == 0) {						\
 		if ((__prefix >> 26) == OP_PREFIX) {			\
 			__gui_ret = gu_op(__suffix, __gui_ptr + 1);	\
-			__gui_inst = ppc_inst_prefix(__prefix,		\
-						     __suffix);		\
+			__gui_inst = ppc_inst_prefix(__prefix, __suffix); \
 		} else {						\
 			__gui_inst = ppc_inst(__prefix);		\
 		}							\
@@ -36,11 +35,9 @@
 })
 #endif /* CONFIG_PPC64 */
 
-#define get_user_instr(x, ptr) \
-	___get_user_instr(get_user, x, ptr)
+#define get_user_instr(x, ptr) ___get_user_instr(get_user, x, ptr)
 
-#define __get_user_instr(x, ptr) \
-	___get_user_instr(__get_user, x, ptr)
+#define __get_user_instr(x, ptr) ___get_user_instr(__get_user, x, ptr)
 
 /*
  * Instruction data type for POWER
@@ -75,13 +72,12 @@ static inline u32 ppc_inst_suffix(struct ppc_inst x)
 
 static inline bool ppc_inst_prefixed(struct ppc_inst x)
 {
-	return (ppc_inst_primary_opcode(x) == 1) && ppc_inst_suffix(x) != 0xff;
+	return ppc_inst_primary_opcode(x) == OP_PREFIX && ppc_inst_suffix(x) != 0xff;
 }
 
 static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
 {
-	return ppc_inst_prefix(swab32(ppc_inst_val(x)),
-			       swab32(ppc_inst_suffix(x)));
+	return ppc_inst_prefix(swab32(ppc_inst_val(x)), swab32(ppc_inst_suffix(x)));
 }
 
 static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 02/12] powerpc/inst: Reduce casts in get_user_instr()
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

Declare __gui_ptr as 'unsigned int *' instead of casting it
at each use.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/inst.h | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 887ef150fdda..57c31e712e67 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -9,16 +9,15 @@
 #define ___get_user_instr(gu_op, dest, ptr)				\
 ({									\
 	long __gui_ret = 0;						\
-	unsigned long __gui_ptr = (unsigned long)ptr;			\
+	unsigned int __user *__gui_ptr = (unsigned int __user *)ptr;	\
 	struct ppc_inst __gui_inst;					\
 	unsigned int __prefix, __suffix;				\
 									\
 	__chk_user_ptr(ptr);						\
-	__gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr);	\
+	__gui_ret = gu_op(__prefix, __gui_ptr);				\
 	if (__gui_ret == 0) {						\
 		if ((__prefix >> 26) == OP_PREFIX) {			\
-			__gui_ret = gu_op(__suffix,			\
-				(unsigned int __user *)__gui_ptr + 1);	\
+			__gui_ret = gu_op(__suffix, __gui_ptr + 1);	\
 			__gui_inst = ppc_inst_prefix(__prefix,		\
 						     __suffix);		\
 		} else {						\
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 01/12] powerpc/inst: Fix sparse detection on get_user_instr()
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1621516826.git.christophe.leroy@csgroup.eu>

get_user_instr() lacks sparse detection for the __user tag.

This is because __gui_ptr is assigned with a cast.

Fix that by adding a __chk_user_ptr()

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/inst.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 268d3bd073c8..887ef150fdda 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -12,6 +12,8 @@
 	unsigned long __gui_ptr = (unsigned long)ptr;			\
 	struct ppc_inst __gui_inst;					\
 	unsigned int __prefix, __suffix;				\
+									\
+	__chk_user_ptr(ptr);						\
 	__gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr);	\
 	if (__gui_ret == 0) {						\
 		if ((__prefix >> 26) == OP_PREFIX) {			\
@@ -29,7 +31,10 @@
 })
 #else /* !CONFIG_PPC64 */
 #define ___get_user_instr(gu_op, dest, ptr)				\
-	gu_op((dest).val, (u32 __user *)(ptr))
+({									\
+	__chk_user_ptr(ptr);						\
+	gu_op((dest).val, (u32 __user *)(ptr));				\
+})
 #endif /* CONFIG_PPC64 */
 
 #define get_user_instr(x, ptr) \
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 00/12] powerpc: Cleanup use of 'struct ppc_inst'
From: Christophe Leroy @ 2021-05-20 13:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	naveen.n.rao, jniethe5
  Cc: linuxppc-dev, linux-kernel

This series is a cleanup of the use of 'struct ppc_inst'.

A confusion is made between internal representation of powerpc
instructions with 'struct ppc_inst' and in-memory code which is
and will always be an array of 'unsigned int'.

This series cleans it up.

First patch is fixing detection of missing '__user' flag by sparse
when using get_user_instr().

Last part of the series does some source code cleanup in
optprobes, it is put at the ends of this series because of
clashes with the 'struct ppc_inst' cleanups.

Changes in v2:
- Temporarily don't use get_op() and don't include asm/disassemble.h in asm/inst.h because it exhibits a conflict for function get_ra() with drivers/staging/rtl8723bs/include/wifi.h
- Fix a create_branch() in ftrace and take copy_inst_from_kernel_nofault() into account

Christophe Leroy (12):
  powerpc/inst: Fix sparse detection on get_user_instr()
  powerpc/inst: Reduce casts in get_user_instr()
  powerpc/inst: Improve readability of get_user_instr() and friends
  powerpc/inst: Avoid pointer dereferencing in ppc_inst_equal()
  powerpc: Do not dereference code as 'struct ppc_inst' (uprobe,
    code-patching, feature-fixups)
  powerpc/lib/code-patching: Make instr_is_branch_to_addr() static
  powerpc/lib/code-patching: Don't use struct 'ppc_inst' for runnable
    code in tests.
  powerpc: Don't use 'struct ppc_inst' to reference instruction location
  powerpc/inst: Refactor PPC32 and PPC64 versions
  powerpc/optprobes: Minimise casts
  powerpc/optprobes: Compact code source a bit.
  powerpc/optprobes: use PPC_RAW_ macros

 arch/powerpc/include/asm/code-patching.h  |  23 ++-
 arch/powerpc/include/asm/inst.h           |  92 +++++-------
 arch/powerpc/include/asm/ppc-opcode.h     |  11 +-
 arch/powerpc/include/asm/uprobes.h        |   4 +-
 arch/powerpc/kernel/crash_dump.c          |   4 +-
 arch/powerpc/kernel/epapr_paravirt.c      |   4 +-
 arch/powerpc/kernel/jump_label.c          |   2 +-
 arch/powerpc/kernel/kgdb.c                |   6 +-
 arch/powerpc/kernel/kprobes.c             |  17 ++-
 arch/powerpc/kernel/mce_power.c           |   2 +-
 arch/powerpc/kernel/optprobes.c           | 124 +++++-----------
 arch/powerpc/kernel/setup_32.c            |   2 +-
 arch/powerpc/kernel/trace/ftrace.c        |  33 +++--
 arch/powerpc/kernel/uprobes.c             |   6 +-
 arch/powerpc/lib/code-patching.c          | 165 ++++++++++++----------
 arch/powerpc/lib/feature-fixups.c         |  98 ++++++-------
 arch/powerpc/mm/maccess.c                 |   4 +-
 arch/powerpc/perf/core-book3s.c           |   4 +-
 arch/powerpc/platforms/86xx/mpc86xx_smp.c |   2 +-
 arch/powerpc/platforms/powermac/smp.c     |   4 +-
 arch/powerpc/xmon/xmon.c                  |  10 +-
 21 files changed, 277 insertions(+), 340 deletions(-)

-- 
2.25.0


^ permalink raw reply

* Re: [PATCH] powerpc/kprobes: Pass ppc_inst as a pointer to emulate_step() on ppc32
From: Christophe Leroy @ 2021-05-20 12:55 UTC (permalink / raw)
  To: Naveen N. Rao, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <1621507675.1o3860b85w.naveen@linux.ibm.com>



Le 20/05/2021 à 12:54, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>>
>>
>> Le 20/05/2021 à 09:29, Naveen N. Rao a écrit :
>>> Trying to use a kprobe on ppc32 results in the below splat:
>>>      BUG: Unable to handle kernel data access on read at 0x7c0802a6
>>>      Faulting instruction address: 0xc002e9f0
>>>      Oops: Kernel access of bad area, sig: 11 [#1]
>>>      BE PAGE_SIZE=4K PowerPC 44x Platform
>>>      Modules linked in:
>>>      CPU: 0 PID: 89 Comm: sh Not tainted 5.13.0-rc1-01824-g3a81c0495fdb #7
>>>      NIP:  c002e9f0 LR: c0011858 CTR: 00008a47
>>>      REGS: c292fd50 TRAP: 0300   Not tainted  (5.13.0-rc1-01824-g3a81c0495fdb)
>>>      MSR:  00009000 <EE,ME>  CR: 24002002  XER: 20000000
>>>      DEAR: 7c0802a6 ESR: 00000000
>>>      <snip>
>>>      NIP [c002e9f0] emulate_step+0x28/0x324
>>>      LR [c0011858] optinsn_slot+0x128/0x10000
>>>      Call Trace:
>>>       opt_pre_handler+0x7c/0xb4 (unreliable)
>>>       optinsn_slot+0x128/0x10000
>>>       ret_from_syscall+0x0/0x28
>>
>> I remember running some kprobe tests before submitting the patch, how did I miss that ?
>> Is there anything special to do to activate the use of optprobes and/or to hit this bug ?
> 
> Yeah, I was surprised when I hit this. One of the requirements we have for optprobes on powerpc is 
> that the instruction should be a compute instruction (no load/store -- emulate_update_regs() should 
> be enough) with the exception of conditional branches. It's possible that you ended up probing an 
> instruction that couldn't be optimized.
> 
> An easy way to confirm if a probe has been optimized is to look at kprobes/list in debugfs, and to 
> watch out for [OPTIMIZED] flag there.
> 
>>> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
>>> index cdf87086fa33a0..2bc53fa48a1b33 100644
>>> --- a/arch/powerpc/kernel/optprobes.c
>>> +++ b/arch/powerpc/kernel/optprobes.c
>>> @@ -281,8 +281,12 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe 
>>> *p)
>>>       /*
>>>        * 3. load instruction to be emulated into relevant register, and
>>>        */
>>> -    temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
>>> -    patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
>>> +    if (IS_ENABLED(CONFIG_PPC64)) {
>>> +        temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
>>> +        patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
>>> +    } else {
>>> +        patch_imm_load_insns((unsigned long)p->ainsn.insn, 4, buff + TMPL_INSN_IDX);
>>> +    }
>>
>> It means commit https://github.com/linuxppc/linux/commit/693557ebf407a85ea400a0b501bb97687d8f4856 
>> was not necessary and may be reverted.
> 
> Indeed, I will send a revert for it.
> 

I'm not completely sure it is worth reverting, on an other hand it is pointless anyway to have 
something to convert to a u64 something that cannot be more than 32 bits on a PPC32, so now that we 
have ppc_inst_as_ulong() it is as good I think.

Christophe

^ permalink raw reply

* Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
From: Aneesh Kumar K.V @ 2021-05-20 13:37 UTC (permalink / raw)
  To: Peter Xu
  Cc: npiggin, Nathan Chancellor, linux-mm, kaleshsingh, joel, akpm,
	linuxppc-dev
In-Reply-To: <e6525655-2e51-a0c0-fe54-596cfae9ce21@linux.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:

> On 5/20/21 6:16 PM, Peter Xu wrote:
>> On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote:
>>>> This seems to work at least for my userfaultfd test on shmem, however I don't
>>>> fully understand the commit message [1] on: How do we guarantee we're not
>>>> moving a thp pte?
>>>>
>>>
>>> move_page_tables() checks for pmd_trans_huge() and ends up calling
>>> move_huge_pmd if it is a THP entry.
>> 
>> Sorry to be unclear: what if a huge pud thp?
>> 
>
> I am still checking. Looking at the code before commit 
> c49dd340180260c6239e453263a9a244da9a7c85, I don't see kernel handling 
> huge pud thp. I haven't studied huge pud thp enough to understand 
> whether c49dd340180260c6239e453263a9a244da9a7c85 intent to add that 
> support.
>
> We can do a move_huge_pud() like we do for huge pmd thp. But I am not 
> sure whether we handle those VMA's earlier and restrict mremap on them?

something like this? (not even compile tested). I am still not sure
whether this is really needed or we handle DAX VMA's in some other form.

diff --git a/mm/mremap.c b/mm/mremap.c
index 47c255b60150..037a7bd311f1 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -324,10 +324,51 @@ static inline bool move_normal_pud(struct vm_area_struct *vma,
 }
 #endif
 
+
+static bool move_huge_pud(struct vm_area_struct *vma, unsigned long old_addr,
+			  unsigned long new_addr, pud_t *old_pud, pud_t *new_pud)
+{
+	spinlock_t *old_ptl, *new_ptl;
+	struct mm_struct *mm = vma->vm_mm;
+	pud_t pud;
+
+	/*
+	 * The destination pud shouldn't be established, free_pgtables()
+	 * should have released it.
+	 */
+	if (WARN_ON_ONCE(!pud_none(*new_pud)))
+		return false;
+
+	/*
+	 * We don't have to worry about the ordering of src and dst
+	 * ptlocks because exclusive mmap_lock prevents deadlock.
+	 */
+	old_ptl = pud_lock(vma->vm_mm, old_pud);
+	new_ptl = pud_lockptr(mm, new_pud);
+	if (new_ptl != old_ptl)
+		spin_lock_nested(new_ptl, SINGLE_DEPTH_NESTING);
+
+	/* Clear the pud */
+	pud = *old_pud;
+	pud_clear(old_pud);
+
+	VM_BUG_ON(!pud_none(*new_pud));
+
+	/* Set the new pud */
+	set_pud_at(mm, new_addr, new_pud, pud);
+	flush_pud_tlb_range(vma, old_addr, old_addr + HPAGE_PUD_SIZE);
+	if (new_ptl != old_ptl)
+		spin_unlock(new_ptl);
+	spin_unlock(old_ptl);
+
+	return true;
+}
+
 enum pgt_entry {
 	NORMAL_PMD,
 	HPAGE_PMD,
 	NORMAL_PUD,
+	HPAGE_PUD,
 };
 
 /*
@@ -347,6 +388,7 @@ static __always_inline unsigned long get_extent(enum pgt_entry entry,
 		mask = PMD_MASK;
 		size = PMD_SIZE;
 		break;
+	case HPAGE_PUD:
 	case NORMAL_PUD:
 		mask = PUD_MASK;
 		size = PUD_SIZE;
@@ -395,6 +437,12 @@ static bool move_pgt_entry(enum pgt_entry entry, struct vm_area_struct *vma,
 			move_huge_pmd(vma, old_addr, new_addr, old_entry,
 				      new_entry);
 		break;
+	case HPAGE_PUD:
+		moved = IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE_PUD) &&
+			move_huge_pud(vma, old_addr, new_addr, old_entry,
+				      new_entry);
+		break;
+
 	default:
 		WARN_ON_ONCE(1);
 		break;
@@ -429,15 +477,23 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 		 * PUD level if possible.
 		 */
 		extent = get_extent(NORMAL_PUD, old_addr, old_end, new_addr);
-		if (IS_ENABLED(CONFIG_HAVE_MOVE_PUD) && extent == PUD_SIZE) {
-			pud_t *old_pud, *new_pud;
 
-			old_pud = get_old_pud(vma->vm_mm, old_addr);
-			if (!old_pud)
+		old_pud = get_old_pud(vma->vm_mm, old_addr);
+		if (!old_pud)
+			continue;
+		new_pud = alloc_new_pud(vma->vm_mm, vma, new_addr);
+		if (!new_pud)
+			break;
+		if (pud_trans_huge(*old_pud) || pud_devmap(*old_pud)) {
+			if (extent == HPAGE_PUD_SIZE) {
+				move_pgt_entry(HPAGE_PUD, vma, old_addr, new_addr,
+					       old_pud, new_pud, need_rmap_locks);
+				/* We ignore and continue on error? */
 				continue;
-			new_pud = alloc_new_pud(vma->vm_mm, vma, new_addr);
-			if (!new_pud)
-				break;
+			}
+		} else if (IS_ENABLED(CONFIG_HAVE_MOVE_PUD) && extent == PUD_SIZE) {
+			pud_t *old_pud, *new_pud;
+
 			if (move_pgt_entry(NORMAL_PUD, vma, old_addr, new_addr,
 					   old_pud, new_pud, need_rmap_locks))
 				continue;


>
> Are huge pud thp only allowed with DAX vmas?
>
>
> -aneesh

^ permalink raw reply related

* Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
From: Aneesh Kumar K.V @ 2021-05-20 13:23 UTC (permalink / raw)
  To: Peter Xu
  Cc: npiggin, Nathan Chancellor, linux-mm, kaleshsingh, joel, akpm,
	linuxppc-dev
In-Reply-To: <YKZaLHurH9nJWvbj@t490s>

On 5/20/21 6:16 PM, Peter Xu wrote:
> On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote:
>>> This seems to work at least for my userfaultfd test on shmem, however I don't
>>> fully understand the commit message [1] on: How do we guarantee we're not
>>> moving a thp pte?
>>>
>>
>> move_page_tables() checks for pmd_trans_huge() and ends up calling
>> move_huge_pmd if it is a THP entry.
> 
> Sorry to be unclear: what if a huge pud thp?
> 

I am still checking. Looking at the code before commit 
c49dd340180260c6239e453263a9a244da9a7c85, I don't see kernel handling 
huge pud thp. I haven't studied huge pud thp enough to understand 
whether c49dd340180260c6239e453263a9a244da9a7c85 intent to add that 
support.

We can do a move_huge_pud() like we do for huge pmd thp. But I am not 
sure whether we handle those VMA's earlier and restrict mremap on them?

Are huge pud thp only allowed with DAX vmas?


-aneesh

^ permalink raw reply

* Re: [PATCH 6/9] tty: hvc_console: Fix coding style issues of block comments
From: Xiaofei Tan @ 2021-05-20 13:21 UTC (permalink / raw)
  To: Johan Hovold; +Cc: gregkh, linuxppc-dev, jirislaby, linux-kernel, linuxarm
In-Reply-To: <YKYcFfKiHT39Gyey@hovoldconsulting.com>

Hi Johan,

On 2021/5/20 16:21, Johan Hovold wrote:
> On Tue, May 18, 2021 at 12:01:22PM +0800, Xiaofei Tan wrote:
>> Hi Johan,
>>
>> Thanks for reviewing this patch.
>>
>> On 2021/5/17 22:15, Johan Hovold wrote:
>>> On Mon, May 17, 2021 at 02:37:10PM +0800, Xiaofei Tan wrote:
>>>> Fix coding style issues of block comments, reported by checkpatch.pl.
>>>> Besides, add a period at the end of the sentenses.
>>>>
>>>> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
>>>> ---
>>>>  drivers/tty/hvc/hvc_console.c | 15 ++++++++++-----
>>>>  1 file changed, 10 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
>>>> index 39018e5..a61cdf0 100644
>>>> --- a/drivers/tty/hvc/hvc_console.c
>>>> +++ b/drivers/tty/hvc/hvc_console.c
>>>> @@ -177,7 +177,8 @@ static void hvc_console_print(struct console *co, const char *b,
>>>>  			r = cons_ops[index]->put_chars(vtermnos[index], c, i);
>>>>  			if (r <= 0) {
>>>>  				/* throw away characters on error
>>>> -				 * but spin in case of -EAGAIN */
>>>> +				 * but spin in case of -EAGAIN.
>>>> +				 */
>>>
>>> How is this an improvement? First, the multi-line comment style is
>>>
>>> 	/*
>>> 	 * ...
>>> 	 */
>>>
>>
>> Yes, mostly we use this style. I can follow it if new version is needed.
>
> This is the preferred style outside of networking.
>
>> BTW, How about add the '/*' check into checkpatch.pl?
>
> Checkpatch already has too many checks IMO and I'm a bit surprised that
> it doesn't check this already. Perhaps it's because you used the -f to
> run checkpatch on in-kernel code, which you should not.
>
>>> Second, that sentence is not capitalised so why do add a period?
>>>
>>
>> How about capitalize the sentence, or just remove the period ?
>
> How about just leaving this unchanged?
>

OK
And I will keep the patch 8/9, and combine space issues into
one new patch, and remove the others.

>>> Third, why are you sending checkpatch.pl cleanups for files outside of
>>> staging?
>>>
>>
>> I'm sorry, Is this a rule, or kind of tradition? I've never heard of
>> this before.
>
> Many subsystems reject pure style changes unless you're also doing some
> real changes to the code in question. This is a good default rule, even
> if some maintainers may occasionally accept churn like this.
>
> You appear to be paid to do kernel work. Why don't you start fixing bugs
> or help out reviewing new code instead of sending trivial patches like
> this? We can always need another hand.
>
> But if all you you want is to increase your company patch count then
> please go work in drivers/staging where most trivial style changes are
> currently accepted.
>
>>> Unless doing some real changes to the files in question as well this is
>>> mostly just churn and noise that makes it harder to backport fixes and
>>> do code forensics for no real gain.
>>
>> I'm not sure. But if cleanup patches have made it hard to backport fixes
>> and do code forensics, then the code quality may not be
>> good enough.
>
> No, that has nothing to do with code quality, it's just that you
> introduce noise in the logs and do pointless changes of context which
> makes it harder to use tools like git blame and makes backporting harder
> for no good reason.
>
> Johan
>
> .
>


^ permalink raw reply

* Re: [PATCH 09/31] powerpc/pseries/pci: Add a msi_free() handler to clear XIVE data
From: Cédric Le Goater @ 2021-05-20 12:33 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Thomas Gleixner, Marc Zyngier
In-Reply-To: <20210430080407.4030767-10-clg@kaod.org>

Adding Marc.

On 4/30/21 10:03 AM, Cédric Le Goater wrote:
> The MSI domain clears the IRQ with msi_domain_free(), which calls
> irq_domain_free_irqs_top(), which clears the handler data. This is a
> problem for the XIVE controller since we need to unmap MMIO pages and
> free a specific XIVE structure.
> 
> The 'msi_free()' handler is called before irq_domain_free_irqs_top()
> when the handler data is still available. Use that to clear the XIVE
> controller data.
This feels like a clumsy way of doing so. 

irq_domain_free_irqs_parent() would be my preferred way to clear the 
lowlevel handler data but we can't today. Could there be a smarter way ?

Thanks,

C.


> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>
> ---
>  arch/powerpc/include/asm/xive.h      |  1 +
>  arch/powerpc/platforms/pseries/msi.c | 16 +++++++++++++++-
>  arch/powerpc/sysdev/xive/common.c    |  5 ++++-
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index aa094a8655b0..20ae50ab083c 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -111,6 +111,7 @@ void xive_native_free_vp_block(u32 vp_base);
>  int xive_native_populate_irq_data(u32 hw_irq,
>  				  struct xive_irq_data *data);
>  void xive_cleanup_irq_data(struct xive_irq_data *xd);
> +void xive_irq_free_data(unsigned int virq);
>  void xive_native_free_irq(u32 irq);
>  int xive_native_configure_irq(u32 hw_irq, u32 target, u8 prio, u32 sw_irq);
>  
> diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
> index a41c448520d4..da9d63a088bb 100644
> --- a/arch/powerpc/platforms/pseries/msi.c
> +++ b/arch/powerpc/platforms/pseries/msi.c
> @@ -529,6 +529,19 @@ static int pseries_msi_ops_prepare(struct irq_domain *domain, struct device *dev
>  	return rtas_prepare_msi_irqs(pdev, nvec, type, arg);
>  }
>  
> +/*
> + * ->msi_free() is called before irq_domain_free_irqs_top() when the
> + * handler data is still available. Use that to clear the XIVE
> + * controller data.
> + */
> +static void pseries_msi_ops_msi_free(struct irq_domain *domain,
> +				     struct msi_domain_info *info,
> +				     unsigned int irq)
> +{
> +	if (xive_enabled())
> +		xive_irq_free_data(irq);
> +}
> +
>  /*
>   * RTAS can not disable one MSI at a time. It's all or nothing. Do it
>   * at the end after all IRQs have been freed.
> @@ -546,6 +559,7 @@ static void pseries_msi_domain_free_irqs(struct irq_domain *domain,
>  
>  static struct msi_domain_ops pseries_pci_msi_domain_ops = {
>  	.msi_prepare	= pseries_msi_ops_prepare,
> +	.msi_free	= pseries_msi_ops_msi_free,
>  	.domain_free_irqs = pseries_msi_domain_free_irqs,
>  };
>  
> @@ -660,7 +674,7 @@ static void pseries_irq_domain_free(struct irq_domain *domain, unsigned int virq
>  
>  	pr_debug("%s bridge %pOF %d #%d\n", __func__, phb->dn, virq, nr_irqs);
>  
> -	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
> +	/* XIVE domain data is cleared through ->msi_free() */
>  }
>  
>  static const struct irq_domain_ops pseries_irq_domain_ops = {
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 3485baf9ec8c..191cd80ec534 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -980,6 +980,8 @@ EXPORT_SYMBOL_GPL(is_xive_irq);
>  
>  void xive_cleanup_irq_data(struct xive_irq_data *xd)
>  {
> +	pr_debug("%s for HW %x\n", __func__, xd->hw_irq);
> +
>  	if (xd->eoi_mmio) {
>  		unmap_kernel_range((unsigned long)xd->eoi_mmio,
>  				   1u << xd->esb_shift);
> @@ -1025,7 +1027,7 @@ static int xive_irq_alloc_data(unsigned int virq, irq_hw_number_t hw)
>  	return 0;
>  }
>  
> -static void xive_irq_free_data(unsigned int virq)
> +void xive_irq_free_data(unsigned int virq)
>  {
>  	struct xive_irq_data *xd = irq_get_handler_data(virq);
>  
> @@ -1035,6 +1037,7 @@ static void xive_irq_free_data(unsigned int virq)
>  	xive_cleanup_irq_data(xd);
>  	kfree(xd);
>  }
> +EXPORT_SYMBOL_GPL(xive_irq_free_data);
>  
>  #ifdef CONFIG_SMP
>  
> 


^ permalink raw reply

* Re: [PATCH 00/16] Rid W=1 warnings from Char
From: Arnd Bergmann @ 2021-05-20 12:56 UTC (permalink / raw)
  To: Lee Jones
  Cc: Tyrel Datwyler, Jason A. Donenfeld, David Airlie, Clemens Ladisch,
	C. Scott Ananian, Paul Mackerras, Kylene Hall, Jerome Glisse,
	Peter Huewe, Michael Neuling, Herbert Xu, Dave Safford,
	Harald Welte, Jason Gunthorpe, Kanoj Sarcar, David Woodhouse,
	Bob Picco, Uwe Kleine-König, Thirupathaiah Annapureddy, cs.c,
	Lijun Pan, Reiner Sailer, Matt Mackall, van Doorn,
	Theodore Ts'o, Greg Kroah-Hartman, Linux Kernel Mailing List,
	Paul Fulghum, Jarkko Sakkinen,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, linux-integrity,
	linuxppc-dev
In-Reply-To: <20210520121347.3467794-1-lee.jones@linaro.org>

On Thu, May 20, 2021 at 2:13 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> This set is part of a larger effort attempting to clean-up W=1
> kernel builds, which are currently overwhelmingly riddled with
> niggly little warnings.
>
> Lee Jones (16):
>   char: pcmcia: cm4000_cs: Remove unused variable 'tmp'
>   char: pcmcia: cm4040_cs: Remove unused variable 'uc'
>   char: random: Include header containing our prototypes
>   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
>   char: pcmcia: synclink_cs: Fix a bunch of kernel-doc issues
>   char: applicom: Remove 3 unused variables 'ret' and 2 instances of
>     'byte_reset_it'
>   char: tpm: tpm1-cmd: Fix a couple of misnamed functions
>   char: tpm: tpm_ftpm_tee: Fix a couple of kernel-doc misdemeanours
>   char: agp: backend: Demote some non-conformant kernel-doc headers
>   char: agp: frontend: Include header file containing our prototypes
>   char: agp: via-agp: Remove unused variable 'current_size'
>   char: hpet: Remove unused variable 'm'
>   char: agp: generic: Place braces around optimised out function in if()
>   char: agp: uninorth-agp: Remove unused variable 'size'
>   char: hw_random: pseries-rng: Demote non-conformant kernel-doc header
>   char: mem: Provide local prototype for non-static function

Thanks a lot!

I've looked all the patches now and commented on patches 6 and 16.
With my comments addressed

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

       Arnd

^ permalink raw reply

* Re: [PATCH 31/31] genirq: Improve "hwirq" output in /proc and /sys/
From: Thomas Gleixner @ 2021-05-20 12:57 UTC (permalink / raw)
  To: Cédric Le Goater, linuxppc-dev
In-Reply-To: <14b71a4b-fac6-a1c0-1707-436c86b9b6ec@kaod.org>

On Thu, May 20 2021 at 14:27, Cédric Le Goater wrote:
> On 5/14/21 10:49 PM, Thomas Gleixner wrote:
>> On Fri, Apr 30 2021 at 10:04, Cédric Le Goater wrote:
>>> The HW IRQ numbers generated by the PCI MSI layer can be quite large
>>> on a pSeries machine when running under the IBM Hypervisor and they
>>> appear as negative. Use '%u' to show them correctly.
>>>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>  kernel/irq/irqdesc.c | 2 +-
>>>  kernel/irq/proc.c    | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
>>> index cc1a09406c6e..85054eb2ae51 100644
>>> --- a/kernel/irq/irqdesc.c
>>> +++ b/kernel/irq/irqdesc.c
>>> @@ -188,7 +188,7 @@ static ssize_t hwirq_show(struct kobject *kobj,
>>>  
>>>  	raw_spin_lock_irq(&desc->lock);
>>>  	if (desc->irq_data.domain)
>>> -		ret = sprintf(buf, "%d\n", (int)desc->irq_data.hwirq);
>>> +		ret = sprintf(buf, "%u\n", (int)desc->irq_data.hwirq);
>> 
>> Which makes the (int) cast pointless, right?
>
> Well, hwirq is a long.

And that makes '%u' plus an int casted argument any more correct?

Aside of that hwirq is an unsigned long type and not long.

> Would you prefer a "%lu" for both ?

That's the obvious right thing to do, isn't it?

Thanks,

        tglx

^ permalink raw reply

* Re: [PATCH v5 3/9] mm/mremap: Use pmd/pud_poplulate to update page table entries
From: Peter Xu @ 2021-05-20 12:46 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: npiggin, Nathan Chancellor, linux-mm, kaleshsingh, joel, akpm,
	linuxppc-dev
In-Reply-To: <6e0dbb76-2b33-53f1-246e-30cec2b871e2@linux.ibm.com>

On Thu, May 20, 2021 at 01:56:54PM +0530, Aneesh Kumar K.V wrote:
> > This seems to work at least for my userfaultfd test on shmem, however I don't
> > fully understand the commit message [1] on: How do we guarantee we're not
> > moving a thp pte?
> > 
> 
> move_page_tables() checks for pmd_trans_huge() and ends up calling
> move_huge_pmd if it is a THP entry.

Sorry to be unclear: what if a huge pud thp?

-- 
Peter Xu


^ permalink raw reply

* Re: [PATCH v5 5/9] powerpc/mm/book3s64: Update tlb flush routines to take a page walk cache flush argument
From: Segher Boessenkool @ 2021-05-20 12:17 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Aneesh Kumar K.V, npiggin, linux-mm, kaleshsingh, joel, akpm,
	linuxppc-dev, Guenter Roeck
In-Reply-To: <87y2c9j1ov.fsf@mpe.ellerman.id.au>

Hi!

On Thu, May 20, 2021 at 05:37:20PM +1000, Michael Ellerman wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > On Tue, May 18, 2021 at 07:45:14PM -0500, Segher Boessenkool wrote:
> >> On Wed, May 19, 2021 at 10:26:22AM +1000, Michael Ellerman wrote:
> >> > Guenter Roeck <linux@roeck-us.net> writes:
> >> > > Ah, sorry. I wasn't aware that the following is valid C code
> >> > >
> >> > > void f1()
> >> > > {
> >> > >      return f2();
> >> > >      ^^^^^^
> >> > > }
> >> > >
> >> > > as long as f2() is void as well. Confusing, but we live and learn.
> >> > 
> >> > It might be valid, but it's still bad IMHO.
> >> > 
> >> > It's confusing to readers, and serves no useful purpose.
> >> 
> >> And it actually explicitly is undefined behaviour in C90 already
> >> (3.6.6.4 in C90, 6.8.6.4 in C99 and later).
> 
> We use gnu89, which presumably does not make it UB.

Indeed.  That is kind of implied by the "as a GNU extension" below, but
some explicit statement would be better, yup.

> > ... but there is a GCC extension that allows this by default:
> > <https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wreturn-type>
> >   For C only, warn about a 'return' statement with an expression in a
> >   function whose return type is 'void', unless the expression type is
> >   also 'void'.  As a GNU extension, the latter case is accepted
> >   without a warning unless '-Wpedantic' is used.
> 
> There's no chance we'll ever enable -Wpedantic,

Good, because -pedantic adds a lot of much more annoying warnings as
well.  I find this extension questionable (like Guenter says it is
confusing and has no purpose), so the only thing it is "good" for is it
causes long email threads ;-)

Other than those things it is harmless though.

> so I guess it's allowed
> for practical purposes. I guess clang must accept it too or we'd be
> seeing warnings from it.

Yup.


Segher

^ permalink raw reply

* Re: [PATCH 1/1] powerpc/ps3: Fix error return code in ps3_register_devices()
From: Michael Ellerman @ 2021-05-20 12:20 UTC (permalink / raw)
  To: Zhen Lei, Geoff Levand, Benjamin Herrenschmidt, Paul Mackerras,
	linuxppc-dev
  Cc: Zhen Lei
In-Reply-To: <20210518065853.7590-1-thunder.leizhen@huawei.com>

Zhen Lei <thunder.leizhen@huawei.com> writes:
> When call ps3_start_probe_thread() failed, further initialization should
> be stopped and the returned error code should be propagated.

It's not clear to me that's a good change.

> diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
> index e87360a0fb40..9b6d8ca8fc01 100644
> --- a/arch/powerpc/platforms/ps3/device-init.c
> +++ b/arch/powerpc/platforms/ps3/device-init.c
> @@ -955,6 +955,8 @@ static int __init ps3_register_devices(void)
>  	/* ps3_repository_dump_bus_info(); */
>  
>  	result = ps3_start_probe_thread(PS3_BUS_TYPE_STORAGE);
> +	if (result < 0)
> +		return result;

If you bail out here you skip:

>  	ps3_register_vuart_devices();


Which I suspect means there will be no console output?

Presumably the system won't boot if the probe thread fails, but it might
at least print an oops, whereas if we return we might get nothing at
all. Though I'm just guessing, I don't know this code that well.

Anyway please leave this code alone unless you're willing to test your
changes, or at least provide a more thorough justification for them.

cheers

^ permalink raw reply

* Re: [PATCH 15/31] KVM: PPC: Book3S HV: XIVE: Fix mapping of passthrough interrupts
From: Cédric Le Goater @ 2021-05-20 12:09 UTC (permalink / raw)
  To: Marc Zyngier, Thomas Gleixner; +Cc: linuxppc-dev
In-Reply-To: <878s4g5lfs.wl-maz@kernel.org>

On 5/15/21 12:40 PM, Marc Zyngier wrote:
> On Fri, 14 May 2021 21:51:51 +0100,
> Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> On Fri, Apr 30 2021 at 10:03, Cédric Le Goater wrote:
>>
>> CC: +Marc
> 
> Thanks Thomas.
> 
>>
>>> PCI MSI interrupt numbers are now mapped in a PCI-MSI domain but the
>>> underlying calls handling the passthrough of the interrupt in the
>>> guest need a number in the XIVE IRQ domain.
>>>
>>> Use the IRQ data mapped in the XIVE IRQ domain and not the one in the
>>> PCI-MSI domain.
>>>
>>> Exporting irq_get_default_host() might not be the best solution.
>>>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Paul Mackerras <paulus@ozlabs.org>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>  arch/powerpc/kvm/book3s_xive.c | 3 ++-
>>>  kernel/irq/irqdomain.c         | 1 +
>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
>>> index 3a7da42bed57..81b9f4fc3978 100644
>>> --- a/arch/powerpc/kvm/book3s_xive.c
>>> +++ b/arch/powerpc/kvm/book3s_xive.c
>>> @@ -861,7 +861,8 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,
>>>  	struct kvmppc_xive *xive = kvm->arch.xive;
>>>  	struct kvmppc_xive_src_block *sb;
>>>  	struct kvmppc_xive_irq_state *state;
>>> -	struct irq_data *host_data = irq_get_irq_data(host_irq);
>>> +	struct irq_data *host_data =
>>> +		irq_domain_get_irq_data(irq_get_default_host(), host_irq);
>>>  	unsigned int hw_irq = (unsigned int)irqd_to_hwirq(host_data);
>>>  	u16 idx;
>>>  	u8 prio;
>>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>>> index d10ab1d689d5..8a073d1ce611 100644
>>> --- a/kernel/irq/irqdomain.c
>>> +++ b/kernel/irq/irqdomain.c
>>> @@ -481,6 +481,7 @@ struct irq_domain *irq_get_default_host(void)
>>>  {
>>>  	return irq_default_domain;
>>>  }
>>> +EXPORT_SYMBOL_GPL(irq_get_default_host);
>>>  
>>>  static void irq_domain_clear_mapping(struct irq_domain *domain,
>>>  				     irq_hw_number_t hwirq)
>>
> 
> Is there any reason why we should add more users of the "default host"
> fallback? I would really hope that new code would actually track their
> irqdomain in a more fine-grained way, specially when using the
> hierarchical MSi setup, which seems to be the goal of this series.
> 
> Don't you have enough topology information that you can make use of to
> correctly assign a domain identifier (of_node or otherwise)?


PHB have a node ID and this is taken into account by the MSI domains.
However, one thing PPC (pSeries and PowerNV) lacks is an interrupt
controller node per chip which makes the IRQ domain hierarchy a bit
incomplete.

It will be difficult to change the pseries platform (VM) since the
PAPR architecture only specifies a single interrupt domain for the
whole machine. The PowerNV platform is designed in a similar way
(because the pseries platform preexisted) and the OPAL firmware hides
the interrupt controllers of each chip behind a single node. The
underlying topology is encoded in HW interrupt numbers. This is a bit
unfortunate since some PowerNV Linux drivers need that information.
Rewriting a new interrupt controller driver in OPAL would be a lot of
work and it won't happen any time soon. But it's feasible.

All that to say that we have a default IRQ domain on these platforms
and not one  IRQ domain per node/chip.

Also, there are two types of interrupt models to consider: the older
XICS (for P8/P7 processors) and the newer XIVE (for P9/P10).

Regarding MSI passthrough, the XIVE side is simpler (I can't believe I
am saying that, XIVE is anything but simple) and I think we can rework
kvmppc_xive_set_mapped() and xive_irq_set_vcpu_affinity() to remove
the IRQ domain bypass. 

XICS adds optimizations for passthrough done in real mode:

 e3c13e56a471 ("KVM: PPC: Book3S HV: Handle passthrough interrupts in guest")
 5d375199ea96 ("KVM: PPC: Book3S HV: Set server for passed-through interrupts")

That's a ~10% bandwidth improvements on CX5 adapters, it's good to
have but they are much more complex to rework. I took some time to
look for a solution for these because of the use of irq_to_desc() and
the use of the host IRQ in the XICS domain which are ugly but nothing
comes to mind yet.

For the time being, I think these changes bypassing the IRQ domains
are fine. I need some more time to mature an alternative.

Thanks,

C. 


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox