diff mbox series

[v8,02/33] x86/fred: make unions for the cs and ss fields in struct pt_regs

Message ID 20230410081438.1750-3-xin3.li@intel.com
State New
Headers
Series x86: enable FRED for x86-64 |

Commit Message

Li, Xin3 April 10, 2023, 8:14 a.m. UTC 
  From: "H. Peter Anvin (Intel)" <hpa@zytor.com>

Make the cs and ss fields in struct pt_regs unions between the actual
selector and the unsigned long stack slot. FRED uses this space to
store additional flags.

The printk changes are simply due to the cs and ss fields changed to
unsigned short from unsigned long.

Signed-off-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Tested-by: Shan Kang <shan.kang@intel.com>
Signed-off-by: Xin Li <xin3.li@intel.com>
---

Changes since v3:
* Rename csl/ssl of the pt_regs structure to csx/ssx (x for extended)
  (Andrew Cooper).
---
 arch/x86/entry/vsyscall/vsyscall_64.c |  2 +-
 arch/x86/include/asm/ptrace.h         | 34 ++++++++++++++++++++++++---
 arch/x86/kernel/process_64.c          |  2 +-
 3 files changed, 33 insertions(+), 5 deletions(-)

Comments

Borislav Petkov June 3, 2023, 9:48 a.m. UTC | #1 
On Mon, Apr 10, 2023 at 01:14:07AM -0700, Xin Li wrote:
> diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
> index f4db78b09c8f..2abb23e6c1e2 100644
> --- a/arch/x86/include/asm/ptrace.h
> +++ b/arch/x86/include/asm/ptrace.h
> @@ -82,12 +82,40 @@ struct pt_regs {
>   * On hw interrupt, it's IRQ number:
>   */
>  	unsigned long orig_ax;
> -/* Return frame for iretq */
> +/* Return frame for iretq/eretu/erets */
>  	unsigned long ip;
> -	unsigned long cs;
> +	union {
> +		unsigned long  csx;	/* cs extended: CS + any fields above it */
> +		struct __attribute__((__packed__)) {

diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
index 2abb23e6c1e2..d850d3072263 100644
--- a/arch/x86/include/asm/ptrace.h
+++ b/arch/x86/include/asm/ptrace.h
@@ -86,7 +86,7 @@ struct pt_regs {
 	unsigned long ip;
 	union {
 		unsigned long  csx;	/* cs extended: CS + any fields above it */
-		struct __attribute__((__packed__)) {
+		struct {
 			unsigned short cs;	/* CS selector proper */
 			unsigned int current_stack_level: 2;
 			unsigned int __csx_resv1	: 6;
@@ -96,13 +96,13 @@ struct pt_regs {
 			unsigned int nmi		: 1;
 			unsigned int __csx_resv3	: 3;
 			unsigned int __csx_resv4	: 32;
-		};
+		} __packed;
 	};
 	unsigned long flags;
 	unsigned long sp;
 	union {
 		unsigned long  ssx;	/* ss extended: SS + any fields above it */
-		struct __attribute__((__packed__)) {
+		struct {
 			unsigned short ss;	/* SS selector proper */
 			unsigned int __ssx_resv1	: 16;
 			unsigned int vector		: 8;
@@ -114,7 +114,7 @@ struct pt_regs {
 			unsigned int nested		: 1;
 			unsigned int __ssx_resv4	: 1;
 			unsigned int instr_len		: 4;
-		};
+		} __packed;
 	};
 /* top of stack page */
 };

> +			unsigned short cs;	/* CS selector proper */

Also, please put all those comments above the members, like the rest of the
file does.
Thomas Gleixner June 5, 2023, 12:07 p.m. UTC | #2 
On Mon, Apr 10 2023 at 01:14, Xin Li wrote:
> +	union {
> +		unsigned long  csx;	/* cs extended: CS + any fields above it */
> +		struct __attribute__((__packed__)) {
> +			unsigned short cs;	/* CS selector proper */
> +			unsigned int current_stack_level: 2;
> +			unsigned int __csx_resv1	: 6;
> +			unsigned int interrupt_shadowed	: 1;
> +			unsigned int software_initiated	: 1;
> +			unsigned int __csx_resv2	: 2;
> +			unsigned int nmi		: 1;
> +			unsigned int __csx_resv3	: 3;
> +			unsigned int __csx_resv4	: 32;
> +		};
> +	};
>  	unsigned long flags;
>  	unsigned long sp;
> -	unsigned long ss;
> +	union {
> +		unsigned long  ssx;	/* ss extended: SS + any fields above it */
> +		struct __attribute__((__packed__)) {
> +			unsigned short ss;	/* SS selector proper */
> +			unsigned int __ssx_resv1	: 16;
> +			unsigned int vector		: 8;
> +			unsigned int __ssx_resv2	: 8;
> +			unsigned int type		: 4;
> +			unsigned int __ssx_resv3	: 4;
> +			unsigned int enclv		: 1;
> +			unsigned int long_mode		: 1;
> +			unsigned int nested		: 1;
> +			unsigned int __ssx_resv4	: 1;
> +			unsigned int instr_len		: 4;
> +		};
> +	};

This does not match section

    5.2.1 Saving Information on the Regular Stack?

of version 4 and later of the specification.

Thanks,

        tglx
H. Peter Anvin June 5, 2023, 5:12 p.m. UTC | #3 
> 
> This does not match section
> 
>      5.2.1 Saving Information on the Regular Stack?
> 
> of version 4 and later of the specification.
> 

Yes, this is correct. This patchset was posted in April, and once we got 
some very late requests for changes I asked Xin to hold off any further 
iterations until the spec had restabilized.

	-hpa
Thomas Gleixner June 5, 2023, 5:29 p.m. UTC | #4 
On Mon, Jun 05 2023 at 10:12, H. Peter Anvin wrote:
>> 
>> This does not match section
>> 
>>      5.2.1 Saving Information on the Regular Stack?
>> 
>> of version 4 and later of the specification.
>> 
>
> Yes, this is correct. This patchset was posted in April, and once we got 
> some very late requests for changes I asked Xin to hold off any further 
> iterations until the spec had restabilized.

I assumed that, but I stumbled over it when trying to review :)
diff mbox series

Patch

diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
index d234ca797e4a..2429ad0df068 100644
--- a/arch/x86/entry/vsyscall/vsyscall_64.c
+++ b/arch/x86/entry/vsyscall/vsyscall_64.c
@@ -76,7 +76,7 @@  static void warn_bad_vsyscall(const char *level, struct pt_regs *regs,
 	if (!show_unhandled_signals)
 		return;
 
-	printk_ratelimited("%s%s[%d] %s ip:%lx cs:%lx sp:%lx ax:%lx si:%lx di:%lx\n",
+	printk_ratelimited("%s%s[%d] %s ip:%lx cs:%x sp:%lx ax:%lx si:%lx di:%lx\n",
 			   level, current->comm, task_pid_nr(current),
 			   message, regs->ip, regs->cs,
 			   regs->sp, regs->ax, regs->si, regs->di);
diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
index f4db78b09c8f..2abb23e6c1e2 100644
--- a/arch/x86/include/asm/ptrace.h
+++ b/arch/x86/include/asm/ptrace.h
@@ -82,12 +82,40 @@  struct pt_regs {
  * On hw interrupt, it's IRQ number:
  */
 	unsigned long orig_ax;
-/* Return frame for iretq */
+/* Return frame for iretq/eretu/erets */
 	unsigned long ip;
-	unsigned long cs;
+	union {
+		unsigned long  csx;	/* cs extended: CS + any fields above it */
+		struct __attribute__((__packed__)) {
+			unsigned short cs;	/* CS selector proper */
+			unsigned int current_stack_level: 2;
+			unsigned int __csx_resv1	: 6;
+			unsigned int interrupt_shadowed	: 1;
+			unsigned int software_initiated	: 1;
+			unsigned int __csx_resv2	: 2;
+			unsigned int nmi		: 1;
+			unsigned int __csx_resv3	: 3;
+			unsigned int __csx_resv4	: 32;
+		};
+	};
 	unsigned long flags;
 	unsigned long sp;
-	unsigned long ss;
+	union {
+		unsigned long  ssx;	/* ss extended: SS + any fields above it */
+		struct __attribute__((__packed__)) {
+			unsigned short ss;	/* SS selector proper */
+			unsigned int __ssx_resv1	: 16;
+			unsigned int vector		: 8;
+			unsigned int __ssx_resv2	: 8;
+			unsigned int type		: 4;
+			unsigned int __ssx_resv3	: 4;
+			unsigned int enclv		: 1;
+			unsigned int long_mode		: 1;
+			unsigned int nested		: 1;
+			unsigned int __ssx_resv4	: 1;
+			unsigned int instr_len		: 4;
+		};
+	};
 /* top of stack page */
 };
 
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index bb65a68b4b49..a1aa74864c8b 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -116,7 +116,7 @@  void __show_regs(struct pt_regs *regs, enum show_regs_mode mode,
 
 	printk("%sFS:  %016lx(%04x) GS:%016lx(%04x) knlGS:%016lx\n",
 	       log_lvl, fs, fsindex, gs, gsindex, shadowgs);
-	printk("%sCS:  %04lx DS: %04x ES: %04x CR0: %016lx\n",
+	printk("%sCS:  %04x DS: %04x ES: %04x CR0: %016lx\n",
 		log_lvl, regs->cs, ds, es, cr0);
 	printk("%sCR2: %016lx CR3: %016lx CR4: %016lx\n",
 		log_lvl, cr2, cr3, cr4);