LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] powerpc/64: Fix build failure with GCC 8.1
From: Michael Ellerman @ 2018-05-31  5:54 UTC (permalink / raw)
  To: Christophe LEROY, Geert Uytterhoeven
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Linux Kernel Mailing List,
	linuxppc-dev, Geoff Levand
In-Reply-To: <3a438dd8-49c3-ad39-e2a1-040e0ce67279@c-s.fr>

Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 29/05/2018 =C3=A0 11:05, Geert Uytterhoeven a =C3=A9crit=C2=A0:
>> Hi Christophe,
>> On Tue, May 29, 2018 at 10:56 AM, Christophe LEROY
>> <christophe.leroy@c-s.fr> wrote:
>>> Le 29/05/2018 =C3=A0 09:47, Geert Uytterhoeven a =C3=A9crit :
>>>> On Tue, May 29, 2018 at 8:03 AM, Christophe Leroy
>>>>> --- a/arch/powerpc/kernel/nvram_64.c
>>>>> +++ b/arch/powerpc/kernel/nvram_64.c
>>>>> @@ -1039,7 +1039,7 @@ loff_t __init nvram_create_partition(const char
>>>>> *name, int sig,
>>>>>           new_part->index =3D free_part->index;
>>>>>           new_part->header.signature =3D sig;
>>>>>           new_part->header.length =3D size;
>>>>> -       strncpy(new_part->header.name, name, 12);
>>>>> +       memcpy(new_part->header.name, name, strnlen(name,
>>>>> sizeof(new_part->header.name)));
>>>>
>>>>
>>>> The comment for nvram_header.lgnth says:
>>>>
>>>>           /* Terminating null required only for names < 12 chars. */
>>>>
>>>> This will not terminate the string with a zero (the struct is
>>>> allocated with kmalloc).
>>>> So the original code is correct, the new one isn't.
>>>
>>> Right, then I have to first zeroize the destination.
>>=20
>> Using kzalloc() instead of kmalloc() will do.
>>=20
>> Still, papering around these warnings seems to obscure things, IMHO.
>> And it increases code size, as you had to add a call to strnlen().


The right fix is to not try and mirror the on-device structure in the
kernel struct. We should just use a proper NULL terminated string, which
would avoid the need to explicitly do strncmp(.., .., 12) in the code
and be less bug prone in general.

The only place where we should need to worry about the 12 byte buffer is
in nvram_write_header().

Anyway that's a bigger change, so I'll take this for now with kzalloc().

cheers

^ permalink raw reply

* Re: [PATCH v2] powerpc/64: Fix build failure with GCC 8.1
From: Christophe LEROY @ 2018-05-31  5:57 UTC (permalink / raw)
  To: Michael Ellerman, Geert Uytterhoeven
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Linux Kernel Mailing List,
	linuxppc-dev, Geoff Levand
In-Reply-To: <87efhsz7xz.fsf@concordia.ellerman.id.au>



Le 31/05/2018 à 07:54, Michael Ellerman a écrit :
> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>> Le 29/05/2018 à 11:05, Geert Uytterhoeven a écrit :
>>> Hi Christophe,
>>> On Tue, May 29, 2018 at 10:56 AM, Christophe LEROY
>>> <christophe.leroy@c-s.fr> wrote:
>>>> Le 29/05/2018 à 09:47, Geert Uytterhoeven a écrit :
>>>>> On Tue, May 29, 2018 at 8:03 AM, Christophe Leroy
>>>>>> --- a/arch/powerpc/kernel/nvram_64.c
>>>>>> +++ b/arch/powerpc/kernel/nvram_64.c
>>>>>> @@ -1039,7 +1039,7 @@ loff_t __init nvram_create_partition(const char
>>>>>> *name, int sig,
>>>>>>            new_part->index = free_part->index;
>>>>>>            new_part->header.signature = sig;
>>>>>>            new_part->header.length = size;
>>>>>> -       strncpy(new_part->header.name, name, 12);
>>>>>> +       memcpy(new_part->header.name, name, strnlen(name,
>>>>>> sizeof(new_part->header.name)));
>>>>>
>>>>>
>>>>> The comment for nvram_header.lgnth says:
>>>>>
>>>>>            /* Terminating null required only for names < 12 chars. */
>>>>>
>>>>> This will not terminate the string with a zero (the struct is
>>>>> allocated with kmalloc).
>>>>> So the original code is correct, the new one isn't.
>>>>
>>>> Right, then I have to first zeroize the destination.
>>>
>>> Using kzalloc() instead of kmalloc() will do.
>>>
>>> Still, papering around these warnings seems to obscure things, IMHO.
>>> And it increases code size, as you had to add a call to strnlen().
> 
> 
> The right fix is to not try and mirror the on-device structure in the
> kernel struct. We should just use a proper NULL terminated string, which
> would avoid the need to explicitly do strncmp(.., .., 12) in the code
> and be less bug prone in general.
> 
> The only place where we should need to worry about the 12 byte buffer is
> in nvram_write_header().
> 
> Anyway that's a bigger change, so I'll take this for now with kzalloc().

Thanks. You take it as is and add the kzalloc() or you expect a v3 from 
me with the kzalloc()

Christophe

^ permalink raw reply

* [PATCH 4.14 1/4] powerpc/mm/slice: Remove intermediate bitmap copy
From: Christophe Leroy @ 2018-05-31  8:54 UTC (permalink / raw)
  To: stable, Greg Kroah-Hartman; +Cc: linux-kernel, linuxppc-dev

[ Upstream commit 326691ad4f179e6edc7eb1271e618dd673e4736d ]

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>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 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 a4f93699194b..b79897bb89e3 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -379,21 +379,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

* [PATCH 4.14 2/4] powerpc/mm/slice: create header files dedicated to slices
From: Christophe Leroy @ 2018-05-31  8:54 UTC (permalink / raw)
  To: stable, Greg Kroah-Hartman; +Cc: linux-kernel, linuxppc-dev
In-Reply-To: <170bd2b5b5cb1e474599165e8f21f121225a2df9.1527755908.git.christophe.leroy@c-s.fr>

[ Upstream commit a3286f05bc5a5bc7fc73a9783ec89de78fcd07f8 ]

In preparation for the following patch which will enhance 'slices'
for supporting PPC32 in order to fix an issue on hugepages on 8xx,
this patch takes out of page*.h all bits related to 'slices' and put
them into newly created slice.h header files.
While common parts go into asm/slice.h, subarch specific
parts go into respective books3s/64/slice.c and nohash/64/slice.c
'slices'

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/book3s/64/slice.h | 27 ++++++++++++++
 arch/powerpc/include/asm/nohash/64/slice.h | 12 ++++++
 arch/powerpc/include/asm/page.h            |  1 +
 arch/powerpc/include/asm/page_64.h         | 59 ------------------------------
 arch/powerpc/include/asm/slice.h           | 40 ++++++++++++++++++++
 5 files changed, 80 insertions(+), 59 deletions(-)
 create mode 100644 arch/powerpc/include/asm/book3s/64/slice.h
 create mode 100644 arch/powerpc/include/asm/nohash/64/slice.h
 create mode 100644 arch/powerpc/include/asm/slice.h

diff --git a/arch/powerpc/include/asm/book3s/64/slice.h b/arch/powerpc/include/asm/book3s/64/slice.h
new file mode 100644
index 000000000000..db0dedab65ee
--- /dev/null
+++ b/arch/powerpc/include/asm/book3s/64/slice.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_BOOK3S_64_SLICE_H
+#define _ASM_POWERPC_BOOK3S_64_SLICE_H
+
+#ifdef CONFIG_PPC_MM_SLICES
+
+#define SLICE_LOW_SHIFT		28
+#define SLICE_LOW_TOP		(0x100000000ul)
+#define SLICE_NUM_LOW		(SLICE_LOW_TOP >> SLICE_LOW_SHIFT)
+#define GET_LOW_SLICE_INDEX(addr)	((addr) >> SLICE_LOW_SHIFT)
+
+#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)
+
+#else /* CONFIG_PPC_MM_SLICES */
+
+#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)
+
+#endif /* CONFIG_PPC_MM_SLICES */
+
+#endif /* _ASM_POWERPC_BOOK3S_64_SLICE_H */
diff --git a/arch/powerpc/include/asm/nohash/64/slice.h b/arch/powerpc/include/asm/nohash/64/slice.h
new file mode 100644
index 000000000000..ad0d6e3cc1c5
--- /dev/null
+++ b/arch/powerpc/include/asm/nohash/64/slice.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_NOHASH_64_SLICE_H
+#define _ASM_POWERPC_NOHASH_64_SLICE_H
+
+#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 /* _ASM_POWERPC_NOHASH_64_SLICE_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 c4d9654bd637..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_STD_MMU_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_STD_MMU_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_STD_MMU_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..17c5a5d8c418
--- /dev/null
+++ b/arch/powerpc/include/asm/slice.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_SLICE_H
+#define _ASM_POWERPC_SLICE_H
+
+#ifdef CONFIG_PPC_BOOK3S_64
+#include <asm/book3s/64/slice.h>
+#else
+#include <asm/nohash/64/slice.h>
+#endif
+
+#ifdef CONFIG_PPC_MM_SLICES
+
+#ifdef CONFIG_HUGETLB_PAGE
+#define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
+#endif
+#define HAVE_ARCH_UNMAPPED_AREA
+#define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
+
+#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);
+#endif /* __ASSEMBLY__ */
+
+#else /* CONFIG_PPC_MM_SLICES */
+
+#define slice_set_range_psize(mm, start, len, psize)	\
+	slice_set_user_psize((mm), (psize))
+#endif /* CONFIG_PPC_MM_SLICES */
+
+#endif /* _ASM_POWERPC_SLICE_H */
-- 
2.13.3

^ permalink raw reply related

* [PATCH 4.14 3/4] powerpc/mm/slice: Enhance for supporting PPC32
From: Christophe Leroy @ 2018-05-31  8:54 UTC (permalink / raw)
  To: stable, Greg Kroah-Hartman; +Cc: linux-kernel, linuxppc-dev
In-Reply-To: <170bd2b5b5cb1e474599165e8f21f121225a2df9.1527755908.git.christophe.leroy@c-s.fr>

[ Upstream commit db3a528db41caaa6dfd4c64e9f5efb1c81a80467 ]

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.

The high slices use bitmaps. As bitmap functions are not prepared to
handle bitmaps of size 0, this patch ensures that bitmap functions
are called only when SLICE_NUM_HIGH is not nul.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/nohash/32/slice.h | 18 +++++++++++++++
 arch/powerpc/include/asm/slice.h           |  4 +++-
 arch/powerpc/mm/slice.c                    | 37 +++++++++++++++++++++++-------
 3 files changed, 50 insertions(+), 9 deletions(-)
 create mode 100644 arch/powerpc/include/asm/nohash/32/slice.h

diff --git a/arch/powerpc/include/asm/nohash/32/slice.h b/arch/powerpc/include/asm/nohash/32/slice.h
new file mode 100644
index 000000000000..95d532e18092
--- /dev/null
+++ b/arch/powerpc/include/asm/nohash/32/slice.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_NOHASH_32_SLICE_H
+#define _ASM_POWERPC_NOHASH_32_SLICE_H
+
+#ifdef CONFIG_PPC_MM_SLICES
+
+#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)
+
+#define SLICE_HIGH_SHIFT	0
+#define SLICE_NUM_HIGH		0ul
+#define GET_HIGH_SLICE_INDEX(addr)	(addr & 0)
+
+#endif /* CONFIG_PPC_MM_SLICES */
+
+#endif /* _ASM_POWERPC_NOHASH_32_SLICE_H */
diff --git a/arch/powerpc/include/asm/slice.h b/arch/powerpc/include/asm/slice.h
index 17c5a5d8c418..172711fadb1c 100644
--- a/arch/powerpc/include/asm/slice.h
+++ b/arch/powerpc/include/asm/slice.h
@@ -4,8 +4,10 @@
 
 #ifdef CONFIG_PPC_BOOK3S_64
 #include <asm/book3s/64/slice.h>
-#else
+#elif defined(CONFIG_PPC64)
 #include <asm/nohash/64/slice.h>
+#elif defined(CONFIG_PPC_MMU_NOHASH)
+#include <asm/nohash/32/slice.h>
 #endif
 
 #ifdef CONFIG_PPC_MM_SLICES
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index b79897bb89e3..8baaa6c6f21c 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -73,10 +73,12 @@ 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);
+	if (SLICE_NUM_HIGH)
+		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));
@@ -113,11 +115,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);
 }
@@ -127,7 +131,8 @@ 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);
+	if (SLICE_NUM_HIGH)
+		bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
 
 	for (i = 0; i < SLICE_NUM_LOW; i++)
 		if (!slice_low_has_vma(mm, i))
@@ -149,7 +154,8 @@ 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);
+	if (SLICE_NUM_HIGH)
+		bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
 
 	lpsizes = mm->context.low_slices_psize;
 	for (i = 0; i < SLICE_NUM_LOW; i++)
@@ -171,6 +177,10 @@ static int slice_check_fit(struct mm_struct *mm,
 	DECLARE_BITMAP(result, SLICE_NUM_HIGH);
 	unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.addr_limit);
 
+	if (!SLICE_NUM_HIGH)
+		return (mask.low_slices & available.low_slices) ==
+		       mask.low_slices;
+
 	bitmap_and(result, mask.high_slices,
 		   available.high_slices, slice_count);
 
@@ -180,6 +190,7 @@ static int slice_check_fit(struct mm_struct *mm,
 
 static void slice_flush_segments(void *parm)
 {
+#ifdef CONFIG_PPC64
 	struct mm_struct *mm = parm;
 	unsigned long flags;
 
@@ -191,6 +202,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)
@@ -380,6 +392,8 @@ 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;
+	if (!SLICE_NUM_HIGH)
+		return;
 	bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
 		  SLICE_NUM_HIGH);
 }
@@ -388,6 +402,8 @@ static inline void slice_andnot_mask(struct slice_mask *dst, struct slice_mask *
 {
 	dst->low_slices &= ~src->low_slices;
 
+	if (!SLICE_NUM_HIGH)
+		return;
 	bitmap_andnot(dst->high_slices, dst->high_slices, src->high_slices,
 		      SLICE_NUM_HIGH);
 }
@@ -437,14 +453,17 @@ 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);
 
 	/* silence stupid warning */;
 	potential_mask.low_slices = 0;
-	bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
 
 	compat_mask.low_slices = 0;
-	bitmap_zero(compat_mask.high_slices, SLICE_NUM_HIGH);
+
+	if (SLICE_NUM_HIGH) {
+		bitmap_zero(mask.high_slices, SLICE_NUM_HIGH);
+		bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
+		bitmap_zero(compat_mask.high_slices, SLICE_NUM_HIGH);
+	}
 
 	/* Sanity checks */
 	BUG_ON(mm->task_size == 0);
@@ -582,7 +601,9 @@ 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_NUM_HIGH &&
+	     !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 4.14 4/4] powerpc/mm/slice: Fix hugepage allocation at hint address on 8xx
From: Christophe Leroy @ 2018-05-31  8:54 UTC (permalink / raw)
  To: stable, Greg Kroah-Hartman; +Cc: linux-kernel, linuxppc-dev
In-Reply-To: <170bd2b5b5cb1e474599165e8f21f121225a2df9.1527755908.git.christophe.leroy@c-s.fr>

[ Upstream commit aa0ab02ba992eb956934b21373e0138211486ddd ]

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>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 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..1325e5b5f680 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 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 b4fcb54b9686..008447664643 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -915,6 +915,8 @@ void __init setup_arch(char **cmdline_p)
 #ifdef CONFIG_PPC_MM_SLICES
 #ifdef CONFIG_PPC64
 	init_mm.context.addr_limit = DEFAULT_MAP_WINDOW_USER64;
+#elif defined(CONFIG_PPC_8xx)
+	init_mm.context.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 1571a498a33f..4c9e5f9c7a44 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -552,9 +552,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..e2b28b3a512e 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.addr_limit)
+		mm->context.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 a78f255111f2..3ce376b42330 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -325,6 +325,7 @@ config PPC_BOOK3E_MMU
 config PPC_MM_SLICES
 	bool
 	default y if PPC_STD_MMU_64
+	default y if PPC_8xx && HUGETLB_PAGE
 	default n
 
 config PPC_HAVE_PMU_SUPPORT
-- 
2.13.3

^ permalink raw reply related

* powerpc/powernv: copy/paste - Mask XERS0 bit in CR
From: Haren Myneni @ 2018-05-31  4:29 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, sukadev

    
NX can set 3rd bit in CR register for XER[SO] (Summation overflow)
which is not related to paste request. The current paste function
returns failure for the successful request when this bit is set.
So mask this bit and check the proper return status.

Fixes: 2392c8c8c045 ("powerpc/powernv/vas: Define copy/paste interfaces")
Cc: stable@vger.kernel.org # v4.14+    
Signed-off-by: Haren Myneni <haren@us.ibm.com>

diff --git a/arch/powerpc/platforms/powernv/copy-paste.h b/arch/powerpc/platforms/powernv/copy-paste.h
index c9a5036..82392e3 100644
--- a/arch/powerpc/platforms/powernv/copy-paste.h
+++ b/arch/powerpc/platforms/powernv/copy-paste.h
@@ -9,7 +9,8 @@
 #include <asm/ppc-opcode.h>
 
 #define CR0_SHIFT	28
-#define CR0_MASK	0xF
+#define CR0_MASK	0xE /* 3rd bit undefined or set for XER[SO] */
+
 /*
  * Copy/paste instructions:
  *

^ permalink raw reply related

* Re: [RFC PATCH 0/8] Remove unneccessary included headers
From: Michael Ellerman @ 2018-05-31  9:55 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <cover.1527697995.git.christophe.leroy@c-s.fr>

Christophe Leroy <christophe.leroy@c-s.fr> writes:

> The purpose of this serie is to limit the number of includes to
> only the necessary ones in order to reduce the number of files
> recompiled everytime a header file is modified.
>
> This is the start of the work, please provide feedback if any so
> that I don't go in the wrong direction.

Thanks for starting on this.

There's a few build errors, summary below.

  http://kisskb.ellerman.id.au/kisskb/head/14031/

cheers

arch/powerpc/kernel/machine_kexec.c:140:26: error: 'KDUMP_KERNELBASE' undeclared (first use in this function):
  corenet32_smp_defconfig powerpc
  corenet32_smp_defconfig powerpc-5.3
  g5_defconfig powerpc
  g5_defconfig powerpc-5.3
  gamecube_defconfig powerpc
  gamecube_defconfig powerpc-5.3
  maple_defconfig powerpc
  maple_defconfig powerpc-5.3
  mpc85xx_defconfig powerpc
  mpc85xx_defconfig powerpc-5.3
  mpc85xx_smp_defconfig powerpc
  mpc85xx_smp_defconfig powerpc-5.3
  mpc86xx_defconfig powerpc
  mpc86xx_defconfig powerpc-5.3
  powernv_defconfig+NO_NUMA ppc64le
  powernv_defconfig+NO_PERF ppc64le
  powernv_defconfig+NO_RADIX ppc64le
  powernv_defconfig+STRICT_RWX ppc64le
  powernv_defconfig+THIN ppc64le
  powerpc-randconfig powerpc-5.3
  ppc64e_defconfig+KEXEC powerpc
  ppc64e_defconfig+KEXEC powerpc-5.3
  ps3_defconfig powerpc
  ps3_defconfig powerpc-5.3
  pseries_defconfig powerpc
  pseries_defconfig powerpc-5.3
  pseries_defconfig+NO_MEMORY_HOTPLUG powerpc
  pseries_defconfig+NO_MEMORY_HOTPLUG powerpc-5.3
  pseries_defconfig+NO_MEMORY_HOTREMOVE powerpc
  pseries_defconfig+NO_SPLPAR powerpc
  pseries_defconfig+NO_SPLPAR powerpc-5.3
  pseries_le_defconfig ppc64le
  pseries_le_defconfig+NO_NUMA ppc64le
  pseries_le_defconfig+NO_SPLPAR ppc64le
  skiroot_defconfig ppc64le
  wii_defconfig powerpc
  wii_defconfig powerpc-5.3

arch/powerpc/include/asm/cputable.h:146:23: error: implicit declaration of function 'ASM_CONST' [-Werror=implicit-function-declaration]:
  amigaone_defconfig powerpc-5.3
  corenet_basic_defconfig powerpc-5.3
  holly_defconfig powerpc-5.3
  mpc85xx_basic_defconfig powerpc-5.3
  pmac32_defconfig powerpc-5.3
  pmac32_defconfig+KVM powerpc-5.3
  pmac32_defconfig+kexec powerpc-5.3

arch/powerpc/include/asm/cputable.h:538:6: error: enumerator value for 'CPU_FTRS_POSSIBLE' is not an integer constant:
  44x/akebono_defconfig powerpc
  44x/akebono_defconfig powerpc-5.3
  44x/currituck_defconfig powerpc
  44x/currituck_defconfig powerpc-5.3
  amigaone_defconfig powerpc
  amigaone_defconfig powerpc-5.3
  chrp32_defconfig powerpc
  chrp32_defconfig powerpc-5.3
  corenet_basic_defconfig powerpc
  corenet_basic_defconfig powerpc-5.3
  holly_defconfig powerpc
  holly_defconfig powerpc-5.3
  mpc85xx_basic_defconfig powerpc
  mpc85xx_basic_defconfig powerpc-5.3
  pmac32_defconfig powerpc
  pmac32_defconfig powerpc-5.3
  pmac32_defconfig+KVM powerpc
  pmac32_defconfig+KVM powerpc-5.3
  pmac32_defconfig+SMP powerpc
  pmac32_defconfig+SMP powerpc-5.3
  pmac32_defconfig+kexec powerpc
  pmac32_defconfig+kexec powerpc-5.3
  ppc6xx_defconfig powerpc
  ppc6xx_defconfig powerpc-5.3

arch/powerpc/include/asm/cputable.h:614:6: error: enumerator value for 'CPU_FTRS_ALWAYS' is not an integer constant:
  44x/akebono_defconfig powerpc
  44x/akebono_defconfig powerpc-5.3
  44x/currituck_defconfig powerpc
  44x/currituck_defconfig powerpc-5.3
  amigaone_defconfig powerpc
  amigaone_defconfig powerpc-5.3
  chrp32_defconfig powerpc
  chrp32_defconfig powerpc-5.3
  corenet_basic_defconfig powerpc
  corenet_basic_defconfig powerpc-5.3
  holly_defconfig powerpc
  holly_defconfig powerpc-5.3
  mpc85xx_basic_defconfig powerpc
  mpc85xx_basic_defconfig powerpc-5.3
  pmac32_defconfig powerpc
  pmac32_defconfig powerpc-5.3
  pmac32_defconfig+KVM powerpc
  pmac32_defconfig+KVM powerpc-5.3
  pmac32_defconfig+SMP powerpc
  pmac32_defconfig+SMP powerpc-5.3
  pmac32_defconfig+kexec powerpc
  pmac32_defconfig+kexec powerpc-5.3
  ppc6xx_defconfig powerpc
  ppc6xx_defconfig powerpc-5.3

arch/powerpc/include/asm/cputable.h:505:6: error: implicit declaration of function 'ASM_CONST' [-Werror=implicit-function-declaration]:
  amigaone_defconfig powerpc
  chrp32_defconfig powerpc
  corenet_basic_defconfig powerpc
  holly_defconfig powerpc
  mpc85xx_basic_defconfig powerpc
  pmac32_defconfig powerpc
  pmac32_defconfig+KVM powerpc
  pmac32_defconfig+SMP powerpc
  pmac32_defconfig+kexec powerpc
  ppc6xx_defconfig powerpc

arch/powerpc/kernel/machine_kexec.c:155:22: error: 'KDUMP_KERNELBASE' undeclared (first use in this function):
  44x/fsp2_defconfig powerpc-5.3

arch/powerpc/include/asm/cputable.h:158:32: error: implicit declaration of function 'ASM_CONST' [-Werror=implicit-function-declaration]:
  44x/akebono_defconfig powerpc-5.3
  44x/currituck_defconfig powerpc-5.3
  chrp32_defconfig powerpc-5.3
  pmac32_defconfig+SMP powerpc-5.3
  ppc6xx_defconfig powerpc-5.3

arch/powerpc/include/asm/cputable.h:515:6: error: implicit declaration of function 'ASM_CONST' [-Werror=implicit-function-declaration]:
  44x/akebono_defconfig powerpc
  44x/currituck_defconfig powerpc

^ permalink raw reply

* Re: [PATCH 1/2] error-injection: Simplify arch specific helpers
From: Naveen N. Rao @ 2018-05-31 10:09 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Josef Bacik, linux-kernel, linuxppc-dev, Ingo Molnar,
	Michael Ellerman
In-Reply-To: <20180530174156.cb1799687fb834f26fe570a9@kernel.org>

Masami Hiramatsu wrote:
> On Tue, 29 May 2018 18:06:02 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>=20
>> We already have an arch-independent way to set the instruction pointer
>> with instruction_pointer_set(). Using this allows us to get rid of the
>> need for override_function_with_return() that each architecture has to
>> implement.
>>=20
>> Furthermore, just_return_func() only has to encode arch-specific
>> assembly instructions to return from a function. Introduce a macro
>> ARCH_FUNC_RET to provide the arch-specific instruction and move over
>> just_return_func() to generic code.
>>=20
>> With these changes, architectures that already support kprobes, only
>> just need to ensure they provide regs_set_return_value(), GET_IP() (for
>> instruction_pointer_set()), and ARCH_FUNC_RET to support error
>> injection.
>=20
> Nice! the code basically good to me. Just one comment, ARCH_FUNC_RET soun=
ds
> like a function. Maybe ARCH_RETURN_INSTRUCTION will be better name, isn't=
 it? :)

Sure -- I thought of writing ARCH_FUNCTION_RETURN, but felt that was too=20
verbose. How about ARCH_FUNC_RET_INST?

Thanks for the review,
Naveen

=

^ permalink raw reply

* Re: [PATCH 2/2] powerpc: Add support for function error injection
From: Naveen N. Rao @ 2018-05-31 10:11 UTC (permalink / raw)
  To: Josef Bacik, Masami Hiramatsu, Ingo Molnar, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <87h8mozajv.fsf@concordia.ellerman.id.au>

Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>> ...
>=20
> A change log is always nice even if it's short :)

I tried, but really couldn't come up with anything more to write. I'll=20
try harder for v2 :)

>=20
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/Kconfig                       | 1 +
>>  arch/powerpc/include/asm/error-injection.h | 9 +++++++++
>>  arch/powerpc/include/asm/ptrace.h          | 5 +++++
>>  3 files changed, 15 insertions(+)
>>  create mode 100644 arch/powerpc/include/asm/error-injection.h
>=20
> This looks fine to me, it's probably easiest if it goes in via tip along
> with patch 1.
>=20
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>

Thanks for the review. I'll base v2 on -tip


- Naveen

=

^ permalink raw reply

* Build Failure on linux-next on Power8 machine
From: vrbagal1 @ 2018-05-31 10:38 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: sfr

Greetings!!!


I am observing a build failure on linux-next kernel.

Machine: Power8
Platform: PowerNV(Baremetal)
Git Tree: 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

Below is the build error.

14:20:48 make[1]: *** [kernel/umh.o] Error 1
14:20:48 make[1]: *** [kernel/fork.o] Error 1
14:20:48 In file included from 
./arch/powerpc/include/asm/highmem.h:30:0,
14:20:48                  from ./include/linux/highmem.h:35,
14:20:48                  from ./include/linux/pagemap.h:11,
14:20:48                  from security/commoncap.c:19:
14:20:48 ./arch/powerpc/include/asm/fixmap.h:52:2: error: overflow in 
enumeration values
14:20:48   FIX_KMAP_BEGIN, /* reserved pte's for temporary kernel 
mappings */
14:20:48   ^
14:20:48 make[2]: *** [kernel/sched/swait.o] Error 1
14:20:48 make[1]: *** [mm/page_alloc.o] Error 1
14:20:48 make[1]: *** [kernel/sched] Error 2
14:20:48 make: *** [mm] Error 2
14:20:48 make[1]: *** [kernel/exit.o] Error 1
14:20:48 make[1]: *** [security/commoncap.o] Error 1
14:20:48 make[1]: *** [fs/kernfs] Error 2
14:20:48 make: *** [fs] Error 2
14:20:48 make: *** [arch/powerpc/kernel] Error 2
14:20:48 make: *** [security] Error 2
14:20:48 make: *** [kernel] Error 2

Cheers.

^ permalink raw reply

* Re: Build Failure on linux-next on Power8 machine
From: Christophe LEROY @ 2018-05-31 11:14 UTC (permalink / raw)
  To: vrbagal1, linuxppc-dev; +Cc: sfr
In-Reply-To: <d377489b3333eca0951510c19b7d8878@linux.vnet.ibm.com>



Le 31/05/2018 à 12:38, vrbagal1 a écrit :
> Greetings!!!
> 
> 
> I am observing a build failure on linux-next kernel.
> 
> Machine: Power8
> Platform: PowerNV(Baremetal)
> Git Tree: 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

Which defconfig do you use, or can you give your .config ?

Christophe


> 
> Below is the build error.
> 
> 14:20:48 make[1]: *** [kernel/umh.o] Error 1
> 14:20:48 make[1]: *** [kernel/fork.o] Error 1
> 14:20:48 In file included from ./arch/powerpc/include/asm/highmem.h:30:0,
> 14:20:48                  from ./include/linux/highmem.h:35,
> 14:20:48                  from ./include/linux/pagemap.h:11,
> 14:20:48                  from security/commoncap.c:19:
> 14:20:48 ./arch/powerpc/include/asm/fixmap.h:52:2: error: overflow in 
> enumeration values
> 14:20:48   FIX_KMAP_BEGIN, /* reserved pte's for temporary kernel 
> mappings */
> 14:20:48   ^
> 14:20:48 make[2]: *** [kernel/sched/swait.o] Error 1
> 14:20:48 make[1]: *** [mm/page_alloc.o] Error 1
> 14:20:48 make[1]: *** [kernel/sched] Error 2
> 14:20:48 make: *** [mm] Error 2
> 14:20:48 make[1]: *** [kernel/exit.o] Error 1
> 14:20:48 make[1]: *** [security/commoncap.o] Error 1
> 14:20:48 make[1]: *** [fs/kernfs] Error 2
> 14:20:48 make: *** [fs] Error 2
> 14:20:48 make: *** [arch/powerpc/kernel] Error 2
> 14:20:48 make: *** [security] Error 2
> 14:20:48 make: *** [kernel] Error 2
> 
> Cheers.

^ permalink raw reply

* Re: [PATCH v2] powerpc/64: Fix build failure with GCC 8.1
From: Michael Ellerman @ 2018-05-31 11:17 UTC (permalink / raw)
  To: Christophe LEROY, Geert Uytterhoeven
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Linux Kernel Mailing List,
	linuxppc-dev, Geoff Levand
In-Reply-To: <fd687f6c-0846-07da-328d-dd01bb2d0fdb@c-s.fr>

Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 31/05/2018 =C3=A0 07:54, Michael Ellerman a =C3=A9crit=C2=A0:
>> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>>> Le 29/05/2018 =C3=A0 11:05, Geert Uytterhoeven a =C3=A9crit=C2=A0:
>>>> Hi Christophe,
>>>> On Tue, May 29, 2018 at 10:56 AM, Christophe LEROY
>>>> <christophe.leroy@c-s.fr> wrote:
>>>>> Le 29/05/2018 =C3=A0 09:47, Geert Uytterhoeven a =C3=A9crit :
>>>>>> On Tue, May 29, 2018 at 8:03 AM, Christophe Leroy
>>>>>>> --- a/arch/powerpc/kernel/nvram_64.c
>>>>>>> +++ b/arch/powerpc/kernel/nvram_64.c
>>>>>>> @@ -1039,7 +1039,7 @@ loff_t __init nvram_create_partition(const ch=
ar
>>>>>>> *name, int sig,
>>>>>>>            new_part->index =3D free_part->index;
>>>>>>>            new_part->header.signature =3D sig;
>>>>>>>            new_part->header.length =3D size;
>>>>>>> -       strncpy(new_part->header.name, name, 12);
>>>>>>> +       memcpy(new_part->header.name, name, strnlen(name,
>>>>>>> sizeof(new_part->header.name)));
>>>>>>
>>>>>>
>>>>>> The comment for nvram_header.lgnth says:
>>>>>>
>>>>>>            /* Terminating null required only for names < 12 chars. */
>>>>>>
>>>>>> This will not terminate the string with a zero (the struct is
>>>>>> allocated with kmalloc).
>>>>>> So the original code is correct, the new one isn't.
>>>>>
>>>>> Right, then I have to first zeroize the destination.
>>>>
>>>> Using kzalloc() instead of kmalloc() will do.
>>>>
>>>> Still, papering around these warnings seems to obscure things, IMHO.
>>>> And it increases code size, as you had to add a call to strnlen().
>>=20
>>=20
>> The right fix is to not try and mirror the on-device structure in the
>> kernel struct. We should just use a proper NULL terminated string, which
>> would avoid the need to explicitly do strncmp(.., .., 12) in the code
>> and be less bug prone in general.
>>=20
>> The only place where we should need to worry about the 12 byte buffer is
>> in nvram_write_header().
>>=20
>> Anyway that's a bigger change, so I'll take this for now with kzalloc().
>
> Thanks. You take it as is and add the kzalloc() or you expect a v3 from=20
> me with the kzalloc()

Sorry that wasn't clear was it. I'll add the kzalloc().

cheers

^ permalink raw reply

* Re: Build Failure on linux-next on Power8 machine
From: Venkat Rao B @ 2018-05-31 11:28 UTC (permalink / raw)
  To: Christophe LEROY, linuxppc-dev; +Cc: sfr
In-Reply-To: <5abede57-06bd-bb3c-385e-11a61a380027@c-s.fr>



On Thursday 31 May 2018 04:44 PM, Christophe LEROY wrote:
> 
> 
> Le 31/05/2018 à 12:38, vrbagal1 a écrit :
>> Greetings!!!
>>
>>
>> I am observing a build failure on linux-next kernel.
>>
>> Machine: Power8
>> Platform: PowerNV(Baremetal)
>> Git Tree: 
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> 
> Which defconfig do you use, or can you give your .config ?
> 
> Christophe
> 
> 
Unfortunately I lost it. I  only have complete fail logs. Will that be 
helpful?

Venkat
>>
>> Below is the build error.
>>
>> 14:20:48 make[1]: *** [kernel/umh.o] Error 1
>> 14:20:48 make[1]: *** [kernel/fork.o] Error 1
>> 14:20:48 In file included from ./arch/powerpc/include/asm/highmem.h:30:0,
>> 14:20:48                  from ./include/linux/highmem.h:35,
>> 14:20:48                  from ./include/linux/pagemap.h:11,
>> 14:20:48                  from security/commoncap.c:19:
>> 14:20:48 ./arch/powerpc/include/asm/fixmap.h:52:2: error: overflow in 
>> enumeration values
>> 14:20:48   FIX_KMAP_BEGIN, /* reserved pte's for temporary kernel 
>> mappings */
>> 14:20:48   ^
>> 14:20:48 make[2]: *** [kernel/sched/swait.o] Error 1
>> 14:20:48 make[1]: *** [mm/page_alloc.o] Error 1
>> 14:20:48 make[1]: *** [kernel/sched] Error 2
>> 14:20:48 make: *** [mm] Error 2
>> 14:20:48 make[1]: *** [kernel/exit.o] Error 1
>> 14:20:48 make[1]: *** [security/commoncap.o] Error 1
>> 14:20:48 make[1]: *** [fs/kernfs] Error 2
>> 14:20:48 make: *** [fs] Error 2
>> 14:20:48 make: *** [arch/powerpc/kernel] Error 2
>> 14:20:48 make: *** [security] Error 2
>> 14:20:48 make: *** [kernel] Error 2
>>
>> Cheers.
> 

^ permalink raw reply

* [PATCH v2 20/21] powerpc/xmon: use match_string() helper
From: Yisheng Xie @ 2018-05-31 11:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: andy.shevchenko, Yisheng Xie, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, linuxppc-dev
In-Reply-To: <1527765086-19873-1-git-send-email-xieyisheng1@huawei.com>

match_string() returns the index of an array for a matching string,
which can be used instead of open coded variant.

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: Yisheng Xie <xieyisheng1@huawei.com>
---
 arch/powerpc/xmon/xmon.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a0842f1..872ac8c 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3161,7 +3161,7 @@ static void proccall(void)
 }
 
 #define N_PTREGS	44
-static char *regnames[N_PTREGS] = {
+static const char *regnames[N_PTREGS] = {
 	"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
 	"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
 	"r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
@@ -3196,18 +3196,17 @@ static void proccall(void)
 			regname[i] = c;
 		}
 		regname[i] = 0;
-		for (i = 0; i < N_PTREGS; ++i) {
-			if (strcmp(regnames[i], regname) == 0) {
-				if (xmon_regs == NULL) {
-					printf("regs not available\n");
-					return 0;
-				}
-				*vp = ((unsigned long *)xmon_regs)[i];
-				return 1;
-			}
+		i = match_string(regnames, N_PTREGS, regname);
+		if (i < 0) {
+			printf("invalid register name '%%%s'\n", regname);
+			return 0;
 		}
-		printf("invalid register name '%%%s'\n", regname);
-		return 0;
+		if (xmon_regs == NULL) {
+			printf("regs not available\n");
+			return 0;
+		}
+		*vp = ((unsigned long *)xmon_regs)[i];
+		return 1;
 	}
 
 	/* skip leading "0x" if any */
-- 
1.7.12.4

^ permalink raw reply related

* Re: [PATCH v5 0/4] powerpc patches for new Kconfig language
From: Michael Ellerman @ 2018-05-31 11:51 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: Nicholas Piggin, Linux Kbuild mailing list, linuxppc-dev
In-Reply-To: <CAK7LNARbz8YC7=vB_Pt-FFepLfLY-Yt=8ZiOAoPeZV3EA3VQOg@mail.gmail.com>

Masahiro Yamada <yamada.masahiro@socionext.com> writes:
> 2018-05-31 13:31 GMT+09:00 Michael Ellerman <mpe@ellerman.id.au>:
>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>> 2018-05-30 23:39 GMT+09:00 Michael Ellerman <mpe@ellerman.id.au>:
>>>> Nicholas Piggin <npiggin@gmail.com> writes:
>>>>
>>>>> This series of patches improves th powerpc kbuild system. The
>>>>> motivation was to to be compatible with the new Kconfig scripting
>>>>> language that Yamada-san has implemented here for merge:
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/log/?h=kconfig
>>>>>
>>>>> I have tested on top of that tree, powerpc now builds there.
>>>>>
>>>>> To avoid build breakage, the first 3 patches must go before the
>>>>> kconfig change, and the 4th patch must go after it.
>>>>>
>>>>> v5 changes:
>>>>> - Patch 4 update to syntax changed since kconfig-shell-v3 release.
>>>>> - Patch 4 suggestions from Masahiro Yamada, remove unnecessary "OK"
>>>>>   output from check mprofile script, and fold CC_USING_MPROFILE_KERNEL
>>>>>   into CONFIG_MPROFILE_KERNEL.
>>>>> - Reduce whitespace disturbance in patch 1.
>>>>
>>>> I've put this series in a topic branch.
>>>>
>>>>   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/log/?h=topic/kbuild
>>>
>>>
>>> No, you can't.
>>>
>>> This series depends on my Kconfig work.
>>
>> The *last* commit depends on your work, the rest do not.
>>
>>> You queued it up on v4.17-rc3,
>>> but necessary patches are not there.
>>>
>>> You will get build errors.
>>
>> I do not get any build errors for the first three commits.
>
>
> Right, the first three are fine.
>
> If we make sure commit cfff26c2dc7a1
> does not appear in Linus' tree,
> we are good.

Yep agreed.

>>>> I'll plan to merge the first three into the powerpc tree.
>>>
>>> Please do not do this.
>>>
>>> You can issue Acked-by instead.
>>
>> I'd prefer the first three commits were in my tree so they're tested
>> properly.
>>
>>> I need all the 4 patches to my tree.
>>> Otherwise, the git-bisect'ability breaks for PowerPC.
>>
>> You should merge all 4 into your tree, ie. all of my topic/kbuild
>> branch.
>
> This depends on what "merge" means.
>
> If it means "git pull", I cannot pull your topic/kbuild branch
> since it is broken.

Yeah OK I understand what you mean now.

> So, how do you want me to handle the last patch?
>
> I can pull the first three from your branch,
> then I can cherry-pick the last one on top of my kconfig tree.

Yes, please do that.

cheers

^ permalink raw reply

* [PATCH] cpuidle:powernv: Make the snooze timeout dynamic.
From: Gautham R. Shenoy @ 2018-05-31 12:15 UTC (permalink / raw)
  To: Rafael J. Wysocki, Daniel Lezcano, Michael Ellerman,
	Stewart Smith, Michael Neuling, Vaidyanathan Srinivasan,
	Shilpasri G Bhat, Akshay Adiga, Nicholas Piggin
  Cc: linuxppc-dev, linux-kernel, linux-pm, Gautham R. Shenoy

From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>

The commit 78eaa10f027c ("cpuidle: powernv/pseries: Auto-promotion of
snooze to deeper idle state") introduced a timeout for the snooze idle
state so that it could be eventually be promoted to a deeper idle
state. The snooze timeout value is static and set to the target
residency of the next idle state, which would train the cpuidle
governor to pick the next idle state eventually.

The unfortunate side-effect of this is that if the next idle state(s)
is disabled, the CPU will forever remain in snooze, despite the fact
that the system is completely idle, and other deeper idle states are
available.

This patch fixes the issue by dynamically setting the snooze timeout
to the target residency of the next enabled state on the device.

Before Patch
==================
POWER8 : Only nap disabled.
 $cpupower monitor sleep 30
sleep took 30.01297 seconds and exited with status 0
              |Idle_Stats
PKG |CORE|CPU | snoo | Nap  | Fast
   0|   8|   0| 96.41|  0.00|  0.00
   0|   8|   1| 96.43|  0.00|  0.00
   0|   8|   2| 96.47|  0.00|  0.00
   0|   8|   3| 96.35|  0.00|  0.00
   0|   8|   4| 96.37|  0.00|  0.00
   0|   8|   5| 96.37|  0.00|  0.00
   0|   8|   6| 96.47|  0.00|  0.00
   0|   8|   7| 96.47|  0.00|  0.00

POWER9: Shallow states (stop0lite, stop1lite, stop2lite, stop0, stop1,
stop2) disabled:
$cpupower monitor sleep 30
sleep took 30.05033 seconds and exited with status 0
              |Idle_Stats
PKG |CORE|CPU | snoo | stop | stop | stop | stop | stop | stop | stop | stop
   0|  16|   0| 89.79|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00
   0|  16|   1| 90.12|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00
   0|  16|   2| 90.21|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00
   0|  16|   3| 90.29|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00

After Patch
======================
POWER8 : Only nap disabled.
$ cpupower monitor sleep 30
sleep took 30.01200 seconds and exited with status 0
              |Idle_Stats
PKG |CORE|CPU | snoo | Nap  | Fast
   0|   8|   0| 16.58|  0.00| 77.21
   0|   8|   1| 18.42|  0.00| 75.38
   0|   8|   2|  4.70|  0.00| 94.09
   0|   8|   3| 17.06|  0.00| 81.73
   0|   8|   4|  3.06|  0.00| 95.73
   0|   8|   5|  7.00|  0.00| 96.80
   0|   8|   6|  1.00|  0.00| 98.79
   0|   8|   7|  5.62|  0.00| 94.17

POWER9: Shallow states (stop0lite, stop1lite, stop2lite, stop0, stop1,
stop2) disabled:

$cpupower monitor sleep 30
sleep took 30.02110 seconds and exited with status 0
              |Idle_Stats
PKG |CORE|CPU | snoo | stop | stop | stop | stop | stop | stop | stop | stop
   0|   0|   0|  0.69|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  9.39| 89.70
   0|   0|   1|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.05| 93.21
   0|   0|   2|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00| 89.93
   0|   0|   3|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00| 93.26

Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
 drivers/cpuidle/cpuidle-powernv.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 1a8234e..d29e4f0 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -43,9 +43,31 @@ struct stop_psscr_table {
 
 static struct stop_psscr_table stop_psscr_table[CPUIDLE_STATE_MAX] __read_mostly;
 
-static u64 snooze_timeout __read_mostly;
+static u64 default_snooze_timeout __read_mostly;
 static bool snooze_timeout_en __read_mostly;
 
+static u64 get_snooze_timeout(struct cpuidle_device *dev,
+			      struct cpuidle_driver *drv,
+			      int index)
+{
+	int i;
+
+	if (unlikely(!snooze_timeout_en))
+		return default_snooze_timeout;
+
+	for (i = index + 1; i < drv->state_count; i++) {
+		struct cpuidle_state *s = &drv->states[i];
+		struct cpuidle_state_usage *su = &dev->states_usage[i];
+
+		if (s->disabled || su->disable)
+			continue;
+
+		return s->target_residency * tb_ticks_per_usec;
+	}
+
+	return default_snooze_timeout;
+}
+
 static int snooze_loop(struct cpuidle_device *dev,
 			struct cpuidle_driver *drv,
 			int index)
@@ -56,7 +78,7 @@ static int snooze_loop(struct cpuidle_device *dev,
 
 	local_irq_enable();
 
-	snooze_exit_time = get_tb() + snooze_timeout;
+	snooze_exit_time = get_tb() + get_snooze_timeout(dev, drv, index);
 	ppc64_runlatch_off();
 	HMT_very_low();
 	while (!need_resched()) {
@@ -465,11 +487,9 @@ static int powernv_idle_probe(void)
 		cpuidle_state_table = powernv_states;
 		/* Device tree can indicate more idle states */
 		max_idle_state = powernv_add_idle_states();
-		if (max_idle_state > 1) {
+		default_snooze_timeout = TICK_USEC * tb_ticks_per_usec;
+		if (max_idle_state > 1)
 			snooze_timeout_en = true;
-			snooze_timeout = powernv_states[1].target_residency *
-					 tb_ticks_per_usec;
-		}
  	} else
  		return -ENODEV;
 
-- 
1.9.4

^ permalink raw reply related

* Re: [PATCH v5 0/4] powerpc patches for new Kconfig language
From: Masahiro Yamada @ 2018-05-31 13:08 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Nicholas Piggin, Linux Kbuild mailing list, linuxppc-dev
In-Reply-To: <876034yrek.fsf@concordia.ellerman.id.au>

2018-05-31 20:51 GMT+09:00 Michael Ellerman <mpe@ellerman.id.au>:
> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>> 2018-05-31 13:31 GMT+09:00 Michael Ellerman <mpe@ellerman.id.au>:
>>> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>>>> 2018-05-30 23:39 GMT+09:00 Michael Ellerman <mpe@ellerman.id.au>:
>>>>> Nicholas Piggin <npiggin@gmail.com> writes:
>>>>>
>>>>>> This series of patches improves th powerpc kbuild system. The
>>>>>> motivation was to to be compatible with the new Kconfig scripting
>>>>>> language that Yamada-san has implemented here for merge:
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/log/?h=kconfig
>>>>>>
>>>>>> I have tested on top of that tree, powerpc now builds there.
>>>>>>
>>>>>> To avoid build breakage, the first 3 patches must go before the
>>>>>> kconfig change, and the 4th patch must go after it.
>>>>>>
>>>>>> v5 changes:
>>>>>> - Patch 4 update to syntax changed since kconfig-shell-v3 release.
>>>>>> - Patch 4 suggestions from Masahiro Yamada, remove unnecessary "OK"
>>>>>>   output from check mprofile script, and fold CC_USING_MPROFILE_KERNEL
>>>>>>   into CONFIG_MPROFILE_KERNEL.
>>>>>> - Reduce whitespace disturbance in patch 1.
>>>>>
>>>>> I've put this series in a topic branch.
>>>>>
>>>>>   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/log/?h=topic/kbuild
>>>>
>>>>
>>>> No, you can't.
>>>>
>>>> This series depends on my Kconfig work.
>>>
>>> The *last* commit depends on your work, the rest do not.
>>>
>>>> You queued it up on v4.17-rc3,
>>>> but necessary patches are not there.
>>>>
>>>> You will get build errors.
>>>
>>> I do not get any build errors for the first three commits.
>>
>>
>> Right, the first three are fine.
>>
>> If we make sure commit cfff26c2dc7a1
>> does not appear in Linus' tree,
>> we are good.
>
> Yep agreed.
>
>>>>> I'll plan to merge the first three into the powerpc tree.
>>>>
>>>> Please do not do this.
>>>>
>>>> You can issue Acked-by instead.
>>>
>>> I'd prefer the first three commits were in my tree so they're tested
>>> properly.
>>>
>>>> I need all the 4 patches to my tree.
>>>> Otherwise, the git-bisect'ability breaks for PowerPC.
>>>
>>> You should merge all 4 into your tree, ie. all of my topic/kbuild
>>> branch.
>>
>> This depends on what "merge" means.
>>
>> If it means "git pull", I cannot pull your topic/kbuild branch
>> since it is broken.
>
> Yeah OK I understand what you mean now.
>
>> So, how do you want me to handle the last patch?
>>
>> I can pull the first three from your branch,
>> then I can cherry-pick the last one on top of my kconfig tree.
>
> Yes, please do that.


OK. I will.



BTW, if you try to offer a little more kindness,
you may want to check some typos in the commit description.

I suspect some.
https://patchwork.kernel.org/patch/10438869/
https://patchwork.kernel.org/patch/10438873/


Also, the change logs could be dropped.

I see

Since v1: reworded changelog to explain the cause of the problem (thanks
Segher) and moved the flags into the 64-32 cross compile case.

or

Since v1: removed extra -EB in the recordmcount script (thanks mpe)


above your signed-off-by.


Of course, this is your call,
and you do not need to disturb the git history if it is too late.





-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: Build Failure on linux-next on Power8 machine
From: Michael Ellerman @ 2018-05-31 14:05 UTC (permalink / raw)
  To: Venkat Rao B, Christophe LEROY, linuxppc-dev; +Cc: sfr
In-Reply-To: <1fd71673-9e1e-9f95-057e-ed8fb87e503d@linux.vnet.ibm.com>

Venkat Rao B <vrbagal1@linux.vnet.ibm.com> writes:
> On Thursday 31 May 2018 04:44 PM, Christophe LEROY wrote:
>> Le 31/05/2018 =C3=A0 12:38, vrbagal1 a =C3=A9crit=C2=A0:
>>> Greetings!!!
>>>
>>> I am observing a build failure on linux-next kernel.
>>>
>>> Machine: Power8
>>> Platform: PowerNV(Baremetal)
>>> Git Tree:=20
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>>=20
>> Which defconfig do you use, or can you give your .config ?
>>=20
>> Christophe
>>=20
>>=20
> Unfortunately I lost it. I  only have complete fail logs. Will that be=20
> helpful?

We really need the .config, or at least some idea of which config it is.

We also really need to know which exact revision you're building.
linux-next is updated every day.

Today's build results are here:

  http://kisskb.ellerman.id.au/kisskb/head/14032/


I don't see any fails that look similar to yours. So perhaps you're
using a different compiler or something. That's another detail you
should tell us, the exact compiler version :)

cheers

^ permalink raw reply

* Re: [PATCH 2/2] powerpc: Add support for function error injection
From: Michael Ellerman @ 2018-05-31 14:13 UTC (permalink / raw)
  To: Naveen N. Rao, Josef Bacik, Masami Hiramatsu, Ingo Molnar
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <1527761352.z5jz8jg2d5.naveen@linux.ibm.com>

"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:

> Michael Ellerman wrote:
>> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
>>> ...
>> 
>> A change log is always nice even if it's short :)
>
> I tried, but really couldn't come up with anything more to write. I'll 
> try harder for v2 :)

Yeah true.

You could just say "All that's required is to define x and y and select
the Kconfig symbol".

>> This looks fine to me, it's probably easiest if it goes in via tip along
>> with patch 1.
>> 
>> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
>
> Thanks for the review. I'll base v2 on -tip

I guess if it doesn't already apply to tip you should rebase it. You've
probably missed 4.18 anyway.

cheers

^ permalink raw reply

* Re: [RFC PATCH] powerpc/fsl: Add barrier_nospec implementation for NXP PowerPC Book E
From: Michael Ellerman @ 2018-05-31 14:21 UTC (permalink / raw)
  To: Scott Wood, Diana Madalina Craciun, linuxppc-dev@lists.ozlabs.org; +Cc: Leo Li
In-Reply-To: <1527621245.30674.30.camel@buserror.net>

Scott Wood <oss@buserror.net> writes:
> On Tue, 2018-05-29 at 15:22 +0000, Diana Madalina Craciun wrote:
>> On 05/22/2018 11:31 PM, Scott Wood wrote:
>
>> > Should there be a way for the user to choose not to enable this (editing
>> > the
>> > device tree doesn't count), for a use case that is not sufficiently
>> > security
>> > sensitive to justify the performance loss?  What is the performance impact
>> > of
>> > this patch?
>> 
>> My reason was that on the other architectures Spectre variant 1
>> mitigations are not disabled either. But I think that it might be a good
>> idea to add a bootarg parameter to disable the barrier.
>
> Is there a specific policy reason why they allow spectre v2 to be disabled but
> not v1,

No.

> or just a matter of not having a mechanism to disable it,

Yes and no. Some of the v1 mitigation is done via masking which can't be
easily patched. eg. array_index_nospec()

> or the parts which could practically be disabled not impacting
> performance much?

That's the mean reason AIUI.

We can add a nospectre_v1 command line option if necessary.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/64s: Fix compiler store ordering to SLB shadow area
From: Michael Ellerman @ 2018-05-31 14:22 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K . V, Nicholas Piggin
In-Reply-To: <20180530103122.27674-1-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> The stores to update the SLB shadow area must be made as they appear
> in the C code, so that the hypervisor does not see an entry with
> mismatched vsid and esid. Use WRITE_ONCE for this.
>
> GCC has been observed to elide the first store to esid in the update,
> which means that if the hypervisor interrupts the guest after storing
> to vsid, it could see an entry with old esid and new vsid, which may
> possibly result in memory corruption.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/mm/slb.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index 66577cc66dc9..2f4b33b24b3b 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -63,14 +63,14 @@ static inline void slb_shadow_update(unsigned long ea, int ssize,
>  	 * updating it.  No write barriers are needed here, provided
>  	 * we only update the current CPU's SLB shadow buffer.
>  	 */
> -	p->save_area[index].esid = 0;
> -	p->save_area[index].vsid = cpu_to_be64(mk_vsid_data(ea, ssize, flags));
> -	p->save_area[index].esid = cpu_to_be64(mk_esid_data(ea, ssize, index));
> +	WRITE_ONCE(p->save_area[index].esid, 0);
> +	WRITE_ONCE(p->save_area[index].vsid, cpu_to_be64(mk_vsid_data(ea, ssize, flags)));
> +	WRITE_ONCE(p->save_area[index].esid, cpu_to_be64(mk_esid_data(ea, ssize, index)));

What's the code-gen for that look like? I suspect it's terrible?

Should we just do it in inline-asm I wonder?

cheers

^ permalink raw reply

* Re: [RFC PATCH] powerpc/fsl: Add barrier_nospec implementation for NXP PowerPC Book E
From: Diana Madalina Craciun @ 2018-05-31 14:35 UTC (permalink / raw)
  To: Michael Ellerman, Scott Wood, linuxppc-dev@lists.ozlabs.org; +Cc: Leo Li
In-Reply-To: <87wovjykh8.fsf@concordia.ellerman.id.au>

On 5/31/2018 5:21 PM, Michael Ellerman wrote:=0A=
> Scott Wood <oss@buserror.net> writes:=0A=
>> On Tue, 2018-05-29 at 15:22 +0000, Diana Madalina Craciun wrote:=0A=
>>> On 05/22/2018 11:31 PM, Scott Wood wrote:=0A=
>>>> Should there be a way for the user to choose not to enable this (editi=
ng=0A=
>>>> the=0A=
>>>> device tree doesn't count), for a use case that is not sufficiently=0A=
>>>> security=0A=
>>>> sensitive to justify the performance loss?  What is the performance im=
pact=0A=
>>>> of=0A=
>>>> this patch?=0A=
>>> My reason was that on the other architectures Spectre variant 1=0A=
>>> mitigations are not disabled either. But I think that it might be a goo=
d=0A=
>>> idea to add a bootarg parameter to disable the barrier.=0A=
>> Is there a specific policy reason why they allow spectre v2 to be disabl=
ed but=0A=
>> not v1,=0A=
> No.=0A=
>=0A=
>> or just a matter of not having a mechanism to disable it,=0A=
> Yes and no. Some of the v1 mitigation is done via masking which can't be=
=0A=
> easily patched. eg. array_index_nospec()=0A=
>=0A=
>> or the parts which could practically be disabled not impacting=0A=
>> performance much?=0A=
> That's the mean reason AIUI.=0A=
>=0A=
> We can add a nospectre_v1 command line option if necessary.=0A=
=0A=
What about nobarrier_nospec (or similar) instead of nospectre_v1 command=0A=
line? We are not disabling all the v1 mitigations, the masking part will=0A=
remain unchanged.=0A=
=0A=
Diana=0A=
=0A=
=0A=

^ permalink raw reply

* Re: [PATCH] cpuidle:powernv: Make the snooze timeout dynamic.
From: Balbir Singh @ 2018-05-31 14:51 UTC (permalink / raw)
  To: Gautham R. Shenoy
  Cc: Rafael J. Wysocki, Daniel Lezcano, Michael Ellerman,
	Stewart Smith, Michael Neuling, Vaidyanathan Srinivasan,
	Shilpasri G Bhat, Akshay Adiga, Nicholas Piggin,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
	linux-kernel@vger.kernel.org, linux-pm
In-Reply-To: <1527768909-32637-1-git-send-email-ego@linux.vnet.ibm.com>

On Thu, May 31, 2018 at 10:15 PM, Gautham R. Shenoy
<ego@linux.vnet.ibm.com> wrote:
> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>
> The commit 78eaa10f027c ("cpuidle: powernv/pseries: Auto-promotion of
> snooze to deeper idle state") introduced a timeout for the snooze idle
> state so that it could be eventually be promoted to a deeper idle
> state. The snooze timeout value is static and set to the target
> residency of the next idle state, which would train the cpuidle
> governor to pick the next idle state eventually.
>
> The unfortunate side-effect of this is that if the next idle state(s)
> is disabled, the CPU will forever remain in snooze, despite the fact
> that the system is completely idle, and other deeper idle states are
> available.
>
> This patch fixes the issue by dynamically setting the snooze timeout
> to the target residency of the next enabled state on the device.
>
> Before Patch
> ==================
> POWER8 : Only nap disabled.
>  $cpupower monitor sleep 30
> sleep took 30.01297 seconds and exited with status 0
>               |Idle_Stats
> PKG |CORE|CPU | snoo | Nap  | Fast
>    0|   8|   0| 96.41|  0.00|  0.00
>    0|   8|   1| 96.43|  0.00|  0.00
>    0|   8|   2| 96.47|  0.00|  0.00
>    0|   8|   3| 96.35|  0.00|  0.00
>    0|   8|   4| 96.37|  0.00|  0.00
>    0|   8|   5| 96.37|  0.00|  0.00
>    0|   8|   6| 96.47|  0.00|  0.00
>    0|   8|   7| 96.47|  0.00|  0.00
>
> POWER9: Shallow states (stop0lite, stop1lite, stop2lite, stop0, stop1,
> stop2) disabled:
> $cpupower monitor sleep 30
> sleep took 30.05033 seconds and exited with status 0
>               |Idle_Stats
> PKG |CORE|CPU | snoo | stop | stop | stop | stop | stop | stop | stop | stop
>    0|  16|   0| 89.79|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00
>    0|  16|   1| 90.12|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00
>    0|  16|   2| 90.21|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00
>    0|  16|   3| 90.29|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00
>
> After Patch
> ======================
> POWER8 : Only nap disabled.
> $ cpupower monitor sleep 30
> sleep took 30.01200 seconds and exited with status 0
>               |Idle_Stats
> PKG |CORE|CPU | snoo | Nap  | Fast
>    0|   8|   0| 16.58|  0.00| 77.21
>    0|   8|   1| 18.42|  0.00| 75.38
>    0|   8|   2|  4.70|  0.00| 94.09
>    0|   8|   3| 17.06|  0.00| 81.73
>    0|   8|   4|  3.06|  0.00| 95.73
>    0|   8|   5|  7.00|  0.00| 96.80
>    0|   8|   6|  1.00|  0.00| 98.79
>    0|   8|   7|  5.62|  0.00| 94.17
>
> POWER9: Shallow states (stop0lite, stop1lite, stop2lite, stop0, stop1,
> stop2) disabled:
>
> $cpupower monitor sleep 30
> sleep took 30.02110 seconds and exited with status 0
>               |Idle_Stats
> PKG |CORE|CPU | snoo | stop | stop | stop | stop | stop | stop | stop | stop
>    0|   0|   0|  0.69|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  9.39| 89.70
>    0|   0|   1|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.05| 93.21
>    0|   0|   2|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00| 89.93
>    0|   0|   3|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00|  0.00| 93.26
>
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> ---
>  drivers/cpuidle/cpuidle-powernv.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index 1a8234e..d29e4f0 100644
> --- a/drivers/cpuidle/cpuidle-powernv.c
> +++ b/drivers/cpuidle/cpuidle-powernv.c
> @@ -43,9 +43,31 @@ struct stop_psscr_table {
>
>  static struct stop_psscr_table stop_psscr_table[CPUIDLE_STATE_MAX] __read_mostly;
>
> -static u64 snooze_timeout __read_mostly;
> +static u64 default_snooze_timeout __read_mostly;
>  static bool snooze_timeout_en __read_mostly;
>
> +static u64 get_snooze_timeout(struct cpuidle_device *dev,
> +                             struct cpuidle_driver *drv,
> +                             int index)
> +{
> +       int i;
> +
> +       if (unlikely(!snooze_timeout_en))
> +               return default_snooze_timeout;
> +
> +       for (i = index + 1; i < drv->state_count; i++) {
> +               struct cpuidle_state *s = &drv->states[i];
> +               struct cpuidle_state_usage *su = &dev->states_usage[i];
> +
> +               if (s->disabled || su->disable)
> +                       continue;
> +
> +               return s->target_residency * tb_ticks_per_usec;

Can we ensure this is not prone to overflow?

Otherwise looks good

Reviewed-by: Balbir Singh <bsingharora@gmail.com>

^ permalink raw reply

* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Michael S. Tsirkin @ 2018-05-31 17:43 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Ram Pai, robh, aik, jasowang, linux-kernel, virtualization, hch,
	joe, linuxppc-dev, elfring, david
In-Reply-To: <0c508eb2-08df-3f76-c260-90cf7137af80@linux.vnet.ibm.com>

On Thu, May 31, 2018 at 09:09:24AM +0530, Anshuman Khandual wrote:
> On 05/24/2018 12:51 PM, Ram Pai wrote:
> > On Wed, May 23, 2018 at 09:50:02PM +0300, Michael S. Tsirkin wrote:
> >> subj: s/virito/virtio/
> >>
> > ..snip..
> >>>  machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
> >>> +
> >>> +bool platform_forces_virtio_dma(struct virtio_device *vdev)
> >>> +{
> >>> +	/*
> >>> +	 * On protected guest platforms, force virtio core to use DMA
> >>> +	 * MAP API for all virtio devices. But there can also be some
> >>> +	 * exceptions for individual devices like virtio balloon.
> >>> +	 */
> >>> +	return (of_find_compatible_node(NULL, NULL, "ibm,ultravisor") != NULL);
> >>> +}
> >>
> >> Isn't this kind of slow?  vring_use_dma_api is on
> >> data path and supposed to be very fast.
> > 
> > Yes it is slow and not ideal. This won't be the final code. The final
> > code will cache the information in some global variable and used
> > in this function.
> 
> Right this will be a simple check based on a global variable.
> 

Pls work on a long term solution. Short term needs can be served by
enabling the iommu platform in qemu.

-- 
MST

^ permalink raw reply


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