[PATCH V2 0/3] Add new PowerPC specific ELF core notes

[PATCH V2 0/3] Add new PowerPC specific ELF core notes

[Anshuman Khandual]

	This patch series adds five new ELF core note sections which can be
used with existing ptrace request PTRACE_GETREGSET/SETREGSET for accessing
various transactional memory and miscellaneous register sets on PowerPC
platform. Please find a test program exploiting these new ELF core note
types on a POWER8 system.

RFC: https://lkml.org/lkml/2014/4/1/292
V1:  https://lkml.org/lkml/2014/4/2/43

Changes in V2
=============
(1) Removed all the power specific ptrace requests corresponding to new NT_PPC_*
    elf core note types. Now all the register sets can be accessed from ptrace
    through PTRACE_GETREGSET/PTRACE_SETREGSET using the individual NT_PPC* core
    note type instead
(2) Fixed couple of attribute values for REGSET_TM_CGPR register set
(3) Renamed flush_tmreg_to_thread as flush_tmregs_to_thread
(4) Fixed 32 bit checkpointed GPR support
(5) Changed commit messages accordingly

Outstanding Issues
==================
(1) Running DSCR register value inside a transaction does not seem to be saved
    at thread.dscr when the process stops for ptrace examination.
    
Test programs
=============
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <malloc.h>
#include <errno.h>
#include <sys/ptrace.h>
#include <sys/uio.h>
#include <sys/types.h>
#include <sys/signal.h>
#include <sys/user.h>
#include <linux/elf.h>
#include <linux/types.h>
#include <linux/ptrace.h>

typedef long long u64;
typedef unsigned int u32;
typedef __vector128 vector128;

/* TM CFPR */
struct tm_cfpr {
	u64	fpr[32];
	u64	fpscr;
};

/* TM CVMX */
struct tm_cvmx {
        vector128	vr[32] __attribute__((aligned(16)));
        vector128	vscr __attribute__((aligned(16)));
	u32		vrsave;	
};

/* TM SPR */
struct tm_spr_regs {
	u64	tm_tfhar;
	u64	tm_texasr;
	u64	tm_tfiar;
	u64	tm_orig_msr;
	u64	tm_tar;
	u64	tm_ppr;
	u64	tm_dscr;
};

/* Miscellaneous registers */
struct misc_regs {
	u64	dscr;
	u64	ppr;
	u64	tar;
};

/* TM instructions */
#define TBEGIN          ".long 0x7C00051D ;"
#define TEND            ".long 0x7C00055D ;"

/* SPR number */
#define SPRN_DSCR	0x3
#define SPRN_TAR	815

/* ELF core notes */
#define NT_PPC_TM_SPR  0x103           /* PowerPC transactional memory special registers */
#define NT_PPC_TM_CGPR 0x104           /* PowerpC transactional memory checkpointed GPR */
#define NT_PPC_TM_CFPR 0x105           /* PowerPC transactional memory checkpointed FPR */
#define NT_PPC_TM_CVMX 0x106           /* PowerPC transactional memory checkpointed VMX */
#define NT_PPC_MISC    0x107           /* PowerPC miscellaneous registers */

#define VAL1 1
#define VAL2 2
#define VAL3 3
#define VAL4 4

int main(int argc, char *argv[])
{
	struct tm_spr_regs *tmr1;
	struct pt_regs *pregs1, *pregs2;
	struct tm_cfpr *fpr, *fpr1;
	struct misc_regs *dbr1;
	struct iovec iov;

	pid_t child;
	int ret = 0, status = 0, i = 0, flag = 1;

	pregs2 = (struct pt_regs *) malloc(sizeof(struct pt_regs));
	fpr = (struct tm_cfpr *) malloc(sizeof(struct tm_cfpr));

	child = fork();
	if (child < 0) {
		printf("fork() failed \n");
		exit(-1);
	}

	/* Child code */
	if (child == 0) {
		asm __volatile__(
			"6: ;"			/* TM checkpointed values */
			"li 1, %[val1];"	/* GPR[1] */
			".long 0x7C210166;"	/* FPR[1] */
			"li 2, %[val2];"	/* GPR[2] */
			".long 0x7C420166;"	/* FPR[2] */
			"mtspr %[tar], 1;"	/* TAR */
 			"mtspr %[dscr], 2;"	/* DSCR */
			"1: ;"
			TBEGIN			/* TM running values */
			"beq 2f ;"
		 	"li 1, %[val3];"	/* GPR[1] */
                        ".long 0x7C210166;"	/* FPR[1] */
                        "li 2, %[val4];"	/* GPR[2] */
                        ".long 0x7C420166;"	/* FPR[2] */
                        "mtspr %[tar], 1;"	/* TAR */
			"mtspr %[dscr], 2;"	/* DSCR */
			"b .;"
			TEND
			"2: ;"			/* Abort handler */
			"b 1b;"			/* Start from TBEGIN */

			"3: ;"
			"b 6b;"			/* Start all over again */
			:: [dscr]"i"(SPRN_DSCR), [tar]"i"(SPRN_TAR), [val1]"i"(VAL1), [val2]"i"(VAL2), [val3]"i"(VAL3), [val4]"i"(VAL4)
			: "memory", "r7");
	}

	/* Parent */
	if (child) {
		do {
			memset(pregs2, 0 , sizeof(struct pt_regs));
			memset(fpr, 0 , sizeof(struct tm_cfpr));

			/* Wait till child hits "b ." instruction */
			sleep(3);

			/* Attach tracee */
			ret = ptrace(PTRACE_ATTACH, child, NULL, NULL);
			if (ret == -1) {
				printf("PTRACE_ATTACH failed: %s\n", strerror(errno));
				exit(-1);
			}

			ret = waitpid(child, NULL, 0);
			if (ret != child) {
				printf("PID does not match\n");
				exit(-1);
			}

			/* TM specific SPR */
			iov.iov_base = (struct tm_spr_regs *) malloc(sizeof(struct tm_spr_regs));
			iov.iov_len = sizeof(struct tm_spr_regs);
			ret = ptrace(PTRACE_GETREGSET, child, NT_PPC_TM_SPR, &iov);
			if (ret == -1) {
				printf("PTRACE_GETREGSET: NT_PPC_TM_SPR failed %s\n", strerror(errno));
				exit(-1);
			}

			if (iov.iov_len != sizeof(struct tm_spr_regs)) {
				printf("NT_PPC_TM_SPR: Length returned is wrong\n");
				exit(-1);
			}

			tmr1 = iov.iov_base;
			printf("-------TM specific SPR------\n");
			printf("TM TFHAR: %llx\n", tmr1->tm_tfhar);
			printf("TM TEXASR: %llx\n", tmr1->tm_texasr);
			printf("TM TFIAR: %llx\n", tmr1->tm_tfiar);
			printf("TM CH ORIG_MSR: %llx\n", tmr1->tm_orig_msr);
			printf("TM CH TAR: %llx\n", tmr1->tm_tar);
			printf("TM CH PPR: %llx\n", tmr1->tm_ppr);
			printf("TM CH DSCR: %llx\n", tmr1->tm_dscr);

			if (tmr1->tm_tar == VAL1)
				printf("TAR PASSED\n");
			else
				printf("TAR FAILED\n");

			if (tmr1->tm_dscr == VAL2)
				printf("DSCR PASSED\n");
			else
				printf("DSCR FAILED\n");

			/* TM checkpointed GPR */
			iov.iov_base = (struct pt_regs *) malloc(sizeof(struct pt_regs));;
			iov.iov_len = sizeof(struct pt_regs);
			ret = ptrace(PTRACE_GETREGSET, child, NT_PPC_TM_CGPR, &iov);
			if (ret == -1) {
				printf("PTRACE_GETREGSET: NT_PPC_TM_CGPR failed: %s\n", strerror(errno));
				exit(-1);
			}

			if (iov.iov_len != sizeof(struct pt_regs)) {
				printf("NT_PPC_TM_CGPR: Length returned is wrong\n");
				exit(-1);
			}

			pregs1 = iov.iov_base;
			printf("-------TM checkpointed GPR-----\n");
			printf("TM CH GPR[1]: %x\n", pregs1->gpr[1]);
			printf("TM CH GPR[2]: %x\n", pregs1->gpr[2]);
			printf("TM CH NIP: %x\n", pregs1->nip);
			printf("TM CH LINK: %x\n", pregs1->link);
			printf("TM CH CCR: %x\n", pregs1->ccr);

			if (pregs1->gpr[1] == VAL1)
				printf("GPR[1] PASSED\n");
			else
				printf("GPR[1] FAILED\n");

			if (pregs1->gpr[2] == VAL2)
				printf("GPR[2] PASSED\n");
			else
				printf("GPR[2] FAILED\n");

			/* TM running GPR */
			ret = ptrace(PTRACE_GETREGS, child, NULL, pregs2);
			if (ret == -1) {
				printf("PTRACE_GETREGS fail: %s\n", strerror(errno));
				exit(-1);
			}

			printf("-------TM running GPR-----\n");
			printf("TM RN GPR[1]: %x\n", pregs2->gpr[1]);
			printf("TM RN GPR[2]: %x\n", pregs2->gpr[2]);
			printf("TM RN NIP: %x\n", pregs2->nip);
			printf("TM RN LINK: %x\n", pregs2->link);
			printf("TM RN CCR: %x\n", pregs2->ccr);

			
			if (pregs2->gpr[1] == VAL3)
				printf("GPR[1] PASSED\n");
			else
				printf("GPR[1] FAILED\n");

			if (pregs2->gpr[2] == VAL4)
				printf("GPR[2] PASSED\n");
			else
				printf("GPR[2] FAILED\n");

			/* TM checkpointed FPR */
			iov.iov_base = (struct tm_cfpr *) malloc(sizeof(struct tm_cfpr));;
			iov.iov_len = sizeof(struct tm_cfpr);
			ret = ptrace(PTRACE_GETREGSET, child, NT_PPC_TM_CFPR, &iov);
			if (ret == -1) {
				printf("PTRACE_GETREGSET: NT_PPC_TM_CFPR: Failed: %s\n", strerror(errno));
				exit(-1);
			}

			if (iov.iov_len != sizeof(struct tm_cfpr)) {
				printf("NT_PPC_TM_CFPR: Length returned is wrong\n");
				exit(-1);
			}

			fpr1 = iov.iov_base;
			printf("-------TM checkpointed FPR-----\n");
			printf("TM CH FPR[1]: %llx\n", fpr1->fpr[1]);
			printf("TM CH FPR[2]: %llx\n", fpr1->fpr[2]);
			printf("TM CH FPSCR: %llx\n", fpr1->fpscr);

			if (fpr1->fpr[1] == VAL1)
				printf("FPR[1] PASSED\n");
			else
				printf("FPR[1] FAILED\n");

			if (fpr1->fpr[2] == VAL2)
				printf("FPR[2] PASSED\n");
			else
				printf("FPR[2] FAILED\n");

			/* TM running FPR */
			ret = ptrace(PTRACE_GETFPREGS, child, NULL, fpr);
			if (ret == -1) {
				printf("PTRACE_GETFPREGS failed: %s\n", strerror(errno));
				exit(-1);
			}

			printf("-------TM running FPR-----\n");
			printf("TM RN FPR[1]: %llx\n", fpr->fpr[1]);
			printf("TM RN FPR[2]: %llx\n", fpr->fpr[2]);
			printf("TM RN FPSCR: %llx\n", fpr->fpscr);

			if (fpr->fpr[1] == VAL3)
				printf("FPR[1] PASSED\n");
			else
				printf("FPR[1] FAILED\n");

			if (fpr->fpr[2] == VAL4)
				printf("FPR[2] PASSED\n");
			else
				printf("FPR[2] FAILED\n");

			/* Misc registers */
			iov.iov_base = (struct misc_regs *) malloc(sizeof(struct misc_regs));
			iov.iov_len = sizeof(struct misc_regs);
			ret = ptrace(PTRACE_GETREGSET, child, NT_PPC_MISC, &iov);
			if (ret == -1) {
				printf("PTRACE_GETREGSET: NT_PPC_MISC: Failed: %s\n", strerror(errno));
				exit(-1);
			}

			if (iov.iov_len != sizeof(struct misc_regs)) {
				printf("NT_PPC_TM_MISC: Length returned is wrong\n");
				exit(-1);
			}

			dbr1  = iov.iov_base;
			printf("-------Running miscellaneous registers-------\n");
			printf("TM RN DSCR: %llx\n", dbr1->dscr);
			printf("TM RN PPR: %llx\n", dbr1->ppr);
			printf("TM RN TAR: %llx\n", dbr1->tar);

			if (dbr1->tar == VAL3)
				printf("TAR PASSED\n");
			else
				printf("TAR FAILED\n");

			if (dbr1->dscr == VAL4)
				printf("DSCR PASSED\n");
			else
				printf("DSCR FAILED\n");

			/* Detach tracee */
			ret = ptrace(PTRACE_DETACH, child, NULL, NULL);
			if (ret == -1) {
				printf("PTRACE_DETACH failed: %s\n", strerror(errno));
				exit(-1);
			}
		} while (0);
	}
	return 0;
}

Test Results
============
(1) 64 bit application ==>

-------TM specific SPR------
TM TFHAR: 10000960
TM TEXASR: de000001ac000001
TM TFIAR: c00000000003f9a6
TM CH ORIG_MSR: 900000050000f032
TM CH TAR: 1
TM CH PPR: c000000000000
TM CH DSCR: 2
TAR PASSED
DSCR PASSED
-------TM checkpointed GPR-----
TM CH GPR[1]: 1
TM CH GPR[2]: 2
TM CH NIP: 10000960
TM CH LINK: 10000904
TM CH CCR: 22000022
GPR[1] PASSED
GPR[2] PASSED
-------TM running GPR-----
TM RN GPR[1]: 3
TM RN GPR[2]: 4
TM RN NIP: 1000097c
TM RN LINK: 10000904
TM RN CCR: 2000022
GPR[1] PASSED
GPR[2] PASSED
-------TM checkpointed FPR-----
TM CH FPR[1]: 1
TM CH FPR[2]: 2
TM CH FPSCR: 0
FPR[1] PASSED
FPR[2] PASSED
-------TM running FPR-----
TM RN FPR[1]: 3
TM RN FPR[2]: 4
TM RN FPSCR: 0
FPR[1] PASSED
FPR[2] PASSED
-------Running miscellaneous registers-------
TM RN DSCR: 0
TM RN PPR: c000000000000
TM RN TAR: 3
TAR PASSED
DSCR FAILED

