[PATCH 0/5] x86: introduce read_sregs() and elf_core_save_regs() adjustments

Jan Beulich posted 5 patches 3 years, 6 months ago
Only 0 patches received!
[PATCH 0/5] x86: introduce read_sregs() and elf_core_save_regs() adjustments
Posted by Jan Beulich 3 years, 6 months ago
1: introduce read_sregs() to allow storing to memory directly
2: ELF: don't open-code read_sreg()
3: ELF: don't store function pointer in elf_core_save_regs()
4: ELF: also record FS/GS bases in elf_core_save_regs()
5: ELF: eliminate pointless local variable from elf_core_save_regs()

Jan

[PATCH 1/5] x86: introduce read_sregs() to allow storing to memory directly
Posted by Jan Beulich 3 years, 6 months ago
When storing all (data) segment registers in one go, prefer writing the
selector values directly to memory (as opposed to read_sreg()).

Also move the single register variant into the regs.h.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1703,10 +1703,7 @@ static void save_segments(struct vcpu *v
 {
     struct cpu_user_regs *regs = &v->arch.user_regs;
 
-    regs->ds = read_sreg(ds);
-    regs->es = read_sreg(es);
-    regs->fs = read_sreg(fs);
-    regs->gs = read_sreg(gs);
+    read_sregs(regs);
 
     if ( !is_pv_32bit_vcpu(v) )
     {
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -43,10 +43,7 @@ static void read_registers(struct cpu_us
     crs[2] = read_cr2();
     crs[3] = read_cr3();
     crs[4] = read_cr4();
-    regs->ds = read_sreg(ds);
-    regs->es = read_sreg(es);
-    regs->fs = read_sreg(fs);
-    regs->gs = read_sreg(gs);
+    read_sregs(regs);
     crs[5] = rdfsbase();
     crs[6] = rdgsbase();
     crs[7] = rdgsshadow();
--- a/xen/include/asm-x86/regs.h
+++ b/xen/include/asm-x86/regs.h
@@ -15,4 +15,18 @@
     (diff == 0);                                                              \
 })
 
+#define read_sreg(name) ({                                    \
+    unsigned int __sel;                                       \
+    asm volatile ( "mov %%" STR(name) ",%0" : "=r" (__sel) ); \
+    __sel;                                                    \
+})
+
+static inline void read_sregs(struct cpu_user_regs *regs)
+{
+    asm volatile ( "mov %%ds, %0" : "=m" (regs->ds) );
+    asm volatile ( "mov %%es, %0" : "=m" (regs->es) );
+    asm volatile ( "mov %%fs, %0" : "=m" (regs->fs) );
+    asm volatile ( "mov %%gs, %0" : "=m" (regs->gs) );
+}
+
 #endif /* __X86_REGS_H__ */
--- a/xen/include/asm-x86/system.h
+++ b/xen/include/asm-x86/system.h
@@ -5,12 +5,6 @@
 #include <xen/bitops.h>
 #include <asm/processor.h>
 
