| Message ID | 20230914044805.301390-17-xin3.li@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: enable FRED for x86-64 | expand |
On 14.09.23 г. 7:47 ч., Xin Li wrote: > FRED defines additional information in the upper 48 bits of cs/ss > fields. Therefore add the information definitions into the pt_regs > structure. > > Specially introduce a new structure fred_ss to denote the FRED flags > above SS selector, which avoids FRED_SSX_ macros and makes the code > simpler and easier to read. > > Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com> > Tested-by: Shan Kang <shan.kang@intel.com> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: Xin Li <xin3.li@intel.com> > --- > > Changes since v9: > * Introduce a new structure fred_ss to denote the FRED flags above SS > selector, which avoids FRED_SSX_ macros and makes the code simpler > and easier to read (Thomas Gleixner). > * Use type u64 to define FRED bit fields instead of type unsigned int > (Thomas Gleixner). > > Changes since v8: > * Reflect stack frame definition changes from FRED spec 3.0 to 5.0. > * Use __packed instead of __attribute__((__packed__)) (Borislav Petkov). > * Put all comments above the members, like the rest of the file does > (Borislav Petkov). > > Changes since v3: > * Rename csl/ssl of the pt_regs structure to csx/ssx (x for extended) > (Andrew Cooper). > --- > arch/x86/include/asm/ptrace.h | 51 +++++++++++++++++++++++++++++++---- > 1 file changed, 46 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h > index f08ea073edd6..5786c8ca5f4c 100644 > --- a/arch/x86/include/asm/ptrace.h > +++ b/arch/x86/include/asm/ptrace.h > @@ -56,6 +56,25 @@ struct pt_regs { > > #else /* __i386__ */ > > +struct fred_ss { > + u64 ss : 16, // SS selector Is this structure conformant to the return state as described in FRED 5.0? — The stack segment of the interrupted context, 64 bits formatted as follows: • Bits 15:0 contain the SS selector. < - WE HAVE THIS • Bits 31:16 are not currently defined and will be zero until they are. < - MISSING hole? > + sti : 1, // STI state < - > + swevent : 1, // Set if syscall, sysenter or INT n > + nmi : 1, // Event is NMI type > + : 13, > + vector : 8, // Event vector > + : 8, > + type : 4, // Event type > + : 4, > + enclave : 1, // Event was incident to enclave execution > + lm : 1, // CPU was in long mode > + nested : 1, // Nested exception during FRED delivery > + // not set for #DF > + : 1, > + insnlen : 4; // The length of the instruction causing the event > + // Only set for INT0, INT1, INT3, INT n, SYSCALL > +}; // and SYSENTER. 0 otherwise. > + <Snip>
> > +struct fred_ss { > > + u64 ss : 16, // SS selector > > Is this structure conformant to the return state as described in FRED 5.0? > > — The stack segment of the interrupted context, 64 bits formatted as follows: > > • Bits 15:0 contain the SS selector. < - WE HAVE THIS > > • Bits 31:16 are not currently defined and will be zero until they are. Where did you download the FRED 5.0 spec from? Mine says bit 16 is sti, bit 17 for sw initiated events and bit 18 is NMI. I guess you have FRED 3.0 spec, no? > < - MISSING > hole? > > > + sti : 1, // STI state < - > > + swevent : 1, // Set if syscall, sysenter or INT n > > + nmi : 1, // Event is NMI type > > + : 13,
On 20.09.23 г. 20:23 ч., Li, Xin3 wrote: >>> +struct fred_ss { >>> + u64 ss : 16, // SS selector >> >> Is this structure conformant to the return state as described in FRED 5.0? >> >> — The stack segment of the interrupted context, 64 bits formatted as follows: >> >> • Bits 15:0 contain the SS selector. < - WE HAVE THIS >> >> • Bits 31:16 are not currently defined and will be zero until they are. > > Where did you download the FRED 5.0 spec from? > > Mine says bit 16 is sti, bit 17 for sw initiated events and bit 18 is NMI. > > I guess you have FRED 3.0 spec, no? Doh you are right, I was looking at the wrong version of the document .... sorry for the noise. > >> < - MISSING > hole? >> >>> + sti : 1, // STI state < - >>> + swevent : 1, // Set if syscall, sysenter or INT n >>> + nmi : 1, // Event is NMI type >>> + : 13, >
> > I guess you have FRED 3.0 spec, no? > Doh you are right, I was looking at the wrong version of the document .... sorry for > the noise. Actually I appreciate your review so much!
diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h index f08ea073edd6..5786c8ca5f4c 100644 --- a/arch/x86/include/asm/ptrace.h +++ b/arch/x86/include/asm/ptrace.h @@ -56,6 +56,25 @@ struct pt_regs { #else /* __i386__ */ +struct fred_ss { + u64 ss : 16, // SS selector + sti : 1, // STI state + swevent : 1, // Set if syscall, sysenter or INT n + nmi : 1, // Event is NMI type + : 13, + vector : 8, // Event vector + : 8, + type : 4, // Event type + : 4, + enclave : 1, // Event was incident to enclave execution + lm : 1, // CPU was in long mode + nested : 1, // Nested exception during FRED delivery + // not set for #DF + : 1, + insnlen : 4; // The length of the instruction causing the event + // Only set for INT0, INT1, INT3, INT n, SYSCALL +}; // and SYSENTER. 0 otherwise. + struct pt_regs { /* * C ABI says these regs are callee-preserved. They aren't saved on @@ -85,6 +104,12 @@ struct pt_regs { * - the syscall number (syscall, sysenter, int80) * - error_code stored by the CPU on traps and exceptions * - the interrupt number for device interrupts + * + * A FRED stack frame starts here: + * 1) It _always_ includes an error code; + * + * 2) The return frame for ERET[US] starts here, but + * the content of orig_ax is ignored. */ unsigned long orig_ax; @@ -92,20 +117,36 @@ struct pt_regs { unsigned long ip; union { - u64 csx; // The full 64-bit data slot containing CS - u16 cs; // CS selector + u64 csx; // The full data for FRED + /* + * The 'cs' member should be defined as a 16-bit bit-field + * along with the 'sl' and 'wfe' members, which however + * breaks compiling REG_OFFSET_NAME(cs), because compilers + * disallow calculating the address of a bit-field. + * + * Therefore 'cs" is defined as an individual member with + * type u16. + */ + u16 cs; // CS selector + u64 : 16, + sl : 2, // Stack level at event time + wfe : 1, // IBT is in WAIT_FOR_BRANCH_STATE + : 45; }; unsigned long flags; unsigned long sp; union { - u64 ssx; // The full 64-bit data slot containing SS - u16 ss; // SS selector + u64 ssx; // The full data for FRED + u16 ss; // SS selector + struct fred_ss fred_ss; // The fred extension }; /* - * Top of stack on IDT systems. + * Top of stack on IDT systems, while FRED systems have extra fields + * defined above for storing exception related information, e.g. CR2 or + * DR6. */ };