(2) 32 bit application ==>

-------TM specific SPR------
TM TFHAR: 100006b8
TM TEXASR: de000001ac000001
TM TFIAR: c00000000003f9a6
TM CH ORIG_MSR: 100000050000f032
TM CH TAR: 1
TM CH PPR: c000000000000
TM CH DSCR: 2
TAR PASSED
DSCR PASSED
-------TM checkpointed GPR-----
TM CH GPR[1]: 1
TM CH GPR[2]: 2
TM CH NIP: 100006b8
TM CH LINK: 1000066c
TM CH CCR: 22000022
GPR[1] PASSED
GPR[2] PASSED
-------TM running GPR-----
TM RN GPR[1]: 3
TM RN GPR[2]: 4
TM RN NIP: 100006d4
TM RN LINK: 1000066c
TM RN CCR: 2000022
GPR[1] PASSED
GPR[2] PASSED
TM CH FPR[2]: 2
TM CH FPSCR: 0
FPR[1] PASSED
FPR[2] PASSED
-------TM running FPR-----
TM RN FPR[1]: 3
TM RN FPR[2]: 4
TM RN FPSCR: 0
FPR[1] PASSED
FPR[2] PASSED
-------Running miscellaneous registers-------
TM RN DSCR: 0
TM RN PPR: c000000000000
TM RN TAR: 3
TAR PASSED
DSCR FAILED

Anshuman Khandual (3):
  elf: Add some new PowerPC specifc note sections
  powerpc, ptrace: Enable support for transactional memory register sets
  powerpc, ptrace: Enable support for miscellaneous registers

 arch/powerpc/include/asm/switch_to.h |   8 +
 arch/powerpc/kernel/process.c        |  24 ++
 arch/powerpc/kernel/ptrace.c         | 764 +++++++++++++++++++++++++++++++++--
 include/uapi/linux/elf.h             |   5 +
 4 files changed, 773 insertions(+), 28 deletions(-)

-- 
1.7.11.7

[PATCH V2 1/3] elf: Add some new PowerPC specifc note sections

[Anshuman Khandual]

This patch adds four new note sections for transactional memory
and one note section for some miscellaneous registers. This addition
of new elf note sections extends the existing elf ABI without affecting
it in any manner.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
 include/uapi/linux/elf.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index ef6103b..4040124 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -379,6 +379,11 @@ typedef struct elf64_shdr {
 #define NT_PPC_VMX	0x100		/* PowerPC Altivec/VMX registers */
 #define NT_PPC_SPE	0x101		/* PowerPC SPE/EVR registers */
 #define NT_PPC_VSX	0x102		/* PowerPC VSX registers */
+#define NT_PPC_TM_SPR	0x103		/* PowerPC TM special registers */
+#define NT_PPC_TM_CGPR	0x104		/* PowerpC TM checkpointed GPR */
+#define NT_PPC_TM_CFPR	0x105		/* PowerPC TM checkpointed FPR */
+#define NT_PPC_TM_CVMX	0x106		/* PowerPC TM checkpointed VMX */
+#define NT_PPC_MISC	0x107		/* PowerPC miscellaneous registers */
 #define NT_386_TLS	0x200		/* i386 TLS slots (struct user_desc) */
 #define NT_386_IOPERM	0x201		/* x86 io permission bitmap (1=deny) */
 #define NT_X86_XSTATE	0x202		/* x86 extended state using xsave */
-- 
1.7.11.7

[PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Anshuman Khandual]

This patch enables get and set of transactional memory related register
sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
ELF core note types added previously in this regard.

	(1) NT_PPC_TM_SPR
	(2) NT_PPC_TM_CGPR
	(3) NT_PPC_TM_CFPR
	(4) NT_PPC_TM_CVMX

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/switch_to.h |   8 +
 arch/powerpc/kernel/process.c        |  24 ++
 arch/powerpc/kernel/ptrace.c         | 683 +++++++++++++++++++++++++++++++++--
 3 files changed, 687 insertions(+), 28 deletions(-)

diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index 0e83e7d..2737f46 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -80,6 +80,14 @@ static inline void flush_spe_to_thread(struct task_struct *t)
 }
 #endif
 
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+extern void flush_tmregs_to_thread(struct task_struct *);
+#else
+static inline void flush_tmregs_to_thread(struct task_struct *t)
+{
+}
+#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+
 static inline void clear_task_ebb(struct task_struct *t)
 {
 #ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 31d0215..e247898 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -695,6 +695,30 @@ static inline void __switch_to_tm(struct task_struct *prev)
 	}
 }
 
+void flush_tmregs_to_thread(struct task_struct *tsk)
+{
+	/*
+	 * If task is not current, it should have been flushed
+	 * already to it's thread_struct during __switch_to().
+	 */
+	if (tsk != current)
+		return;
+
+	preempt_disable();
+	if (tsk->thread.regs) {
+		/*
+		 * If we are still current, the TM state need to
+		 * be flushed to thread_struct as it will be still
+		 * present in the current cpu.
+		 */
+		if (MSR_TM_ACTIVE(tsk->thread.regs->msr)) {
+			__switch_to_tm(tsk);
+			tm_recheckpoint_new_task(tsk);
+		}
+	}
+	preempt_enable();
+}
+
 /*
  * This is called if we are on the way out to userspace and the
  * TIF_RESTORE_TM flag is set.  It checks if we need to reload
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 2e3d2bf..92faded 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -357,6 +357,17 @@ static int gpr_set(struct task_struct *target, const struct user_regset *regset,
 	return ret;
 }
 
+/*
+ * When any transaction is active, "thread_struct->transact_fp" holds
+ * the current running value of all FPR registers and "thread_struct->
+ * fp_state" holds the last checkpointed FPR registers state for the
+ * current transaction.
+ *
+ * struct data {
+ * 	u64	fpr[32];
+ * 	u64	fpscr;
+ * };
+ */
 static int fpr_get(struct task_struct *target, const struct user_regset *regset,
 		   unsigned int pos, unsigned int count,
 		   void *kbuf, void __user *ubuf)
@@ -365,21 +376,41 @@ static int fpr_get(struct task_struct *target, const struct user_regset *regset,
 	u64 buf[33];
 	int i;
 #endif
-	flush_fp_to_thread(target);
+	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+		flush_fp_to_thread(target);
+		flush_altivec_to_thread(target);
+		flush_tmregs_to_thread(target);
+	} else {
+		flush_fp_to_thread(target);
+	}
 
 #ifdef CONFIG_VSX
 	/* copy to local buffer then write that out */
-	for (i = 0; i < 32 ; i++)
-		buf[i] = target->thread.TS_FPR(i);
-	buf[32] = target->thread.fp_state.fpscr;
+	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+		for (i = 0; i < 32 ; i++)
+			buf[i] = target->thread.TS_TRANS_FPR(i);
+		buf[32] = target->thread.transact_fp.fpscr;
+	} else {
+		for (i = 0; i < 32 ; i++)
+			buf[i] = target->thread.TS_FPR(i);
+		buf[32] = target->thread.fp_state.fpscr;
+	}
 	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
 
 #else
-	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
-		     offsetof(struct thread_fp_state, fpr[32][0]));
+	if (MSR_TM_ACTIVE(tsk->thread.regs->msr)) {
+		BUILD_BUG_ON(offsetof(struct transact_fp, fpscr) !=
+				offsetof(struct transact_fp, fpr[32][0]));
 
-	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+		return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+				   &target->thread.transact_fp, 0, -1);
+	} esle {
+		BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+			     offsetof(struct thread_fp_state, fpr[32][0]));
+
+		return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
 				   &target->thread.fp_state, 0, -1);
+	}
 #endif
 }
 
@@ -391,23 +422,44 @@ static int fpr_set(struct task_struct *target, const struct user_regset *regset,
 	u64 buf[33];
 	int i;
 #endif
-	flush_fp_to_thread(target);
+	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+		flush_fp_to_thread(target);
+		flush_altivec_to_thread(target);
+		flush_tmregs_to_thread(target);
+	} else {
+		flush_fp_to_thread(target);
+	}
 
 #ifdef CONFIG_VSX
 	/* copy to local buffer then write that out */
 	i = user_regset_copyin(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
 	if (i)
 		return i;
-	for (i = 0; i < 32 ; i++)
-		target->thread.TS_FPR(i) = buf[i];
-	target->thread.fp_state.fpscr = buf[32];
+	for (i = 0; i < 32 ; i++) {
+		if (MSR_TM_ACTIVE(target->thread.regs->msr))
+			target->thread.TS_TRANS_FPR(i) = buf[i];
+		else
+			target->thread.TS_FPR(i) = buf[i];
+	}
+	if (MSR_TM_ACTIVE(target->thread.regs->msr))
+		target->thread.transact_fp.fpscr = buf[32];
+	else
+		target->thread.fp_state.fpscr = buf[32];
 	return 0;
 #else
-	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
-		     offsetof(struct thread_fp_state, fpr[32][0]));
+	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+		BUILD_BUG_ON(offsetof(struct transact_fp, fpscr) !=
+			     offsetof(struct transact_fp, fpr[32][0]));
 
-	return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
-				  &target->thread.fp_state, 0, -1);
+		return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+					  &target->thread.transact_fp, 0, -1);
+	} else {
+		BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+			     offsetof(struct thread_fp_state, fpr[32][0]));
+
+		return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+				&target->thread.fp_state, 0, -1);
+	}
 #endif
 }
 
@@ -432,20 +484,44 @@ static int vr_active(struct task_struct *target,
 	return target->thread.used_vr ? regset->n : 0;
 }
 
+/*
+ * When any transaction is active, "thread_struct->transact_vr" holds
+ * the current running value of all VMX registers and "thread_struct->
+ * vr_state" holds the last checkpointed value of VMX registers for the
+ * current transaction.
+ *
+ * struct data {
+ * 	vector128	vr[32];
+ * 	vector128	vscr;
+ * 	vector128	vrsave;
+ * };
+ */
 static int vr_get(struct task_struct *target, const struct user_regset *regset,
 		  unsigned int pos, unsigned int count,
 		  void *kbuf, void __user *ubuf)
 {
 	int ret;
+	struct thread_vr_state *addr;
 
-	flush_altivec_to_thread(target);
+	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+		flush_fp_to_thread(target);
+		flush_altivec_to_thread(target);
+		flush_tmregs_to_thread(target);
+	} else {
+		flush_altivec_to_thread(target);
+	}
 
 	BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
 		     offsetof(struct thread_vr_state, vr[32]));
 
+	if (MSR_TM_ACTIVE(target->thread.regs->msr))
+		addr = &target->thread.transact_vr;
+	else
+		addr = &target->thread.vr_state;
+
 	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
