LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 3/5] powerpc/32: Fix hugepage allocation on 8xx at hint address
From: Christophe Leroy @ 2018-01-24  9:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Scott Wood, aneesh.kumar
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <6920f6efe2dcdabf59350b2d31ee6bd4bdef57f4.1516783089.git.christophe.leroy@c-s.fr>

On the 8xx, the page size is set in the PMD entry and applies to
all pages of the page table pointed by the said PMD entry.

When an app has some regular pages allocated (e.g. see below) and tries
to mmap() a huge page at a hint address covered by the same PMD entry,
the kernel accepts the hint allthough the 8xx cannot handle different
page sizes in the same PMD entry.

10000000-10001000 r-xp 00000000 00:0f 2597 /root/malloc
10010000-10011000 rwxp 00000000 00:0f 2597 /root/malloc

mmap(0x10080000, 524288, PROT_READ|PROT_WRITE,
     MAP_PRIVATE|MAP_ANONYMOUS|0x40000, -1, 0) = 0x10080000

This results the app remaining forever in do_page_fault()/hugetlb_fault()
and when interrupting that app, we get the following warning:

[162980.035629] WARNING: CPU: 0 PID: 2777 at arch/powerpc/mm/hugetlbpage.c:354 hugetlb_free_pgd_range+0xc8/0x1e4
[162980.035699] CPU: 0 PID: 2777 Comm: malloc Tainted: G W       4.14.6 #85
[162980.035744] task: c67e2c00 task.stack: c668e000
[162980.035783] NIP:  c000fe18 LR: c00e1eec CTR: c00f90c0
[162980.035830] REGS: c668fc20 TRAP: 0700   Tainted: G W        (4.14.6)
[162980.035854] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 24044224 XER: 20000000
[162980.036003]
[162980.036003] GPR00: c00e1eec c668fcd0 c67e2c00 00000010 c6869410 10080000 00000000 77fb4000
[162980.036003] GPR08: ffff0001 0683c001 00000000 ffffff80 44028228 10018a34 00004008 418004fc
[162980.036003] GPR16: c668e000 00040100 c668e000 c06c0000 c668fe78 c668e000 c6835ba0 c668fd48
[162980.036003] GPR24: 00000000 73ffffff 74000000 00000001 77fb4000 100fffff 10100000 10100000
[162980.036743] NIP [c000fe18] hugetlb_free_pgd_range+0xc8/0x1e4
[162980.036839] LR [c00e1eec] free_pgtables+0x12c/0x150
[162980.036861] Call Trace:
[162980.036939] [c668fcd0] [c00f0774] unlink_anon_vmas+0x1c4/0x214 (unreliable)
[162980.037040] [c668fd10] [c00e1eec] free_pgtables+0x12c/0x150
[162980.037118] [c668fd40] [c00eabac] exit_mmap+0xe8/0x1b4
[162980.037210] [c668fda0] [c0019710] mmput.part.9+0x20/0xd8
[162980.037301] [c668fdb0] [c001ecb0] do_exit+0x1f0/0x93c
[162980.037386] [c668fe00] [c001f478] do_group_exit+0x40/0xcc
[162980.037479] [c668fe10] [c002a76c] get_signal+0x47c/0x614
[162980.037570] [c668fe70] [c0007840] do_signal+0x54/0x244
[162980.037654] [c668ff30] [c0007ae8] do_notify_resume+0x34/0x88
[162980.037744] [c668ff40] [c000dae8] do_user_signal+0x74/0xc4
[162980.037781] Instruction dump:
[162980.037821] 7fdff378 81370000 54a3463a 80890020 7d24182e 7c841a14 712a0004 4082ff94
[162980.038014] 2f890000 419e0010 712a0ff0 408200e0 <0fe00000> 54a9000a 7f984840 419d0094
[162980.038216] ---[ end trace c0ceeca8e7a5800a ]---
[162980.038754] BUG: non-zero nr_ptes on freeing mm: 1
[162985.363322] BUG: non-zero nr_ptes on freeing mm: -1

In order to fix this, this patch uses the address space "slices"
implemented for BOOK3S/64 and enhanced to support PPC32 by the
preceding patch.

This patch modifies the context.id on the 8xx to be in the range
[1:16] instead of [0:15] in order to identify context.id == 0 as
not initialised contexts as done on BOOK3S

This patch activates CONFIG_PPC_MM_SLICES when CONFIG_HUGETLB_PAGE is
selected for the 8xx

Alltough we could in theory have as many slices as PMD entries, the
current slices implementation limits the number of low slices to 16.
This limitation is not preventing us to fix the initial issue allthough
it is suboptimal. It will be cured in a subsequent patch.

Fixes: 4b91428699477 ("powerpc/8xx: Implement support of hugepages")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 v2: First patch of v1 serie split in two parts
 v3: No changes

 arch/powerpc/include/asm/mmu-8xx.h     |  6 ++++++
 arch/powerpc/kernel/setup-common.c     |  2 ++
 arch/powerpc/mm/8xx_mmu.c              |  2 +-
 arch/powerpc/mm/hugetlbpage.c          |  2 ++
 arch/powerpc/mm/mmu_context_nohash.c   | 18 ++++++++++++++++--
 arch/powerpc/platforms/Kconfig.cputype |  1 +
 6 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/mmu-8xx.h b/arch/powerpc/include/asm/mmu-8xx.h
index 5bb3dbede41a..5f89b6010453 100644
--- a/arch/powerpc/include/asm/mmu-8xx.h
+++ b/arch/powerpc/include/asm/mmu-8xx.h
@@ -169,6 +169,12 @@ typedef struct {
 	unsigned int id;
 	unsigned int active;
 	unsigned long vdso_base;
+#ifdef CONFIG_PPC_MM_SLICES
+	u16 user_psize;		/* page size index */
+	u64 low_slices_psize;	/* page size encodings */
+	unsigned char high_slices_psize[0];
+	unsigned long slb_addr_limit;
+#endif
 } mm_context_t;
 
 #define PHYS_IMMR_BASE (mfspr(SPRN_IMMR) & 0xfff80000)
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 8fd3a70047f1..edf98ea92035 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -916,6 +916,8 @@ void __init setup_arch(char **cmdline_p)
 #ifdef CONFIG_PPC64
 	if (!radix_enabled())
 		init_mm.context.slb_addr_limit = DEFAULT_MAP_WINDOW_USER64;
+#elif defined(CONFIG_PPC_8xx)
+	init_mm.context.slb_addr_limit = DEFAULT_MAP_WINDOW;
 #else
 #error	"context.addr_limit not initialized."
 #endif
diff --git a/arch/powerpc/mm/8xx_mmu.c b/arch/powerpc/mm/8xx_mmu.c
index f29212e40f40..0be77709446c 100644
--- a/arch/powerpc/mm/8xx_mmu.c
+++ b/arch/powerpc/mm/8xx_mmu.c
@@ -192,7 +192,7 @@ void set_context(unsigned long id, pgd_t *pgd)
 	mtspr(SPRN_M_TW, __pa(pgd) - offset);
 
 	/* Update context */
-	mtspr(SPRN_M_CASID, id);
+	mtspr(SPRN_M_CASID, id - 1);
 	/* sync */
 	mb();
 }
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index a9b9083c5e49..79e1378ee303 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -553,9 +553,11 @@ unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
 	struct hstate *hstate = hstate_file(file);
 	int mmu_psize = shift_to_mmu_psize(huge_page_shift(hstate));
 
+#ifdef CONFIG_PPC_RADIX_MMU
 	if (radix_enabled())
 		return radix__hugetlb_get_unmapped_area(file, addr, len,
 						       pgoff, flags);
+#endif
 	return slice_get_unmapped_area(addr, len, flags, mmu_psize, 1);
 }
 #endif
diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index 4554d6527682..d98f7e5c141b 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -331,6 +331,20 @@ int init_new_context(struct task_struct *t, struct mm_struct *mm)
 {
 	pr_hard("initing context for mm @%p\n", mm);
 
+#ifdef	CONFIG_PPC_MM_SLICES
+	if (!mm->context.slb_addr_limit)
+		mm->context.slb_addr_limit = DEFAULT_MAP_WINDOW;
+
+	/*
+	 * We have MMU_NO_CONTEXT set to be ~0. Hence check
+	 * explicitly against context.id == 0. This ensures that we properly
+	 * initialize context slice details for newly allocated mm's (which will
+	 * have id == 0) and don't alter context slice inherited via fork (which
+	 * will have id != 0).
+	 */
+	if (mm->context.id == 0)
+		slice_set_user_psize(mm, mmu_virtual_psize);
+#endif
 	mm->context.id = MMU_NO_CONTEXT;
 	mm->context.active = 0;
 	return 0;
@@ -428,8 +442,8 @@ void __init mmu_context_init(void)
 	 *      -- BenH
 	 */
 	if (mmu_has_feature(MMU_FTR_TYPE_8xx)) {
-		first_context = 0;
-		last_context = 15;
+		first_context = 1;
+		last_context = 16;
 		no_selective_tlbil = true;
 	} else if (mmu_has_feature(MMU_FTR_TYPE_47x)) {
 		first_context = 1;
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index ae07470fde3c..73a7ea333e9e 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -334,6 +334,7 @@ config PPC_BOOK3E_MMU
 config PPC_MM_SLICES
 	bool
 	default y if PPC_BOOK3S_64
+	default y if PPC_8xx && HUGETLB_PAGE
 	default n
 
 config PPC_HAVE_PMU_SUPPORT
-- 
2.13.3

^ permalink raw reply related

* [PATCH v3 2/5] powerpc/mm: Enhance 'slice' for supporting PPC32
From: Christophe Leroy @ 2018-01-24  9:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Scott Wood, aneesh.kumar
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <6920f6efe2dcdabf59350b2d31ee6bd4bdef57f4.1516783089.git.christophe.leroy@c-s.fr>

In preparation for the following patch which will fix an issue on
the 8xx by re-using the 'slices', this patch enhances the
'slices' implementation to support 32 bits CPUs.

On PPC32, the address space is limited to 4Gbytes, hence only the low
slices will be used.

This patch moves "slices" functions prototypes from page64.h to slice.h

The high slices use bitmaps. As bitmap functions are not prepared to
handling bitmaps of size 0, the bitmap_xxx() calls are wrapped into
slice_bitmap_xxx() functions which will void on PPC32

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 v2: First patch of v1 serie split in two parts ; added slice_bitmap_xxx() macros.
 v3: Moving slice related stuff in slice.h and slice_32/64.h
     slice_bitmap_xxx() are now static inline functions and platform dependent
     SLICE_LOW_TOP declared ull on PPC32 with correct casts allows to keep it 0x100000000

 arch/powerpc/include/asm/page.h     |  1 +
 arch/powerpc/include/asm/page_64.h  | 59 ----------------------------------
 arch/powerpc/include/asm/slice.h    | 63 +++++++++++++++++++++++++++++++++++++
 arch/powerpc/include/asm/slice_32.h | 56 +++++++++++++++++++++++++++++++++
 arch/powerpc/include/asm/slice_64.h | 61 +++++++++++++++++++++++++++++++++++
 arch/powerpc/mm/slice.c             | 38 ++++++++++++----------
 6 files changed, 203 insertions(+), 75 deletions(-)
 create mode 100644 arch/powerpc/include/asm/slice.h
 create mode 100644 arch/powerpc/include/asm/slice_32.h
 create mode 100644 arch/powerpc/include/asm/slice_64.h

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 8da5d4c1cab2..d5f1c41b7dba 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -344,5 +344,6 @@ typedef struct page *pgtable_t;
 
 #include <asm-generic/memory_model.h>
 #endif /* __ASSEMBLY__ */
+#include <asm/slice.h>
 
 #endif /* _ASM_POWERPC_PAGE_H */
diff --git a/arch/powerpc/include/asm/page_64.h b/arch/powerpc/include/asm/page_64.h
index 56234c6fcd61..af04acdb873f 100644
--- a/arch/powerpc/include/asm/page_64.h
+++ b/arch/powerpc/include/asm/page_64.h
@@ -86,65 +86,6 @@ extern u64 ppc64_pft_size;
 
 #endif /* __ASSEMBLY__ */
 
-#ifdef CONFIG_PPC_MM_SLICES
-
-#define SLICE_LOW_SHIFT		28
-#define SLICE_HIGH_SHIFT	40
-
-#define SLICE_LOW_TOP		(0x100000000ul)
-#define SLICE_NUM_LOW		(SLICE_LOW_TOP >> SLICE_LOW_SHIFT)
-#define SLICE_NUM_HIGH		(H_PGTABLE_RANGE >> SLICE_HIGH_SHIFT)
-
-#define GET_LOW_SLICE_INDEX(addr)	((addr) >> SLICE_LOW_SHIFT)
-#define GET_HIGH_SLICE_INDEX(addr)	((addr) >> SLICE_HIGH_SHIFT)
-
-#ifndef __ASSEMBLY__
-struct mm_struct;
-
-extern unsigned long slice_get_unmapped_area(unsigned long addr,
-					     unsigned long len,
-					     unsigned long flags,
-					     unsigned int psize,
-					     int topdown);
-
-extern unsigned int get_slice_psize(struct mm_struct *mm,
-				    unsigned long addr);
-
-extern void slice_set_user_psize(struct mm_struct *mm, unsigned int psize);
-extern void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
-				  unsigned long len, unsigned int psize);
-
-#endif /* __ASSEMBLY__ */
-#else
-#define slice_init()
-#ifdef CONFIG_PPC_BOOK3S_64
-#define get_slice_psize(mm, addr)	((mm)->context.user_psize)
-#define slice_set_user_psize(mm, psize)		\
-do {						\
-	(mm)->context.user_psize = (psize);	\
-	(mm)->context.sllp = SLB_VSID_USER | mmu_psize_defs[(psize)].sllp; \
-} while (0)
-#else /* !CONFIG_PPC_BOOK3S_64 */
-#ifdef CONFIG_PPC_64K_PAGES
-#define get_slice_psize(mm, addr)	MMU_PAGE_64K
-#else /* CONFIG_PPC_64K_PAGES */
-#define get_slice_psize(mm, addr)	MMU_PAGE_4K
-#endif /* !CONFIG_PPC_64K_PAGES */
-#define slice_set_user_psize(mm, psize)	do { BUG(); } while(0)
-#endif /* CONFIG_PPC_BOOK3S_64 */
-
-#define slice_set_range_psize(mm, start, len, psize)	\
-	slice_set_user_psize((mm), (psize))
-#endif /* CONFIG_PPC_MM_SLICES */
-
-#ifdef CONFIG_HUGETLB_PAGE
-
-#ifdef CONFIG_PPC_MM_SLICES
-#define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
-#endif
-
-#endif /* !CONFIG_HUGETLB_PAGE */
-
 #define VM_DATA_DEFAULT_FLAGS \
 	(is_32bit_task() ? \
 	 VM_DATA_DEFAULT_FLAGS32 : VM_DATA_DEFAULT_FLAGS64)
diff --git a/arch/powerpc/include/asm/slice.h b/arch/powerpc/include/asm/slice.h
new file mode 100644
index 000000000000..2b4b70de7e71
--- /dev/null
+++ b/arch/powerpc/include/asm/slice.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_SLICE_H
+#define _ASM_POWERPC_SLICE_H
+
+#ifdef CONFIG_PPC_MM_SLICES
+
+#ifdef CONFIG_PPC64
+#include <asm/slice_64.h>
+#else
+#include <asm/slice_32.h>
+#endif
+
+#ifdef CONFIG_HUGETLB_PAGE
+#define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
+#endif
+#define HAVE_ARCH_UNMAPPED_AREA
+#define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
+
+#define SLICE_LOW_SHIFT		28
+#define SLICE_LOW_TOP		(0x100000000ull)
+#define SLICE_NUM_LOW		(SLICE_LOW_TOP >> SLICE_LOW_SHIFT)
+#define GET_LOW_SLICE_INDEX(addr)	((addr) >> SLICE_LOW_SHIFT)
+
+#ifndef __ASSEMBLY__
+
+struct mm_struct;
+
+unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
+				      unsigned long flags, unsigned int psize,
+				      int topdown);
+
+unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr);
+
+void slice_set_user_psize(struct mm_struct *mm, unsigned int psize);
+void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
+			   unsigned long len, unsigned int psize);
+
+#else
+#define slice_init()
+#ifdef CONFIG_PPC_BOOK3S_64
+#define get_slice_psize(mm, addr)	((mm)->context.user_psize)
+#define slice_set_user_psize(mm, psize)		\
+do {						\
+	(mm)->context.user_psize = (psize);	\
+	(mm)->context.sllp = SLB_VSID_USER | mmu_psize_defs[(psize)].sllp; \
+} while (0)
+#else /* !CONFIG_PPC_BOOK3S_64 */
+#ifdef CONFIG_PPC_64K_PAGES
+#define get_slice_psize(mm, addr)	MMU_PAGE_64K
+#else /* CONFIG_PPC_64K_PAGES */
+#define get_slice_psize(mm, addr)	MMU_PAGE_4K
+#endif /* !CONFIG_PPC_64K_PAGES */
+#define slice_set_user_psize(mm, psize)	do { BUG(); } while(0)
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
+#define slice_set_range_psize(mm, start, len, psize)	\
+	slice_set_user_psize((mm), (psize))
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* CONFIG_PPC_MM_SLICES */
+
+#endif /* _ASM_POWERPC_SLICE_H */
diff --git a/arch/powerpc/include/asm/slice_32.h b/arch/powerpc/include/asm/slice_32.h
new file mode 100644
index 000000000000..7e27c0dfb913
--- /dev/null
+++ b/arch/powerpc/include/asm/slice_32.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_SLICE_32_H
+#define _ASM_POWERPC_SLICE_32_H
+
+#define SLICE_HIGH_SHIFT	0
+#define SLICE_NUM_HIGH		0ul
+#define GET_HIGH_SLICE_INDEX(addr)	(addr & 0)
+
+#ifndef __ASSEMBLY__
+
+static inline void slice_bitmap_zero(unsigned long *dst, unsigned int nbits)
+{
+}
+
+static inline int slice_bitmap_and(unsigned long *dst,
+				   const unsigned long *src1,
+				   const unsigned long *src2,
+				   unsigned int nbits)
+{
+	return 0;
+}
+
+static inline void slice_bitmap_or(unsigned long *dst,
+				   const unsigned long *src1,
+				   const unsigned long *src2,
+				   unsigned int nbits)
+{
+}
+
+static inline int slice_bitmap_andnot(unsigned long *dst,
+				      const unsigned long *src1,
+				      const unsigned long *src2,
+				      unsigned int nbits)
+{
+	return 0;
+}
+
+static inline int slice_bitmap_equal(const unsigned long *src1,
+				     const unsigned long *src2,
+				     unsigned int nbits)
+{
+	return 1;
+}
+
+static inline int slice_bitmap_empty(const unsigned long *src, unsigned nbits)
+{
+	return 1;
+}
+
+static inline void slice_bitmap_set(unsigned long *map, unsigned int start,
+				    unsigned int nbits)
+{
+}
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASM_POWERPC_SLICE_32_H */
diff --git a/arch/powerpc/include/asm/slice_64.h b/arch/powerpc/include/asm/slice_64.h
new file mode 100644
index 000000000000..9d1c97b83010
--- /dev/null
+++ b/arch/powerpc/include/asm/slice_64.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_SLICE_64_H
+#define _ASM_POWERPC_SLICE_64_H
+
+#define SLICE_HIGH_SHIFT	40
+#define SLICE_NUM_HIGH		(H_PGTABLE_RANGE >> SLICE_HIGH_SHIFT)
+#define GET_HIGH_SLICE_INDEX(addr)	((addr) >> SLICE_HIGH_SHIFT)
+
+#ifndef __ASSEMBLY__
+
+#include <linux/bitmap.h>
+
+static inline void slice_bitmap_zero(unsigned long *dst, unsigned int nbits)
+{
+	bitmap_zero(dst, nbits);
+}
+
+static inline int slice_bitmap_and(unsigned long *dst,
+				   const unsigned long *src1,
+				   const unsigned long *src2,
+				   unsigned int nbits)
+{
+	return bitmap_and(dst, src1, src2, nbits);
+}
+
+static inline void slice_bitmap_or(unsigned long *dst,
+				   const unsigned long *src1,
+				   const unsigned long *src2,
+				   unsigned int nbits)
+{
+	bitmap_or(dst, src1, src2, nbits);
+}
+
+static inline int slice_bitmap_andnot(unsigned long *dst,
+				      const unsigned long *src1,
+				      const unsigned long *src2,
+				      unsigned int nbits)
+{
+	return bitmap_andnot(dst, src1, src2, nbits);
+}
+
+static inline int slice_bitmap_equal(const unsigned long *src1,
+				     const unsigned long *src2,
+				     unsigned int nbits)
+{
+	return bitmap_equal(src1, src2, nbits);
+}
+
+static inline int slice_bitmap_empty(const unsigned long *src, unsigned nbits)
+{
+	return bitmap_empty(src, nbits);
+}
+
+static inline void slice_bitmap_set(unsigned long *map, unsigned int start,
+				    unsigned int nbits)
+{
+	bitmap_set(map, start, nbits);
+}
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASM_POWERPC_SLICE_64_H */
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 98b53d48968f..549704dfa777 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -73,10 +73,11 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
 	unsigned long end = start + len - 1;
 
 	ret->low_slices = 0;
-	bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
+	slice_bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
 
 	if (start < SLICE_LOW_TOP) {
-		unsigned long mend = min(end, (SLICE_LOW_TOP - 1));
+		unsigned long mend = min(end,
+					 (unsigned long)(SLICE_LOW_TOP - 1));
 
 		ret->low_slices = (1u << (GET_LOW_SLICE_INDEX(mend) + 1))
 			- (1u << GET_LOW_SLICE_INDEX(start));
@@ -87,7 +88,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
 		unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
 		unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
 
-		bitmap_set(ret->high_slices, start_index, count);
+		slice_bitmap_set(ret->high_slices, start_index, count);
 	}
 }
 
@@ -113,11 +114,13 @@ static int slice_high_has_vma(struct mm_struct *mm, unsigned long slice)
 	unsigned long start = slice << SLICE_HIGH_SHIFT;
 	unsigned long end = start + (1ul << SLICE_HIGH_SHIFT);
 
+#ifdef CONFIG_PPC64
 	/* Hack, so that each addresses is controlled by exactly one
 	 * of the high or low area bitmaps, the first high area starts
 	 * at 4GB, not 0 */
 	if (start == 0)
 		start = SLICE_LOW_TOP;
+#endif
 
 	return !slice_area_is_free(mm, start, end - start);
 }
@@ -128,7 +131,7 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
 	unsigned long i;
 
 	ret->low_slices = 0;
-	bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
+	slice_bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
 
 	for (i = 0; i < SLICE_NUM_LOW; i++)
 		if (!slice_low_has_vma(mm, i))
@@ -151,7 +154,7 @@ static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_ma
 	u64 lpsizes;
 
 	ret->low_slices = 0;
-	bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
+	slice_bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
 
 	lpsizes = mm->context.low_slices_psize;
 	for (i = 0; i < SLICE_NUM_LOW; i++)
@@ -180,15 +183,16 @@ static int slice_check_fit(struct mm_struct *mm,
 	 */
 	unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
 
-	bitmap_and(result, mask.high_slices,
-		   available.high_slices, slice_count);
+	slice_bitmap_and(result, mask.high_slices, available.high_slices,
+			 slice_count);
 
 	return (mask.low_slices & available.low_slices) == mask.low_slices &&
-		bitmap_equal(result, mask.high_slices, slice_count);
+		slice_bitmap_equal(result, mask.high_slices, slice_count);
 }
 
 static void slice_flush_segments(void *parm)
 {
+#ifdef CONFIG_PPC64
 	struct mm_struct *mm = parm;
 	unsigned long flags;
 
@@ -200,6 +204,7 @@ static void slice_flush_segments(void *parm)
 	local_irq_save(flags);
 	slb_flush_and_rebolt();
 	local_irq_restore(flags);
+#endif
 }
 
 static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psize)
@@ -389,16 +394,16 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
 static inline void slice_or_mask(struct slice_mask *dst, struct slice_mask *src)
 {
 	dst->low_slices |= src->low_slices;
-	bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
-		  SLICE_NUM_HIGH);
+	slice_bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
+			SLICE_NUM_HIGH);
 }
 
 static inline void slice_andnot_mask(struct slice_mask *dst, struct slice_mask *src)
 {
 	dst->low_slices &= ~src->low_slices;
 
-	bitmap_andnot(dst->high_slices, dst->high_slices, src->high_slices,
-		      SLICE_NUM_HIGH);
+	slice_bitmap_andnot(dst->high_slices, dst->high_slices,
+			    src->high_slices, SLICE_NUM_HIGH);
 }
 
 #ifdef CONFIG_PPC_64K_PAGES
@@ -446,14 +451,14 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
 	 * init different masks
 	 */
 	mask.low_slices = 0;
-	bitmap_zero(mask.high_slices, SLICE_NUM_HIGH);
+	slice_bitmap_zero(mask.high_slices, SLICE_NUM_HIGH);
 
 	/* silence stupid warning */;
 	potential_mask.low_slices = 0;
