POP_GPRS is shorter than RESTORE_ALL in terms of emitted code.
By separating the popping of entry_vector/error_code off the stack,
SPEC_CTRL_COND_VERW doesn't need custom displacements.
Get rid of the compat=1 special case for PV32. It's not obviously a win, and
PV32 is getting increasingly rare these days.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Teddy Astie <teddy.astie@vates.tech>
Bloat-o-meter reports:
add/remove: 0/0 grow/shrink: 0/3 up/down: 0/-128 (-128)
Function old new delta
compat_restore_all_guest 187 158 -29
restore_all_guest 372 324 -48
restore_all_xen 165 114 -51
---
xen/arch/x86/include/asm/asm_defns.h | 50 +---------------------------
xen/arch/x86/x86_64/compat/entry.S | 9 +++--
xen/arch/x86/x86_64/entry.S | 12 +++----
3 files changed, 10 insertions(+), 61 deletions(-)
diff --git a/xen/arch/x86/include/asm/asm_defns.h b/xen/arch/x86/include/asm/asm_defns.h
index 87bd6b1193e7..d1b80186c14c 100644
--- a/xen/arch/x86/include/asm/asm_defns.h
+++ b/xen/arch/x86/include/asm/asm_defns.h
@@ -264,54 +264,6 @@ static always_inline void stac(void)
xor %r15d, %r15d
.endm
-#define LOAD_ONE_REG(reg, compat) \
-.if !(compat); \
- movq UREGS_r##reg(%rsp),%r##reg; \
-.else; \
- movl UREGS_r##reg(%rsp),%e##reg; \
-.endif
-
-/*
- * Restore all previously saved registers.
- *
- * @adj: extra stack pointer adjustment to be folded into the adjustment done
- * anyway at the end of the macro
- * @compat: R8-R15 don't need reloading, but they are clobbered for added
- * safety against information leaks.
- */
-.macro RESTORE_ALL adj=0, compat=0
-.if !\compat
- movq UREGS_r15(%rsp), %r15
- movq UREGS_r14(%rsp), %r14
- movq UREGS_r13(%rsp), %r13
- movq UREGS_r12(%rsp), %r12
-.else
- xor %r15d, %r15d
- xor %r14d, %r14d
- xor %r13d, %r13d
- xor %r12d, %r12d
-.endif
- LOAD_ONE_REG(bp, \compat)
- LOAD_ONE_REG(bx, \compat)
-.if !\compat
- movq UREGS_r11(%rsp),%r11
- movq UREGS_r10(%rsp),%r10
- movq UREGS_r9(%rsp),%r9
- movq UREGS_r8(%rsp),%r8
-.else
- xor %r11d, %r11d
- xor %r10d, %r10d
- xor %r9d, %r9d
- xor %r8d, %r8d
-.endif
- LOAD_ONE_REG(ax, \compat)
- LOAD_ONE_REG(cx, \compat)
- LOAD_ONE_REG(dx, \compat)
- LOAD_ONE_REG(si, \compat)
- LOAD_ONE_REG(di, \compat)
- subq $-(UREGS_error_code-UREGS_r15+\adj), %rsp
-.endm
-
/*
* Push and clear GPRs
*/
@@ -369,7 +321,7 @@ static always_inline void stac(void)
pop %r9
pop %r8
.if \skip_rax
- pop %rcx
+ pop %rcx /* Any register yet to restore. */
.else
pop %rax
.endif
diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index 39925d80a677..4bf4ee7c944c 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -160,12 +160,11 @@ FUNC(compat_restore_all_guest)
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
- RESTORE_ALL adj=8, compat=1
+ POP_GPRS
- /* Account for ev/ec having already been popped off the stack. */
- SPEC_CTRL_COND_VERW \
- scf=STK_REL(CPUINFO_scf, CPUINFO_rip), \
- sel=STK_REL(CPUINFO_verw_sel, CPUINFO_rip)
+ SPEC_CTRL_COND_VERW /* Req: %rsp=eframe Clob: efl */
+
+ add $8, %rsp /* Pop ev/ec off the stack */
jmp iret_to_guest
END(compat_restore_all_guest)
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index de5d854f5533..cd3532d7d174 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -226,7 +226,8 @@ FUNC_LOCAL(restore_all_guest)
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
- RESTORE_ALL
+ POP_GPRS
+
BUILD_BUG_ON(TRAP_syscall & 0xff)
testb $TRAP_syscall >> 8, EFRAME_entry_vector + 1(%rsp)
jz iret_exit_to_guest
@@ -753,20 +754,17 @@ UNLIKELY_END(exit_cr3)
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
SPEC_CTRL_EXIT_TO_XEN /* Req: %r12=ist_exit %r14=end %rsp=regs, Clob: abcd */
- RESTORE_ALL adj=8
+ POP_GPRS
/*
* When the CPU pushed this exception frame, it zero-extended eflags.
* For an IST exit, SPEC_CTRL_EXIT_TO_XEN stashed shadow copies of
* scf and ver_sel above eflags, as we can't use any GPRs,
* and we're at a random place on the stack, not in a CPUFINFO block.
- *
- * Account for ev/ec having already been popped off the stack.
*/
- SPEC_CTRL_COND_VERW \
- scf=STK_REL(EFRAME_shadow_scf, EFRAME_rip), \
- sel=STK_REL(EFRAME_shadow_sel, EFRAME_rip)
+ SPEC_CTRL_COND_VERW /* Req: %rsp=eframe Clob: efl */
+ add $8, %rsp /* Pop ev/ec off the stack */
iretq
END(restore_all_xen)
--
2.39.5
On 06.07.2026 17:34, Andrew Cooper wrote: > POP_GPRS is shorter than RESTORE_ALL in terms of emitted code. > > By separating the popping of entry_vector/error_code off the stack, > SPEC_CTRL_COND_VERW doesn't need custom displacements. > > Get rid of the compat=1 special case for PV32. It's not obviously a win, and > PV32 is getting increasingly rare these days. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> as-is, however ... > --- a/xen/arch/x86/x86_64/compat/entry.S > +++ b/xen/arch/x86/x86_64/compat/entry.S > @@ -160,12 +160,11 @@ FUNC(compat_restore_all_guest) > /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */ > SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */ > > - RESTORE_ALL adj=8, compat=1 > + POP_GPRS > > - /* Account for ev/ec having already been popped off the stack. */ > - SPEC_CTRL_COND_VERW \ > - scf=STK_REL(CPUINFO_scf, CPUINFO_rip), \ > - sel=STK_REL(CPUINFO_verw_sel, CPUINFO_rip) > + SPEC_CTRL_COND_VERW /* Req: %rsp=eframe Clob: efl */ > + > + add $8, %rsp /* Pop ev/ec off the stack */ > > jmp iret_to_guest > END(compat_restore_all_guest) > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -226,7 +226,8 @@ FUNC_LOCAL(restore_all_guest) > /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */ > SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */ > > - RESTORE_ALL > + POP_GPRS > + > BUILD_BUG_ON(TRAP_syscall & 0xff) > testb $TRAP_syscall >> 8, EFRAME_entry_vector + 1(%rsp) > jz iret_exit_to_guest > @@ -753,20 +754,17 @@ UNLIKELY_END(exit_cr3) > /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */ > SPEC_CTRL_EXIT_TO_XEN /* Req: %r12=ist_exit %r14=end %rsp=regs, Clob: abcd */ > > - RESTORE_ALL adj=8 > + POP_GPRS > > /* > * When the CPU pushed this exception frame, it zero-extended eflags. > * For an IST exit, SPEC_CTRL_EXIT_TO_XEN stashed shadow copies of > * scf and ver_sel above eflags, as we can't use any GPRs, > * and we're at a random place on the stack, not in a CPUFINFO block. > - * > - * Account for ev/ec having already been popped off the stack. > */ > - SPEC_CTRL_COND_VERW \ > - scf=STK_REL(EFRAME_shadow_scf, EFRAME_rip), \ > - sel=STK_REL(EFRAME_shadow_sel, EFRAME_rip) > + SPEC_CTRL_COND_VERW /* Req: %rsp=eframe Clob: efl */ > > + add $8, %rsp /* Pop ev/ec off the stack */ > iretq > END(restore_all_xen) ... both uses of SPEC_CTRL_COND_VERW's with arguments go away. Shouldn't the macro thus also be pruned of its parameters? Jan
On 07/07/2026 8:03 am, Jan Beulich wrote: > On 06.07.2026 17:34, Andrew Cooper wrote: >> POP_GPRS is shorter than RESTORE_ALL in terms of emitted code. >> >> By separating the popping of entry_vector/error_code off the stack, >> SPEC_CTRL_COND_VERW doesn't need custom displacements. >> >> Get rid of the compat=1 special case for PV32. It's not obviously a win, and >> PV32 is getting increasingly rare these days. >> >> No functional change. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> Thanks. > as-is, however ... > >> --- a/xen/arch/x86/x86_64/compat/entry.S >> +++ b/xen/arch/x86/x86_64/compat/entry.S >> @@ -160,12 +160,11 @@ FUNC(compat_restore_all_guest) >> /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */ >> SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */ >> >> - RESTORE_ALL adj=8, compat=1 >> + POP_GPRS >> >> - /* Account for ev/ec having already been popped off the stack. */ >> - SPEC_CTRL_COND_VERW \ >> - scf=STK_REL(CPUINFO_scf, CPUINFO_rip), \ >> - sel=STK_REL(CPUINFO_verw_sel, CPUINFO_rip) >> + SPEC_CTRL_COND_VERW /* Req: %rsp=eframe Clob: efl */ >> + >> + add $8, %rsp /* Pop ev/ec off the stack */ >> >> jmp iret_to_guest >> END(compat_restore_all_guest) >> --- a/xen/arch/x86/x86_64/entry.S >> +++ b/xen/arch/x86/x86_64/entry.S >> @@ -226,7 +226,8 @@ FUNC_LOCAL(restore_all_guest) >> /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */ >> SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */ >> >> - RESTORE_ALL >> + POP_GPRS >> + >> BUILD_BUG_ON(TRAP_syscall & 0xff) >> testb $TRAP_syscall >> 8, EFRAME_entry_vector + 1(%rsp) >> jz iret_exit_to_guest >> @@ -753,20 +754,17 @@ UNLIKELY_END(exit_cr3) >> /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */ >> SPEC_CTRL_EXIT_TO_XEN /* Req: %r12=ist_exit %r14=end %rsp=regs, Clob: abcd */ >> >> - RESTORE_ALL adj=8 >> + POP_GPRS >> >> /* >> * When the CPU pushed this exception frame, it zero-extended eflags. >> * For an IST exit, SPEC_CTRL_EXIT_TO_XEN stashed shadow copies of >> * scf and ver_sel above eflags, as we can't use any GPRs, >> * and we're at a random place on the stack, not in a CPUFINFO block. >> - * >> - * Account for ev/ec having already been popped off the stack. >> */ >> - SPEC_CTRL_COND_VERW \ >> - scf=STK_REL(EFRAME_shadow_scf, EFRAME_rip), \ >> - sel=STK_REL(EFRAME_shadow_sel, EFRAME_rip) >> + SPEC_CTRL_COND_VERW /* Req: %rsp=eframe Clob: efl */ >> >> + add $8, %rsp /* Pop ev/ec off the stack */ >> iretq >> END(restore_all_xen) > ... both uses of SPEC_CTRL_COND_VERW's with arguments go away. Shouldn't > the macro thus also be pruned of its parameters? Not in this patch. ~Andrew
© 2016 - 2026 Red Hat, Inc.