2021-04-15 00:24:48

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v3 1/4] powerpc: Remove probe_user_read_inst()

Its name comes from former probe_user_read() function.
That function is now called copy_from_user_nofault().

probe_user_read_inst() uses copy_from_user_nofault() to read only
a few bytes. It is suboptimal.

It does the same as get_user_inst() but in addition disables
page faults.

But on the other hand, it is not used for the time being. So remove it
for now. If one day it is really needed, we can give it a new name
more in line with today's naming, and implement it using get_user_inst()

Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/inst.h | 3 ---
arch/powerpc/lib/inst.c | 31 -------------------------------
2 files changed, 34 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 19e18af2fac9..2902d4e6a363 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -175,9 +175,6 @@ static inline char *__ppc_inst_as_str(char str[PPC_INST_STR_LEN], struct ppc_ins
__str; \
})

-int probe_user_read_inst(struct ppc_inst *inst,
- struct ppc_inst __user *nip);
-
int probe_kernel_read_inst(struct ppc_inst *inst,
struct ppc_inst *src);

diff --git a/arch/powerpc/lib/inst.c b/arch/powerpc/lib/inst.c
index 9cc17eb62462..c57b3548de37 100644
--- a/arch/powerpc/lib/inst.c
+++ b/arch/powerpc/lib/inst.c
@@ -9,24 +9,6 @@
#include <asm/ppc-opcode.h>

#ifdef CONFIG_PPC64
-int probe_user_read_inst(struct ppc_inst *inst,
- struct ppc_inst __user *nip)
-{
- unsigned int val, suffix;
- int err;
-
- err = copy_from_user_nofault(&val, nip, sizeof(val));
- if (err)
- return err;
- if (get_op(val) == OP_PREFIX) {
- err = copy_from_user_nofault(&suffix, (void __user *)nip + 4, 4);
- *inst = ppc_inst_prefix(val, suffix);
- } else {
- *inst = ppc_inst(val);
- }
- return err;
-}
-
int probe_kernel_read_inst(struct ppc_inst *inst,
struct ppc_inst *src)
{
@@ -45,19 +27,6 @@ int probe_kernel_read_inst(struct ppc_inst *inst,
return err;
}
#else /* !CONFIG_PPC64 */
-int probe_user_read_inst(struct ppc_inst *inst,
- struct ppc_inst __user *nip)
-{
- unsigned int val;
- int err;
-
- err = copy_from_user_nofault(&val, nip, sizeof(val));
- if (!err)
- *inst = ppc_inst(val);
-
- return err;
-}
-
int probe_kernel_read_inst(struct ppc_inst *inst,
struct ppc_inst *src)
{
--
2.25.0


2021-04-15 00:25:14

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v3 4/4] powerpc: Move copy_from_kernel_nofault_inst()

When probe_kernel_read_inst() was created, there was no good place to
put it, so a file called lib/inst.c was dedicated for it.

Since then, probe_kernel_read_inst() has been renamed
copy_from_kernel_nofault_inst(). And mm/maccess.h didn't exist at that
time. Today, mm/maccess.h is related to copy_from_kernel_nofault().

Move copy_from_kernel_nofault_inst() into mm/maccess.c

Signed-off-by: Christophe Leroy <[email protected]>
---
v2: Remove inst.o from Makefile
---
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/inst.c | 26 --------------------------
arch/powerpc/mm/maccess.c | 21 +++++++++++++++++++++
3 files changed, 22 insertions(+), 27 deletions(-)
delete mode 100644 arch/powerpc/lib/inst.c

diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index d4efc182662a..f2c690ee75d1 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -16,7 +16,7 @@ CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
endif

-obj-y += alloc.o code-patching.o feature-fixups.o pmem.o inst.o test_code-patching.o
+obj-y += alloc.o code-patching.o feature-fixups.o pmem.o test_code-patching.o

ifndef CONFIG_KASAN
obj-y += string.o memcmp_$(BITS).o
diff --git a/arch/powerpc/lib/inst.c b/arch/powerpc/lib/inst.c
deleted file mode 100644
index e554d1357f2f..000000000000
--- a/arch/powerpc/lib/inst.c
+++ /dev/null
@@ -1,26 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Copyright 2020, IBM Corporation.
- */
-
-#include <linux/uaccess.h>
-#include <asm/disassemble.h>
-#include <asm/inst.h>
-#include <asm/ppc-opcode.h>
-
-int copy_inst_from_kernel_nofault(struct ppc_inst *inst, struct ppc_inst *src)
-{
- unsigned int val, suffix;
- int err;
-
- err = copy_from_kernel_nofault(&val, src, sizeof(val));
- if (err)
- return err;
- if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
- err = copy_from_kernel_nofault(&suffix, (void *)src + 4, 4);
- *inst = ppc_inst_prefix(val, suffix);
- } else {
- *inst = ppc_inst(val);
- }
- return err;
-}
diff --git a/arch/powerpc/mm/maccess.c b/arch/powerpc/mm/maccess.c
index fa9a7a718fc6..a3c30a884076 100644
--- a/arch/powerpc/mm/maccess.c
+++ b/arch/powerpc/mm/maccess.c
@@ -3,7 +3,28 @@
#include <linux/uaccess.h>
#include <linux/kernel.h>

+#include <asm/disassemble.h>
+#include <asm/inst.h>
+#include <asm/ppc-opcode.h>
+
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)
+{
+ unsigned int val, suffix;
+ int err;
+
+ err = copy_from_kernel_nofault(&val, src, sizeof(val));
+ if (err)
+ return err;
+ if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
+ err = copy_from_kernel_nofault(&suffix, (void *)src + 4, 4);
+ *inst = ppc_inst_prefix(val, suffix);
+ } else {
+ *inst = ppc_inst(val);
+ }
+ return err;
+}
--
2.25.0

2021-04-21 16:28:21

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] powerpc: Remove probe_user_read_inst()

On Wed, 14 Apr 2021 13:08:40 +0000 (UTC), Christophe Leroy wrote:
> Its name comes from former probe_user_read() function.
> That function is now called copy_from_user_nofault().
>
> probe_user_read_inst() uses copy_from_user_nofault() to read only
> a few bytes. It is suboptimal.
>
> It does the same as get_user_inst() but in addition disables
> page faults.
>
> [...]

Applied to powerpc/next.

[1/4] powerpc: Remove probe_user_read_inst()
https://git.kernel.org/powerpc/c/6ac7897f08e04b47df3955d7691652e9d12d4068
[2/4] powerpc: Make probe_kernel_read_inst() common to PPC32 and PPC64
https://git.kernel.org/powerpc/c/6449078d50111c839bb7156c3b99b9def80eed42
[3/4] powerpc: Rename probe_kernel_read_inst()
https://git.kernel.org/powerpc/c/41d6cf68b5f611934bcc6a7d4a1a2d9bfd04b420
[4/4] powerpc: Move copy_from_kernel_nofault_inst()
https://git.kernel.org/powerpc/c/39352430aaa05fbe4ba710231c70b334513078f2

cheers