-	bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
+	slice_bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
 
 	compat_mask.low_slices = 0;
-	bitmap_zero(compat_mask.high_slices, SLICE_NUM_HIGH);
+	slice_bitmap_zero(compat_mask.high_slices, SLICE_NUM_HIGH);
 
 	/* Sanity checks */
 	BUG_ON(mm->task_size == 0);
@@ -591,7 +596,8 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
  convert:
 	slice_andnot_mask(&mask, &good_mask);
 	slice_andnot_mask(&mask, &compat_mask);
-	if (mask.low_slices || !bitmap_empty(mask.high_slices, SLICE_NUM_HIGH)) {
+	if (mask.low_slices ||
+	    !slice_bitmap_empty(mask.high_slices, SLICE_NUM_HIGH)) {
 		slice_convert(mm, mask, psize);
 		if (psize > MMU_PAGE_BASE)
 			on_each_cpu(slice_flush_segments, mm, 1);
-- 
2.13.3

^ permalink raw reply related

* [PATCH v3 1/5] powerpc/mm: Remove intermediate bitmap copy in 'slices'
From: Christophe Leroy @ 2018-01-24  9:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Scott Wood, aneesh.kumar
  Cc: linux-kernel, linuxppc-dev

bitmap_or() and bitmap_andnot() can work properly with dst identical
to src1 or src2. There is no need of an intermediate result bitmap
that is copied back to dst in a second step.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 v2: New in v2
 v3: patch moved up front of the serie to avoid ephemeral slice_bitmap_copy() function in following patch

 arch/powerpc/mm/slice.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 23ec2c5e3b78..98b53d48968f 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -388,21 +388,17 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
 
 static inline void slice_or_mask(struct slice_mask *dst, struct slice_mask *src)
 {
-	DECLARE_BITMAP(result, SLICE_NUM_HIGH);
-
 	dst->low_slices |= src->low_slices;
-	bitmap_or(result, dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
-	bitmap_copy(dst->high_slices, result, SLICE_NUM_HIGH);
+	bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
+		  SLICE_NUM_HIGH);
 }
 
 static inline void slice_andnot_mask(struct slice_mask *dst, struct slice_mask *src)
 {
-	DECLARE_BITMAP(result, SLICE_NUM_HIGH);
-
 	dst->low_slices &= ~src->low_slices;
 
-	bitmap_andnot(result, dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
-	bitmap_copy(dst->high_slices, result, SLICE_NUM_HIGH);
+	bitmap_andnot(dst->high_slices, dst->high_slices, src->high_slices,
+		      SLICE_NUM_HIGH);
 }
 
 #ifdef CONFIG_PPC_64K_PAGES
-- 
2.13.3

^ permalink raw reply related

* Re: [PATCH-next] powerpc/fsl_pci: Use PTR_ERR_OR_ZERO
From: Tyrel Datwyler @ 2018-01-24  3:33 UTC (permalink / raw)
  To: Christopher Díaz Riveros, benh, paulus, mpe, joe,
	christian.koenig, tyreld, konrad.wilk, robh
  Cc: linuxppc-dev, linux-kernel, kernel-janitors
In-Reply-To: <20180123203752.15057-1-chrisadr@gentoo.org>

On 01/23/2018 12:37 PM, Christopher Díaz Riveros wrote:
> Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Christopher Díaz Riveros <chrisadr@gentoo.org>
> ---

Reviewed-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

>  arch/powerpc/sysdev/fsl_pci.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index cc20d2255d7f..142184635c81 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -1304,10 +1304,7 @@ static int add_err_dev(struct platform_device *pdev)
>  						   pdev->resource,
>  						   pdev->num_resources,
>  						   &pd, sizeof(pd));
> -	if (IS_ERR(errdev))
> -		return PTR_ERR(errdev);
> -
> -	return 0;
> +	return PTR_ERR_OR_ZERO(errdev);
>  }
>  
>  static int fsl_pci_probe(struct platform_device *pdev)
> 

^ permalink raw reply

* [PATCH] ibmvfc: fix misdefined reserved field in ibmvfc_fcp_rsp_info
From: Tyrel Datwyler @ 2018-01-24  2:11 UTC (permalink / raw)
  To: james.bottomley
  Cc: martin.petersen, linux-scsi, linuxppc-dev, brking, nfont, hare,
	Tyrel Datwyler, stable

The fcp_rsp_info structure as defined in the FC spec has an initial 3 bytes
reserved field. The ibmvfc driver mistakenly defined this field as 4 bytes
resulting in the rsp_code field being defined in what should be the start of
the second reserved field and thus always being reported as zero by the
driver.

Ideally, we should wire ibmvfc up with libfc for the sake of code
deduplication, and ease of maintaining standardized structures in a single
place. However, for now simply fixup the definition in ibmvfc for
backporting to distros on older kernels. Wiring up with libfc will be done
in a followup patch.

Cc: stable@vger.kernel.org
Reported-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvfc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 9a0696f..b81a53c 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -367,7 +367,7 @@ enum ibmvfc_fcp_rsp_info_codes {
 };
 
 struct ibmvfc_fcp_rsp_info {
-	__be16 reserved;
+	u8 reserved[3];
 	u8 rsp_code;
 	u8 reserved2[4];
 }__attribute__((packed, aligned (2)));
-- 
2.7.4

^ permalink raw reply related

* [PATCH] macintosh/ams-input: Use true and false for boolean values
From: Gustavo A. R. Silva @ 2018-01-24  1:48 UTC (permalink / raw)
  To: Michael Hanselmann, Benjamin Herrenschmidt
  Cc: linuxppc-dev, linux-kernel, Gustavo A. R. Silva

Assign true or false to boolean variables instead of an integer value.

This issue was detected with the help of Coccinelle

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/macintosh/ams/ams-input.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/macintosh/ams/ams-input.c b/drivers/macintosh/ams/ams-input.c
index 2edae7d..fe248f6 100644
--- a/drivers/macintosh/ams/ams-input.c
+++ b/drivers/macintosh/ams/ams-input.c
@@ -91,7 +91,7 @@ static int ams_input_enable(void)
 		return error;
 	}
 
-	joystick = 1;
+	joystick = true;
 
 	return 0;
 }
@@ -104,7 +104,7 @@ static void ams_input_disable(void)
 		ams_info.idev = NULL;
 	}
 
