lkml.org 
[lkml]   [2018]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC PATCH v2 25/27] x86/cet: Add PTRACE interface for CET

* Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:

> Add PTRACE interface for CET MSRs.

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

> 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?

> 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.)


> --- 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?

Thanks,

Ingo

\
 
 \ /
  Last update: 2018-07-11 12:21    [W:0.222 / U:5.876 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site