On 13.09.2023 22:27, Andrew Cooper wrote: > --- a/xen/arch/x86/x86_64/compat/entry.S > +++ b/xen/arch/x86/x86_64/compat/entry.S > @@ -117,8 +117,15 @@ compat_process_trap: > call compat_create_bounce_frame > jmp compat_test_all_events > > -/* %rbx: struct vcpu, interrupts disabled */ > +/* %rbx: struct vcpu, %r12: ist_exit, interrupts disabled */ > ENTRY(compat_restore_all_guest) > + > +#ifdef CONFIG_DEBUG > + mov %rsp, %rdi > + mov %r12, %rsi > + call check_ist_exit > +#endif > + > ASSERT_INTERRUPTS_DISABLED > mov $~(X86_EFLAGS_IOPL | X86_EFLAGS_VM), %r11d > and UREGS_eflags(%rsp),%r11d
Without having peeked ahead, is there any use of %r12 going to appear on this path? I thought it's only going to be restore_all_xen? > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -142,10 +142,16 @@ process_trap: > > .section .text.entry, "ax", @progbits > > -/* %rbx: struct vcpu, interrupts disabled */ > +/* %rbx: struct vcpu, %r12: ist_exit, interrupts disabled */ > restore_all_guest: > - ASSERT_INTERRUPTS_DISABLED > > +#ifdef CONFIG_DEBUG > + mov %rsp, %rdi > + mov %r12, %rsi > + call check_ist_exit > +#endif > + > + ASSERT_INTERRUPTS_DISABLED > /* Stash guest SPEC_CTRL value while we can read struct vcpu. */ > mov VCPU_arch_msrs(%rbx), %rdx > mov VCPUMSR_spec_ctrl_raw(%rdx), %r15d Even here I don't think I see a need for the addition. Plus if the check is warranted here, is it really necessary for it to live ahead of the interrupts-disabled check? Further, seeing that being marco-ized, would it make sense to have a CHECK_IST_EXIT macro in case more than a single use site remains? > @@ -1087,6 +1100,10 @@ handle_ist_exception: > .L_ist_dispatch_done: > mov %r15, STACK_CPUINFO_FIELD(xen_cr3)(%r14) > mov %bl, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14) > + > + /* This is an IST exit */ > + mov $1, %r12 > + > cmpb $X86_EXC_NMI, UREGS_entry_vector(%rsp) > jne ret_from_intr Could I talk you into using a less than 7-byte insn here? "or $1, %r12" would be 4 bytes, "mov $1, %r12b", "inc %r12", and "not %r12" would be just 3. All have certain downsides, yes, but I wonder whether switching isn't worth it. Even "mov $1, %r12d" would be at least one byte less, without any downsides. And the OR and INC variants would allow the remaining 63 bits to be used for another purpose down the road. Jan
