livepatch: Remove klp_arch_set_pc() and asm/livepatch.h

[PATCH] livepatch: Remove klp_arch_set_pc() and asm/livepatch.h

Posted by Christophe Leroy 4 years, 2 months ago

All three versions of klp_arch_set_pc() do exactly the same: they
call ftrace_instruction_pointer_set().

Call ftrace_instruction_pointer_set() directly and remove
klp_arch_set_pc().

As klp_arch_set_pc() was the only thing remaining in asm/livepatch.h
on x86 and s390, remove asm/livepatch.h

livepatch.h remains on powerpc but its content is exclusively used
by powerpc specific code.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 MAINTAINERS                          |  2 --
 arch/powerpc/include/asm/livepatch.h | 10 +---------
 arch/powerpc/kernel/irq.c            |  1 -
 arch/powerpc/kernel/setup_64.c       |  2 +-
 arch/s390/include/asm/livepatch.h    | 22 ----------------------
 arch/x86/include/asm/livepatch.h     | 20 --------------------
 include/linux/livepatch.h            |  2 --
 kernel/livepatch/patch.c             |  2 +-
 8 files changed, 3 insertions(+), 58 deletions(-)
 delete mode 100644 arch/s390/include/asm/livepatch.h
 delete mode 100644 arch/x86/include/asm/livepatch.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 1ef8327aef1f..cb5f0d4a8fcc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11253,8 +11253,6 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching.g
 F:	Documentation/ABI/testing/sysfs-kernel-livepatch
 F:	Documentation/livepatch/
 F:	arch/powerpc/include/asm/livepatch.h
-F:	arch/s390/include/asm/livepatch.h
-F:	arch/x86/include/asm/livepatch.h
 F:	include/linux/livepatch.h
 F:	kernel/livepatch/
 F:	lib/livepatch/
diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h
index 1c60094ea0cd..d044a1fd4f44 100644
--- a/arch/powerpc/include/asm/livepatch.h
+++ b/arch/powerpc/include/asm/livepatch.h
@@ -7,17 +7,9 @@
 #ifndef _ASM_POWERPC_LIVEPATCH_H
 #define _ASM_POWERPC_LIVEPATCH_H
 
-#include <linux/module.h>
-#include <linux/ftrace.h>
+#include <linux/sched.h>
 #include <linux/sched/task_stack.h>
 
