LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 2/3] powerpc/sysfs: Show idle_purr and idle_spurr for every CPU
From: Naveen N. Rao @ 2020-02-04  7:52 UTC (permalink / raw)
  To: ego
  Cc: Nathan Lynch, Tyrel Datwyler, linux-kernel, Kamalesh Babulal,
	Vaidyanathan Srinivasan, linuxppc-dev
In-Reply-To: <20200203045013.GC13468@in.ibm.com>

Gautham R Shenoy wrote:
> Hi Naveen,
> 
> On Thu, Dec 05, 2019 at 10:23:58PM +0530, Naveen N. Rao wrote:
>> >diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
>> >index 80a676d..42ade55 100644
>> >--- a/arch/powerpc/kernel/sysfs.c
>> >+++ b/arch/powerpc/kernel/sysfs.c
>> >@@ -1044,6 +1044,36 @@ static ssize_t show_physical_id(struct device *dev,
>> > }
>> > static DEVICE_ATTR(physical_id, 0444, show_physical_id, NULL);
>> >
>> >+static ssize_t idle_purr_show(struct device *dev,
>> >+			      struct device_attribute *attr, char *buf)
>> >+{
>> >+	struct cpu *cpu = container_of(dev, struct cpu, dev);
>> >+	unsigned int cpuid = cpu->dev.id;
>> >+	struct lppaca *cpu_lppaca_ptr = paca_ptrs[cpuid]->lppaca_ptr;
>> >+	u64 idle_purr_cycles = be64_to_cpu(cpu_lppaca_ptr->wait_state_cycles);
>> >+
>> >+	return sprintf(buf, "%llx\n", idle_purr_cycles);
>> >+}
>> >+static DEVICE_ATTR_RO(idle_purr);
>> >+
>> >+DECLARE_PER_CPU(u64, idle_spurr_cycles);
>> >+static ssize_t idle_spurr_show(struct device *dev,
>> >+			       struct device_attribute *attr, char *buf)
>> >+{
>> >+	struct cpu *cpu = container_of(dev, struct cpu, dev);
>> >+	unsigned int cpuid = cpu->dev.id;
>> >+	u64 *idle_spurr_cycles_ptr = per_cpu_ptr(&idle_spurr_cycles, cpuid);
>> 
>> Is it possible for a user to read stale values if a particular cpu is in an
>> extended cede? Is it possible to use smp_call_function_single() to force the
>> cpu out of idle?
> 
> Yes, if the CPU whose idle_spurr cycle is being read is still in idle,
> then we will miss reporting the delta spurr cycles for this last
> idle-duration. Yes, we can use an smp_call_function_single(), though
> that will introduce IPI noise. How often will idle_[s]purr be read ?

Since it is possible for a cpu to go into extended cede for multiple 
seconds during which time it is possible to mis-report utilization, I 
think it is better to ensure that the sysfs interface for idle_[s]purr 
report the proper values through use of IPI.

With repect to lparstat, the read interval is user-specified and just 
gets passed onto sleep().

- Naveen


^ permalink raw reply

* [PATCH 4/4] powerpc/uaccess: Implement user_read_access_begin and user_write_access_begin
From: Christophe Leroy @ 2020-02-04  7:37 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <f96ed94dc57ea810b738c4e02263e08c2c8781b6.1580801787.git.christophe.leroy@c-s.fr>

Add support for selective read or write user access with
user_read_access_begin/end and user_write_access_begin/end.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/include/asm/book3s/32/kup.h |  4 ++--
 arch/powerpc/include/asm/kup.h           | 14 +++++++++++++-
 arch/powerpc/include/asm/uaccess.h       | 22 ++++++++++++++++++++++
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
index 3c0ba22dc360..1617e73bee30 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -108,7 +108,7 @@ static __always_inline void allow_user_access(void __user *to, const void __user
 	u32 addr, end;
 
 	BUILD_BUG_ON(!__builtin_constant_p(dir));
-	BUILD_BUG_ON(dir == KUAP_CURRENT);
+	BUILD_BUG_ON(dir & ~KUAP_READ_WRITE);
 
 	if (!(dir & KUAP_WRITE))
 		return;
@@ -131,7 +131,7 @@ static __always_inline void prevent_user_access(void __user *to, const void __us
 
 	BUILD_BUG_ON(!__builtin_constant_p(dir));
 
-	if (dir == KUAP_CURRENT) {
+	if (dir & KUAP_CURRENT_WRITE) {
 		u32 kuap = current->thread.kuap;
 
 		if (unlikely(!kuap))
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index 92bcd1a26d73..c745ee41ad66 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -10,7 +10,9 @@
  * Use the current saved situation instead of the to/from/size params.
  * Used on book3s/32
  */
-#define KUAP_CURRENT	4
+#define KUAP_CURRENT_READ	4
+#define KUAP_CURRENT_WRITE	8
+#define KUAP_CURRENT		(KUAP_CURRENT_READ | KUAP_CURRENT_WRITE)
 
 #ifdef CONFIG_PPC64
 #include <asm/book3s/64/kup-radix.h>
@@ -101,6 +103,16 @@ static inline void prevent_current_access_user(void)
 	prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT);
 }
 
+static inline void prevent_current_read_from_user(void)
+{
+	prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT_READ);
+}
+
+static inline void prevent_current_write_to_user(void)
+{
+	prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT_WRITE);
+}
+
 #endif /* !__ASSEMBLY__ */
 
 #endif /* _ASM_POWERPC_KUAP_H_ */
diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 2f500debae21..4427d419eb1d 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -468,6 +468,28 @@ static __must_check inline bool user_access_begin(const void __user *ptr, size_t
 #define user_access_save	prevent_user_access_return
 #define user_access_restore	restore_user_access
 
+static __must_check inline bool
+user_read_access_begin(const void __user *ptr, size_t len)
+{
+	if (unlikely(!access_ok(ptr, len)))
+		return false;
+	allow_read_from_user(ptr, len);
+	return true;
+}
+#define user_read_access_begin	user_read_access_begin
+#define user_read_access_end		prevent_current_read_from_user
+
+static __must_check inline bool
+user_write_access_begin(const void __user *ptr, size_t len)
+{
+	if (unlikely(!access_ok(ptr, len)))
+		return false;
+	allow_write_to_user((void __user *)ptr, len);
+	return true;
+}
+#define user_write_access_begin	user_write_access_begin
+#define user_write_access_end		prevent_current_write_to_user
+
 #define unsafe_op_wrap(op, err) do { if (unlikely(op)) goto err; } while (0)
 #define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
 #define unsafe_put_user(x, p, e) unsafe_op_wrap(__put_user_allowed(x, p), e)
-- 
2.25.0


^ permalink raw reply related

* [PATCH 3/4] drm/i915/gem: Replace user_access_begin by user_write_access_begin
From: Christophe Leroy @ 2020-02-04  7:37 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <f96ed94dc57ea810b738c4e02263e08c2c8781b6.1580801787.git.christophe.leroy@c-s.fr>

When i915_gem_execbuffer2_ioctl() is using user_access_begin(),
that's only to perform unsafe_put_user() so use
user_write_access_begin() in order to only open write access.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index d5a0f5ae4a8b..f8ebd54fe69c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1610,14 +1610,14 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
 		 * happened we would make the mistake of assuming that the
 		 * relocations were valid.
 		 */
-		if (!user_access_begin(urelocs, size))
+		if (!user_write_access_begin(urelocs, size))
 			goto end;
 
 		for (copied = 0; copied < nreloc; copied++)
 			unsafe_put_user(-1,
 					&urelocs[copied].presumed_offset,
 					end_user);
-		user_access_end();
+		user_write_access_end();
 
 		eb->exec[i].relocs_ptr = (uintptr_t)relocs;
 	}
@@ -1625,7 +1625,7 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
 	return 0;
 
 end_user:
-	user_access_end();
+	user_write_access_end();
 end:
 	kvfree(relocs);
 	err = -EFAULT;
@@ -2955,7 +2955,8 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
 		 * And this range already got effectively checked earlier
 		 * when we did the "copy_from_user()" above.
 		 */
-		if (!user_access_begin(user_exec_list, count * sizeof(*user_exec_list)))
+		if (!user_write_access_begin(user_exec_list,
+					     count * sizeof(*user_exec_list)))
 			goto end;
 
 		for (i = 0; i < args->buffer_count; i++) {
@@ -2969,7 +2970,7 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
 					end_user);
 		}
 end_user:
-		user_access_end();
+		user_write_access_end();
 end:;
 	}
 
-- 
2.25.0


^ permalink raw reply related

* [PATCH 2/4] uaccess: Selectively open read or write user access
From: Christophe Leroy @ 2020-02-04  7:37 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <f96ed94dc57ea810b738c4e02263e08c2c8781b6.1580801787.git.christophe.leroy@c-s.fr>

When opening user access to only perform reads, only open read access.
When opening user access to only perform writes, only open write
access.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 fs/readdir.c            | 12 ++++++------
 kernel/compat.c         | 12 ++++++------
 kernel/exit.c           | 12 ++++++------
 lib/strncpy_from_user.c |  4 ++--
 lib/strnlen_user.c      |  4 ++--
 lib/usercopy.c          |  6 +++---
 6 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/fs/readdir.c b/fs/readdir.c
