PUSH_AND_CLEAR_GPRS is shorter than SAVE_ALL in terms of emitted code.
lstar_enter() and cstar_enter() do not need CLD. The SYSCALL instruction
sanitises flags based on MSR_SYSCALL_MASK. For all other cases, place the CLD
instruction next to STAC/CLAC so the flag handling is together.
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/12 up/down: 0/-518 (-518)
Function old new delta
symbols_names 135277 135276 -1
symbols_offsets 53664 53660 -4
entry_int82 229 213 -16
sysenter_entry 383 328 -55
handle_ist_exception 450 395 -55
handle_exception 683 628 -55
entry_int80 432 377 -55
entry_DF 294 239 -55
early_page_fault 139 84 -55
common_interrupt 327 272 -55
lstar_enter 322 266 -56
cstar_enter 323 267 -56
---
xen/arch/x86/include/asm/asm_defns.h | 45 ----------------------------
xen/arch/x86/x86_64/compat/entry.S | 3 +-
xen/arch/x86/x86_64/entry.S | 25 ++++++++++------
3 files changed, 18 insertions(+), 55 deletions(-)
diff --git a/xen/arch/x86/include/asm/asm_defns.h b/xen/arch/x86/include/asm/asm_defns.h
index d1b80186c14c..6c65acd08366 100644
--- a/xen/arch/x86/include/asm/asm_defns.h
+++ b/xen/arch/x86/include/asm/asm_defns.h
@@ -219,51 +219,6 @@ static always_inline void stac(void)
#endif
#ifdef __ASSEMBLER__
-.macro SAVE_ALL compat=0
- addq $-(UREGS_error_code-UREGS_r15), %rsp
- cld
- movq %rdi,UREGS_rdi(%rsp)
- xor %edi, %edi
- movq %rsi,UREGS_rsi(%rsp)
- xor %esi, %esi
- movq %rdx,UREGS_rdx(%rsp)
- xor %edx, %edx
- movq %rcx,UREGS_rcx(%rsp)
- xor %ecx, %ecx
- movq %rax,UREGS_rax(%rsp)
- xor %eax, %eax
-.if !\compat
- movq %r8,UREGS_r8(%rsp)
- movq %r9,UREGS_r9(%rsp)
- movq %r10,UREGS_r10(%rsp)
- movq %r11,UREGS_r11(%rsp)
-.endif
- xor %r8d, %r8d
- xor %r9d, %r9d
- xor %r10d, %r10d
- xor %r11d, %r11d
- movq %rbx,UREGS_rbx(%rsp)
- xor %ebx, %ebx
- movq %rbp,UREGS_rbp(%rsp)
-#ifdef CONFIG_FRAME_POINTER
-/* Indicate special exception stack frame by inverting the frame pointer. */
- leaq UREGS_rbp(%rsp), %rbp
- notq %rbp
-#else
- xor %ebp, %ebp
-#endif
-.if !\compat
- movq %r12,UREGS_r12(%rsp)
- movq %r13,UREGS_r13(%rsp)
- movq %r14,UREGS_r14(%rsp)
- movq %r15,UREGS_r15(%rsp)
-.endif
- xor %r12d, %r12d
- xor %r13d, %r13d
- xor %r14d, %r14d
- xor %r15d, %r15d
-.endm
-
/*
* Push and clear GPRs
*/
diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index 4bf4ee7c944c..2503a973ffc2 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -13,10 +13,11 @@
FUNC(entry_int82)
ENDBR64
+ cld
ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
pushq $0
movb $HYPERCALL_VECTOR, EFRAME_entry_vector(%rsp)
- SAVE_ALL compat=1 /* DPL1 gate, restricted to 32bit PV guests only. */
+ PUSH_AND_CLEAR_GPRS
GET_STACK_END(14)
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index cd3532d7d174..22729b1f43b8 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -305,7 +305,7 @@ FUNC(lstar_enter)
pushq $0
BUILD_BUG_ON(TRAP_syscall & 0xff)
movb $TRAP_syscall >> 8, EFRAME_entry_vector + 1(%rsp)
- SAVE_ALL
+ PUSH_AND_CLEAR_GPRS
GET_STACK_END(14)
@@ -345,7 +345,7 @@ FUNC(cstar_enter)
pushq $0
BUILD_BUG_ON(TRAP_syscall & 0xff)
movb $TRAP_syscall >> 8, EFRAME_entry_vector + 1(%rsp)
- SAVE_ALL
+ PUSH_AND_CLEAR_GPRS
GET_STACK_END(14)
@@ -383,13 +383,14 @@ FUNC(sysenter_entry)
pushq $0
pushfq
LABEL(sysenter_eflags_saved, 0)
+ cld
ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
pushq $3 /* ring 3 null cs */
pushq $0 /* null rip */
pushq $0
BUILD_BUG_ON(TRAP_syscall & 0xff)
movb $TRAP_syscall >> 8, EFRAME_entry_vector + 1(%rsp)
- SAVE_ALL
+ PUSH_AND_CLEAR_GPRS
GET_STACK_END(14)
@@ -442,10 +443,11 @@ END(sysenter_entry)
FUNC(entry_int80)
ENDBR64
+ cld
ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
pushq $0
movb $0x80, EFRAME_entry_vector(%rsp)
- SAVE_ALL
+ PUSH_AND_CLEAR_GPRS
GET_STACK_END(14)
@@ -722,8 +724,9 @@ END(ret_from_intr)
.section .init.text, "ax", @progbits
FUNC(early_page_fault)
ENDBR64
+ cld
movb $X86_EXC_PF, EFRAME_entry_vector(%rsp)
- SAVE_ALL
+ PUSH_AND_CLEAR_GPRS
movq %rsp, %rdi
call do_early_page_fault
jmp restore_all_xen
@@ -769,8 +772,9 @@ UNLIKELY_END(exit_cr3)
END(restore_all_xen)
FUNC(common_interrupt)
+ cld
ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
- SAVE_ALL
+ PUSH_AND_CLEAR_GPRS
GET_STACK_END(14)
@@ -805,8 +809,9 @@ FUNC(entry_PF)
END(entry_PF)
/* No special register assumptions. */
FUNC(handle_exception, 0)
+ cld
ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
- SAVE_ALL
+ PUSH_AND_CLEAR_GPRS
GET_STACK_END(14)
@@ -1103,9 +1108,10 @@ END(entry_CP)
FUNC(entry_DF)
ENDBR64
movb $X86_EXC_DF, EFRAME_entry_vector(%rsp)
+ cld
/* Set AC to reduce chance of further SMAP faults */
ALTERNATIVE "", stac, X86_FEATURE_XEN_SMAP
- SAVE_ALL
+ PUSH_AND_CLEAR_GPRS
GET_STACK_END(14)
@@ -1132,8 +1138,9 @@ FUNC(entry_NMI)
END(entry_NMI)
FUNC(handle_ist_exception)
+ cld
ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
- SAVE_ALL
+ PUSH_AND_CLEAR_GPRS
GET_STACK_END(14)
--
2.39.5
On 06.07.2026 17:34, Andrew Cooper wrote:
> PUSH_AND_CLEAR_GPRS is shorter than SAVE_ALL in terms of emitted code.
Could patch 1 perhaps also gain this sentence?
> lstar_enter() and cstar_enter() do not need CLD. The SYSCALL instruction
> sanitises flags based on MSR_SYSCALL_MASK. For all other cases, place the CLD
> instruction next to STAC/CLAC so the flag handling is together.
And really the earlier the better. Relevant perhaps for early_page_fault(),
which has no CLAC.
> 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>
> Bloat-o-meter reports:
> add/remove: 0/0 grow/shrink: 0/12 up/down: 0/-518 (-518)
> Function old new delta
> symbols_names 135277 135276 -1
> symbols_offsets 53664 53660 -4
Interesting. Likely an artifact of padding going away between two functions.
Which likely would change again if the SLS patches would finally land
("x86: guard against straight-line speculation past JMP or RET in assembly
files" here in particular).
Btw, as it occurs to me while mentioning SLS: Judging from the patches I
have, IRET looks to be immune, albeit I can't find any statement in the doc.
What about ERET{S,U}, uses of which we've gained only relatively recently?
Jan
On 07/07/2026 8:16 am, Jan Beulich wrote:
> On 06.07.2026 17:34, Andrew Cooper wrote:
>> PUSH_AND_CLEAR_GPRS is shorter than SAVE_ALL in terms of emitted code.
> Could patch 1 perhaps also gain this sentence?
Done.
>
>> lstar_enter() and cstar_enter() do not need CLD. The SYSCALL instruction
>> sanitises flags based on MSR_SYSCALL_MASK. For all other cases, place the CLD
>> instruction next to STAC/CLAC so the flag handling is together.
> And really the earlier the better. Relevant perhaps for early_page_fault(),
> which has no CLAC.
>
>> 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.
>
>> Bloat-o-meter reports:
>> add/remove: 0/0 grow/shrink: 0/12 up/down: 0/-518 (-518)
>> Function old new delta
>> symbols_names 135277 135276 -1
>> symbols_offsets 53664 53660 -4
> Interesting. Likely an artifact of padding going away between two functions.
> Which likely would change again if the SLS patches would finally land
> ("x86: guard against straight-line speculation past JMP or RET in assembly
> files" here in particular).
>
> Btw, as it occurs to me while mentioning SLS: Judging from the patches I
> have, IRET looks to be immune, albeit I can't find any statement in the doc.
> What about ERET{S,U}, uses of which we've gained only relatively recently?
I think the bit you want is in SDM Vol2. For ERETS:
"Instruction ordering. Instructions following execution of ERETS may be
fetched from memory before earlier instructions complete execution, but
they will not execute (even speculatively) until all instructions prior
to ERETS have completed execution (the later instructions may execute
before data stored by the earlier instructions have become globally
visible)."
and says the same for ERETU. Sadly, "following" is ambiguous in the
context of SLS, but an ERETS-to-self could end up legitimately executing
the next sequential instruction. Either way, the "not execute even
speculatively" is the property wanted.
IRET notes that it's architecturally serialising, which is a much
stronger guarantee.
~Andrew
On 07.07.2026 18:09, Andrew Cooper wrote:
> On 07/07/2026 8:16 am, Jan Beulich wrote:
>> Btw, as it occurs to me while mentioning SLS: Judging from the patches I
>> have, IRET looks to be immune, albeit I can't find any statement in the doc.
>> What about ERET{S,U}, uses of which we've gained only relatively recently?
> I think the bit you want is in SDM Vol2. For ERETS:
>
> "Instruction ordering. Instructions following execution of ERETS may be
> fetched from memory before earlier instructions complete execution, but
> they will not execute (even speculatively) until all instructions prior
> to ERETS have completed execution (the later instructions may execute
> before data stored by the earlier instructions have become globally
> visible)."
>
> and says the same for ERETU. Sadly, "following" is ambiguous in the
> context of SLS, but an ERETS-to-self could end up legitimately executing
> the next sequential instruction. Either way, the "not execute even
> speculatively" is the property wanted.
There's a worse ambiguity here, imo: What they say excludes ERETS itself.
I.e. while ERETS itself executes, subsequent instruction may already be
speculated.
Interestingly the FRED spec (9.0) says such only for ERETU, not ERETS.
Jan
© 2016 - 2026 Red Hat, Inc.