* Re: [PATCH 1/6] powerpc/prom: Introduce early_reserve_mem_old()
From: Christophe Leroy @ 2021-08-19 14:42 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Christophe Leroy
In-Reply-To: <20210819125656.14498-2-clg@kaod.org>
Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
> and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
> compile error with W=1.
>
> arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
> arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not used [-Werror=unused-but-set-variable]
> __be64 *reserve_map;
> ^~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> Christophe, I think you had comments on this one ? Yes, I am being a bit lazy.
Yeah, my comment was to leave thing almost as is, just drop the #ifdef CONFIG_PPC32 and instead put
something like:
if (!IS_ENABLED(CONFIG_PPC32))
return;
>
> arch/powerpc/kernel/prom.c | 37 ++++++++++++++++++++-----------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index f620e04dc9bf..52869d12bc1d 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -621,27 +621,14 @@ static void __init early_reserve_mem_dt(void)
> }
> }
>
> -static void __init early_reserve_mem(void)
> +static void __init early_reserve_mem_old(void)
Why old ? Because ppc32 ?
I think that's more changes than needed.
> {
> __be64 *reserve_map;
>
> reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
> fdt_off_mem_rsvmap(initial_boot_params));
>
> - /* Look for the new "reserved-regions" property in the DT */
> - early_reserve_mem_dt();
> -
> -#ifdef CONFIG_BLK_DEV_INITRD
> - /* Then reserve the initrd, if any */
> - if (initrd_start && (initrd_end > initrd_start)) {
> - memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
> - ALIGN(initrd_end, PAGE_SIZE) -
> - ALIGN_DOWN(initrd_start, PAGE_SIZE));
> - }
> -#endif /* CONFIG_BLK_DEV_INITRD */
> -
> -#ifdef CONFIG_PPC32
> - /*
> + /*
> * Handle the case where we might be booting from an old kexec
> * image that setup the mem_rsvmap as pairs of 32-bit values
> */
> @@ -659,9 +646,25 @@ static void __init early_reserve_mem(void)
> DBG("reserving: %x -> %x\n", base_32, size_32);
> memblock_reserve(base_32, size_32);
> }
> - return;
> }
> -#endif
> +}
> +
> +static void __init early_reserve_mem(void)
> +{
> + /* Look for the new "reserved-regions" property in the DT */
> + early_reserve_mem_dt();
> +
> +#ifdef CONFIG_BLK_DEV_INITRD
> + /* Then reserve the initrd, if any */
> + if (initrd_start && (initrd_end > initrd_start)) {
> + memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
> + ALIGN(initrd_end, PAGE_SIZE) -
> + ALIGN_DOWN(initrd_start, PAGE_SIZE));
> + }
> +#endif /* CONFIG_BLK_DEV_INITRD */
> +
> + if (IS_ENABLED(CONFIG_PPC32))
> + early_reserve_mem_old();
> }
>
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>
^ permalink raw reply
* Re: [PATCH 3/6] KVM: PPC: Book3S PR: Declare kvmppc_handle_exit_pr()
From: Christophe Leroy @ 2021-08-19 14:44 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Christophe Leroy
In-Reply-To: <20210819125656.14498-4-clg@kaod.org>
Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
> This fixes a compile error with W=1.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> arch/powerpc/kvm/book3s.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
> index 740e51def5a5..c08f93b7f523 100644
> --- a/arch/powerpc/kvm/book3s.h
> +++ b/arch/powerpc/kvm/book3s.h
> @@ -24,6 +24,7 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu,
> int sprn, ulong *spr_val);
> extern int kvmppc_book3s_init_pr(void);
> extern void kvmppc_book3s_exit_pr(void);
> +extern int kvmppc_handle_exit_pr(struct kvm_vcpu *vcpu, unsigned int exit_nr);
Don't add new 'extern' keywords, they are useless and pointless for functions prototypes.
>
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> extern void kvmppc_emulate_tabort(struct kvm_vcpu *vcpu, int ra_val);
>
^ permalink raw reply
* Re: [PATCH 5/6] audit: Declare ppc32_classify_syscall()
From: Christophe Leroy @ 2021-08-19 14:56 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Christophe Leroy
In-Reply-To: <20210819125656.14498-6-clg@kaod.org>
Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
> This fixes a compile error with W=1.
>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> I don't think this is correct. Which file could we use ?
I think you can completely remove ppc32_classify_syscall(), and instead add the following in the
default case in audit_classify_syscall():
default:
+ if (IS_ENABLED(CONFIG_PPC64) && abi == AUDIT_ARCH_PPC)
+ return 1;
return 0;
>
> arch/powerpc/include/asm/unistd.h | 3 +++
> arch/powerpc/kernel/audit.c | 1 -
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
> index b541c690a31c..d9025a7e973c 100644
> --- a/arch/powerpc/include/asm/unistd.h
> +++ b/arch/powerpc/include/asm/unistd.h
> @@ -47,6 +47,9 @@
> #define __ARCH_WANT_SYS_UTIME
> #define __ARCH_WANT_SYS_NEWFSTATAT
> #define __ARCH_WANT_COMPAT_SYS_SENDFILE
> +#ifdef CONFIG_AUDIT
> +extern int ppc32_classify_syscall(unsigned int syscall);
> +#endif
> #endif
> #define __ARCH_WANT_SYS_FORK
> #define __ARCH_WANT_SYS_VFORK
> diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
> index a2dddd7f3d09..c3c6c6a1069b 100644
> --- a/arch/powerpc/kernel/audit.c
> +++ b/arch/powerpc/kernel/audit.c
> @@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
> int audit_classify_syscall(int abi, unsigned syscall)
> {
> #ifdef CONFIG_PPC64
> - extern int ppc32_classify_syscall(unsigned);
> if (abi == AUDIT_ARCH_PPC)
> return ppc32_classify_syscall(syscall);
> #endif
>
^ permalink raw reply
* [PATCH v2 1/3] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
From: Christophe Leroy @ 2021-08-19 15:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin
Cc: linuxppc-dev, linux-kernel
In those hot functions that are called at every interrupt, any saved
cycle is worth it.
interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are
called from three places:
- From entry_32.S
- From interrupt_64.S
- From interrupt_exit_user_restart() and interrupt_exit_kernel_restart()
In entry_32.S, there are inambiguously called based on MSR_PR:
interrupt_return:
lwz r4,_MSR(r1)
addi r3,r1,STACK_FRAME_OVERHEAD
andi. r0,r4,MSR_PR
beq .Lkernel_interrupt_return
bl interrupt_exit_user_prepare
...
.Lkernel_interrupt_return:
bl interrupt_exit_kernel_prepare
In interrupt_64.S, that's similar:
interrupt_return_\srr\():
ld r4,_MSR(r1)
andi. r0,r4,MSR_PR
beq interrupt_return_\srr\()_kernel
interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
addi r3,r1,STACK_FRAME_OVERHEAD
bl interrupt_exit_user_prepare
...
interrupt_return_\srr\()_kernel:
addi r3,r1,STACK_FRAME_OVERHEAD
bl interrupt_exit_kernel_prepare
In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(),
MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and
BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare()
and interrupt_exit_kernel_prepare().
The verification in interrupt_exit_user_prepare() and
interrupt_exit_kernel_prepare() are therefore useless and can be removed.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/interrupt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 21bbd615ca41..f26caf911ab5 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -465,7 +465,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
BUG_ON(!(regs->msr & MSR_RI));
- BUG_ON(!(regs->msr & MSR_PR));
BUG_ON(arch_irq_disabled_regs(regs));
CT_WARN_ON(ct_state() == CONTEXT_USER);
@@ -499,7 +498,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
unlikely(!(regs->msr & MSR_RI)))
unrecoverable_exception(regs);
- BUG_ON(regs->msr & MSR_PR);
/*
* CT_WARN_ON comes here via program_check_exception,
* so avoid recursion.
--
2.25.0
^ permalink raw reply related
* [PATCH v2 2/3] powerpc: Refactor verification of MSR_RI
From: Christophe Leroy @ 2021-08-19 15:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <385ead49ccb66a259b25fee3eebf0bd4094068f3.1629386461.git.christophe.leroy@csgroup.eu>
40x and BOOKE don't have MSR_RI therefore all tests involving
MSR_RI may be problematic on those plateforms.
Create helpers to check or set MSR_RI in regs, and use them
in common code.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Remove superflous {
---
arch/powerpc/include/asm/ptrace.h | 23 +++++++++++++++++++
arch/powerpc/kernel/interrupt.c | 9 +++-----
arch/powerpc/kernel/traps.c | 8 +++----
arch/powerpc/mm/book3s64/slb.c | 2 +-
arch/powerpc/platforms/embedded6xx/holly.c | 2 +-
.../platforms/embedded6xx/mpc7448_hpc2.c | 2 +-
arch/powerpc/platforms/pasemi/idle.c | 2 +-
arch/powerpc/platforms/powernv/opal.c | 2 +-
arch/powerpc/platforms/pseries/ras.c | 2 +-
arch/powerpc/sysdev/fsl_rio.c | 2 +-
arch/powerpc/xmon/xmon.c | 16 +++----------
11 files changed, 40 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index fd60538737a0..0cdb7b9c2c9c 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -22,6 +22,7 @@
#include <linux/err.h>
#include <uapi/asm/ptrace.h>
#include <asm/asm-const.h>
+#include <asm/reg.h>
#ifndef __ASSEMBLY__
struct pt_regs
@@ -282,6 +283,28 @@ static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
regs->gpr[3] = rc;
}
+static inline bool cpu_has_msr_ri(void)
+{
+ return !IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x);
+}
+
+static inline bool regs_is_unrecoverable(struct pt_regs *regs)
+{
+ return unlikely(cpu_has_msr_ri() && !(regs->msr & MSR_RI));
+}
+
+static inline void regs_set_recoverable(struct pt_regs *regs)
+{
+ if (cpu_has_msr_ri())
+ regs_set_return_msr(regs, regs->msr | MSR_RI);
+}
+
+static inline void regs_set_unrecoverable(struct pt_regs *regs)
+{
+ if (cpu_has_msr_ri())
+ regs_set_return_msr(regs, regs->msr & ~MSR_RI);
+}
+
#define arch_has_single_step() (1)
#define arch_has_block_step() (true)
#define ARCH_HAS_USER_SINGLE_STEP_REPORT
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index f26caf911ab5..f06c38e8fe36 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -93,8 +93,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
CT_WARN_ON(ct_state() == CONTEXT_KERNEL);
user_exit_irqoff();
- if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
- BUG_ON(!(regs->msr & MSR_RI));
+ BUG_ON(regs_is_unrecoverable(regs));
BUG_ON(!(regs->msr & MSR_PR));
BUG_ON(arch_irq_disabled_regs(regs));
@@ -463,8 +462,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
{
unsigned long ret;
- if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
- BUG_ON(!(regs->msr & MSR_RI));
+ BUG_ON(regs_is_unrecoverable(regs));
BUG_ON(arch_irq_disabled_regs(regs));
CT_WARN_ON(ct_state() == CONTEXT_USER);
@@ -495,8 +493,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
bool stack_store = current_thread_info()->flags &
_TIF_EMULATE_STACK_STORE;
- if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
- unlikely(!(regs->msr & MSR_RI)))
+ if (regs_is_unrecoverable(regs))
unrecoverable_exception(regs);
/*
* CT_WARN_ON comes here via program_check_exception,
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 3e2adb3487e7..8310147b5e7b 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -428,7 +428,7 @@ void hv_nmi_check_nonrecoverable(struct pt_regs *regs)
return;
nonrecoverable:
- regs_set_return_msr(regs, regs->msr & ~MSR_RI);
+ regs_set_unrecoverable(regs);
#endif
}
DEFINE_INTERRUPT_HANDLER_NMI(system_reset_exception)
@@ -498,7 +498,7 @@ DEFINE_INTERRUPT_HANDLER_NMI(system_reset_exception)
die("Unrecoverable nested System Reset", regs, SIGABRT);
#endif
/* Must die if the interrupt is not recoverable */
- if (!(regs->msr & MSR_RI)) {
+ if (regs_is_unrecoverable(regs)) {
/* For the reason explained in die_mce, nmi_exit before die */
nmi_exit();
die("Unrecoverable System Reset", regs, SIGABRT);
@@ -550,7 +550,7 @@ static inline int check_io_access(struct pt_regs *regs)
printk(KERN_DEBUG "%s bad port %lx at %p\n",
(*nip & 0x100)? "OUT to": "IN from",
regs->gpr[rb] - _IO_BASE, nip);
- regs_set_return_msr(regs, regs->msr | MSR_RI);
+ regs_set_recoverable(regs);
regs_set_return_ip(regs, extable_fixup(entry));
return 1;
}
@@ -840,7 +840,7 @@ DEFINE_INTERRUPT_HANDLER_NMI(machine_check_exception)
bail:
/* Must die if the interrupt is not recoverable */
- if (!(regs->msr & MSR_RI))
+ if (regs_is_unrecoverable(regs))
die_mce("Unrecoverable Machine check", regs, SIGBUS);
#ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/mm/book3s64/slb.c b/arch/powerpc/mm/book3s64/slb.c
index c91bd85eb90e..f0037bcc47a0 100644
--- a/arch/powerpc/mm/book3s64/slb.c
+++ b/arch/powerpc/mm/book3s64/slb.c
@@ -822,7 +822,7 @@ DEFINE_INTERRUPT_HANDLER_RAW(do_slb_fault)
/* IRQs are not reconciled here, so can't check irqs_disabled */
VM_WARN_ON(mfmsr() & MSR_EE);
- if (unlikely(!(regs->msr & MSR_RI)))
+ if (regs_is_unrecoverable(regs))
return -EINVAL;
/*
diff --git a/arch/powerpc/platforms/embedded6xx/holly.c b/arch/powerpc/platforms/embedded6xx/holly.c
index 85521b3e7098..7a85b117f7a4 100644
--- a/arch/powerpc/platforms/embedded6xx/holly.c
+++ b/arch/powerpc/platforms/embedded6xx/holly.c
@@ -251,7 +251,7 @@ static int ppc750_machine_check_exception(struct pt_regs *regs)
/* Are we prepared to handle this fault */
if ((entry = search_exception_tables(regs->nip)) != NULL) {
tsi108_clear_pci_cfg_error();
- regs_set_return_msr(regs, regs->msr | MSR_RI);
+ regs_set_recoverable(regs);
regs_set_return_ip(regs, extable_fixup(entry));
return 1;
}
diff --git a/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c b/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c
index d8da6a483e59..9eb9abb5bce2 100644
--- a/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c
+++ b/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c
@@ -173,7 +173,7 @@ static int mpc7448_machine_check_exception(struct pt_regs *regs)
/* Are we prepared to handle this fault */
if ((entry = search_exception_tables(regs->nip)) != NULL) {
tsi108_clear_pci_cfg_error();
- regs_set_return_msr(regs, regs->msr | MSR_RI);
+ regs_set_recoverable(regs);
regs_set_return_ip(regs, extable_fixup(entry));
return 1;
}
diff --git a/arch/powerpc/platforms/pasemi/idle.c b/arch/powerpc/platforms/pasemi/idle.c
index 534b0317fc15..6087c70ed2ef 100644
--- a/arch/powerpc/platforms/pasemi/idle.c
+++ b/arch/powerpc/platforms/pasemi/idle.c
@@ -59,7 +59,7 @@ static int pasemi_system_reset_exception(struct pt_regs *regs)
restore_astate(hard_smp_processor_id());
/* everything handled */
- regs_set_return_msr(regs, regs->msr | MSR_RI);
+ regs_set_recoverable(regs);
return 1;
}
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 2835376e61a4..e9d18519e650 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -588,7 +588,7 @@ static int opal_recover_mce(struct pt_regs *regs,
{
int recovered = 0;
- if (!(regs->msr & MSR_RI)) {
+ if (regs_is_unrecoverable(regs)) {
/* If MSR_RI isn't set, we cannot recover */
pr_err("Machine check interrupt unrecoverable: MSR(RI=0)\n");
recovered = 0;
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 167f2e1b8d39..56092dccfdb8 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -783,7 +783,7 @@ static int recover_mce(struct pt_regs *regs, struct machine_check_event *evt)
{
int recovered = 0;
- if (!(regs->msr & MSR_RI)) {
+ if (regs_is_unrecoverable(regs)) {
/* If MSR_RI isn't set, we cannot recover */
pr_err("Machine check interrupt unrecoverable: MSR(RI=0)\n");
recovered = 0;
diff --git a/arch/powerpc/sysdev/fsl_rio.c b/arch/powerpc/sysdev/fsl_rio.c
index 5a95b8ea23d8..ff7906b48ca1 100644
--- a/arch/powerpc/sysdev/fsl_rio.c
+++ b/arch/powerpc/sysdev/fsl_rio.c
@@ -108,7 +108,7 @@ int fsl_rio_mcheck_exception(struct pt_regs *regs)
__func__);
out_be32((u32 *)(rio_regs_win + RIO_LTLEDCSR),
0);
- regs_set_return_msr(regs, regs->msr | MSR_RI);
+ regs_set_recoverable(regs);
regs_set_return_ip(regs, extable_fixup(entry));
return 1;
}
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index ead460b80905..dd8241c009e5 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -482,16 +482,6 @@ static inline void get_output_lock(void) {}
static inline void release_output_lock(void) {}
#endif
-static inline int unrecoverable_excp(struct pt_regs *regs)
-{
-#if defined(CONFIG_4xx) || defined(CONFIG_PPC_BOOK3E)
- /* We have no MSR_RI bit on 4xx or Book3e, so we simply return false */
- return 0;
-#else
- return ((regs->msr & MSR_RI) == 0);
-#endif
-}
-
static void xmon_touch_watchdogs(void)
{
touch_softlockup_watchdog_sync();
@@ -565,7 +555,7 @@ static int xmon_core(struct pt_regs *regs, volatile int fromipi)
bp = NULL;
if ((regs->msr & (MSR_IR|MSR_PR|MSR_64BIT)) == (MSR_IR|MSR_64BIT))
bp = at_breakpoint(regs->nip);
- if (bp || unrecoverable_excp(regs))
+ if (bp || regs_is_unrecoverable(regs))
fromipi = 0;
if (!fromipi) {
@@ -577,7 +567,7 @@ static int xmon_core(struct pt_regs *regs, volatile int fromipi)
cpu, BP_NUM(bp));
xmon_print_symbol(regs->nip, " ", ")\n");
}
- if (unrecoverable_excp(regs))
+ if (regs_is_unrecoverable(regs))
printf("WARNING: exception is not recoverable, "
"can't continue\n");
release_output_lock();
@@ -693,7 +683,7 @@ static int xmon_core(struct pt_regs *regs, volatile int fromipi)
printf("Stopped at breakpoint %tx (", BP_NUM(bp));
xmon_print_symbol(regs->nip, " ", ")\n");
}
- if (unrecoverable_excp(regs))
+ if (regs_is_unrecoverable(regs))
printf("WARNING: exception is not recoverable, "
"can't continue\n");
remove_bpts();
--
2.25.0
^ permalink raw reply related
* [PATCH v2 3/3] powerpc: Define and use MSR_RI only on non booke/40x
From: Christophe Leroy @ 2021-08-19 15:21 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <385ead49ccb66a259b25fee3eebf0bd4094068f3.1629386461.git.christophe.leroy@csgroup.eu>
40x and BOOKE don't have MSR_RI.
Define MSR_RI only for platforms where it exists. For the other ones,
defines it as BUILD_BUG for C and do not define it for ASM.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/reg.h | 4 ++++
arch/powerpc/include/asm/reg_booke.h | 6 +++---
arch/powerpc/kernel/head_32.h | 4 ++++
arch/powerpc/kernel/process.c | 2 +-
arch/powerpc/lib/sstep.c | 2 +-
5 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index be85cf156a1f..656a9aaa1e8e 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -109,7 +109,11 @@
#ifndef MSR_PMM
#define MSR_PMM __MASK(MSR_PMM_LG) /* Performance monitor */
#endif
+#if !defined(CONFIG_BOOKE) && !defined(CONFIG_40x)
#define MSR_RI __MASK(MSR_RI_LG) /* Recoverable Exception */
+#elif !defined(__ASSEMBLY__)
+#define MSR_RI ({BUILD_BUG(); 0; })
+#endif
#define MSR_LE __MASK(MSR_LE_LG) /* Little Endian */
#define MSR_TM __MASK(MSR_TM_LG) /* Transactional Mem Available */
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index 17b8dcd9a40d..6f40a8420ad0 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -38,15 +38,15 @@
#if defined(CONFIG_PPC_BOOK3E_64)
#define MSR_64BIT MSR_CM
-#define MSR_ (MSR_ME | MSR_RI | MSR_CE)
+#define MSR_ (MSR_ME | MSR_CE)
#define MSR_KERNEL (MSR_ | MSR_64BIT)
#define MSR_USER32 (MSR_ | MSR_PR | MSR_EE)
#define MSR_USER64 (MSR_USER32 | MSR_64BIT)
#elif defined (CONFIG_40x)
-#define MSR_KERNEL (MSR_ME|MSR_RI|MSR_IR|MSR_DR|MSR_CE)
+#define MSR_KERNEL (MSR_ME|MSR_IR|MSR_DR|MSR_CE)
#define MSR_USER (MSR_KERNEL|MSR_PR|MSR_EE)
#else
-#define MSR_KERNEL (MSR_ME|MSR_RI|MSR_CE)
+#define MSR_KERNEL (MSR_ME|MSR_CE)
#define MSR_USER (MSR_KERNEL|MSR_PR|MSR_EE)
#endif
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 6b1ec9e3541b..6c5f4183dc8e 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -63,7 +63,11 @@
mtspr SPRN_DAR, r11 /* Tag DAR, to be used in DTLB Error */
.endif
#endif
+#ifdef CONFIG_40x
+ LOAD_REG_IMMEDIATE(r11, MSR_KERNEL) /* re-enable MMU */
+#else
LOAD_REG_IMMEDIATE(r11, MSR_KERNEL & ~MSR_RI) /* re-enable MMU */
+#endif
mtspr SPRN_SRR1, r11
lis r11, 1f@h
ori r11, r11, 1f@l
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 185beb290580..5ba72e31de28 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1420,7 +1420,7 @@ static struct regbit msr_bits[] = {
{MSR_IR, "IR"},
{MSR_DR, "DR"},
{MSR_PMM, "PMM"},
-#ifndef CONFIG_BOOKE
+#if !defined(CONFIG_BOOKE) && !defined(CONFIG_40x)
{MSR_RI, "RI"},
{MSR_LE, "LE"},
#endif
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index d8d5f901cee1..357cc1fb4f67 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -3559,7 +3559,7 @@ int emulate_step(struct pt_regs *regs, struct ppc_inst instr)
case MTMSR:
val = regs->gpr[op.reg];
- if ((val & MSR_RI) == 0)
+ if (cpu_has_msr_ri() && (val & MSR_RI) == 0)
/* can't step mtmsr[d] that would clear MSR_RI */
return -1;
/* here op.val is the mask of bits to change */
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v2 02/12] mm: Introduce a function to check for virtualization protection features
From: Tom Lendacky @ 2021-08-19 16:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-s390, Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh,
kvm, Tianyu Lan, Joerg Roedel, x86, kexec, linux-kernel, amd-gfx,
platform-driver-x86, iommu, Andi Kleen, linux-graphics-maintainer,
dri-devel, Joerg Roedel, linux-fsdevel, Borislav Petkov,
linuxppc-dev
In-Reply-To: <YR4ohWC4/cLsuCvv@infradead.org>
On 8/19/21 4:46 AM, Christoph Hellwig wrote:
> On Fri, Aug 13, 2021 at 11:59:21AM -0500, Tom Lendacky wrote:
>> +#define PATTR_MEM_ENCRYPT 0 /* Encrypted memory */
>> +#define PATTR_HOST_MEM_ENCRYPT 1 /* Host encrypted memory */
>> +#define PATTR_GUEST_MEM_ENCRYPT 2 /* Guest encrypted memory */
>> +#define PATTR_GUEST_PROT_STATE 3 /* Guest encrypted state */
>
> Please write an actual detailed explanaton of what these mean, that
> is what implications it has on the kernel.
Will do.
Thanks,
Tom
>
^ permalink raw reply
* Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
From: Borislav Petkov @ 2021-08-19 17:26 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
Peter Zijlstra, Dave Hansen, dri-devel, platform-driver-x86,
linux-s390, Andi Kleen, Joerg Roedel, x86, amd-gfx, Ingo Molnar,
linux-graphics-maintainer, Tom Lendacky, Joerg Roedel, Tianyu Lan,
Andy Lutomirski, Thomas Gleixner, kexec, linux-kernel, iommu,
linux-fsdevel, linuxppc-dev
In-Reply-To: <YR4p9TqKTLdN1A96@infradead.org>
On Thu, Aug 19, 2021 at 10:52:53AM +0100, Christoph Hellwig wrote:
> Which suggest that the name is not good to start with. Maybe protected
> hardware, system or platform might be a better choice?
Yah, coming up with a proper name here hasn't been easy.
prot_guest_has() is not the first variant.
From all three things you suggest above, I guess calling it a "platform"
is the closest. As in, this is a confidential computing platform which
provides host and guest facilities etc.
So calling it
confidential_computing_platform_has()
is obviously too long.
ccp_has() clashes with the namespace of drivers/crypto/ccp/ which is
used by the technology too.
coco_platform_has() is too unserious.
So I guess
cc_platform_has()
ain't all that bad.
Unless you have a better idea, ofc.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
From: Tom Lendacky @ 2021-08-19 18:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
Peter Zijlstra, Dave Hansen, dri-devel, platform-driver-x86,
linux-s390, Andi Kleen, Joerg Roedel, x86, amd-gfx, Ingo Molnar,
linux-graphics-maintainer, Joerg Roedel, Tianyu Lan,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner, kexec,
linux-kernel, iommu, linux-fsdevel, linuxppc-dev
In-Reply-To: <YR4p9TqKTLdN1A96@infradead.org>
On 8/19/21 4:52 AM, Christoph Hellwig wrote:
> On Fri, Aug 13, 2021 at 11:59:22AM -0500, Tom Lendacky wrote:
>> While the name suggests this is intended mainly for guests, it will
>> also be used for host memory encryption checks in place of sme_active().
>
> Which suggest that the name is not good to start with. Maybe protected
> hardware, system or platform might be a better choice?
>
>> +static inline bool prot_guest_has(unsigned int attr)
>> +{
>> +#ifdef CONFIG_AMD_MEM_ENCRYPT
>> + if (sme_me_mask)
>> + return amd_prot_guest_has(attr);
>> +#endif
>> +
>> + return false;
>> +}
>
> Shouldn't this be entirely out of line?
I did it as inline originally because the presence of the function will be
decided based on the ARCH_HAS_PROTECTED_GUEST config. For now, that is
only selected by the AMD memory encryption support, so if I went out of
line I could put in mem_encrypt.c. But with TDX wanting to also use it, it
would have to be in an always built file with some #ifdefs or in its own
file that is conditionally built based on the ARCH_HAS_PROTECTED_GUEST
setting (they've already tried building with ARCH_HAS_PROTECTED_GUEST=y
and AMD_MEM_ENCRYPT not set).
To take it out of line, I'm leaning towards the latter, creating a new
file that is built based on the ARCH_HAS_PROTECTED_GUEST setting.
>
>> +/* 0x800 - 0x8ff reserved for AMD */
>> +#define PATTR_SME 0x800
>> +#define PATTR_SEV 0x801
>> +#define PATTR_SEV_ES 0x802
>
> Why do we need reservations for a purely in-kernel namespace?
>
> And why are you overoading a brand new generic API with weird details
> of a specific implementation like this?
There was some talk about this on the mailing list where TDX and SEV may
need to be differentiated, so we wanted to reserve a range of values per
technology. I guess I can remove them until they are actually needed.
Thanks,
Tom
>
^ permalink raw reply
* Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
From: Tom Lendacky @ 2021-08-19 18:34 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
dri-devel, platform-driver-x86, Paul Mackerras, linux-s390,
Andi Kleen, Joerg Roedel, x86, amd-gfx, linux-graphics-maintainer,
Tianyu Lan, Borislav Petkov, kexec, linux-kernel, iommu,
linux-fsdevel, linuxppc-dev
In-Reply-To: <YR4qfZdkv+91zNZk@infradead.org>
On 8/19/21 4:55 AM, Christoph Hellwig wrote:
> On Fri, Aug 13, 2021 at 11:59:23AM -0500, Tom Lendacky wrote:
>> +static inline bool prot_guest_has(unsigned int attr)
>
> No reall need to have this inline. In fact I'd suggest we havea the
> prototype in a common header so that everyone must implement it out
> of line.
I'll do the same thing I end up doing for x86.
Thanks,
Tom
>
^ permalink raw reply
* Re: [PATCH 5/6] audit: Declare ppc32_classify_syscall()
From: Christophe Leroy @ 2021-08-19 18:36 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Christophe Leroy
In-Reply-To: <d268f141-4ec3-eb1d-a6c1-4cd5f535ea49@csgroup.eu>
Le 19/08/2021 à 16:56, Christophe Leroy a écrit :
>
>
> Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
>> This fixes a compile error with W=1.
>>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>> I don't think this is correct. Which file could we use ?
>
> I think you can completely remove ppc32_classify_syscall(), and instead add the following in the
> default case in audit_classify_syscall():
>
> default:
> + if (IS_ENABLED(CONFIG_PPC64) && abi == AUDIT_ARCH_PPC)
> + return 1;
> return 0;
>
After looking more in details, in fact I think we should convert powerpc to
CONFIG_AUDIT_ARCH_COMPAT_GENERIC
>
>>
>> arch/powerpc/include/asm/unistd.h | 3 +++
>> arch/powerpc/kernel/audit.c | 1 -
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
>> index b541c690a31c..d9025a7e973c 100644
>> --- a/arch/powerpc/include/asm/unistd.h
>> +++ b/arch/powerpc/include/asm/unistd.h
>> @@ -47,6 +47,9 @@
>> #define __ARCH_WANT_SYS_UTIME
>> #define __ARCH_WANT_SYS_NEWFSTATAT
>> #define __ARCH_WANT_COMPAT_SYS_SENDFILE
>> +#ifdef CONFIG_AUDIT
>> +extern int ppc32_classify_syscall(unsigned int syscall);
>> +#endif
>> #endif
>> #define __ARCH_WANT_SYS_FORK
>> #define __ARCH_WANT_SYS_VFORK
>> diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
>> index a2dddd7f3d09..c3c6c6a1069b 100644
>> --- a/arch/powerpc/kernel/audit.c
>> +++ b/arch/powerpc/kernel/audit.c
>> @@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
>> int audit_classify_syscall(int abi, unsigned syscall)
>> {
>> #ifdef CONFIG_PPC64
>> - extern int ppc32_classify_syscall(unsigned);
>> if (abi == AUDIT_ARCH_PPC)
>> return ppc32_classify_syscall(syscall);
>> #endif
>>
^ permalink raw reply
* Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
From: Kuppuswamy, Sathyanarayanan @ 2021-08-19 19:57 UTC (permalink / raw)
To: Tom Lendacky, Christoph Hellwig
Cc: linux-efi, Brijesh Singh, kvm, Peter Zijlstra, Dave Hansen,
dri-devel, platform-driver-x86, linux-s390, Andi Kleen,
Joerg Roedel, x86, amd-gfx, Ingo Molnar,
linux-graphics-maintainer, Joerg Roedel, Tianyu Lan,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner, kexec,
linux-kernel, iommu, linux-fsdevel, linuxppc-dev
In-Reply-To: <4272eaf5-b654-2669-62ac-ba768acd6b91@amd.com>
On 8/19/21 11:33 AM, Tom Lendacky wrote:
> There was some talk about this on the mailing list where TDX and SEV may
> need to be differentiated, so we wanted to reserve a range of values per
> technology. I guess I can remove them until they are actually needed.
In TDX also we have similar requirements and we need some flags for
TDX specific checks. So I think it is fine to leave some space for vendor
flags.
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply
* Re: [PATCH 3/7] powerpc: replace cc-option-yn uses with cc-option
From: Masahiro Yamada @ 2021-08-19 20:04 UTC (permalink / raw)
To: Michael Ellerman
Cc: Linux Kbuild mailing list, Nick Desaulniers, clang-built-linux,
Paul Mackerras, linuxppc-dev
In-Reply-To: <87a6lghkdj.fsf@mpe.ellerman.id.au>
On Tue, Aug 17, 2021 at 11:31 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Nick Desaulniers <ndesaulniers@google.com> writes:
> > cc-option-yn can be replaced with cc-option. ie.
> > Checking for support:
> > ifeq ($(call cc-option-yn,$(FLAG)),y)
> > becomes:
> > ifneq ($(call cc-option,$(FLAG)),)
> >
> > Checking for lack of support:
> > ifeq ($(call cc-option-yn,$(FLAG)),n)
> > becomes:
> > ifeq ($(call cc-option,$(FLAG)),)
> >
> > This allows us to pursue removing cc-option-yn.
> >
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> > arch/powerpc/Makefile | 12 ++++++------
> > arch/powerpc/boot/Makefile | 5 +----
> > 2 files changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index 9aaf1abbc641..85e224536cf7 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -12,12 +12,12 @@
> > # Rewritten by Cort Dougan and Paul Mackerras
> > #
> >
> > -HAS_BIARCH := $(call cc-option-yn, -m32)
> > +HAS_BIARCH := $(call cc-option,-m32)
> >
> > # Set default 32 bits cross compilers for vdso and boot wrapper
> > CROSS32_COMPILE ?=
> >
> > -ifeq ($(HAS_BIARCH),y)
> > +ifeq ($(HAS_BIARCH),-m32)
>
> I don't love that we have to repeat "-m32" in each check.
>
> I'm pretty sure you can use ifdef here, because HAS_BIARCH is a simple
> variable (assigned with ":=").
>
> ie, this can be:
>
> ifdef HAS_BIARCH
>
>
> And that avoids having to spell out "-m32" everywhere.
>
> cheers
Yes.
Comments from Nathan and Michael
both sound good.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 2/5] entry: rseq: Call rseq_handle_notify_resume() in tracehook_notify_resume()
From: Mathieu Desnoyers @ 2021-08-19 21:41 UTC (permalink / raw)
To: Sean Christopherson
Cc: KVM list, Peter Zijlstra, linux-kernel, Will Deacon, Guo Ren,
linux-kselftest, Ben Gardon, shuah, Paul Mackerras, linux-s390,
gor, Russell King, ARM Linux, linux-csky, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-mips, Boqun Feng, paulmck,
Heiko Carstens, rostedt, Shakeel Butt, Andy Lutomirski,
Thomas Gleixner, Peter Foley, linux-arm-kernel,
Thomas Bogendoerfer, Oleg Nesterov, Paolo Bonzini, linuxppc-dev
In-Reply-To: <20210818001210.4073390-3-seanjc@google.com>
----- On Aug 17, 2021, at 8:12 PM, Sean Christopherson seanjc@google.com wrote:
> Invoke rseq_handle_notify_resume() from tracehook_notify_resume() now
> that the two function are always called back-to-back by architectures
> that have rseq. The rseq helper is stubbed out for architectures that
> don't support rseq, i.e. this is a nop across the board.
>
> Note, tracehook_notify_resume() is horribly named and arguably does not
> belong in tracehook.h as literally every line of code in it has nothing
> to do with tracing. But, that's been true since commit a42c6ded827d
> ("move key_repace_session_keyring() into tracehook_notify_resume()")
> first usurped tracehook_notify_resume() back in 2012. Punt cleaning that
> mess up to future patches.
>
> No functional change intended.
This will make it harder to introduce new code paths which consume the
NOTIFY_RESUME without calling the rseq callback, which introduces issues.
Agreed.
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/arm/kernel/signal.c | 1 -
> arch/arm64/kernel/signal.c | 1 -
> arch/csky/kernel/signal.c | 4 +---
> arch/mips/kernel/signal.c | 4 +---
> arch/powerpc/kernel/signal.c | 4 +---
> arch/s390/kernel/signal.c | 1 -
> include/linux/tracehook.h | 2 ++
> kernel/entry/common.c | 4 +---
> kernel/entry/kvm.c | 4 +---
> 9 files changed, 7 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
> index a3a38d0a4c85..9df68d139965 100644
> --- a/arch/arm/kernel/signal.c
> +++ b/arch/arm/kernel/signal.c
> @@ -670,7 +670,6 @@ do_work_pending(struct pt_regs *regs, unsigned int
> thread_flags, int syscall)
> uprobe_notify_resume(regs);
> } else {
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> }
> }
> local_irq_disable();
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index 23036334f4dc..22b55db13da6 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -951,7 +951,6 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
>
> if (thread_flags & _TIF_NOTIFY_RESUME) {
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
>
> /*
> * If we reschedule after checking the affinity
> diff --git a/arch/csky/kernel/signal.c b/arch/csky/kernel/signal.c
> index 312f046d452d..bc4238b9f709 100644
> --- a/arch/csky/kernel/signal.c
> +++ b/arch/csky/kernel/signal.c
> @@ -260,8 +260,6 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
> if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> do_signal(regs);
>
> - if (thread_info_flags & _TIF_NOTIFY_RESUME) {
> + if (thread_info_flags & _TIF_NOTIFY_RESUME)
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> - }
> }
> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
> index f1e985109da0..c9b2a75563e1 100644
> --- a/arch/mips/kernel/signal.c
> +++ b/arch/mips/kernel/signal.c
> @@ -906,10 +906,8 @@ asmlinkage void do_notify_resume(struct pt_regs *regs, void
> *unused,
> if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> do_signal(regs);
>
> - if (thread_info_flags & _TIF_NOTIFY_RESUME) {
> + if (thread_info_flags & _TIF_NOTIFY_RESUME)
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> - }
>
> user_enter();
> }
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> index e600764a926c..b93b87df499d 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -293,10 +293,8 @@ void do_notify_resume(struct pt_regs *regs, unsigned long
> thread_info_flags)
> do_signal(current);
> }
>
> - if (thread_info_flags & _TIF_NOTIFY_RESUME) {
> + if (thread_info_flags & _TIF_NOTIFY_RESUME)
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> - }
> }
>
> static unsigned long get_tm_stackpointer(struct task_struct *tsk)
> diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
> index 78ef53b29958..b307db26bf2d 100644
> --- a/arch/s390/kernel/signal.c
> +++ b/arch/s390/kernel/signal.c
> @@ -537,5 +537,4 @@ void arch_do_signal_or_restart(struct pt_regs *regs, bool
> has_signal)
> void do_notify_resume(struct pt_regs *regs)
> {
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> }
> diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
> index 3e80c4bc66f7..2564b7434b4d 100644
> --- a/include/linux/tracehook.h
> +++ b/include/linux/tracehook.h
> @@ -197,6 +197,8 @@ static inline void tracehook_notify_resume(struct pt_regs
> *regs)
>
> mem_cgroup_handle_over_high();
> blkcg_maybe_throttle_current();
> +
> + rseq_handle_notify_resume(NULL, regs);
> }
>
> /*
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index bf16395b9e13..d5a61d565ad5 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -171,10 +171,8 @@ static unsigned long exit_to_user_mode_loop(struct pt_regs
> *regs,
> if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> handle_signal_work(regs, ti_work);
>
> - if (ti_work & _TIF_NOTIFY_RESUME) {
> + if (ti_work & _TIF_NOTIFY_RESUME)
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> - }
>
> /* Architecture specific TIF work */
> arch_exit_to_user_mode_work(regs, ti_work);
> diff --git a/kernel/entry/kvm.c b/kernel/entry/kvm.c
> index 049fd06b4c3d..49972ee99aff 100644
> --- a/kernel/entry/kvm.c
> +++ b/kernel/entry/kvm.c
> @@ -19,10 +19,8 @@ static int xfer_to_guest_mode_work(struct kvm_vcpu *vcpu,
> unsigned long ti_work)
> if (ti_work & _TIF_NEED_RESCHED)
> schedule();
>
> - if (ti_work & _TIF_NOTIFY_RESUME) {
> + if (ti_work & _TIF_NOTIFY_RESUME)
> tracehook_notify_resume(NULL);
> - rseq_handle_notify_resume(NULL, NULL);
> - }
>
> ret = arch_xfer_to_guest_mode_handle_work(vcpu, ti_work);
> if (ret)
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH 1/5] KVM: rseq: Update rseq when processing NOTIFY_RESUME on xfer to KVM guest
From: Mathieu Desnoyers @ 2021-08-19 21:39 UTC (permalink / raw)
To: Sean Christopherson
Cc: KVM list, Peter Zijlstra, linux-kernel, Will Deacon, Guo Ren,
linux-kselftest, Ben Gardon, shuah, Paul Mackerras, linux-s390,
gor, Russell King, ARM Linux, linux-csky, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-mips, Boqun Feng, paulmck,
Heiko Carstens, rostedt, Shakeel Butt, Andy Lutomirski,
Thomas Gleixner, Peter Foley, linux-arm-kernel,
Thomas Bogendoerfer, Oleg Nesterov, Paolo Bonzini, linuxppc-dev
In-Reply-To: <20210818001210.4073390-2-seanjc@google.com>
----- On Aug 17, 2021, at 8:12 PM, Sean Christopherson seanjc@google.com wrote:
> Invoke rseq's NOTIFY_RESUME handler when processing the flag prior to
> transferring to a KVM guest, which is roughly equivalent to an exit to
> userspace and processes many of the same pending actions. While the task
> cannot be in an rseq critical section as the KVM path is reachable only
> via ioctl(KVM_RUN), the side effects that apply to rseq outside of a
> critical section still apply, e.g. the CPU ID needs to be updated if the
> task is migrated.
>
> Clearing TIF_NOTIFY_RESUME without informing rseq can lead to segfaults
> and other badness in userspace VMMs that use rseq in combination with KVM,
> e.g. due to the CPU ID being stale after task migration.
I agree with the problem assessment, but I would recommend a small change
to this fix.
>
> Fixes: 72c3c0fe54a3 ("x86/kvm: Use generic xfer to guest work function")
> Reported-by: Peter Foley <pefoley@google.com>
> Bisected-by: Doug Evans <dje@google.com>
> Cc: Shakeel Butt <shakeelb@google.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> kernel/entry/kvm.c | 4 +++-
> kernel/rseq.c | 4 ++--
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/entry/kvm.c b/kernel/entry/kvm.c
> index 49972ee99aff..049fd06b4c3d 100644
> --- a/kernel/entry/kvm.c
> +++ b/kernel/entry/kvm.c
> @@ -19,8 +19,10 @@ static int xfer_to_guest_mode_work(struct kvm_vcpu *vcpu,
> unsigned long ti_work)
> if (ti_work & _TIF_NEED_RESCHED)
> schedule();
>
> - if (ti_work & _TIF_NOTIFY_RESUME)
> + if (ti_work & _TIF_NOTIFY_RESUME) {
> tracehook_notify_resume(NULL);
> + rseq_handle_notify_resume(NULL, NULL);
> + }
>
> ret = arch_xfer_to_guest_mode_handle_work(vcpu, ti_work);
> if (ret)
> diff --git a/kernel/rseq.c b/kernel/rseq.c
> index 35f7bd0fced0..58c79a7918cd 100644
> --- a/kernel/rseq.c
> +++ b/kernel/rseq.c
> @@ -236,7 +236,7 @@ static bool in_rseq_cs(unsigned long ip, struct rseq_cs
> *rseq_cs)
>
> static int rseq_ip_fixup(struct pt_regs *regs)
> {
> - unsigned long ip = instruction_pointer(regs);
> + unsigned long ip = regs ? instruction_pointer(regs) : 0;
> struct task_struct *t = current;
> struct rseq_cs rseq_cs;
> int ret;
> @@ -250,7 +250,7 @@ static int rseq_ip_fixup(struct pt_regs *regs)
> * If not nested over a rseq critical section, restart is useless.
> * Clear the rseq_cs pointer and return.
> */
> - if (!in_rseq_cs(ip, &rseq_cs))
> + if (!regs || !in_rseq_cs(ip, &rseq_cs))
I think clearing the thread's rseq_cs unconditionally here when regs is NULL
is not the behavior we want when this is called from xfer_to_guest_mode_work.
If we have a scenario where userspace ends up calling this ioctl(KVM_RUN)
from within a rseq c.s., we really want a CONFIG_DEBUG_RSEQ=y kernel to
kill this application in the rseq_syscall handler when exiting back to usermode
when the ioctl eventually returns.
However, clearing the thread's rseq_cs will prevent this from happening.
So I would favor an approach where we simply do:
if (!regs)
return 0;
Immediately at the beginning of rseq_ip_fixup, before getting the instruction
pointer, so effectively skip all side-effects of the ip fixup code. Indeed, it
is not relevant to do any fixup here, because it is nested in a ioctl system
call.
Effectively, this would preserve the SIGSEGV behavior when this ioctl is
erroneously called by user-space from a rseq critical section.
Thanks for looking into this !
Mathieu
> return clear_rseq_cs(t);
> ret = rseq_need_restart(t, rseq_cs.flags);
> if (ret <= 0)
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH 4/5] KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs
From: Mathieu Desnoyers @ 2021-08-19 21:52 UTC (permalink / raw)
To: Sean Christopherson
Cc: KVM list, Peter Zijlstra, linux-kernel, Will Deacon, Guo Ren,
linux-kselftest, Ben Gardon, shuah, Paul Mackerras, linux-s390,
gor, Russell King, ARM Linux, linux-csky, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-mips, Boqun Feng, paulmck,
Heiko Carstens, rostedt, Shakeel Butt, Andy Lutomirski,
Thomas Gleixner, Peter Foley, linux-arm-kernel,
Thomas Bogendoerfer, Oleg Nesterov, Paolo Bonzini, linuxppc-dev
In-Reply-To: <20210818001210.4073390-5-seanjc@google.com>
----- On Aug 17, 2021, at 8:12 PM, Sean Christopherson seanjc@google.com wrote:
> Add a test to verify an rseq's CPU ID is updated correctly if the task is
> migrated while the kernel is handling KVM_RUN. This is a regression test
> for a bug introduced by commit 72c3c0fe54a3 ("x86/kvm: Use generic xfer
> to guest work function"), where TIF_NOTIFY_RESUME would be cleared by KVM
> without updating rseq, leading to a stale CPU ID and other badness.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
[...]
> +
> +static void *migration_worker(void *ign)
> +{
> + cpu_set_t allowed_mask;
> + int r, i, nr_cpus, cpu;
> +
> + CPU_ZERO(&allowed_mask);
> +
> + nr_cpus = CPU_COUNT(&possible_mask);
> +
> + for (i = 0; i < 20000; i++) {
> + cpu = i % nr_cpus;
> + if (!CPU_ISSET(cpu, &possible_mask))
> + continue;
> +
> + CPU_SET(cpu, &allowed_mask);
> +
> + r = sched_setaffinity(0, sizeof(allowed_mask), &allowed_mask);
> + TEST_ASSERT(!r, "sched_setaffinity failed, errno = %d (%s)", errno,
> + strerror(errno));
> +
> + CPU_CLR(cpu, &allowed_mask);
> +
> + usleep(10);
> + }
> + done = true;
> + return NULL;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + struct kvm_vm *vm;
> + u32 cpu, rseq_cpu;
> + int r;
> +
> + /* Tell stdout not to buffer its content */
> + setbuf(stdout, NULL);
> +
> + r = sched_getaffinity(0, sizeof(possible_mask), &possible_mask);
> + TEST_ASSERT(!r, "sched_getaffinity failed, errno = %d (%s)", errno,
> + strerror(errno));
> +
> + if (CPU_COUNT(&possible_mask) < 2) {
> + print_skip("Only one CPU, task migration not possible\n");
> + exit(KSFT_SKIP);
> + }
> +
> + sys_rseq(0);
> +
> + /*
> + * Create and run a dummy VM that immediately exits to userspace via
> + * GUEST_SYNC, while concurrently migrating the process by setting its
> + * CPU affinity.
> + */
> + vm = vm_create_default(VCPU_ID, 0, guest_code);
> +
> + pthread_create(&migration_thread, NULL, migration_worker, 0);
> +
> + while (!done) {
> + vcpu_run(vm, VCPU_ID);
> + TEST_ASSERT(get_ucall(vm, VCPU_ID, NULL) == UCALL_SYNC,
> + "Guest failed?");
> +
> + cpu = sched_getcpu();
> + rseq_cpu = READ_ONCE(__rseq.cpu_id);
> +
> + /*
> + * Verify rseq's CPU matches sched's CPU, and that sched's CPU
> + * is stable. This doesn't handle the case where the task is
> + * migrated between sched_getcpu() and reading rseq, and again
> + * between reading rseq and sched_getcpu(), but in practice no
> + * false positives have been observed, while on the other hand
> + * blocking migration while this thread reads CPUs messes with
> + * the timing and prevents hitting failures on a buggy kernel.
> + */
I think you could get a stable cpu id between sched_getcpu and __rseq_abi.cpu_id
if you add a pthread mutex to protect:
sched_getcpu and __rseq_abi.cpu_id reads
vs
sched_setaffinity calls within the migration thread.
Thoughts ?
Thanks,
Mathieu
> + TEST_ASSERT(rseq_cpu == cpu || cpu != sched_getcpu(),
> + "rseq CPU = %d, sched CPU = %d\n", rseq_cpu, cpu);
> + }
> +
> + pthread_join(migration_thread, NULL);
> +
> + kvm_vm_free(vm);
> +
> + sys_rseq(RSEQ_FLAG_UNREGISTER);
> +
> + return 0;
> +}
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc: kvm: remove obsolete and unneeded select
From: Daniel Axtens @ 2021-08-19 22:39 UTC (permalink / raw)
To: Lukas Bulwahn, Paul Mackerras, Michael Ellerman,
Benjamin Herrenschmidt, Michael Neuling, Anshuman Khandual,
kvm-ppc, linuxppc-dev
Cc: Lukas Bulwahn, kernel-janitors, linux-kernel, stable
In-Reply-To: <20210819113954.17515-2-lukas.bulwahn@gmail.com>
Hi Lukas,
> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
> index e45644657d49..ff581d70f20c 100644
> --- a/arch/powerpc/kvm/Kconfig
> +++ b/arch/powerpc/kvm/Kconfig
> @@ -38,7 +38,6 @@ config KVM_BOOK3S_32_HANDLER
> config KVM_BOOK3S_64_HANDLER
> bool
> select KVM_BOOK3S_HANDLER
> - select PPC_DAWR_FORCE_ENABLE
I looked at some of the history here. It looks like this select was left
over from an earlier version of the patch series that added PPC_DAWR: v2
of the series has a new symbol PPC_DAWR_FORCE_ENABLE but by version 4
that new symbol had disappeared but the select had not.
v2: https://lore.kernel.org/linuxppc-dev/20190513071703.25243-1-mikey@neuling.org/
v5: https://lore.kernel.org/linuxppc-dev/20190604030037.9424-2-mikey@neuling.org/
The rest of the patch reasoning makes sense to me: DAWR support will be
selected anyway by virtue of PPC64->PPC_DAWR so there's no need to try
to select it again anyway.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
>
> config KVM_BOOK3S_PR_POSSIBLE
> bool
> --
> 2.26.2
^ permalink raw reply
* Re: [PATCH 4/5] KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs
From: Sean Christopherson @ 2021-08-19 23:33 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: KVM list, Peter Zijlstra, linux-kernel, Will Deacon, Guo Ren,
linux-kselftest, Ben Gardon, shuah, Paul Mackerras, linux-s390,
gor, Russell King, ARM Linux, linux-csky, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-mips, Boqun Feng, paulmck,
Heiko Carstens, rostedt, Shakeel Butt, Andy Lutomirski,
Thomas Gleixner, Peter Foley, linux-arm-kernel,
Thomas Bogendoerfer, Oleg Nesterov, Paolo Bonzini, linuxppc-dev
In-Reply-To: <1540548616.19739.1629409956315.JavaMail.zimbra@efficios.com>
On Thu, Aug 19, 2021, Mathieu Desnoyers wrote:
> ----- On Aug 17, 2021, at 8:12 PM, Sean Christopherson seanjc@google.com wrote:
>
> > Add a test to verify an rseq's CPU ID is updated correctly if the task is
> > migrated while the kernel is handling KVM_RUN. This is a regression test
> > for a bug introduced by commit 72c3c0fe54a3 ("x86/kvm: Use generic xfer
> > to guest work function"), where TIF_NOTIFY_RESUME would be cleared by KVM
> > without updating rseq, leading to a stale CPU ID and other badness.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
>
> [...]
>
> > + while (!done) {
> > + vcpu_run(vm, VCPU_ID);
> > + TEST_ASSERT(get_ucall(vm, VCPU_ID, NULL) == UCALL_SYNC,
> > + "Guest failed?");
> > +
> > + cpu = sched_getcpu();
> > + rseq_cpu = READ_ONCE(__rseq.cpu_id);
> > +
> > + /*
> > + * Verify rseq's CPU matches sched's CPU, and that sched's CPU
> > + * is stable. This doesn't handle the case where the task is
> > + * migrated between sched_getcpu() and reading rseq, and again
> > + * between reading rseq and sched_getcpu(), but in practice no
> > + * false positives have been observed, while on the other hand
> > + * blocking migration while this thread reads CPUs messes with
> > + * the timing and prevents hitting failures on a buggy kernel.
> > + */
>
> I think you could get a stable cpu id between sched_getcpu and __rseq_abi.cpu_id
> if you add a pthread mutex to protect:
>
> sched_getcpu and __rseq_abi.cpu_id reads
>
> vs
>
> sched_setaffinity calls within the migration thread.
>
> Thoughts ?
I tried that and couldn't reproduce the bug. That's what I attempted to call out
in the blurb "blocking migration while this thread reads CPUs ... prevents hitting
failures on a buggy kernel".
I considered adding arbitrary delays around the mutex to try and hit the bug, but
I was worried that even if I got it "working" for this bug, the test would be too
tailored to this bug and potentially miss future regression. Letting the two
threads run wild seemed like it would provide the best coverage, at the cost of
potentially causing to false failures.
^ permalink raw reply
* Re: [PATCH 1/5] KVM: rseq: Update rseq when processing NOTIFY_RESUME on xfer to KVM guest
From: Sean Christopherson @ 2021-08-19 23:48 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: KVM list, Peter Zijlstra, linux-kernel, Will Deacon, Guo Ren,
linux-kselftest, Ben Gardon, shuah, Paul Mackerras, linux-s390,
gor, Russell King, ARM Linux, linux-csky, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, linux-mips, Boqun Feng, paulmck,
Heiko Carstens, rostedt, Shakeel Butt, Andy Lutomirski,
Thomas Gleixner, Peter Foley, linux-arm-kernel,
Thomas Bogendoerfer, Oleg Nesterov, Paolo Bonzini, linuxppc-dev
In-Reply-To: <1673583543.19718.1629409152244.JavaMail.zimbra@efficios.com>
On Thu, Aug 19, 2021, Mathieu Desnoyers wrote:
> ----- On Aug 17, 2021, at 8:12 PM, Sean Christopherson seanjc@google.com wrote:
> > @@ -250,7 +250,7 @@ static int rseq_ip_fixup(struct pt_regs *regs)
> > * If not nested over a rseq critical section, restart is useless.
> > * Clear the rseq_cs pointer and return.
> > */
> > - if (!in_rseq_cs(ip, &rseq_cs))
> > + if (!regs || !in_rseq_cs(ip, &rseq_cs))
>
> I think clearing the thread's rseq_cs unconditionally here when regs is NULL
> is not the behavior we want when this is called from xfer_to_guest_mode_work.
>
> If we have a scenario where userspace ends up calling this ioctl(KVM_RUN)
> from within a rseq c.s., we really want a CONFIG_DEBUG_RSEQ=y kernel to
> kill this application in the rseq_syscall handler when exiting back to usermode
> when the ioctl eventually returns.
>
> However, clearing the thread's rseq_cs will prevent this from happening.
>
> So I would favor an approach where we simply do:
>
> if (!regs)
> return 0;
>
> Immediately at the beginning of rseq_ip_fixup, before getting the instruction
> pointer, so effectively skip all side-effects of the ip fixup code. Indeed, it
> is not relevant to do any fixup here, because it is nested in a ioctl system
> call.
>
> Effectively, this would preserve the SIGSEGV behavior when this ioctl is
> erroneously called by user-space from a rseq critical section.
Ha, that's effectively what I implemented first, but I changed it because of the
comment in clear_rseq_cs() that says:
The rseq_cs field is set to NULL on preemption or signal delivery ... as well
as well as on top of code outside of the rseq assembly block.
Which makes it sound like something might rely on clearing rseq_cs?
Ah, or is it the case that rseq_cs is non-NULL if and only if userspace is in an
rseq critical section, and because syscalls in critical sections are illegal, by
definition clearing rseq_cs is a nop unless userspace is misbehaving.
If that's true, what about explicitly checking that at NOTIFY_RESUME? Or is it
not worth the extra code to detect an error that will likely be caught anyways?
diff --git a/kernel/rseq.c b/kernel/rseq.c
index 35f7bd0fced0..28b8342290b0 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -282,6 +282,13 @@ void __rseq_handle_notify_resume(struct ksignal *ksig, struct pt_regs *regs)
if (unlikely(t->flags & PF_EXITING))
return;
+ if (!regs) {
+#ifdef CONFIG_DEBUG_RSEQ
+ if (t->rseq && rseq_get_rseq_cs(t, &rseq_cs))
+ goto error;
+#endif
+ return;
+ }
ret = rseq_ip_fixup(regs);
if (unlikely(ret < 0))
goto error;
> Thanks for looking into this !
>
> Mathieu
>
> > return clear_rseq_cs(t);
> > ret = rseq_need_restart(t, rseq_cs.flags);
> > if (ret <= 0)
> > --
> > 2.33.0.rc1.237.g0d66db33f3-goog
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
^ permalink raw reply related
* Re: [PATCH v2 2/2] powerpc: rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK
From: Daniel Axtens @ 2021-08-20 0:03 UTC (permalink / raw)
To: Lukas Bulwahn, Paul Mackerras, Michael Ellerman,
Benjamin Herrenschmidt, Michael Neuling, Anshuman Khandual,
kvm-ppc, linuxppc-dev
Cc: Lukas Bulwahn, kernel-janitors, linux-kernel, stable
In-Reply-To: <20210819113954.17515-3-lukas.bulwahn@gmail.com>
Lukas Bulwahn <lukas.bulwahn@gmail.com> writes:
> Commit 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
> selects the non-existing config ARCH_ENABLE_PMD_SPLIT_PTLOCK in
> ./arch/powerpc/platforms/Kconfig.cputype, but clearly it intends to select
> ARCH_ENABLE_SPLIT_PMD_PTLOCK here (notice the word swapping!), as this
> commit does select that for all other architectures.
>
> Rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK instead.
>
Yikes, yes, 66f24fa766e3 does seem to have got that wrong. It looks like
that went into 5.13.
I think we want to specifically target this for stable so that we don't
lose the perfomance and scalability benefits of split pmd ptlocks:
Cc: stable@vger.kernel.org # v5.13+
(I don't think you need to do another revision for this, I think mpe
could add it when merging.)
I tried to check whether we accidentally broke SPLIT_PMD_PTLOCKs while
they were disabled:
- There hasn't been any change to the pgtable_pmd_page_ctor or _dtor
prototypes, and we haven't made any relevant changes to any of the
files in arch/powerpc that called it.
- I checked out v5.13 and powerpc/merge, applied this patch, built a
pseries_le_defconfig and boot tested it in qemu. It didn't crash on
boot or with /bin/sh and some shell commands, but I didn't exactly
stress test the VM subsystem either.
This gives me some confidence it's both good for powerpc and stable-worthy.
Overall:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> Fixes: 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> ---
> arch/powerpc/platforms/Kconfig.cputype | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 6794145603de..a208997ade88 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -98,7 +98,7 @@ config PPC_BOOK3S_64
> select PPC_HAVE_PMU_SUPPORT
> select HAVE_ARCH_TRANSPARENT_HUGEPAGE
> select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
> - select ARCH_ENABLE_PMD_SPLIT_PTLOCK
> + select ARCH_ENABLE_SPLIT_PMD_PTLOCK
> select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
> select ARCH_SUPPORTS_HUGETLBFS
> select ARCH_SUPPORTS_NUMA_BALANCING
> --
> 2.26.2
^ permalink raw reply
* [Bug 213079] [bisected] IRQ problems and crashes on a PowerMac G5 with 5.12.3
From: bugzilla-daemon @ 2021-08-20 0:12 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-213079-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=213079
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #297473|0 |1
is obsolete| |
--- Comment #16 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 298371
--> https://bugzilla.kernel.org/attachment.cgi?id=298371&action=edit
dmesg (5.14-rc6, PowerMac G5 11,2)
As there is a fix now for bug #213803 I was able to build v5.14-rc6 and gave it
a testride. Looks like the issue persists:
[...]
irq 63: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 10732 Comm: emerge Tainted: G W
5.14.0-rc6-PowerMacG5+ #2
Call Trace:
[c00000000fff7af0] [c00000000054de24] .dump_stack_lvl+0x98/0xe0 (unreliable)
[c00000000fff7b80] [c0000000000e1724] .__report_bad_irq+0x34/0xf0
[c00000000fff7c20] [c0000000000e160c] .note_interrupt+0x258/0x300
[c00000000fff7ce0] [c0000000000dd840] .handle_irq_event_percpu+0x5c/0x88
[c00000000fff7d70] [c0000000000dd8b0] .handle_irq_event+0x44/0x70
[c00000000fff7e00] [c0000000000e2d34] .handle_fasteoi_irq+0xac/0x158
[c00000000fff7ea0] [c0000000000dc8bc] .handle_irq_desc+0x34/0x54
[c00000000fff7f10] [c000000000012058] .__do_irq+0x15c/0x238
[c00000000fff7f90] [c000000000012978] .__do_IRQ+0xac/0xb4
[c00000001e9cfcf0] [c00000001e9cfd90] 0xc00000001e9cfd90
[c00000001e9cfd90] [c000000000012ac4] .do_IRQ+0x144/0x194
[c00000001e9cfe10] [c000000000008050]
hardware_interrupt_common_virt+0x210/0x220
--- interrupt: 500 at 0x3fffb9b25d9c
NIP: 00003fffb9b25d9c LR: 00003fffb9b2811c CTR: 00003fffb9b25d9c
REGS: c00000001e9cfe80 TRAP: 0500 Tainted: G W
(5.14.0-rc6-PowerMacG5+)
MSR: 900000000000f032 <SF,HV,EE,PR,FP,ME,IR,DR,RI> CR: 22482822 XER:
20000000
IRQMASK: 0
GPR00: 00003fffb9b28100 00003ffffd4e7550 00003fffb9ef6200 00003fffb7977790
GPR04: 00003fffb7977790 00003fffb55e8b80 0000000000000000 00003fffb9eccac0
GPR08: 00003fffb9b25d9c 0000000000000000 000000000000000f 0000000000000000
GPR12: 00003fffb9b7eeb0 00003fffb9fc8890 00003ffffd4e7658 00003fffb395c548
GPR16: 00003ffffd4e7670 ffffffffffffffff 00003fffb7902480 ffffffffffffffff
GPR20: 0000000000000000 00003fffb395c528 000000014b8f7878 0000000000000000
GPR24: 00003fffb7969a80 000000014b8f7830 00003fffb7a750d0 000000000000000a
GPR28: 00003fffb7a750dc 000000000000007c 000000014b8f9420 00003fffb395c3c0
NIP [00003fffb9b25d9c] 0x3fffb9b25d9c
LR [00003fffb9b2811c] 0x3fffb9b2811c
--- interrupt: 500
handlers:
[<c0000000015a6568>] .nvme_irq
[<c0000000015a6568>] .nvme_irq
Disabling IRQ #63
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 213079] [bisected] IRQ problems and crashes on a PowerMac G5 with 5.12.3
From: bugzilla-daemon @ 2021-08-20 0:13 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-213079-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=213079
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #297439|0 |1
is obsolete| |
--- Comment #17 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 298373
--> https://bugzilla.kernel.org/attachment.cgi?id=298373&action=edit
kernel .config (5.14-rc6, PowerMac G5 11,2)
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [powerpc:fixes-test] BUILD SUCCESS 9f7853d7609d59172eecfc5e7ccf503bc1b690bd
From: kernel test robot @ 2021-08-20 0:18 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: 9f7853d7609d59172eecfc5e7ccf503bc1b690bd powerpc/mm: Fix set_memory_*() against concurrent accesses
elapsed time: 1357m
configs tested: 129
configs skipped: 123
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
i386 randconfig-c001-20210819
i386 randconfig-c001-20210818
powerpc pcm030_defconfig
arm socfpga_defconfig
m68k m5275evb_defconfig
sh se7619_defconfig
powerpc iss476-smp_defconfig
mips rt305x_defconfig
arc allyesconfig
powerpc linkstation_defconfig
arm netwinder_defconfig
alpha defconfig
arm hackkit_defconfig
mips maltasmvp_eva_defconfig
mips ar7_defconfig
x86_64 defconfig
arm ixp4xx_defconfig
powerpc mpc834x_mds_defconfig
powerpc ps3_defconfig
sh se7724_defconfig
parisc generic-32bit_defconfig
powerpc pmac32_defconfig
sh rsk7203_defconfig
arm stm32_defconfig
powerpc ppc64_defconfig
mips decstation_r4k_defconfig
powerpc sam440ep_defconfig
powerpc pq2fads_defconfig
m68k m5407c3_defconfig
sh j2_defconfig
m68k amcore_defconfig
arm viper_defconfig
riscv nommu_k210_defconfig
powerpc ep88xc_defconfig
arm cerfcube_defconfig
arm aspeed_g4_defconfig
nios2 10m50_defconfig
m68k m5249evb_defconfig
m68k bvme6000_defconfig
microblaze defconfig
um defconfig
x86_64 allyesconfig
mips rm200_defconfig
sh sh7770_generic_defconfig
arm iop32x_defconfig
powerpc icon_defconfig
x86_64 allnoconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a004-20210818
x86_64 randconfig-a006-20210818
x86_64 randconfig-a003-20210818
x86_64 randconfig-a005-20210818
x86_64 randconfig-a002-20210818
x86_64 randconfig-a001-20210818
i386 randconfig-a004-20210818
i386 randconfig-a006-20210818
i386 randconfig-a002-20210818
i386 randconfig-a001-20210818
i386 randconfig-a003-20210818
i386 randconfig-a005-20210818
i386 randconfig-a015-20210819
i386 randconfig-a011-20210819
i386 randconfig-a014-20210819
i386 randconfig-a013-20210819
i386 randconfig-a016-20210819
i386 randconfig-a012-20210819
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um x86_64_defconfig
um i386_defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
i386 randconfig-c001-20210818
x86_64 randconfig-a004-20210819
x86_64 randconfig-a006-20210819
x86_64 randconfig-a003-20210819
x86_64 randconfig-a002-20210819
x86_64 randconfig-a005-20210819
x86_64 randconfig-a001-20210819
i386 randconfig-a004-20210819
i386 randconfig-a006-20210819
i386 randconfig-a001-20210819
i386 randconfig-a002-20210819
i386 randconfig-a003-20210819
i386 randconfig-a005-20210819
x86_64 randconfig-a013-20210818
x86_64 randconfig-a011-20210818
x86_64 randconfig-a012-20210818
x86_64 randconfig-a016-20210818
x86_64 randconfig-a014-20210818
x86_64 randconfig-a015-20210818
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [PATCH linux-next] macintosh: fix warning comparing pointer to 0
From: jing yangyang @ 2021-08-20 2:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Zeal Robot, linuxppc-dev, linux-kernel, jing yangyang
Fix the following coccicheck warning:
./drivers/macintosh/windfarm_pm91.c:152:12-13:WARNING comparing pointer to 0
Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: jing yangyang <jing.yangyang@zte.com.cn>
---
drivers/macintosh/windfarm_pm91.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/macintosh/windfarm_pm91.c b/drivers/macintosh/windfarm_pm91.c
index 3f346af..568f8a2 100644
--- a/drivers/macintosh/windfarm_pm91.c
+++ b/drivers/macintosh/windfarm_pm91.c
@@ -149,7 +149,7 @@ static void wf_smu_create_cpu_fans(void)
/* First, locate the PID params in SMU SBD */
hdr = smu_get_sdb_partition(SMU_SDB_CPUPIDDATA_ID, NULL);
- if (hdr == 0) {
+ if (!hdr) {
printk(KERN_WARNING "windfarm: CPU PID fan config not found "
"max fan speed\n");
goto fail;
--
1.8.3.1
^ permalink raw reply related
* [PATCH linux-next] ps3: remove unneeded semicolon
From: jing yangyang @ 2021-08-20 2:49 UTC (permalink / raw)
To: Geoff Levand
Cc: jing yangyang, Zeal Robot, linux-kernel, Paul Mackerras,
linuxppc-dev
Eliminate the following coccicheck warning:
./arch/powerpc/platforms/ps3/system-bus.c:606:2-3: Unneeded semicolon
./arch/powerpc/platforms/ps3/system-bus.c:765:2-3: Unneeded semicolon
Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: jing yangyang <jing.yangyang@zte.com.cn>
---
arch/powerpc/platforms/ps3/system-bus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c
index c8b50fe..b637bf2 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -603,7 +603,7 @@ static dma_addr_t ps3_ioc0_map_page(struct device *_dev, struct page *page,
default:
/* not happned */
BUG();
- };
+ }
result = ps3_dma_map(dev->d_region, (unsigned long)ptr, size,
&bus_addr, iopte_flag);
@@ -762,7 +762,7 @@ int ps3_system_bus_device_register(struct ps3_system_bus_device *dev)
break;
default:
BUG();
- };
+ }
dev->core.of_node = NULL;
set_dev_node(&dev->core, 0);
--
1.8.3.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox