Christophe Leroy [off-list ref] writes:

Le 06/05/2020 à 05:40, Jordan Niethe a écrit :

quoted

For powerpc64, redefine the ppc_inst type so both word and prefixed
instructions can be represented. On powerpc32 the type will remain the
same.  Update places which had assumed instructions to be 4 bytes long.

...

quoted

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index c0a35e4586a5..217897927926 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h

@@ -105,11 +105,49 @@ static inline int __access_ok(unsigned long addr, unsigned long size,
  #define __put_user_inatomic(x, ptr) \
  	__put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
  
+#ifdef __powerpc64__

Replace by CONFIG_PPC64

quoted

+#define __get_user_instr(x, ptr)			\
+({							\
+	long __gui_ret = 0;				\
+	unsigned long __gui_ptr = (unsigned long)ptr;	\
+	struct ppc_inst __gui_inst;			\
+	unsigned int prefix, suffix;			\
+	__gui_ret = __get_user(prefix, (unsigned int __user *)__gui_ptr);	\

__get_user() can be costly especially with KUAP. I think you should 
perform a 64 bits read and fallback on a 32 bits read only if the 64 
bits read failed.

I worry that might break something.

It _should_ be safe, but I'm paranoid.

If we think the KUAP overhead is a problem then I think we'd be better
off pulling the KUAP disable/enable into this macro.

quoted

diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 2bd2b752de4f..a8238eff3a31 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c

@@ -84,12 +84,13 @@ static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
  	src = alt_start;
  	dest = start;
  
-	for (; src < alt_end; src++, dest++) {
+	for (; src < alt_end; src = (void *)src + ppc_inst_len(ppc_inst_read(src)),
+	     (dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest)))) {

Can we do this outside the for() for readability ?

I have an idea for cleaning these up, will post it as a follow-up to the series.

quoted

diff --git a/arch/powerpc/lib/inst.c b/arch/powerpc/lib/inst.c
index 08dedd927268..eb6f9ee28ac6 100644
--- a/arch/powerpc/lib/inst.c
+++ b/arch/powerpc/lib/inst.c

@@ -3,9 +3,49 @@
   *  Copyright 2020, IBM Corporation.
   */
  
+#include <asm/ppc-opcode.h>
  #include <linux/uaccess.h>
  #include <asm/inst.h>
  
+#ifdef __powerpc64__
+int probe_user_read_inst(struct ppc_inst *inst,
+			 struct ppc_inst *nip)
+{
+	unsigned int val, suffix;
+	int err;
+
+	err = probe_user_read(&val, nip, sizeof(val));

A user read is costly with KUAP. Can we do a 64 bits read and perform a 
32 bits read only when 64 bits read fails ?

Same comment as above.


cheers