-				  &target->thread.vr_state, 0,
-				  33 * sizeof(vector128));
+				addr, 0, 33 * sizeof(vector128));
+
 	if (!ret) {
 		/*
 		 * Copy out only the low-order word of vrsave.
@@ -455,11 +531,14 @@ static int vr_get(struct task_struct *target, const struct user_regset *regset,
 			u32 word;
 		} vrsave;
 		memset(&vrsave, 0, sizeof(vrsave));
-		vrsave.word = target->thread.vrsave;
+		if (MSR_TM_ACTIVE(target->thread.regs->msr))
+			vrsave.word = target->thread.transact_vrsave;
+		else
+			vrsave.word = target->thread.vrsave;
+
 		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
 					  33 * sizeof(vector128), -1);
 	}
-
 	return ret;
 }
 
@@ -467,16 +546,27 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
 		  unsigned int pos, unsigned int count,
 		  const void *kbuf, const void __user *ubuf)
 {
+	struct thread_vr_state *addr;
 	int ret;
 
-	flush_altivec_to_thread(target);
+	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+		flush_fp_to_thread(target);
+		flush_altivec_to_thread(target);
+		flush_tmregs_to_thread(target);
+	} else {
+		flush_altivec_to_thread(target);
+	}
 
 	BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
 		     offsetof(struct thread_vr_state, vr[32]));
 
+	if (MSR_TM_ACTIVE(target->thread.regs->msr))
+		addr = &target->thread.transact_vr;
+	else
+		addr = &target->thread.vr_state;
 	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
-				 &target->thread.vr_state, 0,
-				 33 * sizeof(vector128));
+			addr, 0, 33 * sizeof(vector128));
+
 	if (!ret && count > 0) {
 		/*
 		 * We use only the first word of vrsave.
@@ -486,13 +576,21 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
 			u32 word;
 		} vrsave;
 		memset(&vrsave, 0, sizeof(vrsave));
-		vrsave.word = target->thread.vrsave;
+
+		if (MSR_TM_ACTIVE(target->thread.regs->msr))
+			vrsave.word = target->thread.transact_vrsave;
+		else
+			vrsave.word = target->thread.vrsave;
+
 		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
 					 33 * sizeof(vector128), -1);
-		if (!ret)
-			target->thread.vrsave = vrsave.word;
+		if (!ret) {
+			if (MSR_TM_ACTIVE(target->thread.regs->msr))
+				target->thread.transact_vrsave = vrsave.word;
+			else
+				target->thread.vrsave = vrsave.word;
+		}
 	}
-
 	return ret;
 }
 #endif /* CONFIG_ALTIVEC */
@@ -613,6 +711,347 @@ static int evr_set(struct task_struct *target, const struct user_regset *regset,
 }
 #endif /* CONFIG_SPE */
 
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+
+/*
+ *  Transactional memory SPR
+ *
+ * struct {
+ * 	u64		tm_tfhar;
+ *	u64		tm_texasr;
+ *	u64		tm_tfiar;
+ *	unsigned long	tm_orig_msr;
+ * 	unsigned long	tm_tar;
+ *	unsigned long	tm_ppr;
+ *	unsigned long	tm_dscr;
+ * };
+ */
+static int tm_spr_get(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   void *kbuf, void __user *ubuf)
+{
+	int ret;
+
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+
+	/* TFHAR register */
+	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_tfhar, 0, sizeof(u64));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfhar) +
+			sizeof(u64) != offsetof(struct thread_struct, tm_texasr));
+
+	/* TEXASR register */
+	if (!ret)
+		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_texasr, sizeof(u64), 2 * sizeof(u64));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_texasr) +
+			sizeof(u64) != offsetof(struct thread_struct, tm_tfiar));
+
+	/* TFIAR register */
+	if (!ret)
+		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_tfiar, 2 * sizeof(u64), 3 * sizeof(u64));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfiar) +
+			sizeof(u64) != offsetof(struct thread_struct, tm_orig_msr));
+
+	/* TM checkpointed original MSR */
+	if (!ret)
+		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_orig_msr, 3 * sizeof(u64),
+				3 * sizeof(u64) + sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_orig_msr) +
+			sizeof(unsigned long) + sizeof(struct pt_regs)
+				!= offsetof(struct thread_struct, tm_tar));
+
+	/* TM checkpointed TAR register */
+	if (!ret)
+		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_tar, 3 * sizeof(u64) +
+				sizeof(unsigned long) , 3 * sizeof(u64) +
+					2 * sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tar)
+			+ sizeof(unsigned long) !=
+				offsetof(struct thread_struct, tm_ppr));
+
+	/* TM checkpointed PPR register */
+	if (!ret)
+		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+				&target->thread.tm_ppr, 3 * sizeof(u64) +
+					2 * sizeof(unsigned long), 3 * sizeof(u64) +
+						3 * sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_ppr) +
+			sizeof(unsigned long) !=
+				offsetof(struct thread_struct, tm_dscr));
+
+	/* TM checkpointed DSCR register */
+	if (!ret)
+		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_dscr, 3 * sizeof(u64)
+				+ 3 * sizeof(unsigned long), 3 * sizeof(u64)
+						+ 4 * sizeof(unsigned long));
+	return ret;
+}
+
+static int tm_spr_set(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   const void *kbuf, const void __user *ubuf)
+{
+	int ret;
+
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+
+	/* TFHAR register */
+	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+				&target->thread.tm_tfhar, 0, sizeof(u64));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfhar)
+		+ sizeof(u64) != offsetof(struct thread_struct, tm_texasr));
+
+	/* TEXASR register */
+	if (!ret)
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_texasr, sizeof(u64), 2 * sizeof(u64));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_texasr)
+		+ sizeof(u64) != offsetof(struct thread_struct, tm_tfiar));
+
+	/* TFIAR register */
+	if (!ret)
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_tfiar, 2 * sizeof(u64), 3 * sizeof(u64));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfiar)
+		+ sizeof(u64) != offsetof(struct thread_struct, tm_orig_msr));
+
+	/* TM checkpointed orig MSR */
+	if (!ret)
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_orig_msr, 3 * sizeof(u64),
+				3 * sizeof(u64) + sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_orig_msr)
+		+ sizeof(unsigned long) + sizeof(struct pt_regs) !=
+			offsetof(struct thread_struct, tm_tar));
+
+	/* TM checkpointed TAR register */
+	if (!ret)
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+			&target->thread.tm_tar, 3 * sizeof(u64) +
+				sizeof(unsigned long), 3 * sizeof(u64) +
+					2 * sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tar)
+			+ sizeof(unsigned long) != offsetof(struct thread_struct, tm_ppr));
+
+	/* TM checkpointed PPR register */
+	if (!ret)
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+				&target->thread.tm_ppr, 3 * sizeof(u64)
+					+ 2 * sizeof(unsigned long), 3 * sizeof(u64)
+					+ 3 * sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, tm_ppr) +
+			sizeof(unsigned long) !=
+				offsetof(struct thread_struct, tm_dscr));
+
+	/* TM checkpointed DSCR register */
+	if (!ret)
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+				&target->thread.tm_dscr,
+					3 * sizeof(u64) + 3 * sizeof(unsigned long),
+					3 * sizeof(u64) + 4 * sizeof(unsigned long));
+
+	return ret;
+}
+
+/*
+ * TM Checkpointed GPR
+ *
+ * struct data {
+ * 	struct pt_regs ckpt_regs;
+ * };
+ */
+static int tm_cgpr_get(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   void *kbuf, void __user *ubuf)
+{
+	int ret;
+
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+			&target->thread.ckpt_regs, 0,
+				sizeof(struct pt_regs));
+	return ret;
+}
+
+static int tm_cgpr_set(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   const void *kbuf, const void __user *ubuf)
+{
+	int ret;
+
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+					&target->thread.ckpt_regs, 0,
+						sizeof(struct pt_regs));
+	return ret;
+}
+
+/*
+ * TM Checkpointed FPR
+ *
+ * struct data {
+ * 	u64	fpr[32];
+ * 	u64	fpscr;
+ * };
+ */
+static int tm_cfpr_get(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   void *kbuf, void __user *ubuf)
+{
+#ifdef CONFIG_VSX
+	u64 buf[33];
+	int i;
+#endif
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+
+#ifdef CONFIG_VSX
+	/* copy to local buffer then write that out */
+	for (i = 0; i < 32 ; i++)
+		buf[i] = target->thread.TS_FPR(i);
+	buf[32] = target->thread.fp_state.fpscr;
+	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
+
+#else
+	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+		offsetof(struct thread_fp_state, fpr[32][0]));
+
+	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+			&target->thread.thread_fp_state, 0, -1);
+#endif
+}
+
+static int tm_cfpr_set(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   const void *kbuf, const void __user *ubuf)
+{
+#ifdef CONFIG_VSX
+	u64 buf[33];
+	int i;
+#endif
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+
+#ifdef CONFIG_VSX
+	/* copy to local buffer then write that out */
+	i = user_regset_copyin(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
+	if (i)
+		return i;
+	for (i = 0; i < 32 ; i++)
+		target->thread.TS_FPR(i) = buf[i];
+	target->thread.fp_state.fpscr = buf[32];
+	return 0;
+#else
+	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+		      offsetof(struct thread_fp_state, fpr[32][0]));
+
+	return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+				&target->thread.fp_state, 0, -1);
+#endif
+}
+
+/*
+ * TM Checkpointed VMX
+ *
+ * struct data {
+ * 	vector128	vr[32];
+ * 	vector128	vscr;
+ * 	vector128	vrsave;
+ *};
+ */
+static int tm_cvmx_get(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   void *kbuf, void __user *ubuf)
+{
+	int ret;
+
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+
+	BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
+		     offsetof(struct thread_vr_state, vr[32]));
+
+	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+				  &target->thread.vr_state, 0,
+				  33 * sizeof(vector128));
+	if (!ret) {
+		/*
+		 * Copy out only the low-order word of vrsave.
+		 */
+		union {
+			elf_vrreg_t reg;
+			u32 word;
+		} vrsave;
+		memset(&vrsave, 0, sizeof(vrsave));
+		vrsave.word = target->thread.vrsave;
+		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
+					  33 * sizeof(vector128), -1);
+	}
+	return ret;
+}
+
+static int tm_cvmx_set(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   const void *kbuf, const void __user *ubuf)
+{
+	int ret;
+
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+
+	BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
+		offsetof(struct thread_vr_state, vr[32]));
+
+	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+				 &target->thread.vr_state, 0,
+				 33 * sizeof(vector128));
+	if (!ret && count > 0) {
+		/*
+		 * We use only the first word of vrsave.
+		 */
+		union {
+			elf_vrreg_t reg;
+			u32 word;
+		} vrsave;
+		memset(&vrsave, 0, sizeof(vrsave));
+		vrsave.word = target->thread.vrsave;
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
+					 33 * sizeof(vector128), -1);
+		if (!ret)
+			target->thread.vrsave = vrsave.word;
+	}
+	return ret;
+}
+#endif	/* CONFIG_PPC_TRANSACTIONAL_MEM */
 
 /*
  * These are our native regset flavors.
@@ -629,6 +1068,12 @@ enum powerpc_regset {
 #ifdef CONFIG_SPE
 	REGSET_SPE,
 #endif
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	REGSET_TM_SPR,		/* TM specific SPR */
+	REGSET_TM_CGPR,		/* TM checkpointed GPR */
+	REGSET_TM_CFPR,		/* TM checkpointed FPR */
+	REGSET_TM_CVMX,		/* TM checkpointed VMX */
+#endif
 };
 
 static const struct user_regset native_regsets[] = {
@@ -663,6 +1108,28 @@ static const struct user_regset native_regsets[] = {
 		.active = evr_active, .get = evr_get, .set = evr_set
 	},
 #endif
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	[REGSET_TM_SPR] = {
+		.core_note_type = NT_PPC_TM_SPR, .n = 7,
+		.size = sizeof(u64), .align = sizeof(u64),
+		.get = tm_spr_get, .set = tm_spr_set
+	},
+	[REGSET_TM_CGPR] = {
+		.core_note_type = NT_PPC_TM_CGPR, .n = ELF_NGREG,
+		.size = sizeof(long), .align = sizeof(long),
+		.get = tm_cgpr_get, .set = tm_cgpr_set
+	},
+	[REGSET_TM_CFPR] = {
+		.core_note_type = NT_PPC_TM_CFPR, .n = ELF_NFPREG,
+		.size = sizeof(double), .align = sizeof(double),
+		.get = tm_cfpr_get, .set = tm_cfpr_set
+	},
+	[REGSET_TM_CVMX] = {
+		.core_note_type = NT_PPC_TM_CVMX, .n = 34,
+		.size = sizeof(vector128), .align = sizeof(vector128),
+		.get = tm_cvmx_get, .set = tm_cvmx_set
+	},
+#endif
 };
 
 static const struct user_regset_view user_ppc_native_view = {
@@ -803,6 +1270,145 @@ static int gpr32_set(struct task_struct *target,
 					 (PT_TRAP + 1) * sizeof(reg), -1);
 }
 
+static int tm_cgpr32_get(struct task_struct *target,
+		     const struct user_regset *regset,
+		     unsigned int pos, unsigned int count,
+		     void *kbuf, void __user *ubuf)
+{
+	const unsigned long *regs = &target->thread.ckpt_regs.gpr[0];
+	compat_ulong_t *k = kbuf;
+	compat_ulong_t __user *u = ubuf;
+	compat_ulong_t reg;
+	int i;
+
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+
+	if (target->thread.regs == NULL)
+		return -EIO;
+
+	if (!FULL_REGS(target->thread.regs)) {
+		/* We have a partial register set.  Fill 14-31 with bogus values */
+		for (i = 14; i < 32; i++)
+			target->thread.regs->gpr[i] = NV_REG_POISON; 
+	}
+
+	pos /= sizeof(reg);
+	count /= sizeof(reg);
+
+	if (kbuf)
+		for (; count > 0 && pos < PT_MSR; --count)
+			*k++ = regs[pos++];
+	else
+		for (; count > 0 && pos < PT_MSR; --count)
+			if (__put_user((compat_ulong_t) regs[pos++], u++))
+				return -EFAULT;
+
+	if (count > 0 && pos == PT_MSR) {
+		reg = get_user_msr(target);
+		if (kbuf)
+			*k++ = reg;
+		else if (__put_user(reg, u++))
+			return -EFAULT;
+		++pos;
+		--count;
+	}
+
+	if (kbuf)
+		for (; count > 0 && pos < PT_REGS_COUNT; --count)
+			*k++ = regs[pos++];
+	else
+		for (; count > 0 && pos < PT_REGS_COUNT; --count)
+			if (__put_user((compat_ulong_t) regs[pos++], u++))
+				return -EFAULT;
+
+	kbuf = k;
+	ubuf = u;
+	pos *= sizeof(reg);
+	count *= sizeof(reg);
+	return user_regset_copyout_zero(&pos, &count, &kbuf, &ubuf,
+					PT_REGS_COUNT * sizeof(reg), -1);
+}
+
+static int tm_cgpr32_set(struct task_struct *target,
+		     const struct user_regset *regset,
+		     unsigned int pos, unsigned int count,
+		     const void *kbuf, const void __user *ubuf)
+{
+	unsigned long *regs = &target->thread.ckpt_regs.gpr[0];
+	const compat_ulong_t *k = kbuf;
+	const compat_ulong_t __user *u = ubuf;
+	compat_ulong_t reg;
+
+	flush_fp_to_thread(target);
+	flush_altivec_to_thread(target);
+	flush_tmregs_to_thread(target);
+
+	if (target->thread.regs == NULL)
+		return -EIO;
+
+	CHECK_FULL_REGS(target->thread.regs);
+
+	pos /= sizeof(reg);
+	count /= sizeof(reg);
+
+	if (kbuf)
+		for (; count > 0 && pos < PT_MSR; --count)
+			regs[pos++] = *k++;
+	else
+		for (; count > 0 && pos < PT_MSR; --count) {
+			if (__get_user(reg, u++))
+				return -EFAULT;
+			regs[pos++] = reg;
+		}
+
+
+	if (count > 0 && pos == PT_MSR) {
+		if (kbuf)
+			reg = *k++;
+		else if (__get_user(reg, u++))
+			return -EFAULT;
+		set_user_msr(target, reg);
+		++pos;
+		--count;
+	}
+
+	if (kbuf) {
+		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count)
+			regs[pos++] = *k++;
+		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
+			++k;
+	} else {
+		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
+			if (__get_user(reg, u++))
+				return -EFAULT;
+			regs[pos++] = reg;
+		}
+		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
+			if (__get_user(reg, u++))
+				return -EFAULT;
+	}
+
+	if (count > 0 && pos == PT_TRAP) {
+		if (kbuf)
+			reg = *k++;
+		else if (__get_user(reg, u++))
+			return -EFAULT;
+		set_user_trap(target, reg);
+		++pos;
+		--count;
+	}
+
+	kbuf = k;
+	ubuf = u;
+	pos *= sizeof(reg);
+	count *= sizeof(reg);
+	return user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
+					 (PT_TRAP + 1) * sizeof(reg), -1);
+}
+
+
 /*
  * These are the regset flavors matching the CONFIG_PPC32 native set.
  */
@@ -831,6 +1437,28 @@ static const struct user_regset compat_regsets[] = {
 		.active = evr_active, .get = evr_get, .set = evr_set
 	},
 #endif
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+	[REGSET_TM_SPR] = {
+		.core_note_type = NT_PPC_TM_SPR, .n = 7,
+		.size = sizeof(u64), .align = sizeof(u64),
+		.get = tm_spr_get, .set = tm_spr_set
+	},
+	[REGSET_TM_CGPR] = {
+		.core_note_type = NT_PPC_TM_CGPR, .n = ELF_NGREG,
+		.size = sizeof(long), .align = sizeof(long),
+		.get = tm_cgpr32_get, .set = tm_cgpr32_set
+	},
+	[REGSET_TM_CFPR] = {
+		.core_note_type = NT_PPC_TM_CFPR, .n = ELF_NFPREG,
+		.size = sizeof(double), .align = sizeof(double),
+		.get = tm_cfpr_get, .set = tm_cfpr_set
+	},
+	[REGSET_TM_CVMX] = {
+		.core_note_type = NT_PPC_TM_CVMX, .n = 34,
+		.size = sizeof(vector128), .align = sizeof(vector128),
+		.get = tm_cvmx_get, .set = tm_cvmx_set
+	},
+#endif
 };
 
 static const struct user_regset_view user_ppc_compat_view = {
@@ -1754,7 +2382,6 @@ long arch_ptrace(struct task_struct *child, long request,
 					     REGSET_SPE, 0, 35 * sizeof(u32),
 					     datavp);
 #endif
-
 	default:
 		ret = ptrace_request(child, request, addr, data);
 		break;
-- 
1.7.11.7

[PATCH V2 3/3] powerpc, ptrace: Enable support for miscellaneous registers

[Anshuman Khandual]

This patch enables get and set of miscellaneous registers through ptrace
PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing new powerpc
specific register set REGSET_MISC support corresponding to the new ELF
core note NT_PPC_MISC added previously in this regard.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/ptrace.c | 81 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 92faded..3332dd8 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1054,6 +1054,76 @@ static int tm_cvmx_set(struct task_struct *target, const struct user_regset *reg
 #endif	/* CONFIG_PPC_TRANSACTIONAL_MEM */
 
 /*
+ * Miscellaneous Registers
+ *
+ * struct {
+ * 	unsigned long dscr;
+ *	unsigned long ppr;
+ * 	unsigned long tar;
+ * };
+ */
+static int misc_get(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   void *kbuf, void __user *ubuf)
+{
+	int ret;
+
+	/* DSCR register */
+	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+					&target->thread.dscr, 0,
+					sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, dscr) + sizeof(unsigned long) +
+				sizeof(unsigned long) != offsetof(struct thread_struct, ppr));
+
+	/* PPR register */
+	if (!ret)
+		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+					  &target->thread.ppr, sizeof(unsigned long),
+					  2 * sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, ppr) + sizeof(unsigned long)
+						!= offsetof(struct thread_struct, tar));
+	/* TAR register */
+	if (!ret)
+		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+					  &target->thread.tar, 2 * sizeof(unsigned long),
+					  3 * sizeof(unsigned long));
+	return ret;
+}
+
+static int misc_set(struct task_struct *target, const struct user_regset *regset,
+		   unsigned int pos, unsigned int count,
+		   const void *kbuf, const void __user *ubuf)
+{
+	int ret;
+
+	/* DSCR register */
+	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+					&target->thread.dscr, 0,
+					sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, dscr) + sizeof(unsigned long) +
+			sizeof(unsigned long) != offsetof(struct thread_struct, ppr));
+
+	/* PPR register */
+	if (!ret)
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+						&target->thread.ppr, sizeof(unsigned long),
+						2 * sizeof(unsigned long));
+
+	BUILD_BUG_ON(offsetof(struct thread_struct, ppr) + sizeof(unsigned long)
+						!= offsetof(struct thread_struct, tar));
+
+	/* TAR register */
+	if (!ret)
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+						&target->thread.tar, 2 * sizeof(unsigned long),
+						3 * sizeof(unsigned long));
+	return ret;
+}
+
+/*
  * These are our native regset flavors.
  */
 enum powerpc_regset {
@@ -1074,6 +1144,7 @@ enum powerpc_regset {
 	REGSET_TM_CFPR,		/* TM checkpointed FPR */
 	REGSET_TM_CVMX,		/* TM checkpointed VMX */
 #endif
+	REGSET_MISC		/* Miscellaneous */
 };
 
 static const struct user_regset native_regsets[] = {
@@ -1130,6 +1201,11 @@ static const struct user_regset native_regsets[] = {
 		.get = tm_cvmx_get, .set = tm_cvmx_set
 	},
 #endif
+	[REGSET_MISC] = {
+		.core_note_type = NT_PPC_MISC, .n = 3,
+		.size = sizeof(u64), .align = sizeof(u64),
+		.get = misc_get, .set = misc_set
+	},
 };
 
 static const struct user_regset_view user_ppc_native_view = {
@@ -1459,6 +1535,11 @@ static const struct user_regset compat_regsets[] = {
 		.get = tm_cvmx_get, .set = tm_cvmx_set
 	},
 #endif
+	[REGSET_MISC] = {
+		.core_note_type = NT_PPC_MISC, .n = 3,
+		.size = sizeof(u64), .align = sizeof(u64),
+		.get = misc_get, .set = misc_set
+	},
 };
 
 static const struct user_regset_view user_ppc_compat_view = {
-- 
1.7.11.7

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Pedro Alves]

On 05/05/14 08:54, Anshuman Khandual wrote:
> This patch enables get and set of transactional memory related register
> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
> ELF core note types added previously in this regard.
> 
> 	(1) NT_PPC_TM_SPR
> 	(2) NT_PPC_TM_CGPR
> 	(3) NT_PPC_TM_CFPR
> 	(4) NT_PPC_TM_CVMX

Sorry that I couldn't tell this from the code, but, what does the
kernel return when the ptracer requests these registers and the
program is not in a transaction?  Specifically I'm wondering whether
this follows the same semantics as the s390 port.

-- 
Pedro Alves

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Pedro Alves]

I wonder whether people are getting Roland's address from?

It's frequent that ptrace related patches end up CCed to
roland@redhat.com, but, he's not been at Red Hat for a few years
now.  Roland, do you still want to be CCed on ptrace-related
issues?  If so, there's probably a script somewhere in the
kernel that needs updating.  If not, well, it'd be good
if it were updated anyway.  :-)

It's a little annoying, as Red Hat's servers outright reject
email sent from a @redhat.com address if one tries to send
an email that includes a CC/FROM to a user that no longer
exists in the @redhat.com domain.

-- 
Pedro Alves

