* [PATCH 2/2] mm/vma: Introduce VM_ACCESS_FLAGS
From: Anshuman Khandual @ 2020-03-05 6:50 UTC (permalink / raw)
To: linux-mm
Cc: Catalin Marinas, Dave Hansen, Heiko Carstens, devel, linux-s390,
Yoshinori Sato, hughd, Russell King, Ley Foon Tan, vbabka,
Mark Salter, Anshuman Khandual, Rob Springer, Thomas Gleixner,
Guan Xuetao, linux-arm-kernel, Nick Hu, Greg Kroah-Hartman,
linux-kernel, Andrew Morton, linuxppc-dev
In-Reply-To: <1583391014-8170-1-git-send-email-anshuman.khandual@arm.com>
There are many places where all basic VMA access flags (read, write, exec)
are initialized or checked against as a group. One such example is during
page fault. Existing vma_is_accessible() wrapper already creates the notion
of VMA accessibility as a group access permissions. Hence lets just create
VM_ACCESS_FLAGS (VM_READ|VM_WRITE|VM_EXEC) which will not only reduce code
duplication but also extend the VMA accessibility concept in general.
Cc: Russell King <linux@armlinux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Mark Salter <msalter@redhat.com>
Cc: Nick Hu <nickhu@andestech.com>
CC: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Springer <rspringer@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/arm/mm/fault.c | 2 +-
arch/arm64/mm/fault.c | 2 +-
arch/nds32/mm/fault.c | 2 +-
arch/powerpc/mm/book3s64/pkeys.c | 2 +-
arch/s390/mm/fault.c | 2 +-
arch/unicore32/mm/fault.c | 2 +-
arch/x86/mm/pkeys.c | 2 +-
drivers/staging/gasket/gasket_core.c | 2 +-
include/linux/mm.h | 6 +++++-
mm/mmap.c | 2 +-
mm/mprotect.c | 4 ++--
11 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index bd0f4821f7e1..2c71028d9d6b 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -189,7 +189,7 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
*/
static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
{
- unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned int mask = VM_ACCESS_FLAGS;
if ((fsr & FSR_WRITE) && !(fsr & FSR_CM))
mask = VM_WRITE;
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f..63f31206a12e 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -445,7 +445,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
const struct fault_info *inf;
struct mm_struct *mm = current->mm;
vm_fault_t fault, major = 0;
- unsigned long vm_flags = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned long vm_flags = VM_ACCESS_FLAGS;
unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
if (kprobe_page_fault(regs, esr))
diff --git a/arch/nds32/mm/fault.c b/arch/nds32/mm/fault.c
index 906dfb25353c..55387a31bf42 100644
--- a/arch/nds32/mm/fault.c
+++ b/arch/nds32/mm/fault.c
@@ -79,7 +79,7 @@ void do_page_fault(unsigned long entry, unsigned long addr,
struct vm_area_struct *vma;
int si_code;
vm_fault_t fault;
- unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned int mask = VM_ACCESS_FLAGS;
unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
error_code = error_code & (ITYPE_mskINST | ITYPE_mskETYPE);
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 59e0ebbd8036..11fd52b24f68 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -315,7 +315,7 @@ int __execute_only_pkey(struct mm_struct *mm)
static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
{
/* Do this check first since the vm_flags should be hot */
- if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
+ if ((vma->vm_flags & VM_ACCESS_FLAGS) != VM_EXEC)
return false;
return (vma_pkey(vma) == vma->vm_mm->context.execute_only_pkey);
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 7b0bb475c166..b2cb3c0d0e1a 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -584,7 +584,7 @@ void do_dat_exception(struct pt_regs *regs)
int access;
vm_fault_t fault;
- access = VM_READ | VM_EXEC | VM_WRITE;
+ access = VM_ACCESS_FLAGS;
fault = do_exception(regs, access);
if (unlikely(fault))
do_fault_error(regs, access, fault);
diff --git a/arch/unicore32/mm/fault.c b/arch/unicore32/mm/fault.c
index 76342de9cf8c..fc27c274d358 100644
--- a/arch/unicore32/mm/fault.c
+++ b/arch/unicore32/mm/fault.c
@@ -149,7 +149,7 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
*/
static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
{
- unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+ unsigned int mask = VM_ACCESS_FLAGS;
if (!(fsr ^ 0x12)) /* write? */
mask = VM_WRITE;
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index c6f84c0b5d7a..8873ed1438a9 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -63,7 +63,7 @@ int __execute_only_pkey(struct mm_struct *mm)
static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
{
/* Do this check first since the vm_flags should be hot */
- if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
+ if ((vma->vm_flags & VM_ACCESS_FLAGS) != VM_EXEC)
return false;
if (vma_pkey(vma) != vma->vm_mm->context.execute_only_pkey)
return false;
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index be6b50f454b4..81bb7d58dc49 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -689,7 +689,7 @@ static bool gasket_mmap_has_permissions(struct gasket_dev *gasket_dev,
/* Make sure that no wrong flags are set. */
requested_permissions =
- (vma->vm_flags & (VM_WRITE | VM_READ | VM_EXEC));
+ (vma->vm_flags & VM_ACCESS_FLAGS);
if (requested_permissions & ~(bar_permissions)) {
dev_dbg(gasket_dev->dev,
"Attempting to map a region with requested permissions "
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 3861524368a4..e89512f1c170 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -368,6 +368,10 @@ extern unsigned int kobjsize(const void *objp);
#define VM_STACK_FLAGS (VM_STACK | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
+/* VMA basic access permission flags */
+#define VM_ACCESS_FLAGS (VM_READ | VM_WRITE | VM_EXEC)
+
+
/*
* Special vmas that are non-mergable, non-mlock()able.
* Note: mm/huge_memory.c VM_NO_THP depends on this definition.
@@ -557,7 +561,7 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
static inline bool vma_is_accessible(struct vm_area_struct *vma)
{
- return vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+ return vma->vm_flags & VM_ACCESS_FLAGS;
}
#ifdef CONFIG_SHMEM
diff --git a/mm/mmap.c b/mm/mmap.c
index 0d295f49b24d..57f74ade19a0 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1221,7 +1221,7 @@ static int anon_vma_compatible(struct vm_area_struct *a, struct vm_area_struct *
return a->vm_end == b->vm_start &&
mpol_equal(vma_policy(a), vma_policy(b)) &&
a->vm_file == b->vm_file &&
- !((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC|VM_SOFTDIRTY)) &&
+ !((a->vm_flags ^ b->vm_flags) & ~(VM_ACCESS_FLAGS | VM_SOFTDIRTY)) &&
b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
}
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 7a8e84f86831..8fbb7e7c08a5 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -359,7 +359,7 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
*/
if (arch_has_pfn_modify_check() &&
(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) &&
- (newflags & (VM_READ|VM_WRITE|VM_EXEC)) == 0) {
+ (newflags & VM_ACCESS_FLAGS) == 0) {
pgprot_t new_pgprot = vm_get_page_prot(newflags);
error = walk_page_range(current->mm, start, end,
@@ -538,7 +538,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
newflags |= (vma->vm_flags & ~mask_off_old_flags);
/* newflags >> 4 shift VM_MAY% in place of VM_% */
- if ((newflags & ~(newflags >> 4)) & (VM_READ | VM_WRITE | VM_EXEC)) {
+ if ((newflags & ~(newflags >> 4)) & VM_ACCESS_FLAGS) {
error = -EACCES;
goto out;
}
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] mm/vma: Define a default value for VM_DATA_DEFAULT_FLAGS
From: Anshuman Khandual @ 2020-03-05 6:50 UTC (permalink / raw)
To: linux-mm
Cc: Rich Felker, linux-ia64, linux-sh, Catalin Marinas,
Heiko Carstens, linux-mips, James E.J. Bottomley, Guo Ren,
linux-parisc, sparclinux, linux-hexagon, linux-riscv, Jonas Bonn,
linux-s390, linux-snps-arc, linux-c6x-dev, Brian Cain, hughd,
Russell King, Ley Foon Tan, Geert Uytterhoeven, linux-arm-kernel,
Mark Salter, Paul Burton, uclinux-h8-devel, linux-xtensa,
Anshuman Khandual, Jeff Dike, linux-um, linux-m68k, openrisc,
Paul Walmsley, Thomas Gleixner, Guan Xuetao, vbabka,
Richard Henderson, Chris Zankel, Michal Simek, Tony Luck,
Yoshinori Sato, Nick Hu, Vineet Gupta, linux-kernel, Ralf Baechle,
linux-alpha, nios2-dev, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <1583391014-8170-1-git-send-email-anshuman.khandual@arm.com>
There are many platforms with exact same value for VM_DATA_DEFAULT_FLAGS
This creates a default value for VM_DATA_DEFAULT_FLAGS in line with the
existing VM_STACK_DEFAULT_FLAGS. While here, also define some more macros
with standard VMA access flag combinations that are used frequently across
many platforms. Apart from simplification, this reduces code duplication
as well.
Cc: Richard Henderson <rth@twiddle.net>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Guo Ren <guoren@kernel.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Brian Cain <bcain@codeaurora.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Nick Hu <nickhu@andestech.com>
Cc: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: uclinux-h8-devel@lists.sourceforge.jp
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: nios2-dev@lists.rocketboards.org
Cc: openrisc@lists.librecores.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-mm@kvack.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
arch/alpha/include/asm/page.h | 3 ---
arch/arc/include/asm/page.h | 2 +-
arch/arm/include/asm/page.h | 4 +---
arch/arm64/include/asm/page.h | 4 +---
arch/c6x/include/asm/page.h | 5 +----
arch/csky/include/asm/page.h | 3 ---
arch/h8300/include/asm/page.h | 2 --
arch/hexagon/include/asm/page.h | 3 +--
arch/ia64/include/asm/page.h | 5 +----
arch/m68k/include/asm/page.h | 3 ---
arch/microblaze/include/asm/page.h | 2 --
arch/mips/include/asm/page.h | 5 +----
arch/nds32/include/asm/page.h | 3 ---
arch/nios2/include/asm/page.h | 3 +--
arch/openrisc/include/asm/page.h | 5 -----
arch/parisc/include/asm/page.h | 3 ---
arch/powerpc/include/asm/page.h | 9 ++-------
arch/powerpc/include/asm/page_64.h | 7 ++-----
arch/riscv/include/asm/page.h | 3 +--
arch/s390/include/asm/page.h | 3 +--
arch/sh/include/asm/page.h | 3 ---
arch/sparc/include/asm/page_32.h | 3 ---
arch/sparc/include/asm/page_64.h | 3 ---
arch/unicore32/include/asm/page.h | 3 ---
arch/x86/include/asm/page_types.h | 4 +---
arch/x86/um/asm/vm-flags.h | 10 ++--------
arch/xtensa/include/asm/page.h | 3 ---
include/linux/mm.h | 14 ++++++++++++++
28 files changed, 31 insertions(+), 89 deletions(-)
diff --git a/arch/alpha/include/asm/page.h b/arch/alpha/include/asm/page.h
index f3fb2848470a..e241bd88880f 100644
--- a/arch/alpha/include/asm/page.h
+++ b/arch/alpha/include/asm/page.h
@@ -90,9 +90,6 @@ typedef struct page *pgtable_t;
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
#endif /* CONFIG_DISCONTIGMEM */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/arc/include/asm/page.h b/arch/arc/include/asm/page.h
index 0a32e8cfd074..b0dfed0f12be 100644
--- a/arch/arc/include/asm/page.h
+++ b/arch/arc/include/asm/page.h
@@ -102,7 +102,7 @@ typedef pte_t * pgtable_t;
#define virt_addr_valid(kaddr) pfn_valid(virt_to_pfn(kaddr))
/* Default Permissions for stack/heaps pages (Non Executable) */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#define WANT_PAGE_VIRTUAL 1
diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
index c2b75cba26df..11b058a72a5b 100644
--- a/arch/arm/include/asm/page.h
+++ b/arch/arm/include/asm/page.h
@@ -161,9 +161,7 @@ extern int pfn_valid(unsigned long);
#endif /* !__ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS \
- (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#include <asm-generic/getorder.h>
diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index d39ddb258a04..cb4e1e6ca385 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -32,9 +32,7 @@ extern int pfn_valid(unsigned long);
#endif /* !__ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS \
- (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#include <asm-generic/getorder.h>
diff --git a/arch/c6x/include/asm/page.h b/arch/c6x/include/asm/page.h
index 70db1e7632bc..40079899084d 100644
--- a/arch/c6x/include/asm/page.h
+++ b/arch/c6x/include/asm/page.h
@@ -2,10 +2,7 @@
#ifndef _ASM_C6X_PAGE_H
#define _ASM_C6X_PAGE_H
-#define VM_DATA_DEFAULT_FLAGS \
- (VM_READ | VM_WRITE | \
- ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#include <asm-generic/page.h>
diff --git a/arch/csky/include/asm/page.h b/arch/csky/include/asm/page.h
index 9738eacefdc7..9b98bf31d57c 100644
--- a/arch/csky/include/asm/page.h
+++ b/arch/csky/include/asm/page.h
@@ -85,9 +85,6 @@ extern unsigned long va_pa_offset;
PHYS_OFFSET_OFFSET)
#define virt_to_page(x) (mem_map + MAP_NR(x))
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#define pfn_to_kaddr(x) __va(PFN_PHYS(x))
#include <asm-generic/memory_model.h>
diff --git a/arch/h8300/include/asm/page.h b/arch/h8300/include/asm/page.h
index 8da5124ad344..53e037544239 100644
--- a/arch/h8300/include/asm/page.h
+++ b/arch/h8300/include/asm/page.h
@@ -6,8 +6,6 @@
#include <linux/types.h>
#define MAP_NR(addr) (((uintptr_t)(addr)-PAGE_OFFSET) >> PAGE_SHIFT)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
#ifndef __ASSEMBLY__
extern unsigned long rom_length;
diff --git a/arch/hexagon/include/asm/page.h b/arch/hexagon/include/asm/page.h
index ee31f36f48f3..7cbf719c578e 100644
--- a/arch/hexagon/include/asm/page.h
+++ b/arch/hexagon/include/asm/page.h
@@ -93,8 +93,7 @@ struct page;
#define virt_to_page(kaddr) pfn_to_page(PFN_DOWN(__pa(kaddr)))
/* Default vm area behavior is non-executable. */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#define pfn_valid(pfn) ((pfn) < max_mapnr)
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
diff --git a/arch/ia64/include/asm/page.h b/arch/ia64/include/asm/page.h
index 5798bd2b462c..b69a5499d75b 100644
--- a/arch/ia64/include/asm/page.h
+++ b/arch/ia64/include/asm/page.h
@@ -218,10 +218,7 @@ get_order (unsigned long size)
#define PAGE_OFFSET RGN_BASE(RGN_KERNEL)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC | \
- (((current->personality & READ_IMPLIES_EXEC) != 0) \
- ? VM_EXEC : 0))
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#define GATE_ADDR RGN_BASE(RGN_GATE)
diff --git a/arch/m68k/include/asm/page.h b/arch/m68k/include/asm/page.h
index 05e1e1e77a9a..9d2ef0a51720 100644
--- a/arch/m68k/include/asm/page.h
+++ b/arch/m68k/include/asm/page.h
@@ -55,9 +55,6 @@ extern unsigned long _ramend;
#define __phys_to_pfn(paddr) ((unsigned long)((paddr) >> PAGE_SHIFT))
#define __pfn_to_phys(pfn) PFN_PHYS(pfn)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/getorder.h>
#endif /* _M68K_PAGE_H */
diff --git a/arch/microblaze/include/asm/page.h b/arch/microblaze/include/asm/page.h
index f4b44b24b02e..43bd560b7b67 100644
--- a/arch/microblaze/include/asm/page.h
+++ b/arch/microblaze/include/asm/page.h
@@ -197,8 +197,6 @@ extern int page_is_ram(unsigned long pfn);
#ifdef CONFIG_MMU
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
#endif /* CONFIG_MMU */
#endif /* __KERNEL__ */
diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index 0ba4ce6e2bf3..e2f503fc7a84 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -253,10 +253,7 @@ extern bool __virt_addr_valid(const volatile void *kaddr);
#define virt_addr_valid(kaddr) \
__virt_addr_valid((const volatile void *) (kaddr))
-#define VM_DATA_DEFAULT_FLAGS \
- (VM_READ | VM_WRITE | \
- ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/nds32/include/asm/page.h b/arch/nds32/include/asm/page.h
index 86b32014c5f9..add33a7f02c8 100644
--- a/arch/nds32/include/asm/page.h
+++ b/arch/nds32/include/asm/page.h
@@ -59,9 +59,6 @@ typedef struct page *pgtable_t;
#endif /* !__ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#endif /* __KERNEL__ */
#endif
diff --git a/arch/nios2/include/asm/page.h b/arch/nios2/include/asm/page.h
index 79fcac61f6ef..6a989819a7c1 100644
--- a/arch/nios2/include/asm/page.h
+++ b/arch/nios2/include/asm/page.h
@@ -98,8 +98,7 @@ static inline bool pfn_valid(unsigned long pfn)
# define virt_to_page(vaddr) pfn_to_page(PFN_DOWN(virt_to_phys(vaddr)))
# define virt_addr_valid(vaddr) pfn_valid(PFN_DOWN(virt_to_phys(vaddr)))
-# define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+# define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#include <asm-generic/memory_model.h>
diff --git a/arch/openrisc/include/asm/page.h b/arch/openrisc/include/asm/page.h
index 01069db59454..aab6e64d6db4 100644
--- a/arch/openrisc/include/asm/page.h
+++ b/arch/openrisc/include/asm/page.h
@@ -86,11 +86,6 @@ typedef struct page *pgtable_t;
#endif /* __ASSEMBLY__ */
-
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/parisc/include/asm/page.h b/arch/parisc/include/asm/page.h
index 796ae29e9b9a..6b3f6740a6a6 100644
--- a/arch/parisc/include/asm/page.h
+++ b/arch/parisc/include/asm/page.h
@@ -180,9 +180,6 @@ extern int npmem_ranges;
#define page_to_phys(page) (page_to_pfn(page) << PAGE_SHIFT)
#define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
#include <asm/pdc.h>
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 080a0bf8e54b..3ee8df0f66e0 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -240,13 +240,8 @@ static inline bool pfn_valid(unsigned long pfn)
* and needs to be executable. This means the whole heap ends
* up being executable.
*/
-#define VM_DATA_DEFAULT_FLAGS32 \
- (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
- VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
-#define VM_DATA_DEFAULT_FLAGS64 (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS32 VM_DATA_FLAGS_TSK_EXEC
+#define VM_DATA_DEFAULT_FLAGS64 VM_DATA_FLAGS_NON_EXEC
#ifdef __powerpc64__
#include <asm/page_64.h>
diff --git a/arch/powerpc/include/asm/page_64.h b/arch/powerpc/include/asm/page_64.h
index 5962797f784a..79a9b7c6a132 100644
--- a/arch/powerpc/include/asm/page_64.h
+++ b/arch/powerpc/include/asm/page_64.h
@@ -94,11 +94,8 @@ extern u64 ppc64_pft_size;
* stack by default, so in the absence of a PT_GNU_STACK program header
* we turn execute permission off.
*/
-#define VM_STACK_DEFAULT_FLAGS32 (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
-#define VM_STACK_DEFAULT_FLAGS64 (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_STACK_DEFAULT_FLAGS32 VM_DATA_FLAGS_EXEC
+#define VM_STACK_DEFAULT_FLAGS64 VM_DATA_FLAGS_NON_EXEC
#define VM_STACK_DEFAULT_FLAGS \
(is_32bit_task() ? \
diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 8ca1930caa44..2d50f76efe48 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -137,8 +137,7 @@ extern phys_addr_t __phys_addr_symbol(unsigned long x);
#define virt_addr_valid(vaddr) (pfn_valid(virt_to_pfn(vaddr)))
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
index 1019efd85b9d..089f49230966 100644
--- a/arch/s390/include/asm/page.h
+++ b/arch/s390/include/asm/page.h
@@ -176,8 +176,7 @@ static inline int devmem_is_allowed(unsigned long pfn)
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_NON_EXEC
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/sh/include/asm/page.h b/arch/sh/include/asm/page.h
index 5eef8be3e59f..ea8d68f58e39 100644
--- a/arch/sh/include/asm/page.h
+++ b/arch/sh/include/asm/page.h
@@ -182,9 +182,6 @@ typedef struct page *pgtable_t;
#endif
#define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/sparc/include/asm/page_32.h b/arch/sparc/include/asm/page_32.h
index b76d59edec8c..478260002836 100644
--- a/arch/sparc/include/asm/page_32.h
+++ b/arch/sparc/include/asm/page_32.h
@@ -133,9 +133,6 @@ extern unsigned long pfn_base;
#define pfn_valid(pfn) (((pfn) >= (pfn_base)) && (((pfn)-(pfn_base)) < max_mapnr))
#define virt_addr_valid(kaddr) ((((unsigned long)(kaddr)-PAGE_OFFSET)>>PAGE_SHIFT) < max_mapnr)
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/sparc/include/asm/page_64.h b/arch/sparc/include/asm/page_64.h
index e80f2d5bf62f..254dffd85fb1 100644
--- a/arch/sparc/include/asm/page_64.h
+++ b/arch/sparc/include/asm/page_64.h
@@ -158,9 +158,6 @@ extern unsigned long PAGE_OFFSET;
#endif /* !(__ASSEMBLY__) */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/getorder.h>
#endif /* _SPARC64_PAGE_H */
diff --git a/arch/unicore32/include/asm/page.h b/arch/unicore32/include/asm/page.h
index 8a89335673f9..96d6bdf180bd 100644
--- a/arch/unicore32/include/asm/page.h
+++ b/arch/unicore32/include/asm/page.h
@@ -69,9 +69,6 @@ extern int pfn_valid(unsigned long);
#endif /* !__ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS \
- (VM_READ | VM_WRITE | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/getorder.h>
#endif
diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
index c85e15010f48..e27aa6be6320 100644
--- a/arch/x86/include/asm/page_types.h
+++ b/arch/x86/include/asm/page_types.h
@@ -35,9 +35,7 @@
#define PAGE_OFFSET ((unsigned long)__PAGE_OFFSET)
-#define VM_DATA_DEFAULT_FLAGS \
- (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0 ) | \
- VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#define __PHYSICAL_START ALIGN(CONFIG_PHYSICAL_START, \
CONFIG_PHYSICAL_ALIGN)
diff --git a/arch/x86/um/asm/vm-flags.h b/arch/x86/um/asm/vm-flags.h
index 7c297e9e2413..df7a3896f5dd 100644
--- a/arch/x86/um/asm/vm-flags.h
+++ b/arch/x86/um/asm/vm-flags.h
@@ -9,17 +9,11 @@
#ifdef CONFIG_X86_32
-#define VM_DATA_DEFAULT_FLAGS \
- (VM_READ | VM_WRITE | \
- ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0 ) | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_TSK_EXEC
#else
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-#define VM_STACK_DEFAULT_FLAGS (VM_GROWSDOWN | VM_READ | VM_WRITE | \
- VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_STACK_DEFAULT_FLAGS (VM_GROWSDOWN | VM_DATA_FLAGS_EXEC)
#endif
#endif
diff --git a/arch/xtensa/include/asm/page.h b/arch/xtensa/include/asm/page.h
index f4771c29c7e9..37ce25ef92d6 100644
--- a/arch/xtensa/include/asm/page.h
+++ b/arch/xtensa/include/asm/page.h
@@ -203,8 +203,5 @@ static inline unsigned long ___pa(unsigned long va)
#endif /* __ASSEMBLY__ */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
- VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
#include <asm-generic/memory_model.h>
#endif /* _XTENSA_PAGE_H */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index b0e53ef13ff1..3861524368a4 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -342,6 +342,20 @@ extern unsigned int kobjsize(const void *objp);
/* Bits set in the VMA until the stack is in its final location */
#define VM_STACK_INCOMPLETE_SETUP (VM_RAND_READ | VM_SEQ_READ)
+#define TASK_EXEC ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0)
+
+/* Common data flag combinations */
+#define VM_DATA_FLAGS_TSK_EXEC (VM_READ | VM_WRITE | TASK_EXEC | \
+ VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_FLAGS_NON_EXEC (VM_READ | VM_WRITE | VM_MAYREAD | \
+ VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_FLAGS_EXEC (VM_READ | VM_WRITE | VM_EXEC | \
+ VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+
+#ifndef VM_DATA_DEFAULT_FLAGS /* arch can override this */
+#define VM_DATA_DEFAULT_FLAGS VM_DATA_FLAGS_EXEC
+#endif
+
#ifndef VM_STACK_DEFAULT_FLAGS /* arch can override this */
#define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
#endif
--
2.20.1
^ permalink raw reply related
* [PATCH 0/2] mm/vma: some new flags
From: Anshuman Khandual @ 2020-03-05 6:50 UTC (permalink / raw)
To: linux-mm
Cc: linux-ia64, linux-sh, linux-mips, sparclinux, linux-riscv,
linux-s390, linux-c6x-dev, linux-hexagon, hughd, vbabka,
linux-snps-arc, uclinux-h8-devel, linux-xtensa, Anshuman Khandual,
linux-um, linux-m68k, openrisc, linux-arm-kernel, linux-parisc,
linux-kernel, linux-alpha, nios2-dev, linuxppc-dev
The motivation here is to consolidate VMA flag combinations commonly used
across platforms and reduce code duplication while making it uncluttered
in general.
This first introduces a default VM_DATA_DEFAULT_FLAGS which platforms can
easily fall back on without requiring to define any similar data flag
combinations as they currently do. This also adds some more common data
flag combinations which are generally used when the platforms decide to
override the default.
The second patch consolidates VM_READ, VM_WRITE, VM_EXEC as VM_ACCESS_FLAGS
extending the existing VMA accessibility concept via vma_is_accessibility().
VM_ACCESS_FLAGS replaces many other instances which used check all three
VMA access flags simultaneously.
This series is based on v5.6-rc4 after applying these.
1. https://patchwork.kernel.org/cover/11399319/
2. https://patchwork.kernel.org/patch/11399379/
Changes in V1:
- Dropped the [PATCH 3/3] which was adding more vma_is_* wrappers
- Used VM_DATA_FLAGS_EXEC for VM_DATA_DEFAULT_FLAGS instead per Vlastimil
- Dropped init use cases for VM_ACCESS_FLAGS as suggested by Vlastimil
Changes in RFC: (https://patchwork.kernel.org/project/linux-mm/list/?series=249733)
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: uclinux-h8-devel@lists.sourceforge.jp
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: nios2-dev@lists.rocketboards.org
Cc: openrisc@lists.librecores.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-mm@kvack.org
Anshuman Khandual (2):
mm/vma: Define a default value for VM_DATA_DEFAULT_FLAGS
mm/vma: Introduce VM_ACCESS_FLAGS
arch/alpha/include/asm/page.h | 3 ---
arch/arc/include/asm/page.h | 2 +-
arch/arm/include/asm/page.h | 4 +---
arch/arm/mm/fault.c | 2 +-
arch/arm64/include/asm/page.h | 4 +---
arch/arm64/mm/fault.c | 2 +-
arch/c6x/include/asm/page.h | 5 +----
arch/csky/include/asm/page.h | 3 ---
arch/h8300/include/asm/page.h | 2 --
arch/hexagon/include/asm/page.h | 3 +--
arch/ia64/include/asm/page.h | 5 +----
arch/m68k/include/asm/page.h | 3 ---
arch/microblaze/include/asm/page.h | 2 --
arch/mips/include/asm/page.h | 5 +----
arch/nds32/include/asm/page.h | 3 ---
arch/nds32/mm/fault.c | 2 +-
arch/nios2/include/asm/page.h | 3 +--
arch/openrisc/include/asm/page.h | 5 -----
arch/parisc/include/asm/page.h | 3 ---
arch/powerpc/include/asm/page.h | 9 ++-------
arch/powerpc/include/asm/page_64.h | 7 ++-----
arch/powerpc/mm/book3s64/pkeys.c | 2 +-
arch/riscv/include/asm/page.h | 3 +--
arch/s390/include/asm/page.h | 3 +--
arch/s390/mm/fault.c | 2 +-
arch/sh/include/asm/page.h | 3 ---
arch/sparc/include/asm/page_32.h | 3 ---
arch/sparc/include/asm/page_64.h | 3 ---
arch/unicore32/include/asm/page.h | 3 ---
arch/unicore32/mm/fault.c | 2 +-
arch/x86/include/asm/page_types.h | 4 +---
arch/x86/mm/pkeys.c | 2 +-
arch/x86/um/asm/vm-flags.h | 10 ++--------
arch/xtensa/include/asm/page.h | 3 ---
drivers/staging/gasket/gasket_core.c | 2 +-
include/linux/mm.h | 20 +++++++++++++++++++-
mm/mmap.c | 2 +-
mm/mprotect.c | 4 ++--
38 files changed, 47 insertions(+), 101 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH V14] mm/debug: Add tests validating architecture page table helpers
From: Anshuman Khandual @ 2020-03-05 6:23 UTC (permalink / raw)
To: Christophe Leroy, Qian Cai
Cc: Heiko Carstens, Linux Memory Management List, Paul Mackerras,
H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
the arch/x86 maintainers, 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, LKML,
Palmer Dabbelt, Aneesh Kumar K.V, Andrew Morton, linuxppc-dev
In-Reply-To: <bcba7b7f-f351-4ee7-d74e-004a0bfbee47@c-s.fr>
On 03/05/2020 11:13 AM, Christophe Leroy wrote:
>
>
> Le 05/03/2020 à 01:54, Anshuman Khandual a écrit :
>>
>>
>> On 03/04/2020 04:59 PM, Qian Cai wrote:
>>>
>>>
>>>> On Mar 4, 2020, at 1:49 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>>>
>>>> AFAIU, you are not taking an interrupt here. You are stuck in the pte_update(), most likely due to nested locks. Try with LOCKDEP ?
>>>
>>> Not exactly sure what did you mean here, but the kernel has all lockdep enabled and did not flag anything here.
>>
>> As the patch has been dropped from Linux next (next-20200304) perhaps in
>> order to fold back the __pa_symbol() fix [1], so I am planning to respin
>> the original patch once more as V15 while adding Qian's signed off by for
>> the powerpc part. For now lets enable radix MMU ppc64 along with existing
>> ppc32. As PPC_RADIX_MMU depends on PPC_BOOK3S_64, the following change
>> should be good enough ?
>
> I don't think so, even if you have the Radix MMU compiled in, hash MMU is used when Radix is not available or disabled. So until the Hash MMU problem is fixed, you cannot enable it by default.
So this implies, that with DEBUG_VM given kernel compiled with Radix MMU will
get stuck in soft lock up when forced to use hash MMU in cases where Radix MMU
is either not available or is disabled. Hence, we cannot enable that.
I will still fold the changes from Qian without enabling ppc64 Radix MMU and
respin V15. These new changes dont hurt, build every where and works good
on arm64 and x86 platforms. More over we know that they also fix a problem
for ppc64 Radix MMU platforms. Hence unless there are some other concerns we
should fold them in.
>
> Christophe
>
^ permalink raw reply
* Re: [PATCH v2] powerpc/mm: Fix missing KUAP disable in flush_coherent_icache()
From: Michael Ellerman @ 2020-03-05 6:08 UTC (permalink / raw)
To: Andrew Donnellan, linuxppc-dev
In-Reply-To: <ab27d125-54e7-3c31-709e-ff5d28c7bfbd@linux.ibm.com>
Andrew Donnellan <ajd@linux.ibm.com> writes:
> On 4/3/20 10:57 am, Michael Ellerman wrote:
>> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
>> index ef7b1119b2e2..36a8c7b105ce 100644
>> --- a/arch/powerpc/mm/mem.c
>> +++ b/arch/powerpc/mm/mem.c
>> @@ -373,7 +373,9 @@ static inline bool flush_coherent_icache(unsigned long addr)
>> */
>> if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
>> mb(); /* sync */
>> + allow_read_from_user((void *)addr, L1_CACHE_BYTES);
>> icbi((void *)addr);
>> + prevent_read_from_user((void *)addr, L1_CACHE_BYTES);
>> mb(); /* sync */
>> isync();
>> return true;
>>
>
> (void *) should be (const void __user *) to avoid sparse warnings.
>
> snowpatch reports: https://patchwork.ozlabs.org/patch/1248671/
Thanks.
cheers
^ permalink raw reply
* Re: [PATCH -next] powerpc/mm/ptdump: fix an undefined behaviour
From: Christophe Leroy @ 2020-03-05 5:56 UTC (permalink / raw)
To: Qian Cai, mpe, akpm; +Cc: linux-mm, linuxppc-dev, rashmicy, linux-kernel
In-Reply-To: <20200305044759.1279-1-cai@lca.pw>
Le 05/03/2020 à 05:47, Qian Cai a écrit :
> Booting a power9 server with hash MMU could trigger an undefined
> behaviour because pud_offset(p4d, 0) will do,
>
> 0 >> (PAGE_SHIFT:16 + PTE_INDEX_SIZE:8 + H_PMD_INDEX_SIZE:10)
>
> UBSAN: shift-out-of-bounds in arch/powerpc/mm/ptdump/ptdump.c:282:15
> shift exponent 34 is too large for 32-bit type 'int'
> CPU: 6 PID: 1 Comm: swapper/0 Not tainted 5.6.0-rc4-next-20200303+ #13
> Call Trace:
> dump_stack+0xf4/0x164 (unreliable)
> ubsan_epilogue+0x18/0x78
> __ubsan_handle_shift_out_of_bounds+0x160/0x21c
> walk_pagetables+0x2cc/0x700
> walk_pud at arch/powerpc/mm/ptdump/ptdump.c:282
> (inlined by) walk_pagetables at arch/powerpc/mm/ptdump/ptdump.c:311
> ptdump_check_wx+0x8c/0xf0
> mark_rodata_ro+0x48/0x80
> kernel_init+0x74/0x194
> ret_from_kernel_thread+0x5c/0x74
>
> Fixes: 8eb07b187000 ("powerpc/mm: Dump linux pagetables")
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
>
> Notes for maintainers:
>
> This is on the top of the linux-next commit "powerpc: add support for
> folded p4d page tables" which is in the Andrew's tree.
>
> arch/powerpc/mm/ptdump/ptdump.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
> index 9d6256b61df3..b530f81398a7 100644
> --- a/arch/powerpc/mm/ptdump/ptdump.c
> +++ b/arch/powerpc/mm/ptdump/ptdump.c
> @@ -279,7 +279,7 @@ static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start)
>
> static void walk_pud(struct pg_state *st, p4d_t *p4d, unsigned long start)
> {
> - pud_t *pud = pud_offset(p4d, 0);
> + pud_t *pud = pud_offset(p4d, 0UL);
Is that the only place we have to do this ?
(In 5.6-rc) I see the same in:
/arch/powerpc/mm/ptdump/hashpagetable.c
/arch/powerpc/kvm/book3s_64_mmu_radix.c
Wouldn't it be better to:
- Either cast addr to unsigned long in pud_index() macro
- Or change pud_index() macro to a static inline function as x86 ?
Christophe
^ permalink raw reply
* [powerpc:next-test] BUILD SUCCESS 55ad7956ff2cfe2c733cea7cf87e9964c56d8c6e
From: kbuild test robot @ 2020-03-05 5:49 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: 55ad7956ff2cfe2c733cea7cf87e9964c56d8c6e cpufreq:powernv: Fix unsafe notifiers
elapsed time: 8449m
configs tested: 217
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
ia64 defconfig
powerpc defconfig
i386 allnoconfig
nds32 allnoconfig
h8300 edosk2674_defconfig
s390 allmodconfig
arc allyesconfig
openrisc or1ksim_defconfig
nios2 3c120_defconfig
powerpc allnoconfig
i386 alldefconfig
i386 allyesconfig
i386 defconfig
ia64 alldefconfig
ia64 allmodconfig
ia64 allnoconfig
ia64 allyesconfig
c6x allyesconfig
c6x evmc6678_defconfig
nios2 10m50_defconfig
openrisc simple_smp_defconfig
xtensa common_defconfig
xtensa iss_defconfig
nds32 defconfig
csky defconfig
alpha defconfig
h8300 h8300h-sim_defconfig
h8300 h8s-sim_defconfig
m68k allmodconfig
m68k m5475evb_defconfig
m68k multi_defconfig
m68k sun3_defconfig
arc defconfig
microblaze mmu_defconfig
microblaze nommu_defconfig
powerpc ppc64_defconfig
powerpc rhel-kconfig
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-20200228
x86_64 randconfig-a002-20200228
x86_64 randconfig-a003-20200228
i386 randconfig-a001-20200228
i386 randconfig-a002-20200228
i386 randconfig-a003-20200228
x86_64 randconfig-a001-20200229
x86_64 randconfig-a002-20200229
x86_64 randconfig-a003-20200229
i386 randconfig-a001-20200229
i386 randconfig-a002-20200229
i386 randconfig-a003-20200229
alpha randconfig-a001-20200228
m68k randconfig-a001-20200228
mips randconfig-a001-20200228
nds32 randconfig-a001-20200228
parisc randconfig-a001-20200228
riscv randconfig-a001-20200228
c6x randconfig-a001-20200228
h8300 randconfig-a001-20200228
microblaze randconfig-a001-20200228
nios2 randconfig-a001-20200228
sparc64 randconfig-a001-20200228
c6x randconfig-a001-20200229
h8300 randconfig-a001-20200229
microblaze randconfig-a001-20200229
nios2 randconfig-a001-20200229
sparc64 randconfig-a001-20200229
csky randconfig-a001-20200228
openrisc randconfig-a001-20200228
s390 randconfig-a001-20200228
sh randconfig-a001-20200228
xtensa randconfig-a001-20200228
x86_64 randconfig-b001-20200228
x86_64 randconfig-b002-20200228
x86_64 randconfig-b003-20200228
i386 randconfig-b001-20200228
i386 randconfig-b002-20200228
i386 randconfig-b003-20200228
x86_64 randconfig-b001-20200229
x86_64 randconfig-b002-20200229
x86_64 randconfig-b003-20200229
i386 randconfig-b001-20200229
i386 randconfig-b002-20200229
i386 randconfig-b003-20200229
x86_64 randconfig-c001-20200228
x86_64 randconfig-c002-20200228
x86_64 randconfig-c003-20200228
i386 randconfig-c001-20200228
i386 randconfig-c002-20200228
i386 randconfig-c003-20200228
x86_64 randconfig-c001-20200229
x86_64 randconfig-c002-20200229
x86_64 randconfig-c003-20200229
i386 randconfig-c001-20200229
i386 randconfig-c002-20200229
i386 randconfig-c003-20200229
x86_64 randconfig-d001-20200229
x86_64 randconfig-d002-20200229
x86_64 randconfig-d003-20200229
i386 randconfig-d001-20200229
i386 randconfig-d002-20200229
i386 randconfig-d003-20200229
x86_64 randconfig-d001-20200228
x86_64 randconfig-d002-20200228
x86_64 randconfig-d003-20200228
i386 randconfig-d001-20200228
i386 randconfig-d002-20200228
i386 randconfig-d003-20200228
x86_64 randconfig-e001-20200301
x86_64 randconfig-e002-20200301
x86_64 randconfig-e003-20200301
i386 randconfig-e001-20200301
i386 randconfig-e002-20200301
i386 randconfig-e003-20200301
x86_64 randconfig-e002-20200228
i386 randconfig-e001-20200228
x86_64 randconfig-e003-20200228
i386 randconfig-e002-20200228
x86_64 randconfig-e001-20200228
i386 randconfig-e003-20200228
x86_64 randconfig-f001-20200228
x86_64 randconfig-f002-20200228
x86_64 randconfig-f003-20200228
i386 randconfig-f001-20200228
i386 randconfig-f002-20200228
i386 randconfig-f003-20200228
x86_64 randconfig-f001-20200229
x86_64 randconfig-f002-20200229
x86_64 randconfig-f003-20200229
i386 randconfig-f001-20200229
i386 randconfig-f002-20200229
i386 randconfig-f003-20200229
x86_64 randconfig-g001-20200228
x86_64 randconfig-g002-20200228
x86_64 randconfig-g003-20200228
i386 randconfig-g001-20200228
i386 randconfig-g002-20200228
i386 randconfig-g003-20200228
x86_64 randconfig-h001-20200228
x86_64 randconfig-h002-20200228
x86_64 randconfig-h003-20200228
i386 randconfig-h001-20200228
i386 randconfig-h002-20200228
i386 randconfig-h003-20200228
x86_64 randconfig-h001-20200229
x86_64 randconfig-h002-20200229
x86_64 randconfig-h003-20200229
i386 randconfig-h001-20200229
i386 randconfig-h002-20200229
i386 randconfig-h003-20200229
arc randconfig-a001-20200228
arm randconfig-a001-20200228
arm64 randconfig-a001-20200228
ia64 randconfig-a001-20200228
powerpc randconfig-a001-20200228
sparc randconfig-a001-20200228
arc randconfig-a001-20200229
arm randconfig-a001-20200229
arm64 randconfig-a001-20200229
ia64 randconfig-a001-20200229
powerpc randconfig-a001-20200229
sparc randconfig-a001-20200229
riscv allmodconfig
riscv allnoconfig
riscv allyesconfig
riscv defconfig
riscv nommu_virt_defconfig
riscv rv32_defconfig
s390 alldefconfig
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 defconfig
um i386_defconfig
um x86_64_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 e3a1ab299346a9a415f334e91a78da7ea84aa5a2
From: kbuild test robot @ 2020-03-05 5:43 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: e3a1ab299346a9a415f334e91a78da7ea84aa5a2 Automatic merge of branches 'master', 'next' and 'fixes' into merge
elapsed time: 8443m
configs tested: 232
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
arc defconfig
riscv defconfig
i386 allyesconfig
h8300 edosk2674_defconfig
parisc generic-32bit_defconfig
openrisc or1ksim_defconfig
mips fuloong2e_defconfig
riscv rv32_defconfig
ia64 defconfig
riscv nommu_virt_defconfig
xtensa common_defconfig
ia64 alldefconfig
sh rsk7269_defconfig
sparc64 allyesconfig
um x86_64_defconfig
powerpc defconfig
microblaze nommu_defconfig
s390 zfcpdump_defconfig
mips allnoconfig
nds32 allnoconfig
s390 allmodconfig
mips allmodconfig
sh titan_defconfig
nios2 3c120_defconfig
mips malta_kvm_defconfig
ia64 allnoconfig
h8300 h8s-sim_defconfig
xtensa iss_defconfig
powerpc allnoconfig
openrisc simple_smp_defconfig
sh allnoconfig
i386 allnoconfig
s390 allnoconfig
parisc allnoconfig
i386 alldefconfig
i386 defconfig
ia64 allmodconfig
ia64 allyesconfig
c6x allyesconfig
c6x evmc6678_defconfig
nios2 10m50_defconfig
alpha defconfig
csky defconfig
nds32 defconfig
h8300 h8300h-sim_defconfig
m68k allmodconfig
m68k m5475evb_defconfig
m68k multi_defconfig
m68k sun3_defconfig
arc allyesconfig
microblaze mmu_defconfig
powerpc ppc64_defconfig
powerpc rhel-kconfig
mips 32r2_defconfig
mips 64r6el_defconfig
mips allyesconfig
parisc allyesconfig
parisc generic-64bit_defconfig
x86_64 randconfig-a001-20200228
x86_64 randconfig-a002-20200228
x86_64 randconfig-a003-20200228
i386 randconfig-a001-20200228
i386 randconfig-a002-20200228
i386 randconfig-a003-20200228
alpha randconfig-a001-20200228
m68k randconfig-a001-20200228
mips randconfig-a001-20200228
nds32 randconfig-a001-20200228
parisc randconfig-a001-20200228
riscv randconfig-a001-20200228
alpha randconfig-a001-20200229
m68k randconfig-a001-20200229
nds32 randconfig-a001-20200229
parisc randconfig-a001-20200229
riscv randconfig-a001-20200229
nios2 randconfig-a001-20200228
c6x randconfig-a001-20200228
sparc64 randconfig-a001-20200228
h8300 randconfig-a001-20200228
c6x randconfig-a001-20200229
h8300 randconfig-a001-20200229
microblaze randconfig-a001-20200229
nios2 randconfig-a001-20200229
sparc64 randconfig-a001-20200229
microblaze randconfig-a001-20200228
csky randconfig-a001-20200228
openrisc randconfig-a001-20200228
s390 randconfig-a001-20200228
sh randconfig-a001-20200228
xtensa randconfig-a001-20200228
csky randconfig-a001-20200229
openrisc randconfig-a001-20200229
s390 randconfig-a001-20200229
xtensa randconfig-a001-20200229
x86_64 randconfig-b001-20200228
x86_64 randconfig-b002-20200228
x86_64 randconfig-b003-20200228
i386 randconfig-b001-20200228
i386 randconfig-b002-20200228
i386 randconfig-b003-20200228
x86_64 randconfig-b001-20200229
x86_64 randconfig-b002-20200229
x86_64 randconfig-b003-20200229
i386 randconfig-b001-20200229
i386 randconfig-b002-20200229
i386 randconfig-b003-20200229
x86_64 randconfig-c001-20200228
x86_64 randconfig-c002-20200228
x86_64 randconfig-c003-20200228
i386 randconfig-c001-20200228
i386 randconfig-c002-20200228
i386 randconfig-c003-20200228
x86_64 randconfig-c001-20200229
x86_64 randconfig-c002-20200229
x86_64 randconfig-c003-20200229
i386 randconfig-c001-20200229
i386 randconfig-c002-20200229
i386 randconfig-c003-20200229
x86_64 randconfig-d001-20200229
x86_64 randconfig-d002-20200229
x86_64 randconfig-d003-20200229
i386 randconfig-d001-20200229
i386 randconfig-d002-20200229
i386 randconfig-d003-20200229
x86_64 randconfig-d001-20200228
x86_64 randconfig-d002-20200228
x86_64 randconfig-d003-20200228
i386 randconfig-d001-20200228
i386 randconfig-d002-20200228
i386 randconfig-d003-20200228
x86_64 randconfig-e001-20200228
x86_64 randconfig-e002-20200228
x86_64 randconfig-e003-20200228
i386 randconfig-e001-20200228
i386 randconfig-e002-20200228
i386 randconfig-e003-20200228
x86_64 randconfig-e001-20200229
x86_64 randconfig-e002-20200229
x86_64 randconfig-e003-20200229
i386 randconfig-e001-20200229
i386 randconfig-e002-20200229
i386 randconfig-e003-20200229
x86_64 randconfig-e001-20200301
x86_64 randconfig-e002-20200301
x86_64 randconfig-e003-20200301
i386 randconfig-e001-20200301
i386 randconfig-e002-20200301
i386 randconfig-e003-20200301
x86_64 randconfig-f001-20200228
x86_64 randconfig-f002-20200228
x86_64 randconfig-f003-20200228
i386 randconfig-f001-20200228
i386 randconfig-f002-20200228
i386 randconfig-f003-20200228
x86_64 randconfig-f001-20200229
x86_64 randconfig-f002-20200229
x86_64 randconfig-f003-20200229
i386 randconfig-f001-20200229
i386 randconfig-f002-20200229
i386 randconfig-f003-20200229
x86_64 randconfig-g001-20200228
x86_64 randconfig-g002-20200228
x86_64 randconfig-g003-20200228
i386 randconfig-g001-20200228
i386 randconfig-g002-20200228
i386 randconfig-g003-20200228
x86_64 randconfig-g001-20200229
x86_64 randconfig-g002-20200229
x86_64 randconfig-g003-20200229
i386 randconfig-g001-20200229
i386 randconfig-g002-20200229
i386 randconfig-g003-20200229
x86_64 randconfig-h001-20200228
x86_64 randconfig-h002-20200228
x86_64 randconfig-h003-20200228
i386 randconfig-h001-20200228
i386 randconfig-h002-20200228
i386 randconfig-h003-20200228
x86_64 randconfig-h001-20200229
x86_64 randconfig-h002-20200229
x86_64 randconfig-h003-20200229
i386 randconfig-h001-20200229
i386 randconfig-h002-20200229
i386 randconfig-h003-20200229
arc randconfig-a001-20200228
arm randconfig-a001-20200228
arm64 randconfig-a001-20200228
ia64 randconfig-a001-20200228
powerpc randconfig-a001-20200228
sparc randconfig-a001-20200228
arc randconfig-a001-20200229
arm randconfig-a001-20200229
arm64 randconfig-a001-20200229
ia64 randconfig-a001-20200229
powerpc randconfig-a001-20200229
sparc randconfig-a001-20200229
riscv allmodconfig
riscv allnoconfig
riscv allyesconfig
s390 alldefconfig
s390 allyesconfig
s390 debug_defconfig
s390 defconfig
sh allmodconfig
sh sh7785lcr_32bit_defconfig
sparc defconfig
sparc64 allmodconfig
sparc64 allnoconfig
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
* Re: [PATCH V14] mm/debug: Add tests validating architecture page table helpers
From: Christophe Leroy @ 2020-03-05 5:43 UTC (permalink / raw)
To: Anshuman Khandual, Qian Cai
Cc: Heiko Carstens, Linux Memory Management List, Paul Mackerras,
H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
the arch/x86 maintainers, 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, LKML,
Palmer Dabbelt, Aneesh Kumar K.V, Andrew Morton, linuxppc-dev
In-Reply-To: <57a3bc61-bbd5-e251-9621-7bc28f7901a1@arm.com>
Le 05/03/2020 à 01:54, Anshuman Khandual a écrit :
>
>
> On 03/04/2020 04:59 PM, Qian Cai wrote:
>>
>>
>>> On Mar 4, 2020, at 1:49 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>>
>>> AFAIU, you are not taking an interrupt here. You are stuck in the pte_update(), most likely due to nested locks. Try with LOCKDEP ?
>>
>> Not exactly sure what did you mean here, but the kernel has all lockdep enabled and did not flag anything here.
>
> As the patch has been dropped from Linux next (next-20200304) perhaps in
> order to fold back the __pa_symbol() fix [1], so I am planning to respin
> the original patch once more as V15 while adding Qian's signed off by for
> the powerpc part. For now lets enable radix MMU ppc64 along with existing
> ppc32. As PPC_RADIX_MMU depends on PPC_BOOK3S_64, the following change
> should be good enough ?
I don't think so, even if you have the Radix MMU compiled in, hash MMU
is used when Radix is not available or disabled. So until the Hash MMU
problem is fixed, you cannot enable it by default.
Christophe
^ permalink raw reply
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Ravi Bangoria @ 2020-03-05 5:06 UTC (permalink / raw)
To: Andi Kleen
Cc: mark.rutland, Ravi Bangoria, maddy, Peter Zijlstra, jolsa,
linux-kernel, eranian, adrian.hunter, alexander.shishkin, yao.jin,
mingo, paulus, acme, robert.richter, namhyung, kim.phillips,
linuxppc-dev, alexey.budankov, kan.liang
In-Reply-To: <20200303013329.GB1319864@tassilo.jf.intel.com>
Hi Andi,
Sorry for being bit late.
On 3/3/20 7:03 AM, Andi Kleen wrote:
> On Mon, Mar 02, 2020 at 11:13:32AM +0100, Peter Zijlstra wrote:
>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>> Modern processors export such hazard data in Performance
>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>> AMD[3] provides similar information.
>>>
>>> Implementation detail:
>>>
>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>> If it's set, kernel converts arch specific hazard information
>>> into generic format:
>>>
>>> struct perf_pipeline_haz_data {
>>> /* Instruction/Opcode type: Load, Store, Branch .... */
>>> __u8 itype;
>>> /* Instruction Cache source */
>>> __u8 icache;
>>> /* Instruction suffered hazard in pipeline stage */
>>> __u8 hazard_stage;
>>> /* Hazard reason */
>>> __u8 hazard_reason;
>>> /* Instruction suffered stall in pipeline stage */
>>> __u8 stall_stage;
>>> /* Stall reason */
>>> __u8 stall_reason;
>>> __u16 pad;
>>> };
>>
>> Kim, does this format indeed work for AMD IBS?
>
> Intel PEBS has a similar concept for annotation of memory accesses,
> which is already exported through perf_mem_data_src. This is essentially
> an extension. It would be better to have something unified here.
> Right now it seems to duplicate at least part of the PEBS facility.
IIUC there is a distinction from perf mem vs exposing the pipeline details.
perf-mem/perf_mem_data_src is more of memory accesses profiling. And proposal
here is to expose pipeline related details like stalls and latencies. Would
prefer/suggest not to extend the current structure further to capture pipeline
details.
Ravi
^ permalink raw reply
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Ravi Bangoria @ 2020-03-05 5:06 UTC (permalink / raw)
To: Paul Clarke
Cc: mark.rutland, Ravi Bangoria, ak, peterz, jolsa, alexey.budankov,
linux-kernel, acme, adrian.hunter, alexander.shishkin, yao.jin,
maddy, paulus, eranian, robert.richter, namhyung, kim.phillips,
linuxppc-dev, mingo, kan.liang
In-Reply-To: <91026159-1e83-6efd-c624-464b12b18b5c@us.ibm.com>
Hi Paul,
Sorry for bit late reply.
On 3/3/20 2:38 AM, Paul Clarke wrote:
> On 3/1/20 11:23 PM, Ravi Bangoria wrote:
>> Most modern microprocessors employ complex instruction execution
>> pipelines such that many instructions can be 'in flight' at any
>> given point in time. Various factors affect this pipeline and
>> hazards are the primary among them. Different types of hazards
>> exist - Data hazards, Structural hazards and Control hazards.
>> Data hazard is the case where data dependencies exist between
>> instructions in different stages in the pipeline. Structural
>> hazard is when the same processor hardware is needed by more
>> than one instruction in flight at the same time. Control hazards
>> are more the branch misprediction kinds.
>>
>> Information about these hazards are critical towards analyzing
>> performance issues and also to tune software to overcome such
>> issues. Modern processors export such hazard data in Performance
>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>> AMD[3] provides similar information.
>>
>> Implementation detail:
>>
>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>> If it's set, kernel converts arch specific hazard information
>> into generic format:
>>
>> struct perf_pipeline_haz_data {
>> /* Instruction/Opcode type: Load, Store, Branch .... */
>> __u8 itype;
>
> At the risk of bike-shedding (in an RFC, no less), "itype" doesn't convey enough meaning to me. "inst_type"? I see in 03/11, you use "perf_inst_type".
I was thinking to rename itype with operation_type or op_type. Because
AMD IBS and ARM SPE observes micro ops and also op_type is more aligned
to pipeline word.
>
>> /* Instruction Cache source */
>> __u8 icache;
>
> Possibly same here, and you use "perf_inst_cache" in 03/11.
Sure.
>
>> /* Instruction suffered hazard in pipeline stage */
>> __u8 hazard_stage;
>> /* Hazard reason */
>> __u8 hazard_reason;
>> /* Instruction suffered stall in pipeline stage */
>> __u8 stall_stage;
>> /* Stall reason */
>> __u8 stall_reason;
>> __u16 pad;
>> };
>>
>> ... which can be read by user from mmap() ring buffer. With this
>> approach, sample perf report in hazard mode looks like (On IBM
>> PowerPC):
>>
>> # ./perf record --hazard ./ebizzy
>> # ./perf report --hazard
>> Overhead Symbol Shared Instruction Type Hazard Stage Hazard Reason Stall Stage Stall Reason ICache access
>> 36.58% [.] thread_run ebizzy Load LSU Mispredict LSU Load fin L1 hit
>> 9.46% [.] thread_run ebizzy Load LSU Mispredict LSU Dcache_miss L1 hit
>> 1.76% [.] thread_run ebizzy Fixed point - - - - L1 hit
>> 1.31% [.] thread_run ebizzy Load LSU ERAT Miss LSU Load fin L1 hit
>> 1.27% [.] thread_run ebizzy Load LSU Mispredict - - L1 hit
>> 1.16% [.] thread_run ebizzy Fixed point - - FXU Fixed cycle L1 hit
>> 0.50% [.] thread_run ebizzy Fixed point ISU Source Unavailable FXU Fixed cycle L1 hit
>> 0.30% [.] thread_run ebizzy Load LSU LMQ Full, DERAT Miss LSU Load fin L1 hit
>> 0.24% [.] thread_run ebizzy Load LSU ERAT Miss - - L1 hit
>> 0.08% [.] thread_run ebizzy - - - BRU Fixed cycle L1 hit
>> 0.05% [.] thread_run ebizzy Branch - - BRU Fixed cycle L1 hit
>> 0.04% [.] thread_run ebizzy Fixed point ISU Source Unavailable - - L1 hit
>
> How are these to be interpreted? This is great information, but is it possible to make it more readable for non-experts?
For the RFC proposal we just pulled the details from the spec. But yes, will
look into this.
> If each of these map 1:1 with hardware events, should you emit the name of the event here, so that can be used to look up further information? For example, does the first line map to PM_CMPLU_STALL_LSU_FIN?
I'm using PM_MRK_INST_CMPL event in perf record an SIER provides all these
information.
> What was "Mispredict[ed]"? (Is it different from a branch misprediction?) And how does this relate to "L1 hit"?
I'm not 100% sure. I'll check with the hw folks about it.
> Can we emit "Load finish" instead of "Load fin" for easier reading? 03/11 also has "Marked fin before NTC".
> Nit: why does "Dcache_miss" have an underscore and none of the others?
Sure. Will change it.
>
>> Also perf annotate with hazard data:
>
>> │ static int
>> │ compare(const void *p1, const void *p2)
>> │ {
>> 33.23 │ std r31,-8(r1)
>> │ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
>> │ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
>> │ {haz_stage: LSU, haz_reason: Load Hit Store, stall_stage: LSU, stall_reason: -, icache: L3 hit}
>> │ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: -, stall_reason: -, icache: L1 hit}
>> │ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
>> │ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
>> 0.84 │ stdu r1,-64(r1)
>> │ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: -, stall_reason: -, icache: L1 hit}
>> 0.24 │ mr r31,r1
>> │ {haz_stage: -, haz_reason: -, stall_stage: -, stall_reason: -, icache: L1 hit}
>> 21.18 │ std r3,32(r31)
>> │ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
>> │ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
>> │ {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
>>
>>
>> Patches:
>> - Patch #1 is a simple cleanup patch
>> - Patch #2, #3, #4 implements generic and arch specific kernel
>> infrastructure
>> - Patch #5 enables perf record and script with hazard mode
>> - Patch #6, #7, #8 enables perf report with hazard mode
>> - Patch #9, #10, #11 enables perf annotate with hazard mode
>>
>> Note:
>> - This series is based on the talk by Madhavan in LPC 2018[4]. This is
>> just an early RFC to get comments about the approach and not intended
>> to be merged yet.
>> - I've prepared the series base on v5.6-rc3. But it depends on generic
>> perf annotate fixes [5][6] which are already merged by Arnaldo in
>> perf/urgent and perf/core.
>>
>> [1]: Book III, Section 9.4.10:
>> https://openpowerfoundation.org/?resource_lib=power-isa-version-3-0
>> [2]: https://wiki.raptorcs.com/w/images/6/6b/POWER9_PMU_UG_v12_28NOV2018_pub.pdf#G9.1106986
>
> This document is also available from the "IBM Portal for OpenPOWER" under the "All IBM Material for OpenPOWER" https://www-355.ibm.com/systems/power/openpower/tgcmDocumentRepository.xhtml?aliasId=OpenPOWER, under each of the individual modules. (Well hidden, it might be said, and not a simple link like you have here.)
Thanks for pointing it :)
Ravi
^ permalink raw reply
* [PATCH -next] powerpc/mm/ptdump: fix an undefined behaviour
From: Qian Cai @ 2020-03-05 4:47 UTC (permalink / raw)
To: mpe, akpm; +Cc: Qian Cai, linux-mm, linuxppc-dev, rashmicy, linux-kernel
Booting a power9 server with hash MMU could trigger an undefined
behaviour because pud_offset(p4d, 0) will do,
0 >> (PAGE_SHIFT:16 + PTE_INDEX_SIZE:8 + H_PMD_INDEX_SIZE:10)
UBSAN: shift-out-of-bounds in arch/powerpc/mm/ptdump/ptdump.c:282:15
shift exponent 34 is too large for 32-bit type 'int'
CPU: 6 PID: 1 Comm: swapper/0 Not tainted 5.6.0-rc4-next-20200303+ #13
Call Trace:
dump_stack+0xf4/0x164 (unreliable)
ubsan_epilogue+0x18/0x78
__ubsan_handle_shift_out_of_bounds+0x160/0x21c
walk_pagetables+0x2cc/0x700
walk_pud at arch/powerpc/mm/ptdump/ptdump.c:282
(inlined by) walk_pagetables at arch/powerpc/mm/ptdump/ptdump.c:311
ptdump_check_wx+0x8c/0xf0
mark_rodata_ro+0x48/0x80
kernel_init+0x74/0x194
ret_from_kernel_thread+0x5c/0x74
Fixes: 8eb07b187000 ("powerpc/mm: Dump linux pagetables")
Signed-off-by: Qian Cai <cai@lca.pw>
---
Notes for maintainers:
This is on the top of the linux-next commit "powerpc: add support for
folded p4d page tables" which is in the Andrew's tree.
arch/powerpc/mm/ptdump/ptdump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index 9d6256b61df3..b530f81398a7 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -279,7 +279,7 @@ static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start)
static void walk_pud(struct pg_state *st, p4d_t *p4d, unsigned long start)
{
- pud_t *pud = pud_offset(p4d, 0);
+ pud_t *pud = pud_offset(p4d, 0UL);
unsigned long addr;
unsigned int i;
--
2.21.0 (Apple Git-122.2)
^ permalink raw reply related
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Ravi Bangoria @ 2020-03-05 4:46 UTC (permalink / raw)
To: Kim Phillips
Cc: Mark Rutland, Andi Kleen, maddy, Peter Zijlstra, Jiri Olsa, LKML,
Stephane Eranian, Adrian Hunter, Alexander Shishkin, yao.jin,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Robert Richter, Namhyung Kim, linuxppc-dev, Alexey Budankov,
Liang, Kan
In-Reply-To: <df966d6e-8898-029f-e697-8496500a1663@amd.com>
Hi Kim,
Sorry about being bit late.
On 3/3/20 3:55 AM, Kim Phillips wrote:
> On 3/2/20 2:21 PM, Stephane Eranian wrote:
>> On Mon, Mar 2, 2020 at 2:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>>
>>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>>> Modern processors export such hazard data in Performance
>>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>>> AMD[3] provides similar information.
>>>>
>>>> Implementation detail:
>>>>
>>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>>> If it's set, kernel converts arch specific hazard information
>>>> into generic format:
>>>>
>>>> struct perf_pipeline_haz_data {
>>>> /* Instruction/Opcode type: Load, Store, Branch .... */
>>>> __u8 itype;
>>>> /* Instruction Cache source */
>>>> __u8 icache;
>>>> /* Instruction suffered hazard in pipeline stage */
>>>> __u8 hazard_stage;
>>>> /* Hazard reason */
>>>> __u8 hazard_reason;
>>>> /* Instruction suffered stall in pipeline stage */
>>>> __u8 stall_stage;
>>>> /* Stall reason */
>>>> __u8 stall_reason;
>>>> __u16 pad;
>>>> };
>>>
>>> Kim, does this format indeed work for AMD IBS?
>
> It's not really 1:1, we don't have these separations of stages
> and reasons, for example: we have missed in L2 cache, for example.
> So IBS output is flatter, with more cycle latency figures than
> IBM's AFAICT.
AMD IBS captures pipeline latency data incase Fetch sampling like the
Fetch latency, tag to retire latency, completion to retire latency and
so on. Yes, Ops sampling do provide more data on load/store centric
information. But it also captures more detailed data for Branch instructions.
And we also looked at ARM SPE, which also captures more details pipeline
data and latency information.
>
>> Personally, I don't like the term hazard. This is too IBM Power
>> specific. We need to find a better term, maybe stall or penalty.
>
> Right, IBS doesn't have a filter to only count stalled or otherwise
> bad events. IBS' PPR descriptions has one occurrence of the
> word stall, and no penalty. The way I read IBS is it's just
> reporting more sample data than just the precise IP: things like
> hits, misses, cycle latencies, addresses, types, etc., so words
> like 'extended', or the 'auxiliary' already used today even
> are more appropriate for IBS, although I'm the last person to
> bikeshed.
We are thinking of using "pipeline" word instead of Hazard.
>
>> Also worth considering is the support of ARM SPE (Statistical
>> Profiling Extension) which is their version of IBS.
>> Whatever gets added need to cover all three with no limitations.
>
> I thought Intel's various LBR, PEBS, and PT supported providing
> similar sample data in perf already, like with perf mem/c2c?
perf-mem is more of data centric in my opinion. It is more towards
memory profiling. So proposal here is to expose pipeline related
details like stalls and latencies.
Thanks for the review,
Ravi
^ permalink raw reply
* Re: [PATCH v3 17/27] powerpc/powernv/pmem: Implement the Read Error Log command
From: Alastair D'Silva @ 2020-03-05 4:31 UTC (permalink / raw)
To: Frederic Barrat
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <7767dec4-fb78-dd3e-3720-8d15f544639e@linux.ibm.com>
On Tue, 2020-03-03 at 11:36 +0100, Frederic Barrat wrote:
>
> Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > The read error log command extracts information from the
> > controller's
> > internal error log.
> >
> > This patch exposes this information in 2 ways:
> > - During probe, if an error occurs & a log is available, print it
> > to the
> > console
> > - After probe, make the error log available to userspace via an
> > IOCTL.
> > Userspace is notified of pending error logs in a later patch
> > ("powerpc/powernv/pmem: Forward events to userspace")
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > arch/powerpc/platforms/powernv/pmem/ocxl.c | 269
> > ++++++++++++++++++
> > .../platforms/powernv/pmem/ocxl_internal.h | 1 +
> > include/uapi/nvdimm/ocxl-pmem.h | 46 +++
> > 3 files changed, 316 insertions(+)
> > create mode 100644 include/uapi/nvdimm/ocxl-pmem.h
> >
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > index 63109a870d2c..2b64504f9129 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > @@ -447,10 +447,219 @@ static int file_release(struct inode *inode,
> > struct file *file)
> > return 0;
> > }
> >
> > +/**
> > + * error_log_header_parse() - Parse the first 64 bits of the error
> > log command response
> > + * @ocxlpmem: the device metadata
> > + * @length: out, returns the number of bytes in the response
> > (excluding the 64 bit header)
> > + */
> > +static int error_log_header_parse(struct ocxlpmem *ocxlpmem, u16
> > *length)
> > +{
> > + int rc;
> > + u64 val;
> > +
>
> Empty line in the middle of declarations
>
Ok
>
> > + u16 data_identifier;
> > + u32 data_length;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > + ocxlpmem-
> > >admin_command.data_offset,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + data_identifier = val >> 48;
> > + data_length = val & 0xFFFF;
> > +
> > + if (data_identifier != 0x454C) { // 'EL'
> > + dev_err(&ocxlpmem->dev,
> > + "Bad data identifier for error log data,
> > expected 'EL', got '%2s' (%#x), data_length=%u\n",
> > + (char *)&data_identifier,
> > + (unsigned int)data_identifier, data_length);
> > + return -EINVAL;
> > + }
> > +
> > + *length = data_length;
> > + return 0;
> > +}
> > +
> > +static int error_log_offset_0x08(struct ocxlpmem *ocxlpmem,
> > + u32 *log_identifier, u32
> > *program_ref_code)
> > +{
> > + int rc;
> > + u64 val;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > + ocxlpmem-
> > >admin_command.data_offset + 0x08,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + return rc;
> > +
> > + *log_identifier = val >> 32;
> > + *program_ref_code = val & 0xFFFFFFFF;
> > +
> > + return 0;
> > +}
> > +
> > +static int read_error_log(struct ocxlpmem *ocxlpmem,
> > + struct ioctl_ocxl_pmem_error_log *log, bool
> > buf_is_user)
> > +{
> > + u64 val;
> > + u16 user_buf_length;
> > + u16 buf_length;
> > + u16 i;
> > + int rc;
> > +
> > + if (log->buf_size % 8)
> > + return -EINVAL;
> > +
> > + rc = ocxlpmem_chi(ocxlpmem, &val);
> > + if (rc)
> > + goto out;
>
>
> "out" will unlock a mutex not yet taken.
>
Thanks, that should have been a return.
>
>
> > +
> > + if (!(val & GLOBAL_MMIO_CHI_ELA))
> > + return -EAGAIN;
> > +
> > + user_buf_length = log->buf_size;
> > +
> > + mutex_lock(&ocxlpmem->admin_command.lock);
> > +
> > + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_ERRLOG);
> > + if (rc)
> > + goto out;
> > +
> > + rc = admin_command_execute(ocxlpmem);
> > + if (rc)
> > + goto out;
> > +
> > + rc = admin_command_complete_timeout(ocxlpmem,
> > ADMIN_COMMAND_ERRLOG);
> > + if (rc < 0) {
> > + dev_warn(&ocxlpmem->dev, "Read error log timed out\n");
> > + goto out;
> > + }
> > +
> > + rc = admin_response(ocxlpmem);
> > + if (rc < 0)
> > + goto out;
> > + if (rc != STATUS_SUCCESS) {
> > + warn_status(ocxlpmem, "Unexpected status from retrieve
> > error log", rc);
> > + goto out;
> > + }
> > +
> > +
> > + rc = error_log_header_parse(ocxlpmem, &log->buf_size);
> > + if (rc)
> > + goto out;
> > + // log->buf_size now contains the returned buffer size, not the
> > user size
> > +
> > + rc = error_log_offset_0x08(ocxlpmem, &log->log_identifier,
> > + &log->program_reference_code);
> > + if (rc)
> > + goto out;
>
>
> Offset 0x08 gets a preferential treatment compared to 0x10 below and
> it's not clear why.
> I would create a subfonction which parses all the fields linearly.
>
I'll inline the contents of error_log_offset_0x08() - I can't see a big
benefit to factoring out the guts of that function.
>
>
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > + ocxlpmem-
> > >admin_command.data_offset + 0x10,
> > + OCXL_LITTLE_ENDIAN, &val);
> > + if (rc)
> > + goto out;
> > +
> > + log->error_log_type = val >> 56;
> > + log->action_flags = (log->error_log_type ==
> > OCXL_PMEM_ERROR_LOG_TYPE_GENERAL) ?
> > + (val >> 32) & 0xFFFFFF : 0;
> > + log->power_on_seconds = val & 0xFFFFFFFF;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > + ocxlpmem-
> > >admin_command.data_offset + 0x18,
> > + OCXL_LITTLE_ENDIAN, &log-
> > >timestamp);
> > + if (rc)
> > + goto out;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > + ocxlpmem-
> > >admin_command.data_offset + 0x20,
> > + OCXL_HOST_ENDIAN, &log->wwid[0]);
>
>
> A bit of a moot point, but is there a reason why some of those MMIO
> ops
> use OCXL_LITTLE_ENDIAN and the others OCXL_HOST_ENDIAN?
>
Some are little endian values, and some are binary data. WWIDs should
be LE though.
>
>
> > + if (rc)
> > + goto out;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > + ocxlpmem-
> > >admin_command.data_offset + 0x28,
> > + OCXL_HOST_ENDIAN, &log->wwid[1]);
> > + if (rc)
> > + goto out;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > + ocxlpmem-
> > >admin_command.data_offset + 0x30,
> > + OCXL_HOST_ENDIAN, (u64 *)log-
> > >fw_revision);
> > + if (rc)
> > + goto out;
> > + log->fw_revision[8] = '\0';
> > +
> > + buf_length = (user_buf_length < log->buf_size) ?
> > + user_buf_length : log->buf_size;
> > + for (i = 0; i < buf_length + 0x48; i += 8) {
> > + u64 val;
> > +
> > + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> > + ocxlpmem-
> > >admin_command.data_offset + i,
> > + OCXL_HOST_ENDIAN, &val);
> > + if (rc)
> > + goto out;
> > +
> > + if (buf_is_user) {
> > + if (copy_to_user(&log->buf[i], &val,
> > sizeof(u64))) {
> > + rc = -EFAULT;
> > + goto out;
> > + }
> > + } else
> > + log->buf[i] = val;
> > + }
>
>
> I think it could be a bit simplified by keeping the handling of the
> user
> buffer out of this function. Always call it with a kernel buffer.
> And
> have only one copy_to_user() call on the ioctl() path. You'd need to
> allocate a kernel buf on the ioctl path, but you're already doing it
> on
> the probe() path, so it should be doable to share code.
Hmm, the problem then is that on the IOCTL side, I'll have to save,
modify, then restore the buf member of struct
ioctl_ocxl_pmem_error_log, which would be uglier.
>
>
> > +
> > + rc = admin_response_handled(ocxlpmem);
> > + if (rc)
> > + goto out;
> > +
> > +out:
> > + mutex_unlock(&ocxlpmem->admin_command.lock);
> > + return rc;
> > +
> > +}
> > +
> > +static int ioctl_error_log(struct ocxlpmem *ocxlpmem,
> > + struct ioctl_ocxl_pmem_error_log __user *uarg)
> > +{
> > + struct ioctl_ocxl_pmem_error_log args;
> > + int rc;
> > +
> > + if (copy_from_user(&args, uarg, sizeof(args)))
> > + return -EFAULT;
> > +
> > + rc = read_error_log(ocxlpmem, &args, true);
> > + if (rc)
> > + return rc;
> > +
> > + if (copy_to_user(uarg, &args, sizeof(args)))
> > + return -EFAULT;
> > +
> > + return 0;
> > +}
> > +
> > +static long file_ioctl(struct file *file, unsigned int cmd,
> > unsigned long args)
> > +{
> > + struct ocxlpmem *ocxlpmem = file->private_data;
> > + int rc = -EINVAL;
> > +
> > + switch (cmd) {
> > + case IOCTL_OCXL_PMEM_ERROR_LOG:
> > + rc = ioctl_error_log(ocxlpmem,
> > + (struct ioctl_ocxl_pmem_error_log
> > __user *)args);
> > + break;
> > + }
> > + return rc;
> > +}
> > +
> > static const struct file_operations fops = {
> > .owner = THIS_MODULE,
> > .open = file_open,
> > .release = file_release,
> > + .unlocked_ioctl = file_ioctl,
> > + .compat_ioctl = file_ioctl,
> > };
> >
> > /**
> > @@ -527,6 +736,60 @@ static int read_device_metadata(struct
> > ocxlpmem *ocxlpmem)
> > return 0;
> > }
> >
> > +static const char *decode_error_log_type(u8 error_log_type)
> > +{
> > + switch (error_log_type) {
> > + case 0x00:
> > + return "general";
> > + case 0x01:
> > + return "predictive failure";
> > + case 0x02:
> > + return "thermal warning";
> > + case 0x03:
> > + return "data loss";
> > + case 0x04:
> > + return "health & performance";
> > + default:
> > + return "unknown";
> > + }
> > +}
> > +
> > +static void dump_error_log(struct ocxlpmem *ocxlpmem)
> > +{
> > + struct ioctl_ocxl_pmem_error_log log;
> > + u32 buf_size;
> > + u8 *buf;
> > + int rc;
> > +
> > + if (ocxlpmem->admin_command.data_size == 0)
> > + return;
> > +
> > + buf_size = ocxlpmem->admin_command.data_size - 0x48;
> > + buf = kzalloc(buf_size, GFP_KERNEL);
> > + if (!buf)
> > + return;
> > +
> > + log.buf = buf;
> > + log.buf_size = buf_size;
> > +
> > + rc = read_error_log(ocxlpmem, &log, false);
> > + if (rc < 0)
> > + goto out;
> > +
> > + dev_warn(&ocxlpmem->dev,
> > + "OCXL PMEM Error log: WWID=0x%016llx%016llx LID=0x%x
> > PRC=%x type=0x%x %s, Uptime=%u seconds timestamp=0x%llx\n",
> > + log.wwid[0], log.wwid[1],
> > + log.log_identifier, log.program_reference_code,
> > + log.error_log_type,
> > + decode_error_log_type(log.error_log_type),
> > + log.power_on_seconds, log.timestamp);
> > + print_hex_dump(KERN_WARNING, "buf", DUMP_PREFIX_OFFSET, 16, 1,
> > buf,
> > + log.buf_size, false);
>
> dev_warn already logs a warning. Isn't KERN_DEBUG more appropriate
> for
> the hex dump?
>
>
The hex dump is associated binary data for the warning, it doesn't
replicate the contents of the message.
>
> > +
> > +out:
> > + kfree(buf);
> > +}
> > +
> > /**
> > * probe_function0() - Set up function 0 for an OpenCAPI
> > persistent memory device
> > * This is important as it enables templates higher than 0 across
> > all other functions,
> > @@ -568,6 +831,7 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > struct ocxlpmem *ocxlpmem;
> > int rc;
> > u16 elapsed, timeout;
> > + u64 chi;
> >
> > if (PCI_FUNC(pdev->devfn) == 0)
> > return probe_function0(pdev);
> > @@ -667,6 +931,11 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > return 0;
> >
> > err:
> > + if (ocxlpmem &&
> > + (ocxlpmem_chi(ocxlpmem, &chi) == 0) &&
> > + (chi & GLOBAL_MMIO_CHI_ELA))
> > + dump_error_log(ocxlpmem);
> > +
> > /*
> > * Further cleanup is done in the release handler via
> > free_ocxlpmem()
> > * This allows us to keep the character device live to handle
> > IOCTLs to
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > index d2d81fec7bb1..b953ee522ed4 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > @@ -5,6 +5,7 @@
> > #include <linux/cdev.h>
> > #include <misc/ocxl.h>
> > #include <linux/libnvdimm.h>
> > +#include <uapi/nvdimm/ocxl-pmem.h>
>
> Can't we limit the extra include to ocxl.c?
>
Yes, there are no consumers referred to in ocxl_interal.[hc]
> Completely unrelated, but ocxl.c contains most of the code for this
> driver. We should consider renaming it to ocxlpmem.c or something
> along
> those lines, since it does a lot more than just interfacing with the
> opencapi interface. And would avoid confusion with an other already
> existing ocxl.c file.
>
Ok, my thinking was that it's already in a pmem directory, but I can
see arguments both ways.
>
> > #include <linux/mm.h>
> >
> > #define LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> > diff --git a/include/uapi/nvdimm/ocxl-pmem.h
> > b/include/uapi/nvdimm/ocxl-pmem.h
> > new file mode 100644
> > index 000000000000..b10f8ac0c20f
> > --- /dev/null
> > +++ b/include/uapi/nvdimm/ocxl-pmem.h
> > @@ -0,0 +1,46 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> > +/* Copyright 2017 IBM Corp. */
> > +#ifndef _UAPI_OCXL_SCM_H
> > +#define _UAPI_OCXL_SCM_H
> > +
> > +#include <linux/types.h>
> > +#include <linux/ioctl.h>
> > +
> > +#define OCXL_PMEM_ERROR_LOG_ACTION_RESET (1 << (32-32))
> > +#define OCXL_PMEM_ERROR_LOG_ACTION_CHKFW (1 << (53-32))
> > +#define OCXL_PMEM_ERROR_LOG_ACTION_REPLACE (1 << (54-32))
> > +#define OCXL_PMEM_ERROR_LOG_ACTION_DUMP (1 << (55-32))
> > +
> > +#define OCXL_PMEM_ERROR_LOG_TYPE_GENERAL (0x00)
> > +#define OCXL_PMEM_ERROR_LOG_TYPE_PREDICTIVE_FAILURE (0x01)
> > +#define OCXL_PMEM_ERROR_LOG_TYPE_THERMAL_WARNING (0x02)
> > +#define OCXL_PMEM_ERROR_LOG_TYPE_DATA_LOSS (0x03)
> > +#define OCXL_PMEM_ERROR_LOG_TYPE_HEALTH_PERFORMANCE (0x04)
> > +
> > +struct ioctl_ocxl_pmem_error_log {
> > + __u32 log_identifier; /* out */
> > + __u32 program_reference_code; /* out */
> > + __u32 action_flags; /* out, recommended course of action */
> > + __u32 power_on_seconds; /* out, Number of seconds the
> > controller has been on when the error occurred */
> > + __u64 timestamp; /* out, relative time since the current IPL */
> > + __u64 wwid[2]; /* out, the NAA formatted WWID associated with
> > the controller */
> > + char fw_revision[8+1]; /* out, firmware revision as null
> > terminated text */
>
> The 8+1 size will make the compiler add some padding here. Are we
> confident that all the compilers, at least on powerpc, will do the
> same
> thing and we can guarantee a kernel ABI? I would play it safe and
> have a
> discussion with folks who understand compilers better.
>
I'll add some explicit padding.
>
>
> > + __u16 buf_size; /* in/out, buffer size provided/required.
> > + * If required is greater than provided, the
> > buffer
> > + * will be truncated to the amount provided. If
> > its
> > + * less, then only the required bytes will be
> > populated.
> > + * If it is 0, then there are no more error log
> > entries.
> > + */
> > + __u8 error_log_type;
> > + __u8 reserved1;
> > + __u32 reserved2;
> > + __u64 reserved3[2];
> > + __u8 *buf; /* pointer to output buffer */
> > +};
> > +
> > +/* ioctl numbers */
> > +#define OCXL_PMEM_MAGIC 0x5C
>
> Randomly picked?
> See (and add entry in) Documentation/userspace-api/ioctl/ioctl-
> number.rst
>
Ok
>
> Fred
>
>
>
> > +/* SCM devices */
> > +#define IOCTL_OCXL_PMEM_ERROR_LOG _IOWR(OCXL_PMEM
> > _MAGIC, 0x01, struct ioctl_ocxl_pmem_error_log)
> > +
> > +#endif /* _UAPI_OCXL_SCM_H */
> >
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: maddy @ 2020-03-05 4:28 UTC (permalink / raw)
To: Stephane Eranian, Peter Zijlstra
Cc: Mark Rutland, Ravi Bangoria, Andi Kleen, Alexander Shishkin,
Jiri Olsa, LKML, Arnaldo Carvalho de Melo, Adrian Hunter,
Robert Richter, yao.jin, Ingo Molnar, Paul Mackerras,
Namhyung Kim, Phillips, Kim, linuxppc-dev, Alexey Budankov,
Liang, Kan
In-Reply-To: <CABPqkBSzwpR6p7UZs7g1vWGCJRLsh565mRMGc6m0Enn1SnkC4w@mail.gmail.com>
On 3/3/20 1:51 AM, Stephane Eranian wrote:
> On Mon, Mar 2, 2020 at 2:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>> Modern processors export such hazard data in Performance
>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>> AMD[3] provides similar information.
>>>
>>> Implementation detail:
>>>
>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>> If it's set, kernel converts arch specific hazard information
>>> into generic format:
>>>
>>> struct perf_pipeline_haz_data {
>>> /* Instruction/Opcode type: Load, Store, Branch .... */
>>> __u8 itype;
>>> /* Instruction Cache source */
>>> __u8 icache;
>>> /* Instruction suffered hazard in pipeline stage */
>>> __u8 hazard_stage;
>>> /* Hazard reason */
>>> __u8 hazard_reason;
>>> /* Instruction suffered stall in pipeline stage */
>>> __u8 stall_stage;
>>> /* Stall reason */
>>> __u8 stall_reason;
>>> __u16 pad;
>>> };
>> Kim, does this format indeed work for AMD IBS?
>
> Personally, I don't like the term hazard. This is too IBM Power
> specific. We need to find a better term, maybe stall or penalty.
Yes, names can be reworked and thinking more on it, how about these
as "pipeline" data instead of "hazard" data.
> Also worth considering is the support of ARM SPE (Statistical
> Profiling Extension) which is their version of IBS.
> Whatever gets added need to cover all three with no limitations.
Thanks for pointing this out. We looked at the ARM SPE spec and it does
provides information like issue latency, translation latency so on.
And AMD IBS provides data like fetch latency, tag to retire latency,
completion to retire latency and so on when using Fetch sampling.
So yes, will rework the struct definition to include data from ARM SPE
and AMD IBS also. Will post out a newer version soon.
Thanks for the comments
Maddy
^ permalink raw reply
* Re: [PATCHv3 1/2] powerpc/of: split out new_property() for reusing
From: Andrew Donnellan @ 2020-03-05 3:58 UTC (permalink / raw)
To: Pingfan Liu, linuxppc-dev
Cc: kexec, Rob Herring, Oliver O'Halloran, Aneesh Kumar K . V,
Paul Mackerras, Dan Williams, Frank Rowand, Hari Bathini
In-Reply-To: <1583311651-29310-2-git-send-email-kernelfans@gmail.com>
On 4/3/20 7:47 pm, Pingfan Liu wrote:
> Splitting out new_property() for coming reusing and moving it to
> of_helpers.c.
>
> Also do some coding style cleanup.
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> To: linuxppc-dev@lists.ozlabs.org
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Oliver O'Halloran <oohall@gmail.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Andrew Donnellan <ajd@linux.ibm.com>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: kexec@lists.infradead.org
Would this be useful to move into generic OF code?
Also if more functions are moving into of_helpers.c perhaps there should
be a common function name prefix.
Otherwise your style cleanup looks good.
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/powernv: Wire up OPAL address lookups
From: Nicholas Piggin @ 2020-03-05 3:43 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman; +Cc: linux-arch, skiboot, linux-kernel
In-Reply-To: <87v9nlr7dj.fsf@mpe.ellerman.id.au>
Michael Ellerman's on March 3, 2020 9:43 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Use ARCH_HAS_ADDRESS_LOOKUP to look up the opal symbol table. This
>> allows crashes and xmon debugging to print firmware symbols.
>>
>> Oops: System Reset, sig: 6 [#1]
>> LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
>> Modules linked in:
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc2-dirty #903
>> NIP: 0000000030020434 LR: 000000003000378c CTR: 0000000030020414
>> REGS: c0000000fffc3d70 TRAP: 0100 Not tainted (5.6.0-rc2-dirty)
>> MSR: 9000000002101002 <SF,HV,VEC,ME,RI> CR: 28022284 XER: 20040000
>> CFAR: 0000000030003788 IRQMASK: 3
>> GPR00: 000000003000378c 0000000031c13c90 0000000030136200 c0000000012cfa10
>> GPR04: c0000000012cfa10 0000000000000010 0000000000000000 0000000031c10060
>> GPR08: c0000000012cfaaf 0000000030003640 0000000000000000 0000000000000001
>> GPR12: 00000000300e0000 c000000001490000 0000000000000000 c00000000139c588
>> GPR16: 0000000031c10000 c00000000125a900 0000000000000000 c0000000012076a8
>> GPR20: c0000000012a3950 0000000000000001 0000000031c10060 c0000000012cfaaf
>> GPR24: 0000000000000019 0000000030003640 0000000000000000 0000000000000000
>> GPR28: 0000000000000010 c0000000012cfa10 0000000000000000 0000000000000000
>> NIP [0000000030020434] .dummy_console_write_buffer_space+0x20/0x64 [OPAL]
>> LR [000000003000378c] opal_entry+0x14c/0x17c [OPAL]
>>
>> This won't unwind the firmware stack (or its Linux caller) properly if
>> firmware and kernel endians don't match, but that problem could be solved
>> in powerpc's unwinder.
>
> How well does this work if we're tracing opal calls at the time we oops :)
>
> Though it looks like that's already fishy because we don't do anything
> to disable tracing of opal_console_write().
Yeah we don't do perfectly well in this case still. OPAL itself has
locks in its console paths and some issues with stack reentrancy.
We should do a bit better with cutting out more junk including tracing
from crash paths, so this doesn't fundamentally make things harder.
> I guess I'm a bit wary of adding numerous further opal calls in the oops
> path, I'm sure the opal symbol lookup code is bug free, but still.
There's a few, console write, event poll, reboot, and NMI IPI AFAIK,
so we have to make the opal call path itself robust (it's getting
there).
> Could we instead suck in the opal symbols early on, and search them in
> Linux? I suspect you've thought of that and rejected it, but it would be
> good to document why.
We could, I was thinking we might want OPAL to do something special
with them like add module annotations [OPAL] vs [HOMER] or whatever,
relocate itself after boot if we randomize where it's loaded etc.
but perhaps none of those things really prevent the symbols being
discovered at boot time. I don't know, it was easier? :)
>> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
>> index c1f25a760eb1..c3a2a797177a 100644
>> --- a/arch/powerpc/include/asm/opal-api.h
>> +++ b/arch/powerpc/include/asm/opal-api.h
>> @@ -214,7 +214,11 @@
>> #define OPAL_SECVAR_GET 176
>> #define OPAL_SECVAR_GET_NEXT 177
>> #define OPAL_SECVAR_ENQUEUE_UPDATE 178
>> -#define OPAL_LAST 178
>> +#define OPAL_PHB_SET_OPTION 179
>> +#define OPAL_PHB_GET_OPTION 180
>
> Only pull in the calls you need for this patch.
Ah okay I didn't realise that was the policy, makes sense.
Thanks,
Nick
^ permalink raw reply
* [Bug 206733] i2c i2c-3: i2c-powermac: modalias failure on /uni-n@f8000000/i2c@f8001000/cereal@1c0
From: bugzilla-daemon @ 2020-03-05 3:42 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206733-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206733
--- Comment #3 from Benjamin Herrenschmidt (benh@kernel.crashing.org) ---
Yes. I had a look (and had to swap in some historical brain cells from tape
storage :-) But I think the warning should probably be turned into a dev_dbg.
It's probably some gunk in the device-tree but those "cereal" nodes don't
matter (I don't remember what they represent but it's not something we care
about).
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v3 16/27] powerpc/powernv/pmem: Register a character device for userspace to interact with
From: Alastair D'Silva @ 2020-03-05 3:38 UTC (permalink / raw)
To: Frederic Barrat
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
Nicholas Piggin, Cédric Le Goater, Dan Williams,
Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
David S. Miller
In-Reply-To: <e9ebc395-9748-61a2-9125-eefc5c763332@linux.ibm.com>
On Tue, 2020-03-03 at 10:28 +0100, Frederic Barrat wrote:
>
> Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > This patch introduces a character device (/dev/ocxl-scmX) which
> > further
> > patches will use to interact with userspace.
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > arch/powerpc/platforms/powernv/pmem/ocxl.c | 116
> > +++++++++++++++++-
> > .../platforms/powernv/pmem/ocxl_internal.h | 2 +
> > 2 files changed, 116 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > index b8bd7e703b19..63109a870d2c 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > @@ -10,6 +10,7 @@
> > #include <misc/ocxl.h>
> > #include <linux/delay.h>
> > #include <linux/ndctl.h>
> > +#include <linux/fs.h>
> > #include <linux/mm_types.h>
> > #include <linux/memory_hotplug.h>
> > #include "ocxl_internal.h"
> > @@ -339,6 +340,9 @@ static void free_ocxlpmem(struct ocxlpmem
> > *ocxlpmem)
> >
> > free_minor(ocxlpmem);
> >
> > + if (ocxlpmem->cdev.owner)
> > + cdev_del(&ocxlpmem->cdev);
> > +
> > if (ocxlpmem->metadata_addr)
> > devm_memunmap(&ocxlpmem->dev, ocxlpmem->metadata_addr);
> >
> > @@ -396,6 +400,70 @@ static int ocxlpmem_register(struct ocxlpmem
> > *ocxlpmem)
> > return device_register(&ocxlpmem->dev);
> > }
> >
> > +static void ocxlpmem_put(struct ocxlpmem *ocxlpmem)
> > +{
> > + put_device(&ocxlpmem->dev);
> > +}
> > +
> > +static struct ocxlpmem *ocxlpmem_get(struct ocxlpmem *ocxlpmem)
> > +{
> > + return (get_device(&ocxlpmem->dev) == NULL) ? NULL : ocxlpmem;
> > +}
> > +
> > +static struct ocxlpmem *find_and_get_ocxlpmem(dev_t devno)
> > +{
> > + struct ocxlpmem *ocxlpmem;
> > + int minor = MINOR(devno);
> > + /*
> > + * We don't declare an RCU critical section here, as our AFU
> > + * is protected by a reference counter on the device. By the
> > time the
> > + * minor number of a device is removed from the idr, the ref
> > count of
> > + * the device is already at 0, so no user API will access that
> > AFU and
> > + * this function can't return it.
> > + */
>
> I fixed something related in the ocxl driver (which had enough
> changes
> with the introduction of the "info" device to make a similar comment
> become wrong). See commit a58d37bce0d21. The issue is handling a
> simultaneous open() and removal of the device through /sysfs as best
> we can.
>
> We are on a file open path and it's not like we're going to have a
> thousand clients, so performance is not that critical. We can take
> the
> mutex before searching in the IDR and release it after we increment
> the
> reference count on the device.
> But that's not enough: we could still find the device in the IDR
> while
> it is being removed in free_ocxlpmem(). I believe the only safe way
> to
> address it is by removing the user-facing APIs (the char device)
> before
> calling device_unregister(). So that it's not possible to find the
> device in file_open() if it's in the middle of being removed.
>
> Fred
>
>
Ok, I'll replicate that patch & follow your advice.
> > + ocxlpmem = idr_find(&minors_idr, minor);
> > + if (ocxlpmem)
> > + ocxlpmem_get(ocxlpmem);
> > + return ocxlpmem;
> > +}
> > +
> > +static int file_open(struct inode *inode, struct file *file)
> > +{
> > + struct ocxlpmem *ocxlpmem;
> > +
> > + ocxlpmem = find_and_get_ocxlpmem(inode->i_rdev);
> > + if (!ocxlpmem)
> > + return -ENODEV;
> > +
> > + file->private_data = ocxlpmem;
> > + return 0;
> > +}
> > +
> > +static int file_release(struct inode *inode, struct file *file)
> > +{
> > + struct ocxlpmem *ocxlpmem = file->private_data;
> > +
> > + ocxlpmem_put(ocxlpmem);
> > + return 0;
> > +}
> > +
> > +static const struct file_operations fops = {
> > + .owner = THIS_MODULE,
> > + .open = file_open,
> > + .release = file_release,
> > +};
> > +
> > +/**
> > + * create_cdev() - Create the chardev in /dev for the device
> > + * @ocxlpmem: the SCM metadata
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int create_cdev(struct ocxlpmem *ocxlpmem)
> > +{
> > + cdev_init(&ocxlpmem->cdev, &fops);
> > + return cdev_add(&ocxlpmem->cdev, ocxlpmem->dev.devt, 1);
> > +}
> > +
> > /**
> > * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
> > * @pdev: the PCI device information struct
> > @@ -572,6 +640,11 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> > goto err;
> > }
> >
> > + if (create_cdev(ocxlpmem)) {
> > + dev_err(&pdev->dev, "Could not create character
> > device\n");
> > + goto err;
> > + }
>
> As already mentioned in a previous patch, we branch to the err label
> so
> rc needs to be set to a valid error.
>
Ok
>
>
> > +
> > elapsed = 0;
> > timeout = ocxlpmem->readiness_timeout + ocxlpmem-
> > >memory_available_timeout;
> > while (!is_usable(ocxlpmem, false)) {
> > @@ -613,20 +686,59 @@ static struct pci_driver pci_driver = {
> > .shutdown = ocxlpmem_remove,
> > };
> >
> > +static int file_init(void)
> > +{
> > + int rc;
> > +
> > + mutex_init(&minors_idr_lock);
> > + idr_init(&minors_idr);
> > +
> > + rc = alloc_chrdev_region(&ocxlpmem_dev, 0, NUM_MINORS, "ocxl-
> > pmem");
> > + if (rc) {
> > + idr_destroy(&minors_idr);
> > + pr_err("Unable to allocate OpenCAPI persistent memory
> > major number: %d\n", rc);
> > + return rc;
> > + }
> > +
> > + ocxlpmem_class = class_create(THIS_MODULE, "ocxl-pmem");
> > + if (IS_ERR(ocxlpmem_class)) {
> > + idr_destroy(&minors_idr);
> > + pr_err("Unable to create ocxl-pmem class\n");
> > + unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> > + return PTR_ERR(ocxlpmem_class);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void file_exit(void)
> > +{
> > + class_destroy(ocxlpmem_class);
> > + unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> > + idr_destroy(&minors_idr);
> > +}
> > +
> > static int __init ocxlpmem_init(void)
> > {
> > - int rc = 0;
> > + int rc;
> >
> > - rc = pci_register_driver(&pci_driver);
> > + rc = file_init();
> > if (rc)
> > return rc;
> >
> > + rc = pci_register_driver(&pci_driver);
> > + if (rc) {
> > + file_exit();
> > + return rc;
> > + }
> > +
> > return 0;
> > }
> >
> > static void ocxlpmem_exit(void)
> > {
> > pci_unregister_driver(&pci_driver);
> > + file_exit();
> > }
> >
> > module_init(ocxlpmem_init);
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > index 28e2020f6355..d2d81fec7bb1 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> > @@ -2,6 +2,7 @@
> > // Copyright 2019 IBM Corp.
> >
> > #include <linux/pci.h>
> > +#include <linux/cdev.h>
> > #include <misc/ocxl.h>
> > #include <linux/libnvdimm.h>
> > #include <linux/mm.h>
> > @@ -99,6 +100,7 @@ struct ocxlpmem_function0 {
> > struct ocxlpmem {
> > struct device dev;
> > struct pci_dev *pdev;
> > + struct cdev cdev;
> > struct ocxl_fn *ocxl_fn;
> > struct nd_interleave_set nd_set;
> > struct nvdimm_bus_descriptor bus_desc;
> >
--
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819
^ permalink raw reply
* Re: [PATCH v3 24/27] powerpc/powernv/pmem: Expose SMART data via ndctl
From: Andrew Donnellan @ 2020-03-05 3:36 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-25-alastair@au1.ibm.com>
On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> +static int ndctl_smart(struct ocxlpmem *ocxlpmem, struct nd_cmd_pkg *pkg)
> +{
> + u32 length, i;
> + struct nd_ocxl_smart *out;
> + int rc;
> +
> + mutex_lock(&ocxlpmem->admin_command.lock);
> +
> + rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_SMART);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_execute(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = admin_command_complete_timeout(ocxlpmem, ADMIN_COMMAND_SMART);
> + if (rc < 0) {
> + dev_err(&ocxlpmem->dev, "SMART timeout\n");
> + goto out;
> + }
> +
> + rc = admin_response(ocxlpmem);
> + if (rc < 0)
> + goto out;
> + if (rc != STATUS_SUCCESS) {
> + warn_status(ocxlpmem, "Unexpected status from SMART", rc);
> + goto out;
> + }
> +
> + rc = smart_header_parse(ocxlpmem, &length);
> + if (rc)
> + goto out;
> +
> + pkg->nd_fw_size = length;
> +
> + length = min(length, pkg->nd_size_out); // bytes
> + out = (struct nd_ocxl_smart *)pkg->nd_payload;
> + // Each SMART attribute is 2 * 64 bits
> + out->count = length / (2 * sizeof(u64)); // attributes
From what I can tell - 8 bytes of nd_ocxl_smart are taken up for the
count + reserved bytes, so this is going to potentially overrun the user
buffer.
> +
> + for (i = 0; i < length; i += sizeof(u64)) {
It might be neater to make i count up by 1 and then multiply by
sizeof(u64) later.
> + rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> + ocxlpmem->admin_command.data_offset + sizeof(u64) + i,
+ 0x08 rather than + sizeof(u64) for consistency.
> + OCXL_LITTLE_ENDIAN,
> + &out->attribs[i/sizeof(u64)]);
> + if (rc)
> + goto out;
> + }
> +
> + rc = admin_response_handled(ocxlpmem);
> + if (rc)
> + goto out;
> +
> + rc = 0;
> + goto out;
> +
> +out:
> + mutex_unlock(&ocxlpmem->admin_command.lock);
> + return rc;
> +}
> +
> +static int ndctl_call(struct ocxlpmem *ocxlpmem, void *buf, unsigned int buf_len)
> +{
> + struct nd_cmd_pkg *pkg = buf;
> +
> + if (buf_len < sizeof(struct nd_cmd_pkg)) {
> + dev_err(&ocxlpmem->dev, "Invalid ND_CALL size=%u\n", buf_len);
> + return -EINVAL;
> + }
> +
> + if (pkg->nd_family != NVDIMM_FAMILY_OCXL) {
> + dev_err(&ocxlpmem->dev, "Invalid ND_CALL family=0x%llx\n", pkg->nd_family);
> + return -EINVAL;
> + }
> +
> + switch (pkg->nd_command) {
> + case ND_CMD_OCXL_SMART:
> + ndctl_smart(ocxlpmem, pkg);
Did you intend to dispose of the return code here?
> + break;
> +
> + default:
> + dev_err(&ocxlpmem->dev, "Invalid ND_CALL command=0x%llx\n", pkg->nd_command);
> + return -EINVAL;
> + }
> +
> +
> + return 0;
> +}
> +
> static int ndctl(struct nvdimm_bus_descriptor *nd_desc,
> struct nvdimm *nvdimm,
> unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
> @@ -88,6 +211,10 @@ static int ndctl(struct nvdimm_bus_descriptor *nd_desc,
> struct ocxlpmem *ocxlpmem = container_of(nd_desc, struct ocxlpmem, bus_desc);
>
> switch (cmd) {
> + case ND_CMD_CALL:
> + *cmd_rc = ndctl_call(ocxlpmem, buf, buf_len);
> + return 0;
> +
> case ND_CMD_GET_CONFIG_SIZE:
> *cmd_rc = ndctl_config_size(buf);
> return 0;
> @@ -171,6 +298,7 @@ static int register_lpc_mem(struct ocxlpmem *ocxlpmem)
> set_bit(ND_CMD_GET_CONFIG_SIZE, &nvdimm_cmd_mask);
> set_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm_cmd_mask);
> set_bit(ND_CMD_SET_CONFIG_DATA, &nvdimm_cmd_mask);
> + set_bit(ND_CMD_CALL, &nvdimm_cmd_mask);
>
> set_bit(NDD_ALIASING, &nvdimm_flags);
>
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> index 927690f4888f..0eb7a35d24ae 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> @@ -7,6 +7,7 @@
> #include <linux/libnvdimm.h>
> #include <uapi/nvdimm/ocxl-pmem.h>
> #include <linux/mm.h>
> +#include <linux/ndctl.h>
>
> #define LABEL_AREA_SIZE (1UL << PA_SECTION_SHIFT)
> #define DEFAULT_TIMEOUT 100
> @@ -98,6 +99,23 @@ struct ocxlpmem_function0 {
> struct ocxl_fn *ocxl_fn;
> };
>
> +struct nd_ocxl_smart {
> + __u8 count;
> + __u8 reserved[7];
> + __u64 attribs[0];
> +} __packed;
> +
> +struct nd_pkg_ocxl {
> + struct nd_cmd_pkg gen;
> + union {
> + struct nd_ocxl_smart smart;
> + };
> +};
> +
> +enum nd_cmd_ocxl {
> + ND_CMD_OCXL_SMART = 1,
> +};
> +
> struct ocxlpmem {
> struct device dev;
> struct pci_dev *pdev;
> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
> index de5d90212409..2885052e7f40 100644
> --- a/include/uapi/linux/ndctl.h
> +++ b/include/uapi/linux/ndctl.h
> @@ -244,6 +244,7 @@ struct nd_cmd_pkg {
> #define NVDIMM_FAMILY_HPE2 2
> #define NVDIMM_FAMILY_MSFT 3
> #define NVDIMM_FAMILY_HYPERV 4
> +#define NVDIMM_FAMILY_OCXL 6
>
> #define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
> struct nd_cmd_pkg)
>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] powerpc/64: BE option to use ELFv2 ABI for big endian kernels
From: Nicholas Piggin @ 2020-03-05 3:34 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <20200303230950.GM22482@gate.crashing.org>
Segher Boessenkool's on March 4, 2020 9:09 am:
> Hi!
>
> On Tue, Mar 03, 2020 at 11:45:27AM +1000, Nicholas Piggin wrote:
>> Provide an option to use ELFv2 ABI for big endian builds. This works on
>> GCC and clang (since 2014). it is is not officially supported by the GNU
>> toolchain, but it can give some useful advantages of the ELFv2 ABI for
>> BE (e.g., less stack usage). Some distros build BE ELFv2 userspace.
>
> It is not officially supported in the sense that a) as a host config,
> it does not exist *at all* (this isn't relevant for the kernel, it does
> not use a libc or other libraries, of course); and b) as a target config,
> it is not supported in the sense that no one tests it, so we cannot say
> anything about what quality code it generates, if it works at all, etc.
Yep.
> But we *do* allow "-mbig -mabi=elfv2", it's just a chicken-and-egg
> problem to have this properly tested. If someone would regularly test
> it (incl. sending the test results to gcc-testresults@), I don't see why
> it would not become a supported platform.
>
>> +override flavour := linux-ppc64v2
>
> That isn't a good name, heh. This isn't "v2" of anything... Spell out
> the name "ELFv2"? Or as "elfv2"? It is just a name after all, it is
> version 1 in all three version fields in the ELF headers!
Yeah okay. This part is only for some weird little perl asm generator
script, but probably better to be careful. linux-ppc64-elfv2 ?
>
>
> Anyway, looks like it will work, let's see where this goes :-)
>
> Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
>
>
> Segher
>
^ permalink raw reply
* Re: [PATCH v2] ima: add a new CONFIG for loading arch-specific policies
From: Michael Ellerman @ 2020-03-05 3:26 UTC (permalink / raw)
To: James Bottomley, Mimi Zohar, Nayna Jain, linux-integrity,
linuxppc-dev, linux-efi, linux-s390
Cc: Philipp Rudo, Ard Biesheuvel, linux-kernel
In-Reply-To: <1583336133.3284.1.camel@HansenPartnership.com>
James Bottomley <James.Bottomley@HansenPartnership.com> writes:
> On Wed, 2020-03-04 at 07:35 -0500, Mimi Zohar wrote:
>> On Tue, 2020-03-03 at 23:43 -0800, James Bottomley wrote:
>> > On Tue, 2020-03-03 at 21:33 -0500, Nayna Jain wrote:
>> > > diff --git a/security/integrity/ima/Kconfig
>> > > b/security/integrity/ima/Kconfig
>> > > index 3f3ee4e2eb0d..d17972aa413a 100644
>> > > --- a/security/integrity/ima/Kconfig
>> > > +++ b/security/integrity/ima/Kconfig
>> > > @@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
>> > > depends on IMA_MEASURE_ASYMMETRIC_KEYS
>> > > depends on SYSTEM_TRUSTED_KEYRING
>> > > default y
>> > > +
>> > > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
>> > > + bool
>> > > + depends on IMA
>> > > + depends on IMA_ARCH_POLICY
>> > > + default n
>> >
>> > You can't do this: a symbol designed to be selected can't depend on
>> > other symbols because Kconfig doesn't see the dependencies during
>> > select. We even have a doc for this now:
>> >
>> > Documentation/kbuild/Kconfig.select-break
>>
>> The document is discussing a circular dependency, where C selects B.
>> IMA_SECURE_AND_OR_TRUSTED_BOOT is not selecting anything, but is
>> being selected. All of the Kconfig's are now dependent on
>> IMA_ARCH_POLICY being enabled before selecting
>> IMA_SECURE_AND_OR_TRUSTED_BOOT.
>>
>> As Ard pointed out, both IMA and IMA_ARCH_POLICY are not needed, as
>> IMA_ARCH_POLICY is already dependent on IMA.
>
> Then removing them is fine, if they're not necessary ... you just can't
> select a symbol with dependencies because the two Kconfig mechanisms
> don't mix.
You can safely select something if the selector has the same or stricter
set of dependencies than the selectee. And in this case that's true.
config IMA_SECURE_AND_OR_TRUSTED_BOOT
bool
depends on IMA
depends on IMA_ARCH_POLICY
powerpc:
depends on IMA_ARCH_POLICY
select IMA_SECURE_AND_OR_TRUSTED_BOOT
s390: select IMA_SECURE_AND_OR_TRUSTED_BOOT if IMA_ARCH_POLICY
x86: select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI && IMA_ARCH_POLICY
But that's not to say it's the best solution, because you have to ensure
the arch code has the right set of dependencies.
I think this is actually a perfect case for using imply. We want the
arch code to indicate it wants IMA_SECURE_..., but only if all the IMA
related dependencies are met.
I think the patch below should work.
For example:
$ grep PPC_SECURE_BOOT .config
CONFIG_PPC_SECURE_BOOT=y
$ ./scripts/config -d CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
$ grep IMA_SECURE .config
# CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT is not set
$ make oldconfig
scripts/kconfig/conf --oldconfig Kconfig
#
# configuration written to .config
#
$ grep IMA_SECURE .config
CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT=y
$ ./scripts/config -d CONFIG_IMA_ARCH_POLICY
$ grep -e IMA_ARCH_POLICY -e IMA_SECURE .config
# CONFIG_IMA_ARCH_POLICY is not set
CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT=y
$ make olddefconfig
scripts/kconfig/conf --olddefconfig Kconfig
#
# configuration written to .config
#
$ grep -e IMA_ARCH_POLICY -e IMA_SECURE .config
# CONFIG_IMA_ARCH_POLICY is not set
$
cheers
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..5b9f1cba2a44 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -979,6 +979,7 @@ config PPC_SECURE_BOOT
bool
depends on PPC_POWERNV
depends on IMA_ARCH_POLICY
+ imply IMA_SECURE_AND_OR_TRUSTED_BOOT
help
Systems with firmware secure boot enabled need to define security
policies to extend secure boot to the OS. This config allows a user
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 8abe77536d9d..59c216af6264 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -195,6 +195,7 @@ config S390
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
select SWIOTLB
select GENERIC_ALLOCATOR
+ imply IMA_SECURE_AND_OR_TRUSTED_BOOT
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index beea77046f9b..92204a486d97 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -230,6 +230,7 @@ config X86
select VIRT_TO_BUS
select X86_FEATURE_NAMES if PROC_FS
select PROC_PID_ARCH_STATUS if PROC_FS
+ imply IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI
config INSTRUCTION_DECODER
def_bool y
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 1659217e9b60..aefe758f4466 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size);
extern void ima_add_kexec_buffer(struct kimage *image);
#endif
-#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
- || defined(CONFIG_PPC_SECURE_BOOT)
+#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
#else
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 3f3ee4e2eb0d..5ba4ae040fd8 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -327,3 +327,10 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
depends on IMA_MEASURE_ASYMMETRIC_KEYS
depends on SYSTEM_TRUSTED_KEYRING
default y
+
+config IMA_SECURE_AND_OR_TRUSTED_BOOT
+ bool
+ depends on IMA_ARCH_POLICY
+ help
+ This option is selected by architectures to enable secure and/or
+ trusted boot based on IMA runtime policies.
^ permalink raw reply related
* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Jason Yan @ 2020-03-05 3:22 UTC (permalink / raw)
To: Scott Wood, Daniel Axtens, mpe, linuxppc-dev, diana.craciun,
christophe.leroy, benh, paulus, npiggin, keescook,
kernel-hardening
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <d673649d6f371e82d4a94ba62bfd0d44efeca7b4.camel@buserror.net>
在 2020/3/5 5:21, Scott Wood 写道:
> On Wed, 2020-02-26 at 18:16 +1100, Daniel Axtens wrote:
>> Hi Jason,
>>
>>> This is a try to implement KASLR for Freescale BookE64 which is based on
>>> my earlier implementation for Freescale BookE32:
>>> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=131718
>>>
>>> The implementation for Freescale BookE64 is similar as BookE32. One
>>> difference is that Freescale BookE64 set up a TLB mapping of 1G during
>>> booting. Another difference is that ppc64 needs the kernel to be
>>> 64K-aligned. So we can randomize the kernel in this 1G mapping and make
>>> it 64K-aligned. This can save some code to creat another TLB map at
>>> early boot. The disadvantage is that we only have about 1G/64K = 16384
>>> slots to put the kernel in.
>>>
>>> KERNELBASE
>>>
>>> 64K |--> kernel <--|
>>> | | |
>>> +--+--+--+ +--+--+--+--+--+--+--+--+--+ +--+--+
>>> | | | |....| | | | | | | | | |....| | |
>>> +--+--+--+ +--+--+--+--+--+--+--+--+--+ +--+--+
>>> | | 1G
>>> |-----> offset <-----|
>>>
>>> kernstart_virt_addr
>>>
>>> I'm not sure if the slot numbers is enough or the design has any
>>> defects. If you have some better ideas, I would be happy to hear that.
>>>
>>> Thank you all.
>>>
>>
>> Are you making any attempt to hide kernel address leaks in this series?
>> I've just been looking at the stackdump code just now, and it directly
>> prints link registers and stack pointers, which is probably enough to
>> determine the kernel base address:
>>
>> SPs: LRs: %pS pointer
>> [ 0.424506] [c0000000de403970] [c000000001fc0458] dump_stack+0xfc/0x154
>> (unreliable)
>> [ 0.424593] [c0000000de4039c0] [c000000000267eec] panic+0x258/0x5ac
>> [ 0.424659] [c0000000de403a60] [c0000000024d7a00]
>> mount_block_root+0x634/0x7c0
>> [ 0.424734] [c0000000de403be0] [c0000000024d8100]
>> prepare_namespace+0x1ec/0x23c
>> [ 0.424811] [c0000000de403c60] [c0000000024d7010]
>> kernel_init_freeable+0x804/0x880
>>
>> git grep \\\"REG\\\" arch/powerpc shows a few other uses like this, all
>> in process.c or in xmon.
>>
>> Maybe replacing the REG format string in KASLR mode would be sufficient?
>
> Whatever we decide to do here, it's not book3e-specific so it should be
> considered separately from these patches.
>
OK, I will continue to work with this series.
> -Scott
>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH v3 5/6] powerpc/fsl_booke/64: clear the original kernel if randomized
From: Jason Yan @ 2020-03-05 3:20 UTC (permalink / raw)
To: Scott Wood, mpe, linuxppc-dev, diana.craciun, christophe.leroy,
benh, paulus, npiggin, keescook, kernel-hardening
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <5737c82b1ab4c80e53904e4846694884ca429569.camel@buserror.net>
在 2020/3/5 5:53, Scott Wood 写道:
> On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote:
>> The original kernel still exists in the memory, clear it now.
>>
>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>> Cc: Scott Wood <oss@buserror.net>
>> Cc: Diana Craciun <diana.craciun@nxp.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Nicholas Piggin <npiggin@gmail.com>
>> Cc: Kees Cook <keescook@chromium.org>
>> ---
>> arch/powerpc/mm/nohash/kaslr_booke.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c
>> b/arch/powerpc/mm/nohash/kaslr_booke.c
>> index c6f5c1db1394..ed1277059368 100644
>> --- a/arch/powerpc/mm/nohash/kaslr_booke.c
>> +++ b/arch/powerpc/mm/nohash/kaslr_booke.c
>> @@ -378,8 +378,10 @@ notrace void __init kaslr_early_init(void *dt_ptr,
>> phys_addr_t size)
>> unsigned int *__kaslr_offset = (unsigned int *)(KERNELBASE + 0x58);
>> unsigned int *__run_at_load = (unsigned int *)(KERNELBASE + 0x5c);
>>
>> - if (*__run_at_load == 1)
>> + if (*__run_at_load == 1) {
>> + kaslr_late_init();
>> return;
>> + }
>
> What if you're here because kexec set __run_at_load (or
> CONFIG_RELOCATABLE_TEST is enabled), not because kaslr happened?
>
Nothing will happen because kaslr_late_init() only clears memory when
kernstart_virt_addr is not KERNELBASE. When __run_at_load is set then
KASLR will not take effect.
> -Scott
>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH v3 4/6] powerpc/fsl_booke/64: do not clear the BSS for the second pass
From: Jason Yan @ 2020-03-05 3:14 UTC (permalink / raw)
To: Scott Wood, mpe, linuxppc-dev, diana.craciun, christophe.leroy,
benh, paulus, npiggin, keescook, kernel-hardening
Cc: linux-kernel, zhaohongjiang
In-Reply-To: <feda5c76f134b415d2f43b99b8d6880b9b4b1750.camel@buserror.net>
在 2020/3/5 5:49, Scott Wood 写道:
> On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote:
>> The BSS section has already cleared out in the first pass. No need to
>> clear it again. This can save some time when booting with KASLR
>> enabled.
>>
>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>> Cc: Scott Wood <oss@buserror.net>
>> Cc: Diana Craciun <diana.craciun@nxp.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Nicholas Piggin <npiggin@gmail.com>
>> Cc: Kees Cook <keescook@chromium.org>
>> ---
>> arch/powerpc/kernel/head_64.S | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
>> index 744624140fb8..8c644e7c3eaf 100644
>> --- a/arch/powerpc/kernel/head_64.S
>> +++ b/arch/powerpc/kernel/head_64.S
>> @@ -914,6 +914,13 @@ start_here_multiplatform:
>> bl relative_toc
>> tovirt(r2,r2)
>>
>> + /* Do not clear the BSS for the second pass if randomized */
>> + LOAD_REG_ADDR(r3, kernstart_virt_addr)
>> + lwz r3,0(r3)
>> + LOAD_REG_IMMEDIATE(r4, KERNELBASE)
>> + cmpw r3,r4
>> + bne 4f
>
> These are 64-bit values.
>
Oh yes, will fix. Thanks.
> -Scott
>
>
>
> .
>
^ 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