diff mbox series

[RFC,v3,15/32] x86/fred: make unions for the cs and ss fields in struct pt_regs

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

Commit Message

Li, Xin3 Feb. 24, 2023, 7:01 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>
Signed-off-by: Xin Li <xin3.li@intel.com>
---
 arch/x86/entry/vsyscall/vsyscall_64.c |  2 +-
 arch/x86/include/asm/ptrace.h         | 36 ++++++++++++++++++++++++---
 arch/x86/kernel/process_64.c          |  2 +-
 3 files changed, 34 insertions(+), 6 deletions(-)

Comments

Li, Xin3 Feb. 24, 2023, 4:34 p.m. UTC | #1 
> > +	union {
> > +		unsigned long  csl; /* CS + any fields above it */
> 
> I guess that CSL here is supposed to mean cs long, but CSL (Current Stack Level) is a
> new term in the FRED spec which isn't this.

Good catch!

> 
> This causes changes such as the final hunk in patch 27 to read incorrectly, despite
> being technically correct.
> 
> cs_slot would be much clearer in code, but tbh, even cs_l would be better than the
> version without an underscore.

cs_slot sounds a good term unless someone comes up with a better one.

> 
> And obviously, whatever is done here should be mirrored for ss.

Probably ss_slot then.
  Xin
H. Peter Anvin March 1, 2023, 2:17 a.m. UTC | #2 
On February 24, 2023 8:34:36 AM PST, "Li, Xin3" <xin3.li@intel.com> wrote:
>> > +	union {
>> > +		unsigned long  csl; /* CS + any fields above it */
>> 
>> I guess that CSL here is supposed to mean cs long, but CSL (Current Stack Level) is a
>> new term in the FRED spec which isn't this.
>
>Good catch!
>
>> 
>> This causes changes such as the final hunk in patch 27 to read incorrectly, despite
>> being technically correct.
>> 
>> cs_slot would be much clearer in code, but tbh, even cs_l would be better than the
>> version without an underscore.
>
>cs_slot sounds a good term unless someone comes up with a better one.
>
>> 
>> And obviously, whatever is done here should be mirrored for ss.
>
>Probably ss_slot then.
>  Xin
>

I called it csx, for "cs extended". "csq" would work, too.
Li, Xin3 March 1, 2023, 3:17 a.m. UTC | #3 
> >> This causes changes such as the final hunk in patch 27 to read
> >> incorrectly, despite being technically correct.
> >>
> >> cs_slot would be much clearer in code, but tbh, even cs_l would be
> >> better than the version without an underscore.
> >
> >cs_slot sounds a good term unless someone comes up with a better one.
> >
> >>
> >> And obviously, whatever is done here should be mirrored for ss.
> >
> >Probably ss_slot then.
> >  Xin
> >
> 
> I called it csx, for "cs extended". "csq" would work, too.

csx and ssx then.
diff mbox series

Patch

diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
index 4af81df133ee..6349c818d20a 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..341e44847cc1 100644
--- a/arch/x86/include/asm/ptrace.h
+++ b/arch/x86/include/asm/ptrace.h
@@ -82,13 +82,41 @@  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  csl; /* CS + any fields above it */
+		struct __attribute__((__packed__)) {
+			unsigned short cs;  /* CS selector proper */
+			unsigned int current_stack_level: 2;
+			unsigned int __csl_resv1	: 6;
+			unsigned int interrupt_shadowed	: 1;
+			unsigned int software_initiated	: 1;
+			unsigned int __csl_resv2	: 2;
+			unsigned int nmi		: 1;
+			unsigned int __csl_resv3	: 3;
+			unsigned int __csl_resv4	: 32;
+		};
+	};
 	unsigned long flags;
 	unsigned long sp;
-	unsigned long ss;
-/* top of stack page */
+	union {
+		unsigned long  ssl; /* SS + any fields above it */
+		struct __attribute__((__packed__)) {
+			unsigned short ss;  /* SS selector proper */
+			unsigned int __ssl_resv1: 16;
+			unsigned int vector	: 8;
+			unsigned int __ssl_resv2: 8;
+			unsigned int type	: 4;
+			unsigned int __ssl_resv3: 4;
+			unsigned int enclv	: 1;
+			unsigned int long_mode	: 1;
+			unsigned int nested	: 1;
+			unsigned int __ssl_resv4: 1;
+			unsigned int instr_len	: 4;
+		};
+	};
 };
 
 #endif /* !__i386__ */
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 4e34b3b68ebd..57de166dc61c 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);