On 05/05/14 08:54, Anshuman Khandual wrote:
> This patch enables get and set of transactional memory related register
> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
> ELF core note types added previously in this regard.
> 
> 	(1) NT_PPC_TM_SPR
> 	(2) NT_PPC_TM_CGPR
> 	(3) NT_PPC_TM_CFPR
> 	(4) NT_PPC_TM_CVMX
> 
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/switch_to.h |   8 +
>  arch/powerpc/kernel/process.c        |  24 ++
>  arch/powerpc/kernel/ptrace.c         | 683 +++++++++++++++++++++++++++++++++--
>  3 files changed, 687 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
> index 0e83e7d..2737f46 100644
> --- a/arch/powerpc/include/asm/switch_to.h
> +++ b/arch/powerpc/include/asm/switch_to.h
> @@ -80,6 +80,14 @@ static inline void flush_spe_to_thread(struct task_struct *t)
>  }
>  #endif
>  
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +extern void flush_tmregs_to_thread(struct task_struct *);
> +#else
> +static inline void flush_tmregs_to_thread(struct task_struct *t)
> +{
> +}
> +#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
> +
>  static inline void clear_task_ebb(struct task_struct *t)
>  {
>  #ifdef CONFIG_PPC_BOOK3S_64
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 31d0215..e247898 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -695,6 +695,30 @@ static inline void __switch_to_tm(struct task_struct *prev)
>  	}
>  }
>  
> +void flush_tmregs_to_thread(struct task_struct *tsk)
> +{
> +	/*
> +	 * If task is not current, it should have been flushed
> +	 * already to it's thread_struct during __switch_to().
> +	 */
> +	if (tsk != current)
> +		return;
> +
> +	preempt_disable();
> +	if (tsk->thread.regs) {
> +		/*
> +		 * If we are still current, the TM state need to
> +		 * be flushed to thread_struct as it will be still
> +		 * present in the current cpu.
> +		 */
> +		if (MSR_TM_ACTIVE(tsk->thread.regs->msr)) {
> +			__switch_to_tm(tsk);
> +			tm_recheckpoint_new_task(tsk);
> +		}
> +	}
> +	preempt_enable();
> +}
> +
>  /*
>   * This is called if we are on the way out to userspace and the
>   * TIF_RESTORE_TM flag is set.  It checks if we need to reload
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 2e3d2bf..92faded 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -357,6 +357,17 @@ static int gpr_set(struct task_struct *target, const struct user_regset *regset,
>  	return ret;
>  }
>  
> +/*
> + * When any transaction is active, "thread_struct->transact_fp" holds
> + * the current running value of all FPR registers and "thread_struct->
> + * fp_state" holds the last checkpointed FPR registers state for the
> + * current transaction.
> + *
> + * struct data {
> + * 	u64	fpr[32];
> + * 	u64	fpscr;
> + * };
> + */
>  static int fpr_get(struct task_struct *target, const struct user_regset *regset,
>  		   unsigned int pos, unsigned int count,
>  		   void *kbuf, void __user *ubuf)
> @@ -365,21 +376,41 @@ static int fpr_get(struct task_struct *target, const struct user_regset *regset,
>  	u64 buf[33];
>  	int i;
>  #endif
> -	flush_fp_to_thread(target);
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
> +		flush_fp_to_thread(target);
> +		flush_altivec_to_thread(target);
> +		flush_tmregs_to_thread(target);
> +	} else {
> +		flush_fp_to_thread(target);
> +	}
>  
>  #ifdef CONFIG_VSX
>  	/* copy to local buffer then write that out */
> -	for (i = 0; i < 32 ; i++)
> -		buf[i] = target->thread.TS_FPR(i);
> -	buf[32] = target->thread.fp_state.fpscr;
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
> +		for (i = 0; i < 32 ; i++)
> +			buf[i] = target->thread.TS_TRANS_FPR(i);
> +		buf[32] = target->thread.transact_fp.fpscr;
> +	} else {
> +		for (i = 0; i < 32 ; i++)
> +			buf[i] = target->thread.TS_FPR(i);
> +		buf[32] = target->thread.fp_state.fpscr;
> +	}
>  	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
>  
>  #else
> -	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
> -		     offsetof(struct thread_fp_state, fpr[32][0]));
> +	if (MSR_TM_ACTIVE(tsk->thread.regs->msr)) {
> +		BUILD_BUG_ON(offsetof(struct transact_fp, fpscr) !=
> +				offsetof(struct transact_fp, fpr[32][0]));
>  
> -	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +		return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +				   &target->thread.transact_fp, 0, -1);
> +	} esle {
> +		BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
> +			     offsetof(struct thread_fp_state, fpr[32][0]));
> +
> +		return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
>  				   &target->thread.fp_state, 0, -1);
> +	}
>  #endif
>  }
>  
> @@ -391,23 +422,44 @@ static int fpr_set(struct task_struct *target, const struct user_regset *regset,
>  	u64 buf[33];
>  	int i;
>  #endif
> -	flush_fp_to_thread(target);
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
> +		flush_fp_to_thread(target);
> +		flush_altivec_to_thread(target);
> +		flush_tmregs_to_thread(target);
> +	} else {
> +		flush_fp_to_thread(target);
> +	}
>  
>  #ifdef CONFIG_VSX
>  	/* copy to local buffer then write that out */
>  	i = user_regset_copyin(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
>  	if (i)
>  		return i;
> -	for (i = 0; i < 32 ; i++)
> -		target->thread.TS_FPR(i) = buf[i];
> -	target->thread.fp_state.fpscr = buf[32];
> +	for (i = 0; i < 32 ; i++) {
> +		if (MSR_TM_ACTIVE(target->thread.regs->msr))
> +			target->thread.TS_TRANS_FPR(i) = buf[i];
> +		else
> +			target->thread.TS_FPR(i) = buf[i];
> +	}
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr))
> +		target->thread.transact_fp.fpscr = buf[32];
> +	else
> +		target->thread.fp_state.fpscr = buf[32];
>  	return 0;
>  #else
> -	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
> -		     offsetof(struct thread_fp_state, fpr[32][0]));
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
> +		BUILD_BUG_ON(offsetof(struct transact_fp, fpscr) !=
> +			     offsetof(struct transact_fp, fpr[32][0]));
>  
> -	return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> -				  &target->thread.fp_state, 0, -1);
> +		return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +					  &target->thread.transact_fp, 0, -1);
> +	} else {
> +		BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
> +			     offsetof(struct thread_fp_state, fpr[32][0]));
> +
> +		return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +				&target->thread.fp_state, 0, -1);
> +	}
>  #endif
>  }
>  
> @@ -432,20 +484,44 @@ static int vr_active(struct task_struct *target,
>  	return target->thread.used_vr ? regset->n : 0;
>  }
>  
> +/*
> + * When any transaction is active, "thread_struct->transact_vr" holds
> + * the current running value of all VMX registers and "thread_struct->
> + * vr_state" holds the last checkpointed value of VMX registers for the
> + * current transaction.
> + *
> + * struct data {
> + * 	vector128	vr[32];
> + * 	vector128	vscr;
> + * 	vector128	vrsave;
> + * };
> + */
>  static int vr_get(struct task_struct *target, const struct user_regset *regset,
>  		  unsigned int pos, unsigned int count,
>  		  void *kbuf, void __user *ubuf)
>  {
>  	int ret;
> +	struct thread_vr_state *addr;
>  
> -	flush_altivec_to_thread(target);
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
> +		flush_fp_to_thread(target);
> +		flush_altivec_to_thread(target);
> +		flush_tmregs_to_thread(target);
> +	} else {
> +		flush_altivec_to_thread(target);
> +	}
>  
>  	BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
>  		     offsetof(struct thread_vr_state, vr[32]));
>  
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr))
> +		addr = &target->thread.transact_vr;
> +	else
> +		addr = &target->thread.vr_state;
> +
>  	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> -				  &target->thread.vr_state, 0,
> -				  33 * sizeof(vector128));
> +				addr, 0, 33 * sizeof(vector128));
> +
>  	if (!ret) {
>  		/*
>  		 * Copy out only the low-order word of vrsave.
> @@ -455,11 +531,14 @@ static int vr_get(struct task_struct *target, const struct user_regset *regset,
>  			u32 word;
>  		} vrsave;
>  		memset(&vrsave, 0, sizeof(vrsave));
> -		vrsave.word = target->thread.vrsave;
> +		if (MSR_TM_ACTIVE(target->thread.regs->msr))
> +			vrsave.word = target->thread.transact_vrsave;
> +		else
> +			vrsave.word = target->thread.vrsave;
> +
>  		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
>  					  33 * sizeof(vector128), -1);
>  	}
> -
>  	return ret;
>  }
>  
> @@ -467,16 +546,27 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
>  		  unsigned int pos, unsigned int count,
>  		  const void *kbuf, const void __user *ubuf)
>  {
> +	struct thread_vr_state *addr;
>  	int ret;
>  
> -	flush_altivec_to_thread(target);
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
> +		flush_fp_to_thread(target);
> +		flush_altivec_to_thread(target);
> +		flush_tmregs_to_thread(target);
> +	} else {
> +		flush_altivec_to_thread(target);
> +	}
>  
>  	BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
>  		     offsetof(struct thread_vr_state, vr[32]));
>  
> +	if (MSR_TM_ACTIVE(target->thread.regs->msr))
> +		addr = &target->thread.transact_vr;
> +	else
> +		addr = &target->thread.vr_state;
>  	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> -				 &target->thread.vr_state, 0,
> -				 33 * sizeof(vector128));
> +			addr, 0, 33 * sizeof(vector128));
> +
>  	if (!ret && count > 0) {
>  		/*
>  		 * We use only the first word of vrsave.
> @@ -486,13 +576,21 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
>  			u32 word;
>  		} vrsave;
>  		memset(&vrsave, 0, sizeof(vrsave));
> -		vrsave.word = target->thread.vrsave;
> +
> +		if (MSR_TM_ACTIVE(target->thread.regs->msr))
> +			vrsave.word = target->thread.transact_vrsave;
> +		else
> +			vrsave.word = target->thread.vrsave;
> +
>  		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
>  					 33 * sizeof(vector128), -1);
> -		if (!ret)
> -			target->thread.vrsave = vrsave.word;
> +		if (!ret) {
> +			if (MSR_TM_ACTIVE(target->thread.regs->msr))
> +				target->thread.transact_vrsave = vrsave.word;
> +			else
> +				target->thread.vrsave = vrsave.word;
> +		}
>  	}
> -
>  	return ret;
>  }
>  #endif /* CONFIG_ALTIVEC */
> @@ -613,6 +711,347 @@ static int evr_set(struct task_struct *target, const struct user_regset *regset,
>  }
>  #endif /* CONFIG_SPE */
>  
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +
> +/*
> + *  Transactional memory SPR
> + *
> + * struct {
> + * 	u64		tm_tfhar;
> + *	u64		tm_texasr;
> + *	u64		tm_tfiar;
> + *	unsigned long	tm_orig_msr;
> + * 	unsigned long	tm_tar;
> + *	unsigned long	tm_ppr;
> + *	unsigned long	tm_dscr;
> + * };
> + */
> +static int tm_spr_get(struct task_struct *target, const struct user_regset *regset,
> +		   unsigned int pos, unsigned int count,
> +		   void *kbuf, void __user *ubuf)
> +{
> +	int ret;
> +
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +
> +	/* TFHAR register */
> +	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_tfhar, 0, sizeof(u64));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfhar) +
> +			sizeof(u64) != offsetof(struct thread_struct, tm_texasr));
> +
> +	/* TEXASR register */
> +	if (!ret)
> +		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_texasr, sizeof(u64), 2 * sizeof(u64));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_texasr) +
> +			sizeof(u64) != offsetof(struct thread_struct, tm_tfiar));
> +
> +	/* TFIAR register */
> +	if (!ret)
> +		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_tfiar, 2 * sizeof(u64), 3 * sizeof(u64));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfiar) +
> +			sizeof(u64) != offsetof(struct thread_struct, tm_orig_msr));
> +
> +	/* TM checkpointed original MSR */
> +	if (!ret)
> +		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_orig_msr, 3 * sizeof(u64),
> +				3 * sizeof(u64) + sizeof(unsigned long));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_orig_msr) +
> +			sizeof(unsigned long) + sizeof(struct pt_regs)
> +				!= offsetof(struct thread_struct, tm_tar));
> +
> +	/* TM checkpointed TAR register */
> +	if (!ret)
> +		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_tar, 3 * sizeof(u64) +
> +				sizeof(unsigned long) , 3 * sizeof(u64) +
> +					2 * sizeof(unsigned long));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tar)
> +			+ sizeof(unsigned long) !=
> +				offsetof(struct thread_struct, tm_ppr));
> +
> +	/* TM checkpointed PPR register */
> +	if (!ret)
> +		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +				&target->thread.tm_ppr, 3 * sizeof(u64) +
> +					2 * sizeof(unsigned long), 3 * sizeof(u64) +
> +						3 * sizeof(unsigned long));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_ppr) +
> +			sizeof(unsigned long) !=
> +				offsetof(struct thread_struct, tm_dscr));
> +
> +	/* TM checkpointed DSCR register */
> +	if (!ret)
> +		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_dscr, 3 * sizeof(u64)
> +				+ 3 * sizeof(unsigned long), 3 * sizeof(u64)
> +						+ 4 * sizeof(unsigned long));
> +	return ret;
> +}
> +
> +static int tm_spr_set(struct task_struct *target, const struct user_regset *regset,
> +		   unsigned int pos, unsigned int count,
> +		   const void *kbuf, const void __user *ubuf)
> +{
> +	int ret;
> +
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +
> +	/* TFHAR register */
> +	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +				&target->thread.tm_tfhar, 0, sizeof(u64));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfhar)
> +		+ sizeof(u64) != offsetof(struct thread_struct, tm_texasr));
> +
> +	/* TEXASR register */
> +	if (!ret)
> +		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_texasr, sizeof(u64), 2 * sizeof(u64));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_texasr)
> +		+ sizeof(u64) != offsetof(struct thread_struct, tm_tfiar));
> +
> +	/* TFIAR register */
> +	if (!ret)
> +		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_tfiar, 2 * sizeof(u64), 3 * sizeof(u64));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfiar)
> +		+ sizeof(u64) != offsetof(struct thread_struct, tm_orig_msr));
> +
> +	/* TM checkpointed orig MSR */
> +	if (!ret)
> +		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_orig_msr, 3 * sizeof(u64),
> +				3 * sizeof(u64) + sizeof(unsigned long));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_orig_msr)
> +		+ sizeof(unsigned long) + sizeof(struct pt_regs) !=
> +			offsetof(struct thread_struct, tm_tar));
> +
> +	/* TM checkpointed TAR register */
> +	if (!ret)
> +		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.tm_tar, 3 * sizeof(u64) +
> +				sizeof(unsigned long), 3 * sizeof(u64) +
> +					2 * sizeof(unsigned long));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_tar)
> +			+ sizeof(unsigned long) != offsetof(struct thread_struct, tm_ppr));
> +
> +	/* TM checkpointed PPR register */
> +	if (!ret)
> +		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +				&target->thread.tm_ppr, 3 * sizeof(u64)
> +					+ 2 * sizeof(unsigned long), 3 * sizeof(u64)
> +					+ 3 * sizeof(unsigned long));
> +
> +	BUILD_BUG_ON(offsetof(struct thread_struct, tm_ppr) +
> +			sizeof(unsigned long) !=
> +				offsetof(struct thread_struct, tm_dscr));
> +
> +	/* TM checkpointed DSCR register */
> +	if (!ret)
> +		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +				&target->thread.tm_dscr,
> +					3 * sizeof(u64) + 3 * sizeof(unsigned long),
> +					3 * sizeof(u64) + 4 * sizeof(unsigned long));
> +
> +	return ret;
> +}
> +
> +/*
> + * TM Checkpointed GPR
> + *
> + * struct data {
> + * 	struct pt_regs ckpt_regs;
> + * };
> + */
> +static int tm_cgpr_get(struct task_struct *target, const struct user_regset *regset,
> +		   unsigned int pos, unsigned int count,
> +		   void *kbuf, void __user *ubuf)
> +{
> +	int ret;
> +
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.ckpt_regs, 0,
> +				sizeof(struct pt_regs));
> +	return ret;
> +}
> +
> +static int tm_cgpr_set(struct task_struct *target, const struct user_regset *regset,
> +		   unsigned int pos, unsigned int count,
> +		   const void *kbuf, const void __user *ubuf)
> +{
> +	int ret;
> +
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +					&target->thread.ckpt_regs, 0,
> +						sizeof(struct pt_regs));
> +	return ret;
> +}
> +
> +/*
> + * TM Checkpointed FPR
> + *
> + * struct data {
> + * 	u64	fpr[32];
> + * 	u64	fpscr;
> + * };
> + */
> +static int tm_cfpr_get(struct task_struct *target, const struct user_regset *regset,
> +		   unsigned int pos, unsigned int count,
> +		   void *kbuf, void __user *ubuf)
> +{
> +#ifdef CONFIG_VSX
> +	u64 buf[33];
> +	int i;
> +#endif
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +
> +#ifdef CONFIG_VSX
> +	/* copy to local buffer then write that out */
> +	for (i = 0; i < 32 ; i++)
> +		buf[i] = target->thread.TS_FPR(i);
> +	buf[32] = target->thread.fp_state.fpscr;
> +	return user_regset_copyout(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
> +
> +#else
> +	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
> +		offsetof(struct thread_fp_state, fpr[32][0]));
> +
> +	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +			&target->thread.thread_fp_state, 0, -1);
> +#endif
> +}
> +
> +static int tm_cfpr_set(struct task_struct *target, const struct user_regset *regset,
> +		   unsigned int pos, unsigned int count,
> +		   const void *kbuf, const void __user *ubuf)
> +{
> +#ifdef CONFIG_VSX
> +	u64 buf[33];
> +	int i;
> +#endif
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +
> +#ifdef CONFIG_VSX
> +	/* copy to local buffer then write that out */
> +	i = user_regset_copyin(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
> +	if (i)
> +		return i;
> +	for (i = 0; i < 32 ; i++)
> +		target->thread.TS_FPR(i) = buf[i];
> +	target->thread.fp_state.fpscr = buf[32];
> +	return 0;
> +#else
> +	BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
> +		      offsetof(struct thread_fp_state, fpr[32][0]));
> +
> +	return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +				&target->thread.fp_state, 0, -1);
> +#endif
> +}
> +
> +/*
> + * TM Checkpointed VMX
> + *
> + * struct data {
> + * 	vector128	vr[32];
> + * 	vector128	vscr;
> + * 	vector128	vrsave;
> + *};
> + */
> +static int tm_cvmx_get(struct task_struct *target, const struct user_regset *regset,
> +		   unsigned int pos, unsigned int count,
> +		   void *kbuf, void __user *ubuf)
> +{
> +	int ret;
> +
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +
> +	BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
> +		     offsetof(struct thread_vr_state, vr[32]));
> +
> +	ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
> +				  &target->thread.vr_state, 0,
> +				  33 * sizeof(vector128));
> +	if (!ret) {
> +		/*
> +		 * Copy out only the low-order word of vrsave.
> +		 */
> +		union {
> +			elf_vrreg_t reg;
> +			u32 word;
> +		} vrsave;
> +		memset(&vrsave, 0, sizeof(vrsave));
> +		vrsave.word = target->thread.vrsave;
> +		ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
> +					  33 * sizeof(vector128), -1);
> +	}
> +	return ret;
> +}
> +
> +static int tm_cvmx_set(struct task_struct *target, const struct user_regset *regset,
> +		   unsigned int pos, unsigned int count,
> +		   const void *kbuf, const void __user *ubuf)
> +{
> +	int ret;
> +
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +
> +	BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
> +		offsetof(struct thread_vr_state, vr[32]));
> +
> +	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> +				 &target->thread.vr_state, 0,
> +				 33 * sizeof(vector128));
> +	if (!ret && count > 0) {
> +		/*
> +		 * We use only the first word of vrsave.
> +		 */
> +		union {
> +			elf_vrreg_t reg;
> +			u32 word;
> +		} vrsave;
> +		memset(&vrsave, 0, sizeof(vrsave));
> +		vrsave.word = target->thread.vrsave;
> +		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
> +					 33 * sizeof(vector128), -1);
> +		if (!ret)
> +			target->thread.vrsave = vrsave.word;
> +	}
> +	return ret;
> +}
> +#endif	/* CONFIG_PPC_TRANSACTIONAL_MEM */
>  
>  /*
>   * These are our native regset flavors.
> @@ -629,6 +1068,12 @@ enum powerpc_regset {
>  #ifdef CONFIG_SPE
>  	REGSET_SPE,
>  #endif
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	REGSET_TM_SPR,		/* TM specific SPR */
> +	REGSET_TM_CGPR,		/* TM checkpointed GPR */
> +	REGSET_TM_CFPR,		/* TM checkpointed FPR */
> +	REGSET_TM_CVMX,		/* TM checkpointed VMX */
> +#endif
>  };
>  
>  static const struct user_regset native_regsets[] = {
> @@ -663,6 +1108,28 @@ static const struct user_regset native_regsets[] = {
>  		.active = evr_active, .get = evr_get, .set = evr_set
>  	},
>  #endif
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	[REGSET_TM_SPR] = {
> +		.core_note_type = NT_PPC_TM_SPR, .n = 7,
> +		.size = sizeof(u64), .align = sizeof(u64),
> +		.get = tm_spr_get, .set = tm_spr_set
> +	},
> +	[REGSET_TM_CGPR] = {
> +		.core_note_type = NT_PPC_TM_CGPR, .n = ELF_NGREG,
> +		.size = sizeof(long), .align = sizeof(long),
> +		.get = tm_cgpr_get, .set = tm_cgpr_set
> +	},
> +	[REGSET_TM_CFPR] = {
> +		.core_note_type = NT_PPC_TM_CFPR, .n = ELF_NFPREG,
> +		.size = sizeof(double), .align = sizeof(double),
> +		.get = tm_cfpr_get, .set = tm_cfpr_set
> +	},
> +	[REGSET_TM_CVMX] = {
> +		.core_note_type = NT_PPC_TM_CVMX, .n = 34,
> +		.size = sizeof(vector128), .align = sizeof(vector128),
> +		.get = tm_cvmx_get, .set = tm_cvmx_set
> +	},
> +#endif
>  };
>  
>  static const struct user_regset_view user_ppc_native_view = {
> @@ -803,6 +1270,145 @@ static int gpr32_set(struct task_struct *target,
>  					 (PT_TRAP + 1) * sizeof(reg), -1);
>  }
>  
> +static int tm_cgpr32_get(struct task_struct *target,
> +		     const struct user_regset *regset,
> +		     unsigned int pos, unsigned int count,
> +		     void *kbuf, void __user *ubuf)
> +{
> +	const unsigned long *regs = &target->thread.ckpt_regs.gpr[0];
> +	compat_ulong_t *k = kbuf;
> +	compat_ulong_t __user *u = ubuf;
> +	compat_ulong_t reg;
> +	int i;
> +
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +
> +	if (target->thread.regs == NULL)
> +		return -EIO;
> +
> +	if (!FULL_REGS(target->thread.regs)) {
> +		/* We have a partial register set.  Fill 14-31 with bogus values */
> +		for (i = 14; i < 32; i++)
> +			target->thread.regs->gpr[i] = NV_REG_POISON; 
> +	}
> +
> +	pos /= sizeof(reg);
> +	count /= sizeof(reg);
> +
> +	if (kbuf)
> +		for (; count > 0 && pos < PT_MSR; --count)
> +			*k++ = regs[pos++];
> +	else
> +		for (; count > 0 && pos < PT_MSR; --count)
> +			if (__put_user((compat_ulong_t) regs[pos++], u++))
> +				return -EFAULT;
> +
> +	if (count > 0 && pos == PT_MSR) {
> +		reg = get_user_msr(target);
> +		if (kbuf)
> +			*k++ = reg;
> +		else if (__put_user(reg, u++))
> +			return -EFAULT;
> +		++pos;
> +		--count;
> +	}
> +
> +	if (kbuf)
> +		for (; count > 0 && pos < PT_REGS_COUNT; --count)
> +			*k++ = regs[pos++];
> +	else
> +		for (; count > 0 && pos < PT_REGS_COUNT; --count)
> +			if (__put_user((compat_ulong_t) regs[pos++], u++))
> +				return -EFAULT;
> +
> +	kbuf = k;
> +	ubuf = u;
> +	pos *= sizeof(reg);
> +	count *= sizeof(reg);
> +	return user_regset_copyout_zero(&pos, &count, &kbuf, &ubuf,
> +					PT_REGS_COUNT * sizeof(reg), -1);
> +}
> +
> +static int tm_cgpr32_set(struct task_struct *target,
> +		     const struct user_regset *regset,
> +		     unsigned int pos, unsigned int count,
> +		     const void *kbuf, const void __user *ubuf)
> +{
> +	unsigned long *regs = &target->thread.ckpt_regs.gpr[0];
> +	const compat_ulong_t *k = kbuf;
> +	const compat_ulong_t __user *u = ubuf;
> +	compat_ulong_t reg;
> +
> +	flush_fp_to_thread(target);
> +	flush_altivec_to_thread(target);
> +	flush_tmregs_to_thread(target);
> +
> +	if (target->thread.regs == NULL)
> +		return -EIO;
> +
> +	CHECK_FULL_REGS(target->thread.regs);
> +
> +	pos /= sizeof(reg);
> +	count /= sizeof(reg);
> +
> +	if (kbuf)
> +		for (; count > 0 && pos < PT_MSR; --count)
> +			regs[pos++] = *k++;
> +	else
> +		for (; count > 0 && pos < PT_MSR; --count) {
> +			if (__get_user(reg, u++))
> +				return -EFAULT;
> +			regs[pos++] = reg;
> +		}
> +
> +
> +	if (count > 0 && pos == PT_MSR) {
> +		if (kbuf)
> +			reg = *k++;
> +		else if (__get_user(reg, u++))
> +			return -EFAULT;
> +		set_user_msr(target, reg);
> +		++pos;
> +		--count;
> +	}
> +
> +	if (kbuf) {
> +		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count)
> +			regs[pos++] = *k++;
> +		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
> +			++k;
> +	} else {
> +		for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
> +			if (__get_user(reg, u++))
> +				return -EFAULT;
> +			regs[pos++] = reg;
> +		}
> +		for (; count > 0 && pos < PT_TRAP; --count, ++pos)
> +			if (__get_user(reg, u++))
> +				return -EFAULT;
> +	}
> +
> +	if (count > 0 && pos == PT_TRAP) {
> +		if (kbuf)
> +			reg = *k++;
> +		else if (__get_user(reg, u++))
> +			return -EFAULT;
> +		set_user_trap(target, reg);
> +		++pos;
> +		--count;
> +	}
> +
> +	kbuf = k;
> +	ubuf = u;
> +	pos *= sizeof(reg);
> +	count *= sizeof(reg);
> +	return user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
> +					 (PT_TRAP + 1) * sizeof(reg), -1);
> +}
> +
> +
>  /*
>   * These are the regset flavors matching the CONFIG_PPC32 native set.
>   */
> @@ -831,6 +1437,28 @@ static const struct user_regset compat_regsets[] = {
>  		.active = evr_active, .get = evr_get, .set = evr_set
>  	},
>  #endif
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	[REGSET_TM_SPR] = {
> +		.core_note_type = NT_PPC_TM_SPR, .n = 7,
> +		.size = sizeof(u64), .align = sizeof(u64),
> +		.get = tm_spr_get, .set = tm_spr_set
> +	},
> +	[REGSET_TM_CGPR] = {
> +		.core_note_type = NT_PPC_TM_CGPR, .n = ELF_NGREG,
> +		.size = sizeof(long), .align = sizeof(long),
> +		.get = tm_cgpr32_get, .set = tm_cgpr32_set
> +	},
> +	[REGSET_TM_CFPR] = {
> +		.core_note_type = NT_PPC_TM_CFPR, .n = ELF_NFPREG,
> +		.size = sizeof(double), .align = sizeof(double),
> +		.get = tm_cfpr_get, .set = tm_cfpr_set
> +	},
> +	[REGSET_TM_CVMX] = {
> +		.core_note_type = NT_PPC_TM_CVMX, .n = 34,
> +		.size = sizeof(vector128), .align = sizeof(vector128),
> +		.get = tm_cvmx_get, .set = tm_cvmx_set
> +	},
> +#endif
>  };
>  
>  static const struct user_regset_view user_ppc_compat_view = {
> @@ -1754,7 +2382,6 @@ long arch_ptrace(struct task_struct *child, long request,
>  					     REGSET_SPE, 0, 35 * sizeof(u32),
>  					     datavp);
>  #endif
> -
>  	default:
>  		ret = ptrace_request(child, request, addr, data);
>  		break;
> 

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Anshuman Khandual]