-	joystick = 0;
+	joystick = false;
 }
 
 static ssize_t ams_input_show_joystick(struct device *dev,
-- 
2.7.4

^ permalink raw reply related

* [PATCH] macintosh: change some data types from int to bool
From: Gustavo A. R. Silva @ 2018-01-24  1:42 UTC (permalink / raw)
  To: Michael Ellerman, Colin Leroy, Benjamin Herrenschmidt
  Cc: linuxppc-dev, linux-kernel, Gustavo A. R. Silva

Change the data type of the following variables from int to bool
across all macintosh drivers:

started
slots_started
pm121_started
wf_smu_started

Some of these issues were detected with the help of Coccinelle.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/macintosh/therm_adt746x.c  | 4 ++--
 drivers/macintosh/windfarm_pm112.c | 8 ++++----
 drivers/macintosh/windfarm_pm121.c | 5 +++--
 drivers/macintosh/windfarm_pm72.c  | 2 +-
 drivers/macintosh/windfarm_pm81.c  | 5 +++--
 drivers/macintosh/windfarm_pm91.c  | 5 +++--
 drivers/macintosh/windfarm_rm31.c  | 2 +-
 7 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/macintosh/therm_adt746x.c b/drivers/macintosh/therm_adt746x.c
index f433521..d7cd5af 100644
--- a/drivers/macintosh/therm_adt746x.c
+++ b/drivers/macintosh/therm_adt746x.c
@@ -230,7 +230,7 @@ static void update_fans_speed (struct thermostat *th)
 
 	/* we don't care about local sensor, so we start at sensor 1 */
 	for (i = 1; i < 3; i++) {
-		int started = 0;
+		bool started = false;
 		int fan_number = (th->type == ADT7460 && i == 2);
 		int var = th->temps[i] - th->limits[i];
 
@@ -243,7 +243,7 @@ static void update_fans_speed (struct thermostat *th)
 			if (abs(var - th->last_var[fan_number]) < 2)
 				continue;
 
-			started = 1;
+			started = true;
 			new_speed = fan_speed + ((var-1)*step);
 
 			if (new_speed < fan_speed)
diff --git a/drivers/macintosh/windfarm_pm112.c b/drivers/macintosh/windfarm_pm112.c
index 96d16fc..fec91db 100644
--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c
@@ -96,14 +96,14 @@ static int cpu_last_target;
 static struct wf_pid_state backside_pid;
 static int backside_tick;
 static struct wf_pid_state slots_pid;
-static int slots_started;
+static bool slots_started;
 static struct wf_pid_state drive_bay_pid;
 static int drive_bay_tick;
 
 static int nr_cores;
 static int have_all_controls;
 static int have_all_sensors;
-static int started;
+static bool started;
 
 static int failure_state;
 #define FAILURE_SENSOR		1
@@ -462,7 +462,7 @@ static void slots_fan_tick(void)
 		/* first time; initialize things */
 		printk(KERN_INFO "windfarm: Slots control loop started.\n");
 		wf_pid_init(&slots_pid, &slots_param);
-		slots_started = 1;
+		slots_started = true;
 	}
 
 	err = slots_power->ops->get_value(slots_power, &power);
@@ -506,7 +506,7 @@ static void pm112_tick(void)
 	int i, last_failure;
 
 	if (!started) {
-		started = 1;
+		started = true;
 		printk(KERN_INFO "windfarm: CPUs control loops started.\n");
 		for (i = 0; i < nr_cores; ++i) {
 			if (create_cpu_loop(i) < 0) {
diff --git a/drivers/macintosh/windfarm_pm121.c b/drivers/macintosh/windfarm_pm121.c
index b350fb8..4d72d8f 100644
--- a/drivers/macintosh/windfarm_pm121.c
+++ b/drivers/macintosh/windfarm_pm121.c
@@ -246,7 +246,8 @@ enum {
 static struct wf_control *controls[N_CONTROLS] = {};
 
 /* Set to kick the control loop into life */
-static int pm121_all_controls_ok, pm121_all_sensors_ok, pm121_started;
+static int pm121_all_controls_ok, pm121_all_sensors_ok;
+static bool pm121_started;
 
 enum {
 	FAILURE_FAN		= 1 << 0,
@@ -806,7 +807,7 @@ static void pm121_tick(void)
 			pm121_create_sys_fans(i);
 
 		pm121_create_cpu_fans();
-		pm121_started = 1;
+		pm121_started = true;
 	}
 
 	/* skipping ticks */
diff --git a/drivers/macintosh/windfarm_pm72.c b/drivers/macintosh/windfarm_pm72.c
index e88cfb3..8330215 100644
--- a/drivers/macintosh/windfarm_pm72.c
+++ b/drivers/macintosh/windfarm_pm72.c
@@ -611,7 +611,7 @@ static void pm72_tick(void)
 	int i, last_failure;
 
 	if (!started) {
-		started = 1;
+		started = true;
 		printk(KERN_INFO "windfarm: CPUs control loops started.\n");
 		for (i = 0; i < nr_chips; ++i) {
 			if (cpu_setup_pid(i) < 0) {
diff --git a/drivers/macintosh/windfarm_pm81.c b/drivers/macintosh/windfarm_pm81.c
index 93faf29..d9ea455 100644
--- a/drivers/macintosh/windfarm_pm81.c
+++ b/drivers/macintosh/windfarm_pm81.c
@@ -140,7 +140,8 @@ static struct wf_control *fan_system;
 static struct wf_control *cpufreq_clamp;
 
 /* Set to kick the control loop into life */
-static int wf_smu_all_controls_ok, wf_smu_all_sensors_ok, wf_smu_started;
+static int wf_smu_all_controls_ok, wf_smu_all_sensors_ok;
+static bool wf_smu_started;
 
 /* Failure handling.. could be nicer */
 #define FAILURE_FAN		0x01
@@ -549,7 +550,7 @@ static void wf_smu_tick(void)
 		DBG("wf: creating control loops !\n");
 		wf_smu_create_sys_fans();
 		wf_smu_create_cpu_fans();
-		wf_smu_started = 1;
+		wf_smu_started = true;
 	}
 
 	/* Skipping ticks */
diff --git a/drivers/macintosh/windfarm_pm91.c b/drivers/macintosh/windfarm_pm91.c
index 81fdf40..7fd73dc 100644
--- a/drivers/macintosh/windfarm_pm91.c
+++ b/drivers/macintosh/windfarm_pm91.c
@@ -75,7 +75,8 @@ static struct wf_control *fan_slots;
 static struct wf_control *cpufreq_clamp;
 
 /* Set to kick the control loop into life */
-static int wf_smu_all_controls_ok, wf_smu_all_sensors_ok, wf_smu_started;
+static int wf_smu_all_controls_ok, wf_smu_all_sensors_ok;
+static bool wf_smu_started;
 static bool wf_smu_overtemp;
 
 /* Failure handling.. could be nicer */
@@ -467,7 +468,7 @@ static void wf_smu_tick(void)
 		wf_smu_create_drive_fans();
 		wf_smu_create_slots_fans();
 		wf_smu_create_cpu_fans();
-		wf_smu_started = 1;
+		wf_smu_started = true;
 	}
 
 	/* Skipping ticks */
diff --git a/drivers/macintosh/windfarm_rm31.c b/drivers/macintosh/windfarm_rm31.c
index a0cd9c7..9ce87cc 100644
--- a/drivers/macintosh/windfarm_rm31.c
+++ b/drivers/macintosh/windfarm_rm31.c
@@ -514,7 +514,7 @@ static void rm31_tick(void)
 	int i, last_failure;
 
 	if (!started) {
-		started = 1;
+		started = true;
 		printk(KERN_INFO "windfarm: CPUs control loops started.\n");
 		for (i = 0; i < nr_chips; ++i) {
 			if (cpu_setup_pid(i) < 0) {
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] macintosh/ams-input: Use true and false for boolean values
From: Michael Hanselmann @ 2018-01-24  1:55 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Michael Hanselmann, Benjamin Herrenschmidt
  Cc: linuxppc-dev, linux-kernel, Gustavo A. R. Silva
In-Reply-To: <20180124014846.GA27664@embeddedgus>

On 24.01.2018 02:48, Gustavo A. R. Silva wrote:
> Assign true or false to boolean variables instead of an integer value.
> 
> This issue was detected with the help of Coccinelle
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Reviewed-by: Michael Hanselmann <public@hansmi.ch>

^ permalink raw reply

* Re: [PATCH] drivers/macintosh: Use true for boolean value
From: Gustavo A. R. Silva @ 2018-01-24  1:50 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Gustavo A. R. Silva, Benjamin Herrenschmidt, linuxppc-dev,
	linux-kernel
In-Reply-To: <20180123185735.Horde.bV6oSdW0ZLemZoMF-I0rC1M@gator4166.hostgator.com>


Quoting "Gustavo A. R. Silva" <garsilva@embeddedor.com>:

> Quoting Michael Ellerman <mpe@ellerman.id.au>:
>
>> "Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:
>>
>>> Assign true or false to boolean variables instead of an integer value.
>>>
>>> This issue was detected with the help of Coccinelle.
>>>
>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>>> ---
>>> drivers/macintosh/windfarm_pm72.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> This seems to be common across all those drivers. Can you please send
>> one patch to fix them all to use bool:
>>
>>  $ git grep "started = " drivers/macintosh/
>>  drivers/macintosh/therm_adt746x.c:              int started = 0;
>>  drivers/macintosh/therm_adt746x.c:                      started = 1;
>>  drivers/macintosh/windfarm_pm112.c:             slots_started = 1;
>>  drivers/macintosh/windfarm_pm112.c:             started = 1;
>>  drivers/macintosh/windfarm_pm121.c:             pm121_started = 1;
>>  drivers/macintosh/windfarm_pm72.c:              started = 1;
>>  drivers/macintosh/windfarm_pm81.c:              wf_smu_started = 1;
>>  drivers/macintosh/windfarm_pm91.c:              wf_smu_started = 1;
>>  drivers/macintosh/windfarm_rm31.c:              started = 1;
>>
>> cheers
>
> Sure, no problem.
>
> By the way, I've just found the following similar case:
>
> --- a/drivers/macintosh/ams/ams-input.c
> +++ b/drivers/macintosh/ams/ams-input.c
> @@ -91,7 +91,7 @@ static int ams_input_enable(void)
>  		return error;
>  	}
>
> -	joystick = 1;
> +	joystick = true;
>
>  	return 0;
>  }
> @@ -104,7 +104,7 @@ static void ams_input_disable(void)
>  		ams_info.idev = NULL;
>  	}
>
> -	joystick = 0;
> +	joystick = false;
>  }
>
> Do you want me to include them all in the same patch?
>

I sent separate patches for this.

Thanks
--
Gustavo

^ permalink raw reply

* Re: [PATCH v2 1/6] resource: Extend the PPC32 reserved memory hack
From: Michael Ellerman @ 2018-01-24  1:23 UTC (permalink / raw)
  To: Jonathan Neuschäfer
  Cc: Jonathan Neuschäfer, linux-kernel, Tom Lendacky,
	Brijesh Singh, devicetree, Albert Herranz, linux-gpio,
	Thomas Gleixner, Borislav Petkov, linuxppc-dev
In-Reply-To: <20180123163739.2sxhzavghzgbjw4c@latitude>

Jonathan Neusch=C3=A4fer <j.neuschaefer@gmx.net> writes:

> On Tue, Jan 23, 2018 at 11:58:06PM +1100, Michael Ellerman wrote:
>> Jonathan Neusch=C3=A4fer <j.neuschaefer@gmx.net> writes:
>>=20
>> > On the Nintendo Wii, there are two ranges of physical memory, and MMIO
>> > in between, but Linux on ppc32 doesn't support discontiguous memory.
>> > Therefore a hack was introduced in commit c5df7f775148 ("powerpc: allow
>> > ioremap within reserved memory regions") and commit de32400dd26e ("wii:
>> > use both mem1 and mem2 as ram"):
>> >
>> >  - Treat the area from the start of the first memory area (MEM1) to the
>> >    end of the second (MEM2) as one big memory area, but mark the part
>> >    that doesn't belong to MEM1 or MEM2 as reserved.
>> >  - Only on the Wii, allow ioremap to be used on reserved memory.
>> >
>> > This hack, however, doesn't account for the "resource"-based API in
>> > kernel/resource.c, because __request_region performs its own checks.
>> >
>> > Extend the hack to kernel/resource.c, to allow more drivers to allocate
>> > their MMIO regions on the Wii.
>>=20
>> Hi Jonathan,
>>=20
>> Sorry but I can't merge a hack like this in generic code.
>
> Makes sense.
>
>> Has anyone looked at adding proper discontig mem support to PPC32?
>
> I'm not aware of any such effort.
>
> Do you have any pointer on how to implement discontiguous memory
> support? CONFIG_ARCH_SPARSEMEM_ENABLE seems relevant.

I'm not really sure what the key impediment to it working is.

You don't need to go all the way to SPARSEMEM, there is DISCONTIGMEM
which IIUI is quite a bit simpler.

I'd actually be interested to know what happens (ie. breaks) if you just
add the two memblocks and leave the hole in between. Is it the generic
code that breaks or is it something in the powerpc code? If it's the
later maybe we can do a small fix/hack to work around that.

>> Or can we punch a hole in the resource in the right place? Maybe from
>> add_system_ram_resources() ?
>
> Not sure. add_system_ram_resources would need the original memblock
> table, which is overwritten in wii_memory_fixups, if I read the code
> correctly.

Or it just needs to know where the "wii hole" is, and it can skip that
region, that should be doable, but whether it actually works I'm not
100% sure.

> If a proper solution doesn't take an overwhelming amount of work, I'd
> prefer a proper solution.

Thanks.

cheers

^ permalink raw reply

* Re: [PATCH] drivers/macintosh: Use true for boolean value
From: Gustavo A. R. Silva @ 2018-01-24  0:57 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Gustavo A. R. Silva, Benjamin Herrenschmidt, linuxppc-dev,
	linux-kernel
In-Reply-To: <87vafsulq1.fsf@concordia.ellerman.id.au>


Quoting Michael Ellerman <mpe@ellerman.id.au>:

> "Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:
>
>> Assign true or false to boolean variables instead of an integer value.
>>
>> This issue was detected with the help of Coccinelle.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>  drivers/macintosh/windfarm_pm72.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> This seems to be common across all those drivers. Can you please send
> one patch to fix them all to use bool:
>
>   $ git grep "started = " drivers/macintosh/
>   drivers/macintosh/therm_adt746x.c:              int started = 0;
>   drivers/macintosh/therm_adt746x.c:                      started = 1;
>   drivers/macintosh/windfarm_pm112.c:             slots_started = 1;
>   drivers/macintosh/windfarm_pm112.c:             started = 1;
>   drivers/macintosh/windfarm_pm121.c:             pm121_started = 1;
>   drivers/macintosh/windfarm_pm72.c:              started = 1;
>   drivers/macintosh/windfarm_pm81.c:              wf_smu_started = 1;
>   drivers/macintosh/windfarm_pm91.c:              wf_smu_started = 1;
>   drivers/macintosh/windfarm_rm31.c:              started = 1;
>
> cheers

Sure, no problem.

By the way, I've just found the following similar case:

--- a/drivers/macintosh/ams/ams-input.c
+++ b/drivers/macintosh/ams/ams-input.c
@@ -91,7 +91,7 @@ static int ams_input_enable(void)
  		return error;
  	}

-	joystick = 1;
+	joystick = true;

  	return 0;
  }
@@ -104,7 +104,7 @@ static void ams_input_disable(void)
  		ams_info.idev = NULL;
  	}

-	joystick = 0;
+	joystick = false;
  }

Do you want me to include them all in the same patch?

Thanks
--
Gustavo

^ permalink raw reply

* Re: [PATCH v4 0/7] SR-IOV Enablement on PowerVM
From: Russell Currey @ 2018-01-24  1:18 UTC (permalink / raw)
  To: Bryant G. Ly, benh, paulus, mpe
  Cc: seroyer, jjalvare, alex.williamson, helgaas, aik, linux-pci,
	linuxppc-dev, bodong, eli, saeedm
In-Reply-To: <20180105164552.36371-1-bryantly@linux.vnet.ibm.com>

On Fri, 2018-01-05 at 10:45 -0600, Bryant G. Ly wrote:
> This patch series will enable SR-IOV on PowerVM. A specific set of
> lids for PFW/PHYP is required. They are planned to release with
> 920 at the moment.
> 
> For IBM internal testers let me know of a system you want to test on
> and we can put on the lids required or we can provide a system to run
> the tests.
> 
> This patch depends on the three patches:
> 988fc3ba5653278a8c14d6ccf687371775930d2b
> dae7253f9f78a731755ca20c66b2d2c40b86baea
> 608c0d8804ef3ca4cda8ec6ad914e47deb283d7b
> 
> v1 - Initial Patch
> v2 - Addressed Alexey and Russell's comments
> v3 - Unify the call of .error_detected()
> v4 - Fixed subject and change log per Bjorn's comments and
>      fixed Alexey's comments

For the whole series:

Acked-by: Russell Currey <ruscur@russell.cc>

^ permalink raw reply

* Re: [PATCH v4 3/7] platforms/pseries: Set eeh_pe of EEH_PE_VF type
From: Michael Ellerman @ 2018-01-24  1:14 UTC (permalink / raw)
  To: Bryant G. Ly, benh, paulus
  Cc: seroyer, jjalvare, alex.williamson, helgaas, aik, ruscur,
	linux-pci, linuxppc-dev, bodong, eli, saeedm, Bryant G. Ly
In-Reply-To: <20180105164552.36371-4-bryantly@linux.vnet.ibm.com>

"Bryant G. Ly" <bryantly@linux.vnet.ibm.com> writes:

> To correctly use EEH code one has to make
> sure that the EEH_PE_VF is set for dynamic created
> VFs. Therefore this patch allocates an eeh_pe of
> eeh type EEH_PE_VF and associates PE with parent.
>
> Signed-off-by: Bryant G. Ly <bryantly@linux.vnet.ibm.com>
> Signed-off-by: Juan J. Alvarez <jjalvare@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/pci-bridge.h        |  5 ++++-
>  arch/powerpc/platforms/pseries/eeh_pseries.c | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 9f66ddebb799..16d70740a76f 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -211,7 +211,10 @@ struct pci_dn {
>  	unsigned int *pe_num_map;	/* PE# for the first VF PE or array */
>  	bool    m64_single_mode;	/* Use M64 BAR in Single Mode */
>  #define IODA_INVALID_M64        (-1)
> -	int     (*m64_map)[PCI_SRIOV_NUM_BARS];
> +	union {
> +		int     (*m64_map)[PCI_SRIOV_NUM_BARS]; /*Only used in powernv */
> +		int     last_allow_rc;	/* Only used in pSeries */
> +	};
>  #endif /* CONFIG_PCI_IOV */
>  	int	mps;			/* Maximum Payload Size */
>  	struct list_head child_list;

I don't see the point of using a union to save 4 bytes.

And if you look at the current layout of the struct there's actually a 4
byte hole after mps, so it doesn't actually save any space at all.

I can remove it before applying, unless there's some compelling reason
for it I'm not seeing.

cheers

^ permalink raw reply

* Re: [PATCH] drivers/macintosh: Use true for boolean value
From: Michael Ellerman @ 2018-01-24  0:47 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Benjamin Herrenschmidt
  Cc: Gustavo A. R. Silva, linuxppc-dev, linux-kernel
In-Reply-To: <20180123174349.GA19627@embeddedgus>

"Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:

> Assign true or false to boolean variables instead of an integer value.
>
> This issue was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/macintosh/windfarm_pm72.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This seems to be common across all those drivers. Can you please send
one patch to fix them all to use bool:

  $ git grep "started = " drivers/macintosh/
  drivers/macintosh/therm_adt746x.c:              int started = 0;
  drivers/macintosh/therm_adt746x.c:                      started = 1;
  drivers/macintosh/windfarm_pm112.c:             slots_started = 1;
  drivers/macintosh/windfarm_pm112.c:             started = 1;
  drivers/macintosh/windfarm_pm121.c:             pm121_started = 1;
  drivers/macintosh/windfarm_pm72.c:              started = 1;
  drivers/macintosh/windfarm_pm81.c:              wf_smu_started = 1;
  drivers/macintosh/windfarm_pm91.c:              wf_smu_started = 1;
  drivers/macintosh/windfarm_rm31.c:              started = 1;

cheers

^ permalink raw reply

* Re: [PATCH v2 1/5] powerpc/mm: Enhance 'slice' for supporting PPC32
From: Segher Boessenkool @ 2018-01-23 21:47 UTC (permalink / raw)
  To: Christophe LEROY
  Cc: Aneesh Kumar K.V, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Scott Wood, linuxppc-dev, linux-kernel
In-Reply-To: <52c000dd-1625-a205-8ad1-04376beed2ab@c-s.fr>

On Mon, Jan 22, 2018 at 08:52:53AM +0100, Christophe LEROY wrote:
> >Just make sure to declare all functions, or define it to some empty
> >thing, or #ifdeffery if you have to.  There are many options, it is
> >not hard, and if it means you have to pull code further apart that is
> >not so bad: you get cleaner, clearer code.
> 
> Ok, if I understand well, your comment applies to the following indeed, 
> so you confirm the #ifdef is necessary.

As I said, not necessary, but it might be the easiest or even the
cleanest here.  Something for you and the maintainers to fight about,
I'll stay out of it :-)

> However, my question was related to another part of the current 
> patchset, where the functions are always refined:
> 
> 
> On PPC32 we set:
> 
> +#define SLICE_LOW_SHIFT		28
> +#define SLICE_HIGH_SHIFT	0
> 
> On PPC64 we set:
> 
>  #define SLICE_LOW_SHIFT		28
>  #define SLICE_HIGH_SHIFT	40
> 
> We define:
> 
> +#define slice_bitmap_zero(dst, nbits) \
> +	do { if (nbits) bitmap_zero(dst, nbits); } while (0)
> 
> 
> We have a function with:
> {
> 	slice_bitmap_zero(ret->low_slices, SLICE_NUM_LOW);
>  	slice_bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
> }

SLICE_NUM_xx is not the same as SLICE_xx_SHIFT; I don't see how any of
those shift values give nbits == 0.

> So the question is to find the better approach. Is the above approach 
> correct, including performance wise ?

If slice_bitmap_zero is inlined (or partially inlined) it is fine.  Is it?


Segher

^ permalink raw reply

* [PATCH-next] powerpc/fsl_pci: Use PTR_ERR_OR_ZERO
From: Christopher Díaz Riveros @ 2018-01-23 20:37 UTC (permalink / raw)
  To: benh, paulus, mpe, joe, christian.koenig, tyreld, konrad.wilk,
	robh
  Cc: linuxppc-dev, linux-kernel, kernel-janitors

Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

This issue was detected by using the Coccinelle software.

Signed-off-by: Christopher Díaz Riveros <chrisadr@gentoo.org>
---
 arch/powerpc/sysdev/fsl_pci.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index cc20d2255d7f..142184635c81 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -1304,10 +1304,7 @@ static int add_err_dev(struct platform_device *pdev)
 						   pdev->resource,
 						   pdev->num_resources,
 						   &pd, sizeof(pd));
-	if (IS_ERR(errdev))
-		return PTR_ERR(errdev);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(errdev);
 }
 
 static int fsl_pci_probe(struct platform_device *pdev)
-- 
2.16.0

^ permalink raw reply related

* Re: [RFC PATCH v2 0/1] of: easier debugging for node life cycle issues
From: Frank Rowand @ 2018-01-23 19:53 UTC (permalink / raw)
  To: Michael Ellerman, Wolfram Sang
  Cc: devicetree, linux-kernel, Steven Rostedt, linux-renesas-soc,
	Wolfram Sang, Rob Herring, Geert Uytterhoeven, Tyrel Datwyler,
	linuxppc-dev
In-Reply-To: <874lncwza1.fsf@concordia.ellerman.id.au>

On 01/23/18 04:11, Michael Ellerman wrote:
> Wolfram Sang <wsa@the-dreams.de> writes:
> 
>> Hi Frank,
>>
>>> Please go back and read the thread for version 1.  Simply resubmitting a
>>> forward port is ignoring that whole conversation.
>>>
>>> There is a lot of good info in that thread.  I certainly learned stuff in it.
>>
>> Yes, I did that and learned stuff, too. My summary of the discussion was:
>>
>> - you mentioned some drawbacks you saw (like the mixture of trace output
>>   and printk output)
>> - most of them look like addressed to me? (e.g. Steven showed a way to redirect
>>   printk to trace)
>> - you posted your version (which was, however, marked as "not user friendly"
>>   even by yourself)
>> - The discussion stalled over having two approaches
>>
>> So, I thought reposting would be a good way of finding out if your
>> concerns were addressed in the discussion or not. If I overlooked
>> something, I am sorry for that. Still, my intention is to continue the
>> discussion, not to ignore it. Because as it stands, we don't have such a
>> debugging mechanism in place currently, and with people working with DT
>> overlays, I'd think it would be nice to have.
> 
> Yeah I agree with all of that, I didn't think there were really any
> concerns left outstanding. These trace points are very useful, I've
> twice added them to a kernel to debug something, so it would be great
> for them to be in mainline.
> 
> cheers
> 

Yes, I believe there are concerns outstanding.  I'll try to read through
the whole thread today to make sure I'm not missing anything.

-Frank

^ permalink raw reply

* Re: [PATCH] powerpc/ps3: remove an unneeded NULL check
From: Geoff Levand @ 2018-01-23 18:25 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linuxppc-dev, kernel-janitors
In-Reply-To: <20180123083318.GB10091@mwanda>

Hi Dan,

On 01/23/2018 12:33 AM, Dan Carpenter wrote:
> @@ -43,7 +43,7 @@ void ps3_sys_manager_register_ops(const struct ps3_sys_manager_ops *ops)
>  {
>  	BUG_ON(!ops);
>  	BUG_ON(!ops->dev);
> -	ps3_sys_manager_ops = ops ? *ops : ps3_sys_manager_ops;
> +	ps3_sys_manager_ops = *ops;

This seems to be a left over from when I was adding the modular
system-manager support.  It doesn't really make sense if you
look at how the ps3_sys_manager_ops variable is used.

I added your patch to my ps3-queue branch.  Thanks for the
contribution.

-Geoff

^ permalink raw reply

* [PATCH] drivers/macintosh: Use true for boolean value
From: Gustavo A. R. Silva @ 2018-01-23 17:43 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-kernel, Gustavo A. R. Silva

Assign true or false to boolean variables instead of an integer value.

This issue was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/macintosh/windfarm_pm72.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/macintosh/windfarm_pm72.c b/drivers/macintosh/windfarm_pm72.c
index e88cfb3..8330215 100644
--- a/drivers/macintosh/windfarm_pm72.c
+++ b/drivers/macintosh/windfarm_pm72.c
@@ -611,7 +611,7 @@ static void pm72_tick(void)
 	int i, last_failure;
 
 	if (!started) {
-		started = 1;
+		started = true;
 		printk(KERN_INFO "windfarm: CPUs control loops started.\n");
 		for (i = 0; i < nr_chips; ++i) {
 			if (cpu_setup_pid(i) < 0) {
-- 
2.7.4

^ permalink raw reply related

* [PATCH] powerpc/ps3: remove an unneeded NULL check
From: Dan Carpenter @ 2018-01-23  8:33 UTC (permalink / raw)
  To: Geoff Levand
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linuxppc-dev, kernel-janitors

Static checkers don't like the inconsistent NULL checking on "ops".
This function is only called once and "ops" isn't NULL so the check can
be removed.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/ps3/sys-manager-core.c b/drivers/ps3/sys-manager-core.c
index c429ffca1ab7..a5a6def77bb9 100644
--- a/drivers/ps3/sys-manager-core.c
+++ b/drivers/ps3/sys-manager-core.c
@@ -43,7 +43,7 @@ void ps3_sys_manager_register_ops(const struct ps3_sys_manager_ops *ops)
 {
 	BUG_ON(!ops);
 	BUG_ON(!ops->dev);
-	ps3_sys_manager_ops = ops ? *ops : ps3_sys_manager_ops;
+	ps3_sys_manager_ops = *ops;
 }
 EXPORT_SYMBOL_GPL(ps3_sys_manager_register_ops);
 

^ permalink raw reply related

* Re: [PATCH 07/13] ocxl: Add AFU interrupt support
From: Cédric Le Goater @ 2018-01-23 17:10 UTC (permalink / raw)
  To: benh, Frederic Barrat, linuxppc-dev, linux-kernel
  Cc: arnd, gregkh, mpe, andrew.donnellan, alastair
In-Reply-To: <1513656340.2743.3.camel@au1.ibm.com>

On 12/19/2017 04:05 AM, Benjamin Herrenschmidt wrote:
> On Mon, 2017-12-18 at 16:21 +0100, Frederic Barrat wrote:
>> Add user APIs through ioctl to allocate, free, and be notified of an
>> AFU interrupt.
>>
>> For opencapi, an AFU can trigger an interrupt on the host by sending a
>> specific command targeting a 64-bit object handle. On POWER9, this is
>> implemented by mapping a special page in the address space of a
>> process and a write to that page will trigger an interrupt.
> 
> We need to figure out how that plays with KVM. +Cedric..
> 
> For all those "generic xive" interrupts, whether they are used for
> OpenCAPI, plain guest IPIs, NX interrupts etc... but also for actual
> pass-through ones, we'll need a mechanism to map the trigger and ESB
> pages into qemu.
It seems feasible to use a common driver, at least for QEMU/KVM 
and OCXL, to expose the ESB pages of a range of IRQ numbers. Fred 
has already defined a user API, a set of ioctl which allocate, free 
one IRQ and also associate an IRQ with an eventfd for handling. 
The VMA is populated on demand. 

This XIVE IRQ "device", that I don't know how to name, defines 
generic IRQ sources and handlers for a given range. We would need 
a couple of properties to describe it in a device tree, 

  - "ibm,xive-lisn-ranges" for the range.
  
Anymore ? 

The current code needs some changes to distinguish the XIVE IRQ 
driver from the OCXL one, range support should be added, using a 
bitmap to track allocation I guess.

>From a OCXL perspective, the XIVE IRQ device driver would be 
instantiated from the OCXL one using an ioctl returning an fd,
like KVM does with KVM devices. User space would then alloc, free, 
associate IRQs and mmap the ESB pages to configure the OpenCAPI 
device. As for QEMU, I think we could add an extra KVM device, 
QEMU does not need the 'associate' feature though.

Such devices could theoretically be defined by the firmware for 
general purpose also, and be used through a char device. This is 
a possibility.


> We can't have a bazillion VMAs and KVM memory regions either, so we'll
> need some kind of mechanism/driver which allows for a single fairly
> large mmap'ed VMA which can then be "populated" with interrupt control
> pages.

yes. the full address range should mmapped for the IRQ range defined
for the device. access to pages not populated would return EFAULT.  
 
> The issue of course is that we can't really do a "generic" system that
> allows to map any interrupt, it's a security issue. So we need the
> interrupt "owner" to be the one allowing this. VFIO for PCI for
> example, possibly a specific VFIO variant for OpenCAPI, something else
> for guest IPIs ?
If we have defined ranges per devices, that should be enough no ?

Thanks,

C. 
 
> Food for thoughts...
> 
> Ben.
> 
>>
>> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/pnv-ocxl.h   |   3 +
>>  arch/powerpc/platforms/powernv/ocxl.c |  30 +++++
>>  drivers/misc/ocxl/afu_irq.c           | 204 ++++++++++++++++++++++++++++++++++
>>  drivers/misc/ocxl/context.c           |  40 ++++++-
>>  drivers/misc/ocxl/file.c              |  33 ++++++
>>  drivers/misc/ocxl/link.c              |  28 +++++
>>  drivers/misc/ocxl/ocxl_internal.h     |   7 ++
>>  include/uapi/misc/ocxl.h              |   9 ++
>>  8 files changed, 352 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/misc/ocxl/afu_irq.c
>>
>> diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
>> index 5a7ae7f28209..1e26f0a39500 100644
>> --- a/arch/powerpc/include/asm/pnv-ocxl.h
>> +++ b/arch/powerpc/include/asm/pnv-ocxl.h
>> @@ -37,4 +37,7 @@ extern int pnv_ocxl_spa_setup(struct pci_dev *dev, void *spa_mem, int PE_mask,
>>  extern void pnv_ocxl_spa_release(void *platform_data);
>>  extern int pnv_ocxl_spa_remove_pe(void *platform_data, int pe_handle);
>>  
>> +extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
>> +extern void pnv_ocxl_free_xive_irq(u32 irq);
>> +
>>  #endif /* _ASM_PVN_OCXL_H */
>> diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
>> index 6c79924b95c8..96cafba6aef1 100644
>> --- a/arch/powerpc/platforms/powernv/ocxl.c
>> +++ b/arch/powerpc/platforms/powernv/ocxl.c
>> @@ -9,6 +9,7 @@
>>  
>>  #include <asm/pnv-ocxl.h>
>>  #include <asm/opal.h>
>> +#include <asm/xive.h>
>>  #include <misc/ocxl-config.h>
>>  #include "pci.h"
>>  
>> @@ -487,3 +488,32 @@ int pnv_ocxl_spa_remove_pe(void *platform_data, int pe_handle)
>>  	return rc;
>>  }
>>  EXPORT_SYMBOL_GPL(pnv_ocxl_spa_remove_pe);
>> +
>> +int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr)
>> +{
>> +	__be64 flags, trigger_page;
>> +	s64 rc;
>> +	u32 hwirq;
>> +
>> +	hwirq = xive_native_alloc_irq();
>> +	if (!hwirq)
>> +		return -ENOENT;
>> +
>> +	rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page, NULL,
>> +				NULL);
>> +	if (rc || !trigger_page) {
>> +		xive_native_free_irq(hwirq);
>> +		return -ENOENT;
>> +	}
>> +	*irq = hwirq;
>> +	*trigger_addr = be64_to_cpu(trigger_page);
>> +	return 0;
>> +
>> +}
>> +EXPORT_SYMBOL_GPL(pnv_ocxl_alloc_xive_irq);
>> +
>> +void pnv_ocxl_free_xive_irq(u32 irq)
>> +{
>> +	xive_native_free_irq(irq);
>> +}
>> +EXPORT_SYMBOL_GPL(pnv_ocxl_free_xive_irq);
>> diff --git a/drivers/misc/ocxl/afu_irq.c b/drivers/misc/ocxl/afu_irq.c
>> new file mode 100644
>> index 000000000000..0b217a854837
>> --- /dev/null
>> +++ b/drivers/misc/ocxl/afu_irq.c
>> @@ -0,0 +1,204 @@
>> +/*
>> + * Copyright 2017 IBM Corp.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the License, or (at your option) any later version.
>> + */
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/eventfd.h>
>> +#include <asm/pnv-ocxl.h>
>> +#include "ocxl_internal.h"
>> +
>> +struct afu_irq {
>> +	int id;
>> +	int hw_irq;
>> +	unsigned int virq;
>> +	char *name;
>> +	u64 trigger_page;
>> +	struct eventfd_ctx *ev_ctx;
>> +};
>> +
>> +static int irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
>> +{
>> +	return (offset - ctx->afu->irq_base_offset) >> PAGE_SHIFT;
>> +}
>> +
>> +static u64 irq_id_to_offset(struct ocxl_context *ctx, int id)
>> +{
>> +	return ctx->afu->irq_base_offset + (id << PAGE_SHIFT);
>> +}
>> +
>> +static irqreturn_t afu_irq_handler(int virq, void *data)
>> +{
>> +	struct afu_irq *irq = (struct afu_irq *) data;
>> +
>> +	if (irq->ev_ctx)
>> +		eventfd_signal(irq->ev_ctx, 1);
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int setup_afu_irq(struct ocxl_context *ctx, struct afu_irq *irq)
>> +{
>> +	int rc;
>> +
>> +	irq->virq = irq_create_mapping(NULL, irq->hw_irq);
>> +	if (!irq->virq) {
>> +		pr_err("irq_create_mapping failed\n");
>> +		return -ENOMEM;
>> +	}
>> +	pr_debug("hw_irq %d mapped to virq %u\n", irq->hw_irq, irq->virq);
>> +
>> +	irq->name = kasprintf(GFP_KERNEL, "ocxl-afu-%u", irq->virq);
>> +	if (!irq->name) {
>> +		irq_dispose_mapping(irq->virq);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	rc = request_irq(irq->virq, afu_irq_handler, 0, irq->name, irq);
>> +	if (rc) {
>> +		kfree(irq->name);
>> +		irq->name = NULL;
>> +		irq_dispose_mapping(irq->virq);
>> +		pr_err("request_irq failed: %d\n", rc);
>> +		return rc;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static void release_afu_irq(struct afu_irq *irq)
>> +{
>> +	free_irq(irq->virq, irq);
>> +	irq_dispose_mapping(irq->virq);
>> +	kfree(irq->name);
>> +}
>> +
>> +int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset)
>> +{
>> +	struct afu_irq *irq;
>> +	int rc;
>> +
>> +	irq = kzalloc(sizeof(struct afu_irq), GFP_KERNEL);
>> +	if (!irq)
>> +		return -ENOMEM;
>> +
>> +	/*
>> +	 * We limit the number of afu irqs per context and per link to
>> +	 * avoid a single process or user depleting the pool of IPIs
>> +	 */
>> +
>> +	mutex_lock(&ctx->irq_lock);
>> +
>> +	irq->id = idr_alloc(&ctx->irq_idr, irq, 0, MAX_IRQ_PER_CONTEXT,
>> +			GFP_KERNEL);
>> +	if (irq->id < 0) {
>> +		rc = -ENOSPC;
>> +		goto err_unlock;
>> +	}
>> +
>> +	rc = ocxl_link_irq_alloc(ctx->afu->fn->link, &irq->hw_irq,
>> +				&irq->trigger_page);
>> +	if (rc)
>> +		goto err_idr;
>> +
>> +	rc = setup_afu_irq(ctx, irq);
>> +	if (rc)
>> +		goto err_alloc;
>> +
>> +	*irq_offset = irq_id_to_offset(ctx, irq->id);
>> +
>> +	mutex_unlock(&ctx->irq_lock);
>> +	return 0;
>> +
>> +err_alloc:
>> +	ocxl_link_free_irq(ctx->afu->fn->link, irq->hw_irq);
>> +err_idr:
>> +	idr_remove(&ctx->irq_idr, irq->id);
>> +err_unlock:
>> +	mutex_unlock(&ctx->irq_lock);
>> +	kfree(irq);
>> +	return rc;
>> +}
>> +
>> +static void afu_irq_free(struct afu_irq *irq, struct ocxl_context *ctx)
>> +{
>> +	if (ctx->mapping)
>> +		unmap_mapping_range(ctx->mapping,
>> +				irq_id_to_offset(ctx, irq->id),
>> +				1 << PAGE_SHIFT, 1);
>> +	release_afu_irq(irq);
>> +	if (irq->ev_ctx)
>> +		eventfd_ctx_put(irq->ev_ctx);
>> +	ocxl_link_free_irq(ctx->afu->fn->link, irq->hw_irq);
>> +	kfree(irq);
>> +}
>> +
>> +int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset)
>> +{
>> +	struct afu_irq *irq;
>> +	int id = irq_offset_to_id(ctx, irq_offset);
>> +
>> +	mutex_lock(&ctx->irq_lock);
>> +
>> +	irq = idr_find(&ctx->irq_idr, id);
>> +	if (!irq) {
>> +		mutex_unlock(&ctx->irq_lock);
>> +		return -EINVAL;
>> +	}
>> +	idr_remove(&ctx->irq_idr, irq->id);
>> +	afu_irq_free(irq, ctx);
>> +	mutex_unlock(&ctx->irq_lock);
>> +	return 0;
>> +}
>> +
>> +void ocxl_afu_irq_free_all(struct ocxl_context *ctx)
>> +{
>> +	struct afu_irq *irq;
>> +	int id;
>> +
>> +	mutex_lock(&ctx->irq_lock);
>> +	idr_for_each_entry(&ctx->irq_idr, irq, id)
>> +		afu_irq_free(irq, ctx);
>> +	mutex_unlock(&ctx->irq_lock);
>> +}
>> +
>> +int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset, int eventfd)
>> +{
>> +	struct afu_irq *irq;
>> +	struct eventfd_ctx *ev_ctx;
>> +	int rc = 0, id = irq_offset_to_id(ctx, irq_offset);
>> +
>> +	mutex_lock(&ctx->irq_lock);
>> +	irq = idr_find(&ctx->irq_idr, id);
>> +	if (!irq) {
>> +		rc = -EINVAL;
>> +		goto unlock;
>> +	}
>> +
>> +	ev_ctx = eventfd_ctx_fdget(eventfd);
>> +	if (IS_ERR(ev_ctx)) {
>> +		rc = -EINVAL;
>> +		goto unlock;
>> +	}
>> +
>> +	irq->ev_ctx = ev_ctx;
>> +unlock:
>> +	mutex_unlock(&ctx->irq_lock);
>> +	return rc;
>> +}
>> +
>> +u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset)
>> +{
>> +	struct afu_irq *irq;
>> +	int id = irq_offset_to_id(ctx, irq_offset);
>> +	u64 addr = 0;
>> +
>> +	mutex_lock(&ctx->irq_lock);
>> +	irq = idr_find(&ctx->irq_idr, id);
>> +	if (irq)
>> +		addr = irq->trigger_page;
>> +	mutex_unlock(&ctx->irq_lock);
>> +	return addr;
>> +}
>> diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
>> index 0bc0dd97d784..19575269ed22 100644
>> --- a/drivers/misc/ocxl/context.c
>> +++ b/drivers/misc/ocxl/context.c
>> @@ -38,6 +38,8 @@ int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
>>  	mutex_init(&ctx->mapping_lock);
>>  	init_waitqueue_head(&ctx->events_wq);
>>  	mutex_init(&ctx->xsl_error_lock);
>> +	mutex_init(&ctx->irq_lock);
>> +	idr_init(&ctx->irq_idr);
>>  	/*
>>  	 * Keep a reference on the AFU to make sure it's valid for the
>>  	 * duration of the life of the context
>> @@ -87,6 +89,19 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr)
>>  	return rc;
>>  }
>>  
>> +static int map_afu_irq(struct vm_area_struct *vma, unsigned long address,
>> +		u64 offset, struct ocxl_context *ctx)
>> +{
>> +	u64 trigger_addr;
>> +
>> +	trigger_addr = ocxl_afu_irq_get_addr(ctx, offset);
>> +	if (!trigger_addr)
>> +		return VM_FAULT_SIGBUS;
>> +
>> +	vm_insert_pfn(vma, address, trigger_addr >> PAGE_SHIFT);
>> +	return VM_FAULT_NOPAGE;
>> +}
>> +
>>  static int map_pp_mmio(struct vm_area_struct *vma, unsigned long address,
>>  		u64 offset, struct ocxl_context *ctx)
>>  {
>> @@ -125,7 +140,10 @@ static int ocxl_mmap_fault(struct vm_fault *vmf)
>>  	pr_debug("%s: pasid %d address 0x%lx offset 0x%llx\n", __func__,
>>  		ctx->pasid, vmf->address, offset);
>>  
>> -	rc = map_pp_mmio(vma, vmf->address, offset, ctx);
>> +	if (offset < ctx->afu->irq_base_offset)
>> +		rc = map_pp_mmio(vma, vmf->address, offset, ctx);
>> +	else
>> +		rc = map_afu_irq(vma, vmf->address, offset, ctx);
>>  	return rc;
>>  }
>>  
>> @@ -133,6 +151,19 @@ static const struct vm_operations_struct ocxl_vmops = {
>>  	.fault = ocxl_mmap_fault,
>>  };
>>  
>> +static int check_mmap_afu_irq(struct ocxl_context *ctx,
>> +			struct vm_area_struct *vma)
>> +{
>> +	/* only one page */
>> +	if (vma_pages(vma) != 1)
>> +		return -EINVAL;
>> +
>> +	/* check offset validty */
>> +	if (!ocxl_afu_irq_get_addr(ctx, vma->vm_pgoff << PAGE_SHIFT))
>> +		return -EINVAL;
>> +	return 0;
>> +}
>> +
>>  static int check_mmap_mmio(struct ocxl_context *ctx,
>>  			struct vm_area_struct *vma)
>>  {
>> @@ -146,7 +177,10 @@ int ocxl_context_mmap(struct ocxl_context *ctx, struct vm_area_struct *vma)
>>  {
>>  	int rc;
>>  
>> -	rc = check_mmap_mmio(ctx, vma);
>> +	if ((vma->vm_pgoff << PAGE_SHIFT) < ctx->afu->irq_base_offset)
>> +		rc = check_mmap_mmio(ctx, vma);
>> +	else
>> +		rc = check_mmap_afu_irq(ctx, vma);
>>  	if (rc)
>>  		return rc;
>>  
>> @@ -231,6 +265,8 @@ void ocxl_context_free(struct ocxl_context *ctx)
>>  	idr_remove(&ctx->afu->contexts_idr, ctx->pasid);
>>  	mutex_unlock(&ctx->afu->contexts_lock);
>>  
>> +	ocxl_afu_irq_free_all(ctx);
>> +	idr_destroy(&ctx->irq_idr);
>>  	/* reference to the AFU taken in ocxl_context_init */
>>  	ocxl_afu_put(ctx->afu);
>>  	kfree(ctx);
>> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
>> index a51386eff4f5..0a73e2c11ba6 100644
>> --- a/drivers/misc/ocxl/file.c
>> +++ b/drivers/misc/ocxl/file.c
>> @@ -110,12 +110,17 @@ static long afu_ioctl_attach(struct ocxl_context *ctx,
>>  }
>>  
>>  #define CMD_STR(x) (x == OCXL_IOCTL_ATTACH ? "ATTACH" :			\
>> +			x == OCXL_IOCTL_IRQ_ALLOC ? "IRQ_ALLOC" :	\
>> +			x == OCXL_IOCTL_IRQ_FREE ? "IRQ_FREE" :		\
>> +			x == OCXL_IOCTL_IRQ_SET_FD ? "IRQ_SET_FD" :	\
>>  			"UNKNOWN")
>>  
>>  static long afu_ioctl(struct file *file, unsigned int cmd,
>>  		unsigned long args)
>>  {
>>  	struct ocxl_context *ctx = file->private_data;
>> +	struct ocxl_ioctl_irq_fd irq_fd;
>> +	u64 irq_offset;
>>  	long rc;
>>  
>>  	pr_debug("%s for context %d, command %s\n", __func__, ctx->pasid,
>> @@ -130,6 +135,34 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
>>  				(struct ocxl_ioctl_attach __user *) args);
>>  		break;
>>  
>> +	case OCXL_IOCTL_IRQ_ALLOC:
>> +		rc = ocxl_afu_irq_alloc(ctx, &irq_offset);
>> +		if (!rc) {
>> +			rc = copy_to_user((u64 *) args, &irq_offset,
>> +					sizeof(irq_offset));
>> +			if (rc)
>> +				ocxl_afu_irq_free(ctx, irq_offset);
>> +		}
>> +		break;
>> +
>> +	case OCXL_IOCTL_IRQ_FREE:
>> +		rc = copy_from_user(&irq_offset, (u64 *) args,
>> +				sizeof(irq_offset));
>> +		if (rc)
>> +			return -EFAULT;
>> +		rc = ocxl_afu_irq_free(ctx, irq_offset);
>> +		break;
>> +
>> +	case OCXL_IOCTL_IRQ_SET_FD:
>> +		rc = copy_from_user(&irq_fd, (u64 *) args, sizeof(irq_fd));
>> +		if (rc)
>> +			return -EFAULT;
>> +		if (irq_fd.reserved)
>> +			return -EINVAL;
>> +		rc = ocxl_afu_irq_set_fd(ctx, irq_fd.irq_offset,
>> +					irq_fd.eventfd);
>> +		break;
>> +
>>  	default:
>>  		rc = -EINVAL;
>>  	}
>> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
>> index 6b184cd7d2a6..5f12564eea99 100644
>> --- a/drivers/misc/ocxl/link.c
>> +++ b/drivers/misc/ocxl/link.c
>> @@ -608,3 +608,31 @@ int ocxl_link_remove_pe(void *link_handle, int pasid)
>>  	mutex_unlock(&spa->spa_lock);
>>  	return rc;
>>  }
>> +
>> +int ocxl_link_irq_alloc(void *link_handle, int *hw_irq, u64 *trigger_addr)
>> +{
>> +	struct link *link = (struct link *) link_handle;
>> +	int rc, irq;
>> +	u64 addr;
>> +
>> +	if (atomic_dec_if_positive(&link->irq_available) < 0)
>> +		return -ENOSPC;
>> +
>> +	rc = pnv_ocxl_alloc_xive_irq(&irq, &addr);
>> +	if (rc) {
>> +		atomic_inc(&link->irq_available);
>> +		return rc;
>> +	}
>> +
>> +	*hw_irq = irq;
>> +	*trigger_addr = addr;
>> +	return 0;
>> +}
>> +
>> +void ocxl_link_free_irq(void *link_handle, int hw_irq)
>> +{
>> +	struct link *link = (struct link *) link_handle;
>> +
>> +	pnv_ocxl_free_xive_irq(hw_irq);
>> +	atomic_inc(&link->irq_available);
>> +}
>> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
>> index e07f7d523275..829369c5f004 100644
>> --- a/drivers/misc/ocxl/ocxl_internal.h
>> +++ b/drivers/misc/ocxl/ocxl_internal.h
>> @@ -197,4 +197,11 @@ extern void ocxl_context_free(struct ocxl_context *ctx);
>>  extern int ocxl_sysfs_add_afu(struct ocxl_afu *afu);
>>  extern void ocxl_sysfs_remove_afu(struct ocxl_afu *afu);
>>  
>> +extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
>> +extern int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
>> +extern void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
>> +extern int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
>> +			int eventfd);
>> +extern u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
>> +
>>  #endif /* _OCXL_INTERNAL_H_ */
>> diff --git a/include/uapi/misc/ocxl.h b/include/uapi/misc/ocxl.h
>> index 71fa387f2efd..488e75228c33 100644
>> --- a/include/uapi/misc/ocxl.h
>> +++ b/include/uapi/misc/ocxl.h
>> @@ -39,9 +39,18 @@ struct ocxl_ioctl_attach {
>>  	__u64 reserved3;
>>  };
>>  
>> +struct ocxl_ioctl_irq_fd {
>> +	__u64 irq_offset;
>> +	__s32 eventfd;
>> +	__u32 reserved;
>> +};
>> +
>>  /* ioctl numbers */
>>  #define OCXL_MAGIC 0xCA
>>  /* AFU devices */
>>  #define OCXL_IOCTL_ATTACH	_IOW(OCXL_MAGIC, 0x10, struct ocxl_ioctl_attach)
>> +#define OCXL_IOCTL_IRQ_ALLOC	_IOR(OCXL_MAGIC, 0x11, __u64)
>> +#define OCXL_IOCTL_IRQ_FREE	_IOW(OCXL_MAGIC, 0x12, __u64)
>> +#define OCXL_IOCTL_IRQ_SET_FD	_IOW(OCXL_MAGIC, 0x13, struct ocxl_ioctl_irq_fd)
>>  
>>  #endif /* _UAPI_MISC_OCXL_H */
> 