index de2eceffdee8..ed6aaad451aa 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -242,7 +242,7 @@ static int filldir(struct dir_context *ctx, const char *name, int namlen,
 		return -EINTR;
 	dirent = buf->current_dir;
 	prev = (void __user *) dirent - prev_reclen;
-	if (!user_access_begin(prev, reclen + prev_reclen))
+	if (!user_write_access_begin(prev, reclen + prev_reclen))
 		goto efault;
 
 	/* This might be 'dirent->d_off', but if so it will get overwritten */
@@ -251,14 +251,14 @@ static int filldir(struct dir_context *ctx, const char *name, int namlen,
 	unsafe_put_user(reclen, &dirent->d_reclen, efault_end);
 	unsafe_put_user(d_type, (char __user *) dirent + reclen - 1, efault_end);
 	unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
-	user_access_end();
+	user_write_access_end();
 
 	buf->current_dir = (void __user *)dirent + reclen;
 	buf->prev_reclen = reclen;
 	buf->count -= reclen;
 	return 0;
 efault_end:
-	user_access_end();
+	user_write_access_end();
 efault:
 	buf->error = -EFAULT;
 	return -EFAULT;
@@ -327,7 +327,7 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
 		return -EINTR;
 	dirent = buf->current_dir;
 	prev = (void __user *)dirent - prev_reclen;
-	if (!user_access_begin(prev, reclen + prev_reclen))
+	if (!user_write_access_begin(prev, reclen + prev_reclen))
 		goto efault;
 
 	/* This might be 'dirent->d_off', but if so it will get overwritten */
@@ -336,7 +336,7 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
 	unsafe_put_user(reclen, &dirent->d_reclen, efault_end);
 	unsafe_put_user(d_type, &dirent->d_type, efault_end);
 	unsafe_copy_dirent_name(dirent->d_name, name, namlen, efault_end);
-	user_access_end();
+	user_write_access_end();
 
 	buf->prev_reclen = reclen;
 	buf->current_dir = (void __user *)dirent + reclen;
@@ -344,7 +344,7 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen,
 	return 0;
 
 efault_end:
-	user_access_end();
+	user_write_access_end();
 efault:
 	buf->error = -EFAULT;
 	return -EFAULT;
diff --git a/kernel/compat.c b/kernel/compat.c
index 95005f849c68..7b8b59e039ba 100644
--- a/kernel/compat.c
+++ b/kernel/compat.c
@@ -263,7 +263,7 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
 	bitmap_size = ALIGN(bitmap_size, BITS_PER_COMPAT_LONG);
 	nr_compat_longs = BITS_TO_COMPAT_LONGS(bitmap_size);
 
-	if (!user_access_begin(umask, bitmap_size / 8))
+	if (!user_read_access_begin(umask, bitmap_size / 8))
 		return -EFAULT;
 
 	while (nr_compat_longs > 1) {
@@ -275,11 +275,11 @@ long compat_get_bitmap(unsigned long *mask, const compat_ulong_t __user *umask,
 	}
 	if (nr_compat_longs)
 		unsafe_get_user(*mask, umask++, Efault);
-	user_access_end();
+	user_read_access_end();
 	return 0;
 
 Efault:
-	user_access_end();
+	user_read_access_end();
 	return -EFAULT;
 }
 
@@ -292,7 +292,7 @@ long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
 	bitmap_size = ALIGN(bitmap_size, BITS_PER_COMPAT_LONG);
 	nr_compat_longs = BITS_TO_COMPAT_LONGS(bitmap_size);
 
-	if (!user_access_begin(umask, bitmap_size / 8))
+	if (!user_write_access_begin(umask, bitmap_size / 8))
 		return -EFAULT;
 
 	while (nr_compat_longs > 1) {
@@ -303,10 +303,10 @@ long compat_put_bitmap(compat_ulong_t __user *umask, unsigned long *mask,
 	}
 	if (nr_compat_longs)
 		unsafe_put_user((compat_ulong_t)*mask, umask++, Efault);
-	user_access_end();
+	user_write_access_end();
 	return 0;
 Efault:
-	user_access_end();
+	user_write_access_end();
 	return -EFAULT;
 }
 
diff --git a/kernel/exit.c b/kernel/exit.c
index 2833ffb0c211..b33d3bcf9c9f 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1563,7 +1563,7 @@ SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
 	if (!infop)
 		return err;
 
-	if (!user_access_begin(infop, sizeof(*infop)))
+	if (!user_write_access_begin(infop, sizeof(*infop)))
 		return -EFAULT;
 
 	unsafe_put_user(signo, &infop->si_signo, Efault);
@@ -1572,10 +1572,10 @@ SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
 	unsafe_put_user(info.pid, &infop->si_pid, Efault);
 	unsafe_put_user(info.uid, &infop->si_uid, Efault);
 	unsafe_put_user(info.status, &infop->si_status, Efault);
-	user_access_end();
+	user_write_access_end();
 	return err;
 Efault:
-	user_access_end();
+	user_write_access_end();
 	return -EFAULT;
 }
 
@@ -1690,7 +1690,7 @@ COMPAT_SYSCALL_DEFINE5(waitid,
 	if (!infop)
 		return err;
 
-	if (!user_access_begin(infop, sizeof(*infop)))
+	if (!user_write_access_begin(infop, sizeof(*infop)))
 		return -EFAULT;
 
 	unsafe_put_user(signo, &infop->si_signo, Efault);
@@ -1699,10 +1699,10 @@ COMPAT_SYSCALL_DEFINE5(waitid,
 	unsafe_put_user(info.pid, &infop->si_pid, Efault);
 	unsafe_put_user(info.uid, &infop->si_uid, Efault);
 	unsafe_put_user(info.status, &infop->si_status, Efault);
-	user_access_end();
+	user_write_access_end();
 	return err;
 Efault:
-	user_access_end();
+	user_write_access_end();
 	return -EFAULT;
 }
 #endif
diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index 706020b06617..b90ec550183a 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -116,9 +116,9 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
 
 		kasan_check_write(dst, count);
 		check_object_size(dst, count, false);
-		if (user_access_begin(src, max)) {
+		if (user_read_access_begin(src, max)) {
 			retval = do_strncpy_from_user(dst, src, count, max);
-			user_access_end();
+			user_read_access_end();
 			return retval;
 		}
 	}
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 41670d4a5816..1616710b8a82 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -109,9 +109,9 @@ long strnlen_user(const char __user *str, long count)
 		if (max > count)
 			max = count;
 
-		if (user_access_begin(str, max)) {
+		if (user_read_access_begin(str, max)) {
 			retval = do_strnlen_user(str, count, max);
-			user_access_end();
+			user_read_access_end();
 			return retval;
 		}
 	}
diff --git a/lib/usercopy.c b/lib/usercopy.c
index cbb4d9ec00f2..ca2a697a2061 100644
--- a/lib/usercopy.c
+++ b/lib/usercopy.c
@@ -58,7 +58,7 @@ int check_zeroed_user(const void __user *from, size_t size)
 	from -= align;
 	size += align;
 
-	if (!user_access_begin(from, size))
+	if (!user_read_access_begin(from, size))
 		return -EFAULT;
 
 	unsafe_get_user(val, (unsigned long __user *) from, err_fault);
@@ -79,10 +79,10 @@ int check_zeroed_user(const void __user *from, size_t size)
 		val &= aligned_byte_mask(size);
 
 done:
-	user_access_end();
+	user_read_access_end();
 	return (val == 0);
 err_fault:
-	user_access_end();
+	user_read_access_end();
 	return -EFAULT;
 }
 EXPORT_SYMBOL(check_zeroed_user);
-- 
2.25.0


^ permalink raw reply related

* [PATCH 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Christophe Leroy @ 2020-02-04  7:37 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

Some architectures like powerpc64 have the capability to separate
read access and write access protection.
For get_user() and copy_from_user(), powerpc64 only open read access.
For put_user() and copy_to_user(), powerpc64 only open write access.
But when using unsafe_get_user() or unsafe_put_user(),
user_access_begin open both read and write.

In order to avoid any risk based of hacking some variable parameters
passed to user_access_begin/end that would allow hacking and
leaving user access open or opening too much, it is preferable to
use dedicated static functions that can't be overridden.

Add a user_read_access_begin and user_read_access_end to only open
read access.

Add a user_write_access_begin and user_write_access_end to only open
write access.

By default, when undefined, those new access helpers default on the
existing user_access_begin and user_access_end.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 include/linux/uaccess.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 67f016010aad..9861c89f93be 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -378,6 +378,14 @@ extern long strnlen_unsafe_user(const void __user *unsafe_addr, long count);
 static inline unsigned long user_access_save(void) { return 0UL; }
 static inline void user_access_restore(unsigned long flags) { }
 #endif
+#ifndef user_write_access_begin
+#define user_write_access_begin user_access_begin
+#define user_write_access_end user_access_end
+#endif
+#ifndef user_read_access_begin
+#define user_read_access_begin user_access_begin
+#define user_read_access_end user_access_end
+#endif
 
 #ifdef CONFIG_HARDENED_USERCOPY
 void usercopy_warn(const char *name, const char *detail, bool to_user,
-- 
2.25.0


^ permalink raw reply related

* [powerpc:next-test] BUILD SUCCESS 4c25df5640ae6e4491ee2c50d3f70c1559ef037d
From: kbuild test robot @ 2020-02-04  3:52 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev

tree/branch: https://github.com/linuxppc/linux  next-test
branch HEAD: 4c25df5640ae6e4491ee2c50d3f70c1559ef037d  Merge branch 'topic/user-access-begin' into next

elapsed time: 3868m

configs tested: 265
configs skipped: 13

The following configs have been built successfully.
More configs may be tested in the coming days.

arm                              allmodconfig
arm                               allnoconfig
arm                              allyesconfig
arm                         at91_dt_defconfig
arm                           efm32_defconfig
arm                          exynos_defconfig
arm                        multi_v5_defconfig
arm                        multi_v7_defconfig
arm                        shmobile_defconfig
arm                           sunxi_defconfig
arm64                            allmodconfig
arm64                             allnoconfig
arm64                            allyesconfig
arm64                               defconfig
sparc                            allyesconfig
riscv                            allyesconfig
riscv                          rv32_defconfig
ia64                                defconfig
s390                             allmodconfig
alpha                               defconfig
riscv                               defconfig
s390                          debug_defconfig
xtensa                          iss_defconfig
ia64                             alldefconfig
parisc                         b180_defconfig
parisc                              defconfig
i386                                defconfig
s390                             allyesconfig
m68k                           sun3_defconfig
i386                              allnoconfig
openrisc                 simple_smp_defconfig
nds32                               defconfig
i386                             alldefconfig
i386                             allyesconfig
ia64                             allmodconfig
ia64                              allnoconfig
ia64                             allyesconfig
xtensa                       common_defconfig
openrisc                    or1ksim_defconfig
nios2                         3c120_defconfig
c6x                        evmc6678_defconfig
c6x                              allyesconfig
nios2                         10m50_defconfig
csky                                defconfig
nds32                             allnoconfig
m68k                          multi_defconfig
m68k                       m5475evb_defconfig
h8300                    h8300h-sim_defconfig
h8300                     edosk2674_defconfig
h8300                       h8s-sim_defconfig
m68k                             allmodconfig
arc                              allyesconfig
arc                                 defconfig
microblaze                      mmu_defconfig
microblaze                    nommu_defconfig
powerpc                           allnoconfig
powerpc                             defconfig
powerpc                       ppc64_defconfig
powerpc                          rhel-kconfig
mips                           32r2_defconfig
mips                         64r6el_defconfig
mips                             allmodconfig
mips                              allnoconfig
mips                             allyesconfig
mips                      fuloong2e_defconfig
mips                      malta_kvm_defconfig
parisc                            allnoconfig
parisc                            allyesonfig
parisc                        c3000_defconfig
x86_64               randconfig-a001-20200202
x86_64               randconfig-a002-20200202
x86_64               randconfig-a003-20200202
i386                 randconfig-a001-20200202
i386                 randconfig-a002-20200202
i386                 randconfig-a003-20200202
x86_64               randconfig-a001-20200203
x86_64               randconfig-a002-20200203
i386                 randconfig-a001-20200203
i386                 randconfig-a003-20200203
alpha                randconfig-a001-20200203
m68k                 randconfig-a001-20200203
mips                 randconfig-a001-20200203
nds32                randconfig-a001-20200203
parisc               randconfig-a001-20200203
riscv                randconfig-a001-20200203
alpha                randconfig-a001-20200202
m68k                 randconfig-a001-20200202
nds32                randconfig-a001-20200202
parisc               randconfig-a001-20200202
riscv                randconfig-a001-20200202
mips                 randconfig-a001-20200202
c6x                  randconfig-a001-20200203
h8300                randconfig-a001-20200203
microblaze           randconfig-a001-20200203
nios2                randconfig-a001-20200203
sparc64              randconfig-a001-20200203
h8300                randconfig-a001-20200201
nios2                randconfig-a001-20200201
sparc64              randconfig-a001-20200201
c6x                  randconfig-a001-20200201
c6x                  randconfig-a001-20200202
h8300                randconfig-a001-20200202
microblaze           randconfig-a001-20200202
nios2                randconfig-a001-20200202
sparc64              randconfig-a001-20200202
csky                 randconfig-a001-20200202
openrisc             randconfig-a001-20200202
s390                 randconfig-a001-20200202
sh                   randconfig-a001-20200202
xtensa               randconfig-a001-20200202
sh                   randconfig-a001-20200201
s390                 randconfig-a001-20200201
csky                 randconfig-a001-20200201
xtensa               randconfig-a001-20200201
openrisc             randconfig-a001-20200201
csky                 randconfig-a001-20200203
openrisc             randconfig-a001-20200203
s390                 randconfig-a001-20200203
sh                   randconfig-a001-20200203
xtensa               randconfig-a001-20200203
x86_64               randconfig-b001-20200203
x86_64               randconfig-b002-20200203
x86_64               randconfig-b003-20200203
i386                 randconfig-b001-20200203
i386                 randconfig-b002-20200203
i386                 randconfig-b003-20200203
x86_64               randconfig-b001-20200202
x86_64               randconfig-b002-20200202
x86_64               randconfig-b003-20200202
i386                 randconfig-b001-20200202
i386                 randconfig-b002-20200202
i386                 randconfig-b003-20200202
x86_64               randconfig-b001-20200204
x86_64               randconfig-b002-20200204
x86_64               randconfig-b003-20200204
i386                 randconfig-b001-20200204
i386                 randconfig-b002-20200204
i386                 randconfig-b003-20200204
x86_64               randconfig-c001-20200202
x86_64               randconfig-c002-20200202
x86_64               randconfig-c003-20200202
i386                 randconfig-c001-20200202
i386                 randconfig-c002-20200202
i386                 randconfig-c003-20200202
x86_64               randconfig-c003-20200201
i386                 randconfig-c002-20200201
x86_64               randconfig-c002-20200201
i386                 randconfig-c001-20200201
x86_64               randconfig-c001-20200201
i386                 randconfig-c003-20200201
x86_64               randconfig-c001-20200203
x86_64               randconfig-c002-20200203
x86_64               randconfig-c003-20200203
i386                 randconfig-c001-20200203
i386                 randconfig-c002-20200203
i386                 randconfig-c003-20200203
x86_64               randconfig-c001-20200204
x86_64               randconfig-c002-20200204
x86_64               randconfig-c003-20200204
i386                 randconfig-c001-20200204
i386                 randconfig-c002-20200204
i386                 randconfig-c003-20200204
x86_64               randconfig-d001-20200202
x86_64               randconfig-d002-20200202
x86_64               randconfig-d003-20200202
i386                 randconfig-d001-20200202
i386                 randconfig-d002-20200202
i386                 randconfig-d003-20200202
x86_64               randconfig-e001-20200202
x86_64               randconfig-e002-20200202
x86_64               randconfig-e003-20200202
i386                 randconfig-e001-20200202
i386                 randconfig-e002-20200202
i386                 randconfig-e003-20200202
i386                 randconfig-e003-20200201
i386                 randconfig-e002-20200201
x86_64               randconfig-e001-20200201
x86_64               randconfig-e003-20200201
i386                 randconfig-e001-20200201
x86_64               randconfig-e002-20200201
x86_64               randconfig-f001-20200202
x86_64               randconfig-f002-20200202
x86_64               randconfig-f003-20200202
i386                 randconfig-f001-20200202
i386                 randconfig-f002-20200202
i386                 randconfig-f003-20200202
x86_64               randconfig-g003-20200201
x86_64               randconfig-g001-20200201
i386                 randconfig-g001-20200201
x86_64               randconfig-g002-20200201
i386                 randconfig-g002-20200201
i386                 randconfig-g003-20200201
x86_64               randconfig-g001-20200202
x86_64               randconfig-g002-20200202
x86_64               randconfig-g003-20200202
i386                 randconfig-g001-20200202
i386                 randconfig-g002-20200202
i386                 randconfig-g003-20200202
x86_64               randconfig-h001-20200202
x86_64               randconfig-h002-20200202
x86_64               randconfig-h003-20200202
i386                 randconfig-h001-20200202
i386                 randconfig-h002-20200202
i386                 randconfig-h003-20200202
x86_64               randconfig-h001-20200203
x86_64               randconfig-h002-20200203
x86_64               randconfig-h003-20200203
i386                 randconfig-h001-20200203
i386                 randconfig-h002-20200203
i386                 randconfig-h003-20200203
x86_64               randconfig-h001-20200204
x86_64               randconfig-h002-20200204
x86_64               randconfig-h003-20200204
i386                 randconfig-h001-20200204
i386                 randconfig-h002-20200204
i386                 randconfig-h003-20200204
x86_64               randconfig-h001-20200201
i386                 randconfig-h002-20200201
x86_64               randconfig-h002-20200201
i386                 randconfig-h003-20200201
x86_64               randconfig-h003-20200201
i386                 randconfig-h001-20200201
ia64                 randconfig-a001-20200203
arm                  randconfig-a001-20200203
arc                  randconfig-a001-20200203
arm64                randconfig-a001-20200203
powerpc              randconfig-a001-20200203
sparc                randconfig-a001-20200203
powerpc              randconfig-a001-20200202
arc                  randconfig-a001-20200202
ia64                 randconfig-a001-20200202
sparc                randconfig-a001-20200202
arm64                randconfig-a001-20200202
arm                  randconfig-a001-20200202
arc                  randconfig-a001-20200201
ia64                 randconfig-a001-20200201
sparc                randconfig-a001-20200201
arm64                randconfig-a001-20200201
arm                  randconfig-a001-20200201
riscv                            allmodconfig
riscv                             allnoconfig
riscv                    nommu_virt_defconfig
s390                             alldefconfig
s390                              allnoconfig
s390                                defconfig
s390                       zfcpdump_defconfig
sh                               allmodconfig
sh                                allnoconfig
sh                          rsk7269_defconfig
sh                  sh7785lcr_32bit_defconfig
sh                            titan_defconfig
sparc64                          allmodconfig
sparc64                          allyesconfig
sparc                               defconfig
sparc64                           allnoconfig
sparc64                             defconfig
um                                  defconfig
um                             i386_defconfig
um                           x86_64_defconfig
x86_64                              fedora-25
x86_64                                  kexec
x86_64                                    lkp
x86_64                                   rhel
x86_64                         rhel-7.2-clear
x86_64                               rhel-7.6

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

^ permalink raw reply

* Re: [PATCH v6 00/10] mm/memory_hotplug: Shrink zones before removing memory
From: Andrew Morton @ 2020-02-04  1:46 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Mark Rutland, Pankaj Gupta, Michal Hocko, linux-ia64, linux-sh,
	Peter Zijlstra, Catalin Marinas, Dave Hansen, Heiko Carstens,
	Wei Yang, linux-mm, Pavel Tatashin, Rich Felker,
	Alexander Potapenko, H. Peter Anvin, Alexander Duyck, Ira Weiny,
	Thomas Gleixner, Qian Cai, linux-s390, Yu Zhao, Yoshinori Sato,
	Jason Gunthorpe, Aneesh Kumar K . V, x86, Matthew Wilcox (Oracle),
	Mike Rapoport, Halil Pasic, Christian Borntraeger, Ingo Molnar,
	Gerald Schaefer, Fenghua Yu, Pavel Tatashin, Vasily Gorbik,
	Anshuman Khandual, Vlastimil Babka, Will Deacon, Robin Murphy,
	Jun Yao, Borislav Petkov, Andy Lutomirski, Dan Williams,
	linux-arm-kernel, Oscar Salvador, Tony Luck, Mel Gorman,
	Masahiro Yamada, Greg Kroah-Hartman, Steve Capper, linux-kernel,
	Logan Gunthorpe, Wei Yang, Paul Mackerras, Tom Lendacky,
	linuxppc-dev
In-Reply-To: <f7ed4448-8f41-599d-4689-914eeaf84d6d@redhat.com>

On Fri, 31 Jan 2020 10:18:34 +0100 David Hildenbrand <david@redhat.com> wrote:

> On 31.01.20 05:40, Andrew Morton wrote:
> > On Tue, 3 Dec 2019 14:36:38 +0100 Oscar Salvador <osalvador@suse.de> wrote:
> > 
> >> On Mon, Dec 02, 2019 at 10:09:51AM +0100, David Hildenbrand wrote:
> >>> @Michal, @Oscar, can some of you at least have a patch #5 now so we can
> >>> proceed with that? (the other patches can stay in -next some time longer)
> >>
> >> Hi, 
> >>
> >> I will be having a look at patch#5 shortly.
> >>
> >> Thanks for the reminder
> > 
> > Things haven't improved a lot :(
> > 
> > mm-memmap_init-update-variable-name-in-memmap_init_zone.patch
> > mm-memory_hotplug-poison-memmap-in-remove_pfn_range_from_zone.patch
> > mm-memory_hotplug-we-always-have-a-zone-in-find_smallestbiggest_section_pfn.patch
> > mm-memory_hotplug-dont-check-for-all-holes-in-shrink_zone_span.patch
> > mm-memory_hotplug-drop-local-variables-in-shrink_zone_span.patch
> > mm-memory_hotplug-cleanup-__remove_pages.patch
> > 
> > The first patch has reviews, the remainder are unloved.
> 
> Trying hard not to rant about the review mentality on this list, but I'm
> afraid I can't totally bite my tongue ... :)
> 
> Now, this is an uncomfortable situation for you and me. You have to ping
> people about review and patches are stuck in your tree. I have a growing
> list of patches that are somewhat considered "done", but well,
> not-upstream-at-all. I have patches that are long in RHEL and were
> properly tested, but could get dropped any time because -ENOREVIEW.
> 
> Our process nowadays seems to be, to only upstream what has an ACK/RB
> (fixes/features/cleanups).

Yes, we've been doing this for a couple of years now.  I make an
exception for Vitaly's zswap patches because he appears to be the only
person who knows the code (since Harry's internship ended).

I think this is the first time we've hit a significant logjam. 
Presumably the holiday season contributed to this.

It isn't clear to me that we've gained much from this policy.  But
until this cycle I've seen little harm.

> I can understand this is desirable (yet, I am
> not sure if this makes sense with the current take-and-not-give-back
> review mentality on this list).
> 
> Although it will make upstreaming stuff *even harder* and *even slower*,
> maybe we should start to only queue patches that have an ACK/RB, so they
> won't get blocked by this later on? At least that makes your life easier
> and people won't have to eventually follow up on patches that have been
> in linux-next for months.

The merge rate would still be the review rate, but the resulting merges
would be of less tested code.

> Note: the result will be that many of my patches will still not get
> reviewed, won't get queued/upstreamed, I will continuously ping and
> resend, I will lose interest because I have better things to do, I will
> lose interest in our code quality, I will lose interest to review.
> 
> (side note: some people might actually enjoy me sending less cleanup
> patches, so this approach might be desirable for some ;) )
> 
> One alternative is to send patches upstream once they have been lying
> around in linux-next for $RANDOM number of months, because they
> obviously saw some testing and nobody started to yell at them once
> stumbling over them on linux-mm.

Yes, I think that's the case with these patches and I've sent them to
Linus.  Hopefully Michel will be able to find time to look them over in
the next month or so.


^ permalink raw reply

* Re: Latest Git kernel: avahi-daemon[2410]: ioctl(): Inappropriate ioctl for device
From: Jakub Kicinski @ 2020-02-03 17:53 UTC (permalink / raw)
  To: Christian Zigotzky
  Cc: DTML, Darren Stevens, mad skateman, Linux Kernel Mailing List,
	linuxppc-dev, contact@a-eon.com, R.T.Dickinson,
	netdev@vger.kernel.org, Christoph Hellwig
In-Reply-To: <9624aebf-edb9-a3b0-1a29-b61df6b7ba2f@xenosoft.de>

On Sun, 2 Feb 2020 16:02:18 +0100, Christian Zigotzky wrote:
> On 02 February 2020 at 09:19 am, Christophe Leroy wrote:
> > Hello,
> >
> > Le 02/02/2020 à 01:08, Christian Zigotzky a écrit :  
> >> Hello,
> >>
> >> We regularly compile and test Linux kernels every day during the 
> >> merge window. Since Thuesday we have very high CPU loads because of 
> >> the avahi daemon on our desktop Linux systems (Ubuntu, Debian etc).
> >>
> >> Error message: avahi-daemon[2410]: ioctl(): Inappropriate ioctl for 
> >> device  
> >
> > Do you know which ioctl, on which device ?
> > Can you take a trace of running avahi-daemon with 'strace' ?
> >
> > Can you bisect ?
> >
> > Christophe  
> Hi Christophe,
> Hi All,
> 
> I figured out that the avahi-daemon has a problem with the IPv6 address 
> of a network interface since the Git kernel from Thursday. (Log attached)
> This generates high CPU usage because the avahi-daemon tries to access 
> the IPv6 address again and again and thereby it produces a lot of log 
> messages.
> 
> We figured out that the networking updates aren't responsible for this 
> issue because we created a test kernel on Wednesday. The issue is 
> somewhere in the commits from Wednesday night to Thursday (CET).

FWIW Thursday is when the latest networking pull came in, so could well
be networking related..

> Please compile the latest Git kernel and test it with a desktop linux 
> distribution for example Ubuntu. In my point of view there are many 
> desktop machines affected. Many server systems don't use the avahi 
> daemon so they aren't affected.
> 
> It's possible to deactivate the access to the IPv6 address with the 
> following line in the file "/etc/avahi/avahi-daemon.conf":
> 
> use-ipv6=no
> 
> After a reboot the CPU usage is normal again. This is only a temporary 
> solution.
> 
> Unfortunately I don't have the time for bisecting next week. I have a 
> lot of other work to do. In my point of view it is very important that 
> you also compile the latest Git kernels. Then you will see the issue and 
> then you have a better possibility to fix the issue.

^ permalink raw reply

* Re: [PATCH] powerpc/drmem: cache LMBs in xarray to accelerate lookup
From: Scott Cheloha @ 2020-02-03 20:13 UTC (permalink / raw)
  To: Fontenot, Nathan, Nathan Lynch; +Cc: linuxppc-dev, linux-kernel, Rick Lindsley
In-Reply-To: <4dfb2f93-7af8-8c5f-854c-22afead18a8c@gmail.com>

On Thu, Jan 30, 2020 at 10:09:32AM -0600, Fontenot, Nathan wrote:
> On 1/29/2020 12:10 PM, Scott Cheloha wrote:
> > On Tue, Jan 28, 2020 at 05:56:55PM -0600, Nathan Lynch wrote:
> >> Scott Cheloha <cheloha@linux.ibm.com> writes:
> >>> LMB lookup is currently an O(n) linear search.  This scales poorly when
> >>> there are many LMBs.
> >>>
> >>> If we cache each LMB by both its base address and its DRC index
> >>> in an xarray we can cut lookups to O(log n), greatly accelerating
> >>> drmem initialization and memory hotplug.
> >>>
> >>> This patch introduces two xarrays of of LMBs and fills them during
> >>> drmem initialization.  The patch also adds two interfaces for LMB
> >>> lookup.
> >>
> >> Good but can you replace the array of LMBs altogether
> >> (drmem_info->lmbs)? xarray allows iteration over the members if needed.
> > 
> > I don't think we can without potentially changing the current behavior.
> > 
> > The current behavior in dlpar_memory_{add,remove}_by_ic() is to advance
> > linearly through the array from the LMB with the matching DRC index.
> > 
> > Iteration through the xarray via xa_for_each_start() will return LMBs
> > indexed with monotonically increasing DRC indices.> 
> > Are they equivalent?  Or can we have an LMB with a smaller DRC index
> > appear at a greater offset in the array?
> > 
> > If the following condition is possible:
> > 
> > 	drmem_info->lmbs[i].drc_index > drmem_info->lmbs[j].drc_index
> > 
> > where i < j, then we have a possible behavior change because
> > xa_for_each_start() may not return a contiguous array slice.  It might
> > "leap backwards" in the array.  Or it might skip over a chunk of LMBs.
> > 
> 
> The LMB array should have each LMB in monotonically increasing DRC Index
> value. Note that this is set up based on the DT property but I don't recall
> ever seeing the DT specify LMBs out of order or not being contiguous.

Is that ordering guaranteed by the PAPR or some other spec or is that
just a convention?

Code like drmem_update_dt_v1() makes me very nervous:

static int drmem_update_dt_v1(struct device_node *memory,
                              struct property *prop)
{
        struct property *new_prop;
        struct of_drconf_cell_v1 *dr_cell;
        struct drmem_lmb *lmb;
        u32 *p;

        new_prop = clone_property(prop, prop->length);
        if (!new_prop)
                return -1;

        p = new_prop->value;
        *p++ = cpu_to_be32(drmem_info->n_lmbs);

        dr_cell = (struct of_drconf_cell_v1 *)p;

        for_each_drmem_lmb(lmb) {
                dr_cell->base_addr = cpu_to_be64(lmb->base_addr);
                dr_cell->drc_index = cpu_to_be32(lmb->drc_index);
                dr_cell->aa_index = cpu_to_be32(lmb->aa_index);
                dr_cell->flags = cpu_to_be32(drmem_lmb_flags(lmb));

                dr_cell++;
        }

        of_update_property(memory, new_prop);
        return 0;
}

If for whatever reason the firmware has a DRC that isn't monotonically
increasing and we update a firmware property at the wrong offset I have
no idea what would happen.

With the array we preserve the order.  Without it we might violate
some assumption the firmware has made.

^ permalink raw reply

* [PATCH 1/2] kbuild: Add config fragment merge functionality
From: Nicolas Saenz Julienne @ 2020-02-03 18:48 UTC (permalink / raw)
  To: linux, linux-arm-kernel, linuxppc-dev, linux-kbuild,
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Masahiro Yamada, Michal Marek
  Cc: f.fainelli, linux-kernel, yamada.masahiro,
	bcm-kernel-feedback-list, linux-rpi-kernel,
	Nicolas Saenz Julienne
In-Reply-To: <20200203184820.4433-1-nsaenzjulienne@suse.de>

So far this function was only used locally in powerpc, some other
architectures might benefit from it. Move it into
scripts/Makefile.defconf.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 arch/powerpc/Makefile    | 12 +-----------
 scripts/Makefile.defconf | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 11 deletions(-)
 create mode 100644 scripts/Makefile.defconf

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index f35730548e42..517ef8ec774b 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -301,17 +301,7 @@ $(BOOT_TARGETS2): vmlinux
 bootwrapper_install:
 	$(Q)$(MAKE) $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
 
-# Used to create 'merged defconfigs'
-# To use it $(call) it with the first argument as the base defconfig
-# and the second argument as a space separated list of .config files to merge,
-# without the .config suffix.
-define merge_into_defconfig
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/kconfig/merge_config.sh \
-		-m -O $(objtree) $(srctree)/arch/$(ARCH)/configs/$(1) \
-		$(foreach config,$(2),$(srctree)/arch/$(ARCH)/configs/$(config).config)
-	+$(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig
-endef
-
+include $(srctree)/scripts/Makefile.defconf
 PHONY += pseries_le_defconfig
 pseries_le_defconfig:
 	$(call merge_into_defconfig,pseries_defconfig,le)
diff --git a/scripts/Makefile.defconf b/scripts/Makefile.defconf
new file mode 100644
index 000000000000..ab332f7534f5
--- /dev/null
+++ b/scripts/Makefile.defconf
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+# Configuration heplers
+
+# Creates 'merged defconfigs'
+# ---------------------------------------------------------------------------
+# Usage:
+#   $(call merge_into_defconfig,base_config,config_fragment1 config_fragment2 ...)
+#
+# Input config fragments without '.config' suffix
+define merge_into_defconfig
+	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/kconfig/merge_config.sh \
+		-m -O $(objtree) $(srctree)/arch/$(ARCH)/configs/$(1) \
+		$(foreach config,$(2),$(srctree)/arch/$(ARCH)/configs/$(config).config)
+	+$(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig
+endef
-- 
2.25.0


^ permalink raw reply related

* [PATCH 0/2] ARM: Introduce multi_v7_lpae_defconfig
From: Nicolas Saenz Julienne @ 2020-02-03 18:48 UTC (permalink / raw)
  To: linux, linux-arm-kernel, linuxppc-dev, linux-kbuild
  Cc: f.fainelli, linux-kernel, yamada.masahiro,
	bcm-kernel-feedback-list, linux-rpi-kernel,
	Nicolas Saenz Julienne

This series introduces a new configuration target,
multi_v7_lpae_defconfig, built by merging the config fragment
lpae.config with mult_v7_defconfig. Ultimately needed in order for
Raspberry Pi 4's PCIe bus to work on arm builds, but which may benefit
other boards out there.

---

Changes since RFC:
 - Move merge function into the scripts directory.

Nicolas Saenz Julienne (2):
  kbuild: Add config fragment merge functionality
  ARM: add multi_v7_lpae_defconfig

 arch/arm/Makefile            |  4 ++++
 arch/arm/configs/lpae.config |  1 +
 arch/powerpc/Makefile        | 12 +-----------
 scripts/Makefile.defconf     | 15 +++++++++++++++
 4 files changed, 21 insertions(+), 11 deletions(-)
 create mode 100644 arch/arm/configs/lpae.config
 create mode 100644 scripts/Makefile.defconf

-- 
2.25.0


^ permalink raw reply

* [PATCH 2/2] ARM: add multi_v7_lpae_defconfig
From: Nicolas Saenz Julienne @ 2020-02-03 18:48 UTC (permalink / raw)
  To: linux, linux-arm-kernel, linuxppc-dev, linux-kbuild
  Cc: f.fainelli, linux-kernel, yamada.masahiro,
	bcm-kernel-feedback-list, linux-rpi-kernel,
	Nicolas Saenz Julienne
In-Reply-To: <20200203184820.4433-1-nsaenzjulienne@suse.de>

The only missing configuration option preventing us from using
multi_v7_defconfig with the Raspberry Pi 4 is ARM_LPAE. It's needed as
the PCIe controller found on the SoC depends on 64bit addressing, yet
can't be included as not all v7 boards support LPAE.

Introduce multi_v7_lpae_defconfig, built off multi_v7_defconfig, which will
avoid us having to duplicate and maintain multiple similar configurations.

Needless to say the Raspberry Pi 4 is not the only platform that can
benefit from this new configuration.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

---

Changes since RFC:
 - Move config merge function into scripts folder
---
 arch/arm/Makefile            | 4 ++++
 arch/arm/configs/lpae.config | 1 +
 2 files changed, 5 insertions(+)
 create mode 100644 arch/arm/configs/lpae.config

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 16d41efea7f2..1f4f9a90561d 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -359,6 +359,10 @@ archclean:
 # My testing targets (bypasses dependencies)
 bp:;	$(Q)$(MAKE) $(build)=$(boot) MACHINE=$(MACHINE) $(boot)/bootpImage
 
+include $(srctree)/scripts/Makefile.defconf
+PHONY += multi_v7_lpae_defconfig
+multi_v7_lpae_defconfig:
+	$(call merge_into_defconfig,multi_v7_defconfig,lpae)
 
 define archhelp
   echo  '* zImage        - Compressed kernel image (arch/$(ARCH)/boot/zImage)'
diff --git a/arch/arm/configs/lpae.config b/arch/arm/configs/lpae.config
new file mode 100644
index 000000000000..19bab134e014
--- /dev/null
+++ b/arch/arm/configs/lpae.config
@@ -0,0 +1 @@
+CONFIG_ARM_LPAE=y
-- 
2.25.0


^ permalink raw reply related

* Re: [PATCH] powerpc/32s: Slenderize _tlbia() for powerpc 603/603e
From: Christophe Leroy @ 2020-02-03 17:16 UTC (permalink / raw)
  To: Joakim Tjernlund, mpe@ellerman.id.au, paulus@samba.org,
	benh@kernel.crashing.org
  Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <bfab6635148b83deed8ac9fcbb19dde8c32fb988.camel@infinera.com>



Le 03/02/2020 à 17:57, Joakim Tjernlund a écrit :
> On Mon, 2020-02-03 at 16:47 +0000, Christophe Leroy wrote:
>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>
>>
>> _tlbia() is a function used only on 603/603e core, ie on CPUs which
>> don't have a hash table.
>>
>> _tlbia() uses the tlbia macro which implements a loop of 1024 tlbie.
>>
>> On the 603/603e core, flushing the entire TLB requires no more than
>> 32 tlbie.
>>
>> Replace tlbia by a loop of 32 tlbie.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   arch/powerpc/mm/book3s32/hash_low.S | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/book3s32/hash_low.S b/arch/powerpc/mm/book3s32/hash_low.S
>> index c11b0a005196..a5039ad10429 100644
>> --- a/arch/powerpc/mm/book3s32/hash_low.S
>> +++ b/arch/powerpc/mm/book3s32/hash_low.S
>> @@ -696,18 +696,21 @@ _GLOBAL(_tlbia)
>>          bne-    10b
>>          stwcx.  r8,0,r9
>>          bne-    10b
>> +#endif /* CONFIG_SMP */
>> +       li      r5, 32
>> +       lis     r4, KERNELBASE@h
>> +       mtctr   r5
>>          sync
>> -       tlbia
>> +0:     tlbie   r4
>> +       addi    r4, r4, 0x1000
> 
> Is page size always 4096 here or does it not matter ?

603 and its derivatives (G2, e300, ...) only support 4k pages.

And regardless, the reference manual says:

The tlbia instruction is not implemented on the MPC603e and when its 
opcode is encountered, an illegal instruction program exception is 
generated. To invalidate all entries of both TLBs, 32 tlbie instructions 
must be executed, incrementing the value in EA[15–19] by 1 each time

Christophe

^ permalink raw reply

* Re: [PATCH 5/5] libnvdimm/region: Introduce an 'align' attribute
From: Aneesh Kumar K.V @ 2020-02-03 17:10 UTC (permalink / raw)
  To: Dan Williams, linux-nvdimm; +Cc: linuxppc-dev, hch, linux-kernel
In-Reply-To: <158041478371.3889308.14542630147672668068.stgit@dwillia2-desk3.amr.corp.intel.com>

Dan Williams <dan.j.williams@intel.com> writes:

> The align attribute applies an alignment constraint for namespace
> creation in a region. Whereas the 'align' attribute of a namespace
> applied alignment padding via an info block, the 'align' attribute
> applies alignment constraints to the free space allocation.
>
> The default for 'align' is the maximum known memremap_compat_align()
> across all archs (16MiB from PowerPC at time of writing) multiplied by
> the number of interleave ways if there is blk-aliasing. The minimum is
> PAGE_SIZE and allows for the creation of cross-arch incompatible
> namespaces, just as previous kernels allowed, but the expectation is
> cross-arch and mode-independent compatibility by default.
>
> The regression risk with this change is limited to cases that were
> dependent on the ability to create unaligned namespaces, *and* for some
> reason are unable to opt-out of aligned namespaces by writing to
> 'regionX/align'. If such a scenario arises the default can be flipped
> from opt-out to opt-in of compat-aligned namespace creation, but that is
> a last resort. The kernel will otherwise continue to support existing
> defined misaligned namespaces.
>
> Unfortunately this change needs to touch several parts of the
> implementation at once:
>
> - region/available_size: expand busy extents to current align
> - region/max_available_extent: expand busy extents to current align
> - namespace/size: trim free space to current align
>
> ...to keep the free space accounting conforming to the dynamic align
> setting.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Reported-by: Jeff Moyer <jmoyer@redhat.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/nvdimm/dimm_devs.c      |   86 +++++++++++++++++++++++----
>  drivers/nvdimm/namespace_devs.c |    9 ++-
>  drivers/nvdimm/nd.h             |    1 
>  drivers/nvdimm/region_devs.c    |  122 ++++++++++++++++++++++++++++++++++++---
>  4 files changed, 192 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
> index 64159d4d4b8f..b4994abb655f 100644
> --- a/drivers/nvdimm/dimm_devs.c
> +++ b/drivers/nvdimm/dimm_devs.c
> @@ -563,6 +563,21 @@ int nvdimm_security_freeze(struct nvdimm *nvdimm)
>  	return rc;
>  }
>  
> +static unsigned long dpa_align(struct nd_region *nd_region)
> +{
> +	struct device *dev = &nd_region->dev;
> +
> +	if (dev_WARN_ONCE(dev, !is_nvdimm_bus_locked(dev),
> +				"bus lock required for capacity provision\n"))
> +		return 0;
> +	if (dev_WARN_ONCE(dev, !nd_region->ndr_mappings || nd_region->align
> +				% nd_region->ndr_mappings,
> +				"invalid region align %#lx mappings: %d\n",
> +				nd_region->align, nd_region->ndr_mappings))
> +		return 0;
> +	return nd_region->align / nd_region->ndr_mappings;
> +}
> +
>  int alias_dpa_busy(struct device *dev, void *data)
>  {
>  	resource_size_t map_end, blk_start, new;
> @@ -571,6 +586,7 @@ int alias_dpa_busy(struct device *dev, void *data)
>  	struct nd_region *nd_region;
>  	struct nvdimm_drvdata *ndd;
>  	struct resource *res;
> +	unsigned long align;
>  	int i;
>  
>  	if (!is_memory(dev))
> @@ -608,13 +624,21 @@ int alias_dpa_busy(struct device *dev, void *data)
>  	 * Find the free dpa from the end of the last pmem allocation to
>  	 * the end of the interleave-set mapping.
>  	 */
> +	align = dpa_align(nd_region);
> +	if (!align)
> +		return 0;
> +
>  	for_each_dpa_resource(ndd, res) {
> +		resource_size_t start, end;
> +
>  		if (strncmp(res->name, "pmem", 4) != 0)
>  			continue;
> -		if ((res->start >= blk_start && res->start < map_end)
> -				|| (res->end >= blk_start
> -					&& res->end <= map_end)) {
> -			new = max(blk_start, min(map_end + 1, res->end + 1));
> +
> +		start = ALIGN_DOWN(res->start, align);
> +		end = ALIGN(res->end + 1, align) - 1;
> +		if ((start >= blk_start && start < map_end)
> +				|| (end >= blk_start && end <= map_end)) {
> +			new = max(blk_start, min(map_end, end) + 1);
>  			if (new != blk_start) {
>  				blk_start = new;
>  				goto retry;
> @@ -654,6 +678,7 @@ resource_size_t nd_blk_available_dpa(struct nd_region *nd_region)
>  		.res = NULL,
>  	};
>  	struct resource *res;
> +	unsigned long align;
>  
>  	if (!ndd)
>  		return 0;
> @@ -661,10 +686,20 @@ resource_size_t nd_blk_available_dpa(struct nd_region *nd_region)
>  	device_for_each_child(&nvdimm_bus->dev, &info, alias_dpa_busy);
>  
>  	/* now account for busy blk allocations in unaliased dpa */
> +	align = dpa_align(nd_region);
> +	if (!align)
> +		return 0;
>  	for_each_dpa_resource(ndd, res) {
> +		resource_size_t start, end, size;
> +
>  		if (strncmp(res->name, "blk", 3) != 0)
>  			continue;
> -		info.available -= resource_size(res);
> +		start = ALIGN_DOWN(res->start, align);
> +		end = ALIGN(res->end + 1, align) - 1;
> +		size = end - start + 1;
> +		if (size >= info.available)
> +			return 0;
> +		info.available -= size;
>  	}
>  
>  	return info.available;
> @@ -683,19 +718,31 @@ resource_size_t nd_pmem_max_contiguous_dpa(struct nd_region *nd_region,
>  	struct nvdimm_bus *nvdimm_bus;
>  	resource_size_t max = 0;
>  	struct resource *res;
> +	unsigned long align;
>  
>  	/* if a dimm is disabled the available capacity is zero */
>  	if (!ndd)
>  		return 0;
>  
> +	align = dpa_align(nd_region);
> +	if (!align)
> +		return 0;
> +
>  	nvdimm_bus = walk_to_nvdimm_bus(ndd->dev);
>  	if (__reserve_free_pmem(&nd_region->dev, nd_mapping->nvdimm))
>  		return 0;
>  	for_each_dpa_resource(ndd, res) {
> +		resource_size_t start, end;
> +
>  		if (strcmp(res->name, "pmem-reserve") != 0)
>  			continue;
> -		if (resource_size(res) > max)
> -			max = resource_size(res);
> +		/* trim free space relative to current alignment setting */
> +		start = ALIGN(res->start, align);
> +		end = ALIGN_DOWN(res->end + 1, align) - 1;
> +		if (end < start)
> +			continue;
> +		if (end - start + 1 > max)
> +			max = end - start + 1;
>  	}
>  	release_free_pmem(nvdimm_bus, nd_mapping);
>  	return max;
> @@ -723,24 +770,33 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
>  	struct nvdimm_drvdata *ndd = to_ndd(nd_mapping);
>  	struct resource *res;
>  	const char *reason;
> +	unsigned long align;
>  
>  	if (!ndd)
>  		return 0;
>  
> +	align = dpa_align(nd_region);
> +	if (!align)
> +		return 0;
> +
>  	map_start = nd_mapping->start;
>  	map_end = map_start + nd_mapping->size - 1;
>  	blk_start = max(map_start, map_end + 1 - *overlap);
>  	for_each_dpa_resource(ndd, res) {
> -		if (res->start >= map_start && res->start < map_end) {
> +		resource_size_t start, end;
> +
> +		start = ALIGN_DOWN(res->start, align);
> +		end = ALIGN(res->end + 1, align) - 1;
> +		if (start >= map_start && start < map_end) {
>  			if (strncmp(res->name, "blk", 3) == 0)
>  				blk_start = min(blk_start,
> -						max(map_start, res->start));
> -			else if (res->end > map_end) {
> +						max(map_start, start));
> +			else if (end > map_end) {
>  				reason = "misaligned to iset";
>  				goto err;
>  			} else
> -				busy += resource_size(res);
> -		} else if (res->end >= map_start && res->end <= map_end) {
> +				busy += end - start + 1;
> +		} else if (end >= map_start && end <= map_end) {
>  			if (strncmp(res->name, "blk", 3) == 0) {
>  				/*
>  				 * If a BLK allocation overlaps the start of
> @@ -749,8 +805,8 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
>  				 */
>  				blk_start = map_start;
>  			} else
> -				busy += resource_size(res);
> -		} else if (map_start > res->start && map_start < res->end) {
> +				busy += end - start + 1;
> +		} else if (map_start > start && map_start < end) {
>  			/* total eclipse of the mapping */
>  			busy += nd_mapping->size;
>  			blk_start = map_start;
> @@ -760,7 +816,7 @@ resource_size_t nd_pmem_available_dpa(struct nd_region *nd_region,
>  	*overlap = map_end + 1 - blk_start;
>  	available = blk_start - map_start;
>  	if (busy < available)
> -		return available - busy;
> +		return ALIGN_DOWN(available - busy, align);
>  	return 0;
>  
>   err:
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index 30cda9f235de..4720ad69e1c5 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -541,6 +541,11 @@ static void space_valid(struct nd_region *nd_region, struct nvdimm_drvdata *ndd,
>  {
>  	bool is_reserve = strcmp(label_id->id, "pmem-reserve") == 0;
>  	bool is_pmem = strncmp(label_id->id, "pmem", 4) == 0;
> +	unsigned long align;
> +
> +	align = nd_region->align / nd_region->ndr_mappings;
> +	valid->start = ALIGN(valid->start, align);
> +	valid->end = ALIGN_DOWN(valid->end + 1, align) - 1;
>  
>  	if (valid->start >= valid->end)
>  		goto invalid;
> @@ -980,10 +985,10 @@ static ssize_t __size_store(struct device *dev, unsigned long long val)
>  		return -ENXIO;
>  	}
>  
> -	div_u64_rem(val, PAGE_SIZE * nd_region->ndr_mappings, &remainder);
> +	div_u64_rem(val, nd_region->align, &remainder);
>  	if (remainder) {
>  		dev_dbg(dev, "%llu is not %ldK aligned\n", val,
> -				(PAGE_SIZE * nd_region->ndr_mappings) / SZ_1K);
> +				nd_region->align / SZ_1K);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index ca39abe29c7c..c4d69c1cce55 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -146,6 +146,7 @@ struct nd_region {
>  	struct device *btt_seed;
>  	struct device *pfn_seed;
>  	struct device *dax_seed;
> +	unsigned long align;
>  	u16 ndr_mappings;
>  	u64 ndr_size;
>  	u64 ndr_start;
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index a5fc6e4c56ff..bf239e783940 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -216,21 +216,25 @@ int nd_region_to_nstype(struct nd_region *nd_region)
>  }
>  EXPORT_SYMBOL(nd_region_to_nstype);
>  
> -static ssize_t size_show(struct device *dev,
> -		struct device_attribute *attr, char *buf)
> +static unsigned long long region_size(struct nd_region *nd_region)
>  {
> -	struct nd_region *nd_region = to_nd_region(dev);
> -	unsigned long long size = 0;
> -
> -	if (is_memory(dev)) {
> -		size = nd_region->ndr_size;
> +	if (is_memory(&nd_region->dev)) {
> +		return nd_region->ndr_size;
>  	} else if (nd_region->ndr_mappings == 1) {
>  		struct nd_mapping *nd_mapping = &nd_region->mapping[0];
>  
> -		size = nd_mapping->size;
> +		return nd_mapping->size;
>  	}
>  
> -	return sprintf(buf, "%llu\n", size);
> +	return 0;
> +}
> +
> +static ssize_t size_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct nd_region *nd_region = to_nd_region(dev);
> +
> +	return sprintf(buf, "%llu\n", region_size(nd_region));
>  }
>  static DEVICE_ATTR_RO(size);
>  
> @@ -529,6 +533,55 @@ static ssize_t read_only_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(read_only);
>  
> +static ssize_t align_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct nd_region *nd_region = to_nd_region(dev);
> +
> +	return sprintf(buf, "%#lx\n", nd_region->align);
> +}
> +
> +static ssize_t align_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t len)
> +{
> +	struct nd_region *nd_region = to_nd_region(dev);
> +	unsigned long val, dpa;
> +	u32 remainder;
> +	int rc;
> +
> +	rc = kstrtoul(buf, 0, &val);
> +	if (rc)
> +		return rc;
> +
> +	if (!nd_region->ndr_mappings)
> +		return -ENXIO;
> +
> +	/*
> +	 * Ensure space-align is evenly divisible by the region
> +	 * interleave-width because the kernel typically has no facility
> +	 * to determine which DIMM(s), dimm-physical-addresses, would
> +	 * contribute to the tail capacity in system-physical-address
> +	 * space for the namespace.
> +	 */
> +	dpa = val;
> +	remainder = do_div(dpa, nd_region->ndr_mappings);
> +	if (!is_power_of_2(dpa) || dpa < PAGE_SIZE
> +			|| val > region_size(nd_region) || remainder)
> +		return -EINVAL;
> +
> +	/*
> +	 * Given that space allocation consults this value multiple
> +	 * times ensure it does not change for the duration of the
> +	 * allocation.
> +	 */
> +	nvdimm_bus_lock(dev);
> +	nd_region->align = val;
> +	nvdimm_bus_unlock(dev);
> +
> +	return len;
> +}
> +static DEVICE_ATTR_RW(align);
> +
>  static ssize_t region_badblocks_show(struct device *dev,
>  		struct device_attribute *attr, char *buf)
>  {
> @@ -571,6 +624,7 @@ static DEVICE_ATTR_RO(persistence_domain);
>  
>  static struct attribute *nd_region_attributes[] = {
>  	&dev_attr_size.attr,
> +	&dev_attr_align.attr,
>  	&dev_attr_nstype.attr,
>  	&dev_attr_mappings.attr,
>  	&dev_attr_btt_seed.attr,
> @@ -626,6 +680,19 @@ static umode_t region_visible(struct kobject *kobj, struct attribute *a, int n)
>  		return a->mode;
>  	}
>  
> +	if (a == &dev_attr_align.attr) {
> +		int i;
> +
> +		for (i = 0; i < nd_region->ndr_mappings; i++) {
> +			struct nd_mapping *nd_mapping = &nd_region->mapping[i];
> +			struct nvdimm *nvdimm = nd_mapping->nvdimm;
> +
> +			if (test_bit(NDD_LABELING, &nvdimm->flags))
> +				return a->mode;
> +		}
> +		return 0;
> +	}
> +
>  	if (a != &dev_attr_set_cookie.attr
>  			&& a != &dev_attr_available_size.attr)
>  		return a->mode;
> @@ -935,6 +1002,42 @@ void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane)
>  }
>  EXPORT_SYMBOL(nd_region_release_lane);
>  
> +/*
> + * PowerPC requires this alignment for memremap_pages(). All other archs
> + * should be ok with SUBSECTION_SIZE (see memremap_compat_align()).
> + */
> +#define MEMREMAP_COMPAT_ALIGN_MAX SZ_16M
> +
> +static unsigned long default_align(struct nd_region *nd_region)
> +{
> +	unsigned long align, per_mapping;
> +	int i, mappings;
> +	u32 remainder;
> +
> +	if (is_nd_blk(&nd_region->dev))
> +		align = PAGE_SIZE;
> +	else
> +		align = MEMREMAP_COMPAT_ALIGN_MAX;
> +
> +	for (i = 0; i < nd_region->ndr_mappings; i++) {
> +		struct nd_mapping *nd_mapping = &nd_region->mapping[i];
> +		struct nvdimm *nvdimm = nd_mapping->nvdimm;
> +
> +		if (test_bit(NDD_ALIASING, &nvdimm->flags)) {
> +			align = MEMREMAP_COMPAT_ALIGN_MAX;
> +			break;
> +		}
> +	}
> +
> +	mappings = max_t(u16, 1, nd_region->ndr_mappings);
> +	per_mapping = align;
> +	remainder = do_div(per_mapping, mappings);
> +	if (remainder)
> +		align *= mappings;
> +
> +	return align;
> +}
> +
>  static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
>  		struct nd_region_desc *ndr_desc,
>  		const struct device_type *dev_type, const char *caller)
> @@ -1039,6 +1142,7 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
>  	dev->of_node = ndr_desc->of_node;
>  	nd_region->ndr_size = resource_size(ndr_desc->res);
>  	nd_region->ndr_start = ndr_desc->res->start;
> +	nd_region->align = default_align(nd_region);
>  	if (ndr_desc->flush)
>  		nd_region->flush = ndr_desc->flush;
>  	else
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply

* Re: [PATCH 4/5] libnvdimm/region: Introduce NDD_LABELING
From: Aneesh Kumar K.V @ 2020-02-03 17:09 UTC (permalink / raw)
  To: Dan Williams, linux-nvdimm; +Cc: linuxppc-dev, hch, linux-kernel
In-Reply-To: <158041477856.3889308.4212605617834097674.stgit@dwillia2-desk3.amr.corp.intel.com>

Dan Williams <dan.j.williams@intel.com> writes:

> The NDD_ALIASING flag is used to indicate where pmem capacity might
> alias with blk capacity and require labeling. It is also used to
> indicate whether the DIMM supports labeling. Separate this latter
> capability into its own flag so that the NDD_ALIASING flag is scoped to
> true aliased configurations.
>
> To my knowledge aliased configurations only exist in the ACPI spec,
> there are no known platforms that ship this support in production.
>
> This clarity allows namespace-capacity alignment constraints around
> interleave-ways to be relaxed.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Oliver O'Halloran <oohall@gmail.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  arch/powerpc/platforms/pseries/papr_scm.c |    2 +-
>  drivers/acpi/nfit/core.c                  |    4 +++-
>  drivers/nvdimm/dimm.c                     |    2 +-
>  drivers/nvdimm/dimm_devs.c                |    9 +++++----
>  drivers/nvdimm/namespace_devs.c           |    2 +-
>  drivers/nvdimm/nd.h                       |    2 +-
>  drivers/nvdimm/region_devs.c              |   10 +++++-----
>  include/linux/libnvdimm.h                 |    2 ++
>  8 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index c2ef320ba1bf..aae60cfd4e38 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -326,7 +326,7 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>  	}
>  
>  	dimm_flags = 0;
> -	set_bit(NDD_ALIASING, &dimm_flags);
> +	set_bit(NDD_LABELING, &dimm_flags);
>  
>  	p->nvdimm = nvdimm_create(p->bus, p, NULL, dimm_flags,
>  				  PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index a3320f93616d..71d7f2aa1b12 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2026,8 +2026,10 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
>  			continue;
>  		}
>  
> -		if (nfit_mem->bdw && nfit_mem->memdev_pmem)
> +		if (nfit_mem->bdw && nfit_mem->memdev_pmem) {
>  			set_bit(NDD_ALIASING, &flags);
> +			set_bit(NDD_LABELING, &flags);
> +		}
>  
>  		/* collate flags across all memdevs for this dimm */
>  		list_for_each_entry(nfit_memdev, &acpi_desc->memdevs, list) {
> diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
> index 64776ed15bb3..7d4ddc4d9322 100644
> --- a/drivers/nvdimm/dimm.c
> +++ b/drivers/nvdimm/dimm.c
> @@ -99,7 +99,7 @@ static int nvdimm_probe(struct device *dev)
>  	if (ndd->ns_current >= 0) {
>  		rc = nd_label_reserve_dpa(ndd);
>  		if (rc == 0)
> -			nvdimm_set_aliasing(dev);
> +			nvdimm_set_labeling(dev);
>  	}
>  	nvdimm_bus_unlock(dev);
>  
> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
> index 94ea6dba6b4f..64159d4d4b8f 100644
> --- a/drivers/nvdimm/dimm_devs.c
> +++ b/drivers/nvdimm/dimm_devs.c
> @@ -32,7 +32,7 @@ int nvdimm_check_config_data(struct device *dev)
>  
>  	if (!nvdimm->cmd_mask ||
>  	    !test_bit(ND_CMD_GET_CONFIG_DATA, &nvdimm->cmd_mask)) {
> -		if (test_bit(NDD_ALIASING, &nvdimm->flags))
> +		if (test_bit(NDD_LABELING, &nvdimm->flags))
>  			return -ENXIO;
>  		else
>  			return -ENOTTY;
> @@ -173,11 +173,11 @@ int nvdimm_set_config_data(struct nvdimm_drvdata *ndd, size_t offset,
>  	return rc;
>  }
>  
> -void nvdimm_set_aliasing(struct device *dev)
> +void nvdimm_set_labeling(struct device *dev)
>  {
>  	struct nvdimm *nvdimm = to_nvdimm(dev);
>  
> -	set_bit(NDD_ALIASING, &nvdimm->flags);
> +	set_bit(NDD_LABELING, &nvdimm->flags);
>  }
>  
>  void nvdimm_set_locked(struct device *dev)
> @@ -312,8 +312,9 @@ static ssize_t flags_show(struct device *dev,
>  {
>  	struct nvdimm *nvdimm = to_nvdimm(dev);
>  
> -	return sprintf(buf, "%s%s\n",
> +	return sprintf(buf, "%s%s%s\n",
>  			test_bit(NDD_ALIASING, &nvdimm->flags) ? "alias " : "",
> +			test_bit(NDD_LABELING, &nvdimm->flags) ? "label" : "",
>  			test_bit(NDD_LOCKED, &nvdimm->flags) ? "lock " : "");
>  }
>  static DEVICE_ATTR_RO(flags);
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index aff1f32fdb4f..30cda9f235de 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -2531,7 +2531,7 @@ static int init_active_labels(struct nd_region *nd_region)
>  		if (!ndd) {
>  			if (test_bit(NDD_LOCKED, &nvdimm->flags))
>  				/* fail, label data may be unreadable */;
> -			else if (test_bit(NDD_ALIASING, &nvdimm->flags))
> +			else if (test_bit(NDD_LABELING, &nvdimm->flags))
>  				/* fail, labels needed to disambiguate dpa */;
>  			else
>  				return 0;
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index c9f6a5b5253a..ca39abe29c7c 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -252,7 +252,7 @@ int nvdimm_set_config_data(struct nvdimm_drvdata *ndd, size_t offset,
>  		void *buf, size_t len);
>  long nvdimm_clear_poison(struct device *dev, phys_addr_t phys,
>  		unsigned int len);
> -void nvdimm_set_aliasing(struct device *dev);
> +void nvdimm_set_labeling(struct device *dev);
>  void nvdimm_set_locked(struct device *dev);
>  void nvdimm_clear_locked(struct device *dev);
>  int nvdimm_security_setup_events(struct device *dev);
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index a19e535830d9..a5fc6e4c56ff 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -195,16 +195,16 @@ EXPORT_SYMBOL_GPL(nd_blk_region_set_provider_data);
>  int nd_region_to_nstype(struct nd_region *nd_region)
>  {
>  	if (is_memory(&nd_region->dev)) {
> -		u16 i, alias;
> +		u16 i, label;
>  
> -		for (i = 0, alias = 0; i < nd_region->ndr_mappings; i++) {
> +		for (i = 0, label = 0; i < nd_region->ndr_mappings; i++) {
>  			struct nd_mapping *nd_mapping = &nd_region->mapping[i];
>  			struct nvdimm *nvdimm = nd_mapping->nvdimm;
>  
> -			if (test_bit(NDD_ALIASING, &nvdimm->flags))
> -				alias++;
> +			if (test_bit(NDD_LABELING, &nvdimm->flags))
> +				label++;
>  		}
> -		if (alias)
> +		if (label)
>  			return ND_DEVICE_NAMESPACE_PMEM;
>  		else
>  			return ND_DEVICE_NAMESPACE_IO;
> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
> index 9df091bd30ba..18da4059be09 100644
> --- a/include/linux/libnvdimm.h
> +++ b/include/linux/libnvdimm.h
> @@ -37,6 +37,8 @@ enum {
>  	NDD_WORK_PENDING = 4,
>  	/* ignore / filter NSLABEL_FLAG_LOCAL for this DIMM, i.e. no aliasing */
>  	NDD_NOBLK = 5,
> +	/* dimm supports namespace labels */
> +	NDD_LABELING = 6,
>  
>  	/* need to set a limit somewhere, but yes, this is likely overkill */
>  	ND_IOCTL_MAX_BUFLEN = SZ_4M,
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply

* Re: [PATCH 3/5] libnvdimm/namespace: Enforce memremap_compat_align()
From: Aneesh Kumar K.V @ 2020-02-03 17:09 UTC (permalink / raw)
  To: Dan Williams, linux-nvdimm; +Cc: linuxppc-dev, hch, linux-kernel
In-Reply-To: <158041477336.3889308.4581652885008605170.stgit@dwillia2-desk3.amr.corp.intel.com>

Dan Williams <dan.j.williams@intel.com> writes:

> The pmem driver on PowerPC crashes with the following signature when
> instantiating misaligned namespaces that map their capacity via
> memremap_pages().
>
>     BUG: Unable to handle kernel data access at 0xc001000406000000
>     Faulting instruction address: 0xc000000000090790
>     NIP [c000000000090790] arch_add_memory+0xc0/0x130
>     LR [c000000000090744] arch_add_memory+0x74/0x130
>     Call Trace:
>      arch_add_memory+0x74/0x130 (unreliable)
>      memremap_pages+0x74c/0xa30
>      devm_memremap_pages+0x3c/0xa0
>      pmem_attach_disk+0x188/0x770
>      nvdimm_bus_probe+0xd8/0x470
>
> With the assumption that only memremap_pages() has alignment
> constraints, enforce memremap_compat_align() for
> pmem_should_map_pages(), nd_pfn, or nd_dax cases.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/nvdimm/namespace_devs.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index 032dc61725ff..aff1f32fdb4f 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -1739,6 +1739,16 @@ struct nd_namespace_common *nvdimm_namespace_common_probe(struct device *dev)
>  		return ERR_PTR(-ENODEV);
>  	}
>  
> +	if (pmem_should_map_pages(dev) || nd_pfn || nd_dax) {
> +		struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev);
> +		resource_size_t start = nsio->res.start;
> +
> +		if (!IS_ALIGNED(start | size, memremap_compat_align())) {
> +			dev_dbg(&ndns->dev, "misaligned, unable to map\n");
> +			return ERR_PTR(-EOPNOTSUPP);
> +		}
> +	}
> +
>  	if (is_namespace_pmem(&ndns->dev)) {
>  		struct nd_namespace_pmem *nspm;
>  
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply

* Re: [PATCH 2/5] mm/memremap_pages: Introduce memremap_compat_align()
From: Aneesh Kumar K.V @ 2020-02-03 17:09 UTC (permalink / raw)
  To: Dan Williams, linux-nvdimm
  Cc: linux-kernel, Jeff Moyer, Paul Mackerras, vishal.l.verma,
	linuxppc-dev, hch
In-Reply-To: <158041476763.3889308.13149849631980018039.stgit@dwillia2-desk3.amr.corp.intel.com>

Dan Williams <dan.j.williams@intel.com> writes:

> The "sub-section memory hotplug" facility allows memremap_pages() users
> like libnvdimm to compensate for hardware platforms like x86 that have a
> section size larger than their hardware memory mapping granularity.  The
> compensation that sub-section support affords is being tolerant of
> physical memory resources shifting by units smaller (64MiB on x86) than
> the memory-hotplug section size (128 MiB). Where the platform
> physical-memory mapping granularity is limited by the number and
> capability of address-decode-registers in the memory controller.
>
> While the sub-section support allows memremap_pages() to operate on
> sub-section (2MiB) granularity, the Power architecture may still
> require 16MiB alignment on "!radix_enabled()" platforms.
>
> In order for libnvdimm to be able to detect and manage this per-arch
> limitation, introduce memremap_compat_align() as a common minimum
> alignment across all driver-facing memory-mapping interfaces, and let
> Power override it to 16MiB in the "!radix_enabled()" case.
>
> The assumption / requirement for 16MiB to be a viable
> memremap_compat_align() value is that Power does not have platforms
> where its equivalent of address-decode-registers never hardware remaps a
> persistent memory resource on smaller than 16MiB boundaries.
>
> Based on an initial patch by Aneesh.

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

>
> Link: http://lore.kernel.org/r/CAPcyv4gBGNP95APYaBcsocEa50tQj9b5h__83vgngjq3ouGX_Q@mail.gmail.com
> Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Reported-by: Jeff Moyer <jmoyer@redhat.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  arch/powerpc/include/asm/io.h |   10 ++++++++++
>  drivers/nvdimm/pfn_devs.c     |    2 +-
>  include/linux/io.h            |   23 +++++++++++++++++++++++
>  include/linux/mmzone.h        |    1 +
>  4 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> index a63ec938636d..0fa2dc483008 100644
> --- a/arch/powerpc/include/asm/io.h
> +++ b/arch/powerpc/include/asm/io.h
> @@ -734,6 +734,16 @@ extern void __iomem * __ioremap_at(phys_addr_t pa, void *ea,
>  				   unsigned long size, pgprot_t prot);
>  extern void __iounmap_at(void *ea, unsigned long size);
>  
> +#ifdef CONFIG_SPARSEMEM
> +static inline unsigned long memremap_compat_align(void)
> +{
> +	if (radix_enabled())
> +		return SUBSECTION_SIZE;
> +	return (1UL << mmu_psize_defs[mmu_linear_psize].shift);
> +}
> +#define memremap_compat_align memremap_compat_align
> +#endif
> +
>  /*
>   * When CONFIG_PPC_INDIRECT_PIO is set, we use the generic iomap implementation
>   * which needs some additional definitions here. They basically allow PIO
> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index b94f7a7e94b8..a5c25cb87116 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
> @@ -750,7 +750,7 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
>  	start = nsio->res.start;
>  	size = resource_size(&nsio->res);
>  	npfns = PHYS_PFN(size - SZ_8K);
> -	align = max(nd_pfn->align, (1UL << SUBSECTION_SHIFT));
> +	align = max(nd_pfn->align, SUBSECTION_SIZE);
>  	end_trunc = start + size - ALIGN_DOWN(start + size, align);
>  	if (nd_pfn->mode == PFN_MODE_PMEM) {
>  		/*
> diff --git a/include/linux/io.h b/include/linux/io.h
> index 35e8d84935e0..ccd34519fad3 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -6,6 +6,7 @@
>  #ifndef _LINUX_IO_H
>  #define _LINUX_IO_H
>  
> +#include <linux/mmzone.h>
>  #include <linux/types.h>
>  #include <linux/init.h>
>  #include <linux/bug.h>
> @@ -79,6 +80,28 @@ void *devm_memremap(struct device *dev, resource_size_t offset,
>  		size_t size, unsigned long flags);
>  void devm_memunmap(struct device *dev, void *addr);
>  
> +#ifndef memremap_compat_align
> +#ifdef CONFIG_SPARSEMEM
> +/*
> + * Minimum compatible alignment of the resource (start, end) across
> + * memremap interfaces (i.e. memremap + memremap_pages)
> + */
> +static inline unsigned long memremap_compat_align(void)
> +{
> +	return SUBSECTION_SIZE;
> +}
> +#else /* CONFIG_SPARSEMEM */
> +/*
> + * No ZONE_DEVICE / memremap_pages() support so the minimum mapping
> + * granularity is a single page.
> + */
> +static inline unsigned long memremap_compat_align(void)
> +{
> +	return PAGE_SIZE;
> +}
> +#endif /* CONFIG_SPARSEMEM */
> +#endif /* memremap_compat_align */
> +
>  #ifdef CONFIG_PCI
>  /*
>   * The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 89d8ff06c9ce..b0de83620cd7 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1171,6 +1171,7 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
>  #define SECTION_ALIGN_DOWN(pfn)	((pfn) & PAGE_SECTION_MASK)
>  
>  #define SUBSECTION_SHIFT 21
> +#define SUBSECTION_SIZE (1UL << SUBSECTION_SHIFT)
>  
>  #define PFN_SUBSECTION_SHIFT (SUBSECTION_SHIFT - PAGE_SHIFT)
>  #define PAGES_PER_SUBSECTION (1UL << PFN_SUBSECTION_SHIFT)

^ permalink raw reply

* Re: [PATCH 1/5] mm/memremap_pages: Kill unused __devm_memremap_pages()
From: Aneesh Kumar K.V @ 2020-02-03 17:08 UTC (permalink / raw)
  To: Dan Williams, linux-nvdimm; +Cc: linuxppc-dev, Christoph Hellwig, linux-kernel
In-Reply-To: <158041476158.3889308.4221100673554151124.stgit@dwillia2-desk3.amr.corp.intel.com>

Dan Williams <dan.j.williams@intel.com> writes:

> Kill this definition that was introduced in commit 41e94a851304 ("add
> devm_memremap_pages") add never used.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  include/linux/io.h |    2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/include/linux/io.h b/include/linux/io.h
> index a59834bc0a11..35e8d84935e0 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -79,8 +79,6 @@ void *devm_memremap(struct device *dev, resource_size_t offset,
>  		size_t size, unsigned long flags);
>  void devm_memunmap(struct device *dev, void *addr);
>  
> -void *__devm_memremap_pages(struct device *dev, struct resource *res);
> -
>  #ifdef CONFIG_PCI
>  /*
>   * The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply

* Re: [PATCH] powerpc/32s: Slenderize _tlbia() for powerpc 603/603e
From: Joakim Tjernlund @ 2020-02-03 16:57 UTC (permalink / raw)
  To: christophe.leroy@c-s.fr, mpe@ellerman.id.au, paulus@samba.org,
	benh@kernel.crashing.org
  Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <12f4f4f0ff89aeab3b937fc96c84fb35e1b2517e.1580748445.git.christophe.leroy@c-s.fr>

On Mon, 2020-02-03 at 16:47 +0000, Christophe Leroy wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> _tlbia() is a function used only on 603/603e core, ie on CPUs which
> don't have a hash table.
> 
> _tlbia() uses the tlbia macro which implements a loop of 1024 tlbie.
> 
> On the 603/603e core, flushing the entire TLB requires no more than
> 32 tlbie.
> 
> Replace tlbia by a loop of 32 tlbie.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/mm/book3s32/hash_low.S | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/mm/book3s32/hash_low.S b/arch/powerpc/mm/book3s32/hash_low.S
> index c11b0a005196..a5039ad10429 100644
> --- a/arch/powerpc/mm/book3s32/hash_low.S
> +++ b/arch/powerpc/mm/book3s32/hash_low.S
> @@ -696,18 +696,21 @@ _GLOBAL(_tlbia)
>         bne-    10b
>         stwcx.  r8,0,r9
>         bne-    10b
> +#endif /* CONFIG_SMP */
> +       li      r5, 32
> +       lis     r4, KERNELBASE@h
> +       mtctr   r5
>         sync
> -       tlbia
> +0:     tlbie   r4
> +       addi    r4, r4, 0x1000

Is page size always 4096 here or does it not matter ?

> +       bdnz    0b
>         sync
> +#ifdef CONFIG_SMP
>         TLBSYNC
>         li      r0,0
>         stw     r0,0(r9)                /* clear mmu_hash_lock */
>         mtmsr   r10
>         SYNC_601
>         isync
> -#else /* CONFIG_SMP */
> -       sync
> -       tlbia
> -       sync
>  #endif /* CONFIG_SMP */
>         blr
> --
> 2.25.0
> 


^ permalink raw reply

* [PATCH] powerpc/32s: Slenderize _tlbia() for powerpc 603/603e
From: Christophe Leroy @ 2020-02-03 16:47 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

_tlbia() is a function used only on 603/603e core, ie on CPUs which
don't have a hash table.

_tlbia() uses the tlbia macro which implements a loop of 1024 tlbie.

On the 603/603e core, flushing the entire TLB requires no more than
32 tlbie.

Replace tlbia by a loop of 32 tlbie.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/mm/book3s32/hash_low.S | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/mm/book3s32/hash_low.S b/arch/powerpc/mm/book3s32/hash_low.S
index c11b0a005196..a5039ad10429 100644
--- a/arch/powerpc/mm/book3s32/hash_low.S
+++ b/arch/powerpc/mm/book3s32/hash_low.S
@@ -696,18 +696,21 @@ _GLOBAL(_tlbia)
 	bne-	10b
 	stwcx.	r8,0,r9
 	bne-	10b
+#endif /* CONFIG_SMP */
+	li	r5, 32
+	lis	r4, KERNELBASE@h
+	mtctr	r5
 	sync
-	tlbia
+0:	tlbie	r4
+	addi	r4, r4, 0x1000
+	bdnz	0b
 	sync
+#ifdef CONFIG_SMP
 	TLBSYNC
 	li	r0,0
 	stw	r0,0(r9)		/* clear mmu_hash_lock */
 	mtmsr	r10
 	SYNC_601
 	isync
-#else /* CONFIG_SMP */
-	sync
-	tlbia
-	sync
 #endif /* CONFIG_SMP */
 	blr
-- 
2.25.0


^ permalink raw reply related

* [PATCH v2 3/3] selftests/powerpc: Don't rely on segfault to rerun the test
From: Gustavo Luiz Duarte @ 2020-02-03 16:09 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, Gustavo Luiz Duarte, gromero
In-Reply-To: <20200203160906.24482-1-gustavold@linux.ibm.com>

The test case tm-signal-context-force-tm expects a segfault to happen on
returning from signal handler, and then does a setcontext() to run the test
again. However, the test doesn't always segfault, causing the test to run a
single time.

This patch fixes the test by putting it within a loop and jumping, via
setcontext, just prior to the loop in case it segfaults. This way we get the
desired behavior (run the test COUNT_MAX times) regardless if it segfaults or
not. This also reduces the use of setcontext for control flow logic, keeping it
only in the segfault handler.

Also, since 'count' is changed within the signal handler, it is declared as
volatile to prevent any compiler optimization getting confused with
asynchronous changes.

Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
---
 .../powerpc/tm/tm-signal-context-force-tm.c   | 79 +++++++++----------
 1 file changed, 37 insertions(+), 42 deletions(-)

diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c b/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
index 31717625f318..9ff7bdb6d47a 100644
--- a/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
+++ b/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
@@ -42,9 +42,10 @@
 #endif
 
 /* Setting contexts because the test will crash and we want to recover */
-ucontext_t init_context, main_context;
+ucontext_t init_context;
 
-static int count, first_time;
+/* count is changed in the signal handler, so it must be volatile */
+static volatile int count;
 
 void usr_signal_handler(int signo, siginfo_t *si, void *uc)
 {
@@ -98,11 +99,6 @@ void usr_signal_handler(int signo, siginfo_t *si, void *uc)
 
 void seg_signal_handler(int signo, siginfo_t *si, void *uc)
 {
-	if (count == COUNT_MAX) {
-		/* Return to tm_signal_force_msr() and exit */
-		setcontext(&main_context);
-	}
-
 	count++;
 
 	/* Reexecute the test */
@@ -126,37 +122,40 @@ void tm_trap_test(void)
 	 */
 	getcontext(&init_context);
 
-	/* Allocated an alternative signal stack area */
-	ss.ss_sp = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
-			MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
-	ss.ss_size = SIGSTKSZ;
-	ss.ss_flags = 0;
-
-	if (ss.ss_sp == (void *)-1) {
-		perror("mmap error\n");
-		exit(-1);
-	}
-
-	/* Force the allocation through a page fault */
-	if (madvise(ss.ss_sp, SIGSTKSZ, MADV_DONTNEED)) {
-		perror("madvise\n");
-		exit(-1);
-	}
-
-	/* Setting an alternative stack to generate a page fault when
-	 * the signal is raised.
-	 */
-	if (sigaltstack(&ss, NULL)) {
-		perror("sigaltstack\n");
-		exit(-1);
+	while (count < COUNT_MAX) {
+		/* Allocated an alternative signal stack area */
+		ss.ss_sp = mmap(NULL, SIGSTKSZ, PROT_READ | PROT_WRITE,
+				MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+		ss.ss_size = SIGSTKSZ;
+		ss.ss_flags = 0;
+
+		if (ss.ss_sp == (void *)-1) {
+			perror("mmap error\n");
+			exit(-1);
+		}
+
+		/* Force the allocation through a page fault */
+		if (madvise(ss.ss_sp, SIGSTKSZ, MADV_DONTNEED)) {
+			perror("madvise\n");
+			exit(-1);
+		}
+
+		/* Setting an alternative stack to generate a page fault when
+		 * the signal is raised.
+		 */
+		if (sigaltstack(&ss, NULL)) {
+			perror("sigaltstack\n");
+			exit(-1);
+		}
+
+		/* The signal handler will enable MSR_TS */
+		sigaction(SIGUSR1, &usr_sa, NULL);
+		/* If it does not crash, it might segfault, avoid it to retest */
+		sigaction(SIGSEGV, &seg_sa, NULL);
+
+		raise(SIGUSR1);
+		count++;
 	}
-
-	/* The signal handler will enable MSR_TS */
-	sigaction(SIGUSR1, &usr_sa, NULL);
-	/* If it does not crash, it will segfault, avoid it to retest */
-	sigaction(SIGSEGV, &seg_sa, NULL);
-
-	raise(SIGUSR1);
 }
 
 int tm_signal_context_force_tm(void)
@@ -169,11 +168,7 @@ int tm_signal_context_force_tm(void)
 	 */
 	SKIP_IF(!is_ppc64le());
 
-	/* Will get back here after COUNT_MAX interactions */
-	getcontext(&main_context);
-
-	if (!first_time++)
-		tm_trap_test();
+	tm_trap_test();
 
 	return EXIT_SUCCESS;
 }
-- 
2.21.1


^ permalink raw reply related

* [PATCH v2 1/3] powerpc/tm: Clear the current thread's MSR[TS] after treclaim
From: Gustavo Luiz Duarte @ 2020-02-03 16:09 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, Gustavo Luiz Duarte, stable, gromero

After a treclaim, we expect to be in non-transactional state. If we don't
immediately clear the current thread's MSR[TS] and we get preempted, then
tm_recheckpoint_new_task() will recheckpoint and we get rescheduled in
suspended transaction state.

When handling a signal caught in transactional state, handle_rt_signal64()
calls get_tm_stackpointer() that treclaims the transaction using
tm_reclaim_current() but without clearing the thread's MSR[TS]. This can cause
the TM Bad Thing exception below if later we pagefault and get preempted trying
to access the user's sigframe, using __put_user(). Afterwards, when we are
rescheduled back into do_page_fault() (but now in suspended state since the
thread's MSR[TS] was not cleared), upon executing 'rfid' after completion of
the page fault handling, the exception is raised because a transition from
suspended to non-transactional state is invalid.

	Unexpected TM Bad Thing exception at c00000000000de44 (msr 0x8000000302a03031) tm_scratch=800000010280b033
	Oops: Unrecoverable exception, sig: 6 [#1]
	LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
	Modules linked in: nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip6_tables ip_tables nft_compat ip_set nf_tables nfnetlink xts vmx_crypto sg virtio_balloon
	r_mod cdrom virtio_net net_failover virtio_blk virtio_scsi failover dm_mirror dm_region_hash dm_log dm_mod
	CPU: 25 PID: 15547 Comm: a.out Not tainted 5.4.0-rc2 #32
	NIP:  c00000000000de44 LR: c000000000034728 CTR: 0000000000000000
	REGS: c00000003fe7bd70 TRAP: 0700   Not tainted  (5.4.0-rc2)
	MSR:  8000000302a03031 <SF,VEC,VSX,FP,ME,IR,DR,LE,TM[SE]>  CR: 44000884  XER: 00000000
	CFAR: c00000000000dda4 IRQMASK: 0
	PACATMSCRATCH: 800000010280b033
	GPR00: c000000000034728 c000000f65a17c80 c000000001662800 00007fffacf3fd78
	GPR04: 0000000000001000 0000000000001000 0000000000000000 c000000f611f8af0
	GPR08: 0000000000000000 0000000078006001 0000000000000000 000c000000000000
	GPR12: c000000f611f84b0 c00000003ffcb200 0000000000000000 0000000000000000
	GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
	GPR20: 0000000000000000 0000000000000000 0000000000000000 c000000f611f8140
	GPR24: 0000000000000000 00007fffacf3fd68 c000000f65a17d90 c000000f611f7800
	GPR28: c000000f65a17e90 c000000f65a17e90 c000000001685e18 00007fffacf3f000
	NIP [c00000000000de44] fast_exception_return+0xf4/0x1b0
	LR [c000000000034728] handle_rt_signal64+0x78/0xc50
	Call Trace:
	[c000000f65a17c80] [c000000000034710] handle_rt_signal64+0x60/0xc50 (unreliable)
	[c000000f65a17d30] [c000000000023640] do_notify_resume+0x330/0x460
	[c000000f65a17e20] [c00000000000dcc4] ret_from_except_lite+0x70/0x74
	Instruction dump:
	7c4ff120 e8410170 7c5a03a6 38400000 f8410060 e8010070 e8410080 e8610088
	60000000 60000000 e8810090 e8210078 <4c000024> 48000000 e8610178 88ed0989
	---[ end trace 93094aa44b442f87 ]---

The simplified sequence of events that triggers the above exception is:

  ...				# userspace in NON-TRANSACTIONAL state
  tbegin			# userspace in TRANSACTIONAL state
  signal delivery		# kernelspace in SUSPENDED state
  handle_rt_signal64()
    get_tm_stackpointer()
      treclaim			# kernelspace in NON-TRANSACTIONAL state
    __put_user()
      page fault happens. We will never get back here because of the TM Bad Thing exception.

  page fault handling kicks in and we voluntarily preempt ourselves
  do_page_fault()
    __schedule()
      __switch_to(other_task)

  our task is rescheduled and we recheckpoint because the thread's MSR[TS] was not cleared
  __switch_to(our_task)
    switch_to_tm()
      tm_recheckpoint_new_task()
        trechkpt			# kernelspace in SUSPENDED state

  The page fault handling resumes, but now we are in suspended transaction state
  do_page_fault()    completes
  rfid     <----- trying to get back where the page fault happened (we were non-transactional back then)
  TM Bad Thing			# illegal transition from suspended to non-transactional

This patch fixes that issue by clearing the current thread's MSR[TS] just after
treclaim in get_tm_stackpointer() so that we stay in non-transactional state in
case we are preempted. In order to make treclaim and clearing the thread's
MSR[TS] atomic from a preemption perspective when CONFIG_PREEMPT is set,
preempt_disable/enable() is used. It's also necessary to save the previous
value of the thread's MSR before get_tm_stackpointer() is called so that it can
be exposed to the signal handler later in setup_tm_sigcontexts() to inform the
userspace MSR at the moment of the signal delivery.

Found with tm-signal-context-force-tm kernel selftest on P8 KVM.

v2: Fix build failure when tm is disabled.

Fixes: 2b0a576d15e0 ("powerpc: Add new transactional memory state to the signal context")
Cc: stable@vger.kernel.org # v3.9
Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
---
 arch/powerpc/kernel/signal.c    | 17 +++++++++++++++--
 arch/powerpc/kernel/signal_32.c | 28 ++++++++++++++--------------
 arch/powerpc/kernel/signal_64.c | 22 ++++++++++------------
 3 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index e6c30cee6abf..1660be1061ac 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -200,14 +200,27 @@ unsigned long get_tm_stackpointer(struct task_struct *tsk)
 	 * normal/non-checkpointed stack pointer.
 	 */
 
+	unsigned long ret = tsk->thread.regs->gpr[1];
+
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	BUG_ON(tsk != current);
 
 	if (MSR_TM_ACTIVE(tsk->thread.regs->msr)) {
+		preempt_disable();
 		tm_reclaim_current(TM_CAUSE_SIGNAL);
 		if (MSR_TM_TRANSACTIONAL(tsk->thread.regs->msr))
-			return tsk->thread.ckpt_regs.gpr[1];
+			ret = tsk->thread.ckpt_regs.gpr[1];
+
+		/* If we treclaim, we must immediately clear the current
+		 * thread's TM bits. Otherwise we might be preempted and have
+		 * the live MSR[TS] changed behind our back
+		 * (tm_recheckpoint_new_task() would recheckpoint).
+		 * Besides, we enter the signal handler in non-transactional
+		 * state.
+		 */
+		tsk->thread.regs->msr &= ~MSR_TS_MASK;
+		preempt_enable();
 	}
 #endif
-	return tsk->thread.regs->gpr[1];
+	return ret;
 }
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 98600b276f76..1b090a76b444 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -489,19 +489,11 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
  */
 static int save_tm_user_regs(struct pt_regs *regs,
 			     struct mcontext __user *frame,
-			     struct mcontext __user *tm_frame, int sigret)
+			     struct mcontext __user *tm_frame, int sigret,
+			     unsigned long msr)
 {
-	unsigned long msr = regs->msr;
-
 	WARN_ON(tm_suspend_disabled);
 
-	/* Remove TM bits from thread's MSR.  The MSR in the sigcontext
-	 * just indicates to userland that we were doing a transaction, but we
-	 * don't want to return in transactional state.  This also ensures
-	 * that flush_fp_to_thread won't set TIF_RESTORE_TM again.
-	 */
-	regs->msr &= ~MSR_TS_MASK;
-
 	/* Save both sets of general registers */
 	if (save_general_regs(&current->thread.ckpt_regs, frame)
 	    || save_general_regs(regs, tm_frame))
@@ -912,6 +904,10 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 	int sigret;
 	unsigned long tramp;
 	struct pt_regs *regs = tsk->thread.regs;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	/* Save the thread's msr before get_tm_stackpointer() changes it */
+	unsigned long msr = regs->msr;
+#endif
 
 	BUG_ON(tsk != current);
 
@@ -944,13 +940,13 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	tm_frame = &rt_sf->uc_transact.uc_mcontext;
-	if (MSR_TM_ACTIVE(regs->msr)) {
+	if (MSR_TM_ACTIVE(msr)) {
 		if (__put_user((unsigned long)&rt_sf->uc_transact,
 			       &rt_sf->uc.uc_link) ||
 		    __put_user((unsigned long)tm_frame,
 			       &rt_sf->uc_transact.uc_regs))
 			goto badframe;
-		if (save_tm_user_regs(regs, frame, tm_frame, sigret))
+		if (save_tm_user_regs(regs, frame, tm_frame, sigret, msr))
 			goto badframe;
 	}
 	else
@@ -1369,6 +1365,10 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 	int sigret;
 	unsigned long tramp;
 	struct pt_regs *regs = tsk->thread.regs;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	/* Save the thread's msr before get_tm_stackpointer() changes it */
+	unsigned long msr = regs->msr;
+#endif
 
 	BUG_ON(tsk != current);
 
@@ -1402,9 +1402,9 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	tm_mctx = &frame->mctx_transact;
-	if (MSR_TM_ACTIVE(regs->msr)) {
+	if (MSR_TM_ACTIVE(msr)) {
 		if (save_tm_user_regs(regs, &frame->mctx, &frame->mctx_transact,
-				      sigret))
+				      sigret, msr))
 			goto badframe;
 	}
 	else
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 117515564ec7..84ed2e77ef9c 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -192,7 +192,8 @@ static long setup_sigcontext(struct sigcontext __user *sc,
 static long setup_tm_sigcontexts(struct sigcontext __user *sc,
 				 struct sigcontext __user *tm_sc,
 				 struct task_struct *tsk,
-				 int signr, sigset_t *set, unsigned long handler)
+				 int signr, sigset_t *set, unsigned long handler,
+				 unsigned long msr)
 {
 	/* When CONFIG_ALTIVEC is set, we _always_ setup v_regs even if the
 	 * process never used altivec yet (MSR_VEC is zero in pt_regs of
@@ -207,12 +208,11 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,
 	elf_vrreg_t __user *tm_v_regs = sigcontext_vmx_regs(tm_sc);
 #endif
 	struct pt_regs *regs = tsk->thread.regs;
-	unsigned long msr = tsk->thread.regs->msr;
 	long err = 0;
 
 	BUG_ON(tsk != current);
 
-	BUG_ON(!MSR_TM_ACTIVE(regs->msr));
+	BUG_ON(!MSR_TM_ACTIVE(msr));
 
 	WARN_ON(tm_suspend_disabled);
 
@@ -222,13 +222,6 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,
 	 */
 	msr |= tsk->thread.ckpt_regs.msr & (MSR_FP | MSR_VEC | MSR_VSX);
 
-	/* Remove TM bits from thread's MSR.  The MSR in the sigcontext
-	 * just indicates to userland that we were doing a transaction, but we
-	 * don't want to return in transactional state.  This also ensures
-	 * that flush_fp_to_thread won't set TIF_RESTORE_TM again.
-	 */
-	regs->msr &= ~MSR_TS_MASK;
-
 #ifdef CONFIG_ALTIVEC
 	err |= __put_user(v_regs, &sc->v_regs);
 	err |= __put_user(tm_v_regs, &tm_sc->v_regs);
@@ -824,6 +817,10 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 	unsigned long newsp = 0;
 	long err = 0;
 	struct pt_regs *regs = tsk->thread.regs;
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	/* Save the thread's msr before get_tm_stackpointer() changes it */
+	unsigned long msr = regs->msr;
+#endif
 
 	BUG_ON(tsk != current);
 
@@ -841,7 +838,7 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 	err |= __put_user(0, &frame->uc.uc_flags);
 	err |= __save_altstack(&frame->uc.uc_stack, regs->gpr[1]);
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-	if (MSR_TM_ACTIVE(regs->msr)) {
+	if (MSR_TM_ACTIVE(msr)) {
 		/* The ucontext_t passed to userland points to the second
 		 * ucontext_t (for transactional state) with its uc_link ptr.
 		 */
@@ -849,7 +846,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 		err |= setup_tm_sigcontexts(&frame->uc.uc_mcontext,
 					    &frame->uc_transact.uc_mcontext,
 					    tsk, ksig->sig, NULL,
-					    (unsigned long)ksig->ka.sa.sa_handler);
+					    (unsigned long)ksig->ka.sa.sa_handler,
+					    msr);
 	} else
 #endif
 	{
-- 
2.21.1


^ permalink raw reply related

* [PATCH v2 2/3] selftests/powerpc: Add tm-signal-pagefault test
From: Gustavo Luiz Duarte @ 2020-02-03 16:09 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, Gustavo Luiz Duarte, gromero
In-Reply-To: <20200203160906.24482-1-gustavold@linux.ibm.com>

This test triggers a TM Bad Thing by raising a signal in transactional state
and forcing a pagefault to happen in kernelspace when the kernel signal
handling code first touches the user signal stack.

This is inspired by the test tm-signal-context-force-tm but uses userfaultfd to
make the test deterministic. While this test always triggers the bug in one
run, I had to execute tm-signal-context-force-tm several times (the test runs
5000 times each execution) to trigger the same bug.

tm-signal-context-force-tm is kept instead of replaced because, while this test
is more reliable and triggers the same bug, tm-signal-context-force-tm has a
better coverage, in the sense that by running the test several times it might
trigger the pagefault and/or be preempted at different places.

Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.ibm.com>
---
 tools/testing/selftests/powerpc/tm/.gitignore |   1 +
 tools/testing/selftests/powerpc/tm/Makefile   |   3 +-
 .../powerpc/tm/tm-signal-pagefault.c          | 272 ++++++++++++++++++
 3 files changed, 275 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/tm/tm-signal-pagefault.c

diff --git a/tools/testing/selftests/powerpc/tm/.gitignore b/tools/testing/selftests/powerpc/tm/.gitignore
index 98f2708d86cc..e1c72a4a3e91 100644
--- a/tools/testing/selftests/powerpc/tm/.gitignore
+++ b/tools/testing/selftests/powerpc/tm/.gitignore
@@ -13,6 +13,7 @@ tm-signal-context-chk-vmx
 tm-signal-context-chk-vsx
 tm-signal-context-force-tm
 tm-signal-sigreturn-nt
+tm-signal-pagefault
 tm-vmx-unavail
 tm-unavailable
 tm-trap
diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
index b15a1a325bd0..b1d99736f8b8 100644
--- a/tools/testing/selftests/powerpc/tm/Makefile
+++ b/tools/testing/selftests/powerpc/tm/Makefile
@@ -5,7 +5,7 @@ SIGNAL_CONTEXT_CHK_TESTS := tm-signal-context-chk-gpr tm-signal-context-chk-fpu
 TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack \
 	tm-vmxcopy tm-fork tm-tar tm-tmspr tm-vmx-unavail tm-unavailable tm-trap \
 	$(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-signal-sigreturn-nt \
-	tm-signal-context-force-tm tm-poison
+	tm-signal-context-force-tm tm-poison tm-signal-pagefault
 
 top_srcdir = ../../../../..
 include ../../lib.mk
@@ -22,6 +22,7 @@ $(OUTPUT)/tm-resched-dscr: ../pmu/lib.c
 $(OUTPUT)/tm-unavailable: CFLAGS += -O0 -pthread -m64 -Wno-error=uninitialized -mvsx
 $(OUTPUT)/tm-trap: CFLAGS += -O0 -pthread -m64
 $(OUTPUT)/tm-signal-context-force-tm: CFLAGS += -pthread -m64
+$(OUTPUT)/tm-signal-pagefault: CFLAGS += -pthread -m64
 
 SIGNAL_CONTEXT_CHK_TESTS := $(patsubst %,$(OUTPUT)/%,$(SIGNAL_CONTEXT_CHK_TESTS))
 $(SIGNAL_CONTEXT_CHK_TESTS): tm-signal.S
diff --git a/tools/testing/selftests/powerpc/tm/tm-signal-pagefault.c b/tools/testing/selftests/powerpc/tm/tm-signal-pagefault.c
new file mode 100644
index 000000000000..3a2166101d94
--- /dev/null
+++ b/tools/testing/selftests/powerpc/tm/tm-signal-pagefault.c
@@ -0,0 +1,272 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020, Gustavo Luiz Duarte, IBM Corp.
+ *
+ * This test starts a transaction and triggers a signal, forcing a pagefault to
+ * happen when the kernel signal handling code touches the user signal stack.
+ *
+ * In order to avoid pre-faulting the signal stack memory and to force the
+ * pagefault to happen precisely in the kernel signal handling code, the
+ * pagefault handling is done in userspace using the userfaultfd facility.
+ *
+ * Further pagefaults are triggered by crafting the signal handler's ucontext
+ * to point to additional memory regions managed by the userfaultfd, so using
+ * the same mechanism used to avoid pre-faulting the signal stack memory.
+ *
+ * On failure (bug is present) kernel crashes or never returns control back to
+ * userspace. If bug is not present, tests completes almost immediately.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <linux/userfaultfd.h>
+#include <poll.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <pthread.h>
+#include <signal.h>
+
+#include "tm.h"
+
+
+#define UF_MEM_SIZE 655360	/* 10 x 64k pages */
+
+/* Memory handled by userfaultfd */
+static char *uf_mem;
+static size_t uf_mem_offset = 0;
+
+/*
+ * Data that will be copied into the faulting pages (instead of zero-filled
+ * pages). This is used to make the test more reliable and avoid segfaulting
+ * when we return from the signal handler. Since we are making the signal
+ * handler's ucontext point to newly allocated memory, when that memory is
+ * paged-in it will contain the expected content.
+ */
+static char backing_mem[UF_MEM_SIZE];
+
+static size_t pagesize;
+
+/*
+ * Return a chunk of at least 'size' bytes of memory that will be handled by
+ * userfaultfd. If 'backing_data' is not NULL, its content will be save to
+ * 'backing_mem' and then copied into the faulting pages when the page fault
+ * is handled.
+ */
+void *get_uf_mem(size_t size, void *backing_data)
+{
+	void *ret;
+
+	if (uf_mem_offset + size > UF_MEM_SIZE) {
+		fprintf(stderr, "Requesting more uf_mem than expected!\n");
+		exit(EXIT_FAILURE);
+	}
+
+	ret = &uf_mem[uf_mem_offset];
+
+	/* Save the data that will be copied into the faulting page */
+	if (backing_data != NULL)
+		memcpy(&backing_mem[uf_mem_offset], backing_data, size);
+
+	/* Reserve the requested amount of uf_mem */
+	uf_mem_offset += size;
+	/* Keep uf_mem_offset aligned to the page size (round up) */
+	uf_mem_offset = (uf_mem_offset + pagesize - 1) & ~(pagesize - 1);
+
+	return ret;
+}
+
+void *fault_handler_thread(void *arg)
+{
+	struct uffd_msg msg;	/* Data read from userfaultfd */
+	long uffd;		/* userfaultfd file descriptor */
+	struct uffdio_copy uffdio_copy;
+	struct pollfd pollfd;
+	ssize_t nread, offset;
+
+	uffd = (long) arg;
+
+	for (;;) {
+		pollfd.fd = uffd;
+		pollfd.events = POLLIN;
+		if (poll(&pollfd, 1, -1) == -1) {
+			perror("poll() failed");
+			exit(EXIT_FAILURE);
+		}
+
+		nread = read(uffd, &msg, sizeof(msg));
+		if (nread == 0) {
+			fprintf(stderr, "read(): EOF on userfaultfd\n");
+			exit(EXIT_FAILURE);
+		}
+
+		if (nread == -1) {
+			perror("read() failed");
+			exit(EXIT_FAILURE);
+		}
+
+		/* We expect only one kind of event */
+		if (msg.event != UFFD_EVENT_PAGEFAULT) {
+			fprintf(stderr, "Unexpected event on userfaultfd\n");
+			exit(EXIT_FAILURE);
+		}
+
+		/*
+		 * We need to handle page faults in units of pages(!).
+		 * So, round faulting address down to page boundary.
+		 */
+		uffdio_copy.dst = msg.arg.pagefault.address & ~(pagesize-1);
+
+		offset = (char *) uffdio_copy.dst - uf_mem;
+		uffdio_copy.src = (unsigned long) &backing_mem[offset];
+
+		uffdio_copy.len = pagesize;
+		uffdio_copy.mode = 0;
+		uffdio_copy.copy = 0;
+		if (ioctl(uffd, UFFDIO_COPY, &uffdio_copy) == -1) {
+			perror("ioctl-UFFDIO_COPY failed");
+			exit(EXIT_FAILURE);
+		}
+	}
+}
+
+void setup_uf_mem(void)
+{
+	long uffd;		/* userfaultfd file descriptor */
+	pthread_t thr;
+	struct uffdio_api uffdio_api;
+	struct uffdio_register uffdio_register;
+	int ret;
+
+	pagesize = sysconf(_SC_PAGE_SIZE);
+
+	/* Create and enable userfaultfd object */
+	uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
+	if (uffd == -1) {
+		perror("userfaultfd() failed");
+		exit(EXIT_FAILURE);
+	}
+	uffdio_api.api = UFFD_API;
+	uffdio_api.features = 0;
+	if (ioctl(uffd, UFFDIO_API, &uffdio_api) == -1) {
+		perror("ioctl-UFFDIO_API failed");
+		exit(EXIT_FAILURE);
+	}
+
+	/*
+	 * Create a private anonymous mapping. The memory will be demand-zero
+	 * paged, that is, not yet allocated. When we actually touch the memory
+	 * the related page will be allocated via the userfaultfd mechanism.
+	 */
+	uf_mem = mmap(NULL, UF_MEM_SIZE, PROT_READ | PROT_WRITE,
+		      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (uf_mem == MAP_FAILED) {
+		perror("mmap() failed");
+		exit(EXIT_FAILURE);
+	}
+
+	/*
+	 * Register the memory range of the mapping we've just mapped to be
+	 * handled by the userfaultfd object. In 'mode' we request to track
+	 * missing pages (i.e. pages that have not yet been faulted-in).
+	 */
+	uffdio_register.range.start = (unsigned long) uf_mem;
+	uffdio_register.range.len = UF_MEM_SIZE;
+	uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
+	if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
+		perror("ioctl-UFFDIO_REGISTER");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Create a thread that will process the userfaultfd events */
+	ret = pthread_create(&thr, NULL, fault_handler_thread, (void *) uffd);
+	if (ret != 0) {
+		fprintf(stderr, "pthread_create(): Error. Returned %d\n", ret);
+		exit(EXIT_FAILURE);
+	}
+}
+
+/*
+ * Assumption: the signal was delivered while userspace was in transactional or
+ * suspended state, i.e. uc->uc_link != NULL.
+ */
+void signal_handler(int signo, siginfo_t *si, void *uc)
+{
+	ucontext_t *ucp = uc;
+
+	/* Skip 'trap' after returning, otherwise we get a SIGTRAP again */
+	ucp->uc_link->uc_mcontext.regs->nip += 4;
+
+	ucp->uc_mcontext.v_regs =
+		get_uf_mem(sizeof(elf_vrreg_t), ucp->uc_mcontext.v_regs);
+
+	ucp->uc_link->uc_mcontext.v_regs =
+		get_uf_mem(sizeof(elf_vrreg_t), ucp->uc_link->uc_mcontext.v_regs);
+
+	ucp->uc_link = get_uf_mem(sizeof(ucontext_t), ucp->uc_link);
+}
+
+int tm_signal_pagefault(void)
+{
+	struct sigaction sa;
+	stack_t ss;
+
+	SKIP_IF(!have_htm());
+
+	setup_uf_mem();
+
+	/*
+	 * Set an alternative stack that will generate a page fault when the
+	 * signal is raised. The page fault will be treated via userfaultfd,
+	 * i.e. via fault_handler_thread.
+	 */
+	ss.ss_sp = get_uf_mem(SIGSTKSZ, NULL);
+	ss.ss_size = SIGSTKSZ;
+	ss.ss_flags = 0;
+	if (sigaltstack(&ss, NULL) == -1) {
+		perror("sigaltstack() failed");
+		exit(EXIT_FAILURE);
+	}
+
+	sa.sa_flags = SA_SIGINFO | SA_ONSTACK;
+	sa.sa_sigaction = signal_handler;
+	if (sigaction(SIGTRAP, &sa, NULL) == -1) {
+		perror("sigaction() failed");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Trigger a SIGTRAP in transactional state */
+	asm __volatile__(
+			"tbegin.;"
+			"beq    1f;"
+			"trap;"
+			"1: ;"
+			: : : "memory");
+
+	/* Trigger a SIGTRAP in suspended state */
+	asm __volatile__(
+			"tbegin.;"
+			"beq    1f;"
+			"tsuspend.;"
+			"trap;"
+			"tresume.;"
+			"1: ;"
+			: : : "memory");
+
+	return EXIT_SUCCESS;
+}
+
+int main(int argc, char **argv)
+{
+	/*
+	 * Depending on kernel config, the TM Bad Thing might not result in a
+	 * crash, instead the kernel never returns control back to userspace, so
+	 * set a tight timeout. If the test passes it completes almost
+	 * immediately.
+	 */
+	test_harness_set_timeout(2);
+	return test_harness(tm_signal_pagefault, "tm_signal_pagefault");
+}
-- 
2.21.1


^ permalink raw reply related

* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Qian Cai @ 2020-02-03 15:48 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
	Tetsuo Handa, Heiko Carstens, Michal Hocko, linux-mm,
	Paul Mackerras, kasan-dev, sparclinux, Thomas Gleixner,
	linux-s390, x86, Russell King - ARM Linux, Matthew Wilcox,
	Steven Price, Jason Gunthorpe, Gerald Schaefer, linux-snps-arc,
	Ingo Molnar, Kees Cook, Anshuman Khandual, Masahiro Yamada,
	Mark Brown, Kirill A . Shutemov, Dan Williams, Vlastimil Babka,
	linux-arm-kernel, Sri Krishna chowdary, Ard Biesheuvel,
	Greg Kroah-Hartman, Dave Hansen, linux-mips, Ralf Baechle,
	linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
	Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <8e94a073-4045-89aa-6a3b-24847ad7c858@c-s.fr>

On Mon, 2020-02-03 at 16:14 +0100, Christophe Leroy wrote:
> 
> Le 02/02/2020 à 12:26, Qian Cai a écrit :
> > 
> > 
> > > On Jan 30, 2020, at 9:13 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> > > 
> > > config DEBUG_VM_PGTABLE
> > >     bool "Debug arch page table for semantics compliance" if ARCH_HAS_DEBUG_VM_PGTABLE || EXPERT
> > >     depends on MMU
> > >     default 'n' if !ARCH_HAS_DEBUG_VM_PGTABLE
> > >     default 'y' if DEBUG_VM
> > 
> > Does it really necessary to potentially force all bots to run this? Syzbot, kernel test robot etc? Does it ever pay off for all their machine times there?
> > 
> 
> Machine time ?
> 
> On a 32 bits powerpc running at 132 MHz, the tests takes less than 10ms. 
> Is it worth taking the risk of not detecting faults by not selecting it 
> by default ?

The risk is quite low as Catalin mentioned this thing is not to detect
regressions but rather for arch/mm maintainers.

I do appreciate the efforts to get everyone as possible to run this thing,
so it get more notices once it is broken. However, DEBUG_VM seems like such
a generic Kconfig those days that have even been enabled by default for
Fedora Linux, so I would rather see a more sensitive default been taken
even though the test runtime is fairly quickly on a small machine for now.

> 
> [    5.656916] debug_vm_pgtable: debug_vm_pgtable: Validating 
> architecture page table helpers
> [    5.665661] debug_vm_pgtable: debug_vm_pgtable: Validated 
> architecture page table helpers


^ permalink raw reply

* Re: [PATCH V12] mm/debug: Add tests validating architecture page table helpers
From: Christophe Leroy @ 2020-02-03 15:14 UTC (permalink / raw)
  To: Qian Cai
  Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
	Tetsuo Handa, Heiko Carstens, Michal Hocko, linux-mm,
	Paul Mackerras, kasan-dev, sparclinux, Thomas Gleixner,
	linux-s390, x86, Russell King - ARM Linux, Matthew Wilcox,
	Steven Price, Jason Gunthorpe, Gerald Schaefer, linux-snps-arc,
	Ingo Molnar, Kees Cook, Anshuman Khandual, Masahiro Yamada,
	Mark Brown, Kirill A . Shutemov, Dan Williams, Vlastimil Babka,
	linux-arm-kernel, Sri Krishna chowdary, Ard Biesheuvel,
	Greg Kroah-Hartman, Dave Hansen, linux-mips, Ralf Baechle,
	linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
	Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <2C4ADFAE-7BB4-42B7-8F54-F036EA7A4316@lca.pw>



Le 02/02/2020 à 12:26, Qian Cai a écrit :
> 
> 
>> On Jan 30, 2020, at 9:13 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>
>> config DEBUG_VM_PGTABLE
>>     bool "Debug arch page table for semantics compliance" if ARCH_HAS_DEBUG_VM_PGTABLE || EXPERT
>>     depends on MMU
>>     default 'n' if !ARCH_HAS_DEBUG_VM_PGTABLE
>>     default 'y' if DEBUG_VM
> 
> Does it really necessary to potentially force all bots to run this? Syzbot, kernel test robot etc? Does it ever pay off for all their machine times there?
> 

Machine time ?

On a 32 bits powerpc running at 132 MHz, the tests takes less than 10ms. 
Is it worth taking the risk of not detecting faults by not selecting it 
by default ?

[    5.656916] debug_vm_pgtable: debug_vm_pgtable: Validating 
architecture page table helpers
[    5.665661] debug_vm_pgtable: debug_vm_pgtable: Validated 
architecture page table helpers

Christophe

^ 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