-#ifdef CONFIG_LIVEPATCH
-static inline void klp_arch_set_pc(struct ftrace_regs *fregs, unsigned long ip)
-{
-	ftrace_instruction_pointer_set(fregs, ip);
-}
-#endif /* CONFIG_LIVEPATCH */
-
 #ifdef CONFIG_LIVEPATCH_64
 static inline void klp_init_thread_info(struct task_struct *p)
 {
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 1c4715a03cd1..8c20226bcc11 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -64,7 +64,6 @@
 #include <asm/machdep.h>
 #include <asm/udbg.h>
 #include <asm/smp.h>
-#include <asm/livepatch.h>
 #include <asm/hw_irq.h>
 #include <asm/softirq_stack.h>
 
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index e547066a06aa..2c1e9a5ac6ca 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -59,7 +59,7 @@
 #include <asm/udbg.h>
 #include <asm/kexec.h>
 #include <asm/code-patching.h>
-#include <asm/livepatch.h>
+#include <asm/ftrace.h>
 #include <asm/opal.h>
 #include <asm/cputhreads.h>
 #include <asm/hw_irq.h>
diff --git a/arch/s390/include/asm/livepatch.h b/arch/s390/include/asm/livepatch.h
deleted file mode 100644
index 5209f223331a..000000000000
--- a/arch/s390/include/asm/livepatch.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * livepatch.h - s390-specific Kernel Live Patching Core
- *
- *  Copyright (c) 2013-2015 SUSE
- *   Authors: Jiri Kosina
- *	      Vojtech Pavlik
- *	      Jiri Slaby
- */
-
-#ifndef ASM_LIVEPATCH_H
-#define ASM_LIVEPATCH_H
-
-#include <linux/ftrace.h>
-#include <asm/ptrace.h>
-
-static inline void klp_arch_set_pc(struct ftrace_regs *fregs, unsigned long ip)
-{
-	ftrace_instruction_pointer_set(fregs, ip);
-}
-
-#endif
diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h
deleted file mode 100644
index 7c5cc6660e4b..000000000000
--- a/arch/x86/include/asm/livepatch.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * livepatch.h - x86-specific Kernel Live Patching Core
- *
- * Copyright (C) 2014 Seth Jennings <sjenning@redhat.com>
- * Copyright (C) 2014 SUSE
- */
-
-#ifndef _ASM_X86_LIVEPATCH_H
-#define _ASM_X86_LIVEPATCH_H
-
-#include <asm/setup.h>
-#include <linux/ftrace.h>
-
-static inline void klp_arch_set_pc(struct ftrace_regs *fregs, unsigned long ip)
-{
-	ftrace_instruction_pointer_set(fregs, ip);
-}
-
-#endif /* _ASM_X86_LIVEPATCH_H */
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 2614247a9781..293e29960c6e 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -16,8 +16,6 @@
 
 #if IS_ENABLED(CONFIG_LIVEPATCH)
 
-#include <asm/livepatch.h>
-
 /* task patch states */
 #define KLP_UNDEFINED	-1
 #define KLP_UNPATCHED	 0
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index c172bf92b576..4c4f5a776d80 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -118,7 +118,7 @@ static void notrace klp_ftrace_handler(unsigned long ip,
 	if (func->nop)
 		goto unlock;
 
-	klp_arch_set_pc(fregs, (unsigned long)func->new_func);
+	ftrace_instruction_pointer_set(fregs, (unsigned long)func->new_func);
 
 unlock:
 	ftrace_test_recursion_unlock(bit);
-- 
2.35.1

Re: [PATCH] livepatch: Remove klp_arch_set_pc() and asm/livepatch.h

Posted by Peter Zijlstra 4 years, 2 months ago

On Mon, Mar 28, 2022 at 08:26:48AM +0200, Christophe Leroy wrote:
> All three versions of klp_arch_set_pc() do exactly the same: they
> call ftrace_instruction_pointer_set().
> 
> Call ftrace_instruction_pointer_set() directly and remove
> klp_arch_set_pc().
> 
> As klp_arch_set_pc() was the only thing remaining in asm/livepatch.h
> on x86 and s390, remove asm/livepatch.h
> 
> livepatch.h remains on powerpc but its content is exclusively used
> by powerpc specific code.

The only remaining thing seems to be a klp_init_thread_info() for ppc64,
but yeah..

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
> index c172bf92b576..4c4f5a776d80 100644
> --- a/kernel/livepatch/patch.c
> +++ b/kernel/livepatch/patch.c
> @@ -118,7 +118,7 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>  	if (func->nop)
>  		goto unlock;
>  
> -	klp_arch_set_pc(fregs, (unsigned long)func->new_func);
> +	ftrace_instruction_pointer_set(fregs, (unsigned long)func->new_func);
>  
>  unlock:
>  	ftrace_test_recursion_unlock(bit);

So ptrace has instruction_pointer_set(), I'm not sure why we have yet
another variant in the ftrace_ namespace. Perhaps also delete that?

Re: [PATCH] livepatch: Remove klp_arch_set_pc() and asm/livepatch.h

Posted by Miroslav Benes 4 years, 2 months ago

On Mon, 28 Mar 2022, Christophe Leroy wrote:

> All three versions of klp_arch_set_pc() do exactly the same: they
> call ftrace_instruction_pointer_set().
> 
> Call ftrace_instruction_pointer_set() directly and remove
> klp_arch_set_pc().
> 
> As klp_arch_set_pc() was the only thing remaining in asm/livepatch.h
> on x86 and s390, remove asm/livepatch.h
> 
> livepatch.h remains on powerpc but its content is exclusively used
> by powerpc specific code.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

Acked-by: Miroslav Benes <mbenes@suse.cz>

M

Re: [PATCH] livepatch: Remove klp_arch_set_pc() and asm/livepatch.h

Posted by Petr Mladek 4 years, 2 months ago

On Mon 2022-03-28 08:26:48, Christophe Leroy wrote:
> All three versions of klp_arch_set_pc() do exactly the same: they
> call ftrace_instruction_pointer_set().
> 
> Call ftrace_instruction_pointer_set() directly and remove
> klp_arch_set_pc().
> 
> As klp_arch_set_pc() was the only thing remaining in asm/livepatch.h
> on x86 and s390, remove asm/livepatch.h
> 
> livepatch.h remains on powerpc but its content is exclusively used
> by powerpc specific code.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

Acked-by: Petr Mladek <pmladek@suse.com>

I am going to take it via livepatch/livepatch.git for 5.19. We are
already in the middle of the merge window and this is not critical.

Best Regards,
Petr

Re: [PATCH] livepatch: Remove klp_arch_set_pc() and asm/livepatch.h

Posted by Christophe Leroy 4 years, 1 month ago


Le 29/03/2022 à 13:22, Petr Mladek a écrit :
> On Mon 2022-03-28 08:26:48, Christophe Leroy wrote:
>> All three versions of klp_arch_set_pc() do exactly the same: they
>> call ftrace_instruction_pointer_set().
>>
>> Call ftrace_instruction_pointer_set() directly and remove
>> klp_arch_set_pc().
>>
>> As klp_arch_set_pc() was the only thing remaining in asm/livepatch.h
>> on x86 and s390, remove asm/livepatch.h
>>
>> livepatch.h remains on powerpc but its content is exclusively used
>> by powerpc specific code.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> 
> Acked-by: Petr Mladek <pmladek@suse.com>
> 
> I am going to take it via livepatch/livepatch.git for 5.19. We are
> already in the middle of the merge window and this is not critical.
> 

I haven't seen it in linux-next.

Do you still plan to take it for 5.19 ?

Thanks
Christophe

Re: [PATCH] livepatch: Remove klp_arch_set_pc() and asm/livepatch.h

Posted by Petr Mladek 4 years, 1 month ago

On Mon 2022-05-23 06:51:47, Christophe Leroy wrote:
> 
> 
> Le 29/03/2022 à 13:22, Petr Mladek a écrit :
> > On Mon 2022-03-28 08:26:48, Christophe Leroy wrote:
> >> All three versions of klp_arch_set_pc() do exactly the same: they
> >> call ftrace_instruction_pointer_set().
> >>
> >> Call ftrace_instruction_pointer_set() directly and remove
> >> klp_arch_set_pc().
> >>
> >> As klp_arch_set_pc() was the only thing remaining in asm/livepatch.h
> >> on x86 and s390, remove asm/livepatch.h
> >>
> >> livepatch.h remains on powerpc but its content is exclusively used
> >> by powerpc specific code.
> >>
> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > 
> > Acked-by: Petr Mladek <pmladek@suse.com>
> > 
> > I am going to take it via livepatch/livepatch.git for 5.19. We are
> > already in the middle of the merge window and this is not critical.
> > 
> 
> I haven't seen it in linux-next.
> 
> Do you still plan to take it for 5.19 ?

Thanks a lot for pointing this out. I have completely forgot about
this patch /o\

I have just pushed it into livepatching/livepatching.git,
branch for-5.19/cleanup.

I am going to create pull request for 5.19 by the end of this week
after it gets a spin in linux-next.

Best Regards,
Petr

Re: [PATCH] livepatch: Remove klp_arch_set_pc() and asm/livepatch.h

Posted by Christophe Leroy 4 years ago

Hi Petr,

Le 23/05/2022 à 17:15, Petr Mladek a écrit :
> On Mon 2022-05-23 06:51:47, Christophe Leroy wrote:
>>
>>
>> Le 29/03/2022 à 13:22, Petr Mladek a écrit :
>>> On Mon 2022-03-28 08:26:48, Christophe Leroy wrote:
>>>> All three versions of klp_arch_set_pc() do exactly the same: they
>>>> call ftrace_instruction_pointer_set().
>>>>
>>>> Call ftrace_instruction_pointer_set() directly and remove
>>>> klp_arch_set_pc().
>>>>
>>>> As klp_arch_set_pc() was the only thing remaining in asm/livepatch.h
>>>> on x86 and s390, remove asm/livepatch.h
>>>>
>>>> livepatch.h remains on powerpc but its content is exclusively used
>>>> by powerpc specific code.
>>>>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>
>>> Acked-by: Petr Mladek <pmladek@suse.com>
>>>
>>> I am going to take it via livepatch/livepatch.git for 5.19. We are
>>> already in the middle of the merge window and this is not critical.
>>>
>>
>> I haven't seen it in linux-next.
>>
>> Do you still plan to take it for 5.19 ?
> 
> Thanks a lot for pointing this out. I have completely forgot about
> this patch /o\
> 
> I have just pushed it into livepatching/livepatching.git,
> branch for-5.19/cleanup.
> 
> I am going to create pull request for 5.19 by the end of this week
> after it gets a spin in linux-next.
> 

I can't see it yet in linus mainline tree. Any issue ?

Thanks
Christophe