^ permalink raw reply

* Re: [PATCH v2 1/6] resource: Extend the PPC32 reserved memory hack
From: Jonathan Neuschäfer @ 2018-01-23 16:37 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Jonathan Neuschäfer, linux-kernel, Tom Lendacky,
	Brijesh Singh, devicetree, Albert Herranz, linux-gpio,
	Thomas Gleixner, Borislav Petkov, linuxppc-dev
In-Reply-To: <871sigwx41.fsf@concordia.ellerman.id.au>

[-- Attachment #1: Type: text/plain, Size: 1751 bytes --]

On Tue, Jan 23, 2018 at 11:58:06PM +1100, Michael Ellerman wrote:
> Jonathan Neuschäfer <j.neuschaefer@gmx.net> writes:
> 
> > On the Nintendo Wii, there are two ranges of physical memory, and MMIO
> > in between, but Linux on ppc32 doesn't support discontiguous memory.
> > Therefore a hack was introduced in commit c5df7f775148 ("powerpc: allow
> > ioremap within reserved memory regions") and commit de32400dd26e ("wii:
> > use both mem1 and mem2 as ram"):
> >
> >  - Treat the area from the start of the first memory area (MEM1) to the
> >    end of the second (MEM2) as one big memory area, but mark the part
> >    that doesn't belong to MEM1 or MEM2 as reserved.
> >  - Only on the Wii, allow ioremap to be used on reserved memory.
> >
> > This hack, however, doesn't account for the "resource"-based API in
> > kernel/resource.c, because __request_region performs its own checks.
> >
> > Extend the hack to kernel/resource.c, to allow more drivers to allocate
> > their MMIO regions on the Wii.
> 
> Hi Jonathan,
> 
> Sorry but I can't merge a hack like this in generic code.

Makes sense.

> Has anyone looked at adding proper discontig mem support to PPC32?

I'm not aware of any such effort.

Do you have any pointer on how to implement discontiguous memory
support? CONFIG_ARCH_SPARSEMEM_ENABLE seems relevant.

> Or can we punch a hole in the resource in the right place? Maybe from
> add_system_ram_resources() ?

Not sure. add_system_ram_resources would need the original memblock
table, which is overwritten in wii_memory_fixups, if I read the code
correctly.

If a proper solution doesn't take an overwhelming amount of work, I'd
prefer a proper solution.


Thanks,
Jonathan Neuschäfer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH 02/11] powerpc: membarrier: Skip memory barrier in switch_mm() (v7)
From: Mathieu Desnoyers @ 2018-01-23 15:57 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Thomas Gleixner
  Cc: linux-kernel, linux-api, Andy Lutomirski, Paul E . McKenney,
	Boqun Feng, Andrew Hunter, Maged Michael, Avi Kivity,
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Dave Watson, H . Peter Anvin, Andrea Parri, Russell King,
	Greg Hackmann, Will Deacon, David Sehr, Linus Torvalds, x86,
	Mathieu Desnoyers, Alan Stern, Alexander Viro, Nicholas Piggin,
	linuxppc-dev, linux-arch
In-Reply-To: <20180123155733.3404-1-mathieu.desnoyers@efficios.com>

Allow PowerPC to skip the full memory barrier in switch_mm(), and
only issue the barrier when scheduling into a task belonging to a
process that has registered to use expedited private.

Threads targeting the same VM but which belong to different thread
groups is a tricky case. It has a few consequences:

It turns out that we cannot rely on get_nr_threads(p) to count the
number of threads using a VM. We can use
(atomic_read(&mm->mm_users) == 1 && get_nr_threads(p) == 1)
instead to skip the synchronize_sched() for cases where the VM only has
a single user, and that user only has a single thread.

It also turns out that we cannot use for_each_thread() to set
thread flags in all threads using a VM, as it only iterates on the
thread group.

Therefore, test the membarrier state variable directly rather than
relying on thread flags. This means
membarrier_register_private_expedited() needs to set the
MEMBARRIER_STATE_PRIVATE_EXPEDITED flag, issue synchronize_sched(), and
only then set MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY which allows
private expedited membarrier commands to succeed.
membarrier_arch_switch_mm() now tests for the
MEMBARRIER_STATE_PRIVATE_EXPEDITED flag.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Andrew Hunter <ahh@google.com>
CC: Maged Michael <maged.michael@gmail.com>
CC: Avi Kivity <avi@scylladb.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Dave Watson <davejwatson@fb.com>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: Will Deacon <will.deacon@arm.com>
CC: Andy Lutomirski <luto@kernel.org>
CC: Ingo Molnar <mingo@redhat.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Nicholas Piggin <npiggin@gmail.com>
CC: linuxppc-dev@lists.ozlabs.org
CC: linux-arch@vger.kernel.org
---
Changes since v1:
- Use test_ti_thread_flag(next, ...) instead of test_thread_flag() in
  powerpc membarrier_arch_sched_in(), given that we want to specifically
  check the next thread state.
- Add missing ARCH_HAS_MEMBARRIER_HOOKS in Kconfig.
- Use task_thread_info() to pass thread_info from task to
  *_ti_thread_flag().

Changes since v2:
- Move membarrier_arch_sched_in() call to finish_task_switch().
- Check for NULL t->mm in membarrier_arch_fork().
- Use membarrier_sched_in() in generic code, which invokes the
  arch-specific membarrier_arch_sched_in(). This fixes allnoconfig
  build on PowerPC.
- Move asm/membarrier.h include under CONFIG_MEMBARRIER, fixing
  allnoconfig build on PowerPC.
- Build and runtime tested on PowerPC.

Changes since v3:
- Simply rely on copy_mm() to copy the membarrier_private_expedited mm
  field on fork.
- powerpc: test thread flag instead of reading
  membarrier_private_expedited in membarrier_arch_fork().
- powerpc: skip memory barrier in membarrier_arch_sched_in() if coming
  from kernel thread, since mmdrop() implies a full barrier.
- Set membarrier_private_expedited to 1 only after arch registration
  code, thus eliminating a race where concurrent commands could succeed
  when they should fail if issued concurrently with process
  registration.
- Use READ_ONCE() for membarrier_private_expedited field access in
  membarrier_private_expedited. Matches WRITE_ONCE() performed in
  process registration.

Changes since v4:
- Move powerpc hook from sched_in() to switch_mm(), based on feedback
  from Nicholas Piggin.

Changes since v5:
- Rebase on v4.14-rc6.
- Fold "Fix: membarrier: Handle CLONE_VM + !CLONE_THREAD correctly on
  powerpc (v2)"

Changes since v6:
- Rename MEMBARRIER_STATE_SWITCH_MM to MEMBARRIER_STATE_PRIVATE_EXPEDITED.
---
 MAINTAINERS                           |  1 +
 arch/powerpc/Kconfig                  |  1 +
 arch/powerpc/include/asm/membarrier.h | 26 ++++++++++++++++++++++++++
 arch/powerpc/mm/mmu_context.c         |  7 +++++++
 include/linux/sched/mm.h              | 13 ++++++++++++-
 init/Kconfig                          |  3 +++
 kernel/sched/core.c                   | 10 ----------
 kernel/sched/membarrier.c             |  8 ++++++++
 8 files changed, 58 insertions(+), 11 deletions(-)
 create mode 100644 arch/powerpc/include/asm/membarrier.h

diff --git a/MAINTAINERS b/MAINTAINERS
index e3581413420c..11ff47c28b12 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8931,6 +8931,7 @@ L:	linux-kernel@vger.kernel.org
 S:	Supported
 F:	kernel/sched/membarrier.c
 F:	include/uapi/linux/membarrier.h
+F:	arch/powerpc/include/asm/membarrier.h
 
 MEMORY MANAGEMENT
 L:	linux-mm@kvack.org
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2ed525a44734..09b02180b8a0 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -140,6 +140,7 @@ config PPC
 	select ARCH_HAS_FORTIFY_SOURCE
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_HAS_PMEM_API                if PPC64
+	select ARCH_HAS_MEMBARRIER_HOOKS
 	select ARCH_HAS_SCALED_CPUTIME		if VIRT_CPU_ACCOUNTING_NATIVE
 	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_TICK_BROADCAST		if GENERIC_CLOCKEVENTS_BROADCAST
diff --git a/arch/powerpc/include/asm/membarrier.h b/arch/powerpc/include/asm/membarrier.h
new file mode 100644
index 000000000000..98ff4f1fcf2b
--- /dev/null
+++ b/arch/powerpc/include/asm/membarrier.h
@@ -0,0 +1,26 @@
+#ifndef _ASM_POWERPC_MEMBARRIER_H
+#define _ASM_POWERPC_MEMBARRIER_H
+
+static inline void membarrier_arch_switch_mm(struct mm_struct *prev,
+					     struct mm_struct *next,
+					     struct task_struct *tsk)
+{
+	/*
+	 * Only need the full barrier when switching between processes.
+	 * Barrier when switching from kernel to userspace is not
+	 * required here, given that it is implied by mmdrop(). Barrier
+	 * when switching from userspace to kernel is not needed after
+	 * store to rq->curr.
+	 */
+	if (likely(!(atomic_read(&next->membarrier_state) &
+		     MEMBARRIER_STATE_PRIVATE_EXPEDITED) || !prev))
+		return;
+
+	/*
+	 * The membarrier system call requires a full memory barrier
+	 * after storing to rq->curr, before going back to user-space.
+	 */
+	smp_mb();
+}
+
+#endif /* _ASM_POWERPC_MEMBARRIER_H */
diff --git a/arch/powerpc/mm/mmu_context.c b/arch/powerpc/mm/mmu_context.c
index d60a62bf4fc7..0ab297c4cfad 100644
--- a/arch/powerpc/mm/mmu_context.c
+++ b/arch/powerpc/mm/mmu_context.c
@@ -12,6 +12,7 @@
 
 #include <linux/mm.h>
 #include <linux/cpu.h>
+#include <linux/sched/mm.h>
 
 #include <asm/mmu_context.h>
 
@@ -58,6 +59,10 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 		 *
 		 * On the read side the barrier is in pte_xchg(), which orders
 		 * the store to the PTE vs the load of mm_cpumask.
+		 *
+		 * This full barrier is needed by membarrier when switching
+		 * between processes after store to rq->curr, before user-space
+		 * memory accesses.
 		 */
 		smp_mb();
 
@@ -80,6 +85,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 
 	if (new_on_cpu)
 		radix_kvm_prefetch_workaround(next);
+	else
+		membarrier_arch_switch_mm(prev, next, tsk);
 
 	/*
 	 * The actual HW switching method differs between the various
diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 3d49b91b674d..1754396795f6 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -215,14 +215,25 @@ static inline void memalloc_noreclaim_restore(unsigned int flags)
 #ifdef CONFIG_MEMBARRIER
 enum {
 	MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY	= (1U << 0),
-	MEMBARRIER_STATE_SWITCH_MM			= (1U << 1),
+	MEMBARRIER_STATE_PRIVATE_EXPEDITED		= (1U << 1),
 };
 
+#ifdef CONFIG_ARCH_HAS_MEMBARRIER_HOOKS
+#include <asm/membarrier.h>
+#endif
+
 static inline void membarrier_execve(struct task_struct *t)
 {
 	atomic_set(&t->mm->membarrier_state, 0);
 }
 #else
+#ifdef CONFIG_ARCH_HAS_MEMBARRIER_HOOKS
+static inline void membarrier_arch_switch_mm(struct mm_struct *prev,
+					     struct mm_struct *next,
+					     struct task_struct *tsk)
+{
+}
+#endif
 static inline void membarrier_execve(struct task_struct *t)
 {
 }
diff --git a/init/Kconfig b/init/Kconfig
index a9a2e2c86671..2d118b6adee2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1412,6 +1412,9 @@ config USERFAULTFD
 	  Enable the userfaultfd() system call that allows to intercept and
 	  handle page faults in userland.
 
+config ARCH_HAS_MEMBARRIER_HOOKS
+	bool
+
 config EMBEDDED
 	bool "Embedded system"
 	option allnoconfig_y
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a7bf32aabfda..c7e06dfa804b 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2653,16 +2653,6 @@ static struct rq *finish_task_switch(struct task_struct *prev)
 	prev_state = prev->state;
 	vtime_task_switch(prev);
 	perf_event_task_sched_in(prev, current);
-	/*
-	 * The membarrier system call requires a full memory barrier
-	 * after storing to rq->curr, before going back to user-space.
-	 *
-	 * TODO: This smp_mb__after_unlock_lock can go away if PPC end
-	 * up adding a full barrier to switch_mm(), or we should figure
-	 * out if a smp_mb__after_unlock_lock is really the proper API
-	 * to use.
-	 */
-	smp_mb__after_unlock_lock();
 	finish_lock_switch(rq, prev);
 	finish_arch_post_lock_switch();
 
diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c
index 9bcbacba82a8..678577267a9a 100644
--- a/kernel/sched/membarrier.c
+++ b/kernel/sched/membarrier.c
@@ -118,6 +118,14 @@ static void membarrier_register_private_expedited(void)
 	if (atomic_read(&mm->membarrier_state)
 			& MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY)
 		return;
+	atomic_or(MEMBARRIER_STATE_PRIVATE_EXPEDITED, &mm->membarrier_state);
+	if (!(atomic_read(&mm->mm_users) == 1 && get_nr_threads(p) == 1)) {
+		/*
+		 * Ensure all future scheduler executions will observe the
+		 * new thread flag state for this process.
+		 */
+		synchronize_sched();
+	}
 	atomic_or(MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY,
 			&mm->membarrier_state);
 }
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH] powerpc: pseries: use irq_of_parse_and_map helper
From: Rob Herring @ 2018-01-23 14:11 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linux-kernel@vger.kernel.org, Benjamin Herrenschmidt,
	Paul Mackerras, linuxppc-dev
