* Re: [Intel-gfx] [PATCH v7 00/12] Introduce CAP_PERFMON to secure system performance monitoring and observability
From: James Morris @ 2020-03-26 23:28 UTC (permalink / raw)
To: Serge Hallyn
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Will Deacon, Ingo Molnar,
Andi Kleen, Jiri Olsa, Alexey Budankov, Igor Lubashev,
oprofile-list, Stephen Smalley, selinux@vger.kernel.org,
intel-gfx@lists.freedesktop.org, Arnaldo Carvalho de Melo,
Thomas Gleixner, linux-arm-kernel, linux-parisc@vger.kernel.org,
linux-kernel, linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Helge Deller
In-Reply-To: <20200302001913.GA21145@sl>
On Sun, 1 Mar 2020, Serge Hallyn wrote:
> Thanks, this looks good to me, in keeping with the CAP_SYSLOG break.
>
> Acked-by: Serge E. Hallyn <serge@hallyn.com>
>
> for the set.
>
> James/Ingo/Peter, if noone has remaining objections, whose branch
> should these go in through?
>
> thanks,
> -serge
>
> On Tue, Feb 25, 2020 at 12:55:54PM +0300, Alexey Budankov wrote:
> >
> > Hi,
> >
> > Is there anything else I could do in order to move the changes forward
> > or is something still missing from this patch set?
> > Could you please share you mind?
Alexey,
It seems some of the previous Acks are not included in this patchset, e.g.
https://lkml.org/lkml/2020/1/22/655
Every patch needs a Reviewed-by or Acked-by from maintainers of the code
being changed.
You have enough from the security folk, but I can't see any included from
the perf folk.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [powerpc:next] BUILD SUCCESS 7074695ac6fb965d478f373b95bc5c636e9f21b0
From: kbuild test robot @ 2020-03-26 23:36 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
branch HEAD: 7074695ac6fb965d478f373b95bc5c636e9f21b0 powerpc/prom_init: Remove leftover comment
elapsed time: 483m
configs tested: 152
configs skipped: 0
The following configs have been built successfully.
More configs may be tested in the coming days.
arm allmodconfig
arm allnoconfig
arm allyesconfig
arm64 allmodconfig
arm64 allnoconfig
arm64 allyesconfig
arm at91_dt_defconfig
arm efm32_defconfig
arm exynos_defconfig
arm multi_v5_defconfig
arm multi_v7_defconfig
arm shmobile_defconfig
arm sunxi_defconfig
arm64 defconfig
sparc allyesconfig
powerpc ppc64_defconfig
um x86_64_defconfig
xtensa iss_defconfig
i386 alldefconfig
i386 allnoconfig
i386 allyesconfig
i386 defconfig
ia64 defconfig
ia64 allmodconfig
ia64 allnoconfig
ia64 allyesconfig
ia64 alldefconfig
c6x allyesconfig
c6x evmc6678_defconfig
nios2 10m50_defconfig
nios2 3c120_defconfig
openrisc or1ksim_defconfig
openrisc simple_smp_defconfig
xtensa common_defconfig
nds32 allnoconfig
csky defconfig
alpha defconfig
nds32 defconfig
h8300 h8s-sim_defconfig
h8300 edosk2674_defconfig
m68k m5475evb_defconfig
m68k allmodconfig
h8300 h8300h-sim_defconfig
m68k sun3_defconfig
m68k multi_defconfig
powerpc rhel-kconfig
arc defconfig
arc allyesconfig
powerpc defconfig
microblaze mmu_defconfig
microblaze nommu_defconfig
powerpc allnoconfig
mips 32r2_defconfig
mips 64r6el_defconfig
mips allmodconfig
mips allnoconfig
mips allyesconfig
mips fuloong2e_defconfig
mips malta_kvm_defconfig
parisc allnoconfig
parisc allyesconfig
parisc generic-32bit_defconfig
parisc generic-64bit_defconfig
x86_64 randconfig-a001-20200326
x86_64 randconfig-a002-20200326
x86_64 randconfig-a003-20200326
i386 randconfig-a001-20200326
i386 randconfig-a002-20200326
i386 randconfig-a003-20200326
alpha randconfig-a001-20200326
m68k randconfig-a001-20200326
mips randconfig-a001-20200326
nds32 randconfig-a001-20200326
parisc randconfig-a001-20200326
riscv randconfig-a001-20200326
c6x randconfig-a001-20200326
h8300 randconfig-a001-20200326
microblaze randconfig-a001-20200326
nios2 randconfig-a001-20200326
sparc64 randconfig-a001-20200326
s390 randconfig-a001-20200326
csky randconfig-a001-20200326
xtensa randconfig-a001-20200326
openrisc randconfig-a001-20200326
sh randconfig-a001-20200326
x86_64 randconfig-c001-20200326
x86_64 randconfig-c002-20200326
x86_64 randconfig-c003-20200326
i386 randconfig-c001-20200326
i386 randconfig-c002-20200326
i386 randconfig-c003-20200326
x86_64 randconfig-e001-20200326
x86_64 randconfig-e002-20200326
x86_64 randconfig-e003-20200326
i386 randconfig-e001-20200326
i386 randconfig-e002-20200326
i386 randconfig-e003-20200326
x86_64 randconfig-f001-20200326
x86_64 randconfig-f002-20200326
x86_64 randconfig-f003-20200326
i386 randconfig-f001-20200326
i386 randconfig-f002-20200326
i386 randconfig-f003-20200326
x86_64 randconfig-g001-20200326
x86_64 randconfig-g002-20200326
x86_64 randconfig-g003-20200326
i386 randconfig-g001-20200326
i386 randconfig-g002-20200326
i386 randconfig-g003-20200326
x86_64 randconfig-h001-20200326
x86_64 randconfig-h002-20200326
x86_64 randconfig-h003-20200326
i386 randconfig-h001-20200326
i386 randconfig-h002-20200326
i386 randconfig-h003-20200326
arc randconfig-a001-20200326
arm randconfig-a001-20200326
arm64 randconfig-a001-20200326
ia64 randconfig-a001-20200326
powerpc randconfig-a001-20200326
sparc randconfig-a001-20200326
riscv allmodconfig
riscv allnoconfig
riscv allyesconfig
riscv defconfig
riscv nommu_virt_defconfig
riscv rv32_defconfig
s390 alldefconfig
s390 allmodconfig
s390 allnoconfig
s390 allyesconfig
s390 debug_defconfig
s390 defconfig
s390 zfcpdump_defconfig
sh rsk7269_defconfig
sh allmodconfig
sh titan_defconfig
sh sh7785lcr_32bit_defconfig
sh allnoconfig
sparc defconfig
sparc64 allmodconfig
sparc64 allnoconfig
sparc64 allyesconfig
sparc64 defconfig
um defconfig
um i386_defconfig
x86_64 fedora-25
x86_64 kexec
x86_64 lkp
x86_64 rhel
x86_64 rhel-7.2-clear
x86_64 rhel-7.6
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS c6624071c338732402e8c726df6a4074473eaa0e
From: kbuild test robot @ 2020-03-27 2:33 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: c6624071c338732402e8c726df6a4074473eaa0e Automatic merge of branches 'master', 'next' and 'fixes' into merge
elapsed time: 660m
configs tested: 152
configs skipped: 0
The following configs have been built successfully.
More configs may be tested in the coming days.
arm allmodconfig
arm allnoconfig
arm allyesconfig
arm64 allmodconfig
arm64 allnoconfig
arm64 allyesconfig
arm at91_dt_defconfig
arm efm32_defconfig
arm exynos_defconfig
arm multi_v5_defconfig
arm multi_v7_defconfig
arm shmobile_defconfig
arm sunxi_defconfig
arm64 defconfig
sparc allyesconfig
um x86_64_defconfig
xtensa iss_defconfig
i386 allnoconfig
i386 allyesconfig
i386 alldefconfig
i386 defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allnoconfig
ia64 allyesconfig
ia64 alldefconfig
c6x allyesconfig
c6x evmc6678_defconfig
nios2 10m50_defconfig
nios2 3c120_defconfig
openrisc or1ksim_defconfig
openrisc simple_smp_defconfig
xtensa common_defconfig
alpha defconfig
csky defconfig
nds32 allnoconfig
nds32 defconfig
h8300 edosk2674_defconfig
h8300 h8300h-sim_defconfig
h8300 h8s-sim_defconfig
m68k allmodconfig
m68k m5475evb_defconfig
m68k multi_defconfig
m68k sun3_defconfig
arc defconfig
arc allyesconfig
powerpc defconfig
powerpc ppc64_defconfig
powerpc rhel-kconfig
microblaze mmu_defconfig
microblaze nommu_defconfig
powerpc allnoconfig
mips 32r2_defconfig
mips 64r6el_defconfig
mips allmodconfig
mips allnoconfig
mips allyesconfig
mips fuloong2e_defconfig
mips malta_kvm_defconfig
parisc allnoconfig
parisc generic-64bit_defconfig
parisc generic-32bit_defconfig
parisc allyesconfig
i386 randconfig-a002-20200326
i386 randconfig-a001-20200326
x86_64 randconfig-a002-20200326
x86_64 randconfig-a001-20200326
i386 randconfig-a003-20200326
x86_64 randconfig-a003-20200326
mips randconfig-a001-20200326
nds32 randconfig-a001-20200326
m68k randconfig-a001-20200326
parisc randconfig-a001-20200326
alpha randconfig-a001-20200326
riscv randconfig-a001-20200326
c6x randconfig-a001-20200326
h8300 randconfig-a001-20200326
microblaze randconfig-a001-20200326
nios2 randconfig-a001-20200326
sparc64 randconfig-a001-20200326
s390 randconfig-a001-20200326
csky randconfig-a001-20200326
xtensa randconfig-a001-20200326
openrisc randconfig-a001-20200326
sh randconfig-a001-20200326
x86_64 randconfig-c001-20200326
x86_64 randconfig-c002-20200326
x86_64 randconfig-c003-20200326
i386 randconfig-c001-20200326
i386 randconfig-c002-20200326
i386 randconfig-c003-20200326
x86_64 randconfig-e001-20200326
x86_64 randconfig-e002-20200326
x86_64 randconfig-e003-20200326
i386 randconfig-e001-20200326
i386 randconfig-e002-20200326
i386 randconfig-e003-20200326
x86_64 randconfig-f001-20200326
x86_64 randconfig-f002-20200326
x86_64 randconfig-f003-20200326
i386 randconfig-f001-20200326
i386 randconfig-f002-20200326
i386 randconfig-f003-20200326
x86_64 randconfig-g001-20200326
x86_64 randconfig-g002-20200326
x86_64 randconfig-g003-20200326
i386 randconfig-g001-20200326
i386 randconfig-g002-20200326
i386 randconfig-g003-20200326
x86_64 randconfig-h001-20200326
x86_64 randconfig-h002-20200326
x86_64 randconfig-h003-20200326
i386 randconfig-h001-20200326
i386 randconfig-h002-20200326
i386 randconfig-h003-20200326
arc randconfig-a001-20200326
arm randconfig-a001-20200326
arm64 randconfig-a001-20200326
ia64 randconfig-a001-20200326
powerpc randconfig-a001-20200326
sparc randconfig-a001-20200326
riscv allmodconfig
riscv allnoconfig
riscv allyesconfig
riscv defconfig
riscv nommu_virt_defconfig
riscv rv32_defconfig
s390 alldefconfig
s390 allmodconfig
s390 allnoconfig
s390 allyesconfig
s390 debug_defconfig
s390 defconfig
s390 zfcpdump_defconfig
sh allmodconfig
sh allnoconfig
sh rsk7269_defconfig
sh sh7785lcr_32bit_defconfig
sh titan_defconfig
sparc defconfig
sparc64 allmodconfig
sparc64 allnoconfig
sparc64 allyesconfig
sparc64 defconfig
um i386_defconfig
um defconfig
x86_64 fedora-25
x86_64 kexec
x86_64 lkp
x86_64 rhel
x86_64 rhel-7.2-clear
x86_64 rhel-7.6
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v2 01/12] powerpc/64s/exceptions: Fix in_mce accounting in unrecoverable path
From: Mahesh J Salgaonkar @ 2020-03-27 3:43 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Ganesh Goudar, linuxppc-dev, Mahesh Salgaonkar
In-Reply-To: <20200325103410.157573-2-npiggin@gmail.com>
On 2020-03-25 20:33:59 Wed, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/exceptions-64s.S | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 6a936c9199d6..67cbcb2d0c7f 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -1335,6 +1335,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
> andc r10,r10,r3
> mtmsrd r10
>
> + lhz r12,PACA_IN_MCE(r13)
> + subi r12,r12,1
> + sth r12,PACA_IN_MCE(r13)
> +
Acked-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Thanks,
-Mahesh.
^ permalink raw reply
* Re: [PATCH v2 1/1] ppc/crash: Skip spinlocks during crash
From: Michael Ellerman @ 2020-03-27 3:50 UTC (permalink / raw)
To: Leonardo Bras, Peter Zijlstra, Ingo Molnar, Will Deacon,
Benjamin Herrenschmidt, Paul Mackerras, Greg Kroah-Hartman,
Thomas Gleixner, Alexios Zavras, Christophe Leroy, Leonardo Bras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200326232542.503157-1-leonardo@linux.ibm.com>
Hi Leonardo,
Leonardo Bras <leonardo@linux.ibm.com> writes:
> During a crash, there is chance that the cpus that handle the NMI IPI
> are holding a spin_lock. If this spin_lock is needed by crashing_cpu it
> will cause a deadlock. (rtas_lock and printk logbuf_log as of today)
Please give us more detail on how those locks are causing you trouble, a
stack trace would be good if you have it.
> This is a problem if the system has kdump set up, given if it crashes
> for any reason kdump may not be saved for crash analysis.
>
> Skip spinlocks after NMI IPI is sent to all other cpus.
We don't want to add overhead to all spinlocks for the life of the
system, just to handle this one case.
There's already a flag that is set when the system is crashing,
"oops_in_progress", maybe we need to use that somewhere to skip a lock
or do an early return.
cheers
> diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
> index 860228e917dc..a6381d110795 100644
> --- a/arch/powerpc/include/asm/spinlock.h
> +++ b/arch/powerpc/include/asm/spinlock.h
> @@ -111,6 +111,8 @@ static inline void splpar_spin_yield(arch_spinlock_t *lock) {};
> static inline void splpar_rw_yield(arch_rwlock_t *lock) {};
> #endif
>
> +extern bool crash_skip_spinlock __read_mostly;
> +
> static inline bool is_shared_processor(void)
> {
> #ifdef CONFIG_PPC_SPLPAR
> @@ -142,6 +144,8 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
> if (likely(__arch_spin_trylock(lock) == 0))
> break;
> do {
> + if (unlikely(crash_skip_spinlock))
> + return;
> HMT_low();
> if (is_shared_processor())
> splpar_spin_yield(lock);
> @@ -161,6 +165,8 @@ void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned long flags)
> local_save_flags(flags_dis);
> local_irq_restore(flags);
> do {
> + if (unlikely(crash_skip_spinlock))
> + return;
> HMT_low();
> if (is_shared_processor())
> splpar_spin_yield(lock);
> diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
> index d488311efab1..ae081f0f2472 100644
> --- a/arch/powerpc/kexec/crash.c
> +++ b/arch/powerpc/kexec/crash.c
> @@ -66,6 +66,9 @@ static int handle_fault(struct pt_regs *regs)
>
> #ifdef CONFIG_SMP
>
> +bool crash_skip_spinlock;
> +EXPORT_SYMBOL(crash_skip_spinlock);
> +
> static atomic_t cpus_in_crash;
> void crash_ipi_callback(struct pt_regs *regs)
> {
> @@ -129,6 +132,7 @@ static void crash_kexec_prepare_cpus(int cpu)
> /* Would it be better to replace the trap vector here? */
>
> if (atomic_read(&cpus_in_crash) >= ncpus) {
> + crash_skip_spinlock = true;
> printk(KERN_EMERG "IPI complete\n");
> return;
> }
> --
> 2.24.1
^ permalink raw reply
* Re: [PATCH v2 04/12] powerpc/pseries/ras: avoid calling rtas_token in NMI paths
From: Mahesh J Salgaonkar @ 2020-03-27 3:50 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Ganesh Goudar, linuxppc-dev, Mahesh Salgaonkar
In-Reply-To: <20200325103410.157573-5-npiggin@gmail.com>
On 2020-03-25 20:34:02 Wed, Nicholas Piggin wrote:
> In the interest of reducing code and possible failures in the
> machine check and system reset paths, grab the "ibm,nmi-interlock"
> token at init time.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Thanks,
-Mahesh.
> ---
> arch/powerpc/include/asm/firmware.h | 1 +
> arch/powerpc/platforms/pseries/ras.c | 2 +-
> arch/powerpc/platforms/pseries/setup.c | 13 ++++++++++---
> 3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
> index ca33f4ef6cb4..6003c2e533a0 100644
> --- a/arch/powerpc/include/asm/firmware.h
> +++ b/arch/powerpc/include/asm/firmware.h
> @@ -128,6 +128,7 @@ extern void machine_check_fwnmi(void);
>
> /* This is true if we are using the firmware NMI handler (typically LPAR) */
> extern int fwnmi_active;
> +extern int ibm_nmi_interlock_token;
>
> extern unsigned int __start___fw_ftr_fixup, __stop___fw_ftr_fixup;
>
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index 1d7f973c647b..c74d5e740922 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -458,7 +458,7 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
> */
> static void fwnmi_release_errinfo(void)
> {
> - int ret = rtas_call(rtas_token("ibm,nmi-interlock"), 0, 1, NULL);
> + int ret = rtas_call(ibm_nmi_interlock_token, 0, 1, NULL);
> if (ret != 0)
> printk(KERN_ERR "FWNMI: nmi-interlock failed: %d\n", ret);
> }
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index 17d17f064a2d..c31acd7ce0c0 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -83,6 +83,7 @@ unsigned long CMO_PageSize = (ASM_CONST(1) << IOMMU_PAGE_SHIFT_4K);
> EXPORT_SYMBOL(CMO_PageSize);
>
> int fwnmi_active; /* TRUE if an FWNMI handler is present */
> +int ibm_nmi_interlock_token;
>
> static void pSeries_show_cpuinfo(struct seq_file *m)
> {
> @@ -113,9 +114,14 @@ static void __init fwnmi_init(void)
> struct slb_entry *slb_ptr;
> size_t size;
> #endif
> + int ibm_nmi_register_token;
>
> - int ibm_nmi_register = rtas_token("ibm,nmi-register");
> - if (ibm_nmi_register == RTAS_UNKNOWN_SERVICE)
> + ibm_nmi_register_token = rtas_token("ibm,nmi-register");
> + if (ibm_nmi_register_token == RTAS_UNKNOWN_SERVICE)
> + return;
> +
> + ibm_nmi_interlock_token = rtas_token("ibm,nmi-interlock");
> + if (WARN_ON(ibm_nmi_interlock_token == RTAS_UNKNOWN_SERVICE))
> return;
>
> /* If the kernel's not linked at zero we point the firmware at low
> @@ -123,7 +129,8 @@ static void __init fwnmi_init(void)
> system_reset_addr = __pa(system_reset_fwnmi) - PHYSICAL_START;
> machine_check_addr = __pa(machine_check_fwnmi) - PHYSICAL_START;
>
> - if (0 == rtas_call(ibm_nmi_register, 2, 1, NULL, system_reset_addr,
> + if (0 == rtas_call(ibm_nmi_register_token, 2, 1, NULL,
> + system_reset_addr,
> machine_check_addr))
> fwnmi_active = 1;
>
> --
> 2.23.0
>
--
Mahesh J Salgaonkar
^ permalink raw reply
* Re: [PATCH v2 05/12] powerpc/pseries/ras: FWNMI_VALID off by one
From: Mahesh J Salgaonkar @ 2020-03-27 3:51 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Mahesh Salgaonkar, Ganesh Goudar, linuxppc-dev
In-Reply-To: <20200325103410.157573-6-npiggin@gmail.com>
On 2020-03-25 20:34:03 Wed, Nicholas Piggin wrote:
> This was discovered developing qemu fwnmi sreset support. This
> off-by-one bug means the last 16 bytes of the rtas area can not
> be used for a 16 byte save area.
>
> It's not a serious bug, and QEMU implementation has to retain a
> workaround for old kernels, but it's good to tighten it.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/platforms/pseries/ras.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index c74d5e740922..9a37bda47468 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -395,10 +395,11 @@ static irqreturn_t ras_error_interrupt(int irq, void *dev_id)
> /*
> * Some versions of FWNMI place the buffer inside the 4kB page starting at
> * 0x7000. Other versions place it inside the rtas buffer. We check both.
> + * Minimum size of the buffer is 16 bytes.
Acked-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Thanks,
-Mahesh.
> */
> #define VALID_FWNMI_BUFFER(A) \
> - ((((A) >= 0x7000) && ((A) < 0x7ff0)) || \
> - (((A) >= rtas.base) && ((A) < (rtas.base + rtas.size - 16))))
> + ((((A) >= 0x7000) && ((A) <= 0x8000 - 16)) || \
> + (((A) >= rtas.base) && ((A) <= (rtas.base + rtas.size - 16))))
>
> static inline struct rtas_error_log *fwnmi_get_errlog(void)
> {
> --
> 2.23.0
>
--
Mahesh J Salgaonkar
^ permalink raw reply
* Re: [PATCH] x86: Alias memset to __builtin_memset.
From: Michael Ellerman @ 2020-03-27 4:06 UTC (permalink / raw)
To: Clement Courbet
Cc: Kees Cook, Borislav Petkov, Greg Kroah-Hartman, x86,
Nick Desaulniers, linux-kernel, clang-built-linux, Ingo Molnar,
Paul Mackerras, Clement Courbet, H. Peter Anvin, Joe Perches,
Bernd Petrovitsch, Nathan Chancellor, linuxppc-dev,
Thomas Gleixner, Allison Randal
In-Reply-To: <20200326123841.134068-1-courbet@google.com>
Clement Courbet <courbet@google.com> writes:
> I discussed with the original authors who added freestanding to our
> build. It turns out that it was added globally but this was just to
> to workaround powerpc not compiling under clang, but they felt the
> fix was appropriate globally.
>
> Now Nick has dug up https://lkml.org/lkml/2019/8/29/1300, which
> advises against freestanding. Also, I've did some research and
> discovered that the original reason for using freestanding for
> powerpc has been fixed here:
> https://lore.kernel.org/linuxppc-dev/20191119045712.39633-3-natechancellor@gmail.com/
>
> I'm going to remove -ffreestanding from downstream, so we don't really need
> this anymore, sorry for waisting people's time.
>
> I wonder if the freestanding fix from the aforementioned patch is really needed
> though. I think that clang is actually right to point out the issue.
> I don't see any reason why setjmp()/longjmp() are declared as taking longs
> rather than ints. The implementation looks like it only ever propagates the
> value (in longjmp) or sets it to 1 (in setjmp), and we only ever call longjmp
> with integer parameters. But I'm not a PowerPC expert, so I might
> be misreading the code.
>
>
> So it seems that we could just remove freestanding altogether and rewrite the
> code to:
>
> diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
> index 279d03a1eec6..7941ae68fe21 100644
> --- a/arch/powerpc/include/asm/setjmp.h
> +++ b/arch/powerpc/include/asm/setjmp.h
> @@ -12,7 +12,9 @@
>
> #define JMP_BUF_LEN 23
> -extern long setjmp(long *);
> -extern void longjmp(long *, long);
> +typedef long * jmp_buf;
> +
> +extern int setjmp(jmp_buf);
> +extern void longjmp(jmp_buf, int);
>
> I'm happy to send a patch for this, and get rid of more -ffreestanding.
> Opinions ?
If it works then it looks like a much better fix than using -ffreestanding.
Please submit a patch with a change log etc. and I'd be happy to merge
it.
cheers
^ permalink raw reply
* Re: [PATCH v2 08/12] powerpc/pseries: limit machine check stack to 4GB
From: Mahesh J Salgaonkar @ 2020-03-27 5:24 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Mahesh Salgaonkar, Ganesh Goudar, linuxppc-dev
In-Reply-To: <20200325103410.157573-9-npiggin@gmail.com>
On 2020-03-25 20:34:06 Wed, Nicholas Piggin wrote:
> This allows rtas_args to be put on the machine check stack, which
> avoids a lot of complications with re-entrancy deadlocks.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/setup_64.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 3bf03666ee09..ca1041f8a578 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -695,6 +695,9 @@ void __init exc_lvl_early_init(void)
> void __init emergency_stack_init(void)
> {
> u64 limit;
> +#ifdef CONFIG_PPC_BOOK3S_64
> + u64 mce_limit;
> +#endif
> unsigned int i;
>
> /*
> @@ -713,6 +716,16 @@ void __init emergency_stack_init(void)
> */
> limit = min(ppc64_bolted_size(), ppc64_rma_size);
>
> + /*
> + * Machine check on pseries calls rtas, but can't use the static
> + * rtas_args due to a machine check hitting while the lock is held.
> + * rtas args have to be under 4GB, so the machine check stack is
> + * limited to 4GB so args can be put on stack.
> + */
> + mce_limit = limit;
> + if (firmware_has_feature(FW_FEATURE_LPAR) && mce_limit > 4UL*1024*1024*1024)
> + mce_limit = 4UL*1024*1024*1024;
> +
Don't you need this as well under CONFIG_PPC_BOOK3S_64 #ifdef ??
Rest looks good.
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Thanks,
-Mahesh.
^ permalink raw reply
* [PATCH v7 0/5] powerpc/hv-24x7: Expose chip/sockets info to add json file metric support for the hv_24x7 socket/chip level events
From: Kajol Jain @ 2020-03-27 6:36 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
Patchset fixes the inconsistent results we are getting when
we run multiple 24x7 events.
"hv_24x7" pmu interface events needs system dependent parameter
like socket/chip/core. For example, hv_24x7 chip level events needs
specific chip-id to which the data is requested should be added as part
of pmu events.
So to enable JSON file support to "hv_24x7" interface, patchset expose
total number of sockets and chips per-socket details in sysfs
files (sockets, chips) under "/sys/devices/hv_24x7/interface/".
To get sockets and number of chips per sockets, patchset adds a rtas call
with token "PROCESSOR_MODULE_INFO" to get these details. Patchset also
handles partition migration case to re-init these system depended
parameters by adding proper calls in post_mobility_fixup() (mobility.c).
v6: http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=164769
Changelog:
v6 -> v7
- Split patchset into two patch series, one with kernel changes
and another with perf tool side changes. This pachset contain
all kernel side changes.
Kajol Jain (5):
powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple
hv-24x7 events run
powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor
details
powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show
processor details
Documentation/ABI: Add ABI documentation for chips and sockets
powerpc/hv-24x7: Update post_mobility_fixup() to handle migration
.../sysfs-bus-event_source-devices-hv_24x7 | 14 +++
arch/powerpc/perf/hv-24x7.c | 104 ++++++++++++++++--
arch/powerpc/platforms/pseries/mobility.c | 12 ++
arch/powerpc/platforms/pseries/pseries.h | 3 +
4 files changed, 123 insertions(+), 10 deletions(-)
--
2.18.1
^ permalink raw reply
* [PATCH v7 1/5] powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple hv-24x7 events run
From: Kajol Jain @ 2020-03-27 6:36 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200327063642.26175-1-kjain@linux.ibm.com>
Commit 2b206ee6b0df ("powerpc/perf/hv-24x7: Display change in counter
values")' added to print _change_ in the counter value rather then raw
value for 24x7 counters. Incase of transactions, the event count
is set to 0 at the beginning of the transaction. It also sets
the event's prev_count to the raw value at the time of initialization.
Because of setting event count to 0, we are seeing some weird behaviour,
whenever we run multiple 24x7 events at a time.
For example:
command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
-C 0 -I 1000 sleep 100
1.000121704 120 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
1.000121704 5 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
2.000357733 8 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
2.000357733 10 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.000641884 56 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
4.000641884 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
5.000791887 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
Getting these large values in case we do -I.
As we are setting event_count to 0, for interval case, overall event_count is not
coming in incremental order. As we may can get new delta lesser then previous count.
Because of which when we print intervals, we are getting negative value which create
these large values.
This patch removes part where we set event_count to 0 in function
'h_24x7_event_read'. There won't be much impact as we do set event->hw.prev_count
to the raw value at the time of initialization to print change value.
With this patch
In power9 platform
command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
-C 0 -I 1000 sleep 100
1.000117685 93 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
1.000117685 1 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
2.000349331 98 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
2.000349331 2 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
3.000495900 131 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
3.000495900 4 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.000645920 204 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
4.000645920 61 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.284169997 22 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Suggested-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 573e0b309c0c..48e8f4b17b91 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -1400,16 +1400,6 @@ static void h_24x7_event_read(struct perf_event *event)
h24x7hw = &get_cpu_var(hv_24x7_hw);
h24x7hw->events[i] = event;
put_cpu_var(h24x7hw);
- /*
- * Clear the event count so we can compute the _change_
- * in the 24x7 raw counter value at the end of the txn.
- *
- * Note that we could alternatively read the 24x7 value
- * now and save its value in event->hw.prev_count. But
- * that would require issuing a hcall, which would then
- * defeat the purpose of using the txn interface.
- */
- local64_set(&event->count, 0);
}
put_cpu_var(hv_24x7_reqb);
--
2.18.1
^ permalink raw reply related
* [PATCH v7 2/5] powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor details
From: Kajol Jain @ 2020-03-27 6:36 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200327063642.26175-1-kjain@linux.ibm.com>
For hv_24x7 socket/chip level events, specific chip-id to which
the data requested should be added as part of pmu events.
But number of chips/socket in the system details are not exposed.
Patch implements read_sys_info_pseries() to get system
parameter values like number of sockets and chips per socket.
Rtas_call with token "PROCESSOR_MODULE_INFO"
is used to get these values.
Sub-sequent patch exports these values via sysfs.
Patch also make these parameters default to 1.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 72 ++++++++++++++++++++++++
arch/powerpc/platforms/pseries/pseries.h | 3 +
2 files changed, 75 insertions(+)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 48e8f4b17b91..9ae00f29bd21 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -20,6 +20,11 @@
#include <asm/io.h>
#include <linux/byteorder/generic.h>
+#ifdef CONFIG_PPC_RTAS
+#include <asm/rtas.h>
+#include <../../platforms/pseries/pseries.h>
+#endif
+
#include "hv-24x7.h"
#include "hv-24x7-catalog.h"
#include "hv-common.h"
@@ -57,6 +62,69 @@ static bool is_physical_domain(unsigned domain)
}
}
+#ifdef CONFIG_PPC_RTAS
+#define PROCESSOR_MODULE_INFO 43
+#define PROCESSOR_MAX_LENGTH (8 * 1024)
+
+static int strbe16toh(const char *buf, int offset)
+{
+ return (buf[offset] << 8) + buf[offset + 1];
+}
+
+static u32 physsockets; /* Physical sockets */
+static u32 physchips; /* Physical chips */
+
+/*
+ * Function read_sys_info_pseries() make a rtas_call which require
+ * data buffer of size 8K. As standard 'rtas_data_buf' is of size
+ * 4K, we are adding new local buffer 'rtas_local_data_buf'.
+ */
+char rtas_local_data_buf[PROCESSOR_MAX_LENGTH] __cacheline_aligned;
+
+/*
+ * read_sys_info_pseries()
+ * Retrieve the number of sockets and chips per socket details
+ * through the get-system-parameter rtas call.
+ */
+void read_sys_info_pseries(void)
+{
+ int call_status, len, ntypes;
+
+ /*
+ * Making system parameter: chips and sockets default to 1.
+ */
+ physsockets = 1;
+ physchips = 1;
+ memset(rtas_local_data_buf, 0, PROCESSOR_MAX_LENGTH);
+ spin_lock(&rtas_data_buf_lock);
+
+ call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
+ NULL,
+ PROCESSOR_MODULE_INFO,
+ __pa(rtas_local_data_buf),
+ PROCESSOR_MAX_LENGTH);
+
+ spin_unlock(&rtas_data_buf_lock);
+
+ if (call_status != 0) {
+ pr_info("%s %s Error calling get-system-parameter (0x%x)\n",
+ __FILE__, __func__, call_status);
+ } else {
+ rtas_local_data_buf[PROCESSOR_MAX_LENGTH - 1] = '\0';
+ len = strbe16toh(rtas_local_data_buf, 0);
+ if (len < 6)
+ return;
+
+ ntypes = strbe16toh(rtas_local_data_buf, 2);
+
+ if (!ntypes)
+ return;
+ physsockets = strbe16toh(rtas_local_data_buf, 4);
+ physchips = strbe16toh(rtas_local_data_buf, 6);
+ }
+}
+#endif /* CONFIG_PPC_RTAS */
+
/* Domains for which more than one result element are returned for each event. */
static bool domain_needs_aggregation(unsigned int domain)
{
@@ -1605,6 +1673,10 @@ static int hv_24x7_init(void)
if (r)
return r;
+#ifdef CONFIG_PPC_RTAS
+ read_sys_info_pseries();
+#endif
+
return 0;
}
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 13fa370a87e4..1727559ce304 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -19,6 +19,9 @@ extern void request_event_sources_irqs(struct device_node *np,
struct pt_regs;
extern int pSeries_system_reset_exception(struct pt_regs *regs);
+#ifdef CONFIG_PPC_RTAS
+extern void read_sys_info_pseries(void);
+#endif
extern int pSeries_machine_check_exception(struct pt_regs *regs);
extern long pseries_machine_check_realmode(struct pt_regs *regs);
--
2.18.1
^ permalink raw reply related
* [PATCH v7 3/5] powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show processor details
From: Kajol Jain @ 2020-03-27 6:36 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200327063642.26175-1-kjain@linux.ibm.com>
To expose the system dependent parameter like total number of
sockets and numbers of chips per socket, patch adds two sysfs files.
"sockets" and "chips" are added to /sys/devices/hv_24x7/interface/
of the "hv_24x7" pmu.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 9ae00f29bd21..a31bd5b88f7a 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -454,6 +454,20 @@ static ssize_t device_show_string(struct device *dev,
return sprintf(buf, "%s\n", (char *)d->var);
}
+#ifdef CONFIG_PPC_RTAS
+static ssize_t sockets_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", physsockets);
+}
+
+static ssize_t chips_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%d\n", physchips);
+}
+#endif
+
static struct attribute *device_str_attr_create_(char *name, char *str)
{
struct dev_ext_attribute *attr = kzalloc(sizeof(*attr), GFP_KERNEL);
@@ -1100,6 +1114,10 @@ PAGE_0_ATTR(catalog_len, "%lld\n",
(unsigned long long)be32_to_cpu(page_0->length) * 4096);
static BIN_ATTR_RO(catalog, 0/* real length varies */);
static DEVICE_ATTR_RO(domains);
+#ifdef CONFIG_PPC_RTAS
+static DEVICE_ATTR_RO(sockets);
+static DEVICE_ATTR_RO(chips);
+#endif
static struct bin_attribute *if_bin_attrs[] = {
&bin_attr_catalog,
@@ -1110,6 +1128,10 @@ static struct attribute *if_attrs[] = {
&dev_attr_catalog_len.attr,
&dev_attr_catalog_version.attr,
&dev_attr_domains.attr,
+#ifdef CONFIG_PPC_RTAS
+ &dev_attr_sockets.attr,
+ &dev_attr_chips.attr,
+#endif
NULL,
};
--
2.18.1
^ permalink raw reply related
* [PATCH v7 4/5] Documentation/ABI: Add ABI documentation for chips and sockets
From: Kajol Jain @ 2020-03-27 6:36 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200327063642.26175-1-kjain@linux.ibm.com>
Add documentation for the following sysfs files:
/sys/devices/hv_24x7/interface/chips,
/sys/devices/hv_24x7/interface/sockets
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
.../testing/sysfs-bus-event_source-devices-hv_24x7 | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7 b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
index ec27c6c9e737..e17e5b444a1c 100644
--- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
+++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
@@ -22,6 +22,20 @@ Description:
Exposes the "version" field of the 24x7 catalog. This is also
extractable from the provided binary "catalog" sysfs entry.
+What: /sys/devices/hv_24x7/interface/sockets
+Date: March 2020
+Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
+Description: read only
+ This sysfs interface exposes the number of sockets present in the
+ system.
+
+What: /sys/devices/hv_24x7/interface/chips
+Date: March 2020
+Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
+Description: read only
+ This sysfs interface exposes the number of chips per socket
+ present in the system.
+
What: /sys/bus/event_source/devices/hv_24x7/event_descs/<event-name>
Date: February 2014
Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
--
2.18.1
^ permalink raw reply related
* [PATCH v7 5/5] powerpc/hv-24x7: Update post_mobility_fixup() to handle migration
From: Kajol Jain @ 2020-03-27 6:36 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200327063642.26175-1-kjain@linux.ibm.com>
Function 'read_sys_info_pseries()' is added to get system parameter
values like number of sockets and chips per socket.
and it gets these details via rtas_call with token
"PROCESSOR_MODULE_INFO".
Incase lpar migrate from one system to another, system
parameter details like chips per sockets or number of sockets might
change. So, it needs to be re-initialized otherwise, these values
corresponds to previous system values.
This patch adds a call to 'read_sys_info_pseries()' from
'post-mobility_fixup()' to re-init the physsockets and physchips values.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/platforms/pseries/mobility.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index b571285f6c14..226accd6218b 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -371,6 +371,18 @@ void post_mobility_fixup(void)
/* Possibly switch to a new RFI flush type */
pseries_setup_rfi_flush();
+ /*
+ * Incase lpar migrate from one system to another, system
+ * parameter details like chips per sockets and number of sockets
+ * might change. So, it needs to be re-initialized otherwise these
+ * values corresponds to previous system.
+ * Here, adding a call to read_sys_info_pseries() declared in
+ * platforms/pseries/pseries.h to re-init the physsockets and
+ * physchips value.
+ */
+ if (IS_ENABLED(CONFIG_HV_PERF_CTRS) && IS_ENABLED(CONFIG_PPC_RTAS))
+ read_sys_info_pseries();
+
return;
}
--
2.18.1
^ permalink raw reply related
* Re: [PATCH V2 0/3] mm/debug: Add more arch page table helper tests
From: Anshuman Khandual @ 2020-03-27 6:46 UTC (permalink / raw)
To: Christophe Leroy, linux-mm
Cc: linux-doc, Heiko Carstens, Paul Mackerras, H. Peter Anvin,
linux-riscv, Will Deacon, linux-arch, linux-s390, Jonathan Corbet,
x86, Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Catalin Marinas, linux-snps-arc, Vasily Gorbik, Borislav Petkov,
Paul Walmsley, Kirill A . Shutemov, Thomas Gleixner,
linux-arm-kernel, Vineet Gupta, linux-kernel, Palmer Dabbelt,
Andrew Morton, linuxppc-dev
In-Reply-To: <a46d18ed-8911-1ec3-c32f-58b6e0d959d7@c-s.fr>
On 03/26/2020 08:53 PM, Christophe Leroy wrote:
>
>
> Le 26/03/2020 à 03:23, Anshuman Khandual a écrit :
>>
>>
>> On 03/24/2020 10:52 AM, Anshuman Khandual wrote:
>>> This series adds more arch page table helper tests. The new tests here are
>>> either related to core memory functions and advanced arch pgtable helpers.
>>> This also creates a documentation file enlisting all expected semantics as
>>> suggested by Mike Rapoport (https://lkml.org/lkml/2020/1/30/40).
>>>
>>> This series has been tested on arm64 and x86 platforms.
>>
>> If folks can test these patches out on remaining ARCH_HAS_DEBUG_VM_PGTABLE
>> enabled platforms i.e s390, arc, powerpc (32 and 64), that will be really
>> appreciated. Thank you.
>>
>
> On powerpc 8xx (PPC32), I get:
>
> [ 53.338368] debug_vm_pgtable: debug_vm_pgtable: Validating architecture page table helpers
> [ 53.347403] ------------[ cut here ]------------
> [ 53.351832] WARNING: CPU: 0 PID: 1 at mm/debug_vm_pgtable.c:647 debug_vm_pgtable+0x280/0x3f4
mm/debug_vm_pgtable.c:647 ?
With the following commits in place
53a8338ce (HEAD) Documentation/mm: Add descriptions for arch page table helper
5d4913fc1 mm/debug: Add tests validating arch advanced page table helpers
bcaf120a7 mm/debug: Add tests validating arch page table helpers for core features
d6ed5a4a5 x86/memory: Drop pud_mknotpresent()
0739d1f8d mm/debug: Add tests validating architecture page table helpers
16fbf79b0 (tag: v5.6-rc7) Linux 5.6-rc7
mm/debug_vm_pgtable.c:647 is here.
#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot)
{
swp_entry_t swp;
pmd_t pmd; -----------------------------> Line #647
pmd = pfn_pmd(pfn, prot);
swp = __pmd_to_swp_entry(pmd);
WARN_ON(!pmd_same(pmd, __swp_entry_to_pmd(swp)));
}
#else
static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot) { }
#end
Did I miss something ?
> [ 53.360140] CPU: 0 PID: 1 Comm: swapper Not tainted 5.6.0-rc7-s3k-dev-01090-g92710e99881f #3544
> [ 53.368718] NIP: c0777c04 LR: c0777bb8 CTR: 00000000
> [ 53.373720] REGS: c9023df0 TRAP: 0700 Not tainted (5.6.0-rc7-s3k-dev-01090-g92710e99881f)
> [ 53.382042] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 22000222 XER: 20000000
> [ 53.388667]
> [ 53.388667] GPR00: c0777bb8 c9023ea8 c6120000 00000001 1e410000 00000000 00000000 007641c9
> [ 53.388667] GPR08: 00000000 00000001 00000000 ffffffff 82000222 00000000 c00039b8 00000000
> [ 53.388667] GPR16: 00000000 00000000 00000000 fffffff0 065fc000 1e410000 c6600000 000001e4
> [ 53.388667] GPR24: 000001d9 c062d14c c65fc000 c642d448 000006c9 00000000 c65f8000 c65fc040
> [ 53.423400] NIP [c0777c04] debug_vm_pgtable+0x280/0x3f4
> [ 53.428559] LR [c0777bb8] debug_vm_pgtable+0x234/0x3f4
> [ 53.433593] Call Trace:
> [ 53.436048] [c9023ea8] [c0777bb8] debug_vm_pgtable+0x234/0x3f4 (unreliable)
> [ 53.442936] [c9023f28] [c00039e0] kernel_init+0x28/0x124
> [ 53.448184] [c9023f38] [c000f174] ret_from_kernel_thread+0x14/0x1c
> [ 53.454245] Instruction dump:
> [ 53.457180] 41a20008 4bea3ed9 62890021 7d36b92e 7d36b82e 71290fd0 3149ffff 7d2a4910
> [ 53.464838] 0f090000 5789077e 3149ffff 7d2a4910 <0f090000> 38c00000 38a00000 38800000
> [ 53.472671] ---[ end trace fd5dd92744dc0065 ]---
Could you please point me to the exact test which is failing ?
> [ 53.519778] Freeing unused kernel memory: 608K
>
>
So I assume that the system should have come till runtime just fine apart from
the above warning message because.
^ permalink raw reply
* Re: [PATCH v2] powerpc xmon: use `dcbf` inplace of `dcbi` instruction for 64bit Book3S
From: Christophe Leroy @ 2020-03-27 6:48 UTC (permalink / raw)
To: Balamuruhan S, mpe
Cc: ravi.bangoria, jniethe5, paulus, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20200326061522.33123-1-bala24@linux.ibm.com>
Le 26/03/2020 à 07:15, Balamuruhan S a écrit :
> Data Cache Block Invalidate (dcbi) instruction was implemented back in PowerPC
> architecture version 2.03. It is obsolete and attempt to use of this illegal
> instruction results in a hypervisor emulation assistance interrupt. So, ifdef
> it out the option `i` in xmon for 64bit Book3S.
I don't understand. You say two contradictory things:
1/ You say it _was_ added back.
2/ You say it _is_ obsolete.
How can it be obsolete if it was added back ?
[...]
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 0ec9640335bb..bfd5a97689cd 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -335,10 +335,12 @@ static inline void cflush(void *p)
> asm volatile ("dcbf 0,%0; icbi 0,%0" : : "r" (p));
> }
>
> +#ifndef CONFIG_PPC_BOOK3S_64
You don't need that #ifndef. Keeping it should be harmless.
> static inline void cinval(void *p)
> {
> asm volatile ("dcbi 0,%0; icbi 0,%0" : : "r" (p));
> }
> +#endif
>
> /**
> * write_ciabr() - write the CIABR SPR
> @@ -1791,8 +1793,9 @@ static void prregs(struct pt_regs *fp)
>
> static void cacheflush(void)
> {
> - int cmd;
> unsigned long nflush;
> +#ifndef CONFIG_PPC_BOOK3S_64
Don't make it so complex, see below
> + int cmd;
>
> cmd = inchar();
> if (cmd != 'i')
> @@ -1800,13 +1803,14 @@ static void cacheflush(void)
> scanhex((void *)&adrs);
> if (termch != '\n')
> termch = 0;
> +#endif
> nflush = 1;
> scanhex(&nflush);
> nflush = (nflush + L1_CACHE_BYTES - 1) / L1_CACHE_BYTES;
> if (setjmp(bus_error_jmp) == 0) {
> catch_memory_errors = 1;
> sync();
> -
> +#ifndef CONFIG_PPC_BOOK3S_64
You don't need that ifndef, just ensure below that regardless of cmd,
book3s/64 calls cflush and not cinval.
> if (cmd != 'i') {
The only thing you have to do is to replace the above test by:
if (cmd != 'i' || IS_ENABLED(CONFIG_PPC_BOOK3S_64)) {
> for (; nflush > 0; --nflush, adrs += L1_CACHE_BYTES)
> cflush((void *) adrs);
> @@ -1814,6 +1818,10 @@ static void cacheflush(void)
> for (; nflush > 0; --nflush, adrs += L1_CACHE_BYTES)
> cinval((void *) adrs);
> }
> +#else
Don't need that at all, it's a duplication of the above.
> + for (; nflush > 0; --nflush, adrs += L1_CACHE_BYTES)
> + cflush((void *)adrs);
> +#endif
> sync();
> /* wait a little while to see if we get a machine check */
> __delay(200);
>
> base-commit: a87b93bdf800a4d7a42d95683624a4516e516b4f
>
Christophe
^ permalink raw reply
* Re: [PATCH 1/1] ppc/crash: Skip spinlocks during crash
From: Christophe Leroy @ 2020-03-27 6:50 UTC (permalink / raw)
To: Leonardo Bras, Peter Zijlstra, Ingo Molnar, Will Deacon,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Enrico Weigelt, Allison Randal, Thomas Gleixner
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200326222836.501404-1-leonardo@linux.ibm.com>
Le 26/03/2020 à 23:28, Leonardo Bras a écrit :
> During a crash, there is chance that the cpus that handle the NMI IPI
> are holding a spin_lock. If this spin_lock is needed by crashing_cpu it
> will cause a deadlock. (rtas_lock and printk logbuf_log as of today)
>
> This is a problem if the system has kdump set up, given if it crashes
> for any reason kdump may not be saved for crash analysis.
>
> Skip spinlocks after NMI IPI is sent to all other cpus.
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> arch/powerpc/include/asm/spinlock.h | 6 ++++++
> arch/powerpc/kexec/crash.c | 3 +++
> 2 files changed, 9 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
> index 860228e917dc..a6381d110795 100644
> --- a/arch/powerpc/include/asm/spinlock.h
> +++ b/arch/powerpc/include/asm/spinlock.h
> @@ -111,6 +111,8 @@ static inline void splpar_spin_yield(arch_spinlock_t *lock) {};
> static inline void splpar_rw_yield(arch_rwlock_t *lock) {};
> #endif
>
> +extern bool crash_skip_spinlock __read_mostly;
> +
> static inline bool is_shared_processor(void)
> {
> #ifdef CONFIG_PPC_SPLPAR
> @@ -142,6 +144,8 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
> if (likely(__arch_spin_trylock(lock) == 0))
> break;
> do {
> + if (unlikely(crash_skip_spinlock))
> + return;
You are adding a test that reads a global var in the middle of a so hot
path ? That must kill performance. Can we do different ?
Christophe
^ permalink raw reply
* Re: [PATCH V2 0/3] mm/debug: Add more arch page table helper tests
From: Christophe Leroy @ 2020-03-27 7:00 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm
Cc: linux-doc, Heiko Carstens, Paul Mackerras, H. Peter Anvin,
linux-riscv, Will Deacon, linux-arch, linux-s390, Jonathan Corbet,
x86, Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Catalin Marinas, linux-snps-arc, Vasily Gorbik, Borislav Petkov,
Paul Walmsley, Kirill A . Shutemov, Thomas Gleixner,
linux-arm-kernel, Vineet Gupta, linux-kernel, Palmer Dabbelt,
Andrew Morton, linuxppc-dev
In-Reply-To: <9675882f-0ec5-5e46-551f-dd3aa38bf8d8@arm.com>
On 03/27/2020 06:46 AM, Anshuman Khandual wrote:
>
> On 03/26/2020 08:53 PM, Christophe Leroy wrote:
>>
>>
>> Le 26/03/2020 à 03:23, Anshuman Khandual a écrit :
>>>
>>>
>>> On 03/24/2020 10:52 AM, Anshuman Khandual wrote:
>>>> This series adds more arch page table helper tests. The new tests here are
>>>> either related to core memory functions and advanced arch pgtable helpers.
>>>> This also creates a documentation file enlisting all expected semantics as
>>>> suggested by Mike Rapoport (https://lkml.org/lkml/2020/1/30/40).
>>>>
>>>> This series has been tested on arm64 and x86 platforms.
>>>
>>> If folks can test these patches out on remaining ARCH_HAS_DEBUG_VM_PGTABLE
>>> enabled platforms i.e s390, arc, powerpc (32 and 64), that will be really
>>> appreciated. Thank you.
>>>
>>
>> On powerpc 8xx (PPC32), I get:
>>
>> [ 53.338368] debug_vm_pgtable: debug_vm_pgtable: Validating architecture page table helpers
>> [ 53.347403] ------------[ cut here ]------------
>> [ 53.351832] WARNING: CPU: 0 PID: 1 at mm/debug_vm_pgtable.c:647 debug_vm_pgtable+0x280/0x3f4
>
> mm/debug_vm_pgtable.c:647 ?
>
> With the following commits in place
>
> 53a8338ce (HEAD) Documentation/mm: Add descriptions for arch page table helper
> 5d4913fc1 mm/debug: Add tests validating arch advanced page table helpers
> bcaf120a7 mm/debug: Add tests validating arch page table helpers for core features
> d6ed5a4a5 x86/memory: Drop pud_mknotpresent()
> 0739d1f8d mm/debug: Add tests validating architecture page table helpers
> 16fbf79b0 (tag: v5.6-rc7) Linux 5.6-rc7
I have:
facaa5eb5909 (HEAD -> helpers0) mm/debug: Add tests validating arch
advanced page table helpers
6389fed515fc mm/debug: Add tests validating arch page table helpers for
core features
dc14ecc8b94e mm/debug: add tests validating architecture page table helpers
c6624071c338 (origin/merge, merge) Automatic merge of branches 'master',
'next' and 'fixes' into merge
58e05c5508e6 Automatic merge of branches 'master', 'next' and 'fixes'
into merge
1b649e0bcae7 (origin/master, origin/HEAD) Merge
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
origin is https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
I can't see your last patch in powerpc mailing list
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=166237)
>
> mm/debug_vm_pgtable.c:647 is here.
Line 647 is:
WARN_ON(!pte_same(pte, __swp_entry_to_pte(swp)));
>
> #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
> static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot)
> {
> swp_entry_t swp;
> pmd_t pmd; -----------------------------> Line #647
>
> pmd = pfn_pmd(pfn, prot);
> swp = __pmd_to_swp_entry(pmd);
> WARN_ON(!pmd_same(pmd, __swp_entry_to_pmd(swp)));
> }
> #else
> static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot) { }
> #end
>
> Did I miss something ?
>
[...]
> Could you please point me to the exact test which is failing ?
>
>> [ 53.519778] Freeing unused kernel memory: 608K
>>
>>
> So I assume that the system should have come till runtime just fine apart from
> the above warning message because.
>
Yes it boots fine otherwise.
Christophe
^ permalink raw reply
* [PATCH 1/4] powerpc/64s: implement probe_kernel_read/write without touching AMR
From: Nicholas Piggin @ 2020-03-27 7:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
There is no need to allow user accesses when probing kernel addresses.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/uaccess.h | 25 ++++++++++-----
arch/powerpc/lib/Makefile | 2 +-
arch/powerpc/lib/uaccess.c | 50 ++++++++++++++++++++++++++++++
3 files changed, 68 insertions(+), 9 deletions(-)
create mode 100644 arch/powerpc/lib/uaccess.c
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 2f500debae21..670910df3cc7 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -341,8 +341,8 @@ raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
}
#endif /* __powerpc64__ */
-static inline unsigned long raw_copy_from_user(void *to,
- const void __user *from, unsigned long n)
+static inline unsigned long
+raw_copy_from_user_allowed(void *to, const void __user *from, unsigned long n)
{
unsigned long ret;
if (__builtin_constant_p(n) && (n <= 8)) {
@@ -351,19 +351,19 @@ static inline unsigned long raw_copy_from_user(void *to,
switch (n) {
case 1:
barrier_nospec();
- __get_user_size(*(u8 *)to, from, 1, ret);
+ __get_user_size_allowed(*(u8 *)to, from, 1, ret);
break;
case 2:
barrier_nospec();
- __get_user_size(*(u16 *)to, from, 2, ret);
+ __get_user_size_allowed(*(u16 *)to, from, 2, ret);
break;
case 4:
barrier_nospec();
- __get_user_size(*(u32 *)to, from, 4, ret);
+ __get_user_size_allowed(*(u32 *)to, from, 4, ret);
break;
case 8:
barrier_nospec();
- __get_user_size(*(u64 *)to, from, 8, ret);
+ __get_user_size_allowed(*(u64 *)to, from, 8, ret);
break;
}
if (ret == 0)
@@ -371,9 +371,18 @@ static inline unsigned long raw_copy_from_user(void *to,
}
barrier_nospec();
- allow_read_from_user(from, n);
ret = __copy_tofrom_user((__force void __user *)to, from, n);
- prevent_read_from_user(from, n);
+ return ret;
+}
+
+static inline unsigned long
+raw_copy_from_user(void *to, const void __user *from, unsigned long n)
+{
+ unsigned long ret;
+
+ allow_read_from_user(to, n);
+ ret = raw_copy_from_user_allowed(to, from, n);
+ prevent_read_from_user(to, n);
return ret;
}
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index b8de3be10eb4..a15060b5008e 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -36,7 +36,7 @@ extra-$(CONFIG_PPC64) += crtsavres.o
endif
obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
- memcpy_power7.o
+ memcpy_power7.o uaccess.o
obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
memcpy_64.o memcpy_mcsafe_64.o
diff --git a/arch/powerpc/lib/uaccess.c b/arch/powerpc/lib/uaccess.c
new file mode 100644
index 000000000000..0057ab52d6fe
--- /dev/null
+++ b/arch/powerpc/lib/uaccess.c
@@ -0,0 +1,50 @@
+#include <linux/mm.h>
+#include <linux/uaccess.h>
+
+static __always_inline long
+probe_read_common(void *dst, const void __user *src, size_t size)
+{
+ long ret;
+
+ pagefault_disable();
+ ret = raw_copy_from_user_allowed(dst, src, size);
+ pagefault_enable();
+
+ return ret ? -EFAULT : 0;
+}
+
+static __always_inline long
+probe_write_common(void __user *dst, const void *src, size_t size)
+{
+ long ret;
+
+ pagefault_disable();
+ ret = raw_copy_to_user_allowed(dst, src, size);
+ pagefault_enable();
+
+ return ret ? -EFAULT : 0;
+}
+
+long probe_kernel_read(void *dst, const void *src, size_t size)
+{
+ long ret;
+ mm_segment_t old_fs = get_fs();
+
+ set_fs(KERNEL_DS);
+ ret = probe_read_common(dst, (__force const void __user *)src, size);
+ set_fs(old_fs);
+
+ return ret;
+}
+
+long probe_kernel_write(void *dst, const void *src, size_t size)
+{
+ long ret;
+ mm_segment_t old_fs = get_fs();
+
+ set_fs(KERNEL_DS);
+ ret = probe_write_common((__force void __user *)dst, src, size);
+ set_fs(old_fs);
+
+ return ret;
+}
--
2.23.0
^ permalink raw reply related
* [PATCH 2/4] powerpc/64s: use mmu_has_feature in set_kuap() and get_kuap()
From: Nicholas Piggin @ 2020-03-27 7:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20200327070240.427074-1-npiggin@gmail.com>
Commit 8150a153c013 ("powerpc/64s: Use early_mmu_has_feature() in
set_kuap()"), had to switch to using the _early feature test, because
probe_kernel_read was being called very early. After the previous
patch, probe_kernel_read no longer touches kuap, so it can go back to
using the non-_early variant, for better performance.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/book3s/64/kup-radix.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
index 3bcef989a35d..67a7fd0182e6 100644
--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
@@ -79,7 +79,7 @@ static inline void kuap_check_amr(void)
static inline unsigned long get_kuap(void)
{
- if (!early_mmu_has_feature(MMU_FTR_RADIX_KUAP))
+ if (!mmu_has_feature(MMU_FTR_RADIX_KUAP))
return 0;
return mfspr(SPRN_AMR);
@@ -87,7 +87,7 @@ static inline unsigned long get_kuap(void)
static inline void set_kuap(unsigned long value)
{
- if (!early_mmu_has_feature(MMU_FTR_RADIX_KUAP))
+ if (!mmu_has_feature(MMU_FTR_RADIX_KUAP))
return;
/*
--
2.23.0
^ permalink raw reply related
* [PATCH 3/4] powerpc/uaccess: evaluate macro arguments once, before user access is allowed
From: Nicholas Piggin @ 2020-03-27 7:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20200327070240.427074-1-npiggin@gmail.com>
get/put_user can be called with nontrivial arguments. fs/proc/page.c
has a good example:
if (put_user(stable_page_flags(ppage), out)) {
stable_page_flags is quite a lot of code, including spin locks in the
page allocator.
Ensure these arguments are evaluated before user access is allowed.
This improves security by reducing code with access to userspace, but
it also fixes a PREEMPT bug with KUAP on powerpc/64s:
stable_page_flags is currently called with AMR set to allow writes,
it ends up calling spin_unlock(), which can call preempt_schedule. But
the task switch code can not be called with AMR set (it relies on
interrupts saving the register), so this blows up.
It's fine if the code inside allow_user_access is preemptible, because
a timer or IPI will save the AMR, but it's not okay to explicitly
cause a reschedule.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/uaccess.h | 97 ++++++++++++++++++------------
1 file changed, 59 insertions(+), 38 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 670910df3cc7..1cf8595aeef1 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -162,36 +162,48 @@ do { \
prevent_write_to_user(ptr, size); \
} while (0)
-#define __put_user_nocheck(x, ptr, size, do_allow) \
+#define __put_user_nocheck(x, ptr, size, do_allow) \
({ \
long __pu_err; \
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
+ __typeof__(*(ptr)) __pu_val = (x); \
+ __typeof__(size) __pu_size = (size); \
+ \
if (!is_kernel_addr((unsigned long)__pu_addr)) \
might_fault(); \
- __chk_user_ptr(ptr); \
- if (do_allow) \
- __put_user_size((x), __pu_addr, (size), __pu_err); \
- else \
- __put_user_size_allowed((x), __pu_addr, (size), __pu_err); \
+ __chk_user_ptr(__pu_addr); \
+ if (do_allow) \
+ __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
+ else \
+ __put_user_size_allowed(__pu_val, __pu_addr, __pu_size, __pu_err); \
+ \
__pu_err; \
})
-#define __put_user_check(x, ptr, size) \
-({ \
- long __pu_err = -EFAULT; \
- __typeof__(*(ptr)) __user *__pu_addr = (ptr); \
- might_fault(); \
- if (access_ok(__pu_addr, size)) \
- __put_user_size((x), __pu_addr, (size), __pu_err); \
- __pu_err; \
+#define __put_user_check(x, ptr, size) \
+({ \
+ long __pu_err = -EFAULT; \
+ __typeof__(*(ptr)) __user *__pu_addr = (ptr); \
+ __typeof__(*(ptr)) __pu_val = (x); \
+ __typeof__(size) __pu_size = (size); \
+ \
+ might_fault(); \
+ if (access_ok(__pu_addr, __pu_size)) \
+ __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
+ \
+ __pu_err; \
})
#define __put_user_nosleep(x, ptr, size) \
({ \
long __pu_err; \
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
- __chk_user_ptr(ptr); \
- __put_user_size((x), __pu_addr, (size), __pu_err); \
+ __typeof__(*(ptr)) __pu_val = (x); \
+ __typeof__(size) __pu_size = (size); \
+ \
+ __chk_user_ptr(__pu_addr); \
+ __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
+ \
__pu_err; \
})
@@ -278,46 +290,55 @@ do { \
#define __long_type(x) \
__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
-#define __get_user_nocheck(x, ptr, size, do_allow) \
+#define __get_user_nocheck(x, ptr, size, do_allow) \
({ \
long __gu_err; \
__long_type(*(ptr)) __gu_val; \
- __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
- __chk_user_ptr(ptr); \
+ __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
+ __typeof__(size) __gu_size = (size); \
+ \
+ __chk_user_ptr(__gu_addr); \
if (!is_kernel_addr((unsigned long)__gu_addr)) \
might_fault(); \
barrier_nospec(); \
- if (do_allow) \
- __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
- else \
- __get_user_size_allowed(__gu_val, __gu_addr, (size), __gu_err); \
+ if (do_allow) \
+ __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
+ else \
+ __get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
(x) = (__typeof__(*(ptr)))__gu_val; \
+ \
__gu_err; \
})
-#define __get_user_check(x, ptr, size) \
-({ \
- long __gu_err = -EFAULT; \
- __long_type(*(ptr)) __gu_val = 0; \
+#define __get_user_check(x, ptr, size) \
+({ \
+ long __gu_err = -EFAULT; \
+ __long_type(*(ptr)) __gu_val = 0; \
__typeof__(*(ptr)) __user *__gu_addr = (ptr); \
- might_fault(); \
- if (access_ok(__gu_addr, (size))) { \
- barrier_nospec(); \
- __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
- } \
- (x) = (__force __typeof__(*(ptr)))__gu_val; \
- __gu_err; \
+ __typeof__(size) __gu_size = (size); \
+ \
+ might_fault(); \
+ if (access_ok(__gu_addr, __gu_size)) { \
+ barrier_nospec(); \
+ __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
+ } \
+ (x) = (__force __typeof__(*(ptr)))__gu_val; \
+ \
+ __gu_err; \
})
#define __get_user_nosleep(x, ptr, size) \
({ \
long __gu_err; \
__long_type(*(ptr)) __gu_val; \
- __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
- __chk_user_ptr(ptr); \
+ __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
+ __typeof__(size) __gu_size = (size); \
+ \
+ __chk_user_ptr(__gu_addr); \
barrier_nospec(); \
- __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
- (x) = (__force __typeof__(*(ptr)))__gu_val; \
+ __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
+ (x) = (__force __typeof__(*(ptr)))__gu_val; \
+ \
__gu_err; \
})
--
2.23.0
^ permalink raw reply related
* [PATCH 4/4] powerpc/uaccess: add more __builtin_expect annotations
From: Nicholas Piggin @ 2020-03-27 7:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20200327070240.427074-1-npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/uaccess.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 1cf8595aeef1..896d43d8c891 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -48,16 +48,16 @@ static inline void set_fs(mm_segment_t fs)
* gap between user addresses and the kernel addresses
*/
#define __access_ok(addr, size, segment) \
- (((addr) <= (segment).seg) && ((size) <= (segment).seg))
+ likely(((addr) <= (segment).seg) && ((size) <= (segment).seg))
#else
static inline int __access_ok(unsigned long addr, unsigned long size,
mm_segment_t seg)
{
- if (addr > seg.seg)
+ if (unlikely(addr > seg.seg))
return 0;
- return (size == 0 || size - 1 <= seg.seg - addr);
+ return likely(size == 0 || size - 1 <= seg.seg - addr);
}
#endif
@@ -177,7 +177,7 @@ do { \
else \
__put_user_size_allowed(__pu_val, __pu_addr, __pu_size, __pu_err); \
\
- __pu_err; \
+ __builtin_expect(__pu_err, 0); \
})
#define __put_user_check(x, ptr, size) \
@@ -191,7 +191,7 @@ do { \
if (access_ok(__pu_addr, __pu_size)) \
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
\
- __pu_err; \
+ __builtin_expect(__pu_err, 0); \
})
#define __put_user_nosleep(x, ptr, size) \
@@ -204,7 +204,7 @@ do { \
__chk_user_ptr(__pu_addr); \
__put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
\
- __pu_err; \
+ __builtin_expect(__pu_err, 0); \
})
@@ -307,7 +307,7 @@ do { \
__get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
(x) = (__typeof__(*(ptr)))__gu_val; \
\
- __gu_err; \
+ __builtin_expect(__gu_err, 0); \
})
#define __get_user_check(x, ptr, size) \
@@ -324,7 +324,7 @@ do { \
} \
(x) = (__force __typeof__(*(ptr)))__gu_val; \
\
- __gu_err; \
+ __builtin_expect(__gu_err, 0); \
})
#define __get_user_nosleep(x, ptr, size) \
@@ -339,7 +339,7 @@ do { \
__get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
(x) = (__force __typeof__(*(ptr)))__gu_val; \
\
- __gu_err; \
+ __builtin_expect(__gu_err, 0); \
})
--
2.23.0
^ permalink raw reply related
* Re: [PATCH 1/4] powerpc/64s: implement probe_kernel_read/write without touching AMR
From: Christophe Leroy @ 2020-03-27 7:13 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20200327070240.427074-1-npiggin@gmail.com>
Le 27/03/2020 à 08:02, Nicholas Piggin a écrit :
> There is no need to allow user accesses when probing kernel addresses.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/uaccess.h | 25 ++++++++++-----
> arch/powerpc/lib/Makefile | 2 +-
> arch/powerpc/lib/uaccess.c | 50 ++++++++++++++++++++++++++++++
> 3 files changed, 68 insertions(+), 9 deletions(-)
> create mode 100644 arch/powerpc/lib/uaccess.c
>
[...]
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index b8de3be10eb4..a15060b5008e 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -36,7 +36,7 @@ extra-$(CONFIG_PPC64) += crtsavres.o
> endif
>
> obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
> - memcpy_power7.o
> + memcpy_power7.o uaccess.o
Why only book3s/64 ? It applies to the 8xx and book3s/32 as well, I
think it should just be for all powerpc.
>
> obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
> memcpy_64.o memcpy_mcsafe_64.o
> diff --git a/arch/powerpc/lib/uaccess.c b/arch/powerpc/lib/uaccess.c
> new file mode 100644
> index 000000000000..0057ab52d6fe
> --- /dev/null
> +++ b/arch/powerpc/lib/uaccess.c
> @@ -0,0 +1,50 @@
> +#include <linux/mm.h>
> +#include <linux/uaccess.h>
> +
> +static __always_inline long
> +probe_read_common(void *dst, const void __user *src, size_t size)
> +{
> + long ret;
> +
> + pagefault_disable();
> + ret = raw_copy_from_user_allowed(dst, src, size);
> + pagefault_enable();
> +
> + return ret ? -EFAULT : 0;
> +}
> +
> +static __always_inline long
> +probe_write_common(void __user *dst, const void *src, size_t size)
> +{
> + long ret;
> +
> + pagefault_disable();
> + ret = raw_copy_to_user_allowed(dst, src, size);
> + pagefault_enable();
> +
> + return ret ? -EFAULT : 0;
> +}
> +
> +long probe_kernel_read(void *dst, const void *src, size_t size)
> +{
> + long ret;
> + mm_segment_t old_fs = get_fs();
> +
> + set_fs(KERNEL_DS);
> + ret = probe_read_common(dst, (__force const void __user *)src, size);
I think you should squash probe_read_common() here, having it separated
is a lot of lines for no added value. It also may make people believe it
overwrites the generic probe_read_common()
> + set_fs(old_fs);
> +
> + return ret;
> +}
> +
> +long probe_kernel_write(void *dst, const void *src, size_t size)
> +{
> + long ret;
> + mm_segment_t old_fs = get_fs();
> +
> + set_fs(KERNEL_DS);
> + ret = probe_write_common((__force void __user *)dst, src, size);
Same comment as for probe_read_common()
> + set_fs(old_fs);
> +
> + return ret;
> +}
>
Christophe
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/uaccess: evaluate macro arguments once, before user access is allowed
From: Christophe Leroy @ 2020-03-27 7:21 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20200327070240.427074-3-npiggin@gmail.com>
Le 27/03/2020 à 08:02, Nicholas Piggin a écrit :
> get/put_user can be called with nontrivial arguments. fs/proc/page.c
> has a good example:
>
> if (put_user(stable_page_flags(ppage), out)) {
>
> stable_page_flags is quite a lot of code, including spin locks in the
> page allocator.
>
> Ensure these arguments are evaluated before user access is allowed.
> This improves security by reducing code with access to userspace, but
> it also fixes a PREEMPT bug with KUAP on powerpc/64s:
> stable_page_flags is currently called with AMR set to allow writes,
> it ends up calling spin_unlock(), which can call preempt_schedule. But
> the task switch code can not be called with AMR set (it relies on
> interrupts saving the register), so this blows up.
>
> It's fine if the code inside allow_user_access is preemptible, because
> a timer or IPI will save the AMR, but it's not okay to explicitly
> cause a reschedule.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/uaccess.h | 97 ++++++++++++++++++------------
> 1 file changed, 59 insertions(+), 38 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 670910df3cc7..1cf8595aeef1 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -162,36 +162,48 @@ do { \
> prevent_write_to_user(ptr, size); \
> } while (0)
>
> -#define __put_user_nocheck(x, ptr, size, do_allow) \
> +#define __put_user_nocheck(x, ptr, size, do_allow) \
No need to touch this line. Anyway at the end, you still have several \
which are not aligned.
> ({ \
> long __pu_err; \
> __typeof__(*(ptr)) __user *__pu_addr = (ptr); \
> + __typeof__(*(ptr)) __pu_val = (x); \
> + __typeof__(size) __pu_size = (size); \
> + \
> if (!is_kernel_addr((unsigned long)__pu_addr)) \
> might_fault(); \
> - __chk_user_ptr(ptr); \
> - if (do_allow) \
No need to touch that line
> - __put_user_size((x), __pu_addr, (size), __pu_err); \
> - else \
No need to touch that line
> - __put_user_size_allowed((x), __pu_addr, (size), __pu_err); \
> + __chk_user_ptr(__pu_addr); \
> + if (do_allow) \
> + __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
> + else \
> + __put_user_size_allowed(__pu_val, __pu_addr, __pu_size, __pu_err); \
> + \
> __pu_err; \
> })
>
> -#define __put_user_check(x, ptr, size) \
> -({ \
> - long __pu_err = -EFAULT; \
> - __typeof__(*(ptr)) __user *__pu_addr = (ptr); \
> - might_fault(); \
> - if (access_ok(__pu_addr, size)) \
> - __put_user_size((x), __pu_addr, (size), __pu_err); \
> - __pu_err; \
Same comment applies, you are touching some lines just to change the \,
but at the end you still have some misaligned ones.
It would help the review not to touch unchanged lines just for that.
Same comment applies a few places below as well.
> +#define __put_user_check(x, ptr, size) \
> +({ \
> + long __pu_err = -EFAULT; \
> + __typeof__(*(ptr)) __user *__pu_addr = (ptr); \
> + __typeof__(*(ptr)) __pu_val = (x); \
> + __typeof__(size) __pu_size = (size); \
> + \
> + might_fault(); \
> + if (access_ok(__pu_addr, __pu_size)) \
> + __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
> + \
> + __pu_err; \
> })
>
> #define __put_user_nosleep(x, ptr, size) \
> ({ \
> long __pu_err; \
> __typeof__(*(ptr)) __user *__pu_addr = (ptr); \
> - __chk_user_ptr(ptr); \
> - __put_user_size((x), __pu_addr, (size), __pu_err); \
> + __typeof__(*(ptr)) __pu_val = (x); \
> + __typeof__(size) __pu_size = (size); \
> + \
> + __chk_user_ptr(__pu_addr); \
> + __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
> + \
> __pu_err; \
> })
>
> @@ -278,46 +290,55 @@ do { \
> #define __long_type(x) \
> __typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
>
> -#define __get_user_nocheck(x, ptr, size, do_allow) \
> +#define __get_user_nocheck(x, ptr, size, do_allow) \
> ({ \
> long __gu_err; \
> __long_type(*(ptr)) __gu_val; \
> - __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
> - __chk_user_ptr(ptr); \
> + __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
> + __typeof__(size) __gu_size = (size); \
> + \
> + __chk_user_ptr(__gu_addr); \
> if (!is_kernel_addr((unsigned long)__gu_addr)) \
> might_fault(); \
> barrier_nospec(); \
> - if (do_allow) \
> - __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
> - else \
> - __get_user_size_allowed(__gu_val, __gu_addr, (size), __gu_err); \
> + if (do_allow) \
> + __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
> + else \
> + __get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
> (x) = (__typeof__(*(ptr)))__gu_val; \
> + \
> __gu_err; \
> })
>
> -#define __get_user_check(x, ptr, size) \
> -({ \
> - long __gu_err = -EFAULT; \
> - __long_type(*(ptr)) __gu_val = 0; \
> +#define __get_user_check(x, ptr, size) \
> +({ \
> + long __gu_err = -EFAULT; \
> + __long_type(*(ptr)) __gu_val = 0; \
> __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
> - might_fault(); \
> - if (access_ok(__gu_addr, (size))) { \
> - barrier_nospec(); \
> - __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
> - } \
> - (x) = (__force __typeof__(*(ptr)))__gu_val; \
> - __gu_err; \
> + __typeof__(size) __gu_size = (size); \
> + \
> + might_fault(); \
> + if (access_ok(__gu_addr, __gu_size)) { \
> + barrier_nospec(); \
> + __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
> + } \
> + (x) = (__force __typeof__(*(ptr)))__gu_val; \
> + \
> + __gu_err; \
> })
>
> #define __get_user_nosleep(x, ptr, size) \
> ({ \
> long __gu_err; \
> __long_type(*(ptr)) __gu_val; \
> - __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
> - __chk_user_ptr(ptr); \
> + __typeof__(*(ptr)) __user *__gu_addr = (ptr); \
> + __typeof__(size) __gu_size = (size); \
> + \
> + __chk_user_ptr(__gu_addr); \
> barrier_nospec(); \
> - __get_user_size(__gu_val, __gu_addr, (size), __gu_err); \
> - (x) = (__force __typeof__(*(ptr)))__gu_val; \
> + __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
> + (x) = (__force __typeof__(*(ptr)))__gu_val; \
> + \
> __gu_err; \
> })
>
>
Christophe
^ permalink raw reply
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