On 08.08.2025 22:23, Andrew Cooper wrote: > FRED and IDT differ by a Supervisor Token on the base of the shstk. This > means that switch_stack_and_jump() needs to discard one extra word when FRED > is active. > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> > --- > CC: Jan Beulich <jbeul...@suse.com> > CC: Roger Pau Monné <roger....@citrix.com> > > RFC. I don't like this, but it does work. > > This emits opt_fred logic outside of CONFIG_XEN_SHSTK.
opt_fred and XEN_SHSTK are orthogonal, so that's fine anyway. What I guess you may mean is that you now have a shstk-related calculation outside of a respective #ifdef. Given the simplicity of the calculation, ... > But frankly, the > construct is already too unweildly, and all options I can think of make it > moreso. ... I agree having it like this is okay. > @@ -154,7 +155,6 @@ unsigned long get_stack_dump_bottom (unsigned long sp); > "rdsspd %[ssp];" \ > "cmp $1, %[ssp];" \ > "je .L_shstk_done.%=;" /* CET not active? Skip. */ \ > - "mov $%c[skstk_base], %[val];" \ > "and $%c[stack_mask], %[ssp];" \ > "sub %[ssp], %[val];" \ > "shr $3, %[val];" \ With the latter two insns here, ... > @@ -177,6 +177,8 @@ unsigned long get_stack_dump_bottom (unsigned long sp); > > #define switch_stack_and_jump(fn, instr, constr) \ > ({ \ > + unsigned int token_offset = \ > + (PRIMARY_SHSTK_SLOT + 1) * PAGE_SIZE - (opt_fred ? 0 : 8); \ > unsigned int tmp; \ > BUILD_BUG_ON(!ssaj_has_attr_noreturn(fn)); \ > __asm__ __volatile__ ( \ > @@ -184,12 +186,11 @@ unsigned long get_stack_dump_bottom (unsigned long sp); > "mov %[stk], %%rsp;" \ > CHECK_FOR_LIVEPATCH_WORK \ > instr "[fun]" \ > - : [val] "=&r" (tmp), \ > + : [val] "=r" (tmp), \ ... I don't think you can legitimately drop the & from here? With it retained: Reviewed-by: Jan Beulich <jbeul...@suse.com> Jan