Re: [PATCH v5 09/22] powerpc/syscall: Make interrupt.c buildable on PPC32
Excerpts from Christophe Leroy's message of February 9, 2021 4:02 pm:
>
>
> Le 09/02/2021 à 02:27, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
>>> To allow building interrupt.c on PPC32, ifdef out specific PPC64
>>> code or use helpers which are available on both PP32 and PPC64
>>>
>>> Modify Makefile to always build interrupt.o
>>>
>>> Signed-off-by: Christophe Leroy
>>> ---
>>> v5:
>>> - Also for interrupt exit preparation
>>> - Opted out kuap related code, ppc32 keeps it in ASM for the time being
>>> ---
>>> arch/powerpc/kernel/Makefile| 4 ++--
>>> arch/powerpc/kernel/interrupt.c | 31 ---
>>> 2 files changed, 26 insertions(+), 9 deletions(-)
>>>
>
>>> diff --git a/arch/powerpc/kernel/interrupt.c
>>> b/arch/powerpc/kernel/interrupt.c
>>> index d6be4f9a67e5..2dac4d2bb1cf 100644
>>> --- a/arch/powerpc/kernel/interrupt.c
>>> +++ b/arch/powerpc/kernel/interrupt.c
>>> @@ -39,7 +39,7 @@ notrace long system_call_exception(long r3, long r4, long
>>> r5,
>>> BUG_ON(!(regs->msr & MSR_RI));
>>> BUG_ON(!(regs->msr & MSR_PR));
>>> BUG_ON(!FULL_REGS(regs));
>>> - BUG_ON(regs->softe != IRQS_ENABLED);
>>> + BUG_ON(arch_irq_disabled_regs(regs));
>>>
>>> #ifdef CONFIG_PPC_PKEY
>>> if (mmu_has_feature(MMU_FTR_PKEY)) {
>>> @@ -65,7 +65,9 @@ notrace long system_call_exception(long r3, long r4, long
>>> r5,
>>> isync();
>>> } else
>>> #endif
>>> +#ifdef CONFIG_PPC64
>>> kuap_check_amr();
>>> +#endif
>>
>> Wouldn't mind trying to get rid of these ifdefs at some point, but
>> there's some kuap / keys changes going on recently so I'm happy enough
>> to let this settle then look at whether we can refactor.
>
> I have a follow up series that implements interrupts entries/exits in C and
> that removes all kuap
> assembly, I will likely release it as RFC later today.
>
>>
>>>
>>> account_cpu_user_entry();
>>>
>>> @@ -318,7 +323,7 @@ notrace unsigned long syscall_exit_prepare(unsigned
>>> long r3,
>>> return ret;
>>> }
>>>
>>> -#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
>>> +#ifndef CONFIG_PPC_BOOK3E_64 /* BOOK3E not yet using this */
>>> notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs,
>>> unsigned long msr)
>>> {
>>> #ifdef CONFIG_PPC_BOOK3E
>>
>> Why are you building this for 32? I don't mind if it's just to keep
>> things similar and make it build for now, but you're not using it yet,
>> right?
>
> The series using that will follow, I thought it would be worth doing this at
> once.
Yeah that's fine by me then.
Thanks,
Nick
Re: [PATCH v5 09/22] powerpc/syscall: Make interrupt.c buildable on PPC32
Le 09/02/2021 à 02:27, Nicholas Piggin a écrit :
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
To allow building interrupt.c on PPC32, ifdef out specific PPC64
code or use helpers which are available on both PP32 and PPC64
Modify Makefile to always build interrupt.o
Signed-off-by: Christophe Leroy
---
v5:
- Also for interrupt exit preparation
- Opted out kuap related code, ppc32 keeps it in ASM for the time being
---
arch/powerpc/kernel/Makefile| 4 ++--
arch/powerpc/kernel/interrupt.c | 31 ---
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index d6be4f9a67e5..2dac4d2bb1cf 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -39,7 +39,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
BUG_ON(!(regs->msr & MSR_RI));
BUG_ON(!(regs->msr & MSR_PR));
BUG_ON(!FULL_REGS(regs));
- BUG_ON(regs->softe != IRQS_ENABLED);
+ BUG_ON(arch_irq_disabled_regs(regs));
#ifdef CONFIG_PPC_PKEY
if (mmu_has_feature(MMU_FTR_PKEY)) {
@@ -65,7 +65,9 @@ notrace long system_call_exception(long r3, long r4, long r5,
isync();
} else
#endif
+#ifdef CONFIG_PPC64
kuap_check_amr();
+#endif
Wouldn't mind trying to get rid of these ifdefs at some point, but
there's some kuap / keys changes going on recently so I'm happy enough
to let this settle then look at whether we can refactor.
I have a follow up series that implements interrupts entries/exits in C and that removes all kuap
assembly, I will likely release it as RFC later today.
account_cpu_user_entry();
@@ -318,7 +323,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
return ret;
}
-#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
+#ifndef CONFIG_PPC_BOOK3E_64 /* BOOK3E not yet using this */
notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs,
unsigned long msr)
{
#ifdef CONFIG_PPC_BOOK3E
Why are you building this for 32? I don't mind if it's just to keep
things similar and make it build for now, but you're not using it yet,
right?
The series using that will follow, I thought it would be worth doing this at
once.
Reviewed-by: Nicholas Piggin
Re: [PATCH v5 09/22] powerpc/syscall: Make interrupt.c buildable on PPC32
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> To allow building interrupt.c on PPC32, ifdef out specific PPC64
> code or use helpers which are available on both PP32 and PPC64
>
> Modify Makefile to always build interrupt.o
>
> Signed-off-by: Christophe Leroy
> ---
> v5:
> - Also for interrupt exit preparation
> - Opted out kuap related code, ppc32 keeps it in ASM for the time being
> ---
> arch/powerpc/kernel/Makefile| 4 ++--
> arch/powerpc/kernel/interrupt.c | 31 ---
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 26ff8c6e06b7..163755b1cef4 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -57,10 +57,10 @@ obj-y := cputable.o
> syscalls.o \
> prom.o traps.o setup-common.o \
> udbg.o misc.o io.o misc_$(BITS).o \
> of_platform.o prom_parse.o firmware.o \
> -hw_breakpoint_constraints.o
> +hw_breakpoint_constraints.o interrupt.o
> obj-y+= ptrace/
> obj-$(CONFIG_PPC64) += setup_64.o \
> -paca.o nvram_64.o note.o interrupt.o
> +paca.o nvram_64.o note.o
> obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
> obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index d6be4f9a67e5..2dac4d2bb1cf 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -39,7 +39,7 @@ notrace long system_call_exception(long r3, long r4, long
> r5,
> BUG_ON(!(regs->msr & MSR_RI));
> BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(!FULL_REGS(regs));
> - BUG_ON(regs->softe != IRQS_ENABLED);
> + BUG_ON(arch_irq_disabled_regs(regs));
>
> #ifdef CONFIG_PPC_PKEY
> if (mmu_has_feature(MMU_FTR_PKEY)) {
> @@ -65,7 +65,9 @@ notrace long system_call_exception(long r3, long r4, long
> r5,
> isync();
> } else
> #endif
> +#ifdef CONFIG_PPC64
> kuap_check_amr();
> +#endif
Wouldn't mind trying to get rid of these ifdefs at some point, but
there's some kuap / keys changes going on recently so I'm happy enough
to let this settle then look at whether we can refactor.
>
> account_cpu_user_entry();
>
> @@ -77,7 +79,7 @@ notrace long system_call_exception(long r3, long r4, long
> r5,
>* frame, or if the unwinder was taught the first stack frame always
>* returns to user with IRQS_ENABLED, this store could be avoided!
>*/
> - regs->softe = IRQS_ENABLED;
> + irq_soft_mask_regs_set_state(regs, IRQS_ENABLED);
>
> local_irq_enable();
>
> @@ -151,6 +153,7 @@ static notrace inline bool
> __prep_irq_for_enabled_exit(bool clear_ri)
> __hard_EE_RI_disable();
> else
> __hard_irq_disable();
> +#ifdef CONFIG_PPC64
> if (unlikely(lazy_irq_pending_nocheck())) {
> /* Took an interrupt, may have more exit work to do. */
> if (clear_ri)
> @@ -162,7 +165,7 @@ static notrace inline bool
> __prep_irq_for_enabled_exit(bool clear_ri)
> }
> local_paca->irq_happened = 0;
> irq_soft_mask_set(IRQS_ENABLED);
> -
> +#endif
> return true;
> }
>
Do we prefer space before return except in trivial cases?
> @@ -216,7 +219,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long
> r3,
>
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> +#ifdef CONFIG_PPC64
> kuap_check_amr();
> +#endif
>
> regs->result = r3;
>
> @@ -309,7 +314,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long
> r3,
>
> account_cpu_user_exit();
>
> -#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
> +#ifdef CONFIG_PPC_BOOK3S_64 /* BOOK3E and ppc32 not using this */
> /*
>* We do this at the end so that we do context switch with KERNEL AMR
>*/
> @@ -318,7 +323,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long
> r3,
> return ret;
> }
>
> -#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
> +#ifndef CONFIG_PPC_BOOK3E_64 /* BOOK3E not yet using this */
> notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs,
> unsigned long msr)
> {
> #ifdef CONFIG_PPC_BOOK3E
Why are you building this for 32? I don't mind if it's just to keep
things similar and make it build for now, but you're not using it yet,
right?
Reviewed-by: Nicholas Piggin
> @@ -333,14 +338,16 @@ notrace unsigned long
> interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> BUG_ON(!(regs->msr & MSR_RI));
> BUG_ON(!(regs->msr &
[PATCH v5 09/22] powerpc/syscall: Make interrupt.c buildable on PPC32
To allow building interrupt.c on PPC32, ifdef out specific PPC64
code or use helpers which are available on both PP32 and PPC64
Modify Makefile to always build interrupt.o
Signed-off-by: Christophe Leroy
---
v5:
- Also for interrupt exit preparation
- Opted out kuap related code, ppc32 keeps it in ASM for the time being
---
arch/powerpc/kernel/Makefile| 4 ++--
arch/powerpc/kernel/interrupt.c | 31 ---
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 26ff8c6e06b7..163755b1cef4 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -57,10 +57,10 @@ obj-y := cputable.o
syscalls.o \
prom.o traps.o setup-common.o \
udbg.o misc.o io.o misc_$(BITS).o \
of_platform.o prom_parse.o firmware.o \
- hw_breakpoint_constraints.o
+ hw_breakpoint_constraints.o interrupt.o
obj-y += ptrace/
obj-$(CONFIG_PPC64)+= setup_64.o \
- paca.o nvram_64.o note.o interrupt.o
+ paca.o nvram_64.o note.o
obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index d6be4f9a67e5..2dac4d2bb1cf 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -39,7 +39,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
BUG_ON(!(regs->msr & MSR_RI));
BUG_ON(!(regs->msr & MSR_PR));
BUG_ON(!FULL_REGS(regs));
- BUG_ON(regs->softe != IRQS_ENABLED);
+ BUG_ON(arch_irq_disabled_regs(regs));
#ifdef CONFIG_PPC_PKEY
if (mmu_has_feature(MMU_FTR_PKEY)) {
@@ -65,7 +65,9 @@ notrace long system_call_exception(long r3, long r4, long r5,
isync();
} else
#endif
+#ifdef CONFIG_PPC64
kuap_check_amr();
+#endif
account_cpu_user_entry();
@@ -77,7 +79,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
* frame, or if the unwinder was taught the first stack frame always
* returns to user with IRQS_ENABLED, this store could be avoided!
*/
- regs->softe = IRQS_ENABLED;
+ irq_soft_mask_regs_set_state(regs, IRQS_ENABLED);
local_irq_enable();
@@ -151,6 +153,7 @@ static notrace inline bool __prep_irq_for_enabled_exit(bool
clear_ri)
__hard_EE_RI_disable();
else
__hard_irq_disable();
+#ifdef CONFIG_PPC64
if (unlikely(lazy_irq_pending_nocheck())) {
/* Took an interrupt, may have more exit work to do. */
if (clear_ri)
@@ -162,7 +165,7 @@ static notrace inline bool __prep_irq_for_enabled_exit(bool
clear_ri)
}
local_paca->irq_happened = 0;
irq_soft_mask_set(IRQS_ENABLED);
-
+#endif
return true;
}
@@ -216,7 +219,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
CT_WARN_ON(ct_state() == CONTEXT_USER);
+#ifdef CONFIG_PPC64
kuap_check_amr();
+#endif
regs->result = r3;
@@ -309,7 +314,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
account_cpu_user_exit();
-#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
+#ifdef CONFIG_PPC_BOOK3S_64 /* BOOK3E and ppc32 not using this */
/*
* We do this at the end so that we do context switch with KERNEL AMR
*/
@@ -318,7 +323,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
return ret;
}
-#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
+#ifndef CONFIG_PPC_BOOK3E_64 /* BOOK3E not yet using this */
notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs,
unsigned long msr)
{
#ifdef CONFIG_PPC_BOOK3E
@@ -333,14 +338,16 @@ notrace unsigned long interrupt_exit_user_prepare(struct
pt_regs *regs, unsigned
BUG_ON(!(regs->msr & MSR_RI));
BUG_ON(!(regs->msr & MSR_PR));
BUG_ON(!FULL_REGS(regs));
- BUG_ON(regs->softe != IRQS_ENABLED);
+ BUG_ON(arch_irq_disabled_regs(regs));
CT_WARN_ON(ct_state() == CONTEXT_USER);
/*
* We don't need to restore AMR on the way back to userspace for KUAP.
* AMR can only have been unlocked if we interrupted the kernel.
*/
+#ifdef CONFIG_PPC64
kuap_check_amr();
+#endif
local_irq_save(flags);
@@ -407,7 +414,9 @@ notrace unsigned long interrupt_exit_user_prepare(struct
pt_regs *regs, unsigned
/*
* We do this at the end so that we do context switch with KERNEL AMR
*/
+#ifdef CONFIG_PPC64