On 05/13/2014 10:43 PM, Pedro Alves wrote:
> On 05/05/14 08:54, Anshuman Khandual wrote:
>> This patch enables get and set of transactional memory related register
>> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
>> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
>> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
>> ELF core note types added previously in this regard.
>>
>> 	(1) NT_PPC_TM_SPR
>> 	(2) NT_PPC_TM_CGPR
>> 	(3) NT_PPC_TM_CFPR
>> 	(4) NT_PPC_TM_CVMX
> 
> Sorry that I couldn't tell this from the code, but, what does the
> kernel return when the ptracer requests these registers and the
> program is not in a transaction?  Specifically I'm wondering whether
> this follows the same semantics as the s390 port.
> 

Right now, it still returns the saved state of the registers from thread
struct. I had assumed that the user must know the state of the transaction
before initiating the ptrace request. I guess its better to check for
the transaction status before processing the request. In case if TM is not
active on that thread, we should return -EINVAL.

I am not familiar with the s390 side of code. But if we look at the
s390_tdb_get function it checks for (regs->int_code & 0x200) before
processing the request. Not sure what 0x200 signifies though.

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Anshuman Khandual]

On 05/13/2014 10:51 PM, Pedro Alves wrote:
> I wonder whether people are getting Roland's address from?
> 
> It's frequent that ptrace related patches end up CCed to
> roland@redhat.com, but, he's not been at Red Hat for a few years
> now.  Roland, do you still want to be CCed on ptrace-related
> issues?  If so, there's probably a script somewhere in the
> kernel that needs updating.  If not, well, it'd be good
> if it were updated anyway.  :-)
> 
> It's a little annoying, as Red Hat's servers outright reject
> email sent from a @redhat.com address if one tries to send
> an email that includes a CC/FROM to a user that no longer
> exists in the @redhat.com domain.

Got the email address from some of the previous ptrace related
commits.

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Pedro Alves]

On 05/14/14 06:46, Anshuman Khandual wrote:
> On 05/13/2014 10:43 PM, Pedro Alves wrote:
>> On 05/05/14 08:54, Anshuman Khandual wrote:
>>> This patch enables get and set of transactional memory related register
>>> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
>>> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
>>> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
>>> ELF core note types added previously in this regard.
>>>
>>> 	(1) NT_PPC_TM_SPR
>>> 	(2) NT_PPC_TM_CGPR
>>> 	(3) NT_PPC_TM_CFPR
>>> 	(4) NT_PPC_TM_CVMX
>>
>> Sorry that I couldn't tell this from the code, but, what does the
>> kernel return when the ptracer requests these registers and the
>> program is not in a transaction?  Specifically I'm wondering whether
>> this follows the same semantics as the s390 port.
>>
> 
> Right now, it still returns the saved state of the registers from thread
> struct. I had assumed that the user must know the state of the transaction
> before initiating the ptrace request. I guess its better to check for
> the transaction status before processing the request. In case if TM is not
> active on that thread, we should return -EINVAL.

I think s390 returns ENODATA in that case.

 https://sourceware.org/ml/gdb-patches/2013-06/msg00273.html

We'll want some way to tell whether the system actually
supports this.  That could be ENODATA vs something-else (EINVAL
or perhaps better EIO for "request is invalid").

s390 actually screwed that, though it got away because
there's a bit in HWCAP to signal transactions support.  See:

 https://sourceware.org/ml/gdb-patches/2013-11/msg00080.html

Are you adding something to HWCAP too?

> 
> I am not familiar with the s390 side of code. But if we look at the
> s390_tdb_get function it checks for (regs->int_code & 0x200) before
> processing the request. Not sure what 0x200 signifies though.

-- 
Pedro Alves

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Michael Neuling]

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

> s390 actually screwed that, though it got away because
> there's a bit in HWCAP to signal transactions support.  See:
>
>  https://sourceware.org/ml/gdb-patches/2013-11/msg00080.html
>
> Are you adding something to HWCAP too?

Yes but it's in HWCAP2

Mikey

[-- Attachment #2: Type: text/html, Size: 438 bytes --]

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Pedro Alves]

On 05/14/14 12:18, Michael Neuling wrote:
> 
>> s390 actually screwed that, though it got away because
>> there's a bit in HWCAP to signal transactions support.  See:
>>
>>  https://sourceware.org/ml/gdb-patches/2013-11/msg00080.html
>>
>> Are you adding something to HWCAP too?
> 
> Yes but it's in HWCAP2

That's fine.

-- 
Pedro Alves

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Anshuman Khandual]

On 05/14/2014 04:45 PM, Pedro Alves wrote:
> On 05/14/14 06:46, Anshuman Khandual wrote:
>> On 05/13/2014 10:43 PM, Pedro Alves wrote:
>>> On 05/05/14 08:54, Anshuman Khandual wrote:
>>>> This patch enables get and set of transactional memory related register
>>>> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
>>>> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
>>>> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
>>>> ELF core note types added previously in this regard.
>>>>
>>>> 	(1) NT_PPC_TM_SPR
>>>> 	(2) NT_PPC_TM_CGPR
>>>> 	(3) NT_PPC_TM_CFPR
>>>> 	(4) NT_PPC_TM_CVMX
>>>
>>> Sorry that I couldn't tell this from the code, but, what does the
>>> kernel return when the ptracer requests these registers and the
>>> program is not in a transaction?  Specifically I'm wondering whether
>>> this follows the same semantics as the s390 port.
>>>
>>
>> Right now, it still returns the saved state of the registers from thread
>> struct. I had assumed that the user must know the state of the transaction
>> before initiating the ptrace request. I guess its better to check for
>> the transaction status before processing the request. In case if TM is not
>> active on that thread, we should return -EINVAL.
> 
> I think s390 returns ENODATA in that case.
> 
>  https://sourceware.org/ml/gdb-patches/2013-06/msg00273.html
> 
> We'll want some way to tell whether the system actually
> supports this.  That could be ENODATA vs something-else (EINVAL
> or perhaps better EIO for "request is invalid").

As Mickey has pointed out, the transaction memory support in the system can be
checked from the HWCAP2 flags. So when the transaction is not active, we will
return ENODATA instead for TM related ptrace regset requests.

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Pedro Alves]

