On 15.09.2023 17:00, Andrew Cooper wrote: > Use %r12 to hold an ist_exit boolean. This register is zero elsewhere in the > entry/exit asm, so it only needs setting in the IST path. > > As this is subtle and fragile, add check_ist_exit() to be used in debugging > builds to cross-check that the ist_exit boolean matches the entry vector. > > Write check_ist_exit() it in C, because it's debug only and the logic more > complicated than I care to maintain in asm. > > For now, we only need to use this signal in the exit-to-Xen path, but some > exit-to-guest paths happen in IST context too. Check the correctness in all > exit paths to avoid the logic bitrotting. > > Signed-off-by: Andrew Cooper <[email protected]>
Reviewed-by: Jan Beulich <[email protected]> I understand you then didn't like the idea of macro-izing ... > --- 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 > --- a/xen/arch/x86/x86_64/entry.S > +++ b/xen/arch/x86/x86_64/entry.S > @@ -142,8 +142,15 @@ process_trap: > > .section .text.entry, "ax", @progbits > > -/* %rbx: struct vcpu, interrupts disabled */ > +/* %rbx: struct vcpu, %r12: ist_exit, interrupts disabled */ > restore_all_guest: > + > +#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. */ > @@ -659,8 +666,15 @@ ENTRY(early_page_fault) > .section .text.entry, "ax", @progbits > > ALIGN > -/* No special register assumptions. */ > +/* %r12=ist_exit */ > restore_all_xen: > + > +#ifdef CONFIG_DEBUG > + mov %rsp, %rdi > + mov %r12, %rsi > + call check_ist_exit > +#endif ... these three instances of identical code you add, along the lines of ASSERT_INTERRUPTS_DISABLED? Jan
