linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] selftests/powerpc: Add MSR bits
@ 2019-01-03 16:11 Breno Leitao
  2019-01-03 16:11 ` [PATCH v3 2/2] selftests/powerpc: New TM signal self test Breno Leitao
  2019-01-03 17:19 ` [PATCH v3 1/2] selftests/powerpc: Add MSR bits LEROY Christophe
  0 siblings, 2 replies; 10+ messages in thread
From: Breno Leitao @ 2019-01-03 16:11 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Breno Leitao, mikey, gromero

This patch simply adds definitions for the MSR bits and some macros to
test for MSR TM bits.

This was copied from arch/powerpc/include/asm/reg.h generic MSR part.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 tools/testing/selftests/powerpc/include/reg.h | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/tools/testing/selftests/powerpc/include/reg.h b/tools/testing/selftests/powerpc/include/reg.h
index 52b4710469d2..b67a85404255 100644
--- a/tools/testing/selftests/powerpc/include/reg.h
+++ b/tools/testing/selftests/powerpc/include/reg.h
@@ -77,6 +77,51 @@
 #define TEXASR_TE	0x0000000004000000
 #define TEXASR_ROT	0x0000000002000000
 
+/* MSR register bits */
+#define MSR_SF_LG       63              /* Enable 64 bit mode */
+#define MSR_ISF_LG      61              /* Interrupt 64b mode valid on 630 */
+#define MSR_HV_LG       60              /* Hypervisor state */
+#define MSR_TS_T_LG     34              /* Trans Mem state: Transactional */
+#define MSR_TS_S_LG     33              /* Trans Mem state: Suspended */
+#define MSR_TS_LG       33              /* Trans Mem state (2 bits) */
+#define MSR_TM_LG       32              /* Trans Mem Available */
+#define MSR_VEC_LG      25              /* Enable AltiVec */
+#define MSR_VSX_LG      23              /* Enable VSX */
+#define MSR_POW_LG      18              /* Enable Power Management */
+#define MSR_WE_LG       18              /* Wait State Enable */
+#define MSR_TGPR_LG     17              /* TLB Update registers in use */
+#define MSR_CE_LG       17              /* Critical Interrupt Enable */
+#define MSR_ILE_LG      16              /* Interrupt Little Endian */
+#define MSR_EE_LG       15              /* External Interrupt Enable */
+#define MSR_PR_LG       14              /* Problem State / Privilege Level */
+#define MSR_FP_LG       13              /* Floating Point enable */
+#define MSR_ME_LG       12              /* Machine Check Enable */
+#define MSR_FE0_LG      11              /* Floating Exception mode 0 */
+#define MSR_SE_LG       10              /* Single Step */
+#define MSR_BE_LG       9               /* Branch Trace */
+#define MSR_DE_LG       9               /* Debug Exception Enable */
+#define MSR_FE1_LG      8               /* Floating Exception mode 1 */
+#define MSR_IP_LG       6               /* Exception prefix 0x000/0xFFF */
+#define MSR_IR_LG       5               /* Instruction Relocate */
+#define MSR_DR_LG       4               /* Data Relocate */
+#define MSR_PE_LG       3               /* Protection Enable */
+#define MSR_PX_LG       2               /* Protection Exclusive Mode */
+#define MSR_PMM_LG      2               /* Performance monitor */
+#define MSR_RI_LG       1               /* Recoverable Exception */
+#define MSR_LE_LG       0               /* Little Endian */
+
+#ifdef __ASSEMBLY__
+#define __MASK(X)       (1<<(X))
+#else
+#define __MASK(X)       (1UL<<(X))
+#endif
+
+/* macros to check TM MSR bits */
+#define MSR_TM          __MASK(MSR_TM_LG)     /* Transactional Mem Available */
+#define MSR_TS_S        __MASK(MSR_TS_S_LG)   /* Transaction Suspended */
+#define MSR_TS_T        __MASK(MSR_TS_T_LG)   /* Transaction Transactional */
+#define MSR_TS_MASK     (MSR_TS_T | MSR_TS_S) /* Transaction State bits */
+
 /* Vector Instructions */
 #define VSX_XX1(xs, ra, rb)	(((xs) & 0x1f) << 21 | ((ra) << 16) |  \
 				 ((rb) << 11) | (((xs) >> 5)))
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/2] selftests/powerpc: New TM signal self test
  2019-01-03 16:11 [PATCH v3 1/2] selftests/powerpc: Add MSR bits Breno Leitao