On 05/15/2014 09:25 AM, Anshuman Khandual wrote:
> On 05/14/2014 04:45 PM, Pedro Alves wrote:
>> On 05/14/14 06:46, Anshuman Khandual wrote:
>>> On 05/13/2014 10:43 PM, Pedro Alves wrote:
>>>> On 05/05/14 08:54, Anshuman Khandual wrote:
>>>>> This patch enables get and set of transactional memory related register
>>>>> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
>>>>> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
>>>>> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
>>>>> ELF core note types added previously in this regard.
>>>>>
>>>>> 	(1) NT_PPC_TM_SPR
>>>>> 	(2) NT_PPC_TM_CGPR
>>>>> 	(3) NT_PPC_TM_CFPR
>>>>> 	(4) NT_PPC_TM_CVMX
>>>>
>>>> Sorry that I couldn't tell this from the code, but, what does the
>>>> kernel return when the ptracer requests these registers and the
>>>> program is not in a transaction?  Specifically I'm wondering whether
>>>> this follows the same semantics as the s390 port.
>>>>
>>>
>>> Right now, it still returns the saved state of the registers from thread
>>> struct. I had assumed that the user must know the state of the transaction
>>> before initiating the ptrace request. I guess its better to check for
>>> the transaction status before processing the request. In case if TM is not
>>> active on that thread, we should return -EINVAL.
>>
>> I think s390 returns ENODATA in that case.
>>
>>  https://sourceware.org/ml/gdb-patches/2013-06/msg00273.html
>>
>> We'll want some way to tell whether the system actually
>> supports this.  That could be ENODATA vs something-else (EINVAL
>> or perhaps better EIO for "request is invalid").
> 
> As Mickey has pointed out, the transaction memory support in the system can be
> checked from the HWCAP2 flags. So when the transaction is not active, we will
> return ENODATA instead for TM related ptrace regset requests.

Returning ENODATA when the transaction is not active, like
s390 is great.  Thank you.

But I think it's worth it to consider what should the kernel
return when the machine doesn't have these registers at all.

Sure, for this case we happen to have the hwcap flag.  But in
general, I don't know whether we will always have a hwcap bit
for each register set that is added.  Maybe we will, so that
the info ends up in core dumps.

Still, I think it's worth to consider this case in the
general sense, irrespective of hwcap.

That is, what should PTRACE_GETREGSET/PTRACE_SETREGSET return
when the machine doesn't have the registers at all.  We shouldn't
need to consult something elsewhere (like hwcap) to determine
what ENODATA means.  The kernel knows it right there.  I think
s390 goofed here.

Taking a look at x86, for example, we see:

	[REGSET_XSTATE] = {
		.core_note_type = NT_X86_XSTATE,
		.size = sizeof(u64), .align = sizeof(u64),
		.active = xstateregs_active, .get = xstateregs_get,
		.set = xstateregs_set
	},

Note that it installs the ".active" hook.

 24 /**
 25  * user_regset_active_fn - type of @active function in &struct user_regset
 26  * @target:     thread being examined
 27  * @regset:     regset being examined
 28  *
 29  * Return -%ENODEV if not available on the hardware found.
 30  * Return %0 if no interesting state in this thread.
 31  * Return >%0 number of @size units of interesting state.
 32  * Any get call fetching state beyond that number will
 33  * see the default initialization state for this data,
 34  * so a caller that knows what the default state is need
 35  * not copy it all out.
 36  * This call is optional; the pointer is %NULL if there
 37  * is no inexpensive check to yield a value < @n.
 38  */
 39 typedef int user_regset_active_fn(struct task_struct *target,
 40                                   const struct user_regset *regset);
 41

Note the mention of ENODEV.

I couldn't actually find any arch that currently returns -ENODEV in
the "active" hook.  I see that binfmt_elf.c doesn't handle
regset->active() returning < 0.  Guess that may be why.  Looks like
something that could be cleaned up, to me.

Anyway, notice x86's REGSET_XSTATE regset->get hook:

int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
		unsigned int pos, unsigned int count,
		void *kbuf, void __user *ubuf)
{
	int ret;

	if (!cpu_has_xsave)
		return -ENODEV;
        ^^^^^^^^^^^^^^^^^^^^^^

And then we see that xfpregs_get has a similar ENODEV case.

So in sum, it very much looks like the intention is for
PTRACE_GETREGSET/PTRACE_SETREGSET to return ENODEV in the
case the regset doesn't exist on the running machine, and then
it looks like at least x86 works that way.

-- 
Pedro Alves

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Michael Neuling]

> So in sum, it very much looks like the intention is for
> PTRACE_GETREGSET/PTRACE_SETREGSET to return ENODEV in the
> case the regset doesn't exist on the running machine, and then
> it looks like at least x86 works that way.

Good point... agreed.  We should ENODEV when we don't have TM hardware
(like POWER7).

Mikey

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Anshuman Khandual]

On 05/15/2014 05:38 PM, Pedro Alves wrote:
> On 05/15/2014 09:25 AM, Anshuman Khandual wrote:
>> On 05/14/2014 04:45 PM, Pedro Alves wrote:
>>> On 05/14/14 06:46, Anshuman Khandual wrote:
>>>> On 05/13/2014 10:43 PM, Pedro Alves wrote:
>>>>> On 05/05/14 08:54, Anshuman Khandual wrote:
>>>>>> This patch enables get and set of transactional memory related register
>>>>>> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
>>>>>> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
>>>>>> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
>>>>>> ELF core note types added previously in this regard.
>>>>>>
>>>>>> 	(1) NT_PPC_TM_SPR
>>>>>> 	(2) NT_PPC_TM_CGPR
>>>>>> 	(3) NT_PPC_TM_CFPR
>>>>>> 	(4) NT_PPC_TM_CVMX
>>>>>
>>>>> Sorry that I couldn't tell this from the code, but, what does the
>>>>> kernel return when the ptracer requests these registers and the
>>>>> program is not in a transaction?  Specifically I'm wondering whether
>>>>> this follows the same semantics as the s390 port.
>>>>>
>>>>
>>>> Right now, it still returns the saved state of the registers from thread
>>>> struct. I had assumed that the user must know the state of the transaction
>>>> before initiating the ptrace request. I guess its better to check for
>>>> the transaction status before processing the request. In case if TM is not
>>>> active on that thread, we should return -EINVAL.
>>>
>>> I think s390 returns ENODATA in that case.
>>>
>>>  https://sourceware.org/ml/gdb-patches/2013-06/msg00273.html
>>>
>>> We'll want some way to tell whether the system actually
>>> supports this.  That could be ENODATA vs something-else (EINVAL
>>> or perhaps better EIO for "request is invalid").
>>
>> As Mickey has pointed out, the transaction memory support in the system can be
>> checked from the HWCAP2 flags. So when the transaction is not active, we will
>> return ENODATA instead for TM related ptrace regset requests.
> 
> Returning ENODATA when the transaction is not active, like
> s390 is great.  Thank you.
> 
> But I think it's worth it to consider what should the kernel
> return when the machine doesn't have these registers at all.
> 
> Sure, for this case we happen to have the hwcap flag.  But in
> general, I don't know whether we will always have a hwcap bit
> for each register set that is added.  Maybe we will, so that
> the info ends up in core dumps.
> 
> Still, I think it's worth to consider this case in the
> general sense, irrespective of hwcap.
> 
> That is, what should PTRACE_GETREGSET/PTRACE_SETREGSET return
> when the machine doesn't have the registers at all.  We shouldn't
> need to consult something elsewhere (like hwcap) to determine
> what ENODATA means.  The kernel knows it right there.  I think
> s390 goofed here.
> 
> Taking a look at x86, for example, we see:
> 
> 	[REGSET_XSTATE] = {
> 		.core_note_type = NT_X86_XSTATE,
> 		.size = sizeof(u64), .align = sizeof(u64),
> 		.active = xstateregs_active, .get = xstateregs_get,
> 		.set = xstateregs_set
> 	},
> 
> Note that it installs the ".active" hook.
> 
>  24 /**
>  25  * user_regset_active_fn - type of @active function in &struct user_regset
>  26  * @target:     thread being examined
>  27  * @regset:     regset being examined
>  28  *
>  29  * Return -%ENODEV if not available on the hardware found.
>  30  * Return %0 if no interesting state in this thread.
>  31  * Return >%0 number of @size units of interesting state.
>  32  * Any get call fetching state beyond that number will
>  33  * see the default initialization state for this data,
>  34  * so a caller that knows what the default state is need
>  35  * not copy it all out.
>  36  * This call is optional; the pointer is %NULL if there
>  37  * is no inexpensive check to yield a value < @n.
>  38  */
>  39 typedef int user_regset_active_fn(struct task_struct *target,
>  40                                   const struct user_regset *regset);
>  41
> 
> Note the mention of ENODEV.
> 
> I couldn't actually find any arch that currently returns -ENODEV in
> the "active" hook.  I see that binfmt_elf.c doesn't handle
> regset->active() returning < 0.  Guess that may be why.  Looks like
> something that could be cleaned up, to me.
> 
> Anyway, notice x86's REGSET_XSTATE regset->get hook:
> 
> int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
> 		unsigned int pos, unsigned int count,
> 		void *kbuf, void __user *ubuf)
> {
> 	int ret;
> 
> 	if (!cpu_has_xsave)
> 		return -ENODEV;
>         ^^^^^^^^^^^^^^^^^^^^^^
> 
> And then we see that xfpregs_get has a similar ENODEV case.
> 
> So in sum, it very much looks like the intention is for
> PTRACE_GETREGSET/PTRACE_SETREGSET to return ENODEV in the
> case the regset doesn't exist on the running machine, and then
> it looks like at least x86 works that way.
> 

Will work on these suggestions and post it again. Thanks for the
detailed insights and review.

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Anshuman Khandual]

On 05/15/2014 05:38 PM, Pedro Alves wrote:
> On 05/15/2014 09:25 AM, Anshuman Khandual wrote:
>> On 05/14/2014 04:45 PM, Pedro Alves wrote:
>>> On 05/14/14 06:46, Anshuman Khandual wrote:
>>>> On 05/13/2014 10:43 PM, Pedro Alves wrote:
>>>>> On 05/05/14 08:54, Anshuman Khandual wrote:
>>>>>> This patch enables get and set of transactional memory related register
>>>>>> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
>>>>>> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
>>>>>> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
>>>>>> ELF core note types added previously in this regard.
>>>>>>
>>>>>> 	(1) NT_PPC_TM_SPR
>>>>>> 	(2) NT_PPC_TM_CGPR
>>>>>> 	(3) NT_PPC_TM_CFPR
>>>>>> 	(4) NT_PPC_TM_CVMX
>>>>>
>>>>> Sorry that I couldn't tell this from the code, but, what does the
>>>>> kernel return when the ptracer requests these registers and the
>>>>> program is not in a transaction?  Specifically I'm wondering whether
>>>>> this follows the same semantics as the s390 port.
>>>>>
>>>>
>>>> Right now, it still returns the saved state of the registers from thread
>>>> struct. I had assumed that the user must know the state of the transaction
>>>> before initiating the ptrace request. I guess its better to check for
>>>> the transaction status before processing the request. In case if TM is not
>>>> active on that thread, we should return -EINVAL.
>>>
>>> I think s390 returns ENODATA in that case.
>>>
>>>  https://sourceware.org/ml/gdb-patches/2013-06/msg00273.html
>>>
>>> We'll want some way to tell whether the system actually
>>> supports this.  That could be ENODATA vs something-else (EINVAL
>>> or perhaps better EIO for "request is invalid").
>>
>> As Mickey has pointed out, the transaction memory support in the system can be
>> checked from the HWCAP2 flags. So when the transaction is not active, we will
>> return ENODATA instead for TM related ptrace regset requests.
> 
> Returning ENODATA when the transaction is not active, like
> s390 is great.  Thank you.
> 
> But I think it's worth it to consider what should the kernel
> return when the machine doesn't have these registers at all.
> 
> Sure, for this case we happen to have the hwcap flag.  But in
> general, I don't know whether we will always have a hwcap bit
> for each register set that is added.  Maybe we will, so that
> the info ends up in core dumps.
> 
> Still, I think it's worth to consider this case in the
> general sense, irrespective of hwcap.
> 
> That is, what should PTRACE_GETREGSET/PTRACE_SETREGSET return
> when the machine doesn't have the registers at all.  We shouldn't
> need to consult something elsewhere (like hwcap) to determine
> what ENODATA means.  The kernel knows it right there.  I think
> s390 goofed here.
> 
> Taking a look at x86, for example, we see:
> 
> 	[REGSET_XSTATE] = {
> 		.core_note_type = NT_X86_XSTATE,
> 		.size = sizeof(u64), .align = sizeof(u64),
> 		.active = xstateregs_active, .get = xstateregs_get,
> 		.set = xstateregs_set
> 	},
> 
> Note that it installs the ".active" hook.
> 
>  24 /**
>  25  * user_regset_active_fn - type of @active function in &struct user_regset
>  26  * @target:     thread being examined
>  27  * @regset:     regset being examined
>  28  *
>  29  * Return -%ENODEV if not available on the hardware found.
>  30  * Return %0 if no interesting state in this thread.
>  31  * Return >%0 number of @size units of interesting state.
>  32  * Any get call fetching state beyond that number will
>  33  * see the default initialization state for this data,
>  34  * so a caller that knows what the default state is need
>  35  * not copy it all out.
>  36  * This call is optional; the pointer is %NULL if there
>  37  * is no inexpensive check to yield a value < @n.
>  38  */
>  39 typedef int user_regset_active_fn(struct task_struct *target,
>  40                                   const struct user_regset *regset);
>  41
> 
> Note the mention of ENODEV.
> 
> I couldn't actually find any arch that currently returns -ENODEV in
> the "active" hook.  I see that binfmt_elf.c doesn't handle
> regset->active() returning < 0.  Guess that may be why.  Looks like
> something that could be cleaned up, to me.
>

Also it does not consider the return value of regset->active(t->task, regset)
(whose objective is to figure out whether we need to request regset->n number
of elements or less than that) in the subsequent call to regset->get function.

> Anyway, notice x86's REGSET_XSTATE regset->get hook:
> 
> int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
> 		unsigned int pos, unsigned int count,
> 		void *kbuf, void __user *ubuf)
> {
> 	int ret;
> 
> 	if (!cpu_has_xsave)
> 		return -ENODEV;
>         ^^^^^^^^^^^^^^^^^^^^^^
> 
> And then we see that xfpregs_get has a similar ENODEV case.
> 
> So in sum, it very much looks like the intention is for
> PTRACE_GETREGSET/PTRACE_SETREGSET to return ENODEV in the
> case the regset doesn't exist on the running machine, and then
> it looks like at least x86 works that way.

Okay. Looks like for all the "get/set" hooks I have added for the brand new regsets,
we need to implement ENODATA error condition as that of s390 when TM is not active
on the thread in target and implement ENODEV error condition as that of x86 when
TM supports is not at all available on the system. So the code snippet which should
be added to all the new "get/set" functions will be something like this.

+       if (!cpu_has_feature(CPU_FTR_TM))
+               return -ENODEV;
+
+       if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+               return -ENODATA;


Now coming to the installation of the .active hooks part for all the new regsets, it
should be pretty straight forward as well. Though its optional and used for elf_core_dump
purpose only, its worth adding them here. Example of an active function should be something
like this. The function is inexpensive as required.

+static int tm_spr_active(struct task_struct *target,
+                               const struct user_regset *regset)
+{
+       if (!cpu_has_feature(CPU_FTR_TM))
+               return -ENODEV;
+
+       if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+               return 0;
+
+       return regset->n;
+}

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Pedro Alves]

On 05/19/2014 12:46 PM, Anshuman Khandual wrote:

>> > I couldn't actually find any arch that currently returns -ENODEV in
>> > the "active" hook.  I see that binfmt_elf.c doesn't handle
>> > regset->active() returning < 0.  Guess that may be why.  Looks like
>> > something that could be cleaned up, to me.
>> >
> Also it does not consider the return value of regset->active(t->task, regset)
> (whose objective is to figure out whether we need to request regset->n number
> of elements or less than that) in the subsequent call to regset->get function.

Indeed.

TBC, do you plan on fixing this?  Otherwise ...

> Now coming to the installation of the .active hooks part for all the new regsets, it
> should be pretty straight forward as well. Though its optional and used for elf_core_dump
> purpose only, its worth adding them here. Example of an active function should be something
> like this. The function is inexpensive as required.
> 
> +static int tm_spr_active(struct task_struct *target,
> +                               const struct user_regset *regset)
> +{
> +       if (!cpu_has_feature(CPU_FTR_TM))
> +               return -ENODEV;

... unfortunately this will do the wrong thing.

Thanks,
-- 
Pedro Alves

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Anshuman Khandual]

On 05/19/2014 08:13 PM, Pedro Alves wrote:
> On 05/19/2014 12:46 PM, Anshuman Khandual wrote:
> 
>>>> I couldn't actually find any arch that currently returns -ENODEV in
>>>> the "active" hook.  I see that binfmt_elf.c doesn't handle
>>>> regset->active() returning < 0.  Guess that may be why.  Looks like
>>>> something that could be cleaned up, to me.
>>>>
>> Also it does not consider the return value of regset->active(t->task, regset)
>> (whose objective is to figure out whether we need to request regset->n number
>> of elements or less than that) in the subsequent call to regset->get function.
> 
> Indeed.
> 
> TBC, do you plan on fixing this?  Otherwise ...

Sure, thinking something like this as mentioned below. But still not sure how to use
the return type of -ENODEV from the function regset->active(). Right now if any
regset does have the active hook and it returns anything but positive value, it will
be ignored and the control moves to the next regset in view. This prevents the thread
core note type being written to the core dump.

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index aa3cb62..80672fb 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1553,7 +1553,15 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
                if (regset->core_note_type && regset->get &&
                    (!regset->active || regset->active(t->task, regset))) {
                        int ret;
-                       size_t size = regset->n * regset->size;
+                       size_t size;
+
+                       /* Request only the active elements in the regset */
+                       if (!regset->active)
+                               size = regset->n * regset->size;
+                       else
+                               size = regset->active(t->task, regset)
+                                                               * regset->size;
+
                        void *data = kmalloc(size, GFP_KERNEL);
                        if (unlikely(!data))
                                return 0;

> 
>> Now coming to the installation of the .active hooks part for all the new regsets, it
>> should be pretty straight forward as well. Though its optional and used for elf_core_dump
>> purpose only, its worth adding them here. Example of an active function should be something
>> like this. The function is inexpensive as required.
>>
>> +static int tm_spr_active(struct task_struct *target,
>> +                               const struct user_regset *regset)
>> +{
>> +       if (!cpu_has_feature(CPU_FTR_TM))
>> +               return -ENODEV;
> 
> ... unfortunately this will do the wrong thing.

I am not sure whether I understand this correctly. Are you saying that its wrong to return
-ENODEV in this case as above ?

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Pedro Alves]

On 05/20/2014 09:14 AM, Anshuman Khandual wrote:
> On 05/19/2014 08:13 PM, Pedro Alves wrote:
>> On 05/19/2014 12:46 PM, Anshuman Khandual wrote:
>>
>>>>> I couldn't actually find any arch that currently returns -ENODEV in
>>>>> the "active" hook.  I see that binfmt_elf.c doesn't handle
>>>>> regset->active() returning < 0.  Guess that may be why.  Looks like
>>>>> something that could be cleaned up, to me.
>>>>>
>>> Also it does not consider the return value of regset->active(t->task, regset)
>>> (whose objective is to figure out whether we need to request regset->n number
>>> of elements or less than that) in the subsequent call to regset->get function.
>>
>> Indeed.
>>
>> TBC, do you plan on fixing this?  Otherwise ...
> 
> Sure, thinking something like this as mentioned below. But still not sure how to use
> the return type of -ENODEV from the function regset->active(). Right now if any
> regset does have the active hook and it returns anything but positive value, it will
> be ignored and the control moves to the next regset in view. This prevents the thread
> core note type being written to the core dump.

Looks to me that that's exactly what should happen for -ENODEV too.  The regset
should be ignored.  If regset->active() returns -ENODEV, then the machine
doesn't have the registers at all, so what makes sense to me is to not write the
corresponding core note in the dump.  IOW, on such a machine, the kernel
generates a core exactly like if the support for these registers that don't
make sense for this machine wasn't compiled in at all.  And generates a core
exactly like an older kernel that didn't know about that regset
(which is fine for that same machine) yet.

> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index aa3cb62..80672fb 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1553,7 +1553,15 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
>                 if (regset->core_note_type && regset->get &&
>                     (!regset->active || regset->active(t->task, regset))) {
>                         int ret;

So, here, this ?

                    (!regset->active || regset->active(t->task, regset) > 0)) {


> -                       size_t size = regset->n * regset->size;
> +                       size_t size;
> +
> +                       /* Request only the active elements in the regset */
> +                       if (!regset->active)
> +                               size = regset->n * regset->size;
> +                       else
> +                               size = regset->active(t->task, regset)
> +                                                               * regset->size;
> +


I wonder if it wouldn't be cleaner to add a function like:

int
regset_active (tast *task, regseg *regset)
{
   if (!regset->active)
        return regset->n * regset->size;
   else
        return regset->active(task, regset);
}

And then use it like

               if (regset->core_note_type && regset->get) {
                   int size = regset_active (t->task, regset);

                   if (size > 0) {
                          ...
                   }

Though at this point, we don't actually make use of
the distinction between -ENODEV vs 0.  Guess that's what
we should be thinking about.  Seems like there some details that
need to be sorted out, and some verification that consumers aren't
broken by outputting smaller notes -- e.g., ia64 makes me
wonder that.

Maybe we should leave this for another day, and have tm_spr_active
return 0 instead of -ENODEV when the machine doesn't have the hardware,
or not install that hook at all.  Seems like the effect will be the same,
as the note isn't output if ->get fails.

>                         void *data = kmalloc(size, GFP_KERNEL);
>                         if (unlikely(!data))
>                                 return 0;
> 
>>
>>> Now coming to the installation of the .active hooks part for all the new regsets, it
>>> should be pretty straight forward as well. Though its optional and used for elf_core_dump
>>> purpose only, its worth adding them here. Example of an active function should be something
>>> like this. The function is inexpensive as required.
>>>
>>> +static int tm_spr_active(struct task_struct *target,
>>> +                               const struct user_regset *regset)
>>> +{
>>> +       if (!cpu_has_feature(CPU_FTR_TM))
>>> +               return -ENODEV;
>>
>> ... unfortunately this will do the wrong thing.
> 
> I am not sure whether I understand this correctly. Are you saying that its wrong to return
> -ENODEV in this case as above ?

No, sorry for not being clear.  The (...)'s were connected:

   "do you plan on fixing this?  Otherwise ... ... unfortunately
    this will do the wrong thing."

-- 
Pedro Alves

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Anshuman Khandual]

On 05/20/2014 04:03 PM, Pedro Alves wrote:
> On 05/20/2014 09:14 AM, Anshuman Khandual wrote:
>> On 05/19/2014 08:13 PM, Pedro Alves wrote:
>>> On 05/19/2014 12:46 PM, Anshuman Khandual wrote:
>>>
>>>>>> I couldn't actually find any arch that currently returns -ENODEV in
>>>>>> the "active" hook.  I see that binfmt_elf.c doesn't handle
>>>>>> regset->active() returning < 0.  Guess that may be why.  Looks like
>>>>>> something that could be cleaned up, to me.
>>>>>>
>>>> Also it does not consider the return value of regset->active(t->task, regset)
>>>> (whose objective is to figure out whether we need to request regset->n number
>>>> of elements or less than that) in the subsequent call to regset->get function.
>>>
>>> Indeed.
>>>
>>> TBC, do you plan on fixing this?  Otherwise ...
>>
>> Sure, thinking something like this as mentioned below. But still not sure how to use
>> the return type of -ENODEV from the function regset->active(). Right now if any
>> regset does have the active hook and it returns anything but positive value, it will
>> be ignored and the control moves to the next regset in view. This prevents the thread
>> core note type being written to the core dump.
> 
> Looks to me that that's exactly what should happen for -ENODEV too.  The regset
> should be ignored.  If regset->active() returns -ENODEV, then the machine
> doesn't have the registers at all, so what makes sense to me is to not write the
> corresponding core note in the dump.  IOW, on such a machine, the kernel
> generates a core exactly like if the support for these registers that don't
> make sense for this machine wasn't compiled in at all.  And generates a core
> exactly like an older kernel that didn't know about that regset
> (which is fine for that same machine) yet.
> 

All of this happen right now even without specifically checking for the return type
of -ENODEV and just checking for a positive value. I guess thats the reason they had
omitted -ENODEV in the first place. 

 
>>
>> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
>> index aa3cb62..80672fb 100644
>> --- a/fs/binfmt_elf.c
>> +++ b/fs/binfmt_elf.c
>> @@ -1553,7 +1553,15 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
>>                 if (regset->core_note_type && regset->get &&
>>                     (!regset->active || regset->active(t->task, regset))) {
>>                         int ret;
> 
> So, here, this ?
> 
>                     (!regset->active || regset->active(t->task, regset) > 0)) {
> 
> 
>> -                       size_t size = regset->n * regset->size;
>> +                       size_t size;
>> +
>> +                       /* Request only the active elements in the regset */
>> +                       if (!regset->active)
>> +                               size = regset->n * regset->size;
>> +                       else
>> +                               size = regset->active(t->task, regset)
>> +                                                               * regset->size;
>> +
> 
> 
> I wonder if it wouldn't be cleaner to add a function like:
> 
> int
> regset_active (tast *task, regseg *regset)
> {
>    if (!regset->active)
>         return regset->n * regset->size;
>    else
>         return regset->active(task, regset);
> }
> 
> And then use it like
> 
>                if (regset->core_note_type && regset->get) {
>                    int size = regset_active (t->task, regset);
> 
>                    if (size > 0) {
>                           ...
>                    }
> 

Yeah this makes sense.

> Though at this point, we don't actually make use of
> the distinction between -ENODEV vs 0.  Guess that's what
> we should be thinking about.  Seems like there some details that
> need to be sorted out, and some verification that consumers aren't
> broken by outputting smaller notes -- e.g., ia64 makes me
> wonder that.

I agree.

> 
> Maybe we should leave this for another day, and have tm_spr_active
> return 0 instead of -ENODEV when the machine doesn't have the hardware,
> or not install that hook at all.  Seems like the effect will be the same,
> as the note isn't output if ->get fails.

Agree. Active hooks which return 0 in case of -ENODEV sounds good to me and shall
incorporate this in the next version.

> 
>>                         void *data = kmalloc(size, GFP_KERNEL);
>>                         if (unlikely(!data))
>>                                 return 0;
>>
>>>
>>>> Now coming to the installation of the .active hooks part for all the new regsets, it
>>>> should be pretty straight forward as well. Though its optional and used for elf_core_dump
>>>> purpose only, its worth adding them here. Example of an active function should be something
>>>> like this. The function is inexpensive as required.
>>>>
>>>> +static int tm_spr_active(struct task_struct *target,
>>>> +                               const struct user_regset *regset)
>>>> +{
>>>> +       if (!cpu_has_feature(CPU_FTR_TM))
>>>> +               return -ENODEV;
>>>
>>> ... unfortunately this will do the wrong thing.
>>
>> I am not sure whether I understand this correctly. Are you saying that its wrong to return
>> -ENODEV in this case as above ?
> 
> No, sorry for not being clear.  The (...)'s were connected:
> 
>    "do you plan on fixing this?  Otherwise ... ... unfortunately
>     this will do the wrong thing."
> 

:)

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Michael Ellerman]

On Tue, 2014-05-13 at 18:21 +0100, Pedro Alves wrote:
> I wonder whether people are getting Roland's address from?
> 
> It's frequent that ptrace related patches end up CCed to
> roland@redhat.com, but, he's not been at Red Hat for a few years
> now.  Roland, do you still want to be CCed on ptrace-related
> issues?  If so, there's probably a script somewhere in the
> kernel that needs updating.  If not, well, it'd be good
> if it were updated anyway.  :-)

In MAINTAINERS I see:

PTRACE SUPPORT
M:	Roland McGrath <roland@redhat.com>
M:	Oleg Nesterov <oleg@redhat.com>
S:	Maintained


cheers

Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets

[Anshuman Khandual]

On 05/22/2014 10:38 AM, Anshuman Khandual wrote:
> I agree.
> 
>> > 
>> > Maybe we should leave this for another day, and have tm_spr_active
>> > return 0 instead of -ENODEV when the machine doesn't have the hardware,
>> > or not install that hook at all.  Seems like the effect will be the same,
>> > as the note isn't output if ->get fails.

> Agree. Active hooks which return 0 in case of -ENODEV sounds good to me and shall
> incorporate this in the next version.
> 

But from "user_regset_active_fn" definition point of view -ENODEV is the right thing
to do even if we dont use it specifically compared to the return value of 0.