On Wed, 2018-07-11 at 12:20 +0200, Ingo Molnar wrote:
> * Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> wrote:
>
> >
> > Add PTRACE interface for CET MSRs.
> Please *always* describe new ABIs in the changelog, in a precise,
> well-documented
> way.
Ok!
> >
> > 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.
>
> >
> > 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.
>
> >
> > --- 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