@ 2019-01-03 16:11 ` Breno Leitao
  2019-01-03 17:19 ` [PATCH v3 1/2] selftests/powerpc: Add MSR bits LEROY Christophe
  1 sibling, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2019-01-03 16:11 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Breno Leitao, mikey, gromero

A new self test that forces MSR[TS] to be set without calling any TM
instruction. This test also tries to cause a page fault at a signal
handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing
thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG
when tm_recheckpoint() is called.

This test is not deterministic, since it is hard to guarantee that the page
access will cause a page fault. In order to force more page faults at
signal context, the signal handler and the ucontext are being mapped into a
MADV_DONTNEED memory chunks.

Tests have shown that the bug could be exposed with few interactions in a
buggy kernel. This test is configured to loop 5000x, having a good chance
to hit the kernel issue in just one run.  This self test takes less than
two seconds to run.

This test uses set/getcontext because the kernel will recheckpoint
zeroed structures, causing the test to segfault, which is undesired because
the test needs to rerun, so, there is a signal handler for SIGSEGV which
will restart the test.

v2: Uses the MADV_DONTNEED memory advice
v3: Fix memcpy and 32-bits compilation

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 .../testing/selftests/powerpc/include/utils.h |   2 +
 tools/testing/selftests/powerpc/tm/.gitignore |   1 +
 tools/testing/selftests/powerpc/tm/Makefile   |   4 +-
 .../powerpc/tm/tm-signal-context-force-tm.c   | 184 ++++++++++++++++++
 4 files changed, 190 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c

diff --git a/tools/testing/selftests/powerpc/include/utils.h b/tools/testing/selftests/powerpc/include/utils.h
index ae43a614835d..7636bf45d5d5 100644
--- a/tools/testing/selftests/powerpc/include/utils.h
+++ b/tools/testing/selftests/powerpc/include/utils.h
@@ -102,8 +102,10 @@ do {								\
 
 #if defined(__powerpc64__)
 #define UCONTEXT_NIA(UC)	(UC)->uc_mcontext.gp_regs[PT_NIP]
+#define UCONTEXT_MSR(UC)	(UC)->uc_mcontext.gp_regs[PT_MSR]
 #elif defined(__powerpc__)
 #define UCONTEXT_NIA(UC)	(UC)->uc_mcontext.uc_regs->gregs[PT_NIP]
+#define UCONTEXT_MSR(UC)	(UC)->uc_mcontext.uc_regs->gregs[PT_MSR]
 #else
 #error implement UCONTEXT_NIA
 #endif
diff --git a/tools/testing/selftests/powerpc/tm/.gitignore b/tools/testing/selftests/powerpc/tm/.gitignore
index 208452a93e2c..951fe855f7cd 100644
--- a/tools/testing/selftests/powerpc/tm/.gitignore
+++ b/tools/testing/selftests/powerpc/tm/.gitignore
@@ -11,6 +11,7 @@ tm-signal-context-chk-fpu
 tm-signal-context-chk-gpr
 tm-signal-context-chk-vmx
 tm-signal-context-chk-vsx
+tm-signal-context-force-tm
 tm-signal-sigreturn-nt
 tm-vmx-unavail
 tm-unavailable
diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
index 75a685359129..c0734ed0ef56 100644
--- a/tools/testing/selftests/powerpc/tm/Makefile
+++ b/tools/testing/selftests/powerpc/tm/Makefile
@@ -4,7 +4,8 @@ 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
+	$(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-signal-sigreturn-nt \
+	tm-signal-context-force-tm
 
 top_srcdir = ../../../../..
 include ../../lib.mk
@@ -20,6 +21,7 @@ $(OUTPUT)/tm-vmx-unavail: CFLAGS += -pthread -m64
 $(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
 
 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-context-force-tm.c b/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
new file mode 100644
index 000000000000..31717625f318
--- /dev/null
+++ b/tools/testing/selftests/powerpc/tm/tm-signal-context-force-tm.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2018, Breno Leitao, Gustavo Romero, IBM Corp.
+ *
+ * This test raises a SIGUSR1 signal, and toggle the MSR[TS]
+ * fields at the signal handler. With MSR[TS] being set, the kernel will
+ * force a recheckpoint, which may cause a segfault when returning to
+ * user space. Since the test needs to re-run, the segfault needs to be
+ * caught and handled.
+ *
+ * In order to continue the test even after a segfault, the context is
+ * saved prior to the signal being raised, and it is restored when there is
+ * a segmentation fault. This happens for COUNT_MAX times.
+ *
+ * This test never fails (as returning EXIT_FAILURE). It either succeeds,
+ * or crash the kernel (on a buggy kernel).
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <string.h>
+#include <ucontext.h>
+#include <unistd.h>
+#include <sys/mman.h>
+
+#include "tm.h"
+#include "utils.h"
+#include "reg.h"
+
+#define COUNT_MAX       5000		/* Number of interactions */
+
+/*
+ * This test only runs on 64 bits system. Unsetting MSR_TS_S to avoid
+ * compilation issue on 32 bits system. There is no side effect, since the
+ * whole test will be skipped if it is not running on 64 bits system.
+ */
+#ifndef __powerpc64__
+#undef  MSR_TS_S
+#define MSR_TS_S	0
+#endif
+
+/* Setting contexts because the test will crash and we want to recover */
+ucontext_t init_context, main_context;
+
+static int count, first_time;
+
+void usr_signal_handler(int signo, siginfo_t *si, void *uc)
+{
+	ucontext_t *ucp = uc;
+	int ret;
+
+	/*
+	 * Allocating memory in a signal handler, and never freeing it on
+	 * purpose, forcing the heap increase, so, the memory leak is what
+	 * we want here.
+	 */
+	ucp->uc_link = mmap(NULL, sizeof(ucontext_t),
+			    PROT_READ | PROT_WRITE,
+			    MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+	if (ucp->uc_link == (void *)-1) {
+		perror("Mmap failed");
+		exit(-1);
+	}
+
+	/* Forcing the page to be allocated in a page fault */
+	ret = madvise(ucp->uc_link, sizeof(ucontext_t), MADV_DONTNEED);
+	if (ret) {
+		perror("madvise failed");
+		exit(-1);
+	}
+
+	memcpy(&ucp->uc_link->uc_mcontext, &ucp->uc_mcontext,
+		sizeof(ucp->uc_mcontext));
+
+	/* Forcing to enable MSR[TM] */
+	UCONTEXT_MSR(ucp) |= MSR_TS_S;
+
+	/*
+	 * A fork inside a signal handler seems to be more efficient than a
+	 * fork() prior to the signal being raised.
+	 */
+	if (fork() == 0) {
+		/*
+		 * Both child and parent will return, but, child returns
+		 * with count set so it will exit in the next segfault.
+		 * Parent will continue to loop.
+		 */
+		count = COUNT_MAX;
+	}
+
+	/*
+	 * If the change above does not hit the bug, it will cause a
+	 * segmentation fault, since the ck structures are NULL.
+	 */
+}
+
+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 */
+	setcontext(&init_context);
+}
+
+void tm_trap_test(void)
+{
+	struct sigaction usr_sa, seg_sa;
+	stack_t ss;
+
+	usr_sa.sa_flags = SA_SIGINFO | SA_ONSTACK;
+	usr_sa.sa_sigaction = usr_signal_handler;
+
+	seg_sa.sa_flags = SA_SIGINFO;
+	seg_sa.sa_sigaction = seg_signal_handler;
+
+	/*
+	 * Set initial context. Will get back here from
+	 * seg_signal_handler()
+	 */
+	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);
+	}
+
+	/* 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)
+{
+	SKIP_IF(!have_htm());
+	/*
+	 * Skipping if not running on 64 bits system, since I think it is
+	 * not possible to set mcontext's [MSR] with TS, due to it being 32
+	 * bits.
+	 */
+	SKIP_IF(!is_ppc64le());
+
+	/* Will get back here after COUNT_MAX interactions */
+	getcontext(&main_context);
+
+	if (!first_time++)
+		tm_trap_test();
+
+	return EXIT_SUCCESS;
+}
+
+int main(int argc, char **argv)
+{
+	test_harness(tm_signal_context_force_tm, "tm_signal_context_force_tm");
+}
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] selftests/powerpc: Add MSR bits
  2019-01-03 16:11 [PATCH v3 1/2] selftests/powerpc: Add MSR bits Breno Leitao
  2019-01-03 16:11 ` [PATCH v3 2/2] selftests/powerpc: New TM signal self test Breno Leitao
@ 2019-01-03 17:19 ` LEROY Christophe
  2019-01-07 12:44   ` Breno Leitao
  1 sibling, 1 reply; 10+ messages in thread
From: LEROY Christophe @ 2019-01-03 17:19 UTC (permalink / raw)
  To: Breno Leitao; +Cc: mikey, linuxppc-dev, gromero

Breno Leitao <leitao@debian.org> a écrit :

> This patch simply adds definitions for the MSR bits and some macros to
> test for MSR TM bits.
>
> This was copied from arch/powerpc/include/asm/reg.h generic MSR part.

Can't we find a way to avoid duplicating such defines ?

Christophe

>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  tools/testing/selftests/powerpc/include/reg.h | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/tools/testing/selftests/powerpc/include/reg.h  
> b/tools/testing/selftests/powerpc/include/reg.h
> index 52b4710469d2..b67a85404255 100644
> --- a/tools/testing/selftests/powerpc/include/reg.h
> +++ b/tools/testing/selftests/powerpc/include/reg.h
> @@ -77,6 +77,51 @@
>  #define TEXASR_TE	0x0000000004000000
>  #define TEXASR_ROT	0x0000000002000000
>
> +/* MSR register bits */
> +#define MSR_SF_LG       63              /* Enable 64 bit mode */
> +#define MSR_ISF_LG      61              /* Interrupt 64b mode valid  
> on 630 */
> +#define MSR_HV_LG       60              /* Hypervisor state */
> +#define MSR_TS_T_LG     34              /* Trans Mem state: Transactional */
> +#define MSR_TS_S_LG     33              /* Trans Mem state: Suspended */
> +#define MSR_TS_LG       33              /* Trans Mem state (2 bits) */
> +#define MSR_TM_LG       32              /* Trans Mem Available */
> +#define MSR_VEC_LG      25              /* Enable AltiVec */
> +#define MSR_VSX_LG      23              /* Enable VSX */
> +#define MSR_POW_LG      18              /* Enable Power Management */
> +#define MSR_WE_LG       18              /* Wait State Enable */
> +#define MSR_TGPR_LG     17              /* TLB Update registers in use */
> +#define MSR_CE_LG       17              /* Critical Interrupt Enable */
> +#define MSR_ILE_LG      16              /* Interrupt Little Endian */
> +#define MSR_EE_LG       15              /* External Interrupt Enable */
> +#define MSR_PR_LG       14              /* Problem State /  
> Privilege Level */
> +#define MSR_FP_LG       13              /* Floating Point enable */
> +#define MSR_ME_LG       12              /* Machine Check Enable */
> +#define MSR_FE0_LG      11              /* Floating Exception mode 0 */
> +#define MSR_SE_LG       10              /* Single Step */
> +#define MSR_BE_LG       9               /* Branch Trace */
> +#define MSR_DE_LG       9               /* Debug Exception Enable */
> +#define MSR_FE1_LG      8               /* Floating Exception mode 1 */
> +#define MSR_IP_LG       6               /* Exception prefix 0x000/0xFFF */
> +#define MSR_IR_LG       5               /* Instruction Relocate */
> +#define MSR_DR_LG       4               /* Data Relocate */
> +#define MSR_PE_LG       3               /* Protection Enable */
> +#define MSR_PX_LG       2               /* Protection Exclusive Mode */
> +#define MSR_PMM_LG      2               /* Performance monitor */
> +#define MSR_RI_LG       1               /* Recoverable Exception */
> +#define MSR_LE_LG       0               /* Little Endian */
> +
> +#ifdef __ASSEMBLY__
> +#define __MASK(X)       (1<<(X))
> +#else
> +#define __MASK(X)       (1UL<<(X))
> +#endif
> +
> +/* macros to check TM MSR bits */
> +#define MSR_TM          __MASK(MSR_TM_LG)     /* Transactional Mem  
> Available */
> +#define MSR_TS_S        __MASK(MSR_TS_S_LG)   /* Transaction Suspended */
> +#define MSR_TS_T        __MASK(MSR_TS_T_LG)   /* Transaction  
> Transactional */
> +#define MSR_TS_MASK     (MSR_TS_T | MSR_TS_S) /* Transaction State bits */
> +
>  /* Vector Instructions */
>  #define VSX_XX1(xs, ra, rb)	(((xs) & 0x1f) << 21 | ((ra) << 16) |  \
>  				 ((rb) << 11) | (((xs) >> 5)))
> --
> 2.19.0



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] selftests/powerpc: Add MSR bits
  2019-01-03 17:19 ` [PATCH v3 1/2] selftests/powerpc: Add MSR bits LEROY Christophe
@ 2019-01-07 12:44   ` Breno Leitao
  2019-01-07 12:47     ` Christophe Leroy
  2019-01-08  9:20     ` Michael Ellerman
  0 siblings, 2 replies; 10+ messages in thread
From: Breno Leitao @ 2019-01-07 12:44 UTC (permalink / raw)
  To: LEROY Christophe; +Cc: mikey, linuxppc-dev, gromero

hi Christophe,

On 1/3/19 3:19 PM, LEROY Christophe wrote:
> Breno Leitao <leitao@debian.org> a écrit :
> 
>> This patch simply adds definitions for the MSR bits and some macros to
>> test for MSR TM bits.
>>
>> This was copied from arch/powerpc/include/asm/reg.h generic MSR part.
> 
> Can't we find a way to avoid duplicating such defines ?

I think there are three possible ways, but none of them respect the premises
we are used too. These are the possible ways I can think of:

1) Including arch/powerpc/include/asm as part of the selftest compilation
process.
   Problem: This might break the selftest independence of the kbuild system.

2) Generate a temporary header file inside selftests/include which contains
these macros at compilation time.
   Problem: The problem as above.

3) Define MSR fields at userspace headers (/usr/include).
   Problem: I am not sure userspace should have MSR bits information.

Do you suggest me to investigate any other way?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] selftests/powerpc: Add MSR bits
  2019-01-07 12:44   ` Breno Leitao
@ 2019-01-07 12:47     ` Christophe Leroy
  2019-01-07 12:58       ` Breno Leitao
  2019-01-08  9:20     ` Michael Ellerman
  1 sibling, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2019-01-07 12:47 UTC (permalink / raw)
  To: Breno Leitao; +Cc: mikey, linuxppc-dev, gromero

Hi Breno,

Le 07/01/2019 à 13:44, Breno Leitao a écrit :
> hi Christophe,
> 
> On 1/3/19 3:19 PM, LEROY Christophe wrote:
>> Breno Leitao <leitao@debian.org> a écrit :
>>
>>> This patch simply adds definitions for the MSR bits and some macros to
>>> test for MSR TM bits.
>>>
>>> This was copied from arch/powerpc/include/asm/reg.h generic MSR part.
>>
>> Can't we find a way to avoid duplicating such defines ?
> 
> I think there are three possible ways, but none of them respect the premises
> we are used too. These are the possible ways I can think of:
> 
> 1) Including arch/powerpc/include/asm as part of the selftest compilation
> process.
>     Problem: This might break the selftest independence of the kbuild system.
> 
> 2) Generate a temporary header file inside selftests/include which contains
> these macros at compilation time.
>     Problem: The problem as above.
> 
> 3) Define MSR fields at userspace headers (/usr/include).
>     Problem: I am not sure userspace should have MSR bits information.
> 
> Do you suggest me to investigate any other way?

Looking it other .h in selftests, it looks like they are limited to the 
only strictly necessary values.

Are all the values you have listed used ? If not, could you only include 
in the file the necessary ones ?

Christophe

> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] selftests/powerpc: Add MSR bits
  2019-01-07 12:47     ` Christophe Leroy
@ 2019-01-07 12:58       ` Breno Leitao
  0 siblings, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2019-01-07 12:58 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: mikey, linuxppc-dev, gromero

hi Christophe,

On 1/7/19 10:47 AM, Christophe Leroy wrote:
> Hi Breno,
> 
> Le 07/01/2019 à 13:44, Breno Leitao a écrit :
>> hi Christophe,
>>
>> On 1/3/19 3:19 PM, LEROY Christophe wrote:
>>> Breno Leitao <leitao@debian.org> a écrit :
>>>
>>>> This patch simply adds definitions for the MSR bits and some macros to
>>>> test for MSR TM bits.
>>>>
>>>> This was copied from arch/powerpc/include/asm/reg.h generic MSR part.
>>>
>>> Can't we find a way to avoid duplicating such defines ?
>>
>> I think there are three possible ways, but none of them respect the premises
>> we are used too. These are the possible ways I can think of:
>>
>> 1) Including arch/powerpc/include/asm as part of the selftest compilation
>> process.
>>     Problem: This might break the selftest independence of the kbuild system.
>>
>> 2) Generate a temporary header file inside selftests/include which contains
>> these macros at compilation time.
>>     Problem: The problem as above.
>>
>> 3) Define MSR fields at userspace headers (/usr/include).
>>     Problem: I am not sure userspace should have MSR bits information.
>>
>> Do you suggest me to investigate any other way?
> 
> Looking it other .h in selftests, it looks like they are limited to the only
> strictly necessary values.
> 
> Are all the values you have listed used ? If not, could you only include in
> the file the necessary ones ?

Sure. That works also.  Let send a v4 patch.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] selftests/powerpc: Add MSR bits
  2019-01-07 12:44   ` Breno Leitao
  2019-01-07 12:47     ` Christophe Leroy
@ 2019-01-08  9:20     ` Michael Ellerman
  2019-01-08 11:11       ` Breno Leitao
  1 sibling, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2019-01-08  9:20 UTC (permalink / raw)
  To: Breno Leitao, LEROY Christophe; +Cc: mikey, linuxppc-dev, gromero

Breno Leitao <leitao@debian.org> writes:

> hi Christophe,
>
> On 1/3/19 3:19 PM, LEROY Christophe wrote:
>> Breno Leitao <leitao@debian.org> a écrit :
>> 
>>> This patch simply adds definitions for the MSR bits and some macros to
>>> test for MSR TM bits.
>>>
>>> This was copied from arch/powerpc/include/asm/reg.h generic MSR part.
>> 
>> Can't we find a way to avoid duplicating such defines ?
>
> I think there are three possible ways, but none of them respect the premises
> we are used too. These are the possible ways I can think of:
>
> 1) Including arch/powerpc/include/asm as part of the selftest compilation
> process.
>    Problem: This might break the selftest independence of the kbuild system.
>
> 2) Generate a temporary header file inside selftests/include which contains
> these macros at compilation time.
>    Problem: The problem as above.
>
> 3) Define MSR fields at userspace headers (/usr/include).
>    Problem: I am not sure userspace should have MSR bits information.
>
> Do you suggest me to investigate any other way?

In this case I think we can probably just copy the few #defines we need.

If we decide in future we need all or most of the MSR defines then I
would suggest we:
 - split *only* the MSR defines into arch/powerpc/include/asm/msr.h
 - include that file from reg.h
 - symlink that into the selftest code


cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] selftests/powerpc: Add MSR bits
  2019-01-08  9:20     ` Michael Ellerman
@ 2019-01-08 11:11       ` Breno Leitao
  2019-01-08 11:14         ` Christophe Leroy
  0 siblings, 1 reply; 10+ messages in thread
From: Breno Leitao @ 2019-01-08 11:11 UTC (permalink / raw)
  To: Michael Ellerman, LEROY Christophe; +Cc: mikey, linuxppc-dev, gromero

Hi Michael,

On 1/8/19 7:20 AM, Michael Ellerman wrote:
> Breno Leitao <leitao@debian.org> writes:
> 
>> hi Christophe,
>>
>> On 1/3/19 3:19 PM, LEROY Christophe wrote:
>>> Breno Leitao <leitao@debian.org> a écrit :
>>>
>>>> This patch simply adds definitions for the MSR bits and some macros to
>>>> test for MSR TM bits.
>>>>
>>>> This was copied from arch/powerpc/include/asm/reg.h generic MSR part.
>>>
>>> Can't we find a way to avoid duplicating such defines ?
>>
>> I think there are three possible ways, but none of them respect the premises
>> we are used too. These are the possible ways I can think of:
>>
>> 1) Including arch/powerpc/include/asm as part of the selftest compilation
>> process.
>>    Problem: This might break the selftest independence of the kbuild system.
>>
>> 2) Generate a temporary header file inside selftests/include which contains
>> these macros at compilation time.
>>    Problem: The problem as above.
>>
>> 3) Define MSR fields at userspace headers (/usr/include).
>>    Problem: I am not sure userspace should have MSR bits information.
>>
>> Do you suggest me to investigate any other way?
> 
> In this case I think we can probably just copy the few #defines we need.

Cool. That is the very same thing as suggested by Christophe. I sent the v4
yesterday, with this change already.

Thanks for looking at it.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] selftests/powerpc: Add MSR bits
  2019-01-08 11:11       ` Breno Leitao
@ 2019-01-08 11:14         ` Christophe Leroy
  2019-01-08 11:31           ` Breno Leitao
  0 siblings, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2019-01-08 11:14 UTC (permalink / raw)
  To: Breno Leitao, Michael Ellerman; +Cc: mikey, linuxppc-dev, gromero


Le 08/01/2019 à 12:11, Breno Leitao a écrit :
> Hi Michael,
> 
> On 1/8/19 7:20 AM, Michael Ellerman wrote:
>> Breno Leitao <leitao@debian.org> writes:
>>
>>> hi Christophe,
>>>
>>> On 1/3/19 3:19 PM, LEROY Christophe wrote:
>>>> Breno Leitao <leitao@debian.org> a écrit :
>>>>
>>>>> This patch simply adds definitions for the MSR bits and some macros to
>>>>> test for MSR TM bits.
>>>>>
>>>>> This was copied from arch/powerpc/include/asm/reg.h generic MSR part.
>>>>
>>>> Can't we find a way to avoid duplicating such defines ?
>>>
>>> I think there are three possible ways, but none of them respect the premises
>>> we are used too. These are the possible ways I can think of:
>>>
>>> 1) Including arch/powerpc/include/asm as part of the selftest compilation
>>> process.
>>>     Problem: This might break the selftest independence of the kbuild system.
>>>
>>> 2) Generate a temporary header file inside selftests/include which contains
>>> these macros at compilation time.
>>>     Problem: The problem as above.
>>>
>>> 3) Define MSR fields at userspace headers (/usr/include).
>>>     Problem: I am not sure userspace should have MSR bits information.
>>>
>>> Do you suggest me to investigate any other way?
>>
>> In this case I think we can probably just copy the few #defines we need.
> 
> Cool. That is the very same thing as suggested by Christophe. I sent the v4
> yesterday, with this change already.

It didn't get through. Latest is still v3 it seems, look 
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?submitter=71262

Christophe

> 
> Thanks for looking at it.
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] selftests/powerpc: Add MSR bits
  2019-01-08 11:14         ` Christophe Leroy
@ 2019-01-08 11:31           ` Breno Leitao
  0 siblings, 0 replies; 10+ messages in thread
From: Breno Leitao @ 2019-01-08 11:31 UTC (permalink / raw)
  To: Christophe Leroy, Michael Ellerman; +Cc: mikey, linuxppc-dev, gromero

On 1/8/19 9:14 AM, Christophe Leroy wrote:
> Le 08/01/2019 à 12:11, Breno Leitao a écrit :
>> Hi Michael,
>>
>> On 1/8/19 7:20 AM, Michael Ellerman wrote:
>>> Breno Leitao <leitao@debian.org> writes:
>>>
>>>> hi Christophe,
>>>>
>>>> On 1/3/19 3:19 PM, LEROY Christophe wrote:
>>>>> Breno Leitao <leitao@debian.org> a écrit :
>>>>>
>>>>>> This patch simply adds definitions for the MSR bits and some macros to
>>>>>> test for MSR TM bits.
>>>>>>
>>>>>> This was copied from arch/powerpc/include/asm/reg.h generic MSR part.
>>>>>
>>>>> Can't we find a way to avoid duplicating such defines ?
>>>>
>>>> I think there are three possible ways, but none of them respect the premises
>>>> we are used too. These are the possible ways I can think of:
>>>>
>>>> 1) Including arch/powerpc/include/asm as part of the selftest compilation
>>>> process.
>>>>     Problem: This might break the selftest independence of the kbuild
>>>> system.
>>>>
>>>> 2) Generate a temporary header file inside selftests/include which contains
>>>> these macros at compilation time.
>>>>     Problem: The problem as above.
>>>>
>>>> 3) Define MSR fields at userspace headers (/usr/include).
>>>>     Problem: I am not sure userspace should have MSR bits information.
>>>>
>>>> Do you suggest me to investigate any other way?
>>>
>>> In this case I think we can probably just copy the few #defines we need.
>>
>> Cool. That is the very same thing as suggested by Christophe. I sent the v4
>> yesterday, with this change already.
> 
> It didn't get through. Latest is still v3 it seems, look
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?submitter=71262

Weird, I am not finding it as well, although I've just confirmed that I have,
in fact, sent it. Anyway, let me send it again.

Thanks!

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-01-08 11:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-03 16:11 [PATCH v3 1/2] selftests/powerpc: Add MSR bits Breno Leitao
2019-01-03 16:11 ` [PATCH v3 2/2] selftests/powerpc: New TM signal self test Breno Leitao
2019-01-03 17:19 ` [PATCH v3 1/2] selftests/powerpc: Add MSR bits LEROY Christophe
2019-01-07 12:44   ` Breno Leitao
2019-01-07 12:47     ` Christophe Leroy
2019-01-07 12:58       ` Breno Leitao
2019-01-08  9:20     ` Michael Ellerman
2019-01-08 11:11       ` Breno Leitao
2019-01-08 11:14         ` Christophe Leroy
2019-01-08 11:31           ` Breno Leitao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).