On 16.02.2024 11:50, Frediano Ziglio wrote:
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -22,6 +22,14 @@
> #endif
> .endm
>
> +.macro BUILD_BUG_ON condstr cond:vararg
> + .if \cond
> + .error "Condition \condstr not satisfied"
Maybe
.error "Condition \"\condstr\" not satisfied"
?
> @@ -187,7 +195,8 @@ FUNC_LOCAL(restore_all_guest)
> SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob:
> cd */
>
> RESTORE_ALL
> - testw $TRAP_syscall,4(%rsp)
> + BUILD_BUG_ON(TRAP_syscall & 0xff)
> + testb $TRAP_syscall >> 8,4+1(%rsp)
> jz iret_exit_to_guest
Nit: Blank after comma please (and again elsewhere).
Preferably with both adjustments (which I'd be okay making while
committing, so long as you agree specifically on the former)
Reviewed-by: Jan Beulich <jbeul...@suse.com>
That said, especially with this not being an entry point path, and
neither this nor ...
> @@ -254,7 +263,8 @@ FUNC(lstar_enter)
> pushq $FLAT_KERNEL_CS64
> pushq %rcx
> pushq $0
> - movl $TRAP_syscall, 4(%rsp)
> + BUILD_BUG_ON(TRAP_syscall & 0xff)
> + movb $TRAP_syscall >> 8, 4+1(%rsp)
> SAVE_ALL
>
> SPEC_CTRL_ENTRY_FROM_PV /* Req: %rsp=regs/cpuinfo, %rdx=0, Clob: acd
> */
> @@ -292,7 +302,8 @@ FUNC(cstar_enter)
> pushq $FLAT_USER_CS32
> pushq %rcx
> pushq $0
> - movl $TRAP_syscall, 4(%rsp)
> + BUILD_BUG_ON(TRAP_syscall & 0xff)
> + movb $TRAP_syscall >> 8, 4+1(%rsp)
> SAVE_ALL
>
> SPEC_CTRL_ENTRY_FROM_PV /* Req: %rsp=regs/cpuinfo, %rdx=0, Clob: acd
> */
> @@ -334,7 +345,8 @@ LABEL(sysenter_eflags_saved, 0)
> pushq $3 /* ring 3 null cs */
> pushq $0 /* null rip */
> pushq $0
> - movl $TRAP_syscall, 4(%rsp)
> + BUILD_BUG_ON(TRAP_syscall & 0xff)
> + movb $TRAP_syscall >> 8, 4+1(%rsp)
> SAVE_ALL
... any of these being exception entry point paths (and hence none of
these changes being related to the subject), it would likely have been
a good idea to split this into two patches.
Jan