-#define read_sreg(name)                                         \
-({  unsigned int __sel;                                         \
-    asm volatile ( "mov %%" STR(name) ",%0" : "=r" (__sel) );   \
-    __sel;                                                      \
-})
-
 static inline void wbinvd(void)
 {
     asm volatile ( "wbinvd" ::: "memory" );


[PATCH 2/5] x86/ELF: don't open-code read_sreg()
Posted by Jan Beulich 3 years, 6 months ago
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/asm-x86/x86_64/elf.h
+++ b/xen/include/asm-x86/x86_64/elf.h
@@ -1,6 +1,8 @@
 #ifndef __X86_64_ELF_H__
 #define __X86_64_ELF_H__
 
+#include <asm/regs.h>
+
 typedef struct {
     unsigned long r15;
     unsigned long r14;
@@ -53,16 +55,16 @@ static inline void elf_core_save_regs(EL
     asm volatile("movq %%rdi,%0" : "=m"(core_regs->rdi));
     /* orig_rax not filled in for now */
     core_regs->rip = (unsigned long)elf_core_save_regs;
-    asm volatile("movl %%cs, %%eax;" :"=a"(core_regs->cs));
+    core_regs->cs = read_sreg(cs);
     asm volatile("pushfq; popq %0" :"=m"(core_regs->rflags));
     asm volatile("movq %%rsp,%0" : "=m"(core_regs->rsp));
     asm volatile("movl %%ss, %%eax;" :"=a"(core_regs->ss));
     /* thread_fs not filled in for now */
     /* thread_gs not filled in for now */
-    asm volatile("movl %%ds, %%eax;" :"=a"(core_regs->ds));
-    asm volatile("movl %%es, %%eax;" :"=a"(core_regs->es));
-    asm volatile("movl %%fs, %%eax;" :"=a"(core_regs->fs));
-    asm volatile("movl %%gs, %%eax;" :"=a"(core_regs->gs));
+    core_regs->ds = read_sreg(ds);
+    core_regs->es = read_sreg(es);
+    core_regs->fs = read_sreg(fs);
+    core_regs->gs = read_sreg(gs);
 
     asm volatile("mov %%cr0, %0" : "=r" (tmp) : );
     xen_core_regs->cr0 = tmp;

[PATCH 3/5] x86/ELF: don't store function pointer in elf_core_save_regs()
Posted by Jan Beulich 3 years, 6 months ago
This keeps at least gcc 10 from generating a separate function instance
in common/kexec.o alongside the inlining of the function in its sole
caller. I also think putting the address of the actual code storing the
registers is a better indication to consumers than that of an otherwise
unreferenced function.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/asm-x86/x86_64/elf.h
+++ b/xen/include/asm-x86/x86_64/elf.h
@@ -54,7 +54,7 @@ static inline void elf_core_save_regs(EL
     asm volatile("movq %%rsi,%0" : "=m"(core_regs->rsi));
     asm volatile("movq %%rdi,%0" : "=m"(core_regs->rdi));
     /* orig_rax not filled in for now */
-    core_regs->rip = (unsigned long)elf_core_save_regs;
+    asm volatile("call 0f; 0: popq %0" : "=m" (core_regs->rip));
     core_regs->cs = read_sreg(cs);
     asm volatile("pushfq; popq %0" :"=m"(core_regs->rflags));
     asm volatile("movq %%rsp,%0" : "=m"(core_regs->rsp));


[PATCH 4/5] x86/ELF: also record FS/GS bases in elf_core_save_regs()
Posted by Jan Beulich 3 years, 6 months ago
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/asm-x86/x86_64/elf.h
+++ b/xen/include/asm-x86/x86_64/elf.h
@@ -1,6 +1,7 @@
 #ifndef __X86_64_ELF_H__
 #define __X86_64_ELF_H__
 
+#include <asm/msr.h>
 #include <asm/regs.h>
 
 typedef struct {
@@ -59,8 +60,8 @@ static inline void elf_core_save_regs(EL
     asm volatile("pushfq; popq %0" :"=m"(core_regs->rflags));
     asm volatile("movq %%rsp,%0" : "=m"(core_regs->rsp));
     asm volatile("movl %%ss, %%eax;" :"=a"(core_regs->ss));
-    /* thread_fs not filled in for now */
-    /* thread_gs not filled in for now */
+    rdmsrl(MSR_FS_BASE, core_regs->thread_fs);
+    rdmsrl(MSR_GS_BASE, core_regs->thread_gs);
     core_regs->ds = read_sreg(ds);
     core_regs->es = read_sreg(es);
     core_regs->fs = read_sreg(fs);


[PATCH 5/5] x86/ELF: eliminate pointless local variable from elf_core_save_regs()
Posted by Jan Beulich 3 years, 6 months ago
We can just as well specify the CRn structure fields directly in the
asm()s, just like done for all other ones.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/asm-x86/x86_64/elf.h
+++ b/xen/include/asm-x86/x86_64/elf.h
@@ -37,8 +37,6 @@ typedef struct {
 static inline void elf_core_save_regs(ELF_Gregset *core_regs, 
                                       crash_xen_core_t *xen_core_regs)
 {
-    unsigned long tmp;
-
     asm volatile("movq %%r15,%0" : "=m"(core_regs->r15));
     asm volatile("movq %%r14,%0" : "=m"(core_regs->r14));
     asm volatile("movq %%r13,%0" : "=m"(core_regs->r13));
@@ -67,17 +65,10 @@ static inline void elf_core_save_regs(EL
     core_regs->fs = read_sreg(fs);
     core_regs->gs = read_sreg(gs);
 
-    asm volatile("mov %%cr0, %0" : "=r" (tmp) : );
-    xen_core_regs->cr0 = tmp;
-
-    asm volatile("mov %%cr2, %0" : "=r" (tmp) : );
-    xen_core_regs->cr2 = tmp;
-
-    asm volatile("mov %%cr3, %0" : "=r" (tmp) : );
-    xen_core_regs->cr3 = tmp;
-
-    asm volatile("mov %%cr4, %0" : "=r" (tmp) : );
-    xen_core_regs->cr4 = tmp;
+    asm volatile("mov %%cr0, %0" : "=r" (xen_core_regs->cr0));
+    asm volatile("mov %%cr2, %0" : "=r" (xen_core_regs->cr2));
+    asm volatile("mov %%cr3, %0" : "=r" (xen_core_regs->cr3));
+    asm volatile("mov %%cr4, %0" : "=r" (xen_core_regs->cr4));
 }
 
 #endif /* __X86_64_ELF_H__ */


[PATCH 6/5] x86/ELF: drop unnecessary volatile from asm()-s in elf_core_save_regs()
Posted by Jan Beulich 3 years, 6 months ago
There are no hidden side effects here.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/include/asm-x86/x86_64/elf.h
+++ b/xen/include/asm-x86/x86_64/elf.h
@@ -37,26 +37,26 @@ typedef struct {
 static inline void elf_core_save_regs(ELF_Gregset *core_regs, 
                                       crash_xen_core_t *xen_core_regs)
 {
-    asm volatile("movq %%r15,%0" : "=m"(core_regs->r15));
-    asm volatile("movq %%r14,%0" : "=m"(core_regs->r14));
-    asm volatile("movq %%r13,%0" : "=m"(core_regs->r13));
-    asm volatile("movq %%r12,%0" : "=m"(core_regs->r12));
-    asm volatile("movq %%rbp,%0" : "=m"(core_regs->rbp));
-    asm volatile("movq %%rbx,%0" : "=m"(core_regs->rbx));
-    asm volatile("movq %%r11,%0" : "=m"(core_regs->r11));
-    asm volatile("movq %%r10,%0" : "=m"(core_regs->r10));
-    asm volatile("movq %%r9,%0" : "=m"(core_regs->r9));
-    asm volatile("movq %%r8,%0" : "=m"(core_regs->r8));
-    asm volatile("movq %%rax,%0" : "=m"(core_regs->rax));
-    asm volatile("movq %%rcx,%0" : "=m"(core_regs->rcx));
-    asm volatile("movq %%rdx,%0" : "=m"(core_regs->rdx));
-    asm volatile("movq %%rsi,%0" : "=m"(core_regs->rsi));
-    asm volatile("movq %%rdi,%0" : "=m"(core_regs->rdi));
+    asm ( "movq %%r15,%0" : "=m" (core_regs->r15) );
+    asm ( "movq %%r14,%0" : "=m" (core_regs->r14) );
+    asm ( "movq %%r13,%0" : "=m" (core_regs->r13) );
+    asm ( "movq %%r12,%0" : "=m" (core_regs->r12) );
+    asm ( "movq %%rbp,%0" : "=m" (core_regs->rbp) );
+    asm ( "movq %%rbx,%0" : "=m" (core_regs->rbx) );
+    asm ( "movq %%r11,%0" : "=m" (core_regs->r11) );
+    asm ( "movq %%r10,%0" : "=m" (core_regs->r10) );
+    asm ( "movq %%r9,%0" : "=m" (core_regs->r9) );
+    asm ( "movq %%r8,%0" : "=m" (core_regs->r8) );
+    asm ( "movq %%rax,%0" : "=m" (core_regs->rax) );
+    asm ( "movq %%rcx,%0" : "=m" (core_regs->rcx) );
+    asm ( "movq %%rdx,%0" : "=m" (core_regs->rdx) );
+    asm ( "movq %%rsi,%0" : "=m" (core_regs->rsi) );
+    asm ( "movq %%rdi,%0" : "=m" (core_regs->rdi) );
     /* orig_rax not filled in for now */
     asm ( "call 0f; 0: popq %0" : "=m" (core_regs->rip) );
     core_regs->cs = read_sreg(cs);
-    asm volatile("pushfq; popq %0" :"=m"(core_regs->rflags));
-    asm volatile("movq %%rsp,%0" : "=m"(core_regs->rsp));
+    asm ( "pushfq; popq %0" : "=m" (core_regs->rflags) );
+    asm ( "movq %%rsp,%0" : "=m" (core_regs->rsp) );
     core_regs->ss = read_sreg(ss);
     rdmsrl(MSR_FS_BASE, core_regs->thread_fs);
     rdmsrl(MSR_GS_BASE, core_regs->thread_gs);