In-Reply-To: <878tcpoyku.fsf@concordia.ellerman.id.au>

On Tue, Jan 23, 2018 at 12:53 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> Instead of calling both of_irq_parse_one and irq_create_of_mapping, call
>> of_irq_parse_and_map instead which does the same thing. This gets us closer
>> to making the former 2 functions static.
>>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>>  arch/powerpc/platforms/pseries/event_sources.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> Sorry NAK, this doesn't boot.
>
>> diff --git a/arch/powerpc/platforms/pseries/event_sources.c b/arch/powerpc/platforms/pseries/event_sources.c
>> index 6eeb0d4bab61..b0d8c146fe7b 100644
>> --- a/arch/powerpc/platforms/pseries/event_sources.c
>> +++ b/arch/powerpc/platforms/pseries/event_sources.c
>> @@ -16,7 +16,8 @@
>>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
>>   */
>>
>> -#include <asm/prom.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/of_irq.h>
>>
>>  #include "pseries.h"
>>
>> @@ -25,15 +26,11 @@ void request_event_sources_irqs(struct device_node *np,
>>                               const char *name)
>>  {
>>       int i, index, count = 0;
>> -     struct of_phandle_args oirq;
>>       unsigned int virqs[16];
>>
>>       /* First try to do a proper OF tree parsing */
>> -     for (index = 0; of_irq_parse_one(np, index, &oirq) == 0;
>> -          index++) {
>> -             if (count > 15)
>> -                     break;
>> -             virqs[count] = irq_create_of_mapping(&oirq);
>> +     for (index = 0; count < 16; index++) {
>> +             virqs[count] = irq_of_parse_and_map(np, index);
>>               if (!virqs[count]) {
>>                       pr_err("event-sources: Unable to allocate "
>>                              "interrupt number for %pOF\n",
>
>                                np);
>                         WARN_ON(1);
>                 } else {
>                         count++;
>                 }
>         }
>
>
> Which is an infinite loop if we have less than 16 irqs, and spews the
> warning continuously.
>
> Are you trying to remove the low-level routines or is this just a
> cleanup?

The former, but I'm not sure that will happen. There's a handful of
others left, but they aren't simply a call to of_irq_parse_one and
then irq_create_of_mapping.

> The patch below works, it loses the error handling if the interrupts
> property is corrupt/empty, but that's probably overly paranoid anyway.

Not quite. Previously, it was silent if parsing failed. Only the
mapping would give an error which would mean the interrupt parent had
some error.

Actually, we could use of_irq_get here to preserve the error handling.
It will return error codes from parsing, 0 on mapping failure, or the
Linux irq number. It adds an irq_find_host call for deferred probe,
but that should be harmless. I'll respin it.

Rob

^ permalink raw reply

* [PATCH 1/2] powerpc/powernv: Fix trailing semicolon
From: Luis de Bethencourt @ 2018-01-23 13:29 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joe Perches, Paul Mackerras, Benjamin Herrenschmidt,
	Michael Ellerman, Alexey Kardashevskiy, Russell Currey,
	David Gibson, Alistair Popple, Rob Herring, kvm-ppc, linuxppc-dev,
	Luis de Bethencourt

The trailing semicolon is an empty statement that does no operation.
Removing it since it doesn't do anything.

Signed-off-by: Luis de Bethencourt <luisbg@kernel.org>
---

Hi,

After fixing the same thing in drivers/staging/rtl8723bs/, Joe Perches
suggested I fix it treewide [0].

Best regards 
Luis


[0] http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115410.html
[1] http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115390.html

 arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index f6f0c5717e08..101981cc75ac 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1840,7 +1840,7 @@ static int pnv_pci_ioda_dma_set_mask(struct pci_dev *pdev, u64 dma_mask)
 	s64 rc;
 
 	if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
-		return -ENODEV;;
+		return -ENODEV;
 
 	pe = &phb->ioda.pe_array[pdn->pe_number];
 	if (pe->tce_bypass_enabled) {
-- 
2.15.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox