All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption
@ 2017-05-08  7:16 Michael Neuling
  2017-05-08  7:16 ` [PATCH 2/2] selftests/powerpc: Test TM and VMX register state Michael Neuling
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Michael Neuling @ 2017-05-08  7:16 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, mikey, cyrilbur, Breno Leitao, Gustavo Bueno Romero

In this commit:
  commit dc3106690b20305c3df06b42456fe386dd632ac9
  Author: Cyril Bur <cyrilbur@gmail.com>
  powerpc: tm: Always use fp_state and vr_state to store live registers

A section of code was removed that copied the current state to
checkpointed state.  This should not have been removed.

When an FP unavailable is taken inside a transaction, we need to abort
the transaction. This is because at the time of the tbegin, the FP
state is bogus so the state stored in the checkpointed registers is
incorrect. To fix this, we treclaim (to get the checkpointed GPRs) and
then copy the thread_struct FP live state into the checkpointed
state. We then trecheckpoint so that the FP state is correctly
restored into the CPU.

The coping of the FP registers from live to checkpointed is what was
missing.

This simplifies the logic slightly from the original patch.
tm_reclaim_thread() will now always write the checkpointed FP
state. Either the checkpointed FP statte will be written as part of
the actual treclaim (in tm.S), or it'll be a copy of the live
state. Which one we use is based on MSR[FP] from userspace.

Similarly for VMX.

CC: <stable@vger.kernel.org> # 4.9+
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 arch/powerpc/kernel/process.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index d645da302b..6305353237 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -864,6 +864,25 @@ static void tm_reclaim_thread(struct thread_struct *thr,
 	if (!MSR_TM_SUSPENDED(mfmsr()))
 		return;
 
+	/*
+	 * If we are in a transaction and FP is off then we can't have
+	 * used FP inside that transaction. Hence the checkpointed
+	 * state is the same as the live state. We need to copy the
+	 * live state to the checkpointed state so that when the
+	 * transaction is restored, the checkpointed state is correct
+	 * and the aborted transaction sees the correct state.  We use
+	 * ckpt_regs.msr here as that's what tm_reclaim will use to
+	 * determine if it's going to write the checkpointed state or
+	 * not. So either this will write the checkpointed registers,
+	 * or reclaim will.  Similarly for VMX.
+	 */
+	if ((thr->ckpt_regs.msr & MSR_FP) == 0)
+		memcpy(&thr->ckfp_state, &thr->fp_state,
+		       sizeof(struct thread_fp_state));
+	if ((thr->ckpt_regs.msr & MSR_VEC) == 0)
+		memcpy(&thr->ckvr_state, &thr->vr_state,
+		       sizeof(struct thread_vr_state));
+
 	giveup_all(container_of(thr, struct task_struct, thread));
 
 	tm_reclaim(thr, thr->ckpt_regs.msr, cause);
-- 
2.11.0

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

* [PATCH 2/2] selftests/powerpc: Test TM and VMX register state
  2017-05-08  7:16 [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption Michael Neuling
@ 2017-05-08  7:16 ` Michael Neuling
  2017-05-12 15:16   ` Cyril Bur
                     ` (2 more replies)
  2017-05-12 15:26 ` [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption Cyril Bur
  2017-05-17  9:34 ` [1/2] " Michael Ellerman
  2 siblings, 3 replies; 8+ messages in thread
From: Michael Neuling @ 2017-05-08  7:16 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, mikey, cyrilbur, Breno Leitao, Gustavo Bueno Romero

Test that the VMX checkpointed register state is maintained when a VMX
unavailable exception is taken during a transaction.

Thanks to Breno Leitao <brenohl@br.ibm.com> and
Gustavo Bueno Romero <gromero@br.ibm.com> for the original test this
is based heavily on.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 tools/testing/selftests/powerpc/tm/Makefile        |   4 +-
 .../testing/selftests/powerpc/tm/tm-vmx-unavail.c  | 118 +++++++++++++++++++++
 2 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c

diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
index 5576ee6a51..b947995bb1 100644
--- a/tools/testing/selftests/powerpc/tm/Makefile
+++ b/tools/testing/selftests/powerpc/tm/Makefile
@@ -2,7 +2,8 @@ SIGNAL_CONTEXT_CHK_TESTS := tm-signal-context-chk-gpr tm-signal-context-chk-fpu
 	tm-signal-context-chk-vmx tm-signal-context-chk-vsx
 
 TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack \
-	tm-vmxcopy tm-fork tm-tar tm-tmspr $(SIGNAL_CONTEXT_CHK_TESTS)
+	tm-vmxcopy tm-fork tm-tar tm-tmspr tm-vmx-unavail \
+	$(SIGNAL_CONTEXT_CHK_TESTS)
 
 include ../../lib.mk
 
@@ -13,6 +14,7 @@ CFLAGS += -mhtm
 $(OUTPUT)/tm-syscall: tm-syscall-asm.S
 $(OUTPUT)/tm-syscall: CFLAGS += -I../../../../../usr/include
 $(OUTPUT)/tm-tmspr: CFLAGS += -pthread
+$(OUTPUT)/tm-vmx-unavail: CFLAGS += -pthread
 
 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-vmx-unavail.c b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
new file mode 100644
index 0000000000..137185ba49
--- /dev/null
+++ b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
@@ -0,0 +1,118 @@
+/*
+ * Copyright 2017, Michael Neuling, IBM Corp.
+ * Licensed under GPLv2.
+ * Original: Breno Leitao <brenohl@br.ibm.com> &
+ *           Gustavo Bueno Romero <gromero@br.ibm.com>
+ * Edited: Michael Neuling
+ *
+ * Force VMX unavailable during a transaction and see if it corrupts
+ * the checkpointed VMX register state after the abort.
+ */
+
+#include <inttypes.h>
+#include <htmintrin.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <pthread.h>
+#include <sys/mman.h>
+#include <unistd.h>
+#include <pthread.h>
+
+#include "tm.h"
+#include "utils.h"
+
+int passed;
+
+void *worker(void *unused)
+{
+	__int128 vmx0;
+	uint64_t texasr;
+
+	asm goto (
+		"li       3, 1;"  /* Stick non-zero value in VMX0 */
+		"std      3, 0(%[vmx0_ptr]);"
+		"lvx      0, 0, %[vmx0_ptr];"
+
+		/* Wait here a bit so we get scheduled out 255 times */
+		"lis      3, 0x3fff;"
+		"1: ;"
+		"addi     3, 3, -1;"
+		"cmpdi    3, 0;"
+		"bne      1b;"
+
+		/* Kernel will hopefully turn VMX off now */
+
+		"tbegin. ;"
+		"beq      failure;"
+
+		/* Cause VMX unavail. Any VMX instruction */
+		"vaddcuw  0,0,0;"
+
+		"tend. ;"
+		"b        %l[success];"
+
+		/* Check VMX0 sanity after abort */
+		"failure: ;"
+		"lvx       1,  0, %[vmx0_ptr];"
+		"vcmpequb. 2,  0, 1;"
+		"bc        4, 24, %l[value_mismatch];"
+		"b        %l[value_match];"
+		:
+		: [vmx0_ptr] "r"(&vmx0)
+		: "r3"
+		: success, value_match, value_mismatch
+		);
+
+	/* HTM aborted and VMX0 is corrupted */
+value_mismatch:
+	texasr = __builtin_get_texasr();
+
+	printf("\n\n==============\n\n");
+	printf("Failure with error: %lx\n",   _TEXASR_FAILURE_CODE(texasr));
+	printf("Summary error     : %lx\n",   _TEXASR_FAILURE_SUMMARY(texasr));
+	printf("TFIAR exact       : %lx\n\n", _TEXASR_TFIAR_EXACT(texasr));
+
+	passed = 0;
+	return NULL;
+
+	/* HTM aborted but VMX0 is correct */
+value_match:
+//	printf("!");
+	return NULL;
+
+success:
+//	printf(".");
+	return NULL;
+}
+
+int tm_vmx_unavail_test()
+{
+	int threads;
+	pthread_t *thread;
+
+	SKIP_IF(!have_htm());
+
+	passed = 1;
+
+	threads = sysconf(_SC_NPROCESSORS_ONLN) * 4;
+	thread = malloc(sizeof(pthread_t)*threads);
+	if (!thread)
+		return EXIT_FAILURE;
+
+	for (uint64_t i = 0; i < threads; i++)
+		pthread_create(&thread[i], NULL, &worker, NULL);
+
+	for (uint64_t i = 0; i < threads; i++)
+		pthread_join(thread[i], NULL);
+
+	free(thread);
+
+	return passed ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+
+int main(int argc, char **argv)
+{
+	return test_harness(tm_vmx_unavail_test, "tm_vmx_unavail_test");
+}
-- 
2.11.0

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

* Re: [PATCH 2/2] selftests/powerpc: Test TM and VMX register state
  2017-05-08  7:16 ` [PATCH 2/2] selftests/powerpc: Test TM and VMX register state Michael Neuling
@ 2017-05-12 15:16   ` Cyril Bur
  2017-05-15  9:31   ` Michael Ellerman
  2017-05-17  9:34   ` [2/2] " Michael Ellerman
  2 siblings, 0 replies; 8+ messages in thread
From: Cyril Bur @ 2017-05-12 15:16 UTC (permalink / raw)
  To: Michael Neuling, mpe; +Cc: linuxppc-dev, Breno Leitao, Gustavo Bueno Romero

On Mon, 2017-05-08 at 17:16 +1000, Michael Neuling wrote:
> Test that the VMX checkpointed register state is maintained when a VMX
> unavailable exception is taken during a transaction.
> 
> Thanks to Breno Leitao <brenohl@br.ibm.com> and
> Gustavo Bueno Romero <gromero@br.ibm.com> for the original test this
> is based heavily on.
> 

Good looking test!

> Signed-off-by: Michael Neuling <mikey@neuling.org>

Reviewed-by: Cyril Bur <cyrilbur@gmail.com>

> ---
>  tools/testing/selftests/powerpc/tm/Makefile        |   4 +-
>  .../testing/selftests/powerpc/tm/tm-vmx-unavail.c  | 118 +++++++++++++++++++++
>  2 files changed, 121 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> 
> diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
> index 5576ee6a51..b947995bb1 100644
> --- a/tools/testing/selftests/powerpc/tm/Makefile
> +++ b/tools/testing/selftests/powerpc/tm/Makefile
> @@ -2,7 +2,8 @@ SIGNAL_CONTEXT_CHK_TESTS := tm-signal-context-chk-gpr tm-signal-context-chk-fpu
>  	tm-signal-context-chk-vmx tm-signal-context-chk-vsx
>  
>  TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack \
> -	tm-vmxcopy tm-fork tm-tar tm-tmspr $(SIGNAL_CONTEXT_CHK_TESTS)
> +	tm-vmxcopy tm-fork tm-tar tm-tmspr tm-vmx-unavail \
> +	$(SIGNAL_CONTEXT_CHK_TESTS)
>  
>  include ../../lib.mk
>  
> @@ -13,6 +14,7 @@ CFLAGS += -mhtm
>  $(OUTPUT)/tm-syscall: tm-syscall-asm.S
>  $(OUTPUT)/tm-syscall: CFLAGS += -I../../../../../usr/include
>  $(OUTPUT)/tm-tmspr: CFLAGS += -pthread
> +$(OUTPUT)/tm-vmx-unavail: CFLAGS += -pthread
>  
>  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-vmx-unavail.c b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> new file mode 100644
> index 0000000000..137185ba49
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> @@ -0,0 +1,118 @@
> +/*
> + * Copyright 2017, Michael Neuling, IBM Corp.
> + * Licensed under GPLv2.
> + * Original: Breno Leitao <brenohl@br.ibm.com> &
> + *           Gustavo Bueno Romero <gromero@br.ibm.com>
> + * Edited: Michael Neuling
> + *
> + * Force VMX unavailable during a transaction and see if it corrupts
> + * the checkpointed VMX register state after the abort.
> + */
> +
> +#include <inttypes.h>
> +#include <htmintrin.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <pthread.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +
> +#include "tm.h"
> +#include "utils.h"
> +
> +int passed;
> +
> +void *worker(void *unused)
> +{
> +	__int128 vmx0;
> +	uint64_t texasr;
> +
> +	asm goto (
> +		"li       3, 1;"  /* Stick non-zero value in VMX0 */
> +		"std      3, 0(%[vmx0_ptr]);"
> +		"lvx      0, 0, %[vmx0_ptr];"
> +
> +		/* Wait here a bit so we get scheduled out 255 times */
> +		"lis      3, 0x3fff;"
> +		"1: ;"
> +		"addi     3, 3, -1;"
> +		"cmpdi    3, 0;"
> +		"bne      1b;"
> +
> +		/* Kernel will hopefully turn VMX off now */
> +
> +		"tbegin. ;"
> +		"beq      failure;"
> +
> +		/* Cause VMX unavail. Any VMX instruction */
> +		"vaddcuw  0,0,0;"
> +
> +		"tend. ;"
> +		"b        %l[success];"
> +
> +		/* Check VMX0 sanity after abort */
> +		"failure: ;"
> +		"lvx       1,  0, %[vmx0_ptr];"
> +		"vcmpequb. 2,  0, 1;"
> +		"bc        4, 24, %l[value_mismatch];"
> +		"b        %l[value_match];"
> +		:
> +		: [vmx0_ptr] "r"(&vmx0)
> +		: "r3"
> +		: success, value_match, value_mismatch
> +		);
> +
> +	/* HTM aborted and VMX0 is corrupted */
> +value_mismatch:
> +	texasr = __builtin_get_texasr();
> +
> +	printf("\n\n==============\n\n");
> +	printf("Failure with error: %lx\n",   _TEXASR_FAILURE_CODE(texasr));
> +	printf("Summary error     : %lx\n",   _TEXASR_FAILURE_SUMMARY(texasr));
> +	printf("TFIAR exact       : %lx\n\n", _TEXASR_TFIAR_EXACT(texasr));
> +
> +	passed = 0;
> +	return NULL;
> +
> +	/* HTM aborted but VMX0 is correct */
> +value_match:
> +//	printf("!");
> +	return NULL;
> +
> +success:
> +//	printf(".");
> +	return NULL;
> +}
> +
> +int tm_vmx_unavail_test()
> +{
> +	int threads;
> +	pthread_t *thread;
> +
> +	SKIP_IF(!have_htm());
> +
> +	passed = 1;
> +
> +	threads = sysconf(_SC_NPROCESSORS_ONLN) * 4;
> +	thread = malloc(sizeof(pthread_t)*threads);
> +	if (!thread)
> +		return EXIT_FAILURE;
> +
> +	for (uint64_t i = 0; i < threads; i++)
> +		pthread_create(&thread[i], NULL, &worker, NULL);
> +
> +	for (uint64_t i = 0; i < threads; i++)
> +		pthread_join(thread[i], NULL);
> +
> +	free(thread);
> +
> +	return passed ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +
> +int main(int argc, char **argv)
> +{
> +	return test_harness(tm_vmx_unavail_test, "tm_vmx_unavail_test");
> +}

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

* Re: [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption
  2017-05-08  7:16 [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption Michael Neuling
  2017-05-08  7:16 ` [PATCH 2/2] selftests/powerpc: Test TM and VMX register state Michael Neuling
@ 2017-05-12 15:26 ` Cyril Bur
  2017-05-15  9:23   ` Michael Ellerman
  2017-05-17  9:34 ` [1/2] " Michael Ellerman
  2 siblings, 1 reply; 8+ messages in thread
From: Cyril Bur @ 2017-05-12 15:26 UTC (permalink / raw)
  To: Michael Neuling, mpe; +Cc: linuxppc-dev, Breno Leitao, Gustavo Bueno Romero

On Mon, 2017-05-08 at 17:16 +1000, Michael Neuling wrote:
> In this commit:
>   commit dc3106690b20305c3df06b42456fe386dd632ac9
>   Author: Cyril Bur <cyrilbur@gmail.com>
>   powerpc: tm: Always use fp_state and vr_state to store live registers
> 
> A section of code was removed that copied the current state to
> checkpointed state.  This should not have been removed.
> 

-[space]

> When an FP unavailable is taken inside a transaction, we need to abort
> the transaction. This is because at the time of the tbegin, the FP
> state is bogus so the state stored in the checkpointed registers is
> incorrect. To fix this, we treclaim (to get the checkpointed GPRs) and
> then copy the thread_struct FP live state into the checkpointed
> state. We then trecheckpoint so that the FP state is correctly
> restored into the CPU.
> 
> The coping of the FP registers from live to checkpointed is what was
> missing.
> 
> This simplifies the logic slightly from the original patch.
> tm_reclaim_thread() will now always write the checkpointed FP
> state. Either the checkpointed FP statte will be written as part of

state

> the actual treclaim (in tm.S), or it'll be a copy of the live
> state. Which one we use is based on MSR[FP] from userspace.
> 
> Similarly for VMX.
> 
> CC: <stable@vger.kernel.org> # 4.9+
> Signed-off-by: Michael Neuling <mikey@neuling.org>

Reviewed-by: cyrilbur@gmail.com

> ---
>  arch/powerpc/kernel/process.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index d645da302b..6305353237 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -864,6 +864,25 @@ static void tm_reclaim_thread(struct thread_struct *thr,
>  	if (!MSR_TM_SUSPENDED(mfmsr()))
>  		return;
>  
> +	/*
> +	 * If we are in a transaction and FP is off then we can't have
> +	 * used FP inside that transaction. Hence the checkpointed
> +	 * state is the same as the live state. We need to copy the
> +	 * live state to the checkpointed state so that when the
> +	 * transaction is restored, the checkpointed state is correct
> +	 * and the aborted transaction sees the correct state.  We use

-[space]

> +	 * ckpt_regs.msr here as that's what tm_reclaim will use to
> +	 * determine if it's going to write the checkpointed state or
> +	 * not. So either this will write the checkpointed registers,
> +	 * or reclaim will.  Similarly for VMX.

-[space]

> +	 */
> +	if ((thr->ckpt_regs.msr & MSR_FP) == 0)
> +		memcpy(&thr->ckfp_state, &thr->fp_state,
> +		       sizeof(struct thread_fp_state));
> +	if ((thr->ckpt_regs.msr & MSR_VEC) == 0)
> +		memcpy(&thr->ckvr_state, &thr->vr_state,
> +		       sizeof(struct thread_vr_state));
> +
>  	giveup_all(container_of(thr, struct task_struct, thread));
>  
>  	tm_reclaim(thr, thr->ckpt_regs.msr, cause);

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

* Re: [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption
  2017-05-12 15:26 ` [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption Cyril Bur
@ 2017-05-15  9:23   ` Michael Ellerman
  0 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2017-05-15  9:23 UTC (permalink / raw)
  To: Cyril Bur, Michael Neuling
  Cc: linuxppc-dev, Breno Leitao, Gustavo Bueno Romero

Cyril Bur <cyrilbur@gmail.com> writes:

> On Mon, 2017-05-08 at 17:16 +1000, Michael Neuling wrote:
>> In this commit:
>>   commit dc3106690b20305c3df06b42456fe386dd632ac9
>>   Author: Cyril Bur <cyrilbur@gmail.com>
>>   powerpc: tm: Always use fp_state and vr_state to store live registers
>> 
>> A section of code was removed that copied the current state to
>> checkpointed state.  This should not have been removed.
>> 
>
> -[space]
>
>> When an FP unavailable is taken inside a transaction, we need to abort
>> the transaction. This is because at the time of the tbegin, the FP
>> state is bogus so the state stored in the checkpointed registers is
>> incorrect. To fix this, we treclaim (to get the checkpointed GPRs) and
>> then copy the thread_struct FP live state into the checkpointed
>> state. We then trecheckpoint so that the FP state is correctly
>> restored into the CPU.
>> 
>> The coping of the FP registers from live to checkpointed is what was
          ^
Missed the actual spelling error.

8D

cheers

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

* Re: [PATCH 2/2] selftests/powerpc: Test TM and VMX register state
  2017-05-08  7:16 ` [PATCH 2/2] selftests/powerpc: Test TM and VMX register state Michael Neuling
  2017-05-12 15:16   ` Cyril Bur
@ 2017-05-15  9:31   ` Michael Ellerman
  2017-05-17  9:34   ` [2/2] " Michael Ellerman
  2 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2017-05-15  9:31 UTC (permalink / raw)
  To: Michael Neuling
  Cc: linuxppc-dev, mikey, cyrilbur, Breno Leitao, Gustavo Bueno Romero

Michael Neuling <mikey@neuling.org> writes:

> diff --git a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c b/tools/=
testing/selftests/powerpc/tm/tm-vmx-unavail.c
> new file mode 100644
> index 0000000000..137185ba49
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> @@ -0,0 +1,118 @@
> +/*
> + * Copyright 2017, Michael Neuling, IBM Corp.
> + * Licensed under GPLv2.
> + * Original: Breno Leitao <brenohl@br.ibm.com> &
> + *           Gustavo Bueno Romero <gromero@br.ibm.com>
> + * Edited: Michael Neuling
> + *
> + * Force VMX unavailable during a transaction and see if it corrupts
> + * the checkpointed VMX register state after the abort.
> + */
> +
> +#include <inttypes.h>
> +#include <htmintrin.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <pthread.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +
> +#include "tm.h"
> +#include "utils.h"
> +
> +int passed;
> +
> +void *worker(void *unused)
> +{
> +	__int128 vmx0;
> +	uint64_t texasr;

On BE:

  ppc64-gcc -std=3Dgnu99 -O2 -Wall -Werror -DGIT_VERSION=3D'"v4.12-rc1-3-ge=
36b67a25585"' -I/home/michael/work/topics/powerpc-maint/src/maint/tools/tes=
ting/selftests/powerpc/include  -mhtm -pthread    tm-vmx-unavail.c ../harne=
ss.c ../utils.c  -o /home/michael/work/topics/powerpc-maint/src/maint/tools=
/testing/selftests/powerpc/tm/tm-vmx-unavail
  tm-vmx-unavail.c: In function =E2=80=98worker=E2=80=99:
  tm-vmx-unavail.c:29:2: error: expected expression before =E2=80=98__int12=
8=E2=80=99
    __int128 vmx0;
    ^
  tm-vmx-unavail.c:62:21: error: =E2=80=98vmx0=E2=80=99 undeclared (first u=
se in this function)
     : [vmx0_ptr] "r"(&vmx0)
                       ^
  tm-vmx-unavail.c:62:21: note: each undeclared identifier is reported only=
 once for each function it appears in
  tm-vmx-unavail.c:72:9: error: format =E2=80=98%lx=E2=80=99 expects argume=
nt of type =E2=80=98long unsigned int=E2=80=99, but argument 2 has type =E2=
=80=98uint64_t {aka long long unsigned int}=E2=80=99 [-Werror=3Dformat=3D]
    printf("Failure with error: %lx\n",   _TEXASR_FAILURE_CODE(texasr));
           ^
  tm-vmx-unavail.c:73:9: error: format =E2=80=98%lx=E2=80=99 expects argume=
nt of type =E2=80=98long unsigned int=E2=80=99, but argument 2 has type =E2=
=80=98uint64_t {aka long long unsigned int}=E2=80=99 [-Werror=3Dformat=3D]
    printf("Summary error     : %lx\n",   _TEXASR_FAILURE_SUMMARY(texasr));
           ^
  tm-vmx-unavail.c:74:9: error: format =E2=80=98%lx=E2=80=99 expects argume=
nt of type =E2=80=98long unsigned int=E2=80=99, but argument 2 has type =E2=
=80=98uint64_t {aka long long unsigned int}=E2=80=99 [-Werror=3Dformat=3D]
    printf("TFIAR exact       : %lx\n\n", _TEXASR_TFIAR_EXACT(texasr));
           ^
  cc1: all warnings being treated as errors


I tried -maltivec/vsx but that didn't fix it.

cheers

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

* Re: [1/2] powerpc/tm: Fix FP and VMX register corruption
  2017-05-08  7:16 [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption Michael Neuling
  2017-05-08  7:16 ` [PATCH 2/2] selftests/powerpc: Test TM and VMX register state Michael Neuling
  2017-05-12 15:26 ` [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption Cyril Bur
@ 2017-05-17  9:34 ` Michael Ellerman
  2 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2017-05-17  9:34 UTC (permalink / raw)
  To: Michael Neuling
  Cc: Gustavo Bueno Romero, mikey, linuxppc-dev, cyrilbur, Breno Leitao

On Mon, 2017-05-08 at 07:16:26 UTC, Michael Neuling wrote:
> In this commit:
>   commit dc3106690b20305c3df06b42456fe386dd632ac9
>   Author: Cyril Bur <cyrilbur@gmail.com>
>   powerpc: tm: Always use fp_state and vr_state to store live registers
> 
> A section of code was removed that copied the current state to
> checkpointed state.  This should not have been removed.
> 
> When an FP unavailable is taken inside a transaction, we need to abort
> the transaction. This is because at the time of the tbegin, the FP
> state is bogus so the state stored in the checkpointed registers is
> incorrect. To fix this, we treclaim (to get the checkpointed GPRs) and
> then copy the thread_struct FP live state into the checkpointed
> state. We then trecheckpoint so that the FP state is correctly
> restored into the CPU.
> 
> The coping of the FP registers from live to checkpointed is what was
> missing.
> 
> This simplifies the logic slightly from the original patch.
> tm_reclaim_thread() will now always write the checkpointed FP
> state. Either the checkpointed FP statte will be written as part of
> the actual treclaim (in tm.S), or it'll be a copy of the live
> state. Which one we use is based on MSR[FP] from userspace.
> 
> Similarly for VMX.
> 
> CC: <stable@vger.kernel.org> # 4.9+
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Reviewed-by: cyrilbur@gmail.com

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/f48e91e87e67b56bef63393d1a02c6

cheers

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

* Re: [2/2] selftests/powerpc: Test TM and VMX register state
  2017-05-08  7:16 ` [PATCH 2/2] selftests/powerpc: Test TM and VMX register state Michael Neuling
  2017-05-12 15:16   ` Cyril Bur
  2017-05-15  9:31   ` Michael Ellerman
@ 2017-05-17  9:34   ` Michael Ellerman
  2 siblings, 0 replies; 8+ messages in thread
From: Michael Ellerman @ 2017-05-17  9:34 UTC (permalink / raw)
  To: Michael Neuling
  Cc: Gustavo Bueno Romero, mikey, linuxppc-dev, cyrilbur, Breno Leitao

On Mon, 2017-05-08 at 07:16:27 UTC, Michael Neuling wrote:
> Test that the VMX checkpointed register state is maintained when a VMX
> unavailable exception is taken during a transaction.
> 
> Thanks to Breno Leitao <brenohl@br.ibm.com> and
> Gustavo Bueno Romero <gromero@br.ibm.com> for the original test this
> is based heavily on.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Reviewed-by: Cyril Bur <cyrilbur@gmail.com>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/cde97f8492ac4425c4d0647a308e15

cheers

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

end of thread, other threads:[~2017-05-17  9:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-08  7:16 [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption Michael Neuling
2017-05-08  7:16 ` [PATCH 2/2] selftests/powerpc: Test TM and VMX register state Michael Neuling
2017-05-12 15:16   ` Cyril Bur
2017-05-15  9:31   ` Michael Ellerman
2017-05-17  9:34   ` [2/2] " Michael Ellerman
2017-05-12 15:26 ` [PATCH 1/2] powerpc/tm: Fix FP and VMX register corruption Cyril Bur
2017-05-15  9:23   ` Michael Ellerman
2017-05-17  9:34 ` [1/2] " Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.