On Wed, 2018-07-11 at 12:20 +0200, Ingo Molnar wrote:

* Yu-cheng Yu [off-list ref] wrote:

quoted

Add PTRACE interface for CET MSRs.

Please *always* describe new ABIs in the changelog, in a precise,
well-documented 
way.

Ok!

quoted

diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index e2ee403865eb..ac2bc3a18427 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c

@@ -49,7 +49,9 @@ enum x86_regset {

 	REGSET_IOPERM64 = REGSET_XFP,
 	REGSET_XSTATE,
 	REGSET_TLS,
+	REGSET_CET64 = REGSET_TLS,
 	REGSET_IOPERM32,
+	REGSET_CET32,
 };

Why does REGSET_CET64 alias on REGSET_TLS?

In x86_64_regsets[], there is no [REGSET_TLS].  The core dump code
cannot handle holes in the array.

quoted

 struct pt_regs_offset {

@@ -1276,6 +1278,13 @@ static struct user_regset x86_64_regsets[]

__ro_after_init = {
 		.size = sizeof(long), .align = sizeof(long),
 		.active = ioperm_active, .get = ioperm_get
 	},
+	[REGSET_CET64] = {
+		.core_note_type = NT_X86_CET,
+		.n = sizeof(struct cet_user_state) / sizeof(u64),
+		.size = sizeof(u64), .align = sizeof(u64),
+		.active = cetregs_active, .get = cetregs_get,
+		.set = cetregs_set
+	},

Ok, could we first please make this part of the regset code more
readable and 
start the series with a standalone clean-up patch that changes these
initializers 
to something more readable:

	[REGSET_CET64] = {
		.core_note_type	= NT_X86_CET,
		.n		= sizeof(struct cet_user_state) /
sizeof(u64),
		.size		= sizeof(u64),
		.align		= sizeof(u64),
		.active		= cetregs_active,
		.get		= cetregs_get,
		.set		= cetregs_set
	},

? (I'm demonstrating the cleanup based on REGSET_CET64, but this
should be done on 
every other entry first.)

I will fix it.

quoted

--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h

@@ -401,6 +401,7 @@ typedef struct elf64_shdr {

 #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 */
+#define NT_X86_CET	0x203		/* x86 cet state */

Acronyms in comments should be in capital letters.

Also, I think I asked this before: why does "Control Flow
Enforcement" abbreviate 
to "CET" (which is a well-known acronym for "Central European Time"),
not to CFE?

I don't know if I can change that, will find out.

Thanks,
